Merge branch '181-authorized-keys-check-go' into 'master'

Implement AuthorizedKeys command

See merge request gitlab-org/gitlab-shell!321

go/internal/command/authorizedkeys/authorized_keys.go

+package authorizedkeys
+
+import (
+	"fmt"
+	"strconv"
+
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/commandargs"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/readwriter"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/config"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/gitlabnet/authorizedkeys"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/keyline"
+)
+
+type Command struct {
+	Config     *config.Config
+	Args       *commandargs.AuthorizedKeys
+	ReadWriter *readwriter.ReadWriter
+}
+
+func (c *Command) Execute() error {
+	// Do and return nothing when the expected and actual user don't match.
+	// This can happen when the user in sshd_config doesn't match the user
+	// trying to login. When nothing is printed, the user will be denied access.
+	if c.Args.ExpectedUser != c.Args.ActualUser {
+		// TODO: Log this event once we have a consistent way to log in Go.
+		// See https://gitlab.com/gitlab-org/gitlab-shell/issues/192 for more info.
+		return nil
+	}
+
+	if err := c.printKeyLine(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (c *Command) printKeyLine() error {
+	response, err := c.getAuthorizedKey()
+	if err != nil {
+		fmt.Fprintln(c.ReadWriter.Out, fmt.Sprintf("# No key was found for %s", c.Args.Key))
+		return nil
+	}
+
+	keyLine, err := keyline.NewPublicKeyLine(strconv.FormatInt(response.Id, 10), response.Key, c.Config.RootDir)
+	if err != nil {
+		return err
+	}
+
+	fmt.Fprintln(c.ReadWriter.Out, keyLine.ToString())
+
+	return nil
+}
+
+func (c *Command) getAuthorizedKey() (*authorizedkeys.Response, error) {
+	client, err := authorizedkeys.NewClient(c.Config)
+	if err != nil {
+		return nil, err
+	}
+
+	return client.GetByKey(c.Args.Key)
+}

go/internal/command/authorizedkeys/authorized_keys_test.go

+package authorizedkeys
+
+import (
+	"bytes"
+	"encoding/json"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/commandargs"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/readwriter"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/config"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/gitlabnet/testserver"
+)
+
+var (
+	requests = []testserver.TestRequestHandler{
+		{
+			Path: "/api/v4/internal/authorized_keys",
+			Handler: func(w http.ResponseWriter, r *http.Request) {
+				if r.URL.Query().Get("key") == "key" {
+					body := map[string]interface{}{
+						"id":  1,
+						"key": "public-key",
+					}
+					json.NewEncoder(w).Encode(body)
+				} else if r.URL.Query().Get("key") == "broken-message" {
+					body := map[string]string{
+						"message": "Forbidden!",
+					}
+					w.WriteHeader(http.StatusForbidden)
+					json.NewEncoder(w).Encode(body)
+				} else if r.URL.Query().Get("key") == "broken" {
+					w.WriteHeader(http.StatusInternalServerError)
+				} else {
+					w.WriteHeader(http.StatusNotFound)
+				}
+			},
+		},
+	}
+)
+
+func TestExecute(t *testing.T) {
+	url, cleanup := testserver.StartSocketHttpServer(t, requests)
+	defer cleanup()
+
+	testCases := []struct {
+		desc           string
+		arguments      *commandargs.AuthorizedKeys
+		expectedOutput string
+	}{
+		{
+			desc:           "With matching username and key",
+			arguments:      &commandargs.AuthorizedKeys{ExpectedUser: "user", ActualUser: "user", Key: "key"},
+			expectedOutput: "command=\"/tmp/bin/gitlab-shell key-1\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty public-key\n",
+		},
+		{
+			desc:           "When key doesn't match any existing key",
+			arguments:      &commandargs.AuthorizedKeys{ExpectedUser: "user", ActualUser: "user", Key: "not-found"},
+			expectedOutput: "# No key was found for not-found\n",
+		},
+		{
+			desc:           "When the API returns an error",
+			arguments:      &commandargs.AuthorizedKeys{ExpectedUser: "user", ActualUser: "user", Key: "broken-message"},
+			expectedOutput: "# No key was found for broken-message\n",
+		},
+		{
+			desc:           "When the API fails",
+			arguments:      &commandargs.AuthorizedKeys{ExpectedUser: "user", ActualUser: "user", Key: "broken"},
+			expectedOutput: "# No key was found for broken\n",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			buffer := &bytes.Buffer{}
+			cmd := &Command{
+				Config:     &config.Config{RootDir: "/tmp", GitlabUrl: url},
+				Args:       tc.arguments,
+				ReadWriter: &readwriter.ReadWriter{Out: buffer},
+			}
+
+			err := cmd.Execute()
+
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedOutput, buffer.String())
+		})
+	}
+}

go/internal/command/command.go

 package command
 
 import (
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/authorizedkeys"
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/commandargs"
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/discover"
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/fallback"
@@ -35,6 +36,8 @@ func buildCommand(e *executable.Executable, args commandargs.CommandArgs, config
 	switch e.Name {
 	case executable.GitlabShell:
 		return buildShellCommand(args.(*commandargs.Shell), config, readWriter)
+	case executable.AuthorizedKeysCheck:
+		return buildAuthorizedKeysCommand(args.(*commandargs.AuthorizedKeys), config, readWriter)
 	}
 
 	return nil
@@ -62,3 +65,11 @@ func buildShellCommand(args *commandargs.Shell, config *config.Config, readWrite
 
 	return nil
 }
+
+func buildAuthorizedKeysCommand(args *commandargs.AuthorizedKeys, config *config.Config, readWriter *readwriter.ReadWriter) Command {
+	if !config.FeatureEnabled(executable.AuthorizedKeysCheck) {
+		return nil
+	}
+
+	return &authorizedkeys.Command{Config: config, Args: args, ReadWriter: readWriter}
+}

go/internal/command/command_test.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/authorizedkeys"
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/discover"
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/fallback"
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/lfsauthenticate"
@@ -124,6 +125,16 @@ func TestNew(t *testing.T) {
 			arguments:    []string{},
 			expectedType: &uploadarchive.Command{},
 		},
+		{
+			desc:       "it returns a AuthorizedKeys command if the feature is enabled",
+			executable: &executable.Executable{Name: executable.AuthorizedKeysCheck},
+			config: &config.Config{
+				Migration: config.MigrationConfig{Enabled: true, Features: []string{"gitlab-shell-authorized-keys-check"}},
+			},
+			environment:  map[string]string{},
+			arguments:    []string{"git", "git", "key"},
+			expectedType: &authorizedkeys.Command{},
+		},
 		{
 			desc:       "it returns a Fallback command if the feature is unimplemented",
 			executable: &executable.Executable{Name: executable.GitlabShell},

go/internal/command/commandargs/authorized_keys.go

+package commandargs
+
+import (
+	"errors"
+	"fmt"
+)
+
+type AuthorizedKeys struct {
+	Arguments    []string
+	ExpectedUser string
+	ActualUser   string
+	Key          string
+}
+
+func (ak *AuthorizedKeys) Parse() error {
+	if err := ak.validate(); err != nil {
+		return err
+	}
+
+	ak.ExpectedUser = ak.Arguments[0]
+	ak.ActualUser = ak.Arguments[1]
+	ak.Key = ak.Arguments[2]
+
+	return nil
+}
+
+func (ak *AuthorizedKeys) GetArguments() []string {
+	return ak.Arguments
+}
+
+func (ak *AuthorizedKeys) validate() error {
+	argsSize := len(ak.Arguments)
+
+	if argsSize != 3 {
+		return errors.New(fmt.Sprintf("# Insufficient arguments. %d. Usage\n#\tgitlab-shell-authorized-keys-check <expected-username> <actual-username> <key>", argsSize))
+	}
+
+	expectedUsername := ak.Arguments[0]
+	actualUsername := ak.Arguments[1]
+	key := ak.Arguments[2]
+
+	if expectedUsername == "" || actualUsername == "" {
+		return errors.New("# No username provided")
+	}
+
+	if key == "" {
+		return errors.New("# No key provided")
+	}
+
+	return nil
+}

go/internal/command/commandargs/command_args.go

@@ -17,6 +17,8 @@ func Parse(e *executable.Executable, arguments []string) (CommandArgs, error) {
 	switch e.Name {
 	case executable.GitlabShell:
 		args = &Shell{Arguments: arguments}
+	case executable.AuthorizedKeysCheck:
+		args = &AuthorizedKeys{Arguments: arguments}
 	}
 
 	if err := args.Parse(); err != nil {

go/internal/command/commandargs/command_args_test.go

@@ -119,6 +119,11 @@ func TestParseSuccess(t *testing.T) {
 			},
 			arguments:    []string{},
 			expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"git-lfs-authenticate", "group/repo", "download"}, CommandType: LfsAuthenticate},
+		}, {
+			desc:         "It parses authorized-keys command",
+			executable:   &executable.Executable{Name: executable.AuthorizedKeysCheck},
+			arguments:    []string{"git", "git", "key"},
+			expectedArgs: &AuthorizedKeys{Arguments: []string{"git", "git", "key"}, ExpectedUser: "git", ActualUser: "git", Key: "key"},
 		}, {
 			desc:         "Unknown executable",
 			executable:   &executable.Executable{Name: "unknown"},
@@ -162,7 +167,31 @@ func TestParseFailure(t *testing.T) {
 				"SSH_ORIGINAL_COMMAND": `git receive-pack "`,
 			},
 			arguments:     []string{},
-			expectedError: "Invalid SSH allowed",
+			expectedError: "Invalid SSH command",
+		},
+		{
+			desc:          "With not enough arguments for the AuthorizedKeysCheck",
+			executable:    &executable.Executable{Name: executable.AuthorizedKeysCheck},
+			arguments:     []string{"user"},
+			expectedError: "# Insufficient arguments. 1. Usage\n#\tgitlab-shell-authorized-keys-check <expected-username> <actual-username> <key>",
+		},
+		{
+			desc:          "With too many arguments for the AuthorizedKeysCheck",
+			executable:    &executable.Executable{Name: executable.AuthorizedKeysCheck},
+			arguments:     []string{"user", "user", "key", "something-else"},
+			expectedError: "# Insufficient arguments. 4. Usage\n#\tgitlab-shell-authorized-keys-check <expected-username> <actual-username> <key>",
+		},
+		{
+			desc:          "With missing username for the AuthorizedKeysCheck",
+			executable:    &executable.Executable{Name: executable.AuthorizedKeysCheck},
+			arguments:     []string{"user", "", "key"},
+			expectedError: "# No username provided",
+		},
+		{
+			desc:          "With missing key for the AuthorizedKeysCheck",
+			executable:    &executable.Executable{Name: executable.AuthorizedKeysCheck},
+			arguments:     []string{"user", "user", ""},
+			expectedError: "# No key provided",
 		},
 	}
 
@@ -173,7 +202,7 @@ func TestParseFailure(t *testing.T) {
 
 			_, err := Parse(tc.executable, tc.arguments)
 
-			require.Error(t, err, tc.expectedError)
+			require.EqualError(t, err, tc.expectedError)
 		})
 	}
 }

go/internal/executable/executable.go

@@ -6,6 +6,7 @@ import (
 )
 
 const (
+	BinDir                    = "bin"
 	GitlabShell               = "gitlab-shell"
 	AuthorizedKeysCheck       = "gitlab-shell-authorized-keys-check"
 	AuthorizedPrincipalsCheck = "gitlab-shell-authorized-principals-check"

go/internal/gitlabnet/authorizedkeys/client.go

+package authorizedkeys
+
+import (
+	"fmt"
+	"net/url"
+
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/config"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/gitlabnet"
+)
+
+const (
+	AuthorizedKeysPath = "/authorized_keys"
+)
+
+type Client struct {
+	config *config.Config
+	client *gitlabnet.GitlabClient
+}
+
+type Response struct {
+	Id  int64  `json:"id"`
+	Key string `json:"key"`
+}
+
+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(key string) (*Response, error) {
+	path, err := pathWithKey(key)
+	if err != nil {
+		return nil, err
+	}
+
+	response, err := c.client.Get(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(key string) (string, error) {
+	u, err := url.Parse(AuthorizedKeysPath)
+	if err != nil {
+		return "", err
+	}
+
+	params := u.Query()
+	params.Set("key", key)
+	u.RawQuery = params.Encode()
+
+	return u.String(), nil
+}

go/internal/gitlabnet/authorizedkeys/client_test.go

+package authorizedkeys
+
+import (
+	"encoding/json"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/config"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/gitlabnet"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/gitlabnet/testserver"
+)
+
+var (
+	requests []testserver.TestRequestHandler
+)
+
+func init() {
+	requests = []testserver.TestRequestHandler{
+		{
+			Path: "/api/v4/internal/authorized_keys",
+			Handler: func(w http.ResponseWriter, r *http.Request) {
+				if r.URL.Query().Get("key") == "key" {
+					body := &Response{
+						Id:  1,
+						Key: "public-key",
+					}
+					json.NewEncoder(w).Encode(body)
+				} else if r.URL.Query().Get("key") == "broken-message" {
+					w.WriteHeader(http.StatusForbidden)
+					body := &gitlabnet.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, cleanup := setup(t)
+	defer cleanup()
+
+	result, err := client.GetByKey("key")
+	require.NoError(t, err)
+	require.Equal(t, &Response{Id: 1, Key: "public-key"}, result)
+}
+
+func TestGetByKeyErrorResponses(t *testing.T) {
+	client, cleanup := setup(t)
+	defer cleanup()
+
+	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(tc.key)
+
+			require.EqualError(t, err, tc.expectedError)
+			require.Nil(t, resp)
+		})
+	}
+}
+
+func setup(t *testing.T) (*Client, func()) {
+	url, cleanup := testserver.StartSocketHttpServer(t, requests)
+
+	client, err := NewClient(&config.Config{GitlabUrl: url})
+	require.NoError(t, err)
+
+	return client, cleanup
+}

go/internal/keyline/key_line.go

+package keyline
+
+import (
+	"errors"
+	"fmt"
+	"path"
+	"regexp"
+	"strings"
+
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/executable"
+)
+
+var (
+	keyRegex = regexp.MustCompile(`\A[a-z0-9-]+\z`)
+)
+
+const (
+	PublicKeyPrefix = "key"
+	SshOptions      = "no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty"
+)
+
+type KeyLine struct {
+	Id      string // This can be either an ID of a Key or username
+	Value   string // This can be either a public key or a principal name
+	Prefix  string
+	RootDir string
+}
+
+func NewPublicKeyLine(id string, publicKey string, rootDir string) (*KeyLine, error) {
+	if err := validate(id, publicKey); err != nil {
+		return nil, err
+	}
+
+	return &KeyLine{Id: id, Value: publicKey, Prefix: PublicKeyPrefix, RootDir: rootDir}, nil
+}
+
+func (k *KeyLine) ToString() string {
+	command := fmt.Sprintf("%s %s-%s", path.Join(k.RootDir, executable.BinDir, executable.GitlabShell), k.Prefix, k.Id)
+
+	return fmt.Sprintf(`command="%s",%s %s`, command, SshOptions, k.Value)
+}
+
+func validate(id string, value string) error {
+	if !keyRegex.MatchString(id) {
+		return errors.New(fmt.Sprintf("Invalid key_id: %s", id))
+	}
+
+	if strings.Contains(value, "\n") {
+		return errors.New(fmt.Sprintf("Invalid value: %s", value))
+	}
+
+	return nil
+}

go/internal/keyline/key_line_test.go

+package keyline
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestFailingNewPublicKeyLine(t *testing.T) {
+	testCases := []struct {
+		desc          string
+		id            string
+		publicKey     string
+		expectedError string
+	}{
+		{
+			desc:          "When Id has non-alphanumeric and non-dash characters in it",
+			id:            "key\n1",
+			publicKey:     "public-key",
+			expectedError: "Invalid key_id: key\n1",
+		},
+		{
+			desc:          "When public key has newline in it",
+			id:            "key",
+			publicKey:     "public\nkey",
+			expectedError: "Invalid value: public\nkey",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			result, err := NewPublicKeyLine(tc.id, tc.publicKey, "root-dir")
+
+			require.Empty(t, result)
+			require.EqualError(t, err, tc.expectedError)
+		})
+	}
+}
+
+func TestToString(t *testing.T) {
+	keyLine := &KeyLine{
+		Id:      "1",
+		Value:   "public-key",
+		Prefix:  "key",
+		RootDir: "/tmp",
+	}
+
+	result := keyLine.ToString()
+
+	require.Equal(t, `command="/tmp/bin/gitlab-shell key-1",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty public-key`, result)
+}

spec/gitlab_shell_authorized_keys_check_spec.rb

@@ -87,7 +87,7 @@ describe 'bin/gitlab-shell-authorized-keys-check' do
     it_behaves_like 'authorized keys check'
   end
 
-  pending 'with the go authorized-keys-check feature', :go do
+  describe 'with the go authorized-keys-check feature', :go do
     before(:all) do
       write_config(
         'gitlab_url' => "http+unix://#{CGI.escape(tmp_socket_path)}",