JakeHillion/storj
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

satellite/console: fix account creation race condition

Browse Source 
This change fixes an issue where multiple unverified users with the
same email address could be created if registration requests were
sent sufficiently close to one another.

Resolves #6156

Change-Id: If8b1a145bcab842ace718119183de59947430463
This commit is contained in:
Jeremy Wharton 2023-08-17 22:07:10 -05:00 committed by Storj Robot
parent b2780b028d
commit 7f9317aa48
2 changed files with 33 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
satellite/console/consoleweb/consoleapi/auth.go
View File

@ -313,7 +313,9 @@ func (a *Auth) Register(w http.ResponseWriter, r *http.Request) {
secret,
)
if err != nil {
a.serveJSONError(ctx, w, err)
if !console.ErrEmailUsed.Has(err) {
a.serveJSONError(ctx, w, err)
}
return
}

45
satellite/console/service.go
View File

@ -766,18 +766,6 @@ func (s *Service) CreateUser(ctx context.Context, user CreateUser, tokenSecret R
return nil, ErrRegToken.Wrap(err)
}
verified, unverified, err := s.store.Users().GetByEmailWithUnverified(ctx, user.Email)
if err != nil {
return nil, Error.Wrap(err)
}
if verified != nil {
mon.Counter("create_user_duplicate_verified").Inc(1) //mon:locked
return nil, ErrEmailUsed.New(emailUsedErrMsg)
} else if len(unverified) != 0 {
mon.Counter("create_user_duplicate_unverified").Inc(1) //mon:locked
return nil, ErrEmailUsed.New(emailUsedErrMsg)
}
hash, err := bcrypt.GenerateFromPassword([]byte(user.Password), s.config.PasswordCost)
if err != nil {
return nil, Error.Wrap(err)
@ -787,7 +775,7 @@ func (s *Service) CreateUser(ctx context.Context, user CreateUser, tokenSecret R
err = s.store.WithTx(ctx, func(ctx context.Context, tx DBTx) error {
userID, err := uuid.New()
if err != nil {
return Error.Wrap(err)
return err
}
newUser := &User{
@ -825,13 +813,40 @@ func (s *Service) CreateUser(ctx context.Context, user CreateUser, tokenSecret R
newUser,
)
if err != nil {
return Error.Wrap(err)
return err
}
verified, unverified, err := tx.Users().GetByEmailWithUnverified(ctx, user.Email)
if err != nil {
return err
}
if verified != nil {
err = tx.Users().Delete(ctx, u.ID)
if err != nil {
return err
}
mon.Counter("create_user_duplicate_verified").Inc(1) //mon:locked
return ErrEmailUsed.New(emailUsedErrMsg)
}
for _, other := range unverified {
// We compare IDs because a parallel user creation transaction for the same
// email could have created a record at the same time as ours.
if other.CreatedAt.Before(u.CreatedAt) || other.ID.Less(u.ID) {
err = tx.Users().Delete(ctx, u.ID)
if err != nil {
return err
}
mon.Counter("create_user_duplicate_unverified").Inc(1) //mon:locked
return ErrEmailUsed.New(emailUsedErrMsg)
}
}
if registrationToken != nil {
err = tx.RegistrationTokens().UpdateOwner(ctx, registrationToken.Secret, u.ID)
if err != nil {
return Error.Wrap(err)
return err
}
}