Skip to content

Commit 8f00f9c

Browse files
feat: Harden login captcha gate by replacing request-body counter with server-side session counter (UserController::postLogin)
1 parent 9dda289 commit 8f00f9c

4 files changed

Lines changed: 128 additions & 66 deletions

File tree

app/Http/Controllers/UserController.php

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -396,8 +396,8 @@ public function postLogin()
396396
{
397397
$max_login_attempts_2_show_captcha = $this->server_configuration_service->getConfigValue("MaxFailed.LoginAttempts.2ShowCaptcha");
398398
$max_login_failed_attempts = intval($this->server_configuration_service->getConfigValue("MaxFailed.Login.Attempts"));
399-
$login_attempts = 0;
400-
$username = '';
399+
$login_attempts = (int) Session::get('captcha_failed_attempts', 0);
400+
$username = '';
401401
$user = null;
402402

403403
try
@@ -411,7 +411,6 @@ public function postLogin()
411411
if (isset($data['password']))
412412
$data['password'] = trim($data['password']);
413413

414-
$login_attempts = intval(Request::input('login_attempts'));
415414
// Build the validation constraint set.
416415
$rules = [
417416
'username' => 'required|email',
@@ -436,7 +435,10 @@ public function postLogin()
436435
$connection = $data['connection'] ?? null;
437436

438437
try {
438+
$user = $this->auth_service->getUserByUsername($username);
439439
if ($flow == "password" && $this->auth_service->login($username, $password, $remember)) {
440+
$user->setLoginFailedAttempt(0);
441+
Session::forget('captcha_failed_attempts');
440442
return $this->login_strategy->postLogin();
441443
}
442444

@@ -468,15 +470,17 @@ public function postLogin()
468470

469471
$otpClaim = OAuth2OTP::fromParams($username, $connection, $password);
470472
$this->auth_service->loginWithOTP($otpClaim, $client);
473+
$user->setLoginFailedAttempt(0);
474+
Session::forget('captcha_failed_attempts');
471475
return $this->login_strategy->postLogin();
472476
}
473477
} catch (AuthenticationException $ex) {
474478
// failed login attempt...
475479

476-
$user = $this->auth_service->getUserByUsername($username);
477-
if (!is_null($user)) {
478-
$login_attempts = $user->getLoginFailedAttempt();
479-
}
480+
$login_attempts = $user ? $user?->updateLoginFailedAttempt() : $login_attempts + 1;
481+
Session::put('captcha_failed_attempts', $login_attempts);
482+
483+
// User.loginFailedAttempt drives account lockout (persisted by auth_service).
480484

481485
return $this->login_strategy->errorLogin
482486
(

resources/js/login/login.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,6 @@ const PasswordInputForm = ({
185185
<input type="hidden" value={userNameValue} id="username" name="username"/>
186186
<input type="hidden" value={csrfToken} id="_token" name="_token"/>
187187
<input type="hidden" value="password" id="flow" name="flow"/>
188-
<input type="hidden" value={loginAttempts} id="login_attempts" name="login_attempts"/>
189188
{shouldShowCaptcha() && captchaPublicKey &&
190189
<Turnstile
191190
className={styles.turnstile}
@@ -271,7 +270,6 @@ const OTPInputForm = ({
271270
<input type="hidden" value="otp" id="flow" name="flow"/>
272271
<input type="hidden" value={otpCode} id="password" name="password"/>
273272
<input type="hidden" value="email" id="connection" name="connection"/>
274-
<input type="hidden" value={loginAttempts} id="login_attempts" name="login_attempts"/>
275273
{shouldShowCaptcha() && captchaPublicKey &&
276274
<Turnstile
277275
className={styles.turnstile}

resources/views/auth/login.blade.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@
6262
config.maxLoginFailedAttempts = {{Session::get("max_login_failed_attempts")}};
6363
@endif
6464
65-
@if(Session::has('login_attempts'))
66-
config.loginAttempts = {{Session::get("login_attempts")}};
65+
@if(Session::has('captcha_failed_attempts'))
66+
config.loginAttempts = {{Session::get("captcha_failed_attempts")}};
6767
@endif
6868
6969
@if(Session::has('user_is_active'))

tests/UserLoginTurnstileTest.php

Lines changed: 115 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
**/
1515

1616
use Auth\User;
17+
use Illuminate\Cookie\CookieValuePrefix;
1718
use Illuminate\Support\Facades\Session;
1819
use LaravelDoctrine\ORM\Facades\EntityManager;
1920
use RyanChandler\LaravelCloudflareTurnstile\Facades\Turnstile;
@@ -22,10 +23,10 @@
2223
* Class UserLoginTurnstileTest
2324
*
2425
* Covers Cloudflare Turnstile integration in UserController::postLogin():
25-
* - cf-turnstile-response required when login_attempts (from request body) >= threshold
26+
* - cf-turnstile-response required when captcha_failed_attempts (session) >= threshold
2627
* - threshold gating (before / at boundary / above boundary)
27-
* - omitted login_attempts field defaults to zero (no captcha required)
28-
* - captcha is gated on the request-body counter
28+
* - absent captcha_failed_attempts session key defaults to zero (no captcha required)
29+
* - captcha is gated on the server-side session counter, not request-body input
2930
* - login screen emits Turnstile JS config after a failed attempt
3031
* - expired or unsolved token is rejected
3132
*/
@@ -59,18 +60,33 @@ private function getTestUser(): User
5960
->findOneBy(['identifier' => 'sebastian.marcet']);
6061
}
6162

62-
private function postLogin(array $overrides = [])
63+
private function postLogin(array $overrides = [], array $sessionData = [])
6364
{
64-
// GET the login page first so the session (and its CSRF token) is established,
65-
// mirroring how a real browser submits the form.
66-
$this->call('GET', self::LOGIN_URL);
65+
// GET establishes the session and CSRF token, mirroring a real browser.
66+
$getResponse = $this->call('GET', self::LOGIN_URL);
67+
68+
// Inject session data after session is established, before the POST reads it.
69+
foreach ($sessionData as $key => $value) {
70+
$this->app['session']->driver()->put($key, $value);
71+
}
72+
73+
// Persist injected data to the session store so the POST kernel cycle can load it.
74+
$this->app['session']->driver()->save();
75+
76+
// Re-send the session cookie so StartSession loads the same session ID on the POST.
77+
$sessionName = $this->app['session']->getName();
78+
$sessionId = $this->app['session']->driver()->getId();
79+
$encryptedSessionCookie = encrypt(
80+
CookieValuePrefix::create($sessionName, $this->app['encrypter']->getKey()) . $sessionId,
81+
false
82+
);
6783

6884
return $this->call('POST', self::LOGIN_URL, array_merge([
6985
'username' => $this->testEmail,
7086
'password' => $this->testPassword,
7187
'flow' => 'password',
7288
'_token' => Session::token(),
73-
], $overrides));
89+
], $overrides), [$sessionName => $encryptedSessionCookie]);
7490
}
7591

7692
private function fakeTurnstilePass(): void
@@ -101,15 +117,14 @@ private function sessionHasValidationError(string $field): bool
101117

102118
public function testMissingTurnstileResponseFailsValidationWhenAtThreshold(): void
103119
{
104-
$user = $this->getTestUser();
105-
106-
$this->postLogin([
107-
"login_attempts" => self::CAPTCHA_THRESHOLD,
108-
]); // no cf-turnstile-response
120+
$this->postLogin(
121+
[], // no cf-turnstile-response
122+
['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD]
123+
);
109124

110125
$this->assertTrue(
111126
$this->sessionHasValidationError('cf-turnstile-response'),
112-
'Expected a validation error for cf-turnstile-response when user is at threshold'
127+
'Expected a validation error for cf-turnstile-response when session counter is at threshold'
113128
);
114129
}
115130

@@ -119,47 +134,43 @@ public function testMissingTurnstileResponseFailsValidationWhenAtThreshold(): vo
119134

120135
public function testLoginBelowThresholdDoesNotRequireTurnstile(): void
121136
{
122-
$user = $this->getTestUser();
123-
124-
$this->postLogin([
125-
'login_attempts' => self::CAPTCHA_THRESHOLD - 1
126-
]); // correct credentials, no captcha token
137+
$this->postLogin(
138+
[],
139+
['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD - 1]
140+
);
127141

128142
$this->assertFalse(
129143
$this->sessionHasValidationError('cf-turnstile-response'),
130-
'Turnstile must not be required when login attempts are below threshold'
144+
'Turnstile must not be required when session counter is below threshold'
131145
);
132146
}
133147

134148
public function testLoginAtThresholdWithValidTokenPassesValidation(): void
135149
{
136-
$user = $this->getTestUser();
137-
138150
$this->fakeTurnstilePass();
139151

140-
$this->postLogin([
141-
'cf-turnstile-response' => 'dummy-token-accepted-by-mock',
142-
'login_attempts' => self::CAPTCHA_THRESHOLD
143-
]);
152+
$this->postLogin(
153+
['cf-turnstile-response' => 'dummy-token-accepted-by-mock'],
154+
['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD]
155+
);
144156

145157
$this->assertFalse(
146158
$this->sessionHasValidationError('cf-turnstile-response'),
147159
'A valid Turnstile token must clear the captcha validation rule'
148160
);
149161
}
150162

151-
public function testOmittedLoginAttemptsFieldDefaultsToZeroNoCaptchaRequired(): void
163+
public function testAbsentSessionCounterDefaultsToZeroNoCaptchaRequired(): void
152164
{
153-
// No login_attempts key posted → intval(null) = 0 → below threshold →
154-
// captcha rule is never added to the validator.
165+
// No captcha_failed_attempts in session → defaults to 0 → below threshold.
155166
$this->postLogin([
156167
'username' => 'nobody@doesnotexist.example',
157168
'password' => 'irrelevant',
158169
]);
159170

160171
$this->assertFalse(
161172
$this->sessionHasValidationError('cf-turnstile-response'),
162-
'Turnstile must not be required when login_attempts is absent from the request'
173+
'Turnstile must not be required when captcha_failed_attempts is absent from session'
163174
);
164175
}
165176

@@ -169,22 +180,17 @@ public function testOmittedLoginAttemptsFieldDefaultsToZeroNoCaptchaRequired():
169180

170181
public function testLoginScreenIncludesTurnstileConfigWhenAboveThreshold(): void
171182
{
172-
$user = $this->getTestUser();
173-
174-
// Place user one below threshold; the wrong-password attempt crosses it.
175-
$this->postLogin([
176-
'password' => 'wrong-password',
177-
'login_attempts' => self::CAPTCHA_THRESHOLD - 1
178-
]);
183+
// Session counter one below threshold; the wrong-password attempt crosses it.
184+
$this->postLogin(
185+
['password' => 'wrong-password'],
186+
['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD - 1]
187+
);
179188

180-
// errorLogin() flashes max_login_attempts_2_show_captcha into the session;
189+
// errorLogin() flashes captcha_failed_attempts (updated session counter) into the session;
181190
// following the redirect renders login.blade.php which emits those values.
182191
$html = $this->call('GET', self::LOGIN_URL)->getContent();
183192

184-
// captchaPublicKey is always rendered (login.blade.php, not conditional)
185193
$this->assertStringContainsString('captchaPublicKey', $html);
186-
187-
// maxLoginAttempts2ShowCaptcha is emitted when the session key is set
188194
$this->assertStringContainsString('maxLoginAttempts2ShowCaptcha', $html);
189195
}
190196

@@ -194,15 +200,12 @@ public function testLoginScreenIncludesTurnstileConfigWhenAboveThreshold(): void
194200

195201
public function testExpiredTurnstileTokenFailsValidation(): void
196202
{
197-
$user = $this->getTestUser();
198-
199-
// Cloudflare API returns success=false (expired / already-used token)
200203
$this->fakeTurnstileFail();
201204

202-
$this->postLogin([
203-
'cf-turnstile-response' => 'expired-or-invalid-token',
204-
'login_attempts' => self::CAPTCHA_THRESHOLD
205-
]);
205+
$this->postLogin(
206+
['cf-turnstile-response' => 'expired-or-invalid-token'],
207+
['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD]
208+
);
206209

207210
$this->assertTrue(
208211
$this->sessionHasValidationError('cf-turnstile-response'),
@@ -212,17 +215,74 @@ public function testExpiredTurnstileTokenFailsValidation(): void
212215

213216
public function testUnsolvedCaptchaEmptyTokenFailsValidation(): void
214217
{
215-
$user = $this->getTestUser();
216-
217-
// Empty string triggers the 'required' rule before any Cloudflare call
218-
$this->postLogin([
219-
'cf-turnstile-response' => '',
220-
'login_attempts' => self::CAPTCHA_THRESHOLD
221-
]);
218+
$this->postLogin(
219+
['cf-turnstile-response' => ''],
220+
['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD]
221+
);
222222

223223
$this->assertTrue(
224224
$this->sessionHasValidationError('cf-turnstile-response'),
225225
'An empty Turnstile response must be rejected by the required rule'
226226
);
227227
}
228+
229+
// -------------------------------------------------------------------------
230+
// 6. Request-body login_attempts is ignored; only session counter matters
231+
// -------------------------------------------------------------------------
232+
233+
public function testRequestSuppliedLoginAttemptsIsIgnored(): void
234+
{
235+
// Session counter is at threshold but the POST body claims login_attempts=0.
236+
// The captcha gate must still fire because the server ignores the body field.
237+
$this->postLogin(
238+
['login_attempts' => 0], // attacker-supplied body value: below threshold
239+
['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD] // server session: at threshold
240+
);
241+
242+
$this->assertTrue(
243+
$this->sessionHasValidationError('cf-turnstile-response'),
244+
'cf-turnstile-response must be required based on the session counter, not the request body'
245+
);
246+
}
247+
248+
// -------------------------------------------------------------------------
249+
// 7. Enumeration safety: captcha fires for non-existent users too
250+
// -------------------------------------------------------------------------
251+
252+
public function testCaptchaRequiredForUnknownUsernameWhenSessionAtThreshold(): void
253+
{
254+
// A non-existent username must still require captcha when the session counter
255+
// is at threshold — no oracle for whether the account exists.
256+
$this->postLogin(
257+
[
258+
'username' => 'nobody@doesnotexist.example',
259+
'password' => 'irrelevant',
260+
],
261+
['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD]
262+
);
263+
264+
$this->assertTrue(
265+
$this->sessionHasValidationError('cf-turnstile-response'),
266+
'cf-turnstile-response must be required for non-existent users when session counter is at threshold'
267+
);
268+
}
269+
270+
// -------------------------------------------------------------------------
271+
// 8. Successful login resets the session counter
272+
// -------------------------------------------------------------------------
273+
274+
public function testSuccessfulLoginClearsSessionCounter(): void
275+
{
276+
$this->fakeTurnstilePass();
277+
278+
$this->postLogin(
279+
['cf-turnstile-response' => 'valid-token'],
280+
['captcha_failed_attempts' => self::CAPTCHA_THRESHOLD]
281+
);
282+
283+
$this->assertNull(
284+
$this->app['session']->driver()->get('captcha_failed_attempts'),
285+
'captcha_failed_attempts must be removed from session after a successful login'
286+
);
287+
}
228288
}

0 commit comments

Comments
 (0)