test: assert the passwordless rejection is indistinguishable, not just unworded

testInvalidOtpAgainstEnforcedUserDoesNotLeakMFAStatus only asserted that the
flash message lacked the phrase "two-factor authentication". Enumeration is
about distinguishability, so that assertion still passed if a future
enforced-only branch leaked a differently worded message.

It now drives the same invalid code through the same endpoint for the
MFA-enforced admin and for a non-enforced control user, and asserts the two
rejections are byte-identical. The stale flash is cleared between requests so
the comparison cannot read a value against itself. Verified by mutation:
restoring the pre-fix guard order with reworded text fails the new assertSame
and would have passed the old substring assertion.

Also records why testEnforcedUserCannotBypassMFAViaPasswordlessLogin adds no
separate "no login side effect" assertions. Both candidates were tried and
removed as change detectors that cannot fail: the DB-visible effects (OTP
redemption, sibling revocation) are rolled back by the AuthService transaction
regardless of where the guard sits, and the session-visible one (Auth::login()
and the Login event queuing PostLoginUser) is already caught first by the
existing Auth::check() assertion. Both confirmed by mutation.

Full file green: OK (58 tests, 380 assertions).

Author: smarcet

tests/TwoFactorLoginFlowTest.php

@@ -193,6 +193,15 @@ public function testNonAdminWithoutMFALogsInNormally(): void
 
     public function testEnforcedUserCannotBypassMFAViaPasswordlessLogin(): void
     {
+        // No separate "no login side effect" assertions here on purpose: the
+        // DB-visible ones (OTP redemption, sibling revocation) are unreachable
+        // by construction, because the guard throws inside the AuthService
+        // transaction and DoctrineTransactionService rolls the whole closure
+        // back - moving the guard below finalizeRedemption() leaves the OTP
+        // un-redeemed all the same. The session-visible one (Auth::login() and
+        // the Login event that queues PostLoginUser) is already covered by the
+        // Auth::check() assertion below, which is what fails first if the guard
+        // is moved below Auth::login(). Both were verified by mutation.
         $this->emitOTP(self::ADMIN_EMAIL);
         $code = $this->latestOtpCode(self::ADMIN_EMAIL);
 
@@ -222,15 +231,43 @@ public function testInvalidOtpAgainstEnforcedUserDoesNotLeakMFAStatus(): void
         // the inbox - an account-enumeration oracle. The check must now only be
         // reachable after loginWithOTPEnforcing2FA() has proven the code valid,
         // so an invalid code gets the same generic rejection for any account.
-        $this->emitOTP(self::ADMIN_EMAIL);
+        //
+        // Enumeration is about DISTINGUISHABILITY, not about one phrase: merely
+        // asserting the absence of "two-factor authentication" would still pass
+        // if some future enforced-only branch leaked a *differently* worded
+        // message. So the enforced account's rejection is compared byte-for-byte
+        // against a non-enforced control driven through the same endpoint with
+        // the same bad code.
+        $control_email = $this->createPlainUser();
 
-        $response = $this->postLoginOTP(self::ADMIN_EMAIL, 'not-the-real-code');
+        $this->emitOTP(self::ADMIN_EMAIL);
+        $this->postLoginOTP(self::ADMIN_EMAIL, 'not-the-real-code');
 
         $this->assertFalse(Auth::check(), 'an invalid code must never authenticate');
         $this->assertResponseStatus(302);
+        $enforced_notice = Session::get('flash_notice');
+        $this->assertNotNull($enforced_notice, 'the enforced account must get a flashed rejection');
+
+        // Cleared so the control's assertion cannot silently read the enforced
+        // account's leftover flash and compare a value against itself.
+        Session::forget('flash_notice');
+
+        $this->emitOTP($control_email);
+        $this->postLoginOTP($control_email, 'not-the-real-code');
+
+        $this->assertFalse(Auth::check(), 'an invalid code must never authenticate the control account either');
+        $this->assertResponseStatus(302);
+        $control_notice = Session::get('flash_notice');
+        $this->assertNotNull($control_notice, 'the control account must get a flashed rejection');
+
+        $this->assertSame(
+            $control_notice,
+            $enforced_notice,
+            'an invalid code must produce an identical rejection for an enforced and a non-enforced account - any difference is an enumeration oracle'
+        );
         $this->assertStringNotContainsString(
             'two-factor authentication',
-            Session::get('flash_notice'),
+            $enforced_notice,
             'an invalid code must not leak that the account is MFA-enforced'
         );
     }