fix: exempt PKCE recovery sessions from require-current-password check (#2502)

This change fixes a unit test and uses gofmt on top of pr #2497.

---------

Co-authored-by: Ollie Elmgren <ollie@listenlabs.ai>
Co-authored-by: Chris Stockton <chris.stockton@supabase.io>

Author: 3 people

internal/api/user.go

@@ -168,17 +168,8 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
 			if user.HasPassword() {
 				// current password required when updating password
 				if config.Security.UpdatePasswordRequireCurrentPassword {
-					// user may be in a password reset flow, where they do not have a currentPassword
-					isRecoverySession := false
-					for _, claim := range session.AMRClaims {
-						// password recovery flows can be via otp or a magic link, check if the current session
-						// was created with one of those
-						if claim.GetAuthenticationMethod() == "otp" || claim.GetAuthenticationMethod() == "magiclink" {
-							isRecoverySession = true
-							break
-						}
-					}
-					if !isRecoverySession {
+					// ensure user is not in a password recovery flow
+					if !session.IsRecovery() {
 						if params.CurrentPassword == nil || *params.CurrentPassword == "" {
 							return apierrors.NewBadRequestError(apierrors.ErrorCodeCurrentPasswordRequired, "Current password required when setting new password.")
 						}

internal/api/user_test.go

@@ -434,11 +434,17 @@ func (ts *UserTestSuite) TestUserUpdatePasswordViaRecovery() {
 			recoveryType: models.MagicLink,
 			expected:     expected{code: http.StatusOK, isAuthenticated: true},
 		},
+		{
+			desc:         "Current password not required in PKCE recovery flow",
+			newPassword:  "newpassword789",
+			recoveryType: models.Recovery,
+			expected:     expected{code: http.StatusOK, isAuthenticated: true},
+		},
 		{
 			desc:         "Current password required for any other claim",
 			newPassword:  "newpassword456",
 			recoveryType: models.EmailChange,
-			expected:     expected{code: http.StatusBadRequest, isAuthenticated: true},
+			expected:     expected{code: http.StatusBadRequest, isAuthenticated: false},
 		},
 	}
 

internal/models/factor.go

@@ -59,6 +59,15 @@ const (
 	PasskeyLogin
 )
 
+func (authMethod AuthenticationMethod) IsRecovery() bool {
+	switch authMethod {
+	case OTP, MagicLink, Recovery:
+		return true
+	default:
+		return false
+	}
+}
+
 func (authMethod AuthenticationMethod) String() string {
 	switch authMethod {
 	case OAuth:

internal/models/sessions.go

@@ -105,6 +105,21 @@ func (Session) TableName() string {
 	return tableName
 }
 
+func (s *Session) IsRecovery() bool {
+	for _, claim := range s.AMRClaims {
+		amStr := claim.GetAuthenticationMethod()
+		am, err := ParseAuthenticationMethod(amStr)
+		if err != nil {
+			// We want to assert this is a recovery session, skip invalid.
+			continue
+		}
+		if am.IsRecovery() {
+			return true
+		}
+	}
+	return false
+}
+
 func (s *Session) GetRefreshTokenHmacKey(dbEncryption conf.DatabaseEncryptionConfiguration) ([]byte, bool, error) {
 	if s.RefreshTokenHmacKey == nil {
 		return nil, false, nil