fix(auth): support Magic Auth sign-up for emails without a user - #52
Open
grayashh wants to merge 8 commits into
Open
fix(auth): support Magic Auth sign-up for emails without a user#52grayashh wants to merge 8 commits into
grayashh wants to merge 8 commits into
Conversation
Greptile SummaryThe PR adds Magic Auth sign-up for previously unknown emails and verifies those users when they redeem their code.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (6): Last reviewed commit: "fix(auth): finish resolving every email ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Routing account creation through a lookup that was only ever a read exposes what the read could afford to get wrong. An exact-match miss on 'User@x.test' vs 'user@x.test' used to be a harmless 404; once a miss creates a user, the same miss forks the account in two. A bare presence check on the email was likewise fine for a read and turns a typo into a permanent ghost account once it can write. Verifying the email up in the grant also meant persisting a change, and firing user.updated, before the JWT template gate — the one thing that comment is there to prevent, since a failed render is supposed to leave no trace of a login that never completed. Folding it into the sign-in write fixes the ordering and drops the second webhook per login.
Creation resolves the user case-insensitively but redemption matched the
email exactly, so the two halves disagreed about what the same address
means. A code requested for 'user@x.test' against a stored 'User@X.test' is
recorded under the stored casing, and authenticating with the address the
caller actually used then failed invalid_one_time_code — a 201 handing back
a code the request it answered could never spend. Recoverable only because
the 201 body carries the canonical email, which no client should have to
read to finish a flow it already had the address for.
The existing case-insensitivity test never redeemed, which is why it passed.
It does now, and fails without this change.
Also applies the typo guard to POST /user_management/users. That route
creates users too, so the same miss becomes the same unreachable account,
and holding one standard is what stops `{email: 'nope'}` being a 422 on one
path and a 201 on the other. It keeps the route's existing 422 convention
rather than magic auth's 400.
And documents both behavior changes: the endpoint no longer 404s an unknown
email, and redeeming a code verifies the email of any previously-unverified
user, not only ones it just created — which a suite pointed at this can be
surprised by in either direction.
Clarified behavior of Magic Auth and email handling.
Magic Auth stores the case it was handed, so once it creates users the lookups that stayed exact-match strand the accounts it makes. A sign-up as 'Signup@y.test' could not be reached by the password grant, by login_hint, or by password reset using the address the caller actually had — password reset 404d on an account that existed. All three resolve the way magic auth does now. POST /user_management/users deduped exactly, so 'User@x.test' and 'user@x.test' were both created and the two creation paths then disagreed about which account an address names, with magic auth's resolver settling it by insertion order. It answers 409 instead, which is what holding one standard across the two creation paths actually requires. A malformed email reported "email is required" for an address that was supplied — the opposite of what happened, on the guard that exists to keep a typo from becoming an account nothing can reach. Also pins the sign-up's user.created event. A webhook consumer testing a sign-up flow is much of why this endpoint creating a user is worth having.
Resolving an account case-insensitively means lowercasing the address, and
every path that does it type-asserted `email` first. That assertion was
survivable while a lookup by email could only miss — findOneBy returns
undefined for a number as readily as for an unknown address — but toLowerCase
throws, so the same request that used to come back a named 4xx now comes back
Internal Server Error: 401 invalid_credentials on the password grant, 404 on
password reset, and 400 invalid_code on the magic-auth grant all became 500
server_error. A server_error in a consumer's suite reads as a defect in the
emulator rather than a malformed request, which is the one thing an emulator
must never get wrong about its own inputs.
One guard for all of them, since the shape of the value is the same question
everywhere it is asked. Absence is handed back for the caller to report its
own way, so `{}` still says "email is required" while `{email: 123}` says what
it actually is — the distinction the magic auth handler already argued for and
then lost by routing a non-string into its presence check, where it reported
an address that was supplied as one that was missing.
The guard trims, which also closes a smaller gap in the same seam: creation
stored the trimmed address while the grants and password reset resolved the
raw one, so a padded copy of an address could not reach the account written
under it.
Six lookups by email still matched exactly, so the invariant the README
claimed held only where Magic Auth had been touched. Each one strands the
accounts Magic Auth now creates, because sign-up stores whatever case it was
handed:
GET /user_management/users?email= is what an SDK's listUsers({email}) maps to
— the way a caller finds the account a sign-up just made. Filtering exactly,
`Signup@X.test` was returned by its own casing and by nothing else. The test
added for sign-up creation leans on this filter and passed only because its
address is lowercase throughout.
Accepting an invitation resolved the recipient exactly, so for an account
stored under another case it enrolled nobody: 200, invitation.accepted
emitted, invitation spent, no membership, no error. The grant path already
compares the two addresses case-insensitively, so the two halves of one flow
disagreed about which person an address names.
SSO authentication events reported user_id: null for an account that existed.
The invitations email filter matched exactly. And a seeded membership joined
its user exactly, rejecting at startup a reference the running emulator would
have honoured.
Seeded `users` are now unique case-insensitively too, matching the 409 this
branch gave POST /user_management/users: a seed was otherwise the one door
left open to the pair of accounts no lookup by email can tell apart, which is
the state all of this exists to prevent.
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
2 participants
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.
POST /user_management/magic_authnow creates the user when the email has none. Production creates it at code-creation time, not at authenticate: the live API's 201 already carries auser_id, the user is immediately listable withemail_verified: false, and the email it sends uses the "Sign up" template. The emulator's 404 made AuthKit sign-up flows untestable.email_verifiedtotrue, matching the live authenticate response for the same flow; the emulator previously left sign-up users unverified forever.org_idclaim, pinned by a new test.Closes #50