Merge branch 'id-ssh-certificates' into 'main'

Support authentication using SSH Certificates

See merge request https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/812

Merged-by: Ash McKenzie <[email protected]>
Approved-by: Alejandro Rodríguez <[email protected]>
Approved-by: Jessie Young <[email protected]>
Approved-by: Ash McKenzie <[email protected]>
Reviewed-by: Igor Drozdov <[email protected]>
Reviewed-by: Ash McKenzie <[email protected]>
Reviewed-by: Jessie Young <[email protected]>
Co-authored-by: Igor Drozdov <[email protected]>

Author: ashmckenzie

cmd/gitlab-shell/command/command.go

@@ -58,6 +58,20 @@ func NewWithKrb5Principal(gitlabKrb5Principal string, env sshenv.Env, config *co
 	return nil, disallowedcommand.Error
 }
 
+func NewWithUsername(gitlabUsername string, env sshenv.Env, config *config.Config, readWriter *readwriter.ReadWriter) (command.Command, error) {
+	args, err := Parse(nil, env)
+	if err != nil {
+		return nil, err
+	}
+
+	args.GitlabUsername = gitlabUsername
+	if cmd := Build(args, config, readWriter); cmd != nil {
+		return cmd, nil
+	}
+
+	return nil, disallowedcommand.Error
+}
+
 func Parse(arguments []string, env sshenv.Env) (*commandargs.Shell, error) {
 	args := &commandargs.Shell{Arguments: arguments, Env: env}
 

cmd/gitlab-shell/command/command_test.go

@@ -300,3 +300,16 @@ func TestParseFailure(t *testing.T) {
 		})
 	}
 }
+
+func TestNewWithUsername(t *testing.T) {
+	env := sshenv.Env{IsSSHConnection: true, OriginalCommand: "git-receive-pack 'group/repo'"}
+	c, err := cmd.NewWithUsername("username", env, nil, nil)
+	require.NoError(t, err)
+	require.IsType(t, &receivepack.Command{}, c)
+	require.Equal(t, c.(*receivepack.Command).Args.GitlabUsername, "username")
+
+	env = sshenv.Env{IsSSHConnection: true, OriginalCommand: "invalid"}
+	c, err = cmd.NewWithUsername("username", env, nil, nil)
+	require.Error(t, err)
+	require.Nil(t, c)
+}

internal/gitlabnet/accessverifier/client.go

@@ -30,6 +30,9 @@ type Request struct {
 	Username      string                  `json:"username,omitempty"`
 	Krb5Principal string                  `json:"krb5principal,omitempty"`
 	CheckIp       string                  `json:"check_ip,omitempty"`
+	// NamespacePath is the full path of the namespace in which the authenticated
+	// user is allowed to perform operation.
+	NamespacePath string `json:"namespace_path,omitempty"`
 }
 
 type Gitaly struct {
@@ -81,7 +84,13 @@ func NewClient(config *config.Config) (*Client, error) {
 }
 
 func (c *Client) Verify(ctx context.Context, args *commandargs.Shell, action commandargs.CommandType, repo string) (*Response, error) {
-	request := &Request{Action: action, Repo: repo, Protocol: protocol, Changes: anyChanges}
+	request := &Request{
+		Action:        action,
+		Repo:          repo,
+		Protocol:      protocol,
+		Changes:       anyChanges,
+		NamespacePath: args.Env.NamespacePath,
+	}
 
 	if args.GitlabUsername != "" {
 		request.Username = args.GitlabUsername

internal/gitlabnet/accessverifier/client_test.go

@@ -19,9 +19,11 @@ import (
 )
 
 var (
+	namespace         = "group"
 	repo              = "group/private"
 	receivePackAction = commandargs.ReceivePack
 	uploadPackAction  = commandargs.UploadPack
+	defaultEnv        = sshenv.Env{NamespacePath: namespace}
 )
 
 func buildExpectedResponse(who string) *Response {
@@ -71,7 +73,7 @@ func TestSuccessfulResponses(t *testing.T) {
 			who:  "key-1",
 		}, {
 			desc: "Provide username within the request",
-			args: &commandargs.Shell{GitlabUsername: "first"},
+			args: &commandargs.Shell{GitlabUsername: "first", Env: defaultEnv},
 			who:  "user-1",
 		}, {
 			desc: "Provide krb5principal within the request",
@@ -99,7 +101,7 @@ func TestGeoPushGetCustomAction(t *testing.T) {
 		},
 	}, nil)
 
-	args := &commandargs.Shell{GitlabUsername: "custom"}
+	args := &commandargs.Shell{GitlabUsername: "custom", Env: defaultEnv}
 	result, err := client.Verify(context.Background(), args, receivePackAction, repo)
 	require.NoError(t, err)
 
@@ -128,7 +130,7 @@ func TestGeoPullGetCustomAction(t *testing.T) {
 		},
 	}, nil)
 
-	args := &commandargs.Shell{GitlabUsername: "custom"}
+	args := &commandargs.Shell{GitlabUsername: "custom", Env: defaultEnv}
 	result, err := client.Verify(context.Background(), args, uploadPackAction, repo)
 	require.NoError(t, err)
 
@@ -263,6 +265,7 @@ func setup(t *testing.T, userResponses, keyResponses map[string]testResponse) *C
 					w.WriteHeader(tr.status)
 					_, err := w.Write(tr.body)
 					require.NoError(t, err)
+					require.Equal(t, namespace, requestBody.NamespacePath)
 				} else if tr, ok := userResponses[requestBody.Krb5Principal]; ok {
 					w.WriteHeader(tr.status)
 					_, err := w.Write(tr.body)

internal/gitlabnet/authorizedcerts/client.go

@@ -0,0 +1,68 @@
+package authorizedcerts
+
+import (
+	"context"
+	"fmt"
+	"net/url"
+
+	"gitlab.com/gitlab-org/gitlab-shell/v14/client"
+	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/config"
+	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/gitlabnet"
+)
+
+const (
+	AuthorizedCertsPath = "/authorized_certs"
+)
+
+type Client struct {
+	config *config.Config
+	client *client.GitlabNetClient
+}
+
+type Response struct {
+	Username  string `json:"username"`
+	Namespace string `json:"namespace"`
+}
+
+func NewClient(config *config.Config) (*Client, error) {
+	client, err := gitlabnet.GetClient(config)
+	if err != nil {
+		return nil, fmt.Errorf("Error creating http client: %v", err)
+	}
+
+	return &Client{config: config, client: client}, nil
+}
+
+func (c *Client) GetByKey(ctx context.Context, userId, fingerprint string) (*Response, error) {
+	path, err := pathWithKey(userId, fingerprint)
+	if err != nil {
+		return nil, err
+	}
+
+	response, err := c.client.Get(ctx, path)
+	if err != nil {
+		return nil, err
+	}
+	defer response.Body.Close()
+
+	parsedResponse := &Response{}
+	if err := gitlabnet.ParseJSON(response, parsedResponse); err != nil {
+		return nil, err
+	}
+
+	return parsedResponse, nil
+}
+
+func pathWithKey(userId, fingerprint string) (string, error) {
+	u, err := url.Parse(AuthorizedCertsPath)
+	if err != nil {
+		return "", err
+	}
+
+	params := u.Query()
+	params.Set("key", fingerprint)
+	params.Set("user_identifier", userId)
+	u.RawQuery = params.Encode()
+
+	return u.String(), nil
+}

internal/gitlabnet/authorizedcerts/client_test.go

@@ -0,0 +1,103 @@
+package authorizedcerts
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitlab-shell/v14/client"
+	"gitlab.com/gitlab-org/gitlab-shell/v14/client/testserver"
+	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/config"
+)
+
+var (
+	requests []testserver.TestRequestHandler
+)
+
+func init() {
+	requests = []testserver.TestRequestHandler{
+		{
+			Path: "/api/v4/internal/authorized_certs",
+			Handler: func(w http.ResponseWriter, r *http.Request) {
+				if r.URL.Query().Get("key") == "key" {
+					body := &Response{
+						Namespace: "group",
+						Username:  r.URL.Query().Get("user_identifier"),
+					}
+					json.NewEncoder(w).Encode(body)
+				} else if r.URL.Query().Get("key") == "broken-message" {
+					w.WriteHeader(http.StatusForbidden)
+					body := &client.ErrorResponse{
+						Message: "Not allowed!",
+					}
+					json.NewEncoder(w).Encode(body)
+				} else if r.URL.Query().Get("key") == "broken-json" {
+					w.Write([]byte("{ \"message\": \"broken json!\""))
+				} else if r.URL.Query().Get("key") == "broken-empty" {
+					w.WriteHeader(http.StatusForbidden)
+				} else {
+					w.WriteHeader(http.StatusNotFound)
+				}
+			},
+		},
+	}
+}
+
+func TestGetByKey(t *testing.T) {
+	client := setup(t)
+
+	result, err := client.GetByKey(context.Background(), "user-id", "key")
+	require.NoError(t, err)
+	require.Equal(t, &Response{Namespace: "group", Username: "user-id"}, result)
+}
+
+func TestGetByKeyErrorResponses(t *testing.T) {
+	client := setup(t)
+
+	testCases := []struct {
+		desc          string
+		key           string
+		expectedError string
+	}{
+		{
+			desc:          "A response with an error message",
+			key:           "broken-message",
+			expectedError: "Not allowed!",
+		},
+		{
+			desc:          "A response with bad JSON",
+			key:           "broken-json",
+			expectedError: "Parsing failed",
+		},
+		{
+			desc:          "A forbidden (403) response without message",
+			key:           "broken-empty",
+			expectedError: "Internal API error (403)",
+		},
+		{
+			desc:          "A not found (404) response without message",
+			key:           "not-found",
+			expectedError: "Internal API error (404)",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			resp, err := client.GetByKey(context.Background(), "user-id", tc.key)
+
+			require.EqualError(t, err, tc.expectedError)
+			require.Nil(t, resp)
+		})
+	}
+}
+
+func setup(t *testing.T) *Client {
+	url := testserver.StartSocketHttpServer(t, requests)
+
+	client, err := NewClient(&config.Config{GitlabUrl: url})
+	require.NoError(t, err)
+
+	return client
+}