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

Allow oauth2 application redirect_uris to contain wildcards #19627

Closed
wants to merge 4 commits into from

Conversation

glmdev
Copy link

@glmdev glmdev commented May 6, 2022

Currently, Gitea matches the redirect URI for oauth2 authorize requests against a static list of valid URIs. This causes problems for applications like Gitea-based comments engine Vssue that set the redirect URI to the current page to ensure the user gets redirected to the correct post.

This change introduces a setting called ENABLE_REDIRECT_URI_WILDCARD which, when enabled, causes Gitea to check a redirect URI against the list of allowed URIs using wildcard matching.

Example: http://localhost:4000/blog/post.html is valid if the URI is http://localhost:4000/blog/*

The implementation works by transforming the pattern into a regular expression (e.g. http://localhost:4000/blog/.*) and matching the redirect URI against that expression.

This new setting is disabled by default, which preserves the existing behavior. Closes #9514.

models/auth/oauth2.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 6, 2022
@Gusted Gusted added this to the 1.17.0 milestone May 8, 2022
@Gusted Gusted added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label May 8, 2022
@lunny
Copy link
Member

lunny commented May 8, 2022

Maybe we can use github.com/gobwas/glob to match which has been in go.mod.

@glmdev
Copy link
Author

glmdev commented Jun 7, 2022

Sorry for the long delay, but I mentioned this in a reply to wxiaoguang:

Hm. I think the asterisk-only approach is preferable here since there's no way to disable some of the patterns in the glob package. For example, ? is valid in URLs, but in the pattern it would glob to any single character.

@techknowlogick techknowlogick modified the milestones: 1.17.0, 1.18.0 Jun 8, 2022
@zeripath

This comment was marked as duplicate.

@mapreri
Copy link

mapreri commented Jun 8, 2022

I would also like to see this feature merged.

In my use case, I'd like to use this feature to allow authentication to a set of dynamically-generated subdomains; what we are doing is spawning up testing instances of a company's website in URLs in the form of https://branchname.tests.company.com, and this one feature prevents me from doing it.
I suspect that I could accomplish something similar by updating the oauth2 provider settings via Gitea's API every time I create and delete a subdomain, but this solution of using a wildcard just sounds better to me.

@zeripath
Copy link
Contributor

zeripath commented Jun 8, 2022

Hmm... I'm looking at the OAuth2 RFC and specification and this behaviour is explicit disallowed.

https://datatracker.ietf.org/doc/html/rfc6749#section-10.6

I'm kinda concerned about this. Why is Vssue not redirecting back to a standard endpoint, using a cookie or some other mechanism to then redirect the user to the comments page? OAuth2 does not allow people to change the redirect url like this.

https://www.oauth.com/oauth2-servers/redirect-uris/redirect-uri-registration/

@glmdev glmdev closed this Jul 1, 2022
@lunny lunny removed this from the 1.18.0 milestone Dec 20, 2022
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Flexible redirect_uri of OAuth2
9 participants