diff options
author | Patrick Bajao <ebajao@gitlab.com> | 2019-08-08 15:29:33 +0800 |
---|---|---|
committer | Patrick Bajao <ebajao@gitlab.com> | 2019-08-09 09:17:35 +0800 |
commit | 1962b49971cf1602ed5ee20b33193e10022cf8cb (patch) | |
tree | 6a12b351838af281ae2ad34a4340231f2a030d49 | |
parent | 4812f6478771a6d261eb4a5c3aa4b450333fbf00 (diff) | |
download | gitlab-shell-181-authorized-principals-check-go.tar.gz |
Implement AuthorizedPrincipals command181-authorized-principals-check-go
Build this command when `Executable` name is
`gitlab-shell-authorized-principals-check`. Feature flag is the
same name.
-rw-r--r-- | go/internal/command/authorizedprincipals/authorized_principals.go | 47 | ||||
-rw-r--r-- | go/internal/command/authorizedprincipals/authorized_principals_test.go | 47 | ||||
-rw-r--r-- | go/internal/command/command.go | 11 | ||||
-rw-r--r-- | go/internal/command/command_test.go | 11 | ||||
-rw-r--r-- | go/internal/command/commandargs/authorized_principals.go | 50 | ||||
-rw-r--r-- | go/internal/command/commandargs/command_args.go | 2 | ||||
-rw-r--r-- | go/internal/command/commandargs/command_args_test.go | 23 | ||||
-rw-r--r-- | go/internal/keyline/key_line.go | 17 | ||||
-rw-r--r-- | go/internal/keyline/key_line_test.go | 31 | ||||
-rw-r--r-- | spec/gitlab_shell_authorized_principals_check_spec.rb | 2 |
10 files changed, 236 insertions, 5 deletions
diff --git a/go/internal/command/authorizedprincipals/authorized_principals.go b/go/internal/command/authorizedprincipals/authorized_principals.go new file mode 100644 index 0000000..b04e5a4 --- /dev/null +++ b/go/internal/command/authorizedprincipals/authorized_principals.go @@ -0,0 +1,47 @@ +package authorizedprincipals + +import ( + "fmt" + + "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/commandargs" + "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/keyline" +) + +type Command struct { + Config *config.Config + Args *commandargs.AuthorizedPrincipals + ReadWriter *readwriter.ReadWriter +} + +func (c *Command) Execute() error { + if err := c.printPrincipalLines(); err != nil { + return err + } + + return nil +} + +func (c *Command) printPrincipalLines() error { + principals := c.Args.Principals + + for _, principal := range principals { + if err := c.printPrincipalLine(principal); err != nil { + return err + } + } + + return nil +} + +func (c *Command) printPrincipalLine(principal string) error { + principalKeyLine, err := keyline.NewPrincipalKeyLine(c.Args.KeyId, principal, c.Config.RootDir) + if err != nil { + return err + } + + fmt.Fprintln(c.ReadWriter.Out, principalKeyLine.ToString()) + + return nil +} diff --git a/go/internal/command/authorizedprincipals/authorized_principals_test.go b/go/internal/command/authorizedprincipals/authorized_principals_test.go new file mode 100644 index 0000000..2db0d41 --- /dev/null +++ b/go/internal/command/authorizedprincipals/authorized_principals_test.go @@ -0,0 +1,47 @@ +package authorizedprincipals + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/require" + + "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/commandargs" + "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/readwriter" + "gitlab.com/gitlab-org/gitlab-shell/go/internal/config" +) + +func TestExecute(t *testing.T) { + testCases := []struct { + desc string + arguments *commandargs.AuthorizedPrincipals + expectedOutput string + }{ + { + desc: "With single principal", + arguments: &commandargs.AuthorizedPrincipals{KeyId: "key", Principals: []string{"principal"}}, + expectedOutput: "command=\"/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", + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + buffer := &bytes.Buffer{} + cmd := &Command{ + Config: &config.Config{RootDir: "/tmp"}, + Args: tc.arguments, + ReadWriter: &readwriter.ReadWriter{Out: buffer}, + } + + err := cmd.Execute() + + require.NoError(t, err) + require.Equal(t, tc.expectedOutput, buffer.String()) + }) + } +} diff --git a/go/internal/command/command.go b/go/internal/command/command.go index d6b96f1..071cd1d 100644 --- a/go/internal/command/command.go +++ b/go/internal/command/command.go @@ -2,6 +2,7 @@ package command import ( "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/authorizedkeys" + "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/authorizedprincipals" "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/commandargs" "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/discover" "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/fallback" @@ -38,6 +39,8 @@ func buildCommand(e *executable.Executable, args commandargs.CommandArgs, config return buildShellCommand(args.(*commandargs.Shell), config, readWriter) case executable.AuthorizedKeysCheck: return buildAuthorizedKeysCommand(args.(*commandargs.AuthorizedKeys), config, readWriter) + case executable.AuthorizedPrincipalsCheck: + return buildAuthorizedPrincipalsCommand(args.(*commandargs.AuthorizedPrincipals), config, readWriter) } return nil @@ -73,3 +76,11 @@ func buildAuthorizedKeysCommand(args *commandargs.AuthorizedKeys, config *config return &authorizedkeys.Command{Config: config, Args: args, ReadWriter: readWriter} } + +func buildAuthorizedPrincipalsCommand(args *commandargs.AuthorizedPrincipals, config *config.Config, readWriter *readwriter.ReadWriter) Command { + if !config.FeatureEnabled(executable.AuthorizedPrincipalsCheck) { + return nil + } + + return &authorizedprincipals.Command{Config: config, Args: args, ReadWriter: readWriter} +} diff --git a/go/internal/command/command_test.go b/go/internal/command/command_test.go index b651c78..e7e37d8 100644 --- a/go/internal/command/command_test.go +++ b/go/internal/command/command_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/authorizedkeys" + "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/authorizedprincipals" "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/discover" "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/fallback" "gitlab.com/gitlab-org/gitlab-shell/go/internal/command/lfsauthenticate" @@ -136,6 +137,16 @@ func TestNew(t *testing.T) { expectedType: &authorizedkeys.Command{}, }, { + desc: "it returns a AuthorizedPrincipals command if the feature is enabled", + executable: &executable.Executable{Name: executable.AuthorizedPrincipalsCheck}, + config: &config.Config{ + Migration: config.MigrationConfig{Enabled: true, Features: []string{"gitlab-shell-authorized-principals-check"}}, + }, + environment: map[string]string{}, + arguments: []string{"key", "principal"}, + expectedType: &authorizedprincipals.Command{}, + }, + { desc: "it returns a Fallback command if the feature is unimplemented", executable: &executable.Executable{Name: executable.GitlabShell}, config: &config.Config{ diff --git a/go/internal/command/commandargs/authorized_principals.go b/go/internal/command/commandargs/authorized_principals.go new file mode 100644 index 0000000..746ae3f --- /dev/null +++ b/go/internal/command/commandargs/authorized_principals.go @@ -0,0 +1,50 @@ +package commandargs + +import ( + "errors" + "fmt" +) + +type AuthorizedPrincipals struct { + Arguments []string + KeyId string + Principals []string +} + +func (ap *AuthorizedPrincipals) Parse() error { + if err := ap.validate(); err != nil { + return err + } + + ap.KeyId = ap.Arguments[0] + ap.Principals = ap.Arguments[1:] + + return nil +} + +func (ap *AuthorizedPrincipals) GetArguments() []string { + return ap.Arguments +} + +func (ap *AuthorizedPrincipals) validate() error { + argsSize := len(ap.Arguments) + + if argsSize < 2 { + return errors.New(fmt.Sprintf("# Insufficient arguments. %d. Usage\n#\tgitlab-shell-authorized-principals-check <key-id> <principal1> [<principal2>...]", argsSize)) + } + + keyId := ap.Arguments[0] + principals := ap.Arguments[1:] + + if keyId == "" { + return errors.New("# No key_id provided") + } + + for _, principal := range principals { + if principal == "" { + return errors.New("# An invalid principal was provided") + } + } + + return nil +} diff --git a/go/internal/command/commandargs/command_args.go b/go/internal/command/commandargs/command_args.go index dfdf8e5..4831134 100644 --- a/go/internal/command/commandargs/command_args.go +++ b/go/internal/command/commandargs/command_args.go @@ -19,6 +19,8 @@ func Parse(e *executable.Executable, arguments []string) (CommandArgs, error) { args = &Shell{Arguments: arguments} case executable.AuthorizedKeysCheck: args = &AuthorizedKeys{Arguments: arguments} + case executable.AuthorizedPrincipalsCheck: + args = &AuthorizedPrincipals{Arguments: arguments} } if err := args.Parse(); err != nil { diff --git a/go/internal/command/commandargs/command_args_test.go b/go/internal/command/commandargs/command_args_test.go index e981c22..9f1575d 100644 --- a/go/internal/command/commandargs/command_args_test.go +++ b/go/internal/command/commandargs/command_args_test.go @@ -125,6 +125,11 @@ func TestParseSuccess(t *testing.T) { arguments: []string{"git", "git", "key"}, expectedArgs: &AuthorizedKeys{Arguments: []string{"git", "git", "key"}, ExpectedUser: "git", ActualUser: "git", Key: "key"}, }, { + desc: "It parses authorized-principals command", + executable: &executable.Executable{Name: executable.AuthorizedPrincipalsCheck}, + arguments: []string{"key", "principal-1", "principal-2"}, + expectedArgs: &AuthorizedPrincipals{Arguments: []string{"key", "principal-1", "principal-2"}, KeyId: "key", Principals: []string{"principal-1", "principal-2"}}, + }, { desc: "Unknown executable", executable: &executable.Executable{Name: "unknown"}, arguments: []string{}, @@ -193,6 +198,24 @@ func TestParseFailure(t *testing.T) { arguments: []string{"user", "user", ""}, expectedError: "# No key provided", }, + { + desc: "With not enough arguments for the AuthorizedPrincipalsCheck", + executable: &executable.Executable{Name: executable.AuthorizedPrincipalsCheck}, + arguments: []string{"key"}, + expectedError: "# Insufficient arguments. 1. Usage\n#\tgitlab-shell-authorized-principals-check <key-id> <principal1> [<principal2>...]", + }, + { + desc: "With missing key_id for the AuthorizedPrincipalsCheck", + executable: &executable.Executable{Name: executable.AuthorizedPrincipalsCheck}, + arguments: []string{"", "principal"}, + expectedError: "# No key_id provided", + }, + { + desc: "With blank principal for the AuthorizedPrincipalsCheck", + executable: &executable.Executable{Name: executable.AuthorizedPrincipalsCheck}, + arguments: []string{"key", "principal", ""}, + expectedError: "# An invalid principal was provided", + }, } for _, tc := range testCases { diff --git a/go/internal/keyline/key_line.go b/go/internal/keyline/key_line.go index 7b19c87..f92f50b 100644 --- a/go/internal/keyline/key_line.go +++ b/go/internal/keyline/key_line.go @@ -16,6 +16,7 @@ var ( const ( PublicKeyPrefix = "key" + PrincipalPrefix = "username" SshOptions = "no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty" ) @@ -27,11 +28,11 @@ type KeyLine struct { } func NewPublicKeyLine(id string, publicKey string, rootDir string) (*KeyLine, error) { - if err := validate(id, publicKey); err != nil { - return nil, err - } + return newKeyLine(id, publicKey, PublicKeyPrefix, rootDir) +} - return &KeyLine{Id: id, Value: publicKey, Prefix: PublicKeyPrefix, RootDir: rootDir}, nil +func NewPrincipalKeyLine(keyId string, principal string, rootDir string) (*KeyLine, error) { + return newKeyLine(keyId, principal, PrincipalPrefix, rootDir) } func (k *KeyLine) ToString() string { @@ -40,6 +41,14 @@ func (k *KeyLine) ToString() string { return fmt.Sprintf(`command="%s",%s %s`, command, SshOptions, k.Value) } +func newKeyLine(id string, value string, prefix string, rootDir string) (*KeyLine, error) { + if err := validate(id, value); err != nil { + return nil, err + } + + return &KeyLine{Id: id, Value: value, Prefix: prefix, RootDir: rootDir}, nil +} + func validate(id string, value string) error { if !keyRegex.MatchString(id) { return errors.New(fmt.Sprintf("Invalid key_id: %s", id)) diff --git a/go/internal/keyline/key_line_test.go b/go/internal/keyline/key_line_test.go index fc2bdf3..c6883c0 100644 --- a/go/internal/keyline/key_line_test.go +++ b/go/internal/keyline/key_line_test.go @@ -37,6 +37,37 @@ func TestFailingNewPublicKeyLine(t *testing.T) { } } +func TestFailingNewPrincipalKeyLine(t *testing.T) { + testCases := []struct { + desc string + keyId string + principal string + expectedError string + }{ + { + desc: "When username has non-alphanumeric and non-dash characters in it", + keyId: "username\n1", + principal: "principal", + expectedError: "Invalid key_id: username\n1", + }, + { + desc: "When principal has newline in it", + keyId: "username", + principal: "principal\n1", + expectedError: "Invalid value: principal\n1", + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + result, err := NewPrincipalKeyLine(tc.keyId, tc.principal, "root-dir") + + require.Empty(t, result) + require.EqualError(t, err, tc.expectedError) + }) + } +} + func TestToString(t *testing.T) { keyLine := &KeyLine{ Id: "1", diff --git a/spec/gitlab_shell_authorized_principals_check_spec.rb b/spec/gitlab_shell_authorized_principals_check_spec.rb index 8b946bc..c2a3306 100644 --- a/spec/gitlab_shell_authorized_principals_check_spec.rb +++ b/spec/gitlab_shell_authorized_principals_check_spec.rb @@ -55,7 +55,7 @@ describe 'bin/gitlab-shell-authorized-principals-check' do it_behaves_like 'authorized principals check' end - pending 'with the go authorized-principals-check feature', :go do + describe 'with the go authorized-principals-check feature', :go do before(:all) do write_config( 'migration' => { |