feat(oauth): read the returnTo a sign-in asked for and land on it - #146
Merged
Conversation
startOAuthLogin has taken a returnTo since OAuth landed here and nothing ever read one back. The auth server validated it against the configured origins and signed it into the state, but finishOAuthLogin was typed as a bare MessageResult, so the whole callback body was discarded and an adopter had no way to learn where the flow had been asked to end up. finishOAuthLogin now resolves to FinishOAuthLoginResult, the completed response minus its session material, for the same reason LoginStartResult drops it: sessions are carried by cookies, so there is no reason to hand an adopter raw tokens. The bundled OAuthCallback view lands there instead of always going to /. This is the gap the magic link redirect closed in 0.10.0: the headless client could reach the feature and an application using AuthRoutes could not. The view only follows a destination on its own origin. That is not the guard against an open redirect, which the auth server applied before signing the state. It is that these views route with react-router, which cannot leave the application. An adopter that wants to reads returnTo off the result and navigates itself.
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.
Completes the
returnTochain.seamless-auth-types0.20.0 added the response field andseamless-auth-apipopulates it; this reads it.The whole callback body was being discarded
startOAuthLoginhas taken areturnTosince OAuth landed here, and nothing ever read oneback. The auth server validated it against the configured origins and signed it into the
state, but
finishOAuthLoginwas typed as a bareMessageResult, so an adopter had no wayto learn where the flow had been asked to end up.
It now resolves to
FinishOAuthLoginResult, the completed OAuth response minus its sessionmaterial, for the same reason
LoginStartResultdrops it: sessions are carried by cookies,so there is no reason to hand an adopter raw tokens. The new field is
returnTo, absent whenthe caller asked for nothing.
The bundled view lands there
OAuthCallbacknavigated to/unconditionally. It now goes where the flow asked.This is the same gap the magic link redirect closed in 0.10.0, and for the same audience: the
headless client could reach the feature and an application using
AuthRoutescould not,which is the audience least likely to be wiring up its own client.
The view only follows a destination on its own origin. That is not the guard against an
open redirect, which the auth server already applied before signing the state. It is
narrower: these views route with react-router, which cannot leave the application, so another
origin is not somewhere this component can send anyone. An adopter that wants to reads
returnTooff the result and navigates itself.Worth noting the origin check does double duty. A
javascript:URL parses but resolves to anull origin, so it is refused here as well, on top of
RedirectTargetSchemarefusing it atthe API and
allowedReturnTorefusing it by origin comparison. Three independent layers now,none of them relied upon alone.
Dependency
@seamless-auth/typesmoves from^0.16.0to^0.20.0. Nothing else in the four releasestouched anything this package consumes: typecheck, lint and the full suite were clean on the
bump alone before any of this change was written.
Checks
npm run typecheck,npm run lint,npm run format:checkclean. 324 tests pass across 32suites, up from 321.
OAuthCallbackcoverage goes from 92.85 to 95.23 statements, with thenew cases covering the round trip, a cross-origin destination, an unparseable one and a
javascript:one.Not included
Nothing here navigates cross-origin on an adopter's behalf, and no view reads
returnTootherthan
OAuthCallback. The magic link views still land on/; that flow has its own redirectmechanism and is untouched.