From 6308da2cc04e7300e5fc0f2e0e0d1eca46953491 Mon Sep 17 00:00:00 2001 From: Wilfred Asomani Date: Thu, 28 Sep 2023 14:55:24 +0000 Subject: [PATCH] satellite/{payment,console,analytics} extend freeze functionality for violation freeze This change extends the account freeze functionality account for violation freezes as well. Also, debug level logs in the freeze chore have been changed to info. It adds an analytics event for when an invoice is found that belongs to a user frozen for violation. And finally adds whether a user is frozen for violation to the /account/freezestatus response. Issue: https://github.com/storj/storj-private/issues/386 Change-Id: Id8e40282dc8fd8f242da52791ab8ddbbef3da2bc --- satellite/admin/user_test.go | 10 +- satellite/analytics/service.go | 22 ++ satellite/console/accountfreezes.go | 249 +++++++++++++++--- satellite/console/accountfreezes_test.go | 173 ++++++++++-- .../console/consoleweb/consoleapi/auth.go | 12 +- .../console/consoleweb/consoleapi/payments.go | 10 +- .../console/observerpayinvoicewithtokens.go | 8 +- satellite/payments/accountfreeze/chore.go | 60 +++-- .../payments/accountfreeze/chore_test.go | 139 ++++++++-- satellite/satellitedb/accountfreezeevents.go | 26 +- 10 files changed, 585 insertions(+), 124 deletions(-) diff --git a/satellite/admin/user_test.go b/satellite/admin/user_test.go index 7824488ce..c33768f90 100644 --- a/satellite/admin/user_test.go +++ b/satellite/admin/user_test.go @@ -447,19 +447,17 @@ func TestWarnUnwarnUser(t *testing.T) { err = planet.Satellites[0].Admin.FreezeAccounts.Service.BillingWarnUser(ctx, user.ID) require.NoError(t, err) - freeze, warning, err := planet.Satellites[0].DB.Console().AccountFreezeEvents().GetAll(ctx, user.ID) + freezes, err := planet.Satellites[0].DB.Console().AccountFreezeEvents().GetAll(ctx, user.ID) require.NoError(t, err) - require.Nil(t, freeze) - require.NotNil(t, warning) + require.NotNil(t, freezes.BillingWarning) link := fmt.Sprintf("http://"+address.String()+"/api/users/%s/warning", user.Email) body := assertReq(ctx, t, link, http.MethodDelete, "", http.StatusOK, "", planet.Satellites[0].Config.Console.AuthToken) require.Len(t, body, 0) - freeze, warning, err = planet.Satellites[0].DB.Console().AccountFreezeEvents().GetAll(ctx, user.ID) + freezes, err = planet.Satellites[0].DB.Console().AccountFreezeEvents().GetAll(ctx, user.ID) require.NoError(t, err) - require.Nil(t, freeze) - require.Nil(t, warning) + require.Nil(t, freezes.BillingWarning) body = assertReq(ctx, t, link, http.MethodDelete, "", http.StatusInternalServerError, "", planet.Satellites[0].Config.Console.AuthToken) require.Contains(t, string(body), "user is not warned") diff --git a/satellite/analytics/service.go b/satellite/analytics/service.go index 767eb54e1..4ac0b2036 100644 --- a/satellite/analytics/service.go +++ b/satellite/analytics/service.go @@ -87,6 +87,7 @@ const ( eventAccountFreezeWarning = "Account Freeze Warning" eventUnpaidLargeInvoice = "Large Invoice Unpaid" eventUnpaidStorjscanInvoice = "Storjscan Invoice Unpaid" + eventPendingDeletionUnpaidInvoice = "Pending Deletion Invoice Open" eventExpiredCreditNeedsRemoval = "Expired Credit Needs Removal" eventExpiredCreditRemoved = "Expired Credit Removed" eventProjectInvitationAccepted = "Project Invitation Accepted" @@ -127,6 +128,10 @@ type FreezeTracker interface { // TrackLargeUnpaidInvoice sends an event to Segment indicating that a user has not paid a large invoice. TrackLargeUnpaidInvoice(invID string, userID uuid.UUID, email string) + // TrackViolationFrozenUnpaidInvoice sends an event to Segment indicating that a user has not paid an invoice + // and has been frozen due to violating ToS. + TrackViolationFrozenUnpaidInvoice(invID string, userID uuid.UUID, email string) + // TrackStorjscanUnpaidInvoice sends an event to Segment indicating that a user has not paid an invoice, but has storjscan transaction history. TrackStorjscanUnpaidInvoice(invID string, userID uuid.UUID, email string) } @@ -445,6 +450,23 @@ func (service *Service) TrackLargeUnpaidInvoice(invID string, userID uuid.UUID, }) } +// TrackViolationFrozenUnpaidInvoice sends an event to Segment indicating that a user has not paid a large invoice. +func (service *Service) TrackViolationFrozenUnpaidInvoice(invID string, userID uuid.UUID, email string) { + if !service.config.Enabled { + return + } + + props := segment.NewProperties() + props.Set("email", email) + props.Set("invoice", invID) + + service.enqueueMessage(segment.Track{ + UserId: userID.String(), + Event: service.satelliteName + " " + eventPendingDeletionUnpaidInvoice, + Properties: props, + }) +} + // TrackStorjscanUnpaidInvoice sends an event to Segment indicating that a user has not paid an invoice, but has storjscan transaction history. func (service *Service) TrackStorjscanUnpaidInvoice(invID string, userID uuid.UUID, email string) { if !service.config.Enabled { diff --git a/satellite/console/accountfreezes.go b/satellite/console/accountfreezes.go index df8763e20..5f71ec7f5 100644 --- a/satellite/console/accountfreezes.go +++ b/satellite/console/accountfreezes.go @@ -18,6 +18,10 @@ import ( // ErrAccountFreeze is the class for errors that occur during operation of the account freeze service. var ErrAccountFreeze = errs.Class("account freeze service") +// ErrFreezeUserStatusUpdate is error returned if updating the user status as part of violation (un)freeze +// fails. +var ErrFreezeUserStatusUpdate = errs.New("user status update failed") + // AccountFreezeEvents exposes methods to manage the account freeze events table in database. // // architecture: Database @@ -29,7 +33,7 @@ type AccountFreezeEvents interface { // GetAllEvents is a method for querying all account freeze events from the database. GetAllEvents(ctx context.Context, cursor FreezeEventsCursor) (events *FreezeEventsPage, err error) // GetAll is a method for querying all account freeze events from the database by user ID. - GetAll(ctx context.Context, userID uuid.UUID) (freeze *AccountFreezeEvent, warning *AccountFreezeEvent, err error) + GetAll(ctx context.Context, userID uuid.UUID) (freezes *UserFreezeEvents, err error) // DeleteAllByUserID is a method for deleting all account freeze events from the database by user ID. DeleteAllByUserID(ctx context.Context, userID uuid.UUID) error // DeleteByUserIDAndEvent is a method for deleting all account `eventType` events from the database by user ID. @@ -67,6 +71,11 @@ type FreezeEventsPage struct { Next bool } +// UserFreezeEvents holds the freeze events for a user. +type UserFreezeEvents struct { + BillingFreeze, BillingWarning, ViolationFreeze *AccountFreezeEvent +} + // AccountFreezeEventType is used to indicate the account freeze event's type. type AccountFreezeEventType int @@ -128,6 +137,21 @@ func (s *AccountFreezeService) IsUserBillingFrozen(ctx context.Context, userID u } } +// IsUserViolationFrozen returns whether the user specified by the given ID is frozen. +func (s *AccountFreezeService) IsUserViolationFrozen(ctx context.Context, userID uuid.UUID) (_ bool, err error) { + defer mon.Task()(&ctx)(&err) + + _, err = s.freezeEventsDB.Get(ctx, userID, ViolationFreeze) + switch { + case errors.Is(err, sql.ErrNoRows): + return false, nil + case err != nil: + return false, ErrAccountFreeze.Wrap(err) + default: + return true, nil + } +} + // BillingFreezeUser freezes the user specified by the given ID due to nonpayment of invoices. func (s *AccountFreezeService) BillingFreezeUser(ctx context.Context, userID uuid.UUID) (err error) { defer mon.Task()(&ctx)(&err) @@ -137,29 +161,12 @@ func (s *AccountFreezeService) BillingFreezeUser(ctx context.Context, userID uui return ErrAccountFreeze.Wrap(err) } - freeze, warning, err := s.freezeEventsDB.GetAll(ctx, userID) + freezes, err := s.freezeEventsDB.GetAll(ctx, userID) if err != nil { return ErrAccountFreeze.Wrap(err) } - if warning != nil { - err = s.freezeEventsDB.DeleteByUserIDAndEvent(ctx, userID, BillingWarning) - if err != nil { - return ErrAccountFreeze.Wrap(err) - } - } - if freeze == nil { - freeze = &AccountFreezeEvent{ - UserID: userID, - Type: BillingFreeze, - Limits: &AccountFreezeEventLimits{ - User: UsageLimits{ - Storage: user.ProjectStorageLimit, - Bandwidth: user.ProjectBandwidthLimit, - Segment: user.ProjectSegmentLimit, - }, - Projects: make(map[uuid.UUID]UsageLimits), - }, - } + if freezes.ViolationFreeze != nil { + return ErrAccountFreeze.New("User is already frozen due to ToS violation") } userLimits := UsageLimits{ @@ -167,9 +174,22 @@ func (s *AccountFreezeService) BillingFreezeUser(ctx context.Context, userID uui Bandwidth: user.ProjectBandwidthLimit, Segment: user.ProjectSegmentLimit, } + + billingFreeze := freezes.BillingFreeze + if billingFreeze == nil { + billingFreeze = &AccountFreezeEvent{ + UserID: userID, + Type: BillingFreeze, + Limits: &AccountFreezeEventLimits{ + User: userLimits, + Projects: make(map[uuid.UUID]UsageLimits), + }, + } + } + // If user limits have been zeroed already, we should not override what is in the freeze table. if userLimits != (UsageLimits{}) { - freeze.Limits.User = userLimits + billingFreeze.Limits.User = userLimits } projects, err := s.projectsDB.GetOwn(ctx, userID) @@ -189,11 +209,11 @@ func (s *AccountFreezeService) BillingFreezeUser(ctx context.Context, userID uui } // If project limits have been zeroed already, we should not override what is in the freeze table. if projLimits != (UsageLimits{}) { - freeze.Limits.Projects[p.ID] = projLimits + billingFreeze.Limits.Projects[p.ID] = projLimits } } - _, err = s.freezeEventsDB.Upsert(ctx, freeze) + _, err = s.freezeEventsDB.Upsert(ctx, billingFreeze) if err != nil { return ErrAccountFreeze.Wrap(err) } @@ -210,6 +230,13 @@ func (s *AccountFreezeService) BillingFreezeUser(ctx context.Context, userID uui } } + if freezes.BillingWarning != nil { + err = s.freezeEventsDB.DeleteByUserIDAndEvent(ctx, userID, BillingWarning) + if err != nil { + return ErrAccountFreeze.Wrap(err) + } + } + s.tracker.TrackAccountFrozen(userID, user.Email) return nil } @@ -244,7 +271,7 @@ func (s *AccountFreezeService) BillingUnfreezeUser(ctx context.Context, userID u return ErrAccountFreeze.Wrap(err) } - err = ErrAccountFreeze.Wrap(s.freezeEventsDB.DeleteAllByUserID(ctx, userID)) + err = ErrAccountFreeze.Wrap(s.freezeEventsDB.DeleteByUserIDAndEvent(ctx, userID, BillingFreeze)) if err != nil { return err } @@ -262,6 +289,19 @@ func (s *AccountFreezeService) BillingWarnUser(ctx context.Context, userID uuid. return ErrAccountFreeze.Wrap(err) } + freezes, err := s.freezeEventsDB.GetAll(ctx, userID) + if err != nil { + return ErrAccountFreeze.Wrap(err) + } + + if freezes.ViolationFreeze != nil || freezes.BillingFreeze != nil { + return ErrAccountFreeze.New("User is already frozen") + } + + if freezes.BillingWarning != nil { + return nil + } + _, err = s.freezeEventsDB.Upsert(ctx, &AccountFreezeEvent{ UserID: userID, Type: BillingWarning, @@ -297,16 +337,165 @@ func (s *AccountFreezeService) BillingUnWarnUser(ctx context.Context, userID uui return nil } -// GetAll returns all events for a user. -func (s *AccountFreezeService) GetAll(ctx context.Context, userID uuid.UUID) (freeze *AccountFreezeEvent, warning *AccountFreezeEvent, err error) { +// ViolationFreezeUser freezes the user specified by the given ID due to ToS violation. +func (s *AccountFreezeService) ViolationFreezeUser(ctx context.Context, userID uuid.UUID) (err error) { defer mon.Task()(&ctx)(&err) - freeze, warning, err = s.freezeEventsDB.GetAll(ctx, userID) + user, err := s.usersDB.Get(ctx, userID) if err != nil { - return nil, nil, ErrAccountFreeze.Wrap(err) + return ErrAccountFreeze.Wrap(err) } - return freeze, warning, nil + freezes, err := s.freezeEventsDB.GetAll(ctx, userID) + if err != nil { + return ErrAccountFreeze.Wrap(err) + } + + var limits *AccountFreezeEventLimits + if freezes.BillingFreeze != nil { + limits = freezes.BillingFreeze.Limits + } + + userLimits := UsageLimits{ + Storage: user.ProjectStorageLimit, + Bandwidth: user.ProjectBandwidthLimit, + Segment: user.ProjectSegmentLimit, + } + + violationFreeze := freezes.ViolationFreeze + if violationFreeze == nil { + if limits == nil { + limits = &AccountFreezeEventLimits{ + User: userLimits, + Projects: make(map[uuid.UUID]UsageLimits), + } + } + violationFreeze = &AccountFreezeEvent{ + UserID: userID, + Type: ViolationFreeze, + Limits: limits, + } + } + + // If user limits have been zeroed already, we should not override what is in the freeze table. + if userLimits != (UsageLimits{}) { + violationFreeze.Limits.User = userLimits + } + + projects, err := s.projectsDB.GetOwn(ctx, userID) + if err != nil { + return ErrAccountFreeze.Wrap(err) + } + for _, p := range projects { + projLimits := UsageLimits{} + if p.StorageLimit != nil { + projLimits.Storage = p.StorageLimit.Int64() + } + if p.BandwidthLimit != nil { + projLimits.Bandwidth = p.BandwidthLimit.Int64() + } + if p.SegmentLimit != nil { + projLimits.Segment = *p.SegmentLimit + } + // If project limits have been zeroed already, we should not override what is in the freeze table. + if projLimits != (UsageLimits{}) { + violationFreeze.Limits.Projects[p.ID] = projLimits + } + } + + _, err = s.freezeEventsDB.Upsert(ctx, violationFreeze) + if err != nil { + return ErrAccountFreeze.Wrap(err) + } + + err = s.usersDB.UpdateUserProjectLimits(ctx, userID, UsageLimits{}) + if err != nil { + return ErrAccountFreeze.Wrap(err) + } + + for _, proj := range projects { + err := s.projectsDB.UpdateUsageLimits(ctx, proj.ID, UsageLimits{}) + if err != nil { + return ErrAccountFreeze.Wrap(err) + } + } + + status := PendingDeletion + err = s.usersDB.Update(ctx, userID, UpdateUserRequest{ + Status: &status, + }) + if err != nil { + return ErrAccountFreeze.Wrap(errs.Combine(ErrFreezeUserStatusUpdate, err)) + } + + if freezes.BillingWarning != nil { + err = s.freezeEventsDB.DeleteByUserIDAndEvent(ctx, userID, BillingWarning) + if err != nil { + return ErrAccountFreeze.Wrap(err) + } + } + + if freezes.BillingFreeze != nil { + err = s.freezeEventsDB.DeleteByUserIDAndEvent(ctx, userID, BillingFreeze) + if err != nil { + return ErrAccountFreeze.Wrap(err) + } + } + + return nil +} + +// ViolationUnfreezeUser reverses the violation freeze placed on the user specified by the given ID. +func (s *AccountFreezeService) ViolationUnfreezeUser(ctx context.Context, userID uuid.UUID) (err error) { + defer mon.Task()(&ctx)(&err) + + event, err := s.freezeEventsDB.Get(ctx, userID, ViolationFreeze) + if errors.Is(err, sql.ErrNoRows) { + return ErrAccountFreeze.New("user is not violation frozen") + } + + if event.Limits == nil { + return ErrAccountFreeze.New("freeze event limits are nil") + } + + for id, limits := range event.Limits.Projects { + err := s.projectsDB.UpdateUsageLimits(ctx, id, limits) + if err != nil { + return ErrAccountFreeze.Wrap(err) + } + } + + err = s.usersDB.UpdateUserProjectLimits(ctx, userID, event.Limits.User) + if err != nil { + return ErrAccountFreeze.Wrap(err) + } + + err = s.freezeEventsDB.DeleteByUserIDAndEvent(ctx, userID, ViolationFreeze) + if err != nil { + return ErrAccountFreeze.Wrap(err) + } + + status := Active + err = s.usersDB.Update(ctx, userID, UpdateUserRequest{ + Status: &status, + }) + if err != nil { + return ErrAccountFreeze.Wrap(errs.Combine(ErrFreezeUserStatusUpdate, err)) + } + + return nil +} + +// GetAll returns all events for a user. +func (s *AccountFreezeService) GetAll(ctx context.Context, userID uuid.UUID) (freezes *UserFreezeEvents, err error) { + defer mon.Task()(&ctx)(&err) + + freezes, err = s.freezeEventsDB.GetAll(ctx, userID) + if err != nil { + return nil, ErrAccountFreeze.Wrap(err) + } + + return freezes, nil } // GetAllEvents returns all events. diff --git a/satellite/console/accountfreezes_test.go b/satellite/console/accountfreezes_test.go index a2e88a286..4aeeb37f3 100644 --- a/satellite/console/accountfreezes_test.go +++ b/satellite/console/accountfreezes_test.go @@ -38,7 +38,7 @@ func randUsageLimits() console.UsageLimits { return console.UsageLimits{Storage: rand.Int63(), Bandwidth: rand.Int63(), Segment: rand.Int63()} } -func TestAccountFreeze(t *testing.T) { +func TestAccountBillingFreeze(t *testing.T) { testplanet.Run(t, testplanet.Config{ SatelliteCount: 1, }, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) { @@ -64,6 +64,11 @@ func TestAccountFreeze(t *testing.T) { require.NoError(t, err) require.False(t, frozen) + require.NoError(t, service.ViolationFreezeUser(ctx, user.ID)) + // cannot billing freeze a violation frozen user. + require.Error(t, service.BillingFreezeUser(ctx, user.ID)) + require.NoError(t, service.ViolationUnfreezeUser(ctx, user.ID)) + require.NoError(t, service.BillingFreezeUser(ctx, user.ID)) user, err = usersDB.Get(ctx, user.ID) @@ -80,7 +85,7 @@ func TestAccountFreeze(t *testing.T) { }) } -func TestAccountUnfreeze(t *testing.T) { +func TestAccountBillingUnFreeze(t *testing.T) { testplanet.Run(t, testplanet.Config{ SatelliteCount: 1, }, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) { @@ -119,6 +124,79 @@ func TestAccountUnfreeze(t *testing.T) { }) } +func TestAccountViolationFreeze(t *testing.T) { + testplanet.Run(t, testplanet.Config{ + SatelliteCount: 1, + }, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) { + sat := planet.Satellites[0] + usersDB := sat.DB.Console().Users() + projectsDB := sat.DB.Console().Projects() + service := console.NewAccountFreezeService(sat.DB.Console().AccountFreezeEvents(), usersDB, projectsDB, sat.API.Analytics.Service) + + userLimits := randUsageLimits() + user, err := sat.AddUser(ctx, console.CreateUser{ + FullName: "Test User", + Email: "user@mail.test", + }, 2) + require.NoError(t, err) + require.NoError(t, usersDB.UpdateUserProjectLimits(ctx, user.ID, userLimits)) + + projLimits := randUsageLimits() + proj, err := sat.AddProject(ctx, user.ID, "") + require.NoError(t, err) + require.NoError(t, projectsDB.UpdateUsageLimits(ctx, proj.ID, projLimits)) + + checkLimits := func(testT *testing.T) { + user, err = usersDB.Get(ctx, user.ID) + require.NoError(t, err) + require.Zero(t, getUserLimits(user)) + + proj, err = projectsDB.Get(ctx, proj.ID) + require.NoError(t, err) + require.Zero(t, getProjectLimits(proj)) + } + + frozen, err := service.IsUserViolationFrozen(ctx, user.ID) + require.NoError(t, err) + require.False(t, frozen) + + require.NoError(t, service.ViolationFreezeUser(ctx, user.ID)) + frozen, err = service.IsUserViolationFrozen(ctx, user.ID) + require.NoError(t, err) + require.True(t, frozen) + + checkLimits(t) + + require.NoError(t, service.ViolationUnfreezeUser(ctx, user.ID)) + frozen, err = service.IsUserViolationFrozen(ctx, user.ID) + require.NoError(t, err) + require.False(t, frozen) + + require.NoError(t, service.BillingWarnUser(ctx, user.ID)) + frozen, err = service.IsUserViolationFrozen(ctx, user.ID) + require.NoError(t, err) + require.False(t, frozen) + // violation freezing a warned user should be possible. + require.NoError(t, service.ViolationFreezeUser(ctx, user.ID)) + frozen, err = service.IsUserViolationFrozen(ctx, user.ID) + require.NoError(t, err) + require.True(t, frozen) + require.NoError(t, service.ViolationUnfreezeUser(ctx, user.ID)) + + require.NoError(t, service.BillingFreezeUser(ctx, user.ID)) + frozen, err = service.IsUserViolationFrozen(ctx, user.ID) + require.NoError(t, err) + require.False(t, frozen) + // violation freezing a billing frozen user should be possible. + require.NoError(t, service.ViolationFreezeUser(ctx, user.ID)) + frozen, err = service.IsUserViolationFrozen(ctx, user.ID) + require.NoError(t, err) + require.True(t, frozen) + + checkLimits(t) + }) +} + func TestRemoveAccountBillingWarning(t *testing.T) { testplanet.Run(t, testplanet.Config{ SatelliteCount: 1, @@ -137,19 +215,44 @@ func TestRemoveAccountBillingWarning(t *testing.T) { require.NoError(t, service.BillingWarnUser(ctx, user.ID)) require.NoError(t, service.BillingUnWarnUser(ctx, user.ID)) - freeze, warning, err := service.GetAll(ctx, user.ID) + freezes, err := service.GetAll(ctx, user.ID) require.NoError(t, err) - require.Nil(t, warning) - require.Nil(t, freeze) + require.NotNil(t, freezes) + require.Nil(t, freezes.BillingWarning) + require.Nil(t, freezes.BillingFreeze) + require.Nil(t, freezes.ViolationFreeze) require.NoError(t, service.BillingWarnUser(ctx, user.ID)) + + freezes, err = service.GetAll(ctx, user.ID) + require.NoError(t, err) + require.NotNil(t, freezes.BillingWarning) + require.Nil(t, freezes.BillingFreeze) + require.Nil(t, freezes.ViolationFreeze) require.NoError(t, service.BillingFreezeUser(ctx, user.ID)) - freeze, warning, err = service.GetAll(ctx, user.ID) + freezes, err = service.GetAll(ctx, user.ID) require.NoError(t, err) - require.NotNil(t, freeze) + require.NotNil(t, freezes.BillingFreeze) + require.Nil(t, freezes.ViolationFreeze) // billing-freezing should remove prior warning events. - require.Nil(t, warning) + require.Nil(t, freezes.BillingWarning) + + // cannot warn a billing-frozen user. + require.Error(t, service.BillingWarnUser(ctx, user.ID)) + require.NoError(t, service.BillingUnfreezeUser(ctx, user.ID)) + + require.NoError(t, service.BillingWarnUser(ctx, user.ID)) + require.NoError(t, service.ViolationFreezeUser(ctx, user.ID)) + // cannot warn a violation-frozen user. + require.Error(t, service.BillingWarnUser(ctx, user.ID)) + + freezes, err = service.GetAll(ctx, user.ID) + require.NoError(t, err) + require.NotNil(t, freezes.ViolationFreeze) + require.Nil(t, freezes.BillingFreeze) + // billing-freezing should remove prior warning events. + require.Nil(t, freezes.BillingWarning) }) } @@ -244,6 +347,12 @@ func TestAccountFreezeAlreadyFrozen(t *testing.T) { require.NoError(t, err) require.Equal(t, userLimits, getUserLimits(user)) }) + + // Billing freezing a violation frozen user should not be possible. + t.Run("ViolationFrozen user", func(t *testing.T) { + require.NoError(t, service.ViolationFreezeUser(ctx, user.ID)) + require.Error(t, service.BillingFreezeUser(ctx, user.ID)) + }) }) } @@ -282,20 +391,7 @@ func TestFreezeEffects(t *testing.T) { require.Equal(testT, expectedData, data) } - t.Run("BillingFreeze effect on project owner", func(t *testing.T) { - shouldUploadAndDownload(t) - - err = freezeService.BillingWarnUser(ctx, user1.ID) - require.NoError(t, err) - - // Should be able to download because account is not frozen. - data, err := uplink1.Download(ctx, sat, bucketName, path) - require.NoError(t, err) - require.Equal(t, expectedData, data) - - err = freezeService.BillingFreezeUser(ctx, user1.ID) - require.NoError(t, err) - + shouldNotUploadAndDownload := func(testT *testing.T) { // Should not be able to upload because account is frozen. err = uplink1.Upload(ctx, sat, bucketName, path, expectedData) require.Error(t, err) @@ -307,7 +403,9 @@ func TestFreezeEffects(t *testing.T) { // Should not be able to create bucket because account is frozen. err = uplink1.CreateBucket(ctx, sat, "anotherBucket") require.Error(t, err) + } + shouldListAndDelete := func(testT *testing.T) { // Should be able to list even if frozen. objects, err := uplink1.ListObjects(ctx, sat, bucketName) require.NoError(t, err) @@ -316,6 +414,37 @@ func TestFreezeEffects(t *testing.T) { // Should be able to delete even if frozen. err = uplink1.DeleteObject(ctx, sat, bucketName, path) require.NoError(t, err) + } + + t.Run("BillingFreeze effect on project owner", func(t *testing.T) { + shouldUploadAndDownload(t) + + err = freezeService.BillingWarnUser(ctx, user1.ID) + require.NoError(t, err) + + // Should be able to download because account is not frozen. + shouldUploadAndDownload(t) + + err = freezeService.BillingFreezeUser(ctx, user1.ID) + require.NoError(t, err) + + shouldNotUploadAndDownload(t) + + shouldListAndDelete(t) + + err = freezeService.BillingUnfreezeUser(ctx, user1.ID) + require.NoError(t, err) + }) + + t.Run("ViolationFreeze effect on project owner", func(t *testing.T) { + shouldUploadAndDownload(t) + + err = freezeService.ViolationFreezeUser(ctx, user1.ID) + require.NoError(t, err) + + shouldNotUploadAndDownload(t) + + shouldListAndDelete(t) }) }) } diff --git a/satellite/console/consoleweb/consoleapi/auth.go b/satellite/console/consoleweb/consoleapi/auth.go index b6fb592f1..9bc2cf152 100644 --- a/satellite/console/consoleweb/consoleapi/auth.go +++ b/satellite/console/consoleweb/consoleapi/auth.go @@ -404,8 +404,9 @@ func loadSession(req *http.Request) string { // GetFreezeStatus checks to see if an account is frozen or warned. func (a *Auth) GetFreezeStatus(w http.ResponseWriter, r *http.Request) { type FrozenResult struct { - Frozen bool `json:"frozen"` - Warned bool `json:"warned"` + Frozen bool `json:"frozen"` + Warned bool `json:"warned"` + ViolationFrozen bool `json:"violationFrozen"` } ctx := r.Context() @@ -418,7 +419,7 @@ func (a *Auth) GetFreezeStatus(w http.ResponseWriter, r *http.Request) { return } - freeze, warning, err := a.accountFreezeService.GetAll(ctx, userID) + freezes, err := a.accountFreezeService.GetAll(ctx, userID) if err != nil { a.serveJSONError(ctx, w, err) return @@ -426,8 +427,9 @@ func (a *Auth) GetFreezeStatus(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") err = json.NewEncoder(w).Encode(FrozenResult{ - Frozen: freeze != nil, - Warned: warning != nil, + Frozen: freezes.BillingFreeze != nil, + Warned: freezes.BillingWarning != nil, + ViolationFrozen: freezes.ViolationFreeze != nil, }) if err != nil { a.log.Error("could not encode account status", zap.Error(ErrAuthAPI.Wrap(err))) diff --git a/satellite/console/consoleweb/consoleapi/payments.go b/satellite/console/consoleweb/consoleapi/payments.go index 81bc5086f..311756d38 100644 --- a/satellite/console/consoleweb/consoleapi/payments.go +++ b/satellite/console/consoleweb/consoleapi/payments.go @@ -165,22 +165,26 @@ func (p *Payments) triggerAttemptPayment(ctx context.Context) (err error) { return err } - freeze, warning, err := p.accountFreezeService.GetAll(ctx, userID) + freezes, err := p.accountFreezeService.GetAll(ctx, userID) if err != nil { return err } + if freezes.ViolationFreeze != nil { + return nil + } + err = p.service.Payments().AttemptPayOverdueInvoices(ctx) if err != nil { return err } - if freeze != nil { + if freezes.BillingFreeze != nil { err = p.accountFreezeService.BillingUnfreezeUser(ctx, userID) if err != nil { return err } - } else if warning != nil { + } else if freezes.BillingWarning != nil { err = p.accountFreezeService.BillingUnWarnUser(ctx, userID) if err != nil { return err diff --git a/satellite/console/observerpayinvoicewithtokens.go b/satellite/console/observerpayinvoicewithtokens.go index dc17109f1..8f07c6117 100644 --- a/satellite/console/observerpayinvoicewithtokens.go +++ b/satellite/console/observerpayinvoicewithtokens.go @@ -46,12 +46,12 @@ func (o *InvoiceTokenPaymentObserver) Process(ctx context.Context, transaction b return err } - freeze, warning, err := o.freezeService.GetAll(ctx, user.ID) + freezes, err := o.freezeService.GetAll(ctx, user.ID) if err != nil { return err } - if freeze == nil && warning == nil { + if freezes.BillingFreeze == nil && freezes.BillingWarning == nil { return nil } @@ -66,12 +66,12 @@ func (o *InvoiceTokenPaymentObserver) Process(ctx context.Context, transaction b } } - if freeze != nil { + if freezes.BillingFreeze != nil { err = o.freezeService.BillingUnfreezeUser(ctx, user.ID) if err != nil { return err } - } else if warning != nil { + } else if freezes.BillingWarning != nil { err = o.freezeService.BillingUnWarnUser(ctx, user.ID) if err != nil { return err diff --git a/satellite/payments/accountfreeze/chore.go b/satellite/payments/accountfreeze/chore.go index bebceda81..0a97f51ec 100644 --- a/satellite/payments/accountfreeze/chore.go +++ b/satellite/payments/accountfreeze/chore.go @@ -87,7 +87,7 @@ func (chore *Chore) attemptFreezeWarn(ctx context.Context) { chore.log.Error("Could not list invoices", zap.Error(Error.Wrap(err))) return } - chore.log.Debug("failed invoices found", zap.Int("count", len(invoices))) + chore.log.Info("failed invoices found", zap.Int("count", len(invoices))) userMap := make(map[uuid.UUID]struct{}) frozenMap := make(map[uuid.UUID]struct{}) @@ -115,8 +115,8 @@ func (chore *Chore) attemptFreezeWarn(ctx context.Context) { } debugLog := func(message string) { - chore.log.Debug(message, - zap.String("process", "freeze/warn"), + chore.log.Info(message, + zap.String("process", "billing freeze/warn"), zap.String("invoiceID", invoice.ID), zap.String("customerID", invoice.CustomerID), zap.Any("userID", userID), @@ -125,7 +125,7 @@ func (chore *Chore) attemptFreezeWarn(ctx context.Context) { errorLog := func(message string, err error) { chore.log.Error(message, - zap.String("process", "freeze/warn"), + zap.String("process", "billing freeze/warn"), zap.String("invoiceID", invoice.ID), zap.String("customerID", invoice.CustomerID), zap.Any("userID", userID), @@ -176,27 +176,33 @@ func (chore *Chore) attemptFreezeWarn(ctx context.Context) { } } - freeze, warning, err := chore.freezeService.GetAll(ctx, userID) + freezes, err := chore.freezeService.GetAll(ctx, userID) if err != nil { errorLog("Could not get freeze status", err) continue } + if freezes.ViolationFreeze != nil { + debugLog("Ignoring invoice; account already frozen due to violation") + chore.analytics.TrackViolationFrozenUnpaidInvoice(invoice.ID, userID, user.Email) + continue + } + // try to pay the invoice before freezing/warning. err = chore.payments.Invoices().AttemptPayOverdueInvoices(ctx, userID) if err == nil { debugLog("Ignoring invoice; Payment attempt successful") - if warning != nil { + if freezes.BillingWarning != nil { err = chore.freezeService.BillingUnWarnUser(ctx, userID) if err != nil { - errorLog("Could not remove warning event", err) + errorLog("Could not remove billing warning event", err) } } - if freeze != nil { + if freezes.BillingFreeze != nil { err = chore.freezeService.BillingUnfreezeUser(ctx, userID) if err != nil { - errorLog("Could not remove freeze event", err) + errorLog("Could not remove billing freeze event", err) } } @@ -205,12 +211,12 @@ func (chore *Chore) attemptFreezeWarn(ctx context.Context) { errorLog("Could not attempt payment", err) } - if freeze != nil { - debugLog("Ignoring invoice; account already frozen") + if freezes.BillingFreeze != nil { + debugLog("Ignoring invoice; account already billing frozen") continue } - if warning == nil { + if freezes.BillingWarning == nil { // check if the invoice has been paid by the time the chore gets here. isPaid, err := checkInvPaid(invoice.ID) if err != nil { @@ -223,15 +229,15 @@ func (chore *Chore) attemptFreezeWarn(ctx context.Context) { } err = chore.freezeService.BillingWarnUser(ctx, userID) if err != nil { - errorLog("Could not add warning event", err) + errorLog("Could not add billing warning event", err) continue } - debugLog("user warned") + debugLog("user billing warned") warnedMap[userID] = struct{}{} continue } - if chore.nowFn().Sub(warning.CreatedAt) > chore.config.GracePeriod { + if chore.nowFn().Sub(freezes.BillingWarning.CreatedAt) > chore.config.GracePeriod { // check if the invoice has been paid by the time the chore gets here. isPaid, err := checkInvPaid(invoice.ID) if err != nil { @@ -244,15 +250,15 @@ func (chore *Chore) attemptFreezeWarn(ctx context.Context) { } err = chore.freezeService.BillingFreezeUser(ctx, userID) if err != nil { - errorLog("Could not freeze account", err) + errorLog("Could not billing freeze account", err) continue } - debugLog("user frozen") + debugLog("user billing frozen") frozenMap[userID] = struct{}{} } } - chore.log.Debug("freezing/warning executed", + chore.log.Info("billing freezing/warning executed", zap.Int("total invoices", len(invoices)), zap.Int("user total", len(userMap)), zap.Int("total warned", len(warnedMap)), @@ -288,13 +294,23 @@ func (chore *Chore) attemptUnfreezeUnwarn(ctx context.Context) { for _, event := range events.Events { errorLog := func(message string, err error) { chore.log.Error(message, - zap.String("process", "unfreeze/unwarn"), + zap.String("process", "billing unfreeze/unwarn"), zap.Any("userID", event.UserID), zap.String("eventType", event.Type.String()), zap.Error(Error.Wrap(err)), ) } + if event.Type == console.ViolationFreeze { + chore.log.Info("Skipping violation freeze event", + zap.String("process", "billing unfreeze/unwarn"), + zap.Any("userID", event.UserID), + zap.String("eventType", event.Type.String()), + zap.Error(Error.Wrap(err)), + ) + continue + } + usersCount++ invoices, err := chore.payments.Invoices().ListFailed(ctx, &event.UserID) if err != nil { @@ -313,13 +329,13 @@ func (chore *Chore) attemptUnfreezeUnwarn(ctx context.Context) { if event.Type == console.BillingFreeze { err = chore.freezeService.BillingUnfreezeUser(ctx, event.UserID) if err != nil { - errorLog("Could not unfreeze user", err) + errorLog("Could not billing unfreeze user", err) } unfrozenCount++ } else { err = chore.freezeService.BillingUnWarnUser(ctx, event.UserID) if err != nil { - errorLog("Could not unwarn user", err) + errorLog("Could not billing unwarn user", err) } unwarnedCount++ } @@ -331,7 +347,7 @@ func (chore *Chore) attemptUnfreezeUnwarn(ctx context.Context) { } } - chore.log.Debug("unfreezing/unwarning executed", + chore.log.Info("billing unfreezing/unwarning executed", zap.Int("user total", usersCount), zap.Int("total unwarned", unwarnedCount), zap.Int("total unfrozen", unfrozenCount), diff --git a/satellite/payments/accountfreeze/chore_test.go b/satellite/payments/accountfreeze/chore_test.go index 8e9727c2d..58998f12e 100644 --- a/satellite/payments/accountfreeze/chore_test.go +++ b/satellite/payments/accountfreeze/chore_test.go @@ -56,6 +56,91 @@ func TestAutoFreezeChore(t *testing.T) { amount := int64(100) curr := string(stripe.CurrencyUSD) + t.Run("No billing event for violation frozen user", func(t *testing.T) { + // AnalyticsMock tests that events are sent once. + service.TestChangeFreezeTracker(newFreezeTrackerMock(t)) + + violatingUser, err := sat.AddUser(ctx, console.CreateUser{ + FullName: "Violating User", + Email: "violating@mail.test", + }, 1) + require.NoError(t, err) + + cus2, err := customerDB.GetCustomerID(ctx, violatingUser.ID) + require.NoError(t, err) + + inv, err := stripeClient.Invoices().New(&stripe.InvoiceParams{ + Params: stripe.Params{Context: ctx}, + Customer: &cus2, + }) + require.NoError(t, err) + + _, err = stripeClient.InvoiceItems().New(&stripe.InvoiceItemParams{ + Params: stripe.Params{Context: ctx}, + Amount: &amount, + Currency: &curr, + Customer: &cus2, + Invoice: &inv.ID, + }) + require.NoError(t, err) + + paymentMethod := stripe1.MockInvoicesPayFailure + inv, err = stripeClient.Invoices().Pay(inv.ID, &stripe.InvoicePayParams{ + Params: stripe.Params{Context: ctx}, + PaymentMethod: &paymentMethod, + }) + require.Error(t, err) + require.Equal(t, stripe.InvoiceStatusOpen, inv.Status) + + failed, err := invoicesDB.ListFailed(ctx, nil) + require.NoError(t, err) + require.Equal(t, 1, len(failed)) + + require.NoError(t, service.ViolationFreezeUser(ctx, violatingUser.ID)) + + chore.Loop.TriggerWait() + + // user should not be billing warned or frozen. + freezes, err := service.GetAll(ctx, violatingUser.ID) + require.NoError(t, err) + require.NotNil(t, freezes) + require.Nil(t, freezes.BillingWarning) + require.Nil(t, freezes.BillingFreeze) + require.NotNil(t, freezes.ViolationFreeze) + + // forward date to after the grace period + chore.TestSetNow(func() time.Time { + return time.Now().AddDate(0, 0, 50) + }) + chore.Loop.TriggerWait() + + // user should still not be billing warned or frozen. + freezes, err = service.GetAll(ctx, violatingUser.ID) + require.NoError(t, err) + require.NotNil(t, freezes) + require.Nil(t, freezes.BillingFreeze) + require.Nil(t, freezes.BillingWarning) + require.NotNil(t, freezes.ViolationFreeze) + + paymentMethod = stripe1.MockInvoicesPaySuccess + _, err = stripeClient.Invoices().Pay(inv.ID, &stripe.InvoicePayParams{ + Params: stripe.Params{Context: ctx}, + PaymentMethod: &paymentMethod, + }) + require.NoError(t, err) + require.Equal(t, stripe.InvoiceStatusPaid, inv.Status) + + chore.Loop.TriggerWait() + + // paying for the invoice does not remove the violation freeze + freezes, err = service.GetAll(ctx, violatingUser.ID) + require.NoError(t, err) + require.NotNil(t, freezes) + require.Nil(t, freezes.BillingFreeze) + require.Nil(t, freezes.BillingWarning) + require.NotNil(t, freezes.ViolationFreeze) + }) + t.Run("No freeze event for paid invoice", func(t *testing.T) { // AnalyticsMock tests that events are sent once. service.TestChangeFreezeTracker(newFreezeTrackerMock(t)) @@ -88,10 +173,12 @@ func TestAutoFreezeChore(t *testing.T) { chore.Loop.TriggerWait() // user should not be warned or frozen. - freeze, warning, err := service.GetAll(ctx, user.ID) + freezes, err := service.GetAll(ctx, user.ID) require.NoError(t, err) - require.Nil(t, warning) - require.Nil(t, freeze) + require.NotNil(t, freezes) + require.Nil(t, freezes.BillingFreeze) + require.Nil(t, freezes.BillingWarning) + require.Nil(t, freezes.ViolationFreeze) // forward date to after the grace period chore.TestSetNow(func() time.Time { @@ -100,10 +187,11 @@ func TestAutoFreezeChore(t *testing.T) { chore.Loop.TriggerWait() // user should still not be warned or frozen. - freeze, warning, err = service.GetAll(ctx, user.ID) + freezes, err = service.GetAll(ctx, user.ID) require.NoError(t, err) - require.Nil(t, freeze) - require.Nil(t, warning) + require.Nil(t, freezes.BillingFreeze) + require.Nil(t, freezes.BillingWarning) + require.Nil(t, freezes.ViolationFreeze) }) t.Run("BillingFreeze event for failed invoice (failed later payment attempt)", func(t *testing.T) { @@ -143,10 +231,11 @@ func TestAutoFreezeChore(t *testing.T) { chore.Loop.TriggerWait() // user should be warned the first time - freeze, warning, err := service.GetAll(ctx, user.ID) + freezes, err := service.GetAll(ctx, user.ID) require.NoError(t, err) - require.NotNil(t, warning) - require.Nil(t, freeze) + require.NotNil(t, freezes.BillingWarning) + require.Nil(t, freezes.BillingFreeze) + require.Nil(t, freezes.ViolationFreeze) chore.TestSetNow(func() time.Time { // current date is now after grace period @@ -155,9 +244,9 @@ func TestAutoFreezeChore(t *testing.T) { chore.Loop.TriggerWait() // user should be frozen this time around - freeze, _, err = service.GetAll(ctx, user.ID) + freezes, err = service.GetAll(ctx, user.ID) require.NoError(t, err) - require.NotNil(t, freeze) + require.NotNil(t, freezes.BillingFreeze) // Pay invoice so it doesn't show up in the next test. inv, err = stripeClient.Invoices().Pay(inv.ID, &stripe.InvoicePayParams{ @@ -210,10 +299,11 @@ func TestAutoFreezeChore(t *testing.T) { require.NoError(t, err) require.Equal(t, 0, len(failed)) - freeze, warning, err := service.GetAll(ctx, user.ID) + freezes, err := service.GetAll(ctx, user.ID) require.NoError(t, err) - require.Nil(t, warning) - require.Nil(t, freeze) + require.Nil(t, freezes.BillingWarning) + require.Nil(t, freezes.BillingFreeze) + require.Nil(t, freezes.ViolationFreeze) }) t.Run("User unfrozen/unwarned for no failed invoices", func(t *testing.T) { @@ -266,14 +356,14 @@ func TestAutoFreezeChore(t *testing.T) { chore.Loop.TriggerWait() // user(1) should be unfrozen because they have no failed invoices - freeze, _, err := service.GetAll(ctx, user.ID) + freezes, err := service.GetAll(ctx, user.ID) require.NoError(t, err) - require.Nil(t, freeze) + require.Nil(t, freezes.BillingFreeze) // user2 should still be frozen because they have failed invoices - freeze, _, err = service.GetAll(ctx, user2.ID) + freezes, err = service.GetAll(ctx, user2.ID) require.NoError(t, err) - require.NotNil(t, freeze) + require.NotNil(t, freezes.BillingFreeze) // warn user though they have no failed invoices err = service.BillingWarnUser(ctx, user.ID) @@ -282,9 +372,9 @@ func TestAutoFreezeChore(t *testing.T) { chore.Loop.TriggerWait() // warned status should be reset - _, warning, err := service.GetAll(ctx, user.ID) + freezes, err = service.GetAll(ctx, user.ID) require.NoError(t, err) - require.Nil(t, warning) + require.Nil(t, freezes.BillingWarning) // Pay invoice so it doesn't show up in the next test. inv, err = stripeClient.Invoices().Pay(inv.ID, &stripe.InvoicePayParams{ @@ -394,10 +484,11 @@ func TestAutoFreezeChore_StorjscanExclusion(t *testing.T) { chore.Loop.TriggerWait() // user should not be warned or frozen due to storjscan payments - freeze, warning, err := service.GetAll(ctx, storjscanUser.ID) + freezes, err := service.GetAll(ctx, storjscanUser.ID) require.NoError(t, err) - require.Nil(t, warning) - require.Nil(t, freeze) + require.Nil(t, freezes.BillingWarning) + require.Nil(t, freezes.BillingFreeze) + require.Nil(t, freezes.ViolationFreeze) }) } @@ -445,3 +536,5 @@ func (mock *freezeTrackerMock) TrackAccountFreezeWarning(_ uuid.UUID, email stri func (mock *freezeTrackerMock) TrackLargeUnpaidInvoice(_ string, _ uuid.UUID, _ string) {} func (mock *freezeTrackerMock) TrackStorjscanUnpaidInvoice(_ string, _ uuid.UUID, _ string) {} + +func (mock *freezeTrackerMock) TrackViolationFrozenUnpaidInvoice(_ string, _ uuid.UUID, _ string) {} diff --git a/satellite/satellitedb/accountfreezeevents.go b/satellite/satellitedb/accountfreezeevents.go index 8980e4979..457db5b03 100644 --- a/satellite/satellitedb/accountfreezeevents.go +++ b/satellite/satellitedb/accountfreezeevents.go @@ -120,33 +120,41 @@ func (events *accountFreezeEvents) GetAllEvents(ctx context.Context, cursor cons } // GetAll is a method for querying all account freeze events from the database by user ID. -func (events *accountFreezeEvents) GetAll(ctx context.Context, userID uuid.UUID) (freeze *console.AccountFreezeEvent, warning *console.AccountFreezeEvent, err error) { +func (events *accountFreezeEvents) GetAll(ctx context.Context, userID uuid.UUID) (freezes *console.UserFreezeEvents, err error) { defer mon.Task()(&ctx)(&err) - // dbxEvents will have a max length of 2. - // because there's at most 1 instance each of 2 types of events for a user. + // dbxEvents will have a max length of 3. + // because there's at most 1 instance each of 3 types of events for a user. dbxEvents, err := events.db.All_AccountFreezeEvent_By_UserId(ctx, dbx.AccountFreezeEvent_UserId(userID.Bytes()), ) if err != nil { - return nil, nil, err + return nil, err } + freezes = &console.UserFreezeEvents{} for _, event := range dbxEvents { if console.AccountFreezeEventType(event.Event) == console.BillingFreeze { - freeze, err = fromDBXAccountFreezeEvent(event) + freezes.BillingFreeze, err = fromDBXAccountFreezeEvent(event) if err != nil { - return nil, nil, err + return nil, err } continue } - warning, err = fromDBXAccountFreezeEvent(event) + if console.AccountFreezeEventType(event.Event) == console.ViolationFreeze { + freezes.ViolationFreeze, err = fromDBXAccountFreezeEvent(event) + if err != nil { + return nil, err + } + continue + } + freezes.BillingWarning, err = fromDBXAccountFreezeEvent(event) if err != nil { - return nil, nil, err + return nil, err } } - return freeze, warning, nil + return freezes, nil } // DeleteAllByUserID is a method for deleting all account freeze events from the database by user ID.