Skip to content

clarified that redirect_uri scheme must not be used with rfc9101 #403

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

clarified that redirect_uri scheme must not be used with rfc9101 #403

wants to merge 1 commit into from

Conversation

Sakurann
Copy link
Collaborator

resolves #392

@Sakurann Sakurann mentioned this pull request Jan 30, 2025
Closed
jogu
jogu requested changes Jan 31, 2025
View reviewed changes
Copy link
Collaborator

@jogu jogu left a comment

Choose a reason for hiding this comment

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

The justification on the issue for this change is:

rfc9101 does not allow alg = none

However JAR does allow alg none, it's only disallowed if requred_signed_request_object is set to true in server metadata: https://www.rfc-editor.org/rfc/rfc9101.html#require_signed_request_object

@Sakurann
Copy link
Collaborator Author

Sakurann commented Jan 31, 2025 •
edited
Loading

@jogu
so you are suggesting when using JAR with client id scheme redirect_uri, alg = none must be used?
isn't prohibiting using JAR with client id scheme redirect_uri safer and more in spirit of sentiment around alg=none ?
and I do worry that allowing JAR with alg=nonce with client_id_scheme redirect_uri would encourage implementers to unnecessarily use it.

@jogu
Copy link
Collaborator

jogu commented Jan 31, 2025

My thinking didn't go as far as a suggestion, it was more at the "if we're making a breaking change to the spec we should make sure we have a clear correct reason for doing so" level.

I hate alg=none as much as everyone else. The conformance suite does support alg:none here (it calls it "request_uri_unsigned" in the dropdown choice) and I can see over the past few months a few people have used it with redirect_uri client scheme, though whether they did so 100% intentionally I'm not sure, at least a few of the tests did pass so people support it.

I guess if we do want to discourage alg=none, we should do that consistently across all client id schemes?

awoie
awoie requested changes Feb 5, 2025
View reviewed changes
Copy link
Contributor

@awoie awoie left a comment

Choose a reason for hiding this comment

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

I agree with @jogu. I believe this is what some people are using and we never disallowed that.

@Sakurann
Copy link
Collaborator Author

so do you want me to change this to "please use alg=none when you want to use redirect_uri with signed_request"?

@jogu
Copy link
Collaborator

jogu commented Feb 19, 2025

I'm fine with some clarification along those lines

@Sakurann Sakurann added this to the Final 1.0 milestone Mar 4, 2025
@jogu
Copy link
Collaborator

jogu commented Mar 5, 2025

closing this as we seem to have consensus not to go in this direction - let's discuss on issue.

@jogu jogu closed this Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jogu jogu jogu requested changes

@awoie awoie awoie requested changes

@c2bo c2bo Awaiting requested review from c2bo

@tplooker tplooker Awaiting requested review from tplooker

@paulbastian paulbastian Awaiting requested review from paulbastian

@bc-pi bc-pi Awaiting requested review from bc-pi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Final 1.0
Development

Successfully merging this pull request may close these issues.

Conflicting requirement on request object signature
3 participants
@Sakurann @jogu @awoie