Fix incorrect actor used to check permissions for SSH receive-packsh-fix-wrong-user-deploy-key-check

During a SSH receive-pack request (e.g. `git push`), gitlab-shell was
incorrectly using the user returned by the `/internal/allowed` API
endpoint to make an SSHReceivePack RPC call. This caused a number of
problems with deploy keys with write access:

1. Keys that were generated by a blocked user would be denied the
ability to write.

2. Keys that were generated by user that did not have write access to
the project would also be denied.

GitLab 12.4 removed the Ruby implementation of gitlab-shell in favor of
the Golang implementation, and these implementations worked slightly
differently. In
https://gitlab.com/gitlab-org/gitlab-shell/blob/v10.1.0/lib/gitlab_shell.rb,
the Ruby implementation would always use `@who` (e.g. `key-123`), but in
gitlab-shell v10.2.0 the Go implementation would always use the user
from the API response.

Reads did not have this issue because the user/deploy key is never
passed to Gitaly for additional permission checks. Writes need this
information for the pre-receive to check access to protected branches,
push rules, etc.

Relates to https://gitlab.com/gitlab-org/gitlab-shell/-/issues/479

2 files changed, 49 insertions, 25 deletions

diff --git a/internal/command/receivepack/gitalycall.go b/internal/command/receivepack/gitalycall.go
index 0754a3e..a983c1a 100644
--- a/internal/command/receivepack/gitalycall.go
+++ b/internal/command/receivepack/gitalycall.go
@@ -24,7 +24,7 @@ func (c *Command) performGitalyCall(response *accessverifier.Response) error {
 
 	request := &pb.SSHReceivePackRequest{
 		Repository:       &response.Gitaly.Repo,
-		GlId:             response.UserId,
+		GlId:             response.Who,
 		GlRepository:     response.Repo,
 		GlUsername:       response.Username,
 		GitProtocol:      os.Getenv(commandargs.GitProtocolEnv),
diff --git a/internal/command/receivepack/gitalycall_test.go b/internal/command/receivepack/gitalycall_test.go
index 2a0c146..ea07477 100644
--- a/internal/command/receivepack/gitalycall_test.go
+++ b/internal/command/receivepack/gitalycall_test.go
@@ -29,33 +29,57 @@ func TestReceivePack(t *testing.T) {
 	require.NoError(t, err)
 	defer envCleanup()
 
-	output := &bytes.Buffer{}
-	input := &bytes.Buffer{}
+	testCases := []struct {
+		username string
+		keyId    string
+	}{
+		{
+			username: "john.doe",
+		},
+		{
+			keyId: "123",
+		},
+	}
 
-	userId := "1"
-	repo := "group/repo"
+	for _, tc := range testCases {
+		output := &bytes.Buffer{}
+		input := &bytes.Buffer{}
+		repo := "group/repo"
 
-	cmd := &Command{
-		Config:     &config.Config{GitlabUrl: url},
-		Args:       &commandargs.Shell{GitlabKeyId: userId, CommandType: commandargs.ReceivePack, SshArgs: []string{"git-receive-pack", repo}},
-		ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output, In: input},
-	}
+		args := &commandargs.Shell{CommandType: commandargs.ReceivePack, SshArgs: []string{"git-receive-pack", repo}}
 
-	hook := testhelper.SetupLogger()
+		if tc.username != "" {
+			args.GitlabUsername = tc.username
+		} else {
+			args.GitlabKeyId = tc.keyId
+		}
 
-	err = cmd.Execute(context.Background())
-	require.NoError(t, err)
+		cmd := &Command{
+			Config:     &config.Config{GitlabUrl: url},
+			Args:       args,
+			ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output, In: input},
+		}
+
+		hook := testhelper.SetupLogger()
 
-	require.Equal(t, "ReceivePack: "+userId+" "+repo, output.String())
-
-	require.True(t, testhelper.WaitForLogEvent(hook))
-	entries := hook.AllEntries()
-	require.Equal(t, 2, len(entries))
-	require.Equal(t, logrus.InfoLevel, entries[1].Level)
-	require.Contains(t, entries[1].Message, "executing git command")
-	require.Contains(t, entries[1].Message, "command=git-receive-pack")
-	require.Contains(t, entries[1].Message, "remote_ip=127.0.0.1")
-	require.Contains(t, entries[1].Message, "gl_key_type=key")
-	require.Contains(t, entries[1].Message, "gl_key_id=123")
-	require.Contains(t, entries[1].Message, "correlation_id=")
+		err = cmd.Execute(context.Background())
+		require.NoError(t, err)
+
+		if tc.username != "" {
+			require.Equal(t, "ReceivePack: 1 "+repo, output.String())
+		} else {
+			require.Equal(t, "ReceivePack: key-123 "+repo, output.String())
+		}
+
+		require.True(t, testhelper.WaitForLogEvent(hook))
+		entries := hook.AllEntries()
+		require.Equal(t, 2, len(entries))
+		require.Equal(t, logrus.InfoLevel, entries[1].Level)
+		require.Contains(t, entries[1].Message, "executing git command")
+		require.Contains(t, entries[1].Message, "command=git-receive-pack")
+		require.Contains(t, entries[1].Message, "remote_ip=127.0.0.1")
+		require.Contains(t, entries[1].Message, "gl_key_type=key")
+		require.Contains(t, entries[1].Message, "gl_key_id=123")
+		require.Contains(t, entries[1].Message, "correlation_id=")
+	}
 }