From 2cdc1a973f5589357cfe0b366fc15f38447181ab Mon Sep 17 00:00:00 2001 From: Jeremy Wharton Date: Sun, 20 Aug 2023 23:25:40 -0500 Subject: [PATCH] satellite/console: fix project creation race condition This change fixes an issue where the project limit could be exceeded if multiple project creation requests were sent sufficiently close to one another. This could also be used to bypass project name duplication checking. Change-Id: I61cde7abaf25dedc5601c6870275de9938d7b949 --- satellite/console/service.go | 90 +++++++++++++----------------------- 1 file changed, 33 insertions(+), 57 deletions(-) diff --git a/satellite/console/service.go b/satellite/console/service.go index c9d7c5211..cd04e617a 100644 --- a/satellite/console/service.go +++ b/satellite/console/service.go @@ -1659,6 +1659,33 @@ func (s *Service) CreateProject(ctx context.Context, projectInfo UpsertProjectIn return Error.Wrap(err) } + limit, err := tx.Users().GetProjectLimit(ctx, user.ID) + if err != nil { + return err + } + + projects, err := tx.Projects().GetOwn(ctx, user.ID) + if err != nil { + return err + } + + // We check again for project name duplication and whether the project limit + // has been exceeded in case a parallel project creation transaction created + // a project at the same time as this one. + var numBefore int + for _, other := range projects { + if other.CreatedAt.Before(p.CreatedAt) || (other.CreatedAt.Equal(p.CreatedAt) && other.ID.Less(p.ID)) { + if other.Name == p.Name { + return errs.Combine(ErrProjName.New(projNameErrMsg), tx.Projects().Delete(ctx, p.ID)) + } + numBefore++ + } + } + if numBefore >= limit { + s.analytics.TrackProjectLimitError(user.ID, user.Email) + return errs.Combine(ErrProjLimit.New(projLimitErrMsg), tx.Projects().Delete(ctx, p.ID)) + } + _, err = tx.ProjectMembers().Insert(ctx, user.ID, p.ID) if err != nil { return Error.Wrap(err) @@ -1683,69 +1710,18 @@ func (s *Service) GenCreateProject(ctx context.Context, projectInfo UpsertProjec var err error defer mon.Task()(&ctx)(&err) - user, err := s.getUserAndAuditLog(ctx, "create project") + p, err = s.CreateProject(ctx, projectInfo) if err != nil { + status := http.StatusInternalServerError + if _, ctxErr := GetUser(ctx); ctxErr != nil { + status = http.StatusUnauthorized + } return nil, api.HTTPError{ - Status: http.StatusUnauthorized, - Err: Error.Wrap(err), - } - } - - currentProjectCount, err := s.checkProjectLimit(ctx, user.ID) - if err != nil { - return nil, api.HTTPError{ - Status: http.StatusInternalServerError, - Err: ErrProjLimit.Wrap(err), - } - } - - newProjectLimits, err := s.getUserProjectLimits(ctx, user.ID) - if err != nil { - return nil, api.HTTPError{ - Status: http.StatusInternalServerError, - Err: ErrProjLimit.Wrap(err), - } - } - - var projectID uuid.UUID - err = s.store.WithTx(ctx, func(ctx context.Context, tx DBTx) error { - storageLimit := memory.Size(newProjectLimits.Storage) - bandwidthLimit := memory.Size(newProjectLimits.Bandwidth) - p, err = tx.Projects().Insert(ctx, - &Project{ - Description: projectInfo.Description, - Name: projectInfo.Name, - OwnerID: user.ID, - UserAgent: user.UserAgent, - StorageLimit: &storageLimit, - BandwidthLimit: &bandwidthLimit, - SegmentLimit: &newProjectLimits.Segment, - DefaultPlacement: user.DefaultPlacement, - }, - ) - if err != nil { - return Error.Wrap(err) - } - - _, err = tx.ProjectMembers().Insert(ctx, user.ID, p.ID) - if err != nil { - return Error.Wrap(err) - } - - projectID = p.ID - - return nil - }) - - if err != nil { - return nil, api.HTTPError{ - Status: http.StatusInternalServerError, + Status: status, Err: err, } } - s.analytics.TrackProjectCreated(user.ID, user.Email, projectID, currentProjectCount+1) - return p, httpError }