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

Enhancing Gitea OAuth2 Provider with Granular Scopes for Resource Access #32573

Merged
merged 12 commits into from
Nov 22, 2024

Conversation

marcellmars
Copy link
Contributor

@marcellmars marcellmars commented Nov 20, 2024

Resolve #31609

This PR was initiated following my personal research to find the lightest possible Single Sign-On solution for self-hosted setups. The existing solutions often seemed too enterprise-oriented, involving many moving parts and services, demanding significant resources while promising planetary-scale capabilities. Others were adequate in supporting basic OAuth2 flows but lacked proper user management features, such as a change password UI.

Gitea hits the sweet spot for me, provided it supports more granular access permissions for resources under users who accept the OAuth2 application.

This PR aims to introduce granularity in handling user resources as nonintrusively and simply as possible. It allows third parties to inform users about their intent to not ask for the full access and instead request a specific, reduced scope. If the provided scopes are only the typical ones for OIDC/OAuth2—openid, profile, email, and groups—everything remains unchanged (currently full access to user's resources). Additionally, this PR supports processing scopes already introduced with personal tokens (e.g. read:user, write:issue, read:group, write:repository...)

Personal tokens define scopes around specific resources: user info, repositories, issues, packages, organizations, notifications, miscellaneous, admin, and activitypub, with access delineated by read and/or write permissions.

The initial case I wanted to address was to have Gitea act as an OAuth2 Identity Provider. To achieve that, with this PR, I would only add openid public-only to provide access token to the third party to authenticate the Gitea's user but no further access to the API and users resources.

Another example: if a third party wanted to interact solely with Issues, it would need to add read:user (for authorization) and read:issue/write:issue to manage Issues.

My approach is based on my understanding of how scopes can be utilized, supported by examples like Sample Use Cases: Scopes and Claims on auth0.com.

I renamed CheckOAuthAccessToken to GetOAuthAccessTokenScopeAndUserID so now it returns AccessTokenScope and user's ID. In the case of additional scopes in userIDFromToken the default all would be reduced to whatever was asked via those scopes. The main difference is the opportunity to reduce the permissions from all, as is currently the case, to what is provided by the additional scopes described above.

Screenshots:

Screenshot_20241121_121405

Screenshot_20241121_120211

Screenshot_20241121_120119

Screenshot_20241121_120018

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 20, 2024
@pull-request-size pull-request-size bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Nov 20, 2024
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files labels Nov 20, 2024
services/auth/oauth2.go Outdated Show resolved Hide resolved
@wxiaoguang
Copy link
Contributor

By the way, no need to rebase or force push after reviewing starts (it makes the review more difficult). https://github.com/go-gitea/gitea/blob/main/CONTRIBUTING.md#maintaining-open-prs

Just push new commits, and the final merge will squash.

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@marcellmars
Copy link
Contributor Author

By the way, no need to rebase or force push after reviewing starts (it makes the review more difficult). https://github.com/go-gitea/gitea/blob/main/CONTRIBUTING.md#maintaining-open-prs

Just push new commits, and the final merge will squash.

thank you for your patience. this is my first pull request and i still learn it.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 20, 2024
@lunny
Copy link
Member

lunny commented Nov 21, 2024

Could we have a screenshot of the new grant page?

services/auth/oauth2.go Outdated Show resolved Hide resolved
@lunny
Copy link
Member

lunny commented Nov 21, 2024

Please send a update doc PR to https://gitea.com/gitea/docs

@marcellmars
Copy link
Contributor Author

Could we have a screenshot of the new grant page?

Screenshot_20241121_121405

Screenshot_20241121_120211

Screenshot_20241121_120119

Screenshot_20241121_120018

@lunny
Copy link
Member

lunny commented Nov 21, 2024

Could we have a screenshot of the new grant page?

Screenshot_20241121_121405

Screenshot_20241121_120211

Screenshot_20241121_120119

Screenshot_20241121_120018
Igh>

Thank you. The permissions should be highlighted,maybe a bold font?

@marcellmars
Copy link
Contributor Author

Please send a update doc PR to https://gitea.com/gitea/docs

https://gitea.com/gitea/docs/pulls/102

@marcellmars
Copy link
Contributor Author

Thank you. The permissions should be highlighted,maybe a bold font?

committed.

@lunny
Copy link
Member

lunny commented Nov 21, 2024

Please don't add HTML tags in translations. You can

{{ctx.Locale.Tr "auth.authorize_application_with_scopes" (HTMLFormat "<b>%s</b>" .Scope)}}

We have plan to remove all HTML tags from the previous translations.

@marcellmars
Copy link
Contributor Author

{{ctx.Locale.Tr "auth.authorize_application_with_scopes" (HTMLFormat "<b>%s</b>" .Scope)}}

We have plan to remove all HTML tags from the previous translations.

great. i opened translations and few lines below there was some html. so i just copied the pattern ;)

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Nov 21, 2024
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 21, 2024
@lunny lunny added this to the 1.23.0 milestone Nov 21, 2024
@lunny lunny merged commit a3881ff into go-gitea:main Nov 22, 2024
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 22, 2024
zjjhot added a commit to zjjhot/gitea that referenced this pull request Nov 22, 2024
* giteaofficial/main:
  Fix get reviewers' bug (go-gitea#32415)
  Fix issues with inconsistent spacing in areas (go-gitea#32607)
  Refactor markup render system (go-gitea#32589)
  Style unification for the issue_management area (go-gitea#32605)
  Enhancing Gitea OAuth2 Provider with Granular Scopes for Resource Access (go-gitea#32573)
@wxiaoguang
Copy link
Contributor

Hi @marcellmars

I think there is a regression, my woodpecker is using old tokens, if the empty scope couldn't be resolved as "all", then woodpecker would stop working.

Would you like to take a look at my fix: Improve oauth2 scope token handling #32633 ? I also added a new test: "" => all

@marcellmars
Copy link
Contributor Author

I think there is a regression, my woodpecker is using old tokens, if the empty scope couldn't be resolved as "all", then woodpecker would stop working.

Would you like to take a look at my fix: Improve oauth2 scope token handling #32633 ? I also added a new test: "" => all

looks good to me (and it is already approved by lunny)

regarding your question:

TODO: if there are invalid access scopes, then it is treated as "all", but would we really always treat invalid scopes as "all"?

my approach was to intervene as little as possible to what was the current default behaviour. i might failed in covering that as we can see with your case.

we should probably just ignore the invalid scopes and handle the rest with some more info on the invalid scopes. i can do that as a new PR...

@wxiaoguang
Copy link
Contributor

Maybe we could leave the TODO to 1.24 milestone (then there would be much more time to test the new behavior), since 1.23 is releasing soon.

@lunny lunny added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/translation size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. 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.

Enhancing Gitea OAuth2 Provider with Granular Scopes for Resource Access
4 participants