Migrate gitlab-shell-authorized-keys-check to go

A symlink to gitlab-shell is added and the ruby script has been
renamed to `gitlab-shell-authorized-keys-check-ruby`. It'll be
used as a fallback executable.

8 files changed, 394 insertions, 48 deletions

diff --git a/bin/gitlab-shell-authorized-keys-check b/bin/gitlab-shell-authorized-keys-check
index 2ea1a74..3dc14d1 100755..120000
--- a/bin/gitlab-shell-authorized-keys-check
+++ b/bin/gitlab-shell-authorized-keys-check
@@ -1,42 +1 @@
-#!/usr/bin/env ruby
-
-#
-# GitLab shell authorized_keys helper. Query GitLab API to get the authorized
-# command for a given ssh key fingerprint
-#
-# Ex.
-#   bin/gitlab-shell-authorized-keys-check <username> <public-key>
-#
-# Returns
-#   command="/bin/gitlab-shell key-#",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa AAAA...
-#
-# Expects to be called by the SSH daemon, via configuration like:
-#     AuthorizedKeysCommandUser git
-#     AuthorizedKeysCommand /bin/gitlab-shell-authorized-keys-check git %u %k
-
-abort "# Wrong number of arguments. #{ARGV.size}. Usage:
-#     gitlab-shell-authorized-keys-check <expected-username> <actual-username> <key>" unless ARGV.size == 3
-
-expected_username = ARGV[0]
-abort '# No username provided' if expected_username.nil? || expected_username == ''
-
-actual_username = ARGV[1]
-abort '# No username provided' if actual_username.nil? || actual_username == ''
-
-# Only check access if the requested username matches the configured username.
-# Normally, these would both be 'git', but it can be configured by the user
-exit 0 unless expected_username == actual_username
-
-key = ARGV[2]
-abort "# No key provided" if key.nil? || key == ''
-
-require_relative '../lib/gitlab_init'
-require_relative '../lib/gitlab_net'
-require_relative '../lib/gitlab_keys'
-
-authorized_key = GitlabNet.new.authorized_key(key)
-if authorized_key.nil?
-  puts "# No key was found for #{key}"
-else
-  puts GitlabKeys.key_line("key-#{authorized_key['id']}", authorized_key['key'])
-end
+./gitlab-shell
\ No newline at end of file
diff --git a/bin/gitlab-shell-authorized-keys-check-ruby b/bin/gitlab-shell-authorized-keys-check-ruby
new file mode 100755
index 0000000..2ea1a74
--- /dev/null
+++ b/bin/gitlab-shell-authorized-keys-check-ruby
@@ -0,0 +1,42 @@
+#!/usr/bin/env ruby
+
+#
+# GitLab shell authorized_keys helper. Query GitLab API to get the authorized
+# command for a given ssh key fingerprint
+#
+# Ex.
+#   bin/gitlab-shell-authorized-keys-check <username> <public-key>
+#
+# Returns
+#   command="/bin/gitlab-shell key-#",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa AAAA...
+#
+# Expects to be called by the SSH daemon, via configuration like:
+#     AuthorizedKeysCommandUser git
+#     AuthorizedKeysCommand /bin/gitlab-shell-authorized-keys-check git %u %k
+
+abort "# Wrong number of arguments. #{ARGV.size}. Usage:
+#     gitlab-shell-authorized-keys-check <expected-username> <actual-username> <key>" unless ARGV.size == 3
+
+expected_username = ARGV[0]
+abort '# No username provided' if expected_username.nil? || expected_username == ''
+
+actual_username = ARGV[1]
+abort '# No username provided' if actual_username.nil? || actual_username == ''
+
+# Only check access if the requested username matches the configured username.
+# Normally, these would both be 'git', but it can be configured by the user
+exit 0 unless expected_username == actual_username
+
+key = ARGV[2]
+abort "# No key provided" if key.nil? || key == ''
+
+require_relative '../lib/gitlab_init'
+require_relative '../lib/gitlab_net'
+require_relative '../lib/gitlab_keys'
+
+authorized_key = GitlabNet.new.authorized_key(key)
+if authorized_key.nil?
+  puts "# No key was found for #{key}"
+else
+  puts GitlabKeys.key_line("key-#{authorized_key['id']}", authorized_key['key'])
+end
diff --git a/go/cmd/gitlab-shell/main.go b/go/cmd/gitlab-shell/main.go
index 94d3285..0704cf8 100644
--- a/go/cmd/gitlab-shell/main.go
+++ b/go/cmd/gitlab-shell/main.go
@@ -41,7 +41,7 @@ func execRuby(e *executable.Executable, rootDir string, readWriter *readwriter.R
 	}
 }
 
-func execCommand(config *config.Config, rootDir string, readWriter *readwriter.ReadWriter) {
+func execCommand(config *config.Config, readWriter *readwriter.ReadWriter) {
 	cmd, err := command.New(os.Args, config, readWriter)
 	if err != nil {
 		// For now this could happen if `SSH_CONNECTION` is not set on
@@ -58,7 +58,7 @@ func execCommand(config *config.Config, rootDir string, readWriter *readwriter.R
 	}
 }
 
-func execChecker(e *executable.Executable, config *config.Config, rootDir string, readWriter *readwriter.ReadWriter) {
+func execChecker(e *executable.Executable, config *config.Config, readWriter *readwriter.ReadWriter) {
 	checker := checker.New(e, os.Args, config, readWriter)
 
 	// The checker will write to STDOUT on execution or replace the current
@@ -97,8 +97,8 @@ func main() {
 	}
 
 	if executable.IsForExecutingCommand() {
-		execCommand(config, rootDir, readWriter)
+		execCommand(config, readWriter)
 	} else {
-		execChecker(executable, config, rootDir, readWriter)
+		execChecker(executable, config, readWriter)
 	}
 }
diff --git a/go/internal/checker/authorizedkeys/authorized_keys.go b/go/internal/checker/authorizedkeys/authorized_keys.go
index 4cc59b6..769892e 100644
--- a/go/internal/checker/authorizedkeys/authorized_keys.go
+++ b/go/internal/checker/authorizedkeys/authorized_keys.go
@@ -1,8 +1,13 @@
 package authorizedkeys
 
 import (
+	"errors"
+	"fmt"
+	"path"
+
 	"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"
 )
 
 type Checker struct {
@@ -12,5 +17,65 @@ type Checker struct {
 }
 
 func (c *Checker) Execute() error {
+	if err := c.validateArguments(); err != nil {
+		return err
+	}
+
+	args := c.Args
+	expectedUsername := args[0]
+	actualUsername := args[1]
+	key := args[2]
+
+	if expectedUsername == actualUsername {
+		if err := c.printKeyLine(key); err != nil {
+			return fmt.Errorf("Failed to print key line: %v", err)
+		}
+	}
+
+	return nil
+}
+
+func (c *Checker) printKeyLine(key string) error {
+	client, err := authorizedkeys.NewClient(c.Config)
+	if err != nil {
+		return err
+	}
+
+	response, err := client.GetByKey(key)
+	if err != nil {
+		fmt.Fprintln(c.ReadWriter.Out, fmt.Sprintf("# No key was found for %s", key))
+	} else {
+		fmt.Fprintln(c.ReadWriter.Out, c.formatKeyLine(response.Id, response.Key))
+	}
+
+	return nil
+}
+
+func (c *Checker) formatKeyLine(id int64, key string) string {
+	command := fmt.Sprintf("%s key-%d", path.Join(c.Config.RootDir, "bin", "gitlab-shell"), id)
+
+	return fmt.Sprintf(`command="%s",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty %s`, command, key)
+}
+
+func (c *Checker) validateArguments() error {
+	args := c.Args
+	argsSize := len(args)
+
+	if argsSize != 3 {
+		return errors.New(fmt.Sprintf("# Wrong number of arguments. %d. Usage\n#\tgitlab-shell-authorized-keys-check <expected-username> <actual-username> <key>", argsSize))
+	}
+
+	expectedUsername := args[0]
+	actualUsername := args[1]
+	key := args[2]
+
+	if expectedUsername == "" || actualUsername == "" {
+		return errors.New("# No username provided")
+	}
+
+	if key == "" {
+		return errors.New("# No key provided")
+	}
+
 	return nil
 }
diff --git a/go/internal/checker/authorizedkeys/authorized_keys_test.go b/go/internal/checker/authorizedkeys/authorized_keys_test.go
new file mode 100644
index 0000000..e70e06e
--- /dev/null
+++ b/go/internal/checker/authorizedkeys/authorized_keys_test.go
@@ -0,0 +1,132 @@
+package authorizedkeys
+
+import (
+	"bytes"
+	"encoding/json"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"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      []string
+		expectedOutput string
+	}{
+		{
+			desc:           "With matching username and key",
+			arguments:      []string{"user", "user", "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:     []string{"user", "user", "not-found"},
+			expectedOutput: "# No key was found for not-found\n",
+		},
+		{
+			desc:          "When the API returns an error",
+			arguments:     []string{"user", "user", "broken-message"},
+			expectedOutput: "# No key was found for broken-message\n",
+		},
+		{
+			desc:          "When the API fails",
+			arguments:     []string{"user", "user", "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 := &Checker{
+				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())
+		})
+	}
+}
+
+func TestFailingExecute(t *testing.T) {
+	url, cleanup := testserver.StartSocketHttpServer(t, requests)
+	defer cleanup()
+
+	testCases := []struct {
+		desc          string
+		arguments     []string
+		expectedError string
+	}{
+		{
+			desc:          "With wrong number of arguments",
+			arguments:     []string{"user"},
+			expectedError: "# Wrong number of arguments. 1. Usage\n#\tgitlab-shell-authorized-keys-check <expected-username> <actual-username> <key>",
+		},
+		{
+			desc:          "With missing username",
+			arguments:     []string{"user", "", "key"},
+			expectedError: "# No username provided",
+		},
+		{
+			desc:          "With missing key",
+			arguments:     []string{"user", "user", ""},
+			expectedError: "# No key provided",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			buffer := &bytes.Buffer{}
+			cmd := &Checker{
+				Config:     &config.Config{GitlabUrl: url},
+				Args:       tc.arguments,
+				ReadWriter: &readwriter.ReadWriter{Out: buffer},
+			}
+
+			err := cmd.Execute()
+
+			require.Empty(t, buffer.String())
+			require.EqualError(t, err, tc.expectedError)
+		})
+	}
+}
diff --git a/go/internal/gitlabnet/authorizedkeys/client.go b/go/internal/gitlabnet/authorizedkeys/client.go
new file mode 100644
index 0000000..2d793ff
--- /dev/null
+++ b/go/internal/gitlabnet/authorizedkeys/client.go
@@ -0,0 +1,47 @@
+package authorizedkeys
+
+import (
+	"fmt"
+	"net/url"
+
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/config"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/gitlabnet"
+)
+
+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) {
+	params := url.Values{}
+	params.Add("key", key)
+
+	path := "/authorized_keys?" + params.Encode()
+	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
+}
diff --git a/go/internal/gitlabnet/authorizedkeys/client_test.go b/go/internal/gitlabnet/authorizedkeys/client_test.go
new file mode 100644
index 0000000..6fbe357
--- /dev/null
+++ b/go/internal/gitlabnet/authorizedkeys/client_test.go
@@ -0,0 +1,101 @@
+package authorizedkeys
+
+import (
+	"encoding/json"
+	"fmt"
+	"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 {
+					fmt.Fprint(w, "null")
+				}
+			},
+		},
+	}
+}
+
+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:          "An error response without message",
+			key:           "broken-empty",
+			expectedError: "Internal API error (403)",
+		},
+	}
+
+	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
+}
diff --git a/spec/gitlab_shell_authorized_keys_check_spec.rb b/spec/gitlab_shell_authorized_keys_check_ruby_spec.rb
index 7050604..5cd0b37 100644
--- a/spec/gitlab_shell_authorized_keys_check_spec.rb
+++ b/spec/gitlab_shell_authorized_keys_check_ruby_spec.rb
@@ -1,6 +1,6 @@
 require_relative 'spec_helper'
 
-describe 'bin/gitlab-shell-authorized-keys-check' do
+describe 'bin/gitlab-shell-authorized-keys-check-ruby' do
   include_context 'gitlab shell'
 
   def tmp_socket_path
@@ -27,7 +27,7 @@ describe 'bin/gitlab-shell-authorized-keys-check' do
     )
   end
 
-  let(:authorized_keys_check_path) { File.join(tmp_root_path, 'bin', 'gitlab-shell-authorized-keys-check') }
+  let(:authorized_keys_check_path) { File.join(tmp_root_path, 'bin', 'gitlab-shell-authorized-keys-check-ruby') }
 
   it 'succeeds when a valid key is given' do
     output, status = run!