Merge branch 'sh-fix-wrong-user-deploy-key-check' into 'master'

Fix incorrect actor used to check permissions for SSH receive-pack

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

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=")
+	}
 }