Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redirect URI is ambiguous in claims gathering flow #144

Closed
jricher opened this issue Apr 9, 2015 · 6 comments
Closed

Redirect URI is ambiguous in claims gathering flow #144

jricher opened this issue Apr 9, 2015 · 6 comments
Labels
blocking core Related to (original UMA1) core spec scope; may use obsolete language V1.0.1
Milestone

Comments

@jricher
Copy link

jricher commented Apr 9, 2015

§3.4.1.2.2 details using a user redirection as a means of claims gathering. It uses the redirect_uri parameter to direct the user back to the client, and it says that this value "must match" something, but it doesn't define clearly enough what it must match. If this is intended to be the redirect_uri defined in the OAuth client model, then this conflates processing responses from the authorization endpoint and responses from the remote RQP claims gathering endpoint. This is not likely the desired behavior, and the spec should specify a separate field from the client model. The best way to do this would be to extend the model as defined in the OAuth Dynamic Client Registration document, perhaps claims_redirect_uri.

The current text:

redirect_uri
    OPTIONAL. The URI to which the client wishes the authorization server to direct the requesting party's user agent after completing its interaction. The URI MUST be absolute, MAY contain an "application/x-www-form-urlencoded" formatted query parameter component that MUST be retained when adding addition parameters, and MUST NOT contain a fragment component. The authorization server SHOULD require all clients to register their redirection endpoint prior to utilizing the authorization endpoint. If the URI is pre-registered, this URI MUST exactly match one of the pre-registered redirection URIs, with the matching performed as described in Section 6.2.1 of [RFC3986] (Simple String Comparison).
@xmlgrrl xmlgrrl added V1.0 errata core Related to (original UMA1) core spec scope; may use obsolete language labels Apr 10, 2015
@jricher
Copy link
Author

jricher commented Jul 15, 2015

As a note, this is not really 1.0 errata as it would require extension of the OAuth client model in a potentially incompatible fashion.

@xmlgrrl xmlgrrl added this to the V1.0.1 milestone Jul 31, 2015
@xmlgrrl xmlgrrl added the V1.0.1 label Aug 14, 2015
mmachulak added a commit that referenced this issue Aug 20, 2015
xmlgrrl added a commit that referenced this issue Aug 23, 2015
This completes the incomplete claims-gathering flow design.

(It also seems to take out the note related to #161/#162 in Sec 1.3.1
that we are just re-discussing now. Intentional?)
mmachulak added a commit that referenced this issue Aug 27, 2015
* issue_144:
  Suggestion for #144

Conflicts:
	draft-uma-core-v1_0_1.xml
@xmlgrrl
Copy link

xmlgrrl commented Aug 28, 2015

Proposed, discussed, and reviewed in UMA telecon 2015-08-27 (though the WG definitely needs to review closely over the next week). We can close this, presuming Maciej has already implemented amendments made on the call.

@xmlgrrl xmlgrrl closed this as completed Aug 28, 2015
@jricher
Copy link
Author

jricher commented Sep 5, 2015

In §3.6.2.2, the reference to claims_redirect_uri is still ambiguous. Proposed new text:

claims_redirect_uri
    OPTIONAL. The URI to which the client wishes the authorization server to direct the requesting 
party's user agent after completing its interaction. The URI MUST be absolute, MAY contain an 
"application/x-www-form-urlencoded" formatted query parameter component that MUST be retained 
when adding addition parameters, and MUST NOT contain a fragment component. The authorization 
server SHOULD require all clients to register their claims redirection endpoints prior to utilizing the 
claims endpoint (either using a static process or through [RFC7591] or [OIDCDynClientReg]). 
The claims redirection URIs of a client MUST be registered distinct from the client's redirection
URIs as used at the authorization endpoint during redirect-based OAuth flows. If the URI is 
pre-registered, this URI MUST exactly match one of the pre-registered claims redirection URIs, with the 
matching performed as described in Section 6.2.1 of [RFC3986] (Simple String Comparison).

Also I would say it MUST (not SHOULD) require registration otherwise it's an open redirector.

@jricher
Copy link
Author

jricher commented Sep 5, 2015

Same notes for later text in the section talking about "redirection URI", they should all be "claims redirection URI" to be disambiguated from the normal redirection URI.

@xmlgrrl xmlgrrl reopened this Sep 5, 2015
@jricher
Copy link
Author

jricher commented Sep 6, 2015

Additional new text needed in §7.1, with particular mention that claims_redirect_uris is distinct from redirect_uris.

xmlgrrl added a commit that referenced this issue Sep 8, 2015
Added @jricher ’s suggested wording to the claims redirect passages,
and added “claims” to “redirection URI” everywhere. Didn’t change
“SHOULD require all clients to register” into a MUST yet because OAuth
has SHOULD; need to discuss.
@xmlgrrl
Copy link

xmlgrrl commented Sep 8, 2015

In UMA ad hoc telecon 2015-09-08 (which had quorum), this was the discussion and conclusion:

"The changes would, we feel, convince a quick reader used to OAuth spec reading that this redirection URI is something distinct!"

We'll keep the change and close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking core Related to (original UMA1) core spec scope; may use obsolete language V1.0.1
Projects
None yet
Development

No branches or pull requests

2 participants