Skip to content

Conversation

@marcellmars
Copy link
Contributor

@marcellmars marcellmars commented Oct 2, 2024

Enhancing Gitea OAuth2 Provider with Granular Scopes for Resource Access

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 this capability as nonintrusively and simply as possible. It acknowledges that the default scope is all and begins to process only additional scopes typical for OIDC/OAuth2: openid, profile, email, and groups. These then should be the ones already established with personal tokens.

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 public-only and have no access to any API endpoint. The rest of the scopes should be provided as it is now, combining the existing Gitea OAuth2 scopes: openid, profile, email, and groups.

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

Another scenario could involve adding read:user and public-only, preventing the third party from accessing anything the user has kept private.

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 added an additional check to CheckOAuthAccessToken that returns AccessTokenScopeAll if no additional scopes or AccessTokenScope with whatever requested instead. That AccessTokenScope is then assigned to store.GetData()["ApiTokenScope"] in userIDFromToken to be checked against the accepted/stored grant 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.

The intent of this PR is to ensure that if nothing is added, or if scopes are added to openid, profile, email, groups but not in AccessTokenScope, everything continues to work as it currently does.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 2, 2024
@github-actions github-actions bot added modifies/translation modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files labels Oct 2, 2024
@marcellmars marcellmars marked this pull request as draft October 2, 2024 11:17
@techknowlogick
Copy link
Member

Apologies if this is out of scope, but it might be useful to give a description next to the used scopes as a plain language would likely lower any chance at additional scopes are accepted mistakenly (def out of scope for this PR is including a warning if admin scope is requested)

@marcellmars
Copy link
Contributor Author

Apologies if this is out of scope, but it might be useful to give a description next to the used scopes as a plain language would likely lower any chance at additional scopes are accepted mistakenly (def out of scope for this PR is including a warning if admin scope is requested)

are you thinking of adding plain language descriptions as code comments, or user interface changes that would warn users while they approve the required scopes?

currently, users are warned in the web ui about full access to all resources. are you suggesting we provide a clearer explanation of what happens when someone grants more limited or restrictive access?

@github-actions github-actions bot added modifies/translation and removed modifies/translation modifies/api This PR adds API routes or modifies them labels Oct 17, 2024
@marcellmars marcellmars requested a review from lunny October 18, 2024 07:07
@marcellmars marcellmars marked this pull request as ready for review October 18, 2024 08:04
- the logic introduced with this PR will be applied by default, even though it
introduces breaking changes if anyone relied on the previous behavior
regarding personal access tokens or full access for OAuth2 third parties.
}

groups, err := oauth2_provider.GetOAuthGroupsForUser(ctx, ctx.Doer)
onlyPublicGroups := ctx.IsSigned && (ctx.FormString("private") == "" || ctx.FormBool("private"))
Copy link
Member

Choose a reason for hiding this comment

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

ctx.FormString("private") == "" || ctx.FormBool("private")
It seems it's not right here? Maybe ctx.FormString("private") == "" || !ctx.FormBool("private").

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that was verbatim line for the private i copied from routers/web/repo/repo.go#L573

but meanwhile there were some changes with the new public only approach introduced recently.

after those changes oauth2_provider.GetOAuthGroupsForUser had the search into the database so the private can be done via an option without the need to traverse orgs as i originally did.

i changed my code into this:

	form := web.GetForm(ctx).(*forms.AuthorizationForm)
	onlyPublicGroups, _ := oauth2_provider.GrantAdditionalScopes(form.Scope).PublicOnly()
	groups, err := oauth2_provider.GetOAuthGroupsForUser(ctx, ctx.Doer, onlyPublicGroups)
	if err != nil {
		ctx.ServerError("Oauth groups for user", err)
		return
	}
	response.Groups = groups

but dancing around too many repos i ended up in a major mess with my pull request so i think i will just do another pull request which would try to clean it up.

Introduce "idNumber" for each migration, and clarify the difference
between the migration ID number and database version.
lunny and others added 19 commits November 18, 2024 19:14
Many files do not directly depend on jQuery now.

To clarify the usage: use `fomanticQuery` to operate Fomantic
components.

Then developers could focus on removing the remaining jQuery usages by
searching `import $` globally.

21 files now:

```
./components/RepoBranchTagSelector.vue:3:import $ from 'jquery';
./features/admin/common.ts:1:import $ from 'jquery';
./features/admin/emails.ts:1:import $ from 'jquery';
./features/common-button.ts:1:import $ from 'jquery';
./features/comp/ComboMarkdownEditor.ts:3:import $ from 'jquery'; (I am working on it, there will be a new PR)
./features/comp/LabelEdit.ts:1:import $ from 'jquery';
./features/notification.ts:1:import $ from 'jquery';
./features/org-team.ts:1:import $ from 'jquery';
./features/repo-code.ts:1:import $ from 'jquery';
./features/repo-common.ts:1:import $ from 'jquery';
./features/repo-diff.ts:1:import $ from 'jquery';
./features/repo-editor.ts:1:import $ from 'jquery';
./features/repo-issue-content.ts:1:import $ from 'jquery';
./features/repo-issue-list.ts:1:import $ from 'jquery';
./features/repo-issue-sidebar.ts:1:import $ from 'jquery';
./features/repo-issue.ts:1:import $ from 'jquery';
./features/repo-legacy.ts:1:import $ from 'jquery';
./features/repo-new.ts:1:import $ from 'jquery';
./features/repo-projects.ts:1:import $ from 'jquery';
./features/repo-settings.ts:1:import $ from 'jquery';
./features/repo-template.ts:1:import $ from 'jquery';
```
In profiling integration tests, I found a couple places where per-test
overhead could be reduced:

* Avoiding disk IO by synchronizing instead of deleting & copying test
Git repository data. This saves ~100ms per test on my machine
* When flushing queues in `PrintCurrentTest`, invoke `FlushWithContext`
in a parallel.

---------

Co-authored-by: wxiaoguang <[email protected]>
Most modern browsers support it now

` Update ALLOWED_TYPES #96 ` https://gitea.com/gitea/docs/pulls/96

---------

Co-authored-by: silverwind <[email protected]>
Otherwise milestone JS would run on this page and cause errors
Since there is a status column in the database, the transaction is
unnecessary when downloading an archive. The transaction is blocking
database operations, especially with SQLite.

Replace #27563
Fix #32499

- Add the missing `recentupdate` to `OrderByFlatMap`
- Assign default value(`recentupdate`) to `EXPLORE_PAGING_DEFAULT_SORT`
By some CI fine tunes (`run tests`), SQLite & MSSQL could complete
in about 12~13 minutes (before > 14), MySQL could complete in 18 minutes
(before: about 23 or even > 30)

Major changes:

1. use tmpfs for MySQL storage
1. run `make test-mysql` instead of `make integration-test-coverage`
because the code coverage is not really used at the moment.
1. refactor testlogger to make it more reliable and be able to report
stuck stacktrace
1. do not requeue failed items when a queue is being flushed (failed
items would keep failing and make flush uncompleted)
1. reduce the file sizes for testing
1. use math ChaCha20 random data instead of crypot/rand (for testing
purpose only)
1. no need to `DeleteRepository` in `TestLinguist`
1. other related refactoring to make code easier to maintain
Close #31709 

52px is calculate by avatar size in
templates\repo\issue\view_content\comments.tmpl
```html
<img src="{{.Poster.AvatarLink $.Context}}" width="40" height="40">
```
+
```css
.ui.comments .comment > .avatar ~ .content {
  margin-left: 12px;
}
```


![圖片](https://github.com/user-attachments/assets/bf15f4d4-1574-46f6-9f5e-1fbdbf1a98b0)

---------

Co-authored-by: wxiaoguang <[email protected]>
…#32528)

- Move models/GetForks to services/FindForks
- Add doer as a parameter of FindForks to check permissions
- Slight performance optimization for get forks API with batch loading
of repository units
- Add tests for forking repository to organizations

---------

Co-authored-by: wxiaoguang <[email protected]>
Remove unmaintainable sanitizer rules. No need to add special "class"
regexp rules anymore, use RenderInternal.SafeAttr instead, more details
(and examples) are in the tests
@github-actions github-actions bot added modifies/api This PR adds API routes or modifies them modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin modifies/docs modifies/migrations modifies/internal modifies/dependencies modifies/frontend docs-update-needed The document needs to be updated synchronously labels Nov 18, 2024
@marcellmars
Copy link
Contributor Author

the new PR with cleaner commits would make sense.

@go-gitea go-gitea locked as resolved and limited conversation to collaborators Feb 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

docs-update-needed The document needs to be updated synchronously lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/api This PR adds API routes or modifies them modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin modifies/dependencies modifies/docs modifies/frontend modifies/go Pull requests that update Go code modifies/internal modifies/migrations modifies/templates This PR modifies the template files modifies/translation

Projects

None yet

Development

Successfully merging this pull request may close these issues.