From 710c304d9461aa5cceda62f5e0db5fad0a26a4ab Mon Sep 17 00:00:00 2001 From: Zach Wasserman Date: Tue, 19 Jul 2022 09:47:25 -0700 Subject: [PATCH] Apply password requirements to admin-created users (#6667) This was requested by a customer. --- changes/admin-user | 1 + server/fleet/users.go | 48 ++++++---------- server/fleet/users_test.go | 110 +++++++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 32 deletions(-) create mode 100644 changes/admin-user diff --git a/changes/admin-user b/changes/admin-user new file mode 100644 index 0000000000..2f3e523118 --- /dev/null +++ b/changes/admin-user @@ -0,0 +1 @@ +* Enforce the same password requirements when admins create users directly as are already applied for users created through invites. diff --git a/server/fleet/users.go b/server/fleet/users.go index ad39cfec26..53cdd5e51b 100644 --- a/server/fleet/users.go +++ b/server/fleet/users.go @@ -144,28 +144,7 @@ type UserPayload struct { func (p *UserPayload) VerifyInviteCreate() error { invalid := &InvalidArgumentError{} - if p.Name == nil { - invalid.Append("name", "Full name missing required argument") - } else if *p.Name == "" { - invalid.Append("name", "Full name cannot be empty") - } - - // we don't need a password for single sign on - if p.SSOInvite == nil || !*p.SSOInvite { - if p.Password == nil { - invalid.Append("password", "Password missing required argument") - } else if *p.Password == "" { - invalid.Append("password", "Password cannot be empty") - } else if err := ValidatePasswordRequirements(*p.Password); err != nil { - invalid.Append("password", err.Error()) - } - } - - if p.Email == nil { - invalid.Append("email", "Email missing required argument") - } else if *p.Email == "" { - invalid.Append("email", "Email cannot be empty") - } + p.verifyCreateShared(invalid) if p.InviteToken == nil { invalid.Append("invite_token", "Invite token missing required argument") @@ -181,6 +160,19 @@ func (p *UserPayload) VerifyInviteCreate() error { func (p *UserPayload) VerifyAdminCreate() error { invalid := &InvalidArgumentError{} + p.verifyCreateShared(invalid) + + if p.InviteToken != nil { + invalid.Append("invite_token", "Invite token should not be specified with admin user creation") + } + + if invalid.HasErrors() { + return invalid + } + return nil +} + +func (p *UserPayload) verifyCreateShared(invalid *InvalidArgumentError) { if p.Name == nil { invalid.Append("name", "Full name missing required argument") } else if *p.Name == "" { @@ -193,8 +185,9 @@ func (p *UserPayload) VerifyAdminCreate() error { invalid.Append("password", "Password missing required argument") } else if *p.Password == "" { invalid.Append("password", "Password cannot be empty") + } else if err := ValidatePasswordRequirements(*p.Password); err != nil { + invalid.Append("password", err.Error()) } - // Skip password validation in the case of admin created users } if p.SSOEnabled != nil && *p.SSOEnabled && p.Password != nil && len(*p.Password) > 0 { @@ -206,15 +199,6 @@ func (p *UserPayload) VerifyAdminCreate() error { } else if *p.Email == "" { invalid.Append("email", "Email cannot be empty") } - - if p.InviteToken != nil { - invalid.Append("invite_token", "Invite token should not be specified with admin user creation") - } - - if invalid.HasErrors() { - return invalid - } - return nil } func (p *UserPayload) VerifyModify(ownUser bool) error { diff --git a/server/fleet/users_test.go b/server/fleet/users_test.go index 69e855bb51..f1be636ad2 100644 --- a/server/fleet/users_test.go +++ b/server/fleet/users_test.go @@ -4,6 +4,7 @@ import ( "fmt" "testing" + "github.com/fleetdm/fleet/v4/server/ptr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/crypto/bcrypt" @@ -98,3 +99,112 @@ func TestSaltAndHashPassword(t *testing.T) { require.Error(t, err) } } + +func TestAdminCreateValidate(t *testing.T) { + testCases := []struct { + payload UserPayload + // if errContains is empty then no error is expected + errContains []string + }{ + { + payload: UserPayload{}, + errContains: []string{"name", "email", "password"}, + }, + { + payload: UserPayload{Name: ptr.String(""), Email: ptr.String(""), Password: ptr.String("")}, + errContains: []string{"name", "email", "password"}, + }, + { + payload: UserPayload{Name: ptr.String("Foo"), Email: ptr.String(""), Password: ptr.String("")}, + errContains: []string{"email", "password"}, + }, + { + payload: UserPayload{Name: ptr.String("Foo"), Email: ptr.String("foo@example.com"), Password: ptr.String("")}, + errContains: []string{"password"}, + }, + { + payload: UserPayload{Name: ptr.String("Foo"), Email: ptr.String("foo@example.com"), Password: ptr.String("foo")}, + errContains: []string{"password"}, + }, + { + payload: UserPayload{Name: ptr.String("Foo"), Email: ptr.String("foo@example.com"), Password: ptr.String("Foofoofoo1337#"), InviteToken: ptr.String("foo")}, + errContains: []string{"invite_token"}, + }, + { + payload: UserPayload{Name: ptr.String("Foo"), Email: ptr.String("foo@example.com"), Password: ptr.String("Foofoofoo1337#")}, + errContains: nil, + }, + } + + for _, tc := range testCases { + t.Run("", func(t *testing.T) { + err := tc.payload.VerifyAdminCreate() + if len(tc.errContains) == 0 { + require.NoError(t, err) + } else { + ierr := err.(*InvalidArgumentError) + require.Equal(t, len(tc.errContains), len(*ierr)) + for _, expected := range tc.errContains { + assertContainsErrorName(t, *ierr, expected) + } + } + }) + } +} + +func TestInviteCreateValidate(t *testing.T) { + testCases := []struct { + payload UserPayload + // if errContains is empty then no error is expected + errContains []string + }{ + { + payload: UserPayload{}, + errContains: []string{"name", "email", "password", "invite_token"}, + }, + { + payload: UserPayload{Name: ptr.String(""), Email: ptr.String(""), Password: ptr.String("")}, + errContains: []string{"name", "email", "password", "invite_token"}, + }, + { + payload: UserPayload{Name: ptr.String("Foo"), Email: ptr.String(""), Password: ptr.String("")}, + errContains: []string{"email", "password", "invite_token"}, + }, + { + payload: UserPayload{Name: ptr.String("Foo"), Email: ptr.String("foo@example.com"), Password: ptr.String("")}, + errContains: []string{"password", "invite_token"}, + }, + { + payload: UserPayload{Name: ptr.String("Foo"), Email: ptr.String("foo@example.com"), Password: ptr.String("foo")}, + errContains: []string{"password", "invite_token"}, + }, + { + payload: UserPayload{Name: ptr.String("Foo"), Email: ptr.String("foo@example.com"), Password: ptr.String("Foofoofoo1337#"), InviteToken: ptr.String("foo")}, + errContains: nil, + }, + } + + for _, tc := range testCases { + t.Run("", func(t *testing.T) { + err := tc.payload.VerifyInviteCreate() + if len(tc.errContains) == 0 { + require.NoError(t, err) + } else { + ierr := err.(*InvalidArgumentError) + for _, expected := range tc.errContains { + require.Equal(t, len(tc.errContains), len(*ierr)) + assertContainsErrorName(t, *ierr, expected) + } + } + }) + } +} + +func assertContainsErrorName(t *testing.T, invalid InvalidArgumentError, name string) { + for _, argErr := range invalid { + if argErr.name == name { + return + } + } + t.Errorf("%v does not contain error %s", invalid, name) +}