Make sign-in rate limiting atomic across concurrent requests #61

New Issue

2026-11-05T12:09:27Z

spencerm commented

2026-11-05 12:09:27 +00:00

Priority: high.

server/packetstation/auth/signin.py:202 checks recorded failures before LDAP verification; server/packetstation/auth/ratelimit.py:62 reads completed attempts without reserving an in-flight attempt. Concurrent requests can all pass the same limit check.

Reproduce in a disposable environment with a barrier-controlled fake directory: seed failures just below the configured limit, start concurrent wrong-password requests for the same canonical account, and hold their verifications until each has passed the gate. More LDAP verifications are admitted than the configured budget. With an allowed account limit of 15 and sufficient concurrency, this can undermine the intended margin below AD's threshold of 20. Do not test by locking a real AD account.

Acceptance criteria:

Atomically reserve attempts or serialize admission by canonical account across workers/processes.
Handle login aliases and IP limits consistently; release/expire reservations safely on failure.
Keep LDAP work outside long-lived database transactions where practical.
Add deterministic concurrent tests proving the admitted attempt count stays within budget.

Validation: code-path review only; no live AD concurrency reproduction. Source: parent review.

Reviewed revision: 18333600b55b70971877c1a0c72d1d61b6ce9c69. Raised from the November 2026 independent project reviews and parent verification. No production system was tested.

Priority: high. [server/packetstation/auth/signin.py:202](https://gt.pescoinc.biz/spencerm/PacketStation/src/commit/18333600b55b70971877c1a0c72d1d61b6ce9c69/server/packetstation/auth/signin.py#L202) checks recorded failures before LDAP verification; [server/packetstation/auth/ratelimit.py:62](https://gt.pescoinc.biz/spencerm/PacketStation/src/commit/18333600b55b70971877c1a0c72d1d61b6ce9c69/server/packetstation/auth/ratelimit.py#L62) reads completed attempts without reserving an in-flight attempt. Concurrent requests can all pass the same limit check. Reproduce in a disposable environment with a barrier-controlled fake directory: seed failures just below the configured limit, start concurrent wrong-password requests for the same canonical account, and hold their verifications until each has passed the gate. More LDAP verifications are admitted than the configured budget. With an allowed account limit of 15 and sufficient concurrency, this can undermine the intended margin below AD's threshold of 20. Do not test by locking a real AD account. Acceptance criteria: - Atomically reserve attempts or serialize admission by canonical account across workers/processes. - Handle login aliases and IP limits consistently; release/expire reservations safely on failure. - Keep LDAP work outside long-lived database transactions where practical. - Add deterministic concurrent tests proving the admitted attempt count stays within budget. Validation: code-path review only; no live AD concurrency reproduction. Source: parent review. Reviewed revision: `18333600b55b70971877c1a0c72d1d61b6ce9c69`. Raised from the November 2026 independent project reviews and parent verification. No production system was tested.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: spencerm/PacketStation#61