feat(oauth): return returnTo from the callback and restrict its scheme - #63
Merged
Conversation
StartOAuthLoginRequestSchema has taken a returnTo since OAuth landed and nothing ever gave it back. seamless-auth-api validated it against the configured origins and signed it into the state, and seamless-auth-react sent it, but no schema carried it to the end of the flow, so a client that asked to be returned somewhere had no way to learn where. It was a field that looked supported and did nothing. OAuthLoginSuccessResponseSchema now carries an optional returnTo, read back out of the signed state rather than from the callback request, so it is the value validated at /start and not one introduced at the end of the round trip. Both returnTo fields move from z.url() to RedirectTargetSchema. z.url() accepts javascript:alert(1) and data:text/html,..., and a client navigates to whatever comes back out of this flow, so it is the same sink a magic link destination is. That is what RedirectTargetSchema was added for in 0.18.0.
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.
Closes the last finding from the second
seamless-auth-apireview, which could not be fixedin that repo because both OAuth schemas live here and are
z.core.$strip: adding the fieldthere would have been silently discarded by response validation.
returnTo was a field that looked supported and did nothing
StartOAuthLoginRequestSchemahas taken areturnTosince OAuth landed.seamless-auth-apivalidates it against the configured origins and signs it into the state, and
seamless-auth-reactsends it. No schema carried it to the end of the flow, so a client thatasked to be returned somewhere had no way to learn where, and the API's validation of it
changed nothing observable.
OAuthLoginSuccessResponseSchemanow carries an optionalreturnTo. The API reads it backout of the signed state rather than from the callback request, so it is the value accepted at
/startand not one introduced at the end of the round trip. It is absent when the callerasked for nothing, so a client falls through to its own default rather than treating absence
as an error.
Both returnTo fields move to RedirectTargetSchema
z.url()acceptsjavascript:alert(1)anddata:text/html,.... A client navigates towhatever comes back out of this flow, so
returnTois the same sink a magic link destinationis, and that is exactly what
RedirectTargetSchemawas added for in 0.18.0. The OAuth fieldsshould have used it then.
Worth noting the API is not currently exposed by this:
allowedReturnToparses the value andcompares its origin against the configured ones, and a
javascript:URL has no matchingorigin, so it was already dropped. This moves the refusal to the schema, where it is stated
once rather than depending on a downstream origin check to fall the right way.
Compatibility
Additive for any consumer that ignores the new field. The one behaviour change is that a
caller sending a
javascript:ordata:returnTois now refused at the schema instead ofbeing silently dropped later, which is the point.
Minor rather than major, per the repo's pre-1.0 convention.
Follow-up in the API
seamless-auth-apipopulates the field once this is published. That change is small(
finishOAuthLoginalready holdsstatePayload), but it cannot land until the dependency isbumped, so it follows separately rather than being written against an unpublished schema.
Checks
npm run typecheck,npm run lint,npm run format:checkclean. 233 tests pass, up from228: this adds the first test file for the OAuth schemas, covering the round trip, the absent
case, and both refused schemes.