diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 3d7c1213..e0366963 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -396,8 +396,8 @@ public function postLogin() { $max_login_attempts_2_show_captcha = $this->server_configuration_service->getConfigValue("MaxFailed.LoginAttempts.2ShowCaptcha"); $max_login_failed_attempts = intval($this->server_configuration_service->getConfigValue("MaxFailed.Login.Attempts")); - $login_attempts = 0; - $username = ''; + $login_attempts = (int) Session::get('captcha_failed_attempts', 0); + $username = ''; $user = null; try @@ -411,7 +411,6 @@ public function postLogin() if (isset($data['password'])) $data['password'] = trim($data['password']); - $login_attempts = intval(Request::input('login_attempts')); // Build the validation constraint set. $rules = [ 'username' => 'required|email', @@ -436,7 +435,10 @@ public function postLogin() $connection = $data['connection'] ?? null; try { + $user = $this->auth_service->getUserByUsername($username); if ($flow == "password" && $this->auth_service->login($username, $password, $remember)) { + Session::forget('captcha_failed_attempts'); + Session::save(); return $this->login_strategy->postLogin(); } @@ -468,15 +470,18 @@ public function postLogin() $otpClaim = OAuth2OTP::fromParams($username, $connection, $password); $this->auth_service->loginWithOTP($otpClaim, $client); + Session::forget('captcha_failed_attempts'); + Session::save(); return $this->login_strategy->postLogin(); } } catch (AuthenticationException $ex) { // failed login attempt... - $user = $this->auth_service->getUserByUsername($username); - if (!is_null($user)) { - $login_attempts = $user->getLoginFailedAttempt(); - } + $login_attempts = $login_attempts + 1; + Session::put('captcha_failed_attempts', $login_attempts); + Session::save(); + + // User.loginFailedAttempt drives account lockout (persisted by auth_service). return $this->login_strategy->errorLogin ( @@ -525,6 +530,9 @@ public function postLogin() Log::warning($ex1); $user = $this->auth_service->getUserByUsername($username); + $login_attempts = $login_attempts + 1; + Session::put('captcha_failed_attempts', $login_attempts); + Session::save(); $response_data = [ 'max_login_attempts_2_show_captcha' => $max_login_attempts_2_show_captcha, diff --git a/resources/js/login/login.js b/resources/js/login/login.js index ee061b9a..96050dea 100644 --- a/resources/js/login/login.js +++ b/resources/js/login/login.js @@ -185,7 +185,6 @@ const PasswordInputForm = ({ - {shouldShowCaptcha() && captchaPublicKey && - {shouldShowCaptcha() && captchaPublicKey && 'verification'), function () { Route::post('resend', ['middleware' => ['csrf'], 'uses' => 'UserController@resendVerificationEmail']); }); - Route::post('', ['middleware' => 'csrf', 'uses' => 'UserController@postLogin']); + Route::post('', ['middleware' => 'csrf', 'uses' => 'UserController@postLogin'])->block(); Route::get('cancel', "UserController@cancelLogin"); Route::group(array('prefix' => '{provider}'), function () { Route::get('', 'SocialLoginController@redirect')->name("social_login"); diff --git a/tests/UserLoginTurnstileTest.php b/tests/UserLoginTurnstileTest.php index f13b3db8..8819b2e4 100644 --- a/tests/UserLoginTurnstileTest.php +++ b/tests/UserLoginTurnstileTest.php @@ -12,20 +12,18 @@ * See the License for the specific language governing permissions and * limitations under the License. **/ - -use Auth\User; +use Illuminate\Cookie\CookieValuePrefix; use Illuminate\Support\Facades\Session; -use LaravelDoctrine\ORM\Facades\EntityManager; use RyanChandler\LaravelCloudflareTurnstile\Facades\Turnstile; /** * Class UserLoginTurnstileTest * * Covers Cloudflare Turnstile integration in UserController::postLogin(): - * - cf-turnstile-response required when login_attempts (from request body) >= threshold + * - cf-turnstile-response required when captcha_failed_attempts (session) >= threshold * - threshold gating (before / at boundary / above boundary) - * - omitted login_attempts field defaults to zero (no captcha required) - * - captcha is gated on the request-body counter + * - absent captcha_failed_attempts session key defaults to zero (no captcha required) + * - captcha is gated on the server-side session counter, not request-body input * - login screen emits Turnstile JS config after a failed attempt * - expired or unsolved token is rejected */ @@ -53,24 +51,26 @@ protected function prepareForTests(): void // Helpers // ------------------------------------------------------------------------- - private function getTestUser(): User - { - return EntityManager::getRepository(User::class) - ->findOneBy(['identifier' => 'sebastian.marcet']); - } - - private function postLogin(array $overrides = []) + private function postLogin(array $overrides = [], array $sessionData = []) { - // GET the login page first so the session (and its CSRF token) is established, - // mirroring how a real browser submits the form. + // GET establishes the session and CSRF token, mirroring a real browser. $this->call('GET', self::LOGIN_URL); + // Inject session data after session is established, before the POST reads it. + foreach ($sessionData as $key => $value) { + $this->app['session']->driver()->put($key, $value); + } + + // Persist injected data to the session store so the POST kernel cycle can load it. + $this->app['session']->driver()->save(); + + // Re-send the session cookie so StartSession loads the same session ID on the POST. return $this->call('POST', self::LOGIN_URL, array_merge([ 'username' => $this->testEmail, 'password' => $this->testPassword, 'flow' => 'password', '_token' => Session::token(), - ], $overrides)); + ], $overrides), $this->makeEncryptedSessionCookie()); } private function fakeTurnstilePass(): void @@ -95,21 +95,31 @@ private function sessionHasValidationError(string $field): bool return $errors !== null && $errors->has($field); } + private function makeEncryptedSessionCookie(): array + { + $sessionName = $this->app['session']->getName(); + $sessionId = $this->app['session']->driver()->getId(); + $cookie = encrypt( + CookieValuePrefix::create($sessionName, $this->app['encrypter']->getKey()) . $sessionId, + false + ); + return [$sessionName => $cookie]; + } + // ------------------------------------------------------------------------- // 1. Validation failure when cf-turnstile-response is missing // ------------------------------------------------------------------------- public function testMissingTurnstileResponseFailsValidationWhenAtThreshold(): void { - $user = $this->getTestUser(); - - $this->postLogin([ - "login_attempts" => self::CAPTCHA_THRESHOLD, - ]); // no cf-turnstile-response + $this->postLogin( + [], // no cf-turnstile-response + ['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD] + ); $this->assertTrue( $this->sessionHasValidationError('cf-turnstile-response'), - 'Expected a validation error for cf-turnstile-response when user is at threshold' + 'Expected a validation error for cf-turnstile-response when session counter is at threshold' ); } @@ -119,28 +129,25 @@ public function testMissingTurnstileResponseFailsValidationWhenAtThreshold(): vo public function testLoginBelowThresholdDoesNotRequireTurnstile(): void { - $user = $this->getTestUser(); - - $this->postLogin([ - 'login_attempts' => self::CAPTCHA_THRESHOLD - 1 - ]); // correct credentials, no captcha token + $this->postLogin( + [], + ['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD - 1] + ); $this->assertFalse( $this->sessionHasValidationError('cf-turnstile-response'), - 'Turnstile must not be required when login attempts are below threshold' + 'Turnstile must not be required when session counter is below threshold' ); } public function testLoginAtThresholdWithValidTokenPassesValidation(): void { - $user = $this->getTestUser(); - $this->fakeTurnstilePass(); - $this->postLogin([ - 'cf-turnstile-response' => 'dummy-token-accepted-by-mock', - 'login_attempts' => self::CAPTCHA_THRESHOLD - ]); + $this->postLogin( + ['cf-turnstile-response' => 'dummy-token-accepted-by-mock'], + ['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD] + ); $this->assertFalse( $this->sessionHasValidationError('cf-turnstile-response'), @@ -148,10 +155,9 @@ public function testLoginAtThresholdWithValidTokenPassesValidation(): void ); } - public function testOmittedLoginAttemptsFieldDefaultsToZeroNoCaptchaRequired(): void + public function testAbsentSessionCounterDefaultsToZeroNoCaptchaRequired(): void { - // No login_attempts key posted → intval(null) = 0 → below threshold → - // captcha rule is never added to the validator. + // No captcha_failed_attempts in session → defaults to 0 → below threshold. $this->postLogin([ 'username' => 'nobody@doesnotexist.example', 'password' => 'irrelevant', @@ -159,7 +165,7 @@ public function testOmittedLoginAttemptsFieldDefaultsToZeroNoCaptchaRequired(): $this->assertFalse( $this->sessionHasValidationError('cf-turnstile-response'), - 'Turnstile must not be required when login_attempts is absent from the request' + 'Turnstile must not be required when captcha_failed_attempts is absent from session' ); } @@ -169,23 +175,61 @@ public function testOmittedLoginAttemptsFieldDefaultsToZeroNoCaptchaRequired(): public function testLoginScreenIncludesTurnstileConfigWhenAboveThreshold(): void { - $user = $this->getTestUser(); + // Part 1: blade renders counter one below threshold. + // Establish session, inject, save, then GET with explicit cookie (same pattern as + // testLoginScreenEmitsLoginAttemptsFromSessionKey). + $this->call('GET', self::LOGIN_URL); + $this->app['session']->driver()->put('captcha_failed_attempts', self::CAPTCHA_THRESHOLD - 1); + $this->app['session']->driver()->save(); + $html = $this->call('GET', self::LOGIN_URL, [], $this->makeEncryptedSessionCookie())->getContent(); + $this->assertStringContainsString( + 'config.loginAttempts = ' . (self::CAPTCHA_THRESHOLD - 1), + $html, + 'login.blade.php must emit config.loginAttempts (THRESHOLD-1) from the captcha_failed_attempts session key' + ); - // Place user one below threshold; the wrong-password attempt crosses it. - $this->postLogin([ - 'password' => 'wrong-password', - 'login_attempts' => self::CAPTCHA_THRESHOLD - 1 - ]); + // Part 2: a wrong-password attempt with counter at THRESHOLD-1 must push it to THRESHOLD. + // Use an unknown username so the server uses session counter+1 (not the DB counter). + $this->postLogin( + ['username' => 'nobody@doesnotexist.example', 'password' => 'wrong-password'], + ['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD - 1] + ); - // errorLogin() flashes max_login_attempts_2_show_captcha into the session; - // following the redirect renders login.blade.php which emits those values. - $html = $this->call('GET', self::LOGIN_URL)->getContent(); + // After postLogin(), the session driver holds the session the POST wrote to. + // Use its ID so the GET loads the updated captcha_failed_attempts = CAPTCHA_THRESHOLD. + $html = $this->call('GET', self::LOGIN_URL, [], $this->makeEncryptedSessionCookie())->getContent(); - // captchaPublicKey is always rendered (login.blade.php, not conditional) $this->assertStringContainsString('captchaPublicKey', $html); - - // maxLoginAttempts2ShowCaptcha is emitted when the session key is set $this->assertStringContainsString('maxLoginAttempts2ShowCaptcha', $html); + $this->assertStringContainsString( + 'config.loginAttempts = ' . self::CAPTCHA_THRESHOLD, + $html, + 'login.blade.php must emit config.loginAttempts (THRESHOLD) from the captcha_failed_attempts session key' + ); + } + + public function testLoginScreenEmitsLoginAttemptsFromSessionKey(): void + { + // GET establishes the session. + $this->call('GET', self::LOGIN_URL); + + // Inject a known value so we can assert the blade reads exactly this key. + $expectedAttempts = self::CAPTCHA_THRESHOLD + 1; + $this->app['session']->driver()->put('captcha_failed_attempts', $expectedAttempts); + $this->app['session']->driver()->save(); + + // Re-send the session cookie so the next GET kernel cycle loads the same session. + + $html = $this->call('GET', self::LOGIN_URL, [], $this->makeEncryptedSessionCookie())->getContent(); + + // The blade wraps this in `@if(Session::has('captcha_failed_attempts'))` and emits: + // config.loginAttempts = ; + // If the blade reads a different key, this assertion fails. + $this->assertStringContainsString( + 'config.loginAttempts = ' . $expectedAttempts, + $html, + 'login.blade.php must emit config.loginAttempts from captcha_failed_attempts, not any other session key' + ); } // ------------------------------------------------------------------------- @@ -194,15 +238,12 @@ public function testLoginScreenIncludesTurnstileConfigWhenAboveThreshold(): void public function testExpiredTurnstileTokenFailsValidation(): void { - $user = $this->getTestUser(); - - // Cloudflare API returns success=false (expired / already-used token) $this->fakeTurnstileFail(); - $this->postLogin([ - 'cf-turnstile-response' => 'expired-or-invalid-token', - 'login_attempts' => self::CAPTCHA_THRESHOLD - ]); + $this->postLogin( + ['cf-turnstile-response' => 'expired-or-invalid-token'], + ['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD] + ); $this->assertTrue( $this->sessionHasValidationError('cf-turnstile-response'), @@ -212,17 +253,134 @@ public function testExpiredTurnstileTokenFailsValidation(): void public function testUnsolvedCaptchaEmptyTokenFailsValidation(): void { - $user = $this->getTestUser(); - - // Empty string triggers the 'required' rule before any Cloudflare call - $this->postLogin([ - 'cf-turnstile-response' => '', - 'login_attempts' => self::CAPTCHA_THRESHOLD - ]); + $this->postLogin( + ['cf-turnstile-response' => ''], + ['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD] + ); $this->assertTrue( $this->sessionHasValidationError('cf-turnstile-response'), 'An empty Turnstile response must be rejected by the required rule' ); } -} \ No newline at end of file + + // ------------------------------------------------------------------------- + // 6. Request-body login_attempts is ignored; only session counter matters + // ------------------------------------------------------------------------- + + public function testRequestSuppliedLoginAttemptsIsIgnored(): void + { + // Session counter is at threshold but the POST body claims login_attempts=0. + // The captcha gate must still fire because the server ignores the body field. + $this->postLogin( + ['login_attempts' => 0], // attacker-supplied body value: below threshold + ['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD] // server session: at threshold + ); + + $this->assertTrue( + $this->sessionHasValidationError('cf-turnstile-response'), + 'cf-turnstile-response must be required based on the session counter, not the request body' + ); + } + + // ------------------------------------------------------------------------- + // 7. Enumeration safety: captcha fires for non-existent users too + // ------------------------------------------------------------------------- + + public function testCaptchaRequiredForUnknownUsernameWhenSessionAtThreshold(): void + { + // A non-existent username must still require captcha when the session counter + // is at threshold — no oracle for whether the account exists. + $this->postLogin( + [ + 'username' => 'nobody@doesnotexist.example', + 'password' => 'irrelevant', + ], + ['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD] + ); + + $this->assertTrue( + $this->sessionHasValidationError('cf-turnstile-response'), + 'cf-turnstile-response must be required for non-existent users when session counter is at threshold' + ); + } + + public function testRepeatedUnknownUserFailuresIncrementSessionCounterToThreshold(): void + { + // GET establishes the session and CSRF token. + $this->call('GET', self::LOGIN_URL); + + // Make CAPTCHA_THRESHOLD failed attempts with a non-existent username, + // replaying the same session cookie so the server accumulates the counter. + for ($i = 0; $i < self::CAPTCHA_THRESHOLD; $i++) { + $this->call('POST', self::LOGIN_URL, [ + 'username' => 'nobody@doesnotexist.example', + 'password' => 'irrelevant', + 'flow' => 'password', + '_token' => Session::token(), + ], $this->makeEncryptedSessionCookie()); + } + + // One more attempt without a captcha token — the session counter must now + // be at threshold, so cf-turnstile-response is required even for a + // non-existent user (no request-body shortcut available to the attacker). + $this->call('POST', self::LOGIN_URL, [ + 'username' => 'nobody@doesnotexist.example', + 'password' => 'irrelevant', + 'flow' => 'password', + '_token' => Session::token(), + ], $this->makeEncryptedSessionCookie()); + + $this->assertTrue( + $this->sessionHasValidationError('cf-turnstile-response'), + 'After ' . self::CAPTCHA_THRESHOLD . ' failed attempts with an unknown username, cf-turnstile-response must be required' + ); + } + + public function testRepeatedKnownUserFailuresIncrementSessionCounterByOnePerAttempt(): void + { + // GET establishes the session and CSRF token. + $this->call('GET', self::LOGIN_URL); + + // Make CAPTCHA_THRESHOLD failed attempts with a real account and a wrong password, + // replaying the same session cookie so the server accumulates the counter. + // LockUserCounterMeasure already increments the DB counter via CustomAuthProvider; + // the controller must NOT double-increment — each failure must add exactly 1 to the session. + for ($i = 0; $i < self::CAPTCHA_THRESHOLD; $i++) { + $this->call('POST', self::LOGIN_URL, [ + 'username' => $this->testEmail, + 'password' => 'definitely-wrong-password', + 'flow' => 'password', + '_token' => Session::token(), + ], $this->makeEncryptedSessionCookie()); + } + + $sessionCounter = $this->app['session']->driver()->get('captcha_failed_attempts'); + $this->assertEquals( + self::CAPTCHA_THRESHOLD, + $sessionCounter, + 'Each failed attempt must increment the session counter by exactly 1 (not 2). ' . + 'A double-increment here would re-introduce an enumeration oracle: known users would ' . + 'hit the captcha threshold faster than unknown users.' + ); + } + + // ------------------------------------------------------------------------- + // 8. Successful login resets the session counter + // ------------------------------------------------------------------------- + + public function testSuccessfulLoginClearsSessionCounter(): void + { + $this->fakeTurnstilePass(); + + $this->postLogin( + ['cf-turnstile-response' => 'valid-token'], + ['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD] + ); + + $this->assertNull( + $this->app['session']->driver()->get('captcha_failed_attempts'), + 'captcha_failed_attempts must be removed from session after a successful login' + ); + } +}