chore: Add PR's requested changes

Author: matiasperrone-exo

app/Http/Controllers/UserController.php

@@ -631,24 +631,6 @@ private function resolveClientFromMemento(): ?Client
         return $client;
     }
 
-    /**
-     * Resolves the OAuth2 client carried in the pending MFA state (the client the
-     * challenge was issued for), so verification scopes the OTP lookup and
-     * sibling-revoke to the same client. Returns null when the challenge was not
-     * issued in a client context.
-     *
-     * @param array $pending
-     * @return Client|null
-     */
-    private function resolveClientFromPendingState(array $pending): ?Client
-    {
-        $clientId = $pending['client_id'] ?? null;
-        if (is_null($clientId)) {
-            return null;
-        }
-        return $this->client_repository->getClientById($clientId);
-    }
-
     /**
      * Verifies a 2FA OTP challenge and, on success, establishes the session.
      *
@@ -686,7 +668,7 @@ public function verify2FA()
             }
 
             // Scope verification to the client the challenge was issued for.
-            $client = $this->resolveClientFromPendingState($pending);
+            $client = $this->resolveClientFromMemento();
 
             try {
                 // Commits the OTP redeem (+ sibling revoke) in its own tx. The

app/Strategies/MFA/AbstractMFAChallengeStrategy.php

@@ -13,7 +13,6 @@ abstract class AbstractMFAChallengeStrategy implements IMFAChallengeStrategy
     private const KEY_USER_ID           = '2fa_pending_user_id';
     private const KEY_PENDING_AT        = '2fa_pending_at';
     private const KEY_REMEMBER          = '2fa_remember';
-    private const KEY_CLIENT_ID         = '2fa_pending_client_id';
     private const KEY_RECOVERY_ATTEMPTS = '2fa_recovery_attempts';
 
     public function __construct(protected IUserRecoveryCodeRepository $recovery_code_repository) {}
@@ -36,7 +35,6 @@ public function getPendingState(): ?array
             'user_id'    => $user_id,
             'pending_at' => $pending_at,
             'remember'   => Session::get(self::KEY_REMEMBER, false),
-            'client_id'  => Session::get(self::KEY_CLIENT_ID),
         ];
     }
 
@@ -45,7 +43,6 @@ public function clearPendingState(): void
         Session::remove(self::KEY_USER_ID);
         Session::remove(self::KEY_PENDING_AT);
         Session::remove(self::KEY_REMEMBER);
-        Session::remove(self::KEY_CLIENT_ID);
         Session::remove(self::KEY_RECOVERY_ATTEMPTS);
     }
 
@@ -62,23 +59,17 @@ public function verifyRecoveryCode(User $user, string $code): void
                     throw new AuthenticationException("Invalid recovery code.");
                 }
                 $recoveryCode->markUsed();
-                $this->recovery_code_repository->add($recoveryCode, false);
                 return;
             }
         }
         throw new AuthenticationException("Invalid recovery code.");
     }
 
-    protected function storePendingState(int $userId, bool $remember, ?string $clientId = null): void
+    protected function storePendingState(int $userId, bool $remember): void
     {
         Session::put(self::KEY_USER_ID,    $userId);
         Session::put(self::KEY_PENDING_AT, time());
         Session::put(self::KEY_REMEMBER,   $remember);
-        if (is_null($clientId)) {
-            Session::remove(self::KEY_CLIENT_ID);
-        } else {
-            Session::put(self::KEY_CLIENT_ID, $clientId);
-        }
     }
 
     public function verifyChallenge(User $user, string $code, ?Client $client = null): void

app/Strategies/MFA/EmailOTPMFAChallengeStrategy.php

@@ -20,9 +20,7 @@ public function __construct(
 
     public function issueChallenge(User $user, ?Client $client, bool $remember): array
     {
-        // Carry the issuing client into the pending state so verification scopes
-        // the OTP lookup and sibling-revoke to the same client (see verifyChallenge).
-        $this->storePendingState($user->getId(), $remember, $client?->getClientId());
+        $this->storePendingState($user->getId(), $remember);
 
         $otp = $this->token_service->createOTPFromPayload([
             OAuth2Protocol::OAuth2PasswordlessConnection => OAuth2Protocol::OAuth2PasswordlessConnectionEmail,
@@ -75,14 +73,12 @@ public function verifyChallenge(User $user, string $code, ?Client $client = null
         }
 
         $otp->redeem();
-        $this->otp_repository->add($otp, false);
 
         // Revoke other pending OTPs for this user, scoped to the same client so we
         // never burn unrelated OTPs (e.g. passwordless-login codes for other clients).
         foreach ($this->otp_repository->getByUserNameNotRedeemed($user->getEmail(), $client) as $otpToRevoke) {
             if ($otpToRevoke->getValue() !== $otp->getValue()) {
                 $otpToRevoke->redeem();
-                $this->otp_repository->add($otpToRevoke, false);
             }
         }
     }

app/libs/Auth/AuthService.php

@@ -19,7 +19,6 @@
 use App\Services\Auth\IUserService as IAuthUserService;
 use Auth\Exceptions\AuthenticationException;
 use Auth\Exceptions\AuthenticationLockedUserLoginAttempt;
-use Auth\Exceptions\UnverifiedEmailMemberException;
 use Auth\Repositories\IUserRepository;
 use Exception;
 use Illuminate\Support\Facades\Auth;
@@ -393,7 +392,6 @@ public function login(string $username, string $password, bool $remember_me): bo
     {
         Log::debug("AuthService::login");
 
-        $this->last_login_error = "";
         if (!Auth::attempt(['username' => $username, 'password' => $password], $remember_me)) {
             throw new AuthenticationException
             (
@@ -799,10 +797,6 @@ public function verifyMFAChallenge(
         string $value,
         ?Client $client = null
     ): void {
-        // Commits the OTP redeem (+ sibling revoke) as a single tx. Trusted-device
-        // enrollment and audit are applied by the caller as best-effort,
-        // non-blocking side effects after this commits, so a failure in either
-        // does not block (or roll back) an already-verified second factor.
         $this->tx_service->transaction(function () use ($user, $strategy, $value, $client) {
             $strategy->verifyChallenge($user, $value, $client);
         });

tests/Unit/MFA/AbstractMFAChallengeStrategyTest.php

@@ -93,7 +93,6 @@ public function testVerifyRecoveryCode_withMatchingCode_marksAsUsed(): void
         $repo->shouldReceive('getUnusedByUser')->with($user)->andReturn([$recoveryCode]);
         // Lock is taken before mutating (regression guard for 3357348455).
         $repo->shouldReceive('refreshExclusiveLock')->with($recoveryCode)->once();
-        $repo->shouldReceive('add')->with($recoveryCode, false)->once();
 
         $strategy = new class($repo) extends AbstractMFAChallengeStrategy {
             public function issueChallenge(User $user, ?Client $client, bool $remember): array { return []; }

tests/Unit/MFA/EmailOTPMFAChallengeStrategyTest.php

@@ -132,8 +132,6 @@ public function testVerifyChallenge_withValidOtp_redeemsAndRevokesOthers_scopedT
             ->andReturn([$otherOtp]);
 
         // The redeemed code and the revoked sibling are both persisted with deferred flush.
-        $this->otpRepository->shouldReceive('add')->with($storedOtp, false)->once();
-        $this->otpRepository->shouldReceive('add')->with($otherOtp, false)->once();
 
         $this->strategy->verifyChallenge($user, $code, $client);
         $this->addToAssertionCount(1);