From ba891538af9e960bbb8a440fe46b50a4c6f4be34 Mon Sep 17 00:00:00 2001 From: Yehor Butko Date: Wed, 10 Apr 2019 03:15:12 +0300 Subject: [PATCH] V3-1399 Error messages (#1715) * V3-1399 Error messages --- satellite/console/apikeys.go | 2 +- satellite/console/auth.go | 6 +- satellite/console/service.go | 185 +++++++++++++----- satellite/console/validation.go | 2 +- .../apiKeys/footerArea/DeleteApiKeysArea.vue | 4 +- .../components/project/DeleteProjectPopup.vue | 14 +- 6 files changed, 142 insertions(+), 71 deletions(-) diff --git a/satellite/console/apikeys.go b/satellite/console/apikeys.go index 45d045e52..1d09d3e53 100644 --- a/satellite/console/apikeys.go +++ b/satellite/console/apikeys.go @@ -74,7 +74,7 @@ func CreateAPIKey() (*APIKey, error) { n, err := io.ReadFull(rand.Reader, key[:]) if err != nil || n != 24 { - return nil, errs.New("error creating api key") + return nil, errs.New(internalErrMsg) } return key, nil diff --git a/satellite/console/auth.go b/satellite/console/auth.go index 7eeda131c..6daa4893b 100644 --- a/satellite/console/auth.go +++ b/satellite/console/auth.go @@ -65,9 +65,9 @@ func GetAuth(ctx context.Context) (Authorization, error) { return auth, nil } - if err, ok := value.(error); ok { - return Authorization{}, err + if _, ok := value.(error); ok { + return Authorization{}, errs.New(internalErrMsg) } - return Authorization{}, ErrUnauthorized.New("unauthorized") + return Authorization{}, errs.New(unauthorizedErrMsg) } diff --git a/satellite/console/service.go b/satellite/console/service.go index 7780067a4..6e4e011fd 100644 --- a/satellite/console/service.go +++ b/satellite/console/service.go @@ -6,7 +6,6 @@ package console import ( "context" "crypto/subtle" - "fmt" "time" "github.com/skyrings/skyring-common/tools/uuid" @@ -34,6 +33,24 @@ const ( TestPasswordCost = bcrypt.MinCost ) +// Error messages +const ( + internalErrMsg = "It looks like we had a problem on our end. Please try again" + unauthorizedErrMsg = "You are not authorized to perform this action" + vanguardRegTokenErrMsg = "We are unable to create your account. This is an invite-only alpha, please join our waitlist to receive an invitation" + emailUsedErrMsg = "This email is already in use, try another" + activationTokenIsExpiredErrMsg = "Your account activation link has expired, please sign up again" + credentialsErrMsg = "Your email or password was incorrect, please try again" + oldPassIncorrectErrMsg = "Old password is incorrect, please try again" + passwordIncorrectErrMsg = "Your password needs at least %d characters long" + teamMemberDoesNotExistErrMsg = `There is no account on this Satellite for the user(s) you have entered. + Please add team members with active accounts` + + // TODO: remove after vanguard release + usedRegTokenVanguardErrMsg = "This registration token has already been used" + projLimitVanguardErrMsg = "Sorry, during the Vanguard release you have a limited number of projects" +) + // Service is handling accounts related logic type Service struct { Signer @@ -80,10 +97,10 @@ func (s *Service) CreateUser(ctx context.Context, user CreateUser, tokenSecret R // TODO: remove after vanguard release registrationToken, err := s.store.RegistrationTokens().GetBySecret(ctx, tokenSecret) if err != nil { - return nil, err + return nil, errs.New(vanguardRegTokenErrMsg) } if registrationToken.OwnerID != nil { - return nil, errs.New("token is already used") + return nil, errs.New(usedRegTokenVanguardErrMsg) } // TODO: store original email input in the db, @@ -92,12 +109,12 @@ func (s *Service) CreateUser(ctx context.Context, user CreateUser, tokenSecret R u, err = s.store.Users().GetByEmail(ctx, email) if err == nil { - return nil, errs.New(fmt.Sprintf("%s is already in use", email)) + return nil, errs.New(emailUsedErrMsg) } hash, err := bcrypt.GenerateFromPassword([]byte(user.Password), s.passwordCost) if err != nil { - return nil, err + return nil, errs.New(internalErrMsg) } u, err = s.store.Users().Insert(ctx, &User{ @@ -106,13 +123,16 @@ func (s *Service) CreateUser(ctx context.Context, user CreateUser, tokenSecret R ShortName: user.ShortName, PasswordHash: hash, }) + if err != nil { + return nil, errs.New(internalErrMsg) + } err = s.store.RegistrationTokens().UpdateOwner(ctx, registrationToken.Secret, u.ID) if err != nil { - return nil, err + return nil, errs.New(internalErrMsg) } - return u, err + return u, nil } // GenerateActivationToken - is a method for generating activation token @@ -135,7 +155,7 @@ func (s *Service) ActivateAccount(ctx context.Context, activationToken string) ( token, err := consoleauth.FromBase64URLString(activationToken) if err != nil { - return + return errs.New(internalErrMsg) } claims, err := s.authenticate(token) @@ -145,12 +165,12 @@ func (s *Service) ActivateAccount(ctx context.Context, activationToken string) ( _, err = s.store.Users().GetByEmail(ctx, normalizeEmail(claims.Email)) if err == nil { - return errs.New(fmt.Sprintf("%s is already in use", claims.Email)) + return errs.New(emailUsedErrMsg) } user, err := s.store.Users().Get(ctx, claims.ID) if err != nil { - return + return errs.New(internalErrMsg) } now := time.Now() @@ -160,12 +180,17 @@ func (s *Service) ActivateAccount(ctx context.Context, activationToken string) ( } if now.After(user.CreatedAt.Add(tokenExpirationTime)) { - return errs.New("activation token has expired") + return errs.New(activationTokenIsExpiredErrMsg) } user.Status = Active - return s.store.Users().Update(ctx, user) + err = s.store.Users().Update(ctx, user) + if err != nil { + return errs.New(internalErrMsg) + } + + return nil } // Token authenticates User by credentials and returns auth token @@ -176,12 +201,12 @@ func (s *Service) Token(ctx context.Context, email, password string) (token stri user, err := s.store.Users().GetByEmail(ctx, email) if err != nil { - return "", err + return "", errs.New(credentialsErrMsg) } err = bcrypt.CompareHashAndPassword(user.PasswordHash, []byte(password)) if err != nil { - return "", ErrUnauthorized.New("password is incorrect: %s", err.Error()) + return "", ErrUnauthorized.New(credentialsErrMsg) } claims := consoleauth.Claims{ @@ -205,13 +230,17 @@ func (s *Service) GetUser(ctx context.Context, id uuid.UUID) (u *User, err error return nil, err } - return s.store.Users().Get(ctx, id) + user, err := s.store.Users().Get(ctx, id) + if err != nil { + return nil, errs.New(internalErrMsg) + } + + return user, nil } // UpdateAccount updates User func (s *Service) UpdateAccount(ctx context.Context, info UserInfo) (err error) { defer mon.Task()(&ctx)(&err) - _, err = GetAuth(ctx) auth, err := GetAuth(ctx) if err != nil { return err @@ -225,13 +254,18 @@ func (s *Service) UpdateAccount(ctx context.Context, info UserInfo) (err error) // add normalization email := normalizeEmail(info.Email) - return s.store.Users().Update(ctx, &User{ + err = s.store.Users().Update(ctx, &User{ ID: auth.User.ID, FullName: info.FullName, ShortName: info.ShortName, Email: email, PasswordHash: nil, }) + if err != nil { + return errs.New(internalErrMsg) + } + + return nil } // ChangePassword updates password for a given user @@ -244,7 +278,7 @@ func (s *Service) ChangePassword(ctx context.Context, pass, newPass string) (err err = bcrypt.CompareHashAndPassword(auth.User.PasswordHash, []byte(pass)) if err != nil { - return ErrUnauthorized.New("origin password is incorrect: %s", err.Error()) + return errs.New(oldPassIncorrectErrMsg) } if err := validatePassword(newPass); err != nil { @@ -253,11 +287,16 @@ func (s *Service) ChangePassword(ctx context.Context, pass, newPass string) (err hash, err := bcrypt.GenerateFromPassword([]byte(newPass), s.passwordCost) if err != nil { - return err + return errs.New(internalErrMsg) } auth.User.PasswordHash = hash - return s.store.Users().Update(ctx, &auth.User) + err = s.store.Users().Update(ctx, &auth.User) + if err != nil { + return errs.New(internalErrMsg) + } + + return nil } // DeleteAccount deletes User @@ -270,10 +309,15 @@ func (s *Service) DeleteAccount(ctx context.Context, password string) (err error err = bcrypt.CompareHashAndPassword(auth.User.PasswordHash, []byte(password)) if err != nil { - return ErrUnauthorized.New("origin password is incorrect") + return ErrUnauthorized.New(oldPassIncorrectErrMsg) } - return s.store.Users().Delete(ctx, auth.User.ID) + err = s.store.Users().Delete(ctx, auth.User.ID) + if err != nil { + return errs.New(internalErrMsg) + } + + return nil } // GetProject is a method for querying project by id @@ -284,7 +328,12 @@ func (s *Service) GetProject(ctx context.Context, projectID uuid.UUID) (p *Proje return nil, err } - return s.store.Projects().Get(ctx, projectID) + p, err = s.store.Projects().Get(ctx, projectID) + if err != nil { + return nil, errs.New(internalErrMsg) + } + + return } // GetUsersProjects is a method for querying all projects @@ -295,7 +344,12 @@ func (s *Service) GetUsersProjects(ctx context.Context) (ps []Project, err error return nil, err } - return s.store.Projects().GetByUserID(ctx, auth.User.ID) + ps, err = s.store.Projects().GetByUserID(ctx, auth.User.ID) + if err != nil { + return nil, errs.New(internalErrMsg) + } + + return } // CreateProject is a method for creating new project @@ -319,7 +373,7 @@ func (s *Service) CreateProject(ctx context.Context, projectInfo ProjectInfo) (p transaction, err := s.store.BeginTx(ctx) if err != nil { - return nil, err + return nil, errs.New(internalErrMsg) } defer func() { @@ -329,16 +383,19 @@ func (s *Service) CreateProject(ctx context.Context, projectInfo ProjectInfo) (p } err = transaction.Commit() + if err != nil { + err = errs.New(internalErrMsg) + } }() prj, err := transaction.Projects().Insert(ctx, project) if err != nil { - return nil, err + return nil, errs.New(internalErrMsg) } _, err = transaction.ProjectMembers().Insert(ctx, auth.User.ID, prj.ID) if err != nil { - return nil, err + return nil, errs.New(internalErrMsg) } return prj, nil @@ -356,7 +413,12 @@ func (s *Service) DeleteProject(ctx context.Context, projectID uuid.UUID) (err e return ErrUnauthorized.Wrap(err) } - return s.store.Projects().Delete(ctx, projectID) + err = s.store.Projects().Delete(ctx, projectID) + if err != nil { + return errs.New(internalErrMsg) + } + + return nil } // UpdateProject is a method for updating project description by id @@ -377,7 +439,7 @@ func (s *Service) UpdateProject(ctx context.Context, projectID uuid.UUID, descri err = s.store.Projects().Update(ctx, project) if err != nil { - return nil, err + return nil, errs.New(internalErrMsg) } return project, nil @@ -395,7 +457,6 @@ func (s *Service) AddProjectMembers(ctx context.Context, projectID uuid.UUID, em return nil, ErrUnauthorized.Wrap(err) } - //var userIDs []uuid.UUID var userErr errs.Group // collect user querying errors @@ -407,17 +468,16 @@ func (s *Service) AddProjectMembers(ctx context.Context, projectID uuid.UUID, em } users = append(users, user) - //userIDs = append(userIDs, user.ID) } if err = userErr.Err(); err != nil { - return nil, err + return nil, errs.New(teamMemberDoesNotExistErrMsg) } // add project members in transaction scope tx, err := s.store.BeginTx(ctx) if err != nil { - return nil, err + return nil, errs.New(internalErrMsg) } defer func() { @@ -433,7 +493,7 @@ func (s *Service) AddProjectMembers(ctx context.Context, projectID uuid.UUID, em _, err = tx.ProjectMembers().Insert(ctx, user.ID, projectID) if err != nil { - return nil, err + return nil, errs.New(internalErrMsg) } } @@ -468,7 +528,7 @@ func (s *Service) DeleteProjectMembers(ctx context.Context, projectID uuid.UUID, } if err = userErr.Err(); err != nil { - return err + return errs.New(teamMemberDoesNotExistErrMsg) } // delete project members in transaction scope @@ -490,7 +550,7 @@ func (s *Service) DeleteProjectMembers(ctx context.Context, projectID uuid.UUID, err = tx.ProjectMembers().Delete(ctx, uID, projectID) if err != nil { - return err + return errs.New(internalErrMsg) } } @@ -514,7 +574,12 @@ func (s *Service) GetProjectMembers(ctx context.Context, projectID uuid.UUID, pa pagination.Limit = maxLimit } - return s.store.ProjectMembers().GetByProjectID(ctx, projectID, pagination) + pm, err = s.store.ProjectMembers().GetByProjectID(ctx, projectID, pagination) + if err != nil { + return nil, errs.New(internalErrMsg) + } + + return } // CreateAPIKey creates new api key @@ -541,7 +606,11 @@ func (s *Service) CreateAPIKey(ctx context.Context, projectID uuid.UUID, name st Name: name, ProjectID: projectID, }) - return info, key, err + if err != nil { + return nil, nil, errs.New(internalErrMsg) + } + + return info, key, nil } // GetAPIKeyInfo retrieves api key by id @@ -556,7 +625,7 @@ func (s *Service) GetAPIKeyInfo(ctx context.Context, id uuid.UUID) (*APIKeyInfo, key, err := s.store.APIKeys().Get(ctx, id) if err != nil { - return nil, err + return nil, errs.New(internalErrMsg) } _, err = s.isProjectMember(ctx, auth.User.ID, key.ProjectID) @@ -592,12 +661,12 @@ func (s *Service) DeleteAPIKeys(ctx context.Context, ids []uuid.UUID) (err error } if err = keysErr.Err(); err != nil { - return err + return errs.New(internalErrMsg) } tx, err := s.store.BeginTx(ctx) if err != nil { - return err + return errs.New(internalErrMsg) } defer func() { @@ -612,7 +681,7 @@ func (s *Service) DeleteAPIKeys(ctx context.Context, ids []uuid.UUID) (err error for _, keyToDeleteID := range ids { err = tx.APIKeys().Delete(ctx, keyToDeleteID) if err != nil { - return err + return errs.New(internalErrMsg) } } @@ -632,7 +701,12 @@ func (s *Service) GetAPIKeysInfoByProjectID(ctx context.Context, projectID uuid. return nil, ErrUnauthorized.Wrap(err) } - return s.store.APIKeys().GetByProjectID(ctx, projectID) + info, err = s.store.APIKeys().GetByProjectID(ctx, projectID) + if err != nil { + return nil, errs.New(internalErrMsg) + } + + return info, nil } // GetProjectUsage retrieves project usage for a given period @@ -650,7 +724,12 @@ func (s *Service) GetProjectUsage(ctx context.Context, projectID uuid.UUID, sinc return nil, err } - return s.store.UsageRollups().GetProjectTotal(ctx, projectID, since, before) + projectUsage, err := s.store.UsageRollups().GetProjectTotal(ctx, projectID, since, before) + if err != nil { + return nil, errs.New(internalErrMsg) + } + + return projectUsage, nil } // GetBucketUsageRollups retrieves summed usage rollups for every bucket of particular project for a given period @@ -710,10 +789,10 @@ func (s *Service) checkProjectLimit(ctx context.Context, userID uuid.UUID) error projects, err := s.GetUsersProjects(ctx) if err != nil { - return err + return errs.New(internalErrMsg) } if len(projects) >= registrationToken.ProjectLimit { - return errs.New("max project count is %d", registrationToken.ProjectLimit) + return errs.New(projLimitVanguardErrMsg) } return nil @@ -729,13 +808,13 @@ func (s *Service) CreateRegToken(ctx context.Context, projLimit int) (*Registrat func (s *Service) createToken(claims *consoleauth.Claims) (string, error) { json, err := claims.JSON() if err != nil { - return "", err + return "", errs.New(internalErrMsg) } token := consoleauth.Token{Payload: json} err = signToken(&token, s.Signer) if err != nil { - return "", err + return "", errs.New(internalErrMsg) } return token.String(), nil @@ -747,7 +826,7 @@ func (s *Service) authenticate(token consoleauth.Token) (*consoleauth.Claims, er err := signToken(&token, s.Signer) if err != nil { - return nil, err + return nil, errs.New(internalErrMsg) } if subtle.ConstantTimeCompare(signature, token.Signature) != 1 { @@ -756,7 +835,7 @@ func (s *Service) authenticate(token consoleauth.Token) (*consoleauth.Claims, er claims, err := consoleauth.FromJSON(token.Payload) if err != nil { - return nil, err + return nil, errs.New(internalErrMsg) } return claims, nil @@ -789,12 +868,12 @@ var ErrNoMembership = errs.Class("no membership error") func (s *Service) isProjectMember(ctx context.Context, userID uuid.UUID, projectID uuid.UUID) (result isProjectMember, err error) { project, err := s.store.Projects().Get(ctx, projectID) if err != nil { - return + return result, errs.New(internalErrMsg) } memberships, err := s.store.ProjectMembers().GetByMemberID(ctx, userID) if err != nil { - return + return result, errs.New(internalErrMsg) } for _, membership := range memberships { @@ -805,5 +884,5 @@ func (s *Service) isProjectMember(ctx context.Context, userID uuid.UUID, project } } - return isProjectMember{}, ErrNoMembership.New("user %s is not a member of project %s", userID, project.ID) + return isProjectMember{}, ErrNoMembership.New(unauthorizedErrMsg) } diff --git a/satellite/console/validation.go b/satellite/console/validation.go index ae86ce589..a3ecb9298 100644 --- a/satellite/console/validation.go +++ b/satellite/console/validation.go @@ -39,7 +39,7 @@ func validatePassword(pass string) error { var errs validationErrors if len(pass) < passMinLength { - errs.Add("password can't be less than %d characters", passMinLength) + errs.Add(passwordIncorrectErrMsg, passMinLength) } return errs.Combine() diff --git a/web/satellite/src/components/apiKeys/footerArea/DeleteApiKeysArea.vue b/web/satellite/src/components/apiKeys/footerArea/DeleteApiKeysArea.vue index 840103351..5cdc5dd8c 100644 --- a/web/satellite/src/components/apiKeys/footerArea/DeleteApiKeysArea.vue +++ b/web/satellite/src/components/apiKeys/footerArea/DeleteApiKeysArea.vue @@ -37,14 +37,14 @@ import { API_KEYS_ACTIONS, NOTIFICATION_ACTIONS } from '@/utils/constants/action onDelete: async function () { let selectedKeys: any[] = this.$store.getters.selectedAPIKeys.map((key) => {return key.id; }); - const dispatchResult = await this.$store.dispatch(API_KEYS_ACTIONS.DELETE, selectedKeys); + const dispatchResult: RequestResponse = await this.$store.dispatch(API_KEYS_ACTIONS.DELETE, selectedKeys); let keySuffix = selectedKeys.length > 1 ? '\'s' : ''; if (dispatchResult.isSuccess) { this.$store.dispatch(NOTIFICATION_ACTIONS.SUCCESS, `API key${keySuffix} deleted successfully`); } else { - this.$store.dispatch(NOTIFICATION_ACTIONS.ERROR, `Error during deletion API key${keySuffix}`); + this.$store.dispatch(NOTIFICATION_ACTIONS.ERROR, dispatchResult.errorMessage); } }, onClearSelection: function (): void { diff --git a/web/satellite/src/components/project/DeleteProjectPopup.vue b/web/satellite/src/components/project/DeleteProjectPopup.vue index 159400bf5..ab16a30df 100644 --- a/web/satellite/src/components/project/DeleteProjectPopup.vue +++ b/web/satellite/src/components/project/DeleteProjectPopup.vue @@ -84,7 +84,7 @@ import { API_KEYS_ACTIONS } from '@/utils/constants/actionNames'; ); if (!response.isSuccess) { - this.$store.dispatch(NOTIFICATION_ACTIONS.ERROR, 'Error during project deletion'); + this.$store.dispatch(NOTIFICATION_ACTIONS.ERROR, response.errorMessage); this.$data.isLoading = false; return; @@ -102,16 +102,8 @@ import { API_KEYS_ACTIONS } from '@/utils/constants/actionNames'; this.$store.state.projectsModule.projects[0].id, ); - const pmResponse = await this.$store.dispatch(PM_ACTIONS.FETCH); - const keysResponse = await this.$store.dispatch(API_KEYS_ACTIONS.FETCH); - - if (!pmResponse.isSuccess) { - this.$store.dispatch(NOTIFICATION_ACTIONS.ERROR, 'Unable to fetch project members'); - } - - if (!keysResponse.isSuccess) { - this.$store.dispatch(NOTIFICATION_ACTIONS.ERROR, 'Unable to fetch api keys'); - } + this.$store.dispatch(PM_ACTIONS.FETCH); + this.$store.dispatch(API_KEYS_ACTIONS.FETCH); } this.$data.isLoading = false;