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

proposal/discussion: OAuth - separate requirement for redirect_uri string-match registration and handling #1965

Open
elarlang opened this issue May 19, 2024 · 6 comments
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet V51 Group issues related to OAuth _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@elarlang
Copy link
Collaborator

elarlang commented May 19, 2024

spin-off from #1916 "Discussion/Proposal 3"

Probably I was the first one to say that redirect_uri validation is a duplicate of general open-redirect but now I think it's important to have them as a separate requirement:

redirect_uri must be validated with the string-match method, which means no "wildcard" validations.

There are 2 parts:

  • Authorization Server must not accept anything else than (one of) the precise URL from the pre-registered list
  • As a precondition: the OAuth Client must not send business logic URL to the Authorization Server. It is pretty much the same as Referrer leakage.

--
Feedback from @tghosth in #1916 (comment)

Suggest you propose an updated/added requirement.

@elarlang elarlang added the V51 Group issues related to OAuth label May 19, 2024
@jmanico
Copy link
Member

jmanico commented May 20, 2024 via email

@tghosth tghosth added 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet _5.0 - prep This needs to be addressed to prepare 5.0 labels May 23, 2024
@csfreak92
Copy link
Collaborator

@elarlang, is this being addressed by my latest PR #1971? Or am I missing something about it?

@elarlang
Copy link
Collaborator Author

Hi @csfreak92 , let's find the agreement first in the issue and do PR. Discussion over PR's its complicated to follow.

We also need to think, should we have one common requirement for OAuth and OIDC or not.

I prefer requirement text with the idea like:

Verify that Authorisation Server accepts the redirect uri value from the Client that belongs to the pre-registered list of allowed values using the string-match method, e.g. wildcards are not in use.

@csfreak92
Copy link
Collaborator

Understood, but I just pushed my PR since it has been sitting in my local for a while and I thought better to have it out there than get lost somewhere as I have worked on it the past few months. :)

Can we agree over discussion and if they are already covered in the PR then that's good and then if not or needs some modifications, then I would modify them as needed?

@csfreak92
Copy link
Collaborator

Verify that Authorization Server accepts the redirect URI value from the Client that belongs to the pre-registered list of allowed values using the string-match method, e.g. wildcards are not in use.

@elarlang, I like this text though, sorry missed it. How would a pre-registered list of allowed values be handled? Should we add a text in the chapter/section how this should be done? Also, string-match method seems like regex, right?

@elarlang
Copy link
Collaborator Author

elarlang commented Jun 5, 2024

How would a pre-registered list of allowed values be handled? Should we add a text in the chapter/section how this should be done?

I don't think we need to provide guidance for OAuth Client configuration.

Also, string-match method seems like regex, right?

No, it's the opposite. string-match against pre-registered list of full values says that you should not use any regex, substring, wildcard, etc. Provided redirect_uri value must exists in the pre-registered addresses list as it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet V51 Group issues related to OAuth _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

No branches or pull requests

4 participants