satellite/{admin,payments,satellitedb}: add checks for deletion of free tier accounts

This change adds some more checks to the deletion process for projects and
users, since we ran into a race condition during invoicing, where projects
have been deleted before the invoicing was finished, leading to missing
references.
This PR changes the logic to block user deletion if we are in exactly that period,
while also allowing the deletion of projects/users on free tier during the month.

Change-Id: Ic0735205e6633762fb7e3c2fa13e744cdfa5ec32

satellite/admin/project.go

@@ -502,59 +502,82 @@ func (server *Server) deleteProject(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-func (server *Server) checkUsage(ctx context.Context, w http.ResponseWriter, projectID uuid.UUID) (hasUsage bool) {
-	// do not delete projects that have usage for the current month.
+func (server *Server) checkInvoicing(ctx context.Context, w http.ResponseWriter, projectID uuid.UUID) (openInvoices bool) {
 	year, month, _ := server.nowFn().UTC().Date()
 	firstOfMonth := time.Date(year, month, 1, 0, 0, 0, 0, time.UTC)
 
-	currentUsage, err := server.db.ProjectAccounting().GetProjectTotal(ctx, projectID, firstOfMonth, server.nowFn())
-	if err != nil {
-		sendJSONError(w, "unable to list project usage", err.Error(), http.StatusInternalServerError)
-		return true
-	}
-	if currentUsage.Storage > 0 || currentUsage.Egress > 0 || currentUsage.SegmentCount > 0 {
-		sendJSONError(w, "usage for current month exists", "", http.StatusConflict)
-		return true
-	}
-
-	// if usage of last month exist, make sure to look for billing records
-	lastMonthUsage, err := server.db.ProjectAccounting().GetProjectTotal(ctx, projectID, firstOfMonth.AddDate(0, -1, 0), firstOfMonth.AddDate(0, 0, -1))
-	if err != nil {
-		sendJSONError(w, "error getting project totals",
-			"", http.StatusInternalServerError)
-		return true
-	}
-
-	if lastMonthUsage.Storage > 0 || lastMonthUsage.Egress > 0 || lastMonthUsage.SegmentCount > 0 {
-		// time passed into the check function need to be the UTC midnight dates
-		// of the first day of the current month and the first day of the last
-		// month
-		err := server.db.StripeCoinPayments().ProjectRecords().Check(ctx, projectID, firstOfMonth.AddDate(0, -1, 0), firstOfMonth)
-		if errors.Is(err, stripecoinpayments.ErrProjectRecordExists) {
-			record, err := server.db.StripeCoinPayments().ProjectRecords().Get(ctx, projectID, firstOfMonth.AddDate(0, -1, 0), firstOfMonth)
-			if err != nil {
-				sendJSONError(w, "unable to get project records", err.Error(), http.StatusInternalServerError)
-				return true
-			}
-			// state = 0 means unapplied and not invoiced yet.
-			if record.State == 0 {
-				sendJSONError(w, "unapplied project invoice record exist", "", http.StatusConflict)
-				return true
-			}
-			// Record has been applied, so project can be deleted.
-			return false
-		}
+	// Check if an invoice project record exists already
+	err := server.db.StripeCoinPayments().ProjectRecords().Check(ctx, projectID, firstOfMonth.AddDate(0, -1, 0), firstOfMonth)
+	if errors.Is(err, stripecoinpayments.ErrProjectRecordExists) {
+		record, err := server.db.StripeCoinPayments().ProjectRecords().Get(ctx, projectID, firstOfMonth.AddDate(0, -1, 0), firstOfMonth)
 		if err != nil {
 			sendJSONError(w, "unable to get project records", err.Error(), http.StatusInternalServerError)
 			return true
 		}
-		sendJSONError(w, "usage for last month exist, but is not billed yet", "", http.StatusConflict)
+		// state = 0 means unapplied and not invoiced yet.
+		if record.State == 0 {
+			sendJSONError(w, "unapplied project invoice record exist", "", http.StatusConflict)
+			return true
+		}
+		// Record has been applied, so project can be deleted.
+		return false
+	}
+	if err != nil {
+		sendJSONError(w, "unable to get project records", err.Error(), http.StatusInternalServerError)
 		return true
 	}
 
 	return false
 }
 
+func (server *Server) checkUsage(ctx context.Context, w http.ResponseWriter, projectID uuid.UUID) (hasUsage bool) {
+	year, month, _ := server.nowFn().UTC().Date()
+	firstOfMonth := time.Date(year, month, 1, 0, 0, 0, 0, time.UTC)
+
+	prj, err := server.db.Console().Projects().Get(ctx, projectID)
+	if err != nil {
+		sendJSONError(w, "unable to get project details",
+			err.Error(), http.StatusInternalServerError)
+		return
+	}
+
+	// If user is paid tier, check the usage limit, otherwise it is ok to delete it.
+	paid, err := server.db.Console().Users().GetUserPaidTier(ctx, prj.OwnerID)
+	if err != nil {
+		sendJSONError(w, "unable to project owner tier",
+			err.Error(), http.StatusInternalServerError)
+		return
+	}
+	if paid {
+		// check current month usage and do not allow deletion if usage exists
+		currentUsage, err := server.db.ProjectAccounting().GetProjectTotal(ctx, projectID, firstOfMonth, server.nowFn())
+		if err != nil {
+			sendJSONError(w, "unable to list project usage", err.Error(), http.StatusInternalServerError)
+			return true
+		}
+		if currentUsage.Storage > 0 || currentUsage.Egress > 0 || currentUsage.SegmentCount > 0 {
+			sendJSONError(w, "usage for current month exists", "", http.StatusConflict)
+			return true
+		}
+		// check usage for last month
+		lastMonthUsage, err := server.db.ProjectAccounting().GetProjectTotal(ctx, projectID, firstOfMonth.AddDate(0, -1, 0), firstOfMonth.AddDate(0, 0, -1))
+		if err != nil {
+			sendJSONError(w, "error getting project totals",
+				"", http.StatusInternalServerError)
+			return true
+		}
+
+		// project had usage that is not billed yet
+		if lastMonthUsage.Storage > 0 || lastMonthUsage.Egress > 0 || lastMonthUsage.SegmentCount > 0 {
+			sendJSONError(w, "usage for last month exist, but is not billed yet", "", http.StatusConflict)
+			return true
+		}
+	}
+
+	// If we have open invoice items, do not delete the project yet and wait for invoice completion.
+	return server.checkInvoicing(ctx, w, projectID)
+}
+
 func bucketNames(buckets []storj.Bucket) []string {
 	var xs []string
 	for _, b := range buckets {

satellite/admin/project_test.go

@@ -17,6 +17,7 @@ import (
 	"go.uber.org/zap"
 
 	"storj.io/common/macaroon"
+	"storj.io/common/memory"
 	"storj.io/common/storj"
 	"storj.io/common/testcontext"
 	"storj.io/common/uuid"
@@ -437,17 +438,45 @@ func TestProjectCheckUsage_withUsage(t *testing.T) {
 		err = planet.Satellites[0].DB.ProjectAccounting().CreateStorageTally(ctx, tally)
 		require.NoError(t, err)
 
+		// Ensure User is free tier.
+		paid, err := planet.Satellites[0].DB.Console().Users().GetUserPaidTier(ctx, planet.Uplinks[0].Projects[0].Owner.ID)
+		require.NoError(t, err)
+		require.False(t, paid)
+
 		req, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf("http://"+address.String()+"/api/projects/%s/usage", projectID), nil)
 		require.NoError(t, err)
 		req.Header.Set("Authorization", planet.Satellites[0].Config.Console.AuthToken)
 
 		response, err := http.DefaultClient.Do(req)
 		require.NoError(t, err)
-		require.Equal(t, http.StatusConflict, response.StatusCode)
+		require.Equal(t, http.StatusOK, response.StatusCode)
 		require.Equal(t, "application/json", response.Header.Get("Content-Type"))
 
 		responseBody, err := ioutil.ReadAll(response.Body)
 		require.NoError(t, err)
+		require.Equal(t, "{\"result\":\"no project usage exist\"}", string(responseBody))
+		require.NoError(t, response.Body.Close())
+
+		// Make User paid tier.
+		err = planet.Satellites[0].DB.Console().Users().UpdatePaidTier(ctx, planet.Uplinks[0].Projects[0].Owner.ID, true, memory.PB, memory.PB, 1000000)
+		require.NoError(t, err)
+
+		// Ensure User is paid tier.
+		paid, err = planet.Satellites[0].DB.Console().Users().GetUserPaidTier(ctx, planet.Uplinks[0].Projects[0].Owner.ID)
+		require.NoError(t, err)
+		require.True(t, paid)
+
+		req, err = http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf("http://"+address.String()+"/api/projects/%s/usage", projectID), nil)
+		require.NoError(t, err)
+		req.Header.Set("Authorization", planet.Satellites[0].Config.Console.AuthToken)
+
+		response, err = http.DefaultClient.Do(req)
+		require.NoError(t, err)
+		require.Equal(t, http.StatusConflict, response.StatusCode)
+		require.Equal(t, "application/json", response.Header.Get("Content-Type"))
+
+		responseBody, err = ioutil.ReadAll(response.Body)
+		require.NoError(t, err)
 		require.Equal(t, "{\"error\":\"usage for current month exists\",\"detail\":\"\"}", string(responseBody))
 		require.NoError(t, response.Body.Close())
 	})
@@ -531,6 +560,8 @@ func TestProjectCheckUsage_lastMonthUnappliedInvoice(t *testing.T) {
 	})
 }
 
+// TestProjectDelete_withUsageCurrentMonth first tries to delete a actively used project of a paid tier user, which
+// should fail and afterwards converts the user to free tier and tries the deletion again. That deletion should succeed.
 func TestProjectDelete_withUsageCurrentMonth(t *testing.T) {
 	testplanet.Run(t, testplanet.Config{
 		SatelliteCount:   1,
@@ -581,6 +612,15 @@ func TestProjectDelete_withUsageCurrentMonth(t *testing.T) {
 		err = planet.Satellites[0].DB.ProjectAccounting().CreateStorageTally(ctx, tally)
 		require.NoError(t, err)
 
+		// Make User paid tier.
+		err = planet.Satellites[0].DB.Console().Users().UpdatePaidTier(ctx, planet.Uplinks[0].Projects[0].Owner.ID, true, memory.PB, memory.PB, 1000000)
+		require.NoError(t, err)
+
+		// Ensure User is paid tier.
+		paid, err := planet.Satellites[0].DB.Console().Users().GetUserPaidTier(ctx, planet.Uplinks[0].Projects[0].Owner.ID)
+		require.NoError(t, err)
+		require.True(t, paid)
+
 		req, err := http.NewRequestWithContext(ctx, http.MethodDelete, fmt.Sprintf("http://"+address.String()+"/api/projects/%s", projectID), nil)
 		require.NoError(t, err)
 		req.Header.Set("Authorization", planet.Satellites[0].Config.Console.AuthToken)
@@ -594,9 +634,34 @@ func TestProjectDelete_withUsageCurrentMonth(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, "{\"error\":\"usage for current month exists\",\"detail\":\"\"}", string(responseBody))
 		require.NoError(t, response.Body.Close())
+
+		// Make User free tier again.
+		err = planet.Satellites[0].DB.Console().Users().UpdatePaidTier(ctx, planet.Uplinks[0].Projects[0].Owner.ID, false, memory.TB, memory.TB, 100000)
+		require.NoError(t, err)
+
+		// Ensure User is free tier.
+		paid, err = planet.Satellites[0].DB.Console().Users().GetUserPaidTier(ctx, planet.Uplinks[0].Projects[0].Owner.ID)
+		require.NoError(t, err)
+		require.False(t, paid)
+
+		req, err = http.NewRequestWithContext(ctx, http.MethodDelete, fmt.Sprintf("http://"+address.String()+"/api/projects/%s", projectID), nil)
+		require.NoError(t, err)
+		req.Header.Set("Authorization", planet.Satellites[0].Config.Console.AuthToken)
+
+		response, err = http.DefaultClient.Do(req)
+		require.NoError(t, err)
+		require.Equal(t, http.StatusOK, response.StatusCode)
+
+		responseBody, err = ioutil.ReadAll(response.Body)
+		require.NoError(t, err)
+		require.Equal(t, "", string(responseBody))
+		require.NoError(t, response.Body.Close())
 	})
 }
 
+// TestProjectDelete_withUsageCurrentMonth first tries to delete a last month used project of a paid tier user, which
+// should fail and afterwards converts the user to free tier and tries the deletion again. That deletion should succeed.
+// This test ensures we bill paid tier users for past months usage and do not forget to do so.
 func TestProjectDelete_withUsagePreviousMonth(t *testing.T) {
 	testplanet.Run(t, testplanet.Config{
 		SatelliteCount:   1,
@@ -649,6 +714,15 @@ func TestProjectDelete_withUsagePreviousMonth(t *testing.T) {
 		err = planet.Satellites[0].DB.ProjectAccounting().CreateStorageTally(ctx, tally)
 		require.NoError(t, err)
 
+		// Make User paid tier.
+		err = planet.Satellites[0].DB.Console().Users().UpdatePaidTier(ctx, planet.Uplinks[0].Projects[0].Owner.ID, true, memory.PB, memory.PB, 1000000)
+		require.NoError(t, err)
+
+		// Ensure User is paid tier.
+		paid, err := planet.Satellites[0].DB.Console().Users().GetUserPaidTier(ctx, planet.Uplinks[0].Projects[0].Owner.ID)
+		require.NoError(t, err)
+		require.True(t, paid)
+
 		req, err := http.NewRequestWithContext(ctx, http.MethodDelete, fmt.Sprintf("http://"+address.String()+"/api/projects/%s", projectID), nil)
 		require.NoError(t, err)
 		req.Header.Set("Authorization", planet.Satellites[0].Config.Console.AuthToken)
@@ -660,5 +734,27 @@ func TestProjectDelete_withUsagePreviousMonth(t *testing.T) {
 		require.Equal(t, "{\"error\":\"usage for last month exist, but is not billed yet\",\"detail\":\"\"}", string(responseBody))
 		require.NoError(t, response.Body.Close())
 		require.Equal(t, http.StatusConflict, response.StatusCode)
+
+		// Make User free tier again.
+		err = planet.Satellites[0].DB.Console().Users().UpdatePaidTier(ctx, planet.Uplinks[0].Projects[0].Owner.ID, false, memory.TB, memory.TB, 100000)
+		require.NoError(t, err)
+
+		// Ensure User is free tier.
+		paid, err = planet.Satellites[0].DB.Console().Users().GetUserPaidTier(ctx, planet.Uplinks[0].Projects[0].Owner.ID)
+		require.NoError(t, err)
+		require.False(t, paid)
+
+		req, err = http.NewRequestWithContext(ctx, http.MethodDelete, fmt.Sprintf("http://"+address.String()+"/api/projects/%s", projectID), nil)
+		require.NoError(t, err)
+		req.Header.Set("Authorization", planet.Satellites[0].Config.Console.AuthToken)
+
+		response, err = http.DefaultClient.Do(req)
+		require.NoError(t, err)
+		require.Equal(t, http.StatusOK, response.StatusCode)
+
+		responseBody, err = ioutil.ReadAll(response.Body)
+		require.NoError(t, err)
+		require.Equal(t, "", string(responseBody))
+		require.NoError(t, response.Body.Close())
 	})
 }

satellite/admin/user_test.go

@@ -224,7 +224,7 @@ func TestUserDelete(t *testing.T) {
 		body = assertReq(ctx, t, link, http.MethodDelete, "", http.StatusOK, "", planet.Satellites[0].Config.Console.AuthToken)
 		require.Len(t, body, 0)
 
-		// Deleting unexisting user returns Not Found.
+		// Deleting non-existing user returns Not Found.
 		body = assertReq(ctx, t, link, http.MethodDelete, "", http.StatusNotFound, "", planet.Satellites[0].Config.Console.AuthToken)
 		require.Contains(t, string(body), "does not exist")
 	})

satellite/console/users.go

@@ -34,6 +34,8 @@ type Users interface {
 	GetProjectLimit(ctx context.Context, id uuid.UUID) (limit int, err error)
 	// GetUserProjectLimits is a method to get the users storage and bandwidth limits for new projects.
 	GetUserProjectLimits(ctx context.Context, id uuid.UUID) (limit *ProjectLimits, err error)
+	// GetUserPaidTier is a method to gather whether the specified user is on the Paid Tier or not.
+	GetUserPaidTier(ctx context.Context, id uuid.UUID) (isPaid bool, err error)
 }
 
 // UserInfo holds User updatable data.

satellite/payments/stripecoinpayments/service.go

@@ -539,12 +539,14 @@ func (service *Service) applyProjectRecords(ctx context.Context, records []Proje
 
 	for _, record := range records {
 		if err = ctx.Err(); err != nil {
-			return err
+			return errs.Wrap(err)
 		}
 
 		proj, err := service.projectsDB.Get(ctx, record.ProjectID)
 		if err != nil {
-			return err
+			// This should never happen, but be sure to log info to further troubleshoot before exiting.
+			service.log.Error("project ID for corresponding project record not found", zap.Stringer("Record ID", record.ID), zap.Stringer("Project ID", record.ProjectID))
+			return errs.Wrap(err)
 		}
 
 		cusID, err := service.db.Customers().GetCustomerID(ctx, proj.OwnerID)
@@ -554,11 +556,11 @@ func (service *Service) applyProjectRecords(ctx context.Context, records []Proje
 				continue
 			}
 
-			return err
+			return errs.Wrap(err)
 		}
 
 		if err = service.createInvoiceItems(ctx, cusID, proj.Name, record); err != nil {
-			return err
+			return errs.Wrap(err)
 		}
 	}
 

satellite/satellitedb/dbx/satellitedb.dbx

@@ -332,6 +332,10 @@ read one (
     select user.project_limit
     where user.id = ?
 )
+read one (
+    select user.paid_tier
+    where user.id = ?
+)
 
 read one (
     select user.project_storage_limit user.project_bandwidth_limit user.project_segment_limit

satellite/satellitedb/dbx/satellitedb.dbx.go

@@ -10864,6 +10864,10 @@ type Paged_StoragenodeBandwidthRollup_By_StoragenodeId_And_IntervalStart_Greater
 	_set                  bool
 }
 
+type PaidTier_Row struct {
+	PaidTier bool
+}
+
 type Placement_Row struct {
 	Placement *int
 }
@@ -12373,6 +12377,28 @@ func (obj *pgxImpl) Get_User_ProjectLimit_By_Id(ctx context.Context,
 
 }
 
+func (obj *pgxImpl) Get_User_PaidTier_By_Id(ctx context.Context,
+	user_id User_Id_Field) (
+	row *PaidTier_Row, err error) {
+	defer mon.Task()(&ctx)(&err)
+
+	var __embed_stmt = __sqlbundle_Literal("SELECT users.paid_tier FROM users WHERE users.id = ?")
+
+	var __values []interface{}
+	__values = append(__values, user_id.value())
+
+	var __stmt = __sqlbundle_Render(obj.dialect, __embed_stmt)
+	obj.logStmt(__stmt, __values...)
+
+	row = &PaidTier_Row{}
+	err = obj.queryRowContext(ctx, __stmt, __values...).Scan(&row.PaidTier)
+	if err != nil {
+		return (*PaidTier_Row)(nil), obj.makeErr(err)
+	}
+	return row, nil
+
+}
+
 func (obj *pgxImpl) Get_User_ProjectStorageLimit_User_ProjectBandwidthLimit_User_ProjectSegmentLimit_By_Id(ctx context.Context,
 	user_id User_Id_Field) (
 	row *ProjectStorageLimit_ProjectBandwidthLimit_ProjectSegmentLimit_Row, err error) {
@@ -18720,6 +18746,28 @@ func (obj *pgxcockroachImpl) Get_User_ProjectLimit_By_Id(ctx context.Context,
 
 }
 
+func (obj *pgxcockroachImpl) Get_User_PaidTier_By_Id(ctx context.Context,
+	user_id User_Id_Field) (
+	row *PaidTier_Row, err error) {
+	defer mon.Task()(&ctx)(&err)
+
+	var __embed_stmt = __sqlbundle_Literal("SELECT users.paid_tier FROM users WHERE users.id = ?")
+
+	var __values []interface{}
+	__values = append(__values, user_id.value())
+
+	var __stmt = __sqlbundle_Render(obj.dialect, __embed_stmt)
+	obj.logStmt(__stmt, __values...)
+
+	row = &PaidTier_Row{}
+	err = obj.queryRowContext(ctx, __stmt, __values...).Scan(&row.PaidTier)
+	if err != nil {
+		return (*PaidTier_Row)(nil), obj.makeErr(err)
+	}
+	return row, nil
+
+}
+
 func (obj *pgxcockroachImpl) Get_User_ProjectStorageLimit_User_ProjectBandwidthLimit_User_ProjectSegmentLimit_By_Id(ctx context.Context,
 	user_id User_Id_Field) (
 	row *ProjectStorageLimit_ProjectBandwidthLimit_ProjectSegmentLimit_Row, err error) {
@@ -24720,6 +24768,16 @@ func (rx *Rx) Get_User_By_NormalizedEmail_And_Status_Not_Number(ctx context.Cont
 	return tx.Get_User_By_NormalizedEmail_And_Status_Not_Number(ctx, user_normalized_email)
 }
 
+func (rx *Rx) Get_User_PaidTier_By_Id(ctx context.Context,
+	user_id User_Id_Field) (
+	row *PaidTier_Row, err error) {
+	var tx *Tx
+	if tx, err = rx.getTx(ctx); err != nil {
+		return
+	}
+	return tx.Get_User_PaidTier_By_Id(ctx, user_id)
+}
+
 func (rx *Rx) Get_User_ProjectLimit_By_Id(ctx context.Context,
 	user_id User_Id_Field) (
 	row *ProjectLimit_Row, err error) {
@@ -25724,6 +25782,10 @@ type Methods interface {
 		user_normalized_email User_NormalizedEmail_Field) (
 		user *User, err error)
 
+	Get_User_PaidTier_By_Id(ctx context.Context,
+		user_id User_Id_Field) (
+		row *PaidTier_Row, err error)
+
 	Get_User_ProjectLimit_By_Id(ctx context.Context,
 		user_id User_Id_Field) (
 		row *ProjectLimit_Row, err error)

satellite/satellitedb/users.go

@@ -197,6 +197,16 @@ func (users *users) GetUserProjectLimits(ctx context.Context, id uuid.UUID) (lim
 	return limitsFromDBX(ctx, row)
 }
 
+func (users *users) GetUserPaidTier(ctx context.Context, id uuid.UUID) (isPaid bool, err error) {
+	defer mon.Task()(&ctx)(&err)
+
+	row, err := users.db.Get_User_PaidTier_By_Id(ctx, dbx.User_Id(id[:]))
+	if err != nil {
+		return false, err
+	}
+	return row.PaidTier, nil
+}
+
 // toUpdateUser creates dbx.User_Update_Fields with only non-empty fields as updatable.
 func toUpdateUser(user *console.User) (*dbx.User_Update_Fields, error) {
 	update := dbx.User_Update_Fields{