rosa/vernier
1
0
Fork 0
Code Issues 8 Pull requests 1 Projects Releases Packages 1 Wiki Activity Actions

fix: Consume authorization codes atomically, after fallible loads #77

Merged
rosa merged 2 commits from fix/atomic-code-redemption into main 2026-07-03 21:39:14 +00:00
Conversation 0 Commits 2 Files changed 5 +221 -18
rosa commented 2026-07-03 21:14:46 +00:00
Owner
Copy link

Fixes #49
Fixes #61

Problem

redeem_authorization_code was a non-atomic find, an in-memory binding check, and a separate unconditional DELETE whose affected-row count the Postgres adapter ignored. Two concurrent redemptions of the same code could both pass the checks before either DELETE landed, each minting an access token — bypassing ADR-0008's single-use guarantee (#49). The DELETE also preceded the user load, so a transient repository error (or missing user) after the delete permanently burned a valid code with nothing granted (#61).

Changes

  • The repository port replaces delete_authorization_code with consume_authorization_code, an atomic claim returning whether this caller removed the code; exactly one of any number of concurrent callers observes true.
  • The Postgres adapter keeps the same DELETE and reads rows_affected(); the in-memory mock uses DashMap::remove(..).is_some(), mirroring the row count.
  • redeem_authorization_code reorders so the fallible user load happens before the claim — a failure there leaves the code intact and redeemable — and maps a lost claim to AuthorizationError::InvalidGrant. Binding checks (client, redirect URI, PKCE, expiry, verifier) and their error behavior are unchanged.

Tests

  • New conformance suite crates/infra/tests/authorization_code_repository_conformance.rs runs against both adapters: the first consume claims the code and removes the row, a second consume (and a consume of an unknown code) reports already-consumed.
  • transient_user_lookup_failure_leaves_the_code_redeemable (service test): an injected transient find_user_by_id failure errors the redemption without consuming the code, and a retry succeeds. MemoryAppRepo gained a fail_next_user_lookup knob for this.
  • authorization_code_is_single_use now asserts the replay is specifically InvalidGrant.

The ci mise task passes.

Fixes #49 Fixes #61 ## Problem `redeem_authorization_code` was a non-atomic find, an in-memory binding check, and a separate unconditional DELETE whose affected-row count the Postgres adapter ignored. Two concurrent redemptions of the same code could both pass the checks before either DELETE landed, each minting an access token — bypassing ADR-0008's single-use guarantee (#49). The DELETE also preceded the user load, so a transient repository error (or missing user) after the delete permanently burned a valid code with nothing granted (#61). ## Changes - The repository port replaces `delete_authorization_code` with `consume_authorization_code`, an atomic claim returning whether *this* caller removed the code; exactly one of any number of concurrent callers observes `true`. - The Postgres adapter keeps the same `DELETE` and reads `rows_affected()`; the in-memory mock uses `DashMap::remove(..).is_some()`, mirroring the row count. - `redeem_authorization_code` reorders so the fallible user load happens *before* the claim — a failure there leaves the code intact and redeemable — and maps a lost claim to `AuthorizationError::InvalidGrant`. Binding checks (client, redirect URI, PKCE, expiry, verifier) and their error behavior are unchanged. ## Tests - New conformance suite `crates/infra/tests/authorization_code_repository_conformance.rs` runs against both adapters: the first consume claims the code and removes the row, a second consume (and a consume of an unknown code) reports already-consumed. - `transient_user_lookup_failure_leaves_the_code_redeemable` (service test): an injected transient `find_user_by_id` failure errors the redemption without consuming the code, and a retry succeeds. `MemoryAppRepo` gained a `fail_next_user_lookup` knob for this. - `authorization_code_is_single_use` now asserts the replay is specifically `InvalidGrant`. The `ci` mise task passes.
fix: Consume authorization codes atomically, after fallible loads
Some checks failed
ci/woodpecker/pr/test Pipeline is pending
Details
ci/woodpecker/push/test Pipeline was canceled
Details
325a70d9bf 
Redemption was a find, an in-memory check, and an unconditional DELETE
whose affected-row count was ignored, so two concurrent redemptions of
the same code could both pass the checks and each mint an access token,
violating ADR-0008's single-use guarantee. The DELETE also preceded the
user load, so a transient failure there permanently burned a valid code
with nothing granted.

Replace delete_authorization_code on the repository port with
consume_authorization_code, an atomic claim that reports whether this
caller removed the code; exactly one concurrent caller wins, and the
service maps a lost claim to InvalidGrant. Reorder
redeem_authorization_code so the fallible user load happens before the
claim, leaving the code intact and redeemable when the load fails. The
in-memory mock implements the same consume semantics, and a repository
conformance suite pins them for both adapters.

Fixes #49
Fixes #61
Merge remote-tracking branch 'origin/main' into fix/atomic-code-redemption
Some checks failed
ci/woodpecker/push/test Pipeline was canceled
Details
ci/woodpecker/pr/test Pipeline was canceled
Details
016f13a178 
# Conflicts:
#	crates/domain/src/services.rs
rosa merged commit 74f45e085f into main 2026-07-03 21:39:14 +00:00
rosa deleted branch fix/atomic-code-redemption 2026-07-03 21:39:15 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug
Something is broken

  
enhancement
New feature or improvement

  
needs-info
Waiting on reporter for more information

  
needs-triage
Maintainer needs to evaluate this issue

  
ready-for-agent
Fully specified, waiting for an AFK agent

  
ready-for-human
Requires human implementation

  
wontfix
Will not be actioned

No labels
bug
enhancement
needs-info
needs-triage
ready-for-agent
ready-for-human
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
rosa/vernier!77
No description provided.