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

Add support for PAR #1424

Merged
merged 64 commits into from
Oct 24, 2023
Merged

Add support for PAR #1424

merged 64 commits into from
Oct 24, 2023

Conversation

josephdecock
Copy link
Member

No description provided.

@josephdecock josephdecock changed the title Joe/par Add support for PAR Sep 21, 2023
@josephdecock josephdecock added this to the 7.0.0 milestone Sep 21, 2023
@josephdecock josephdecock self-assigned this Sep 21, 2023
To conform with https://datatracker.ietf.org/doc/html/rfc9126#name-request:

> the authorization server MUST accept its ... pushed authorization
> request endpoint URL ... as an intended audience.
@josephdecock josephdecock marked this pull request as ready for review October 18, 2023 16:43
@brockallen brockallen marked this pull request as draft October 20, 2023 12:17
@brockallen
Copy link
Member

Hey Joe -- mark this as ready again once you want one final review. Thx.

We now call the IAuthorizeRequestValidator from the default PAR
validator. This simplifies the endpoint, and also allows for a
customization of the par validator to control validation of the pushed
parameters.
@@ -165,8 +165,9 @@ private static NameValueCollection ToOptimizedRawValues(this ValidatedAuthorizeR
{
// https://openid.net/specs/openid-connect-core-1_0.html#JWTRequests
// requires client id and response type to always be in URL
// REVIEW - Does this apply to pushed requests?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OIDC requires that when request objects are used, the client pass the client id and response type, in order to make the request a valid oauth request. With PAR, I don't think we need to worry about that, since PAR is an OAuth specification, and the requests we're making are valid PAR requests. In fact, an example in RFC 9126 pushes a request object containing a response_type claim without duplicating the response type outside the request object.

Since OIDC is asking for this duplication in other circumstances in order to be a valid oauth request, and the relevant oauth specification doesn't require it in this circumstance, I'm leaving it out.

OIDC requires that when request objects are used, the client pass the
client id and response type, in order to make the request a valid oauth
request. With PAR, we don't need to worry about that, since PAR is an
OAuth specification, and the requests we're making are valid PAR
requests. And in fact, an example in RFC 9126 pushes a request object
containing a response_type claim without duplicating the response type
outside the request object.

Since OIDC is asking for this duplication in other circumstances in
order to be a valid oauth request, and the relevant oauth specification
doesn't require it in this circumstance, I'm leaving it out.
@josephdecock josephdecock marked this pull request as ready for review October 20, 2023 20:28
@josephdecock
Copy link
Member Author

josephdecock commented Oct 20, 2023

@brockallen, I've created new issues to track the follow up work on otel (#1445), and to consider cleaning up the ToOptimized... methods (possibly we can deprecate them along with the IAuthorizationParametersMessageStore? (#1444)).

Also, I'm convinced now that we don't need to duplicate the response_type parameter outside the request parameter when we use PAR with JAR. I don't think the special case in ToOptimizedRawValues applies here - see my comment.

@brockallen brockallen merged commit 7b8ec53 into main Oct 24, 2023
5 checks passed
@brockallen brockallen deleted the joe/par branch October 24, 2023 03:24
@brockallen brockallen added the enhancement New feature or request label Oct 24, 2023
@leastprivilege
Copy link
Member

grats! @josephdecock

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants