diff options
author | Patrick Bajao <ebajao@gitlab.com> | 2020-10-19 09:16:47 +0000 |
---|---|---|
committer | Patrick Bajao <ebajao@gitlab.com> | 2020-10-19 09:16:47 +0000 |
commit | c09bdad64dc5ae1a57c61a435edb62b5f9a7c3a8 (patch) | |
tree | df79ee09d7dfa952589d3bd8dbab61f2f3c2cfb9 | |
parent | 12353c0c1d6ddf57b738b04d86cdd34ce64f2f18 (diff) | |
parent | 0478ba97950bd6606f823c8a26eeeecf617df653 (diff) | |
download | gitlab-shell-c09bdad64dc5ae1a57c61a435edb62b5f9a7c3a8.tar.gz |
Merge branch 'ashmckenzie/set-ssl-cert-dir-env-var' into 'master'
Set SSL_CERT_DIR env var when building command
See merge request gitlab-org/gitlab-shell!423
-rw-r--r-- | internal/command/authorizedkeys/authorized_keys_test.go | 15 | ||||
-rw-r--r-- | internal/command/authorizedprincipals/authorized_principals_test.go | 15 | ||||
-rw-r--r-- | internal/command/command.go | 5 | ||||
-rw-r--r-- | internal/command/command_test.go | 125 | ||||
-rw-r--r-- | internal/keyline/key_line.go | 15 | ||||
-rw-r--r-- | internal/keyline/key_line_test.go | 38 |
6 files changed, 94 insertions, 119 deletions
diff --git a/internal/command/authorizedkeys/authorized_keys_test.go b/internal/command/authorizedkeys/authorized_keys_test.go index f15c34d..ab44580 100644 --- a/internal/command/authorizedkeys/authorized_keys_test.go +++ b/internal/command/authorizedkeys/authorized_keys_test.go @@ -47,11 +47,9 @@ func TestExecute(t *testing.T) { defer cleanup() defaultConfig := &config.Config{RootDir: "/tmp", GitlabUrl: url} - configWithSslCertDir := &config.Config{RootDir: "/tmp", GitlabUrl: url, SslCertDir: "/tmp/certs"} testCases := []struct { desc string - config *config.Config arguments *commandargs.AuthorizedKeys expectedOutput string }{ @@ -61,12 +59,6 @@ func TestExecute(t *testing.T) { expectedOutput: "command=\"/tmp/bin/gitlab-shell key-1\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty public-key\n", }, { - desc: "With SSL cert dir", - config: configWithSslCertDir, - arguments: &commandargs.AuthorizedKeys{ExpectedUser: "user", ActualUser: "user", Key: "key"}, - expectedOutput: "command=\"SSL_CERT_DIR=/tmp/certs /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", @@ -87,13 +79,8 @@ func TestExecute(t *testing.T) { t.Run(tc.desc, func(t *testing.T) { buffer := &bytes.Buffer{} - config := defaultConfig - if tc.config != nil { - config = tc.config - } - cmd := &Command{ - Config: config, + Config: defaultConfig, Args: tc.arguments, ReadWriter: &readwriter.ReadWriter{Out: buffer}, } diff --git a/internal/command/authorizedprincipals/authorized_principals_test.go b/internal/command/authorizedprincipals/authorized_principals_test.go index ec97b65..2450a54 100644 --- a/internal/command/authorizedprincipals/authorized_principals_test.go +++ b/internal/command/authorizedprincipals/authorized_principals_test.go @@ -14,11 +14,9 @@ import ( func TestExecute(t *testing.T) { defaultConfig := &config.Config{RootDir: "/tmp"} - configWithSslCertDir := &config.Config{RootDir: "/tmp", SslCertDir: "/tmp/certs"} testCases := []struct { desc string - config *config.Config arguments *commandargs.AuthorizedPrincipals expectedOutput string }{ @@ -28,12 +26,6 @@ func TestExecute(t *testing.T) { expectedOutput: "command=\"/tmp/bin/gitlab-shell username-key\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty principal\n", }, { - desc: "With SSL cert dir", - config: configWithSslCertDir, - arguments: &commandargs.AuthorizedPrincipals{KeyId: "key", Principals: []string{"principal"}}, - expectedOutput: "command=\"SSL_CERT_DIR=/tmp/certs /tmp/bin/gitlab-shell username-key\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty principal\n", - }, - { desc: "With multiple principals", arguments: &commandargs.AuthorizedPrincipals{KeyId: "key", Principals: []string{"principal-1", "principal-2"}}, expectedOutput: "command=\"/tmp/bin/gitlab-shell username-key\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty principal-1\ncommand=\"/tmp/bin/gitlab-shell username-key\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty principal-2\n", @@ -44,13 +36,8 @@ func TestExecute(t *testing.T) { t.Run(tc.desc, func(t *testing.T) { buffer := &bytes.Buffer{} - config := defaultConfig - if tc.config != nil { - config = tc.config - } - cmd := &Command{ - Config: config, + Config: defaultConfig, Args: tc.arguments, ReadWriter: &readwriter.ReadWriter{Out: buffer}, } diff --git a/internal/command/command.go b/internal/command/command.go index 7e0617e..a2c5912 100644 --- a/internal/command/command.go +++ b/internal/command/command.go @@ -2,6 +2,7 @@ package command import ( "context" + "os" "gitlab.com/gitlab-org/gitlab-shell/internal/command/authorizedkeys" "gitlab.com/gitlab-org/gitlab-shell/internal/command/authorizedprincipals" @@ -34,6 +35,10 @@ func New(e *executable.Executable, arguments []string, config *config.Config, re } if cmd := buildCommand(e, args, config, readWriter); cmd != nil { + if config.SslCertDir != "" { + os.Setenv("SSL_CERT_DIR", config.SslCertDir) + } + return cmd, nil } diff --git a/internal/command/command_test.go b/internal/command/command_test.go index 9160abf..c2a7483 100644 --- a/internal/command/command_test.go +++ b/internal/command/command_test.go @@ -30,7 +30,8 @@ var ( checkExec = &executable.Executable{Name: executable.Healthcheck} gitlabShellExec = &executable.Executable{Name: executable.GitlabShell} - basicConfig = &config.Config{GitlabUrl: "http+unix://gitlab.socket"} + basicConfig = &config.Config{GitlabUrl: "http+unix://gitlab.socket"} + advancedConfig = &config.Config{GitlabUrl: "http+unix://gitlab.socket", SslCertDir: "/tmp/certs"} ) func buildEnv(command string) map[string]string { @@ -42,70 +43,100 @@ func buildEnv(command string) map[string]string { func TestNew(t *testing.T) { testCases := []struct { - desc string - executable *executable.Executable - environment map[string]string - arguments []string - expectedType interface{} + desc string + executable *executable.Executable + environment map[string]string + arguments []string + config *config.Config + expectedType interface{} + expectedSslCertDir string }{ { - desc: "it returns a Discover command", - executable: gitlabShellExec, - environment: buildEnv(""), - expectedType: &discover.Command{}, + desc: "it returns a Discover command", + executable: gitlabShellExec, + environment: buildEnv(""), + config: basicConfig, + expectedType: &discover.Command{}, + expectedSslCertDir: "", }, { - desc: "it returns a TwoFactorRecover command", - executable: gitlabShellExec, - environment: buildEnv("2fa_recovery_codes"), - expectedType: &twofactorrecover.Command{}, + desc: "it returns a Discover command with SSL_CERT_DIR env var set", + executable: gitlabShellExec, + environment: buildEnv(""), + config: advancedConfig, + expectedType: &discover.Command{}, + expectedSslCertDir: "/tmp/certs", }, { - desc: "it returns an LfsAuthenticate command", - executable: gitlabShellExec, - environment: buildEnv("git-lfs-authenticate"), - expectedType: &lfsauthenticate.Command{}, + desc: "it returns a TwoFactorRecover command", + executable: gitlabShellExec, + environment: buildEnv("2fa_recovery_codes"), + config: basicConfig, + expectedType: &twofactorrecover.Command{}, + expectedSslCertDir: "", }, { - desc: "it returns a ReceivePack command", - executable: gitlabShellExec, - environment: buildEnv("git-receive-pack"), - expectedType: &receivepack.Command{}, + desc: "it returns an LfsAuthenticate command", + executable: gitlabShellExec, + environment: buildEnv("git-lfs-authenticate"), + config: basicConfig, + expectedType: &lfsauthenticate.Command{}, + expectedSslCertDir: "", }, { - desc: "it returns an UploadPack command", - executable: gitlabShellExec, - environment: buildEnv("git-upload-pack"), - expectedType: &uploadpack.Command{}, + desc: "it returns a ReceivePack command", + executable: gitlabShellExec, + environment: buildEnv("git-receive-pack"), + config: basicConfig, + expectedType: &receivepack.Command{}, + expectedSslCertDir: "", }, { - desc: "it returns an UploadArchive command", - executable: gitlabShellExec, - environment: buildEnv("git-upload-archive"), - expectedType: &uploadarchive.Command{}, + desc: "it returns an UploadPack command", + executable: gitlabShellExec, + environment: buildEnv("git-upload-pack"), + config: basicConfig, + expectedType: &uploadpack.Command{}, + expectedSslCertDir: "", }, { - desc: "it returns a Healthcheck command", - executable: checkExec, - expectedType: &healthcheck.Command{}, + desc: "it returns an UploadArchive command", + executable: gitlabShellExec, + environment: buildEnv("git-upload-archive"), + config: basicConfig, + expectedType: &uploadarchive.Command{}, + expectedSslCertDir: "", }, { - desc: "it returns a AuthorizedKeys command", - executable: authorizedKeysExec, - arguments: []string{"git", "git", "key"}, - expectedType: &authorizedkeys.Command{}, + desc: "it returns a Healthcheck command", + executable: checkExec, + config: basicConfig, + expectedType: &healthcheck.Command{}, + expectedSslCertDir: "", }, { - desc: "it returns a AuthorizedPrincipals command", - executable: authorizedPrincipalsExec, - arguments: []string{"key", "principal"}, - expectedType: &authorizedprincipals.Command{}, + desc: "it returns a AuthorizedKeys command", + executable: authorizedKeysExec, + arguments: []string{"git", "git", "key"}, + config: basicConfig, + expectedType: &authorizedkeys.Command{}, + expectedSslCertDir: "", }, { - desc: "it returns a PersonalAccessToken command", - executable: gitlabShellExec, - environment: buildEnv("personal_access_token"), - expectedType: &personalaccesstoken.Command{}, + desc: "it returns a AuthorizedPrincipals command", + executable: authorizedPrincipalsExec, + arguments: []string{"key", "principal"}, + config: basicConfig, + expectedType: &authorizedprincipals.Command{}, + expectedSslCertDir: "", + }, + { + desc: "it returns a PersonalAccessToken command", + executable: gitlabShellExec, + environment: buildEnv("personal_access_token"), + config: basicConfig, + expectedType: &personalaccesstoken.Command{}, + expectedSslCertDir: "", }, } @@ -114,10 +145,12 @@ func TestNew(t *testing.T) { restoreEnv := testhelper.TempEnv(tc.environment) defer restoreEnv() - command, err := New(tc.executable, tc.arguments, basicConfig, nil) + os.Unsetenv("SSL_CERT_DIR") + command, err := New(tc.executable, tc.arguments, tc.config, nil) require.NoError(t, err) require.IsType(t, tc.expectedType, command) + require.Equal(t, tc.expectedSslCertDir, os.Getenv("SSL_CERT_DIR")) }) } } diff --git a/internal/keyline/key_line.go b/internal/keyline/key_line.go index e2abb82..c6f2422 100644 --- a/internal/keyline/key_line.go +++ b/internal/keyline/key_line.go @@ -37,22 +37,9 @@ func NewPrincipalKeyLine(keyId, principal string, config *config.Config) (*KeyLi } func (k *KeyLine) ToString() string { - sslCertDirEnvVar := k.sslCertDirEnvVar() command := fmt.Sprintf("%s %s-%s", path.Join(k.Config.RootDir, executable.BinDir, executable.GitlabShell), k.Prefix, k.Id) - if sslCertDirEnvVar != "" { - sslCertDirEnvVar = fmt.Sprintf(`%s `, sslCertDirEnvVar) - } - - return fmt.Sprintf(`command="%s%s",%s %s`, sslCertDirEnvVar, command, SshOptions, k.Value) -} - -func (k *KeyLine) sslCertDirEnvVar() string { - if k.Config.SslCertDir != "" { - return fmt.Sprintf(`SSL_CERT_DIR=%s`, k.Config.SslCertDir) - } - - return "" + return fmt.Sprintf(`command="%s",%s %s`, command, SshOptions, k.Value) } func newKeyLine(id, value, prefix string, config *config.Config) (*KeyLine, error) { diff --git a/internal/keyline/key_line_test.go b/internal/keyline/key_line_test.go index 095de78..e652c23 100644 --- a/internal/keyline/key_line_test.go +++ b/internal/keyline/key_line_test.go @@ -70,37 +70,13 @@ func TestFailingNewPrincipalKeyLine(t *testing.T) { } func TestToString(t *testing.T) { - testCases := []struct { - desc string - keyLine *KeyLine - expectedOutput string - }{ - { - desc: "Without SSL cert dir", - keyLine: &KeyLine{ - Id: "1", - Value: "public-key", - Prefix: "key", - Config: &config.Config{RootDir: "/tmp"}, - }, - expectedOutput: `command="/tmp/bin/gitlab-shell key-1",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty public-key`, - }, - { - desc: "With SSL cert dir", - keyLine: &KeyLine{ - Id: "1", - Value: "public-key", - Prefix: "key", - Config: &config.Config{RootDir: "/tmp", SslCertDir: "/tmp/certs"}, - }, - expectedOutput: `command="SSL_CERT_DIR=/tmp/certs /tmp/bin/gitlab-shell key-1",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty public-key`, - }, + keyLine := &KeyLine{ + Id: "1", + Value: "public-key", + Prefix: "key", + Config: &config.Config{RootDir: "/tmp"}, } - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - result := tc.keyLine.ToString() - require.Equal(t, tc.expectedOutput, result) - }) - } + 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) } |