diff --git a/store/rdbms/labels.gen.go b/store/rdbms/labels.gen.go index 87dd33173..4774b0630 100644 --- a/store/rdbms/labels.gen.go +++ b/store/rdbms/labels.gen.go @@ -11,9 +11,8 @@ package rdbms import ( "context" "database/sql" - "errors" - "fmt" "github.com/Masterminds/squirrel" + "github.com/cortezaproject/corteza-server/pkg/errors" "github.com/cortezaproject/corteza-server/pkg/label/types" "github.com/cortezaproject/corteza-server/store" ) @@ -35,11 +34,11 @@ func (s Store) SearchLabels(ctx context.Context, f types.LabelFilter) (types.Lab return nil, f, err } - return set, f, s.config.ErrorHandler(func() error { + return set, f, func() error { set, _, _, err = s.QueryLabels(ctx, q, nil) return err - }()) + }() } // QueryLabels queries the database, converts and checks each row and @@ -111,7 +110,7 @@ func (s Store) CreateLabel(ctx context.Context, rr ...*types.Label) (err error) // UpdateLabel updates one or more existing rows in labels func (s Store) UpdateLabel(ctx context.Context, rr ...*types.Label) error { - return s.config.ErrorHandler(s.partialLabelUpdate(ctx, nil, rr...)) + return s.partialLabelUpdate(ctx, nil, rr...) } // partialLabelUpdate updates one or more existing rows in labels @@ -129,7 +128,7 @@ func (s Store) partialLabelUpdate(ctx context.Context, onlyColumns []string, rr }, s.internalLabelEncoder(res).Skip("kind", "rel_resource", "name").Only(onlyColumns...)) if err != nil { - return s.config.ErrorHandler(err) + return err } } @@ -144,7 +143,7 @@ func (s Store) UpsertLabel(ctx context.Context, rr ...*types.Label) (err error) return err } - err = s.config.ErrorHandler(s.execUpsertLabels(ctx, s.internalLabelEncoder(res))) + err = s.execUpsertLabels(ctx, s.internalLabelEncoder(res)) if err != nil { return err } @@ -161,7 +160,7 @@ func (s Store) DeleteLabel(ctx context.Context, rr ...*types.Label) (err error) s.preprocessColumn("lbl.kind", ""): store.PreprocessValue(res.Kind, ""), s.preprocessColumn("lbl.rel_resource", ""): store.PreprocessValue(res.ResourceID, ""), s.preprocessColumn("lbl.name", "lower"): store.PreprocessValue(res.Name, "lower"), }) if err != nil { - return s.config.ErrorHandler(err) + return err } } @@ -179,7 +178,7 @@ func (s Store) DeleteLabelByKindResourceIDName(ctx context.Context, kind string, // TruncateLabels Deletes all rows from the labels table func (s Store) TruncateLabels(ctx context.Context) error { - return s.config.ErrorHandler(s.Truncate(ctx, s.labelTable())) + return s.Truncate(ctx, s.labelTable()) } // execLookupLabel prepares Label query and executes it, @@ -204,12 +203,12 @@ func (s Store) execLookupLabel(ctx context.Context, cnd squirrel.Sqlizer) (res * // execCreateLabels updates all matched (by cnd) rows in labels with given data func (s Store) execCreateLabels(ctx context.Context, payload store.Payload) error { - return s.config.ErrorHandler(s.Exec(ctx, s.InsertBuilder(s.labelTable()).SetMap(payload))) + return s.Exec(ctx, s.InsertBuilder(s.labelTable()).SetMap(payload)) } // execUpdateLabels updates all matched (by cnd) rows in labels with given data func (s Store) execUpdateLabels(ctx context.Context, cnd squirrel.Sqlizer, set store.Payload) error { - return s.config.ErrorHandler(s.Exec(ctx, s.UpdateBuilder(s.labelTable("lbl")).Where(cnd).SetMap(set))) + return s.Exec(ctx, s.UpdateBuilder(s.labelTable("lbl")).Where(cnd).SetMap(set)) } // execUpsertLabels inserts new or updates matching (by-primary-key) rows in labels with given data @@ -227,12 +226,12 @@ func (s Store) execUpsertLabels(ctx context.Context, set store.Payload) error { return err } - return s.config.ErrorHandler(s.Exec(ctx, upsert)) + return s.Exec(ctx, upsert) } // execDeleteLabels Deletes all matched (by cnd) rows in labels with given data func (s Store) execDeleteLabels(ctx context.Context, cnd squirrel.Sqlizer) error { - return s.config.ErrorHandler(s.Exec(ctx, s.DeleteBuilder(s.labelTable("lbl")).Where(cnd))) + return s.Exec(ctx, s.DeleteBuilder(s.labelTable("lbl")).Where(cnd)) } func (s Store) internalLabelRowScanner(row rowScanner) (res *types.Label, err error) { @@ -251,11 +250,11 @@ func (s Store) internalLabelRowScanner(row rowScanner) (res *types.Label, err er } if err == sql.ErrNoRows { - return nil, store.ErrNotFound + return nil, store.ErrNotFound.Stack(1) } if err != nil { - return nil, fmt.Errorf("could not scan db row for Label: %w", err) + return nil, errors.Store("could not scan label db row").Wrap(err) } else { return res, nil } @@ -334,8 +333,8 @@ func (s *Store) checkLabelConstraints(ctx context.Context, res *types.Label) err { ex, err := s.LookupLabelByKindResourceIDName(ctx, res.Kind, res.ResourceID, res.Name) if err == nil && ex != nil && ex.Kind != res.Kind && ex.ResourceID != res.ResourceID && ex.Name != res.Name { - return store.ErrNotUnique - } else if !errors.Is(err, store.ErrNotFound) { + return store.ErrNotUnique.Stack(1) + } else if !errors.IsNotFound(err) { return err } } diff --git a/store/rdbms/users.gen.go b/store/rdbms/users.gen.go index 33bd0d6d1..1d8a2654c 100644 --- a/store/rdbms/users.gen.go +++ b/store/rdbms/users.gen.go @@ -236,8 +236,7 @@ func (s Store) LookupUserByEmail(ctx context.Context, email string) (*types.User return s.execLookupUser(ctx, squirrel.Eq{ s.preprocessColumn("usr.email", "lower"): store.PreprocessValue(email, "lower"), - "usr.deleted_at": nil, - "usr.suspended_at": nil, + "usr.deleted_at": nil, }) } @@ -248,8 +247,7 @@ func (s Store) LookupUserByHandle(ctx context.Context, handle string) (*types.Us return s.execLookupUser(ctx, squirrel.Eq{ s.preprocessColumn("usr.handle", "lower"): store.PreprocessValue(handle, "lower"), - "usr.deleted_at": nil, - "usr.suspended_at": nil, + "usr.deleted_at": nil, }) } @@ -260,8 +258,7 @@ func (s Store) LookupUserByUsername(ctx context.Context, username string) (*type return s.execLookupUser(ctx, squirrel.Eq{ s.preprocessColumn("usr.username", "lower"): store.PreprocessValue(username, "lower"), - "usr.deleted_at": nil, - "usr.suspended_at": nil, + "usr.deleted_at": nil, }) } diff --git a/store/users.yaml b/store/users.yaml index b5a7831fb..5bbe98a4e 100644 --- a/store/users.yaml +++ b/store/users.yaml @@ -22,21 +22,21 @@ lookups: It returns user even if deleted or suspended - fields: [ Email ] - filter: { DeletedAt: nil, SuspendedAt: nil } + filter: { DeletedAt: nil } uniqueConstraintCheck: true description: |- searches for user by their email It returns only valid users (not deleted, not suspended) - fields: [ Handle ] - filter: { DeletedAt: nil, SuspendedAt: nil } + filter: { DeletedAt: nil } uniqueConstraintCheck: true description: |- searches for user by their email It returns only valid users (not deleted, not suspended) - fields: [ Username ] - filter: { DeletedAt: nil, SuspendedAt: nil } + filter: { DeletedAt: nil } uniqueConstraintCheck: true description: |- searches for user by their username diff --git a/system/service/auth.go b/system/service/auth.go index ebf45ebf4..784b5273a 100644 --- a/system/service/auth.go +++ b/system/service/auth.go @@ -143,24 +143,9 @@ func (svc auth) External(ctx context.Context, profile goth.User) (u *types.User, return err } - // Add user ID for audit log aam.setUser(u) - ctx = internalAuth.SetIdentityToContext(ctx, u) - if err = svc.eventbus.WaitFor(ctx, event.AuthBeforeLogin(u, authProvider)); err != nil { - return err - } - - if u.Valid() { - // Valid user, Bingo! - c.LastUsedAt = now() - if err = store.UpdateCredentials(ctx, svc.store, c); err != nil { - return err - } - - _ = svc.eventbus.WaitFor(ctx, event.AuthAfterLogin(u, authProvider)) - return svc.recordAction(ctx, aam, AuthActionUpdateCredentials, nil) - } else { + if err = svc.procLogin(ctx, svc.store, u, c, authProvider); err != nil { // Scenario: linked to an invalid user if len(cc) > 1 { // try with next credentials @@ -170,6 +155,8 @@ func (svc auth) External(ctx context.Context, profile goth.User) (u *types.User, return AuthErrCredentialsLinkedToInvalidUser(aam) } + + return svc.recordAction(ctx, aam, AuthActionUpdateCredentials, nil) } // If we could not find anything useful, @@ -210,7 +197,11 @@ func (svc auth) External(ctx context.Context, profile goth.User) (u *types.User, } if u.Handle == "" { - createHandle(ctx, svc.store, u) + createUserHandle(ctx, svc.store, u) + } + + if err = uniqueUserCheck(ctx, svc.store, u); err != nil { + return err } u.ID = nextID() @@ -237,18 +228,10 @@ func (svc auth) External(ctx context.Context, profile goth.User) (u *types.User, } else { // User found aam.setUser(u) - ctx = internalAuth.SetIdentityToContext(ctx, u) - if err = svc.eventbus.WaitFor(ctx, event.AuthBeforeLogin(u, authProvider)); err != nil { + if err = svc.procLogin(ctx, svc.store, u, nil, authProvider); err != nil { return err } - - _ = svc.eventbus.WaitFor(ctx, event.AuthAfterLogin(u, authProvider)) - - // If user - if !u.Valid() { - return AuthErrFailedForDisabledUser(aam).Wrap(err) - } } // If we got to this point, assume that user is authenticated @@ -293,7 +276,6 @@ func (svc auth) InternalSignUp(ctx context.Context, input *types.User, password aam = &authActionProps{ email: input.Email, credentials: &types.Credentials{Kind: credentialsTypePassword}, - user: u, } ) @@ -306,71 +288,50 @@ func (svc auth) InternalSignUp(ctx context.Context, input *types.User, password return AuthErrInvalidEmailFormat(aam) } - if len(password) == 0 { - return AuthErrPasswordNotSecure(aam) - } - if !handle.IsValid(input.Handle) { return AuthErrInvalidHandle(aam) } + if len(password) == 0 { + return AuthErrPasswordNotSecure(aam) + } + var eUser *types.User eUser, err = store.LookupUserByEmail(ctx, svc.store, input.Email) - if err == nil && eUser.Valid() { + + if err == nil && eUser != nil { var ( + c *types.Credentials cc types.CredentialsSet f = types.CredentialsFilter{OwnerID: eUser.ID, Kind: credentialsTypePassword} ) - cc, _, err = store.SearchCredentials(ctx, svc.store, f) - if err != nil { + if cc, _, err = store.SearchCredentials(ctx, svc.store, f); err != nil { return err } - if c := cc.CompareHashAndPassword(password); c == nil { + if c = cc.CompareHashAndPassword(password); c == nil { return AuthErrInvalidCredentials(aam) - } else { - // Update last-used-by timestamp on matching credentials - c.LastUsedAt = now() - c.UpdatedAt = now() - aam.setCredentials(c) - - if err = store.UpdateCredentials(ctx, svc.store, c); err != nil { - return err - } } - // We're not actually doing sign-up here - user exists, - // password is a match, so lets trigger before/after user login events - if err = svc.eventbus.WaitFor(ctx, event.AuthBeforeLogin(eUser, authProvider)); err != nil { - return err - } - - if !eUser.EmailConfirmed { - err = svc.sendEmailAddressConfirmationToken(ctx, eUser) - if err != nil { - return err - } - } - - _ = svc.eventbus.WaitFor(ctx, event.AuthAfterLogin(eUser, authProvider)) + aam.setCredentials(c) u = eUser - return nil - - // if !svc.settings.internalSignUpSendEmailOnExisting { - // return nil,errors.Wrap(err, "user with this email already exists") - // } - - // User already exists, but we're nice and we'll send this user an - // email that will help him to login - // if !u.Valid() { - // return nil,errors.New("could not validate the user") - // } - // - // return nil,nil + return svc.procLogin(ctx, svc.store, eUser, c, authProvider) } else if !errors.IsNotFound(err) { return err } + // if !svc.settings.internalSignUpSendEmailOnExisting { + // return nil,errors.Wrap(err, "user with this email already exists") + // } + + // User already exists, but we're nice and we'll send this user an + // email that will help him to login + // if !u.Valid() { + // return nil,errors.New("could not validate the user") + // } + // + // return nil,nil + if err = svc.CanRegister(ctx); err != nil { return err } @@ -390,8 +351,12 @@ func (svc auth) InternalSignUp(ctx context.Context, input *types.User, password return err } - if input.Handle == "" { - createHandle(ctx, svc.store, input) + if nUser.Handle == "" { + createUserHandle(ctx, svc.store, nUser) + } + + if err = uniqueUserCheck(ctx, svc.store, nUser); err != nil { + return err } // Whitelisted user data to copy @@ -403,12 +368,12 @@ func (svc auth) InternalSignUp(ctx context.Context, input *types.User, password aam.setUser(nUser) _ = svc.eventbus.WaitFor(ctx, event.AuthAfterSignup(nUser, authProvider)) - if err = svc.autoPromote(ctx, u); err != nil { + if err = svc.autoPromote(ctx, nUser); err != nil { return err } if len(password) > 0 { - err = svc.SetPassword(ctx, u.ID, password) + err = svc.SetPassword(ctx, nUser.ID, password) if err != nil { return err } @@ -423,6 +388,7 @@ func (svc auth) InternalSignUp(ctx context.Context, input *types.User, password return svc.recordAction(ctx, aam, AuthActionSendEmailConfirmationToken, nil) } + u = nUser return nil }() @@ -461,13 +427,6 @@ func (svc auth) InternalLogin(ctx context.Context, email string, password string ) u, err = store.LookupUserByEmail(ctx, svc.store, email) - if errors.IsNotFound(err) { - return AuthErrFailedForUnknownUser() - } - - if err != nil { - return err - } // Update audit meta with found user ctx = internalAuth.SetIdentityToContext(ctx, u) @@ -477,37 +436,13 @@ func (svc auth) InternalLogin(ctx context.Context, email string, password string return err } - if c := cc.CompareHashAndPassword(password); c == nil { + c := cc.CompareHashAndPassword(password) + if c == nil { return AuthErrInvalidCredentials(aam) - } else { - // Update last-used-by timestamp on matching credentials - c.UpdatedAt = now() - c.LastUsedAt = now() - aam.setCredentials(c) - - if err = store.UpdateCredentials(ctx, svc.store, c); err != nil { - return err - } } - if err = svc.eventbus.WaitFor(ctx, event.AuthBeforeLogin(u, authProvider)); err != nil { - return err - } - - if !u.Valid() { - return AuthErrFailedForDisabledUser() - } - - if !u.EmailConfirmed { - if err = svc.sendEmailAddressConfirmationToken(ctx, u); err != nil { - return err - } - - return AuthErrFailedUnconfirmedEmail() - } - - _ = svc.eventbus.WaitFor(ctx, event.AuthAfterLogin(u, authProvider)) - return nil + aam.setCredentials(c) + return svc.procLogin(ctx, svc.store, u, c, authProvider) }() return u, svc.recordAction(ctx, aam, AuthActionAuthenticate, err) @@ -791,30 +726,6 @@ func (svc auth) ExchangePasswordResetToken(ctx context.Context, token string) (u return u, t, svc.recordAction(ctx, aam, AuthActionExchangePasswordResetToken, err) } -// SendEmailAddressConfirmationToken sends email with email address confirmation token -func (svc auth) SendEmailAddressConfirmationToken(ctx context.Context, email string) (err error) { - var ( - aam = &authActionProps{ - email: email, - } - ) - - err = func() error { - if !svc.settings.Auth.Internal.Enabled || !svc.settings.Auth.Internal.PasswordReset.Enabled { - return AuthErrPasswordResetDisabledByConfig(aam) - } - - u, err := store.LookupUserByEmail(ctx, svc.store, email) - if err != nil { - return AuthErrInvalidToken(aam) - } - - return svc.sendEmailAddressConfirmationToken(ctx, u) - }() - - return svc.recordAction(ctx, aam, AuthActionSendEmailConfirmationToken, err) -} - func (svc auth) sendEmailAddressConfirmationToken(ctx context.Context, u *types.User) (err error) { var ( notificationLang = "en" @@ -899,6 +810,41 @@ func (svc auth) sendPasswordResetToken(ctx context.Context, u *types.User) (err return svc.notifications.PasswordReset(ctx, notificationLang, u.Email, token) } +// procLogin fn performs standard validation, credentials-update tasks and triggers events +func (svc auth) procLogin(ctx context.Context, s store.Storer, u *types.User, c *types.Credentials, p *types.AuthProvider) (err error) { + ctx = internalAuth.SetIdentityToContext(ctx, u) + if err = svc.eventbus.WaitFor(ctx, event.AuthBeforeLogin(u, p)); err != nil { + return err + } + + // all checks (suspension, deleted, confirmed email) are checked AFTER + // before-login event to enable before-login hooks to alter user and make her + // valid for login + switch true { + case u.SuspendedAt != nil: + return AuthErrFailedForSuspendedUser() + case u.DeletedAt != nil: + return AuthErrFailedForDeletedUser() + case !u.EmailConfirmed && svc.settings.Auth.Internal.Signup.EmailConfirmationRequired: + // Re-send email-confirmation when not confirmed and signup email confirmation required + if err = svc.sendEmailAddressConfirmationToken(ctx, u); err != nil { + return err + } + + return AuthErrFailedUnconfirmedEmail() + } + + if c != nil { + c.LastUsedAt = now() + if err = store.UpdateCredentials(ctx, s, c); err != nil { + return err + } + } + + _ = svc.eventbus.WaitFor(ctx, event.AuthAfterLogin(u, p)) + return nil +} + func (svc auth) loadUserFromToken(ctx context.Context, token, kind string) (u *types.User, err error) { var ( aam = &authActionProps{ diff --git a/system/service/auth_actions.gen.go b/system/service/auth_actions.gen.go index 762970dfa..ff07aa481 100644 --- a/system/service/auth_actions.gen.go +++ b/system/service/auth_actions.gen.go @@ -729,13 +729,13 @@ func AuthErrFailedForUnknownUser(mm ...*authActionProps) *errors.Error { return e } -// AuthErrFailedForDisabledUser returns "system:auth.failedForDisabledUser" as *errors.Error +// AuthErrFailedForDeletedUser returns "system:auth.failedForDeletedUser" as *errors.Error // // Note: This error will be wrapped with safe (system:auth.invalidCredentials) error! // // This function is auto-generated. // -func AuthErrFailedForDisabledUser(mm ...*authActionProps) *errors.Error { +func AuthErrFailedForDeletedUser(mm ...*authActionProps) *errors.Error { var p = &authActionProps{} if len(mm) > 0 { p = mm[0] @@ -744,13 +744,49 @@ func AuthErrFailedForDisabledUser(mm ...*authActionProps) *errors.Error { var e = errors.New( errors.KindInternal, - "failedForDisabledUser", + "failedForDeletedUser", - errors.Meta("type", "failedForDisabledUser"), + errors.Meta("type", "failedForDeletedUser"), errors.Meta("resource", "system:auth"), // action log entry; no formatting, it will be applied inside recordAction fn. - errors.Meta(authLogMetaKey{}, "disabled user {user} tried to log-in with {credentials.kind}"), + errors.Meta(authLogMetaKey{}, "deleted user {user} tried to log-in with {credentials.kind}"), + errors.Meta(authPropsMetaKey{}, p), + + errors.StackSkip(1), + ) + + if len(mm) > 0 { + } + + // Wrap with safe error + e = AuthErrInvalidCredentials().Wrap(e) + + return e +} + +// AuthErrFailedForSuspendedUser returns "system:auth.failedForSuspendedUser" as *errors.Error +// +// Note: This error will be wrapped with safe (system:auth.invalidCredentials) error! +// +// This function is auto-generated. +// +func AuthErrFailedForSuspendedUser(mm ...*authActionProps) *errors.Error { + var p = &authActionProps{} + if len(mm) > 0 { + p = mm[0] + } + + var e = errors.New( + errors.KindInternal, + + "failedForSuspendedUser", + + errors.Meta("type", "failedForSuspendedUser"), + errors.Meta("resource", "system:auth"), + + // action log entry; no formatting, it will be applied inside recordAction fn. + errors.Meta(authLogMetaKey{}, "suspended user {user} tried to log-in with {credentials.kind}"), errors.Meta(authPropsMetaKey{}, p), errors.StackSkip(1), diff --git a/system/service/auth_actions.yaml b/system/service/auth_actions.yaml index 7645fa25c..e70717768 100644 --- a/system/service/auth_actions.yaml +++ b/system/service/auth_actions.yaml @@ -89,9 +89,14 @@ errors: log: "unknown user {email} tried to log-in with {credentials.kind}" severity: warning - - error: failedForDisabledUser + - error: failedForDeletedUser maskedWith: invalidCredentials - log: "disabled user {user} tried to log-in with {credentials.kind}" + log: "deleted user {user} tried to log-in with {credentials.kind}" + severity: warning + + - error: failedForSuspendedUser + maskedWith: invalidCredentials + log: "suspended user {user} tried to log-in with {credentials.kind}" severity: warning - error: failedUnconfirmedEmail diff --git a/system/service/auth_test.go b/system/service/auth_test.go index a43e949b8..b665c0893 100644 --- a/system/service/auth_test.go +++ b/system/service/auth_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "github.com/cortezaproject/corteza-server/pkg/eventbus" + "github.com/cortezaproject/corteza-server/pkg/id" "github.com/cortezaproject/corteza-server/store" "github.com/cortezaproject/corteza-server/store/sqlite3" "github.com/cortezaproject/corteza-server/system/types" @@ -133,8 +134,82 @@ func TestAuth_External(t *testing.T) { } } -func TestAuth_InternalSignU(t *testing.T) { - t.Skip("pending implementation") +func TestAuth_InternalSignUp(t *testing.T) { + var ( + req = require.New(t) + + ctx = context.Background() + svc = makeMockAuthService() + + existingUserID = id.Next() + ) + + svc.settings.Auth.Internal.Enabled = true + svc.settings.Auth.Internal.Signup.Enabled = true + + req.NoError(svc.store.CreateUser(ctx, &types.User{Email: "existing@internal-signup-test.tld", ID: existingUserID, CreatedAt: *now()})) + req.NoError(svc.SetPassword(ctx, existingUserID, "secure password")) + + t.Run("invalid email", func(t *testing.T) { + var ( + req = require.New(t) + ) + + u, err := svc.InternalSignUp(ctx, &types.User{}, "") + req.Nil(u) + req.EqualError(err, AuthErrInvalidEmailFormat().Error()) + }) + + t.Run("invalid handle", func(t *testing.T) { + var ( + req = require.New(t) + ) + + u, err := svc.InternalSignUp(ctx, &types.User{Email: "new@internal-signup-test.tld", Handle: "123"}, "") + req.Nil(u) + req.EqualError(err, AuthErrInvalidHandle().Error()) + }) + + t.Run("invalid password", func(t *testing.T) { + var ( + req = require.New(t) + ) + + u, err := svc.InternalSignUp(ctx, &types.User{Email: "new@internal-signup-test.tld"}, "") + req.Nil(u) + req.EqualError(err, AuthErrPasswordNotSecure().Error()) + }) + + t.Run("valid input", func(t *testing.T) { + var ( + req = require.New(t) + ) + + u, err := svc.InternalSignUp(ctx, &types.User{Email: "new@internal-signup-test.tld"}, "secure password") + req.NoError(err) + req.NotNil(u) + }) + + t.Run("existing user", func(t *testing.T) { + var ( + req = require.New(t) + ) + + u, err := svc.InternalSignUp(ctx, &types.User{Email: "existing@internal-signup-test.tld"}, "secure password") + req.NoError(err) + req.NotNil(u) + req.Equal(existingUserID, u.ID) + }) + + t.Run("invalid password for existing user", func(t *testing.T) { + var ( + req = require.New(t) + ) + + u, err := svc.InternalSignUp(ctx, &types.User{Email: "existing@internal-signup-test.tld"}, "invalid password") + req.EqualError(err, AuthErrInvalidCredentials().Error()) + req.Nil(u) + }) } func TestAuth_InternalLogin(t *testing.T) { diff --git a/system/service/user.go b/system/service/user.go index 7914be21c..b36ba9efd 100644 --- a/system/service/user.go +++ b/system/service/user.go @@ -384,10 +384,10 @@ func (svc user) Create(new *types.User) (u *types.User, err error) { } if new.Handle == "" { - createHandle(svc.ctx, DefaultStore, new) + createUserHandle(svc.ctx, DefaultStore, new) } - if err = svc.UniqueCheck(new); err != nil { + if err = uniqueUserCheck(svc.ctx, svc.store, new); err != nil { return } @@ -460,7 +460,7 @@ func (svc user) Update(upd *types.User) (u *types.User, err error) { return } - if err = svc.UniqueCheck(u); err != nil { + if err = uniqueUserCheck(svc.ctx, svc.store, u); err != nil { return } @@ -483,63 +483,6 @@ func (svc user) Update(upd *types.User) (u *types.User, err error) { return u, svc.recordAction(svc.ctx, uaProps, UserActionUpdate, err) } -// UniqueCheck verifies user's email, username and handle -func (svc user) UniqueCheck(u *types.User) (err error) { - isUnique := func(field string) bool { - f := types.UserFilter{ - // If user exists and is deleted -- not a dup - Deleted: filter.StateExcluded, - - // If user exists and is suspended -- duplicate - Suspended: filter.StateInclusive, - } - - switch field { - case "email": - if u.Email == "" { - return true - } - - f.Email = u.Email - - case "username": - if u.Username == "" { - return true - } - - f.Username = u.Username - case "handle": - if u.Handle == "" { - return true - } - - f.Handle = u.Handle - } - - set, _, err := store.SearchUsers(svc.ctx, svc.store, f) - if err != nil || len(set) > 1 { - // In case of error or multiple users returned - return false - } - - return len(set) == 0 || set[0].ID == u.ID - } - - if !isUnique("email") { - return UserErrEmailNotUnique() - } - - if !isUnique("username") { - return UserErrUsernameNotUnique() - } - - if !isUnique("handle") { - return UserErrHandleNotUnique() - } - - return nil -} - func (svc user) UpdateWithAvatar(mod *types.User, avatar io.Reader) (out *types.User, err error) { // @todo: avatar return svc.Create(mod) @@ -597,7 +540,7 @@ func (svc user) Undelete(userID uint64) (err error) { uaProps.setUser(u) - if err = svc.UniqueCheck(u); err != nil { + if err = uniqueUserCheck(svc.ctx, svc.store, u); err != nil { return err } @@ -778,7 +721,64 @@ rangeLoop: return uu.Walk(s) } -func createHandle(ctx context.Context, s store.Users, u *types.User) { +// UniqueCheck verifies user's email, username and handle +func uniqueUserCheck(ctx context.Context, s store.Storer, u *types.User) (err error) { + isUnique := func(field string) bool { + f := types.UserFilter{ + // If user exists and is deleted -- not a dup + Deleted: filter.StateExcluded, + + // If user exists and is suspended -- duplicate + Suspended: filter.StateInclusive, + } + + switch field { + case "email": + if u.Email == "" { + return true + } + + f.Email = u.Email + + case "username": + if u.Username == "" { + return true + } + + f.Username = u.Username + case "handle": + if u.Handle == "" { + return true + } + + f.Handle = u.Handle + } + + set, _, err := store.SearchUsers(ctx, s, f) + if err != nil || len(set) > 1 { + // In case of error or multiple users returned + return false + } + + return len(set) == 0 || set[0].ID == u.ID + } + + if !isUnique("email") { + return UserErrEmailNotUnique() + } + + if !isUnique("username") { + return UserErrUsernameNotUnique() + } + + if !isUnique("handle") { + return UserErrHandleNotUnique() + } + + return nil +} + +func createUserHandle(ctx context.Context, s store.Users, u *types.User) { if u.Handle == "" { u.Handle, _ = handle.Cast( // Must not exist before