From 87b883b388bffbf82c4e5728b5624540bcf5a137 Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Wed, 8 May 2019 10:00:29 +0200 Subject: [PATCH] Extend repo error, Redo error checking in auth flow --- system/cli/roles.go | 2 +- system/cli/users.go | 2 +- system/internal/repository/error.go | 4 + system/internal/service/auth.go | 141 +++++++++++++--------------- system/rest/auth_internal.go | 3 +- 5 files changed, 73 insertions(+), 79 deletions(-) diff --git a/system/cli/roles.go b/system/cli/roles.go index a1e550525..6737a6578 100644 --- a/system/cli/roles.go +++ b/system/cli/roles.go @@ -195,7 +195,7 @@ func rolesUserAddCmd(ctx context.Context, db *factory.DB) func(cmd *cobra.Comman } } - if user, err = userRepo.FindByEmail(userStr); err != nil && err != repository.ErrUserNotFound { + if user, err = userRepo.FindByEmail(userStr); repository.ErrUserNotFound.Eq(err) { exit(cmd, err) } else if user == nil || user.ID == 0 { if ID, err = strconv.ParseUint(userStr, 10, 64); err != nil { diff --git a/system/cli/users.go b/system/cli/users.go index cc8fe90bc..3d18f464d 100644 --- a/system/cli/users.go +++ b/system/cli/users.go @@ -145,7 +145,7 @@ func usersCmd(ctx context.Context, db *factory.DB) *cobra.Command { userStr = args[0] ) - if user, err = userRepo.FindByEmail(userStr); err != nil && err != repository.ErrUserNotFound { + if user, err = userRepo.FindByEmail(userStr); !repository.ErrUserNotFound.Eq(err) { exit(cmd, err) } else if user == nil || user.ID == 0 { if ID, err = strconv.ParseUint(userStr, 10, 64); err != nil { diff --git a/system/internal/repository/error.go b/system/internal/repository/error.go index 2cc825428..3de0dcb6b 100644 --- a/system/internal/repository/error.go +++ b/system/internal/repository/error.go @@ -16,3 +16,7 @@ func (e repositoryError) Error() string { func (e repositoryError) String() string { return "crust.system.repository." + string(e) } + +func (e repositoryError) Eq(err error) bool { + return err != nil && e.Error() == err.Error() +} diff --git a/system/internal/service/auth.go b/system/internal/service/auth.go index da296b39b..57b83bcc1 100644 --- a/system/internal/service/auth.go +++ b/system/internal/service/auth.go @@ -146,7 +146,7 @@ func (svc auth) External(profile goth.User) (u *types.User, err error) { } if u, err = svc.users.FindByID(c.OwnerID); err != nil { - if err == repository.ErrUserNotFound { + if repository.ErrUserNotFound.Eq(err) { // Orphaned credentials (no owner) // try to auto-fix this by removing credentials and recreating user if err := svc.credentials.DeleteByID(c.ID); err != nil { @@ -186,7 +186,7 @@ func (svc auth) External(profile goth.User) (u *types.User, err error) { findByEmail: // Find user via his email - if u, err = svc.users.FindByEmail(profile.Email); err == repository.ErrUserNotFound { + if u, err = svc.users.FindByEmail(profile.Email); repository.ErrUserNotFound.Eq(err) { // @todo check if it is ok to auto-create a user here // In case we do not have this email, create a new user @@ -266,95 +266,84 @@ func (svc auth) InternalSignUp(input *types.User, password string) (u *types.Use return } - err = svc.db.Transaction(func() error { - existing, err := svc.users.FindByEmail(input.Email) - - if err == nil && existing.Valid() { - if len(password) > 0 { - cc, err := svc.credentials.FindByKind(existing.ID, credentialsTypePassword) - if err != nil { - return errors.Wrap(err, "could not find credentials") - } - - err = svc.checkPassword(password, cc) - if err != nil { - return errors.Wrap(err, "user with this email already exists") - } - - if !existing.EmailConfirmed { - err = svc.sendEmailAddressConfirmationToken(existing) - if err != nil { - return err - } - } - - u = existing - return nil - } - - return errors.Wrap(err, "user with this email already exists") - - // if !svc.settings.internalSignUpSendEmailOnExisting { - // return 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 errors.New("could not validate the user") - // } - // - // return nil - } - - if err != repository.ErrUserNotFound { - return errors.New("could not check existing emails") - } - - // Whitelisted user data to copy - u, err = svc.users.Create(&types.User{ - Email: input.Email, - Name: input.Name, - Username: input.Username, - Handle: input.Handle, - - // Do we need confirmed email? - EmailConfirmed: !svc.settings.internalSignUpEmailConfirmationRequired, - }) - - if err != nil { - return errors.Wrap(err, "could not create user") - } + existing, err := svc.users.FindByEmail(input.Email) + if err == nil && existing.Valid() { if len(password) > 0 { - var hash []byte - hash, err = svc.hashPassword(password) + cc, err := svc.credentials.FindByKind(existing.ID, credentialsTypePassword) if err != nil { - return err + return nil, errors.Wrap(err, "could not find credentials") } - err = svc.changePassword(u.ID, hash) + err = svc.checkPassword(password, cc) if err != nil { - return err + return nil, errors.Wrap(err, "user with this email already exists") } + + if !existing.EmailConfirmed { + err = svc.sendEmailAddressConfirmationToken(existing) + if err != nil { + return nil, err + } + } + + return existing, nil } - if !u.EmailConfirmed { - err = svc.sendEmailAddressConfirmationToken(u) - if err != nil { - return err - } - } + return nil, errors.Wrap(err, "user with this email already exists") - 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 + } else if !repository.ErrUserNotFound.Eq(err) { + return nil, errors.Wrap(err, "could not check existing emails") + } + + // Whitelisted user data to copy + u, err = svc.users.Create(&types.User{ + Email: input.Email, + Name: input.Name, + Username: input.Username, + Handle: input.Handle, + + // Do we need confirmed email? + EmailConfirmed: !svc.settings.internalSignUpEmailConfirmationRequired, }) if err != nil { - return nil, err + return nil, errors.Wrap(err, "could not create user") + } + + if len(password) > 0 { + var hash []byte + hash, err = svc.hashPassword(password) + if err != nil { + return nil, err + } + + err = svc.changePassword(u.ID, hash) + if err != nil { + return nil, err + } } if !u.EmailConfirmed { - return nil, errors.New("user email pending confirmation") + err = svc.sendEmailAddressConfirmationToken(u) + if err != nil { + return nil, err + } + } + + if err != nil { + return nil, err } return u, nil @@ -387,7 +376,7 @@ func (svc auth) InternalLogin(email string, password string) (u *types.User, err ) u, err = svc.users.FindByEmail(email) - if err == repository.ErrUserNotFound { + if repository.ErrUserNotFound.Eq(err) { return errors.New("invalid username/password combination") } diff --git a/system/rest/auth_internal.go b/system/rest/auth_internal.go index 4324b01d9..87ced9f5d 100644 --- a/system/rest/auth_internal.go +++ b/system/rest/auth_internal.go @@ -63,7 +63,8 @@ func (ctrl *AuthInternal) Signup(ctx context.Context, r *request.AuthInternalSig } if !u.EmailConfirmed { - return nil, errors.New("user email pending confirmation") + // When email is not confirmed, do not send back JWT + return authInternalValidUserResponse{User: u}, nil } return authInternalValidUserResponse{