fix(auth): make invite token consumption atomic#392
Open
DeryFerd wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR makes invite token usage atomic and concurrency-safe during invite acceptance.
Before this change, invite consumption was implemented as a check-then-act sequence:
max_usesuse_countThat flow is vulnerable to race conditions under concurrent requests, especially for
max_uses = 1invites. Two requests could both pass validation before either incrementsuse_count, causing oversubscription.This PR changes the flow so invite usage is consumed under a transaction and guarded update, ensuring one-time invites stay one-time.
Problem
The previous implementation in
createUserFromInvite()had a classic TOCTOU pattern:invite.use_countwas validated based on a snapshotThis means invite links with strict limits (
max_uses: 1) were not strongly enforced under contention.What changed
1) Atomic invite consumption in auth flow
File:
backend/auth/auth-service.tscreateUserFromInvite()now:BEGIN IMMEDIATEThis ensures the invite usage decision and user creation happen in one serialized operation.
2) Guarded usage increment query
File:
backend/database/queries/auth-queries.tsincrementUseCount()was changed from:UPDATE ... use_count = use_count + 1 WHERE id = ?)to:
max_uses = 0unlimited ORuse_count < max_uses)expires_at IS NULL OR expires_at >= now)booleanto indicate whether consume succeeded (changes > 0)This converts usage update from a passive side effect into an explicit success/failure gate used by auth flow.
3) Regression test added
File:
backend/database/queries/auth-invite-token.test.tsNew test verifies invite usage cannot exceed cap:
max_uses = 1incrementUseCount()twiceuse_countremains1This test was written as a failing check first, then made green with the guarded update change.
Why this approach
Why
BEGIN IMMEDIATE?Using an immediate transaction acquires a write lock early in SQLite, preventing concurrent writers from interleaving check/consume/create steps. This is a practical and robust choice for serialized invite consumption in this architecture.
Why keep both app-level checks and SQL guards?
Why return boolean from
incrementUseCount()?The caller must know whether usage was actually consumed. Returning
booleanallows auth flow to fail fast and map failures to meaningful invite errors.Behavioral impact
Before
After
Validation
Targeted tests
bun test backend/database/queries/auth-invite-token.test.tsbun test backend/ws/auth/login.test.tsLint/type checks
bun run lintbun run checkstill reports pre-existing unresolved@myrialabs/ptykit/*module/type issues in terminal-related files (not introduced by this PR).Risk and compatibility
(Touches auth invite flow and query contract for one method)
Notes
No changes were made to unrelated auth policies (roles, PAT/session format, invite payload schema).
Scope is intentionally focused on making invite consumption safe and deterministic.