Skip to content

task(auth): add per-uid rate limiting to passkey auth failures#20553

Draft
dschom wants to merge 1 commit into
mainfrom
worktree-FXA-13675
Draft

task(auth): add per-uid rate limiting to passkey auth failures#20553
dschom wants to merge 1 commit into
mainfrom
worktree-FXA-13675

Conversation

@dschom
Copy link
Copy Markdown
Contributor

@dschom dschom commented May 8, 2026

Because

  • We want to rate limit failures since this is a more clear signal of repeated attempts.

This pull request

  • Updates passkey AppErrors to take a UID, so we can rate limit on failure.
  • In the event of failure, we now look up the account and run a rate limit check.
  • Adds configuration that limits repeated passkey failures on an account.

Issue that this pull request solves

Closes: FXA-13675

Checklist

Put an `x` in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

How to Test

  • Edit rate-limit-rules.txt to set a tight `passkeyAuthFinishFailed` rule (e.g. 1 attempt per 10 minutes by uid) for fast verification
  • Sign in to a test account that has a registered passkey.
  • Sign out and trigger `POST /passkey/authentication/finish` repeatedly with a tampered/invalid assertion against the same account.
  • Observer the rate limiting failure.

@dschom dschom force-pushed the worktree-FXA-13675 branch 2 times, most recently from 5d30e41 to 16d2bcd Compare May 8, 2026 22:32
@dschom dschom marked this pull request as ready for review May 8, 2026 22:39
@dschom dschom requested a review from a team as a code owner May 8, 2026 22:39
@dschom dschom force-pushed the worktree-FXA-13675 branch 2 times, most recently from 9b2c4ed to efd76ba Compare May 9, 2026 00:02
@vpomerleau vpomerleau requested review from Copilot and vpomerleau May 11, 2026 19:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds per-UID rate limiting for passkey authentication finish failures by propagating uid context through passkey verification errors and enforcing a new customs action when failures occur. This helps throttle repeated failed assertions against a specific account, reducing signal-noise compared to IP-only limits.

Changes:

  • Extend AppError passkey failure helpers to optionally carry an internal-only uid field (not serialized in responses).
  • Attach uid to passkey verification failures once the credential owner is known, and consume it in the auth-server route to run a per-uid customs check on failure.
  • Add new passkeyAuthFinishFailed rate-limit rules plus unit tests covering the new behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
packages/fxa-auth-server/lib/routes/passkeys.ts Runs a per-uid customs rate-limit check when authenticationFinish fails with an AppError that includes uid.
packages/fxa-auth-server/lib/routes/passkeys.spec.ts Adds route tests ensuring customs per-uid failure recording and rate-limit error propagation.
packages/fxa-auth-server/config/rate-limit-rules.txt Introduces passkeyAuthFinishFailed rules (uid + ip).
libs/accounts/passkey/src/lib/passkey.service.ts Attaches uid to thrown passkey auth AppErrors after credential resolution.
libs/accounts/passkey/src/lib/passkey.service.spec.ts Adds tests verifying uid is/ isn’t attached depending on failure stage.
libs/accounts/errors/src/index.spec.ts Adds assertions that uid is attached when provided and not leaked to payload.
libs/accounts/errors/src/app-error.ts Adds uid?: string to AppError and updates passkey error factories to accept optional uid.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +411 to +417
const failedAccount = await this.db.account(err.uid);
await this.customs.checkAuthenticated(
request,
err.uid,
failedAccount.email,
'passkeyAuthFinishFailed'
);
Comment on lines +166 to +168
passkeyRegisterStart : ip_uid : 10 : 10 minutes : 10 minutes : block
passkeyRegisterFinish : ip_uid : 10 : 10 minutes : 10 minutes : block
passkeyAuthFinishFailed : uid : 50 : 24 hours : 15 minutes : block
Comment on lines +21 to +22
loadAccountFailed : ip : 50 : 24 hours : 15 minutes : ban
passkeyAuthFinishFailed : ip : 50 : 24 hours : 15 minutes : ban
'passkeyAuthFinishFailed'
);
});

@dschom dschom marked this pull request as draft May 12, 2026 16:24
@dschom dschom force-pushed the worktree-FXA-13675 branch from efd76ba to ebea56d Compare May 12, 2026 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants