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.

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

Allow for multiple redirect URIs in OAuth application UI #25068

Closed
denyskon opened this issue Jun 4, 2023 · 3 comments · Fixed by #25072
Closed

Allow for multiple redirect URIs in OAuth application UI #25068

denyskon opened this issue Jun 4, 2023 · 3 comments · Fixed by #25072
Labels
proposal/accepted We have reviewed the proposal and agree that it should be implemented like that/at all. type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@denyskon
Copy link
Member

denyskon commented Jun 4, 2023

Feature Description

It is already possible to create an OAuth application with multiple redirect URIs over the API. It would be great if it also would be possible to do this through the UI.
I propose a comma-separated list, like https://uri1.org/admin/,https://cms.uri2.com,https://uri3.net/cms/

Screenshots

No response
@denyskon denyskon added type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first. labels Jun 4, 2023
@denyskon denyskon changed the title Allow for multiple redirect URIs in OAuth application Allow for multiple redirect URIs in OAuth application UI Jun 4, 2023
@wxiaoguang
Copy link
Contributor

@denyskon
Copy link
Member Author

denyskon commented Jun 4, 2023

I think you're mixing up things. The linked approach talks about unpredictable wildcard uris, but I mean a predictable set of URIs which is already partially supported. As far as I understand the code, we already save the URIs as a string list, and over API I'm already able to set multiple URIs.
Personally I would need it because I want my CMS to be accessible over different URLs and do not want to register multiple OAuth apps for it.

The way to support it would be just treating the input value of the field as a comma-separated list and splitting it afterwards.

@wxiaoguang
Copy link
Contributor

You are right, RFC says "The authorization server MUST require public clients and SHOULD require confidential clients to register their redirection URIs.", so predictable pre-registered URLs are safe.

@yardenshoham yardenshoham added the proposal/accepted We have reviewed the proposal and agree that it should be implemented like that/at all. label Jun 4, 2023
@denyskon denyskon mentioned this issue Jun 4, 2023
Merged
lunny pushed a commit that referenced this issue Jun 5, 2023
@denyskon
Add ability to set multiple redirect URIs in OAuth application UI (#2…
ca35dec 
…5072)

OAuth applications can already have multiple redirect URIs if
created/edited over API.

This change allows for setting multiple redirect URIs through the UI as
a comma-separated list (e. g.
`https://example.org/redirect,https://redirect.example.org`)

<details>
<summary>Screenshots</summary>

![Bildschirmfoto vom 2023-06-04
17-14-40](https://github.com/go-gitea/gitea/assets/47871822/2206dc32-e7e4-4953-9ecb-e098890b3f54)
![Bildschirmfoto vom 2023-06-04
17-14-50](https://github.com/go-gitea/gitea/assets/47871822/cd97c73c-9310-44ee-a83a-b927a1ef94da)

</details>

Closes #25068
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
proposal/accepted We have reviewed the proposal and agree that it should be implemented like that/at all. type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add ability to set multiple redirect URIs in OAuth application UI denyskon/gitea
3 participants
@wxiaoguang @yardenshoham @denyskon