fix(auth): close three account-security holes (0.20.5) - #45
Merged
Merged
Conversation
… challenge `findTwoFaSecretsByUserId` returned every session row holding a secret, revoked or not, so a TOTP secret from a device the user had deliberately revoked — an old phone, a sold one, "log out everywhere" after a compromise — still passed the device-mode 2FA challenge. Present in every 0.x. Both adapters now filter `revokedAt`. The filter alone would have been a lockout. Every ordinary sign-in retires the session that device already held, and nothing carried the secret across, so a re-logged-in user would have had no live secret at all. `carryDeviceTwoFaSecret` now hands it to the replacement in the password, OAuth and magic-link paths. It moves rather than copies, because `Session.twoFaSecret` is unique: a copy throws, and a helper that swallows its own errors turns that throw into a silent lockout. The move is atomic through a new optional `moveTwoFaSecret` adapter method — optional, so an adapter written against 0.20.4 still satisfies the interface — with a clear-then-set fallback that fails recoverably. A failed carry now logs instead of vanishing. The unit fake enforces the unique index itself. A mock that cannot fail the way production fails is how the first version of this passed every unit test. Also repairs push approval after a re-login for a consumer that already filtered revoked rows, which found no live secret until the vault re-minted one.
Two holes in `oAuthLogin`'s attach-by-email step, one class: an email that nobody proved decided which existing account a Google or Apple sign-in landed in. Both present since the initial integration. 1. The client could name the email. The Apple verifier fell back to `extra.email` — a value the client sends — whenever the signed token had no email claim, so anyone with a valid Apple token for their own Apple ID could name a victim's address and be signed into the victim's passwordless account. Google's `email_verified: false` was trusted too. Only a provider-vouched email counts now: Apple's token email, and Google's only when `email_verified === true`. A missing email is not an error, so a user already linked by `sub` keeps signing in; `OAuthResult.email` becomes optional. 2. The account's own email did not have to be proven. A consumer can let a user store an unclaimed address unverified, so an attacker could create a passwordless account under a victim's address and wait for the victim's first genuine Google sign-in — a real token, nothing forged — to land in an account the attacker still held a passkey for. Attaching now requires the existing account's email to be VERIFIED. An unverified match is refused with a way forward — link the provider from Settings, bound to the signed-in session — and an adapter that omits the field refuses every attach. The password-account refusal is unchanged and still wins. A verified passwordless account still attaches, so nobody who proved their address is locked out. The tests run the real `oAuthLogin` through in-memory adapters that return full rows and can produce the dangerous inputs; the new ones fail against the old code.
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
1 participant
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
Three account-security holes in
@factiii/auth, all present in every 0.x:moveTwoFaSecretadapter method), so closing the hole locks nobody out.email_verified === true. A linked user whose token omits the email still signs in bysub.Release
Patch.
changeset statusplans@factiii/auth -> 0.20.5only.Public type changes:
OAuthResult.emailbecomes optional;DeviceAuthAdaptergains an optionalmoveTwoFaSecret.Testing
packages/auth: 15 files, 214 tests pass;check-typesclean. The new tests fail against the old code.🤖 Generated with Claude Code
https://claude.ai/code/session_01EFfQ2y8w3U1jRLZrmwZUwR