fix(cloud): reset lockout on window expiry + constant-time unknown-user path (anti-enumeration)
This commit is contained in:
parent
450c88978f
commit
902bbe93c8
2 changed files with 28 additions and 1 deletions
|
|
@ -8,6 +8,9 @@ use App\Repository\{UserRepository, AuditRepository};
|
||||||
|
|
||||||
final class AuthController
|
final class AuthController
|
||||||
{
|
{
|
||||||
|
// A valid Argon2id hash used only to equalize verification timing for unknown users (anti-enumeration).
|
||||||
|
private const DUMMY_HASH = '$argon2id$v=19$m=65536,t=4,p=1$S25iZnFxVGpGQWRnVFpYYg$8Lorit52NTt4MN/gXR/9xkr9hzFHZvpGWTzJvjqXnCk';
|
||||||
|
|
||||||
public function __construct(
|
public function __construct(
|
||||||
private UserRepository $users,
|
private UserRepository $users,
|
||||||
private AuditRepository $audit,
|
private AuditRepository $audit,
|
||||||
|
|
@ -20,12 +23,23 @@ final class AuthController
|
||||||
$username = (string)$req->post('username');
|
$username = (string)$req->post('username');
|
||||||
$password = (string)$req->post('password');
|
$password = (string)$req->post('password');
|
||||||
$u = $this->users->byUsername($username);
|
$u = $this->users->byUsername($username);
|
||||||
if ($u === null) return Response::json(['error' => 'invalid_credentials'], 401);
|
if ($u === null) {
|
||||||
|
Password::verify($password, self::DUMMY_HASH); // constant-time-ish: burn equivalent work
|
||||||
|
return Response::json(['error' => 'invalid_credentials'], 401);
|
||||||
|
}
|
||||||
|
|
||||||
if ((int)($u['locked_until'] ?? 0) > $now) {
|
if ((int)($u['locked_until'] ?? 0) > $now) {
|
||||||
$this->audit->append($username, 'login_locked', 'auth', [], []);
|
$this->audit->append($username, 'login_locked', 'auth', [], []);
|
||||||
return Response::json(['error' => 'locked'], 423);
|
return Response::json(['error' => 'locked'], 423);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// lock window expired -> reset stale failure counter so the user gets a fresh set of attempts
|
||||||
|
if ((int)($u['locked_until'] ?? 0) !== 0 && (int)$u['locked_until'] <= $now) {
|
||||||
|
$this->users->clearFailures((int)$u['id']);
|
||||||
|
$u['failed_attempts'] = 0;
|
||||||
|
$u['locked_until'] = 0;
|
||||||
|
}
|
||||||
|
|
||||||
if (!Password::verify($password, (string)$u['password_hash'])) {
|
if (!Password::verify($password, (string)$u['password_hash'])) {
|
||||||
$attempts = (int)($u['failed_attempts'] ?? 0) + 1;
|
$attempts = (int)($u['failed_attempts'] ?? 0) + 1;
|
||||||
$lock = $attempts >= $this->maxAttempts ? $now + $this->lockSeconds : 0;
|
$lock = $attempts >= $this->maxAttempts ? $now + $this->lockSeconds : 0;
|
||||||
|
|
|
||||||
|
|
@ -37,4 +37,17 @@ final class AuthControllerTest extends TestCase
|
||||||
// now locked, even correct password is 423
|
// now locked, even correct password is 423
|
||||||
$this->assertSame(423, $c->login($this->req('alice', 'pw'), new ArraySessionStore(), now: 1000)->status());
|
$this->assertSame(423, $c->login($this->req('alice', 'pw'), new ArraySessionStore(), now: 1000)->status());
|
||||||
}
|
}
|
||||||
|
public function testExpiredLockResetsAttempts(): void {
|
||||||
|
$repo = $this->users();
|
||||||
|
$c = new AuthController($repo, new InMemoryAuditRepository(), maxAttempts: 3, lockSeconds: 900);
|
||||||
|
// lock the account with 3 bad tries at now=1000
|
||||||
|
for ($i = 0; $i < 3; $i++) $c->login($this->req('alice', 'bad'), new ArraySessionStore(), now: 1000);
|
||||||
|
// locked_until is now 1000 + 900 = 1900, verify it's locked
|
||||||
|
$this->assertSame(423, $c->login($this->req('alice', 'pw'), new ArraySessionStore(), now: 1000)->status());
|
||||||
|
// now at now=2000 (after lock window expires at 1900), login with correct password should succeed
|
||||||
|
$sess = new ArraySessionStore();
|
||||||
|
$res = $c->login($this->req('alice', 'pw'), $sess, now: 2000);
|
||||||
|
$this->assertSame(200, $res->status());
|
||||||
|
$this->assertSame(1, $sess->get('user_id'));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue