Commit f34e2cd5 authored by Ash McKenzie's avatar Ash McKenzie

Merge branch '181-authorized-principals-check-go' into 'master'

Implement AuthorizedPrincipals command

Closes #181

See merge request gitlab-org/gitlab-shell!322
parents 4812f647 1962b499
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
}
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())
})
}
}
......@@ -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}
}
......@@ -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"
......@@ -135,6 +136,16 @@ func TestNew(t *testing.T) {
arguments: []string{"git", "git", "key"},
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},
......
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
}
......@@ -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 {
......
......@@ -124,6 +124,11 @@ func TestParseSuccess(t *testing.T) {
executable: &executable.Executable{Name: executable.AuthorizedKeysCheck},
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"},
......@@ -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 {
......
......@@ -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))
......
......@@ -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",
......
......@@ -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' => {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment