diff options
author | Ash McKenzie <amckenzie@gitlab.com> | 2019-08-05 05:03:16 +0000 |
---|---|---|
committer | Ash McKenzie <amckenzie@gitlab.com> | 2019-08-05 05:03:16 +0000 |
commit | c577eb9ed8bd0336870f7a83302f70821d510169 (patch) | |
tree | ed7f7281633d97933e4465a2ac0f86d62c9a216e | |
parent | ed0460374a5ca13d9ea17c6a9c21151319b7fd53 (diff) | |
parent | 3b6f9f7583755e041e76142d7caf7716937907fa (diff) | |
download | gitlab-shell-c577eb9ed8bd0336870f7a83302f70821d510169.tar.gz |
Merge branch '181-migrate-gitlab-shell-checks-fallback' into 'master'
Support falling back to ruby version of checkers
See merge request gitlab-org/gitlab-shell!318
44 files changed, 809 insertions, 383 deletions
@@ -16,8 +16,14 @@ test_ruby: # bin/gitlab-shell must exist and needs to be the Ruby version for # rspec to be able to test. cp bin/gitlab-shell-ruby bin/gitlab-shell + # bin/gitlab-shell-authorized-keys-check and bin/gitlab-shell-authorized-principals-check + # should link to ruby scripts for rspec to be able to test. + ln -sf ./gitlab-shell-authorized-keys-check-ruby bin/gitlab-shell-authorized-keys-check + ln -sf ./gitlab-shell-authorized-principals-check-ruby bin/gitlab-shell-authorized-principals-check bundle exec rspec --color --tag '~go' --format d spec rm -f bin/gitlab-shell + ln -sf ./gitlab-shell bin/gitlab-shell-authorized-keys-check + ln -sf ./gitlab-shell bin/gitlab-shell-authorized-principals-check test_golang: support/go-test 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/bin/gitlab-shell-authorized-principals-check b/bin/gitlab-shell-authorized-principals-check index aa6d427..3dc14d1 100755..120000 --- a/bin/gitlab-shell-authorized-principals-check +++ b/bin/gitlab-shell-authorized-principals-check @@ -1,36 +1 @@ -#!/usr/bin/env ruby - -# -# GitLab shell authorized principals helper. Emits the same sort of -# command="..." line as gitlab-shell-authorized-principals-check, with -# the right options. -# -# Ex. -# bin/gitlab-shell-authorized-keys-check <key-id> <principal1> [<principal2>...] -# -# Returns one line per principal passed in, e.g.: -# command="/bin/gitlab-shell username-{KEY_ID}",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty {PRINCIPAL} -# [command="/bin/gitlab-shell username-{KEY_ID}",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty {PRINCIPAL2}] -# -# Expects to be called by the SSH daemon, via configuration like: -# AuthorizedPrincipalsCommandUser root -# AuthorizedPrincipalsCommand /bin/gitlab-shell-authorized-principals-check git %i sshUsers - -abort "# Wrong number of arguments. #{ARGV.size}. Usage: -# gitlab-shell-authorized-principals-check <key-id> <principal1> [<principal2>...]" unless ARGV.size >= 2 - -key_id = ARGV[0] -abort '# No key_id provided' if key_id.nil? || key_id == '' - -principals = ARGV[1..-1] -principals.each { |principal| - abort '# An invalid principal was provided' if principal.nil? || principal == '' -} - -require_relative '../lib/gitlab_init' -require_relative '../lib/gitlab_net' -require_relative '../lib/gitlab_keys' - -principals.each { |principal| - puts GitlabKeys.principal_line("username-#{key_id}", principal.dup) -} +./gitlab-shell
\ No newline at end of file diff --git a/bin/gitlab-shell-authorized-principals-check-ruby b/bin/gitlab-shell-authorized-principals-check-ruby new file mode 100755 index 0000000..25ee612 --- /dev/null +++ b/bin/gitlab-shell-authorized-principals-check-ruby @@ -0,0 +1,36 @@ +#!/usr/bin/env ruby + +# +# GitLab shell authorized principals helper. Emits the same sort of +# command="..." line as gitlab-shell-authorized-principals-check, with +# the right options. +# +# Ex. +# bin/gitlab-shell-authorized-keys-check <key-id> <principal1> [<principal2>...] +# +# Returns one line per principal passed in, e.g.: +# command="/bin/gitlab-shell username-{KEY_ID}",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty {PRINCIPAL} +# [command="/bin/gitlab-shell username-{KEY_ID}",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty {PRINCIPAL2}] +# +# Expects to be called by the SSH daemon, via configuration like: +# AuthorizedPrincipalsCommandUser root +# AuthorizedPrincipalsCommand /bin/gitlab-shell-authorized-principals-check git %i sshUsers + +abort "# Wrong number of arguments. #{ARGV.size}. Usage: +# gitlab-shell-authorized-principals-check <key-id> <principal1> [<principal2>...]" unless ARGV.size >= 2 + +key_id = ARGV[0] +abort '# No key_id provided' if key_id.nil? || key_id == '' + +principals = ARGV[1..-1] +principals.each { |principal| + abort '# An invalid principal was provided' if principal.nil? || principal == '' +} + +require_relative '../lib/gitlab_init' +require_relative '../lib/gitlab_net' +require_relative '../lib/gitlab_keys' + +principals.each { |principal| + puts GitlabKeys.principal_line("username-#{key_id}", principal.dup) +} diff --git a/go/cmd/gitlab-shell/main.go b/go/cmd/gitlab-shell/main.go index f4d519f..b716820 100644 --- a/go/cmd/gitlab-shell/main.go +++ b/go/cmd/gitlab-shell/main.go @@ -3,42 +3,13 @@ package main import ( "fmt" "os" - "path/filepath" "gitlab.com/gitlab-org/gitlab-shell/go/internal/command" - "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/fallback" "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/executable" ) -// findRootDir determines the root directory (and so, the location of the config -// file) from os.Executable() -func findRootDir() (string, error) { - if path := os.Getenv("GITLAB_SHELL_DIR"); path != "" { - return path, nil - } - - path, err := os.Executable() - if err != nil { - return "", err - } - - // Start: /opt/.../gitlab-shell/bin/gitlab-shell - // Ends: /opt/.../gitlab-shell - return filepath.Dir(filepath.Dir(path)), nil -} - -// rubyExec will never return. It either replaces the current process with a -// Ruby interpreter, or outputs an error and kills the process. -func execRuby(rootDir string, readWriter *readwriter.ReadWriter) { - cmd := &fallback.Command{RootDir: rootDir, Args: os.Args} - - if err := cmd.Execute(); err != nil { - fmt.Fprintf(readWriter.ErrOut, "Failed to exec: %v\n", err) - os.Exit(1) - } -} - func main() { readWriter := &readwriter.ReadWriter{ Out: os.Stdout, @@ -46,21 +17,19 @@ func main() { ErrOut: os.Stderr, } - rootDir, err := findRootDir() + executable, err := executable.New() if err != nil { - fmt.Fprintln(readWriter.ErrOut, "Failed to determine root directory, exiting") + fmt.Fprintln(readWriter.ErrOut, "Failed to determine executable, exiting") os.Exit(1) } - // Fall back to Ruby in case of problems reading the config, but issue a - // warning as this isn't something we can sustain indefinitely - config, err := config.NewFromDir(rootDir) + config, err := config.NewFromDir(executable.RootDir) if err != nil { - fmt.Fprintln(readWriter.ErrOut, "Failed to read config, falling back to gitlab-shell-ruby") - execRuby(rootDir, readWriter) + fmt.Fprintln(readWriter.ErrOut, "Failed to read config, exiting") + os.Exit(1) } - cmd, err := command.New(os.Args, config, readWriter) + cmd, err := command.New(executable, os.Args[1:], config, readWriter) if err != nil { // For now this could happen if `SSH_CONNECTION` is not set on // the environment diff --git a/go/internal/command/command.go b/go/internal/command/command.go index a1dde42..27378aa 100644 --- a/go/internal/command/command.go +++ b/go/internal/command/command.go @@ -11,29 +11,40 @@ import ( "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/uploadarchive" "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/uploadpack" "gitlab.com/gitlab-org/gitlab-shell/go/internal/config" + "gitlab.com/gitlab-org/gitlab-shell/go/internal/executable" ) type Command interface { Execute() error } -func New(arguments []string, config *config.Config, readWriter *readwriter.ReadWriter) (Command, error) { - args, err := commandargs.Parse(arguments) - +func New(e *executable.Executable, arguments []string, config *config.Config, readWriter *readwriter.ReadWriter) (Command, error) { + args, err := commandargs.Parse(e, arguments) if err != nil { return nil, err } - if config.FeatureEnabled(string(args.CommandType)) { - if cmd := buildCommand(args, config, readWriter); cmd != nil { - return cmd, nil - } + if cmd := buildCommand(e, args, config, readWriter); cmd != nil { + return cmd, nil + } + + return &fallback.Command{Executable: e, RootDir: config.RootDir, Args: args}, nil +} + +func buildCommand(e *executable.Executable, args commandargs.CommandArgs, config *config.Config, readWriter *readwriter.ReadWriter) Command { + switch e.Name { + case executable.GitlabShell: + return buildShellCommand(args.(*commandargs.Shell), config, readWriter) } - return &fallback.Command{RootDir: config.RootDir, Args: arguments}, nil + return nil } -func buildCommand(args *commandargs.CommandArgs, config *config.Config, readWriter *readwriter.ReadWriter) Command { +func buildShellCommand(args *commandargs.Shell, config *config.Config, readWriter *readwriter.ReadWriter) Command { + if !config.FeatureEnabled(string(args.CommandType)) { + return nil + } + switch args.CommandType { case commandargs.Discover: return &discover.Command{Config: config, Args: args, ReadWriter: readWriter} diff --git a/go/internal/command/command_test.go b/go/internal/command/command_test.go index 07260dd..ea88a6a 100644 --- a/go/internal/command/command_test.go +++ b/go/internal/command/command_test.go @@ -13,18 +13,22 @@ import ( "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/uploadarchive" "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/uploadpack" "gitlab.com/gitlab-org/gitlab-shell/go/internal/config" + "gitlab.com/gitlab-org/gitlab-shell/go/internal/executable" "gitlab.com/gitlab-org/gitlab-shell/go/internal/testhelper" ) func TestNew(t *testing.T) { testCases := []struct { desc string + executable *executable.Executable config *config.Config environment map[string]string + arguments []string expectedType interface{} }{ { - desc: "it returns a Discover command if the feature is enabled", + desc: "it returns a Discover command if the feature is enabled", + executable: &executable.Executable{Name: executable.GitlabShell}, config: &config.Config{ GitlabUrl: "http+unix://gitlab.socket", Migration: config.MigrationConfig{Enabled: true, Features: []string{"discover"}}, @@ -33,10 +37,12 @@ func TestNew(t *testing.T) { "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": "", }, + arguments: []string{}, expectedType: &discover.Command{}, }, { - desc: "it returns a Fallback command no feature is enabled", + desc: "it returns a Fallback command no feature is enabled", + executable: &executable.Executable{Name: executable.GitlabShell}, config: &config.Config{ GitlabUrl: "http+unix://gitlab.socket", Migration: config.MigrationConfig{Enabled: false}, @@ -45,10 +51,12 @@ func TestNew(t *testing.T) { "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": "", }, + arguments: []string{}, expectedType: &fallback.Command{}, }, { - desc: "it returns a TwoFactorRecover command if the feature is enabled", + desc: "it returns a TwoFactorRecover command if the feature is enabled", + executable: &executable.Executable{Name: executable.GitlabShell}, config: &config.Config{ GitlabUrl: "http+unix://gitlab.socket", Migration: config.MigrationConfig{Enabled: true, Features: []string{"2fa_recovery_codes"}}, @@ -57,10 +65,12 @@ func TestNew(t *testing.T) { "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": "2fa_recovery_codes", }, + arguments: []string{}, expectedType: &twofactorrecover.Command{}, }, { - desc: "it returns an LfsAuthenticate command if the feature is enabled", + desc: "it returns an LfsAuthenticate command if the feature is enabled", + executable: &executable.Executable{Name: executable.GitlabShell}, config: &config.Config{ GitlabUrl: "http+unix://gitlab.socket", Migration: config.MigrationConfig{Enabled: true, Features: []string{"git-lfs-authenticate"}}, @@ -69,10 +79,12 @@ func TestNew(t *testing.T) { "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": "git-lfs-authenticate", }, + arguments: []string{}, expectedType: &lfsauthenticate.Command{}, }, { - desc: "it returns a ReceivePack command if the feature is enabled", + desc: "it returns a ReceivePack command if the feature is enabled", + executable: &executable.Executable{Name: executable.GitlabShell}, config: &config.Config{ GitlabUrl: "http+unix://gitlab.socket", Migration: config.MigrationConfig{Enabled: true, Features: []string{"git-receive-pack"}}, @@ -81,10 +93,12 @@ func TestNew(t *testing.T) { "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": "git-receive-pack", }, + arguments: []string{}, expectedType: &receivepack.Command{}, }, { - desc: "it returns a UploadPack command if the feature is enabled", + desc: "it returns an UploadPack command if the feature is enabled", + executable: &executable.Executable{Name: executable.GitlabShell}, config: &config.Config{ GitlabUrl: "http+unix://gitlab.socket", Migration: config.MigrationConfig{Enabled: true, Features: []string{"git-upload-pack"}}, @@ -93,10 +107,12 @@ func TestNew(t *testing.T) { "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": "git-upload-pack", }, + arguments: []string{}, expectedType: &uploadpack.Command{}, }, { - desc: "it returns a UploadArchive command if the feature is enabled", + desc: "it returns an UploadArchive command if the feature is enabled", + executable: &executable.Executable{Name: executable.GitlabShell}, config: &config.Config{ GitlabUrl: "http+unix://gitlab.socket", Migration: config.MigrationConfig{Enabled: true, Features: []string{"git-upload-archive"}}, @@ -105,10 +121,12 @@ func TestNew(t *testing.T) { "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": "git-upload-archive", }, + arguments: []string{}, expectedType: &uploadarchive.Command{}, }, { - desc: "it returns a Fallback command if the feature is unimplemented", + desc: "it returns a Fallback command if the feature is unimplemented", + executable: &executable.Executable{Name: executable.GitlabShell}, config: &config.Config{ GitlabUrl: "http+unix://gitlab.socket", Migration: config.MigrationConfig{Enabled: true, Features: []string{"git-unimplemented-feature"}}, @@ -117,6 +135,14 @@ func TestNew(t *testing.T) { "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": "git-unimplemented-feature", }, + arguments: []string{}, + expectedType: &fallback.Command{}, + }, + { + desc: "it returns a Fallback command if executable is unknown", + executable: &executable.Executable{Name: "unknown"}, + config: &config.Config{}, + arguments: []string{}, expectedType: &fallback.Command{}, }, } @@ -126,7 +152,7 @@ func TestNew(t *testing.T) { restoreEnv := testhelper.TempEnv(tc.environment) defer restoreEnv() - command, err := New([]string{}, tc.config, nil) + command, err := New(tc.executable, tc.arguments, tc.config, nil) require.NoError(t, err) require.IsType(t, tc.expectedType, command) @@ -135,12 +161,9 @@ func TestNew(t *testing.T) { } func TestFailingNew(t *testing.T) { - t.Run("It returns an error when SSH_CONNECTION is not set", func(t *testing.T) { - restoreEnv := testhelper.TempEnv(map[string]string{}) - defer restoreEnv() - - _, err := New([]string{}, &config.Config{}, nil) + t.Run("It returns an error parsing arguments failed", func(t *testing.T) { + _, err := New(&executable.Executable{Name: executable.GitlabShell}, []string{}, &config.Config{}, nil) - require.Error(t, err, "Only ssh allowed") + require.Error(t, err) }) } diff --git a/go/internal/command/commandargs/command_args.go b/go/internal/command/commandargs/command_args.go index d8fe32d..5338d6b 100644 --- a/go/internal/command/commandargs/command_args.go +++ b/go/internal/command/commandargs/command_args.go @@ -1,111 +1,27 @@ package commandargs import ( - "errors" - "os" - "regexp" - - "github.com/mattn/go-shellwords" + "gitlab.com/gitlab-org/gitlab-shell/go/internal/executable" ) type CommandType string -const ( - Discover CommandType = "discover" - TwoFactorRecover CommandType = "2fa_recovery_codes" - LfsAuthenticate CommandType = "git-lfs-authenticate" - ReceivePack CommandType = "git-receive-pack" - UploadPack CommandType = "git-upload-pack" - UploadArchive CommandType = "git-upload-archive" -) - -var ( - whoKeyRegex = regexp.MustCompile(`\bkey-(?P<keyid>\d+)\b`) - whoUsernameRegex = regexp.MustCompile(`\busername-(?P<username>\S+)\b`) -) - -type CommandArgs struct { - GitlabUsername string - GitlabKeyId string - SshArgs []string - CommandType CommandType -} - -func Parse(arguments []string) (*CommandArgs, error) { - if sshConnection := os.Getenv("SSH_CONNECTION"); sshConnection == "" { - return nil, errors.New("Only ssh allowed") - } - - args := &CommandArgs{} - args.parseWho(arguments) - - if err := args.parseCommand(os.Getenv("SSH_ORIGINAL_COMMAND")); err != nil { - return nil, errors.New("Invalid ssh command") - } - args.defineCommandType() - - return args, nil -} - -func (c *CommandArgs) parseWho(arguments []string) { - for _, argument := range arguments { - if keyId := tryParseKeyId(argument); keyId != "" { - c.GitlabKeyId = keyId - break - } - - if username := tryParseUsername(argument); username != "" { - c.GitlabUsername = username - break - } - } +type CommandArgs interface { + Parse() error + GetArguments() []string } -func tryParseKeyId(argument string) string { - matchInfo := whoKeyRegex.FindStringSubmatch(argument) - if len(matchInfo) == 2 { - // The first element is the full matched string - // The second element is the named `keyid` - return matchInfo[1] - } - - return "" -} +func Parse(e *executable.Executable, arguments []string) (CommandArgs, error) { + var args CommandArgs = &GenericArgs{Arguments: arguments} -func tryParseUsername(argument string) string { - matchInfo := whoUsernameRegex.FindStringSubmatch(argument) - if len(matchInfo) == 2 { - // The first element is the full matched string - // The second element is the named `username` - return matchInfo[1] + switch e.Name { + case executable.GitlabShell: + args = &Shell{Arguments: arguments} } - return "" -} - -func (c *CommandArgs) parseCommand(commandString string) error { - args, err := shellwords.Parse(commandString) - if err != nil { - return err + if err := args.Parse(); err != nil { + return nil, err } - // Handle Git for Windows 2.14 using "git upload-pack" instead of git-upload-pack - if len(args) > 1 && args[0] == "git" { - command := args[0] + "-" + args[1] - commandArgs := args[2:] - - args = append([]string{command}, commandArgs...) - } - - c.SshArgs = args - - return nil -} - -func (c *CommandArgs) defineCommandType() { - if len(c.SshArgs) == 0 { - c.CommandType = Discover - } else { - c.CommandType = CommandType(c.SshArgs[0]) - } + return args, nil } diff --git a/go/internal/command/commandargs/command_args_test.go b/go/internal/command/commandargs/command_args_test.go index e60bb92..148c987 100644 --- a/go/internal/command/commandargs/command_args_test.go +++ b/go/internal/command/commandargs/command_args_test.go @@ -3,100 +3,127 @@ package commandargs import ( "testing" - "github.com/stretchr/testify/require" - + "gitlab.com/gitlab-org/gitlab-shell/go/internal/executable" "gitlab.com/gitlab-org/gitlab-shell/go/internal/testhelper" + + "github.com/stretchr/testify/require" ) func TestParseSuccess(t *testing.T) { testCases := []struct { desc string - arguments []string + executable *executable.Executable environment map[string]string - expectedArgs *CommandArgs + arguments []string + expectedArgs CommandArgs }{ // Setting the used env variables for every case to ensure we're // not using anything set in the original env. { - desc: "It sets discover as the command when the command string was empty", + desc: "It sets discover as the command when the command string was empty", + executable: &executable.Executable{Name: executable.GitlabShell}, environment: map[string]string{ "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": "", }, - expectedArgs: &CommandArgs{SshArgs: []string{}, CommandType: Discover}, + arguments: []string{}, + expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{}, CommandType: Discover}, }, { - desc: "It finds the key id in any passed arguments", + desc: "It finds the key id in any passed arguments", + executable: &executable.Executable{Name: executable.GitlabShell}, environment: map[string]string{ "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": "", }, arguments: []string{"hello", "key-123"}, - expectedArgs: &CommandArgs{SshArgs: []string{}, CommandType: Discover, GitlabKeyId: "123"}, + expectedArgs: &Shell{Arguments: []string{"hello", "key-123"}, SshArgs: []string{}, CommandType: Discover, GitlabKeyId: "123"}, }, { - desc: "It finds the username in any passed arguments", + desc: "It finds the username in any passed arguments", + executable: &executable.Executable{Name: executable.GitlabShell}, environment: map[string]string{ "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": "", }, arguments: []string{"hello", "username-jane-doe"}, - expectedArgs: &CommandArgs{SshArgs: []string{}, CommandType: Discover, GitlabUsername: "jane-doe"}, + expectedArgs: &Shell{Arguments: []string{"hello", "username-jane-doe"}, SshArgs: []string{}, CommandType: Discover, GitlabUsername: "jane-doe"}, }, { - desc: "It parses 2fa_recovery_codes command", + desc: "It parses 2fa_recovery_codes command", + executable: &executable.Executable{Name: executable.GitlabShell}, environment: map[string]string{ "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": "2fa_recovery_codes", }, - expectedArgs: &CommandArgs{SshArgs: []string{"2fa_recovery_codes"}, CommandType: TwoFactorRecover}, + arguments: []string{}, + expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"2fa_recovery_codes"}, CommandType: TwoFactorRecover}, }, { - desc: "It parses git-receive-pack command", + desc: "It parses git-receive-pack command", + executable: &executable.Executable{Name: executable.GitlabShell}, environment: map[string]string{ "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": "git-receive-pack group/repo", }, - expectedArgs: &CommandArgs{SshArgs: []string{"git-receive-pack", "group/repo"}, CommandType: ReceivePack}, + arguments: []string{}, + expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"git-receive-pack", "group/repo"}, CommandType: ReceivePack}, }, { - desc: "It parses git-receive-pack command and a project with single quotes", + desc: "It parses git-receive-pack command and a project with single quotes", + executable: &executable.Executable{Name: executable.GitlabShell}, environment: map[string]string{ "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": "git receive-pack 'group/repo'", }, - expectedArgs: &CommandArgs{SshArgs: []string{"git-receive-pack", "group/repo"}, CommandType: ReceivePack}, + arguments: []string{}, + expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"git-receive-pack", "group/repo"}, CommandType: ReceivePack}, }, { - desc: `It parses "git receive-pack" command`, + desc: `It parses "git receive-pack" command`, + executable: &executable.Executable{Name: executable.GitlabShell}, environment: map[string]string{ "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": `git receive-pack "group/repo"`, }, - expectedArgs: &CommandArgs{SshArgs: []string{"git-receive-pack", "group/repo"}, CommandType: ReceivePack}, + arguments: []string{}, + expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"git-receive-pack", "group/repo"}, CommandType: ReceivePack}, }, { - desc: `It parses a command followed by control characters`, + desc: `It parses a command followed by control characters`, + executable: &executable.Executable{Name: executable.GitlabShell}, environment: map[string]string{ "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": `git-receive-pack group/repo; any command`, }, - expectedArgs: &CommandArgs{SshArgs: []string{"git-receive-pack", "group/repo"}, CommandType: ReceivePack}, + arguments: []string{}, + expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"git-receive-pack", "group/repo"}, CommandType: ReceivePack}, }, { - desc: "It parses git-upload-pack command", + desc: "It parses git-upload-pack command", + executable: &executable.Executable{Name: executable.GitlabShell}, environment: map[string]string{ "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": `git upload-pack "group/repo"`, }, - expectedArgs: &CommandArgs{SshArgs: []string{"git-upload-pack", "group/repo"}, CommandType: UploadPack}, + arguments: []string{}, + expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"git-upload-pack", "group/repo"}, CommandType: UploadPack}, }, { - desc: "It parses git-upload-archive command", + desc: "It parses git-upload-archive command", + executable: &executable.Executable{Name: executable.GitlabShell}, environment: map[string]string{ "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": "git-upload-archive 'group/repo'", }, - expectedArgs: &CommandArgs{SshArgs: []string{"git-upload-archive", "group/repo"}, CommandType: UploadArchive}, + arguments: []string{}, + expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"git-upload-archive", "group/repo"}, CommandType: UploadArchive}, }, { - desc: "It parses git-lfs-authenticate command", + desc: "It parses git-lfs-authenticate command", + executable: &executable.Executable{Name: executable.GitlabShell}, environment: map[string]string{ "SSH_CONNECTION": "1", "SSH_ORIGINAL_COMMAND": "git-lfs-authenticate 'group/repo' download", }, - expectedArgs: &CommandArgs{SshArgs: []string{"git-lfs-authenticate", "group/repo", "download"}, CommandType: LfsAuthenticate}, + arguments: []string{}, + expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"git-lfs-authenticate", "group/repo", "download"}, CommandType: LfsAuthenticate}, + }, { + desc: "Unknown executable", + executable: &executable.Executable{Name: "unknown"}, + arguments: []string{}, + expectedArgs: &GenericArgs{Arguments: []string{}}, }, } @@ -105,7 +132,7 @@ func TestParseSuccess(t *testing.T) { restoreEnv := testhelper.TempEnv(tc.environment) defer restoreEnv() - result, err := Parse(tc.arguments) + result, err := Parse(tc.executable, tc.arguments) require.NoError(t, err) require.Equal(t, tc.expectedArgs, result) @@ -114,22 +141,39 @@ func TestParseSuccess(t *testing.T) { } func TestParseFailure(t *testing.T) { - t.Run("It fails if SSH connection is not set", func(t *testing.T) { - _, err := Parse([]string{}) - - require.Error(t, err, "Only ssh allowed") - }) + testCases := []struct { + desc string + executable *executable.Executable + environment map[string]string + arguments []string + expectedError string + }{ + { + desc: "It fails if SSH connection is not set", + executable: &executable.Executable{Name: executable.GitlabShell}, + arguments: []string{}, + expectedError: "Only SSH allowed", + }, + { + desc: "It fails if SSH command is invalid", + executable: &executable.Executable{Name: executable.GitlabShell}, + environment: map[string]string{ + "SSH_CONNECTION": "1", + "SSH_ORIGINAL_COMMAND": `git receive-pack "`, + }, + arguments: []string{}, + expectedError: "Invalid SSH allowed", + }, + } - t.Run("It fails if SSH command is invalid", func(t *testing.T) { - environment := map[string]string{ - "SSH_CONNECTION": "1", - "SSH_ORIGINAL_COMMAND": `git receive-pack "`, - } - restoreEnv := testhelper.TempEnv(environment) - defer restoreEnv() + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + restoreEnv := testhelper.TempEnv(tc.environment) + defer restoreEnv() - _, err := Parse([]string{}) + _, err := Parse(tc.executable, tc.arguments) - require.Error(t, err, "Invalid ssh command") - }) + require.Error(t, err, tc.expectedError) + }) + } } diff --git a/go/internal/command/commandargs/generic_args.go b/go/internal/command/commandargs/generic_args.go new file mode 100644 index 0000000..96bed99 --- /dev/null +++ b/go/internal/command/commandargs/generic_args.go @@ -0,0 +1,14 @@ +package commandargs + +type GenericArgs struct { + Arguments []string +} + +func (b *GenericArgs) Parse() error { + // Do nothing + return nil +} + +func (b *GenericArgs) GetArguments() []string { + return b.Arguments +} diff --git a/go/internal/command/commandargs/shell.go b/go/internal/command/commandargs/shell.go new file mode 100644 index 0000000..7e2b72e --- /dev/null +++ b/go/internal/command/commandargs/shell.go @@ -0,0 +1,131 @@ +package commandargs + +import ( + "errors" + "os" + "regexp" + + "github.com/mattn/go-shellwords" +) + +const ( + Discover CommandType = "discover" + TwoFactorRecover CommandType = "2fa_recovery_codes" + LfsAuthenticate CommandType = "git-lfs-authenticate" + ReceivePack CommandType = "git-receive-pack" + UploadPack CommandType = "git-upload-pack" + UploadArchive CommandType = "git-upload-archive" +) + +var ( + whoKeyRegex = regexp.MustCompile(`\bkey-(?P<keyid>\d+)\b`) + whoUsernameRegex = regexp.MustCompile(`\busername-(?P<username>\S+)\b`) +) + +type Shell struct { + Arguments []string + GitlabUsername string + GitlabKeyId string + SshArgs []string + CommandType CommandType +} + +func (s *Shell) Parse() error { + if err := s.validate(); err != nil { + return err + } + + s.parseWho() + s.defineCommandType() + + return nil +} + +func (s *Shell) GetArguments() []string { + return s.Arguments +} + +func (s *Shell) validate() error { + if !s.isSshConnection() { + return errors.New("Only SSH allowed") + } + + if !s.isValidSshCommand() { + return errors.New("Invalid SSH command") + } + + return nil +} + +func (s *Shell) isSshConnection() bool { + ok := os.Getenv("SSH_CONNECTION") + return ok != "" +} + +func (s *Shell) isValidSshCommand() bool { + err := s.parseCommand(os.Getenv("SSH_ORIGINAL_COMMAND")) + return err == nil +} + +func (s *Shell) parseWho() { + for _, argument := range s.Arguments { + if keyId := tryParseKeyId(argument); keyId != "" { + s.GitlabKeyId = keyId + break + } + + if username := tryParseUsername(argument); username != "" { + s.GitlabUsername = username + break + } + } +} + +func tryParseKeyId(argument string) string { + matchInfo := whoKeyRegex.FindStringSubmatch(argument) + if len(matchInfo) == 2 { + // The first element is the full matched string + // The second element is the named `keyid` + return matchInfo[1] + } + + return "" +} + +func tryParseUsername(argument string) string { + matchInfo := whoUsernameRegex.FindStringSubmatch(argument) + if len(matchInfo) == 2 { + // The first element is the full matched string + // The second element is the named `username` + return matchInfo[1] + } + + return "" +} + +func (s *Shell) parseCommand(commandString string) error { + args, err := shellwords.Parse(commandString) + if err != nil { + return err + } + + // Handle Git for Windows 2.14 using "git upload-pack" instead of git-upload-pack + if len(args) > 1 && args[0] == "git" { + command := args[0] + "-" + args[1] + commandArgs := args[2:] + + args = append([]string{command}, commandArgs...) + } + + s.SshArgs = args + + return nil +} + +func (s *Shell) defineCommandType() { + if len(s.SshArgs) == 0 { + s.CommandType = Discover + } else { + s.CommandType = CommandType(s.SshArgs[0]) + } +} diff --git a/go/internal/command/discover/discover.go b/go/internal/command/discover/discover.go index 7d4ad2b..de94b56 100644 --- a/go/internal/command/discover/discover.go +++ b/go/internal/command/discover/discover.go @@ -11,7 +11,7 @@ import ( type Command struct { Config *config.Config - Args *commandargs.CommandArgs + Args *commandargs.Shell ReadWriter *readwriter.ReadWriter } diff --git a/go/internal/command/discover/discover_test.go b/go/internal/command/discover/discover_test.go index 284610a..7e052f7 100644 --- a/go/internal/command/discover/discover_test.go +++ b/go/internal/command/discover/discover_test.go @@ -49,27 +49,27 @@ func TestExecute(t *testing.T) { testCases := []struct { desc string - arguments *commandargs.CommandArgs + arguments *commandargs.Shell expectedOutput string }{ { desc: "With a known username", - arguments: &commandargs.CommandArgs{GitlabUsername: "alex-doe"}, + arguments: &commandargs.Shell{GitlabUsername: "alex-doe"}, expectedOutput: "Welcome to GitLab, @alex-doe!\n", }, { desc: "With a known key id", - arguments: &commandargs.CommandArgs{GitlabKeyId: "1"}, + arguments: &commandargs.Shell{GitlabKeyId: "1"}, expectedOutput: "Welcome to GitLab, @alex-doe!\n", }, { desc: "With an unknown key", - arguments: &commandargs.CommandArgs{GitlabKeyId: "-1"}, + arguments: &commandargs.Shell{GitlabKeyId: "-1"}, expectedOutput: "Welcome to GitLab, Anonymous!\n", }, { desc: "With an unknown username", - arguments: &commandargs.CommandArgs{GitlabUsername: "unknown"}, + arguments: &commandargs.Shell{GitlabUsername: "unknown"}, expectedOutput: "Welcome to GitLab, Anonymous!\n", }, } @@ -97,22 +97,22 @@ func TestFailingExecute(t *testing.T) { testCases := []struct { desc string - arguments *commandargs.CommandArgs + arguments *commandargs.Shell expectedError string }{ { desc: "With missing arguments", - arguments: &commandargs.CommandArgs{}, + arguments: &commandargs.Shell{}, expectedError: "Failed to get username: who='' is invalid", }, { desc: "When the API returns an error", - arguments: &commandargs.CommandArgs{GitlabUsername: "broken_message"}, + arguments: &commandargs.Shell{GitlabUsername: "broken_message"}, expectedError: "Failed to get username: Forbidden!", }, { desc: "When the API fails", - arguments: &commandargs.CommandArgs{GitlabUsername: "broken"}, + arguments: &commandargs.Shell{GitlabUsername: "broken"}, expectedError: "Failed to get username: Internal API error (500)", }, } diff --git a/go/internal/command/fallback/fallback.go b/go/internal/command/fallback/fallback.go index f525a57..781eda1 100644 --- a/go/internal/command/fallback/fallback.go +++ b/go/internal/command/fallback/fallback.go @@ -1,30 +1,57 @@ package fallback import ( + "errors" + "fmt" "os" "path/filepath" "syscall" + + "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/commandargs" + "gitlab.com/gitlab-org/gitlab-shell/go/internal/executable" ) type Command struct { - RootDir string - Args []string + Executable *executable.Executable + RootDir string + Args commandargs.CommandArgs } var ( // execFunc is overridden in tests - execFunc = syscall.Exec -) - -const ( - RubyProgram = "gitlab-shell-ruby" + execFunc = syscall.Exec + whitelist = []string{ + executable.GitlabShell, + executable.AuthorizedKeysCheck, + executable.AuthorizedPrincipalsCheck, + } ) func (c *Command) Execute() error { - rubyCmd := filepath.Join(c.RootDir, "bin", RubyProgram) + if !c.isWhitelisted() { + return errors.New("Failed to execute unknown executable") + } + + rubyCmd := c.fallbackProgram() // Ensure rubyArgs[0] is the full path to gitlab-shell-ruby - rubyArgs := append([]string{rubyCmd}, c.Args[1:]...) + rubyArgs := append([]string{rubyCmd}, c.Args.GetArguments()...) return execFunc(rubyCmd, rubyArgs, os.Environ()) } + +func (c *Command) isWhitelisted() bool { + for _, item := range whitelist { + if c.Executable.Name == item { + return true + } + } + + return false +} + +func (c *Command) fallbackProgram() string { + fileName := fmt.Sprintf("%s-ruby", c.Executable.Name) + + return filepath.Join(c.RootDir, "bin", fileName) +} diff --git a/go/internal/command/fallback/fallback_test.go b/go/internal/command/fallback/fallback_test.go index afd752b..7485084 100644 --- a/go/internal/command/fallback/fallback_test.go +++ b/go/internal/command/fallback/fallback_test.go @@ -6,6 +6,9 @@ import ( "testing" "github.com/stretchr/testify/require" + + "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/commandargs" + "gitlab.com/gitlab-org/gitlab-shell/go/internal/executable" ) type fakeExec struct { @@ -19,7 +22,7 @@ type fakeExec struct { } var ( - fakeArgs = []string{"./test", "foo", "bar"} + fakeArgs = &commandargs.GenericArgs{Arguments: []string{"foo", "bar"}} ) func (f *fakeExec) Exec(filename string, args []string, env []string) error { @@ -42,7 +45,7 @@ func (f *fakeExec) Cleanup() { } func TestExecuteExecsCommandSuccesfully(t *testing.T) { - cmd := &Command{RootDir: "/tmp", Args: fakeArgs} + cmd := &Command{Executable: &executable.Executable{Name: executable.GitlabShell}, RootDir: "/tmp", Args: fakeArgs} // Override the exec func fake := &fakeExec{} @@ -56,8 +59,14 @@ func TestExecuteExecsCommandSuccesfully(t *testing.T) { require.Equal(t, fake.Env, os.Environ()) } +func TestExecuteExecsUnknownExecutable(t *testing.T) { + cmd := &Command{Executable: &executable.Executable{Name: "unknown"}, RootDir: "/test"} + + require.Error(t, cmd.Execute()) +} + func TestExecuteExecsCommandOnError(t *testing.T) { - cmd := &Command{RootDir: "/test", Args: fakeArgs} + cmd := &Command{Executable: &executable.Executable{Name: executable.GitlabShell}, RootDir: "/test", Args: fakeArgs} // Override the exec func fake := &fakeExec{Error: errors.New("Test error")} @@ -69,7 +78,7 @@ func TestExecuteExecsCommandOnError(t *testing.T) { } func TestExecuteGivenNonexistentCommand(t *testing.T) { - cmd := &Command{RootDir: "/tmp/does/not/exist", Args: fakeArgs} + cmd := &Command{Executable: &executable.Executable{Name: executable.GitlabShell}, RootDir: "/tmp/does/not/exist", Args: fakeArgs} require.Error(t, cmd.Execute()) } diff --git a/go/internal/command/lfsauthenticate/lfsauthenticate.go b/go/internal/command/lfsauthenticate/lfsauthenticate.go index c1dc45f..bff5e7f 100644 --- a/go/internal/command/lfsauthenticate/lfsauthenticate.go +++ b/go/internal/command/lfsauthenticate/lfsauthenticate.go @@ -20,7 +20,7 @@ const ( type Command struct { Config *config.Config - Args *commandargs.CommandArgs + Args *commandargs.Shell ReadWriter *readwriter.ReadWriter } diff --git a/go/internal/command/lfsauthenticate/lfsauthenticate_test.go b/go/internal/command/lfsauthenticate/lfsauthenticate_test.go index 30da94b..a6836a8 100644 --- a/go/internal/command/lfsauthenticate/lfsauthenticate_test.go +++ b/go/internal/command/lfsauthenticate/lfsauthenticate_test.go @@ -25,22 +25,22 @@ func TestFailedRequests(t *testing.T) { testCases := []struct { desc string - arguments *commandargs.CommandArgs + arguments *commandargs.Shell expectedOutput string }{ { desc: "With missing arguments", - arguments: &commandargs.CommandArgs{}, + arguments: &commandargs.Shell{}, expectedOutput: "> GitLab: Disallowed command", }, { desc: "With disallowed command", - arguments: &commandargs.CommandArgs{GitlabKeyId: "1", SshArgs: []string{"git-lfs-authenticate", "group/repo", "unknown"}}, + arguments: &commandargs.Shell{GitlabKeyId: "1", SshArgs: []string{"git-lfs-authenticate", "group/repo", "unknown"}}, expectedOutput: "> GitLab: Disallowed command", }, { desc: "With disallowed user", - arguments: &commandargs.CommandArgs{GitlabKeyId: "disallowed", SshArgs: []string{"git-lfs-authenticate", "group/repo", "download"}}, + arguments: &commandargs.Shell{GitlabKeyId: "disallowed", SshArgs: []string{"git-lfs-authenticate", "group/repo", "download"}}, expectedOutput: "Disallowed by API call", }, } @@ -140,7 +140,7 @@ func TestLfsAuthenticateRequests(t *testing.T) { output := &bytes.Buffer{} cmd := &Command{ Config: &config.Config{GitlabUrl: url}, - Args: &commandargs.CommandArgs{GitlabUsername: tc.username, SshArgs: []string{"git-lfs-authenticate", "group/repo", "upload"}}, + Args: &commandargs.Shell{GitlabUsername: tc.username, SshArgs: []string{"git-lfs-authenticate", "group/repo", "upload"}}, ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output}, } diff --git a/go/internal/command/receivepack/customaction_test.go b/go/internal/command/receivepack/customaction_test.go index 80e849c..bd4991d 100644 --- a/go/internal/command/receivepack/customaction_test.go +++ b/go/internal/command/receivepack/customaction_test.go @@ -92,7 +92,7 @@ func TestCustomReceivePack(t *testing.T) { cmd := &Command{ Config: &config.Config{GitlabUrl: url}, - Args: &commandargs.CommandArgs{GitlabKeyId: keyId, CommandType: commandargs.ReceivePack, SshArgs: []string{"git-receive-pack", repo}}, + Args: &commandargs.Shell{GitlabKeyId: keyId, CommandType: commandargs.ReceivePack, SshArgs: []string{"git-receive-pack", repo}}, ReadWriter: &readwriter.ReadWriter{ErrOut: errBuf, Out: outBuf, In: input}, } diff --git a/go/internal/command/receivepack/gitalycall_test.go b/go/internal/command/receivepack/gitalycall_test.go index 0914be6..eac9218 100644 --- a/go/internal/command/receivepack/gitalycall_test.go +++ b/go/internal/command/receivepack/gitalycall_test.go @@ -29,7 +29,7 @@ func TestReceivePack(t *testing.T) { cmd := &Command{ Config: &config.Config{GitlabUrl: url}, - Args: &commandargs.CommandArgs{GitlabKeyId: userId, CommandType: commandargs.ReceivePack, SshArgs: []string{"git-receive-pack", repo}}, + Args: &commandargs.Shell{GitlabKeyId: userId, CommandType: commandargs.ReceivePack, SshArgs: []string{"git-receive-pack", repo}}, ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output, In: input}, } diff --git a/go/internal/command/receivepack/receivepack.go b/go/internal/command/receivepack/receivepack.go index d6b788c..eb0b2fe 100644 --- a/go/internal/command/receivepack/receivepack.go +++ b/go/internal/command/receivepack/receivepack.go @@ -10,7 +10,7 @@ import ( type Command struct { Config *config.Config - Args *commandargs.CommandArgs + Args *commandargs.Shell ReadWriter *readwriter.ReadWriter } diff --git a/go/internal/command/receivepack/receivepack_test.go b/go/internal/command/receivepack/receivepack_test.go index e5263f5..a45d054 100644 --- a/go/internal/command/receivepack/receivepack_test.go +++ b/go/internal/command/receivepack/receivepack_test.go @@ -23,7 +23,7 @@ func TestForbiddenAccess(t *testing.T) { cmd := &Command{ Config: &config.Config{GitlabUrl: url}, - Args: &commandargs.CommandArgs{GitlabKeyId: "disallowed", SshArgs: []string{"git-receive-pack", "group/repo"}}, + Args: &commandargs.Shell{GitlabKeyId: "disallowed", SshArgs: []string{"git-receive-pack", "group/repo"}}, ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output, In: input}, } diff --git a/go/internal/command/shared/accessverifier/accessverifier.go b/go/internal/command/shared/accessverifier/accessverifier.go index 6d13789..fc6fa17 100644 --- a/go/internal/command/shared/accessverifier/accessverifier.go +++ b/go/internal/command/shared/accessverifier/accessverifier.go @@ -14,7 +14,7 @@ type Response = accessverifier.Response type Command struct { Config *config.Config - Args *commandargs.CommandArgs + Args *commandargs.Shell ReadWriter *readwriter.ReadWriter } diff --git a/go/internal/command/shared/accessverifier/accessverifier_test.go b/go/internal/command/shared/accessverifier/accessverifier_test.go index dd95ded..c19ed37 100644 --- a/go/internal/command/shared/accessverifier/accessverifier_test.go +++ b/go/internal/command/shared/accessverifier/accessverifier_test.go @@ -64,7 +64,7 @@ func TestMissingUser(t *testing.T) { cmd, _, _, cleanup := setup(t) defer cleanup() - cmd.Args = &commandargs.CommandArgs{GitlabKeyId: "2"} + cmd.Args = &commandargs.Shell{GitlabKeyId: "2"} _, err := cmd.Verify(action, repo) require.Equal(t, "missing user", err.Error()) @@ -74,7 +74,7 @@ func TestConsoleMessages(t *testing.T) { cmd, errBuf, outBuf, cleanup := setup(t) defer cleanup() - cmd.Args = &commandargs.CommandArgs{GitlabKeyId: "1"} + cmd.Args = &commandargs.Shell{GitlabKeyId: "1"} cmd.Verify(action, repo) require.Equal(t, "> GitLab: console\n> GitLab: message\n", errBuf.String()) diff --git a/go/internal/command/twofactorrecover/twofactorrecover.go b/go/internal/command/twofactorrecover/twofactorrecover.go index faa35db..c68080a 100644 --- a/go/internal/command/twofactorrecover/twofactorrecover.go +++ b/go/internal/command/twofactorrecover/twofactorrecover.go @@ -12,7 +12,7 @@ import ( type Command struct { Config *config.Config - Args *commandargs.CommandArgs + Args *commandargs.Shell ReadWriter *readwriter.ReadWriter } diff --git a/go/internal/command/twofactorrecover/twofactorrecover_test.go b/go/internal/command/twofactorrecover/twofactorrecover_test.go index 6238e0d..291d499 100644 --- a/go/internal/command/twofactorrecover/twofactorrecover_test.go +++ b/go/internal/command/twofactorrecover/twofactorrecover_test.go @@ -69,13 +69,13 @@ func TestExecute(t *testing.T) { testCases := []struct { desc string - arguments *commandargs.CommandArgs + arguments *commandargs.Shell answer string expectedOutput string }{ { desc: "With a known key id", - arguments: &commandargs.CommandArgs{GitlabKeyId: "1"}, + arguments: &commandargs.Shell{GitlabKeyId: "1"}, answer: "yes\n", expectedOutput: question + "Your two-factor authentication recovery codes are:\n\nrecovery\ncodes\n\n" + @@ -85,31 +85,31 @@ func TestExecute(t *testing.T) { }, { desc: "With bad response", - arguments: &commandargs.CommandArgs{GitlabKeyId: "-1"}, + arguments: &commandargs.Shell{GitlabKeyId: "-1"}, answer: "yes\n", expectedOutput: question + errorHeader + "Parsing failed\n", }, { desc: "With API returns an error", - arguments: &commandargs.CommandArgs{GitlabKeyId: "forbidden"}, + arguments: &commandargs.Shell{GitlabKeyId: "forbidden"}, answer: "yes\n", expectedOutput: question + errorHeader + "Forbidden!\n", }, { desc: "With API fails", - arguments: &commandargs.CommandArgs{GitlabKeyId: "broken"}, + arguments: &commandargs.Shell{GitlabKeyId: "broken"}, answer: "yes\n", expectedOutput: question + errorHeader + "Internal API error (500)\n", }, { desc: "With missing arguments", - arguments: &commandargs.CommandArgs{}, + arguments: &commandargs.Shell{}, answer: "yes\n", expectedOutput: question + errorHeader + "who='' is invalid\n", }, { desc: "With negative answer", - arguments: &commandargs.CommandArgs{}, + arguments: &commandargs.Shell{}, answer: "no\n", expectedOutput: question + "New recovery codes have *not* been generated. Existing codes will remain valid.\n", diff --git a/go/internal/command/uploadarchive/gitalycall_test.go b/go/internal/command/uploadarchive/gitalycall_test.go index 78953a7..5eb2eae 100644 --- a/go/internal/command/uploadarchive/gitalycall_test.go +++ b/go/internal/command/uploadarchive/gitalycall_test.go @@ -29,7 +29,7 @@ func TestUploadPack(t *testing.T) { cmd := &Command{ Config: &config.Config{GitlabUrl: url}, - Args: &commandargs.CommandArgs{GitlabKeyId: userId, CommandType: commandargs.UploadArchive, SshArgs: []string{"git-upload-archive", repo}}, + Args: &commandargs.Shell{GitlabKeyId: userId, CommandType: commandargs.UploadArchive, SshArgs: []string{"git-upload-archive", repo}}, ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output, In: input}, } diff --git a/go/internal/command/uploadarchive/uploadarchive.go b/go/internal/command/uploadarchive/uploadarchive.go index 93a52c0..2846455 100644 --- a/go/internal/command/uploadarchive/uploadarchive.go +++ b/go/internal/command/uploadarchive/uploadarchive.go @@ -10,7 +10,7 @@ import ( type Command struct { Config *config.Config - Args *commandargs.CommandArgs + Args *commandargs.Shell ReadWriter *readwriter.ReadWriter } diff --git a/go/internal/command/uploadarchive/uploadarchive_test.go b/go/internal/command/uploadarchive/uploadarchive_test.go index 369bee7..4cd6832 100644 --- a/go/internal/command/uploadarchive/uploadarchive_test.go +++ b/go/internal/command/uploadarchive/uploadarchive_test.go @@ -22,7 +22,7 @@ func TestForbiddenAccess(t *testing.T) { cmd := &Command{ Config: &config.Config{GitlabUrl: url}, - Args: &commandargs.CommandArgs{GitlabKeyId: "disallowed", SshArgs: []string{"git-upload-archive", "group/repo"}}, + Args: &commandargs.Shell{GitlabKeyId: "disallowed", SshArgs: []string{"git-upload-archive", "group/repo"}}, ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output}, } diff --git a/go/internal/command/uploadpack/gitalycall_test.go b/go/internal/command/uploadpack/gitalycall_test.go index 2097964..eb18aa8 100644 --- a/go/internal/command/uploadpack/gitalycall_test.go +++ b/go/internal/command/uploadpack/gitalycall_test.go @@ -29,7 +29,7 @@ func TestUploadPack(t *testing.T) { cmd := &Command{ Config: &config.Config{GitlabUrl: url}, - Args: &commandargs.CommandArgs{GitlabKeyId: userId, CommandType: commandargs.UploadPack, SshArgs: []string{"git-upload-pack", repo}}, + Args: &commandargs.Shell{GitlabKeyId: userId, CommandType: commandargs.UploadPack, SshArgs: []string{"git-upload-pack", repo}}, ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output, In: input}, } diff --git a/go/internal/command/uploadpack/uploadpack.go b/go/internal/command/uploadpack/uploadpack.go index cff198d..4b08bf2 100644 --- a/go/internal/command/uploadpack/uploadpack.go +++ b/go/internal/command/uploadpack/uploadpack.go @@ -10,7 +10,7 @@ import ( type Command struct { Config *config.Config - Args *commandargs.CommandArgs + Args *commandargs.Shell ReadWriter *readwriter.ReadWriter } diff --git a/go/internal/command/uploadpack/uploadpack_test.go b/go/internal/command/uploadpack/uploadpack_test.go index a06ba24..27a0786 100644 --- a/go/internal/command/uploadpack/uploadpack_test.go +++ b/go/internal/command/uploadpack/uploadpack_test.go @@ -22,7 +22,7 @@ func TestForbiddenAccess(t *testing.T) { cmd := &Command{ Config: &config.Config{GitlabUrl: url}, - Args: &commandargs.CommandArgs{GitlabKeyId: "disallowed", SshArgs: []string{"git-upload-pack", "group/repo"}}, + Args: &commandargs.Shell{GitlabKeyId: "disallowed", SshArgs: []string{"git-upload-pack", "group/repo"}}, ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output}, } diff --git a/go/internal/executable/executable.go b/go/internal/executable/executable.go new file mode 100644 index 0000000..63f4c90 --- /dev/null +++ b/go/internal/executable/executable.go @@ -0,0 +1,58 @@ +package executable + +import ( + "os" + "path/filepath" +) + +const ( + GitlabShell = "gitlab-shell" + AuthorizedKeysCheck = "gitlab-shell-authorized-keys-check" + AuthorizedPrincipalsCheck = "gitlab-shell-authorized-principals-check" +) + +type Executable struct { + Name string + RootDir string +} + +var ( + // osExecutable is overridden in tests + osExecutable = os.Executable +) + +func New() (*Executable, error) { + path, err := osExecutable() + if err != nil { + return nil, err + } + + rootDir, err := findRootDir(path) + if err != nil { + return nil, err + } + + executable := &Executable{ + Name: filepath.Base(path), + RootDir: rootDir, + } + + return executable, nil +} + +func findRootDir(path string) (string, error) { + // Start: /opt/.../gitlab-shell/bin/gitlab-shell + // Ends: /opt/.../gitlab-shell + rootDir := filepath.Dir(filepath.Dir(path)) + pathFromEnv := os.Getenv("GITLAB_SHELL_DIR") + + if pathFromEnv != "" { + if _, err := os.Stat(pathFromEnv); os.IsNotExist(err) { + return "", err + } + + rootDir = pathFromEnv + } + + return rootDir, nil +} diff --git a/go/internal/executable/executable_test.go b/go/internal/executable/executable_test.go new file mode 100644 index 0000000..4d2174b --- /dev/null +++ b/go/internal/executable/executable_test.go @@ -0,0 +1,104 @@ +package executable + +import ( + "errors" + "testing" + + "gitlab.com/gitlab-org/gitlab-shell/go/internal/testhelper" + + "github.com/stretchr/testify/require" +) + +type fakeOs struct { + OldExecutable func() (string, error) + Path string + Error error +} + +func (f *fakeOs) Executable() (string, error) { + return f.Path, f.Error +} + +func (f *fakeOs) Setup() { + f.OldExecutable = osExecutable + osExecutable = f.Executable +} + +func (f *fakeOs) Cleanup() { + osExecutable = f.OldExecutable +} + +func TestNewSuccess(t *testing.T) { + testCases := []struct { + desc string + fakeOs *fakeOs + environment map[string]string + expectedRootDir string + }{ + { + desc: "GITLAB_SHELL_DIR env var is not defined", + fakeOs: &fakeOs{Path: "/tmp/bin/gitlab-shell"}, + expectedRootDir: "/tmp", + }, + { + desc: "GITLAB_SHELL_DIR env var is defined", + fakeOs: &fakeOs{Path: "/opt/bin/gitlab-shell"}, + environment: map[string]string{ + "GITLAB_SHELL_DIR": "/tmp", + }, + expectedRootDir: "/tmp", + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + restoreEnv := testhelper.TempEnv(tc.environment) + defer restoreEnv() + + fake := tc.fakeOs + fake.Setup() + defer fake.Cleanup() + + result, err := New() + + require.NoError(t, err) + require.Equal(t, result.Name, "gitlab-shell") + require.Equal(t, result.RootDir, tc.expectedRootDir) + }) + } +} + +func TestNewFailure(t *testing.T) { + testCases := []struct { + desc string + fakeOs *fakeOs + environment map[string]string + }{ + { + desc: "failed to determine executable", + fakeOs: &fakeOs{Path: "", Error: errors.New("error")}, + }, + { + desc: "GITLAB_SHELL_DIR doesn't exist", + fakeOs: &fakeOs{Path: "/tmp/bin/gitlab-shell"}, + environment: map[string]string{ + "GITLAB_SHELL_DIR": "/tmp/non/existing/directory", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + restoreEnv := testhelper.TempEnv(tc.environment) + defer restoreEnv() + + fake := tc.fakeOs + fake.Setup() + defer fake.Cleanup() + + _, err := New() + + require.Error(t, err) + }) + } +} diff --git a/go/internal/gitlabnet/accessverifier/client.go b/go/internal/gitlabnet/accessverifier/client.go index d87c4ad..92a7434 100644 --- a/go/internal/gitlabnet/accessverifier/client.go +++ b/go/internal/gitlabnet/accessverifier/client.go @@ -71,7 +71,7 @@ func NewClient(config *config.Config) (*Client, error) { return &Client{client: client}, nil } -func (c *Client) Verify(args *commandargs.CommandArgs, action commandargs.CommandType, repo string) (*Response, error) { +func (c *Client) Verify(args *commandargs.Shell, action commandargs.CommandType, repo string) (*Response, error) { request := &Request{Action: action, Repo: repo, Protocol: protocol, Changes: anyChanges} if args.GitlabUsername != "" { @@ -89,7 +89,7 @@ func (c *Client) Verify(args *commandargs.CommandArgs, action commandargs.Comman return parse(response, args) } -func parse(hr *http.Response, args *commandargs.CommandArgs) (*Response, error) { +func parse(hr *http.Response, args *commandargs.Shell) (*Response, error) { response := &Response{} if err := gitlabnet.ParseJSON(hr, response); err != nil { return nil, err diff --git a/go/internal/gitlabnet/accessverifier/client_test.go b/go/internal/gitlabnet/accessverifier/client_test.go index 31175ae..f534185 100644 --- a/go/internal/gitlabnet/accessverifier/client_test.go +++ b/go/internal/gitlabnet/accessverifier/client_test.go @@ -57,16 +57,16 @@ func TestSuccessfulResponses(t *testing.T) { testCases := []struct { desc string - args *commandargs.CommandArgs + args *commandargs.Shell who string }{ { desc: "Provide key id within the request", - args: &commandargs.CommandArgs{GitlabKeyId: "1"}, + args: &commandargs.Shell{GitlabKeyId: "1"}, who: "key-1", }, { desc: "Provide username within the request", - args: &commandargs.CommandArgs{GitlabUsername: "first"}, + args: &commandargs.Shell{GitlabUsername: "first"}, who: "user-1", }, } @@ -86,7 +86,7 @@ func TestGetCustomAction(t *testing.T) { client, cleanup := setup(t) defer cleanup() - args := &commandargs.CommandArgs{GitlabUsername: "custom"} + args := &commandargs.Shell{GitlabUsername: "custom"} result, err := client.Verify(args, action, repo) require.NoError(t, err) @@ -134,7 +134,7 @@ func TestErrorResponses(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - args := &commandargs.CommandArgs{GitlabKeyId: tc.fakeId} + args := &commandargs.Shell{GitlabKeyId: tc.fakeId} resp, err := client.Verify(args, action, repo) require.EqualError(t, err, tc.expectedError) diff --git a/go/internal/gitlabnet/discover/client.go b/go/internal/gitlabnet/discover/client.go index 675670d..46ab2de 100644 --- a/go/internal/gitlabnet/discover/client.go +++ b/go/internal/gitlabnet/discover/client.go @@ -30,7 +30,7 @@ func NewClient(config *config.Config) (*Client, error) { return &Client{config: config, client: client}, nil } -func (c *Client) GetByCommandArgs(args *commandargs.CommandArgs) (*Response, error) { +func (c *Client) GetByCommandArgs(args *commandargs.Shell) (*Response, error) { params := url.Values{} if args.GitlabUsername != "" { params.Add("username", args.GitlabUsername) diff --git a/go/internal/gitlabnet/lfsauthenticate/client.go b/go/internal/gitlabnet/lfsauthenticate/client.go index 2a7cb03..51cb7a4 100644 --- a/go/internal/gitlabnet/lfsauthenticate/client.go +++ b/go/internal/gitlabnet/lfsauthenticate/client.go @@ -13,7 +13,7 @@ import ( type Client struct { config *config.Config client *gitlabnet.GitlabClient - args *commandargs.CommandArgs + args *commandargs.Shell } type Request struct { @@ -30,7 +30,7 @@ type Response struct { ExpiresIn int `json:"expires_in"` } -func NewClient(config *config.Config, args *commandargs.CommandArgs) (*Client, error) { +func NewClient(config *config.Config, args *commandargs.Shell) (*Client, error) { client, err := gitlabnet.GetClient(config) if err != nil { return nil, fmt.Errorf("Error creating http client: %v", err) diff --git a/go/internal/gitlabnet/lfsauthenticate/client_test.go b/go/internal/gitlabnet/lfsauthenticate/client_test.go index 7fd7aca..07484a7 100644 --- a/go/internal/gitlabnet/lfsauthenticate/client_test.go +++ b/go/internal/gitlabnet/lfsauthenticate/client_test.go @@ -59,22 +59,22 @@ func TestFailedRequests(t *testing.T) { testCases := []struct { desc string - args *commandargs.CommandArgs + args *commandargs.Shell expectedOutput string }{ { desc: "With bad response", - args: &commandargs.CommandArgs{GitlabKeyId: "-1", CommandType: commandargs.UploadPack}, + args: &commandargs.Shell{GitlabKeyId: "-1", CommandType: commandargs.UploadPack}, expectedOutput: "Parsing failed", }, { desc: "With API returns an error", - args: &commandargs.CommandArgs{GitlabKeyId: "forbidden", CommandType: commandargs.UploadPack}, + args: &commandargs.Shell{GitlabKeyId: "forbidden", CommandType: commandargs.UploadPack}, expectedOutput: "Internal API error (403)", }, { desc: "With API fails", - args: &commandargs.CommandArgs{GitlabKeyId: "broken", CommandType: commandargs.UploadPack}, + args: &commandargs.Shell{GitlabKeyId: "broken", CommandType: commandargs.UploadPack}, expectedOutput: "Internal API error (500)", }, } @@ -99,7 +99,7 @@ func TestSuccessfulRequests(t *testing.T) { url, cleanup := testserver.StartHttpServer(t, requests) defer cleanup() - args := &commandargs.CommandArgs{GitlabKeyId: keyId, CommandType: commandargs.LfsAuthenticate} + args := &commandargs.Shell{GitlabKeyId: keyId, CommandType: commandargs.LfsAuthenticate} client, err := NewClient(&config.Config{GitlabUrl: url}, args) require.NoError(t, err) diff --git a/go/internal/gitlabnet/twofactorrecover/client.go b/go/internal/gitlabnet/twofactorrecover/client.go index f90a85c..37067db 100644 --- a/go/internal/gitlabnet/twofactorrecover/client.go +++ b/go/internal/gitlabnet/twofactorrecover/client.go @@ -36,7 +36,7 @@ func NewClient(config *config.Config) (*Client, error) { return &Client{config: config, client: client}, nil } -func (c *Client) GetRecoveryCodes(args *commandargs.CommandArgs) ([]string, error) { +func (c *Client) GetRecoveryCodes(args *commandargs.Shell) ([]string, error) { requestBody, err := c.getRequestBody(args) if err != nil { @@ -65,7 +65,7 @@ func parse(hr *http.Response) ([]string, error) { return response.RecoveryCodes, nil } -func (c *Client) getRequestBody(args *commandargs.CommandArgs) (*RequestBody, error) { +func (c *Client) getRequestBody(args *commandargs.Shell) (*RequestBody, error) { client, err := discover.NewClient(c.config) if err != nil { diff --git a/go/internal/gitlabnet/twofactorrecover/client_test.go b/go/internal/gitlabnet/twofactorrecover/client_test.go index 4b15ac5..a560fb1 100644 --- a/go/internal/gitlabnet/twofactorrecover/client_test.go +++ b/go/internal/gitlabnet/twofactorrecover/client_test.go @@ -85,7 +85,7 @@ func TestGetRecoveryCodesByKeyId(t *testing.T) { client, cleanup := setup(t) defer cleanup() - args := &commandargs.CommandArgs{GitlabKeyId: "0"} + args := &commandargs.Shell{GitlabKeyId: "0"} result, err := client.GetRecoveryCodes(args) assert.NoError(t, err) assert.Equal(t, []string{"recovery 1", "codes 1"}, result) @@ -95,7 +95,7 @@ func TestGetRecoveryCodesByUsername(t *testing.T) { client, cleanup := setup(t) defer cleanup() - args := &commandargs.CommandArgs{GitlabUsername: "jane-doe"} + args := &commandargs.Shell{GitlabUsername: "jane-doe"} result, err := client.GetRecoveryCodes(args) assert.NoError(t, err) assert.Equal(t, []string{"recovery 2", "codes 2"}, result) @@ -105,7 +105,7 @@ func TestMissingUser(t *testing.T) { client, cleanup := setup(t) defer cleanup() - args := &commandargs.CommandArgs{GitlabKeyId: "1"} + args := &commandargs.Shell{GitlabKeyId: "1"} _, err := client.GetRecoveryCodes(args) assert.Equal(t, "missing user", err.Error()) } @@ -138,7 +138,7 @@ func TestErrorResponses(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - args := &commandargs.CommandArgs{GitlabKeyId: tc.fakeId} + args := &commandargs.Shell{GitlabKeyId: tc.fakeId} resp, err := client.GetRecoveryCodes(args) assert.EqualError(t, err, tc.expectedError) diff --git a/spec/gitlab_shell_authorized_keys_check_spec.rb b/spec/gitlab_shell_authorized_keys_check_spec.rb index 7050604..eb0cbca 100644 --- a/spec/gitlab_shell_authorized_keys_check_spec.rb +++ b/spec/gitlab_shell_authorized_keys_check_spec.rb @@ -21,54 +21,84 @@ describe 'bin/gitlab-shell-authorized-keys-check' do end end - before(:all) do - write_config( - "gitlab_url" => "http+unix://#{CGI.escape(tmp_socket_path)}", - ) - end - let(:authorized_keys_check_path) { File.join(tmp_root_path, 'bin', 'gitlab-shell-authorized-keys-check') } - it 'succeeds when a valid key is given' do - output, status = run! + shared_examples 'authorized keys check' do + it 'succeeds when a valid key is given' do + output, status = run! - expect(output).to eq("command=\"#{gitlab_shell_path} key-1\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty known-rsa-key\n") - expect(status).to be_success - end + expect(output).to eq("command=\"#{gitlab_shell_path} key-1\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty known-rsa-key\n") + expect(status).to be_success + end - it 'returns nothing when an unknown key is given' do - output, status = run!(key: 'unknown-key') + it 'returns nothing when an unknown key is given' do + output, status = run!(key: 'unknown-key') - expect(output).to eq("# No key was found for unknown-key\n") - expect(status).to be_success - end + expect(output).to eq("# No key was found for unknown-key\n") + expect(status).to be_success + end + + it' fails when not enough arguments are given' do + output, status = run!(key: nil) - it' fails when not enough arguments are given' do - output, status = run!(key: nil) + expect(output).to eq('') + expect(status).not_to be_success + end - expect(output).to eq('') - expect(status).not_to be_success + it' fails when too many arguments are given' do + output, status = run!(key: ['a', 'b']) + + expect(output).to eq('') + expect(status).not_to be_success + end + + it 'skips when run as the wrong user' do + output, status = run!(expected_user: 'unknown-user') + + expect(output).to eq('') + expect(status).to be_success + end + + it 'skips when the wrong users connects' do + output, status = run!(actual_user: 'unknown-user') + + expect(output).to eq('') + expect(status).to be_success + end end - it' fails when too many arguments are given' do - output, status = run!(key: ['a', 'b']) + describe 'without go features' do + before(:all) do + write_config( + "gitlab_url" => "http+unix://#{CGI.escape(tmp_socket_path)}", + ) + end - expect(output).to eq('') - expect(status).not_to be_success + it_behaves_like 'authorized keys check' end - it 'skips when run as the wrong user' do - output, status = run!(expected_user: 'unknown-user') + describe 'without go features (via go)', :go do + before(:all) do + write_config( + "gitlab_url" => "http+unix://#{CGI.escape(tmp_socket_path)}", + ) + end - expect(output).to eq('') - expect(status).to be_success + it_behaves_like 'authorized keys check' end - it 'skips when the wrong users connects' do - output, status = run!(actual_user: 'unknown-user') + pending 'with the go authorized-keys-check feature', :go do + before(:all) do + write_config( + 'gitlab_url' => "http+unix://#{CGI.escape(tmp_socket_path)}", + 'migration' => { + 'enabled' => true, + 'features' => ['authorized-keys-check'] + } + ) + end - expect(output).to eq('') - expect(status).to be_success + it_behaves_like 'authorized keys check' end def run!(expected_user: 'git', actual_user: 'git', key: 'known-rsa-key') diff --git a/spec/gitlab_shell_authorized_principals_check_spec.rb b/spec/gitlab_shell_authorized_principals_check_spec.rb new file mode 100644 index 0000000..8b946bc --- /dev/null +++ b/spec/gitlab_shell_authorized_principals_check_spec.rb @@ -0,0 +1,82 @@ +require_relative 'spec_helper' + +describe 'bin/gitlab-shell-authorized-principals-check' do + include_context 'gitlab shell' + + def mock_server(server) + # Do nothing as we're not connecting to a server in this check. + end + + let(:authorized_principals_check_path) { File.join(tmp_root_path, 'bin', 'gitlab-shell-authorized-principals-check') } + + shared_examples 'authorized principals check' do + it 'succeeds when a valid principal is given' do + output, status = run! + + expect(output).to eq("command=\"#{gitlab_shell_path} username-key\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty principal\n") + expect(status).to be_success + end + + it 'fails when not enough arguments are given' do + output, status = run!(key_id: nil, principals: []) + + expect(output).to eq('') + expect(status).not_to be_success + end + + it 'fails when key_id is blank' do + output, status = run!(key_id: '') + + expect(output).to eq('') + expect(status).not_to be_success + end + + it 'fails when principals include an empty item' do + output, status = run!(principals: ['principal', '']) + + expect(output).to eq('') + expect(status).not_to be_success + end + end + + describe 'without go features' do + before(:all) do + write_config({}) + end + + it_behaves_like 'authorized principals check' + end + + describe 'without go features (via go)', :go do + before(:all) do + write_config({}) + end + + it_behaves_like 'authorized principals check' + end + + pending 'with the go authorized-principals-check feature', :go do + before(:all) do + write_config( + 'migration' => { + 'enabled' => true, + 'features' => ['authorized-principals-check'] + } + ) + end + + it_behaves_like 'authorized principals check' + end + + def run!(key_id: 'key', principals: ['principal']) + cmd = [ + authorized_principals_check_path, + key_id, + principals, + ].flatten.compact + + output = IO.popen(cmd, &:read) + + [output, $?] + end +end diff --git a/spec/gitlab_shell_gitlab_shell_spec.rb b/spec/gitlab_shell_gitlab_shell_spec.rb index 6d6e172..3e714d8 100644 --- a/spec/gitlab_shell_gitlab_shell_spec.rb +++ b/spec/gitlab_shell_gitlab_shell_spec.rb @@ -123,10 +123,10 @@ describe 'bin/gitlab-shell' do it_behaves_like 'results with keys' - it 'outputs "Only ssh allowed"' do + it 'outputs "Only SSH allowed"' do _, stderr, status = run!(["-c/usr/share/webapps/gitlab-shell/bin/gitlab-shell", "username-someuser"], env: {}) - expect(stderr).to eq("Only ssh allowed\n") + expect(stderr).to eq("Only SSH allowed\n") expect(status).not_to be_success end |