fix: add reloading and refactors

supabase · Sep 30, 2024 · 8441b42 · 8441b42
1 parent 8ac5b18
commit 8441b42
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 20 deletions.
diff --git a/internal/api/mfa.go b/internal/api/mfa.go
@@ -115,16 +115,10 @@ func (a *API) enrollPhoneFactor(w http.ResponseWriter, r *http.Request, params *
 	if err != nil {
 		return badRequestError(ErrorCodeValidationFailed, "Invalid phone number format (E.164 required)")
 	}
-	factors := user.Factors
-
-	factorCount := len(factors)
-	numVerifiedFactors := 0
 	if err := models.DeleteExpiredFactors(db, config.MFA.FactorExpiryDuration); err != nil {
 		return err
 	}
-	if err := validateFactors(user, params.FriendlyName, a.config, session); err != nil {
-		return err
-	}
+
 	var factorsToDelete []models.Factor
 	for _, factor := range user.Factors {
 		if factor.IsPhoneFactor() && factor.Phone.String() == phone {
@@ -143,17 +137,10 @@ func (a *API) enrollPhoneFactor(w http.ResponseWriter, r *http.Request, params *
 		return internalServerError("Database error deleting unverified phone factors").WithInternalError(err)
 	}
 
-	if factorCount >= int(config.MFA.MaxEnrolledFactors) {
-		return unprocessableEntityError(ErrorCodeTooManyEnrolledMFAFactors, "Maximum number of verified factors reached, unenroll to continue")
-	}
-
-	if numVerifiedFactors >= config.MFA.MaxVerifiedFactors {
-		return unprocessableEntityError(ErrorCodeTooManyEnrolledMFAFactors, "Maximum number of verified factors reached, unenroll to continue")
+	if err := validateFactors(user, params.FriendlyName, a.config, session); err != nil {
+		return err
 	}
 
-	if numVerifiedFactors > 0 && !session.IsAAL2() {
-		return forbiddenError(ErrorCodeInsufficientAAL, "AAL2 required to enroll a new factor")
-	}
 	factor := models.NewPhoneFactor(user, phone, params.FriendlyName)
 	err = db.Transaction(func(tx *storage.Connection) error {
 		if terr := tx.Create(factor); terr != nil {
@@ -198,6 +185,11 @@ func (a *API) enrollTOTPFactor(w http.ResponseWriter, r *http.Request, params *E
 	if err := models.DeleteExpiredFactors(db, config.MFA.FactorExpiryDuration); err != nil {
 		return err
 	}
+
+	if err := db.Load(user, "Factors"); err != nil {
+		return fmt.Errorf("failed to reload user factors: %w", err)
+	}
+
 	if err := validateFactors(user, params.FriendlyName, config, session); err != nil {
 		return err
 	}

diff --git a/internal/api/mfa_test.go b/internal/api/mfa_test.go
@@ -290,9 +290,7 @@ func (ts *MFATestSuite) TestDuplicateTOTPEnrollsReturnExpectedMessage() {
 	err := json.NewDecoder(response.Body).Decode(&errorResponse)
 	require.NoError(ts.T(), err)
 
-	// Convert the response body to a string and check for the expected error message
-	expectedErrorMessage := fmt.Sprintf("A factor with the friendly name %q for this user likely already exists", friendlyName)
-	require.Contains(ts.T(), errorResponse.Message, expectedErrorMessage)
+	require.Contains(ts.T(), errorResponse.ErrorCode, ErrorCodeMFAFactorNameConflict)
 }
 
 func (ts *MFATestSuite) AAL2RequiredToUpdatePasswordAfterEnrollment() {
@@ -369,7 +367,7 @@ func (ts *MFATestSuite) TestMultipleEnrollsCleanupExpiredFactors() {
 	var w *httptest.ResponseRecorder
 	token := accessTokenResp.Token
 	for i := 0; i < numFactors; i++ {
-		w = performEnrollFlow(ts, token, "", models.TOTP, "https://issuer.com", "", http.StatusOK)
+		w = performEnrollFlow(ts, token, "non-empty-name", models.TOTP, "https://issuer.com", "", http.StatusOK)
 	}
 
 	enrollResp := EnrollFactorResponse{}