From 48da8f9888a46f4d3bb1981da50bc2b8f0fcfda2 Mon Sep 17 00:00:00 2001 From: Mathieu Fenniak Date: Sat, 28 Feb 2026 19:47:06 +0100 Subject: [PATCH] feat: implement repo-specific access tokens broadly for universal API permission checks (#11437) Repository-specific personal access tokens will allow a user's access tokens to be restricted to accessing zero-or-more specific repositories. Currently they can be configured as "All", or "Public only", and this project will add a third configuration option allowing specific repositories. This PR is part of a series (#11311), and builds on the infrastructure work in #11434. In this PR, repository-specific access tokens are implemented on the universal permission checks performed by the API middleware, affecting ~182 API endpoints that perform permission checks based upon repositories referenced in their API path (eg. `/v1/api/repos/{owner}/{repo}/...`). **Breaking change:** API access with a public-only access token would previously return a `403 Forbidden` error when attempting to access a private repository where the repository is on the API path. As part of incorporating the public-only logic into the centralized permission check, these APIs will now return `404 Not Found` instead, consistent with how repository-specific access tokens, and other permissions checks, are implemented in order to reduce the risk of data probing through error messages. For larger context on the usage and future incoming work, the description of #11311 can be referenced. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests for Go changes (can be removed for JavaScript changes) - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I ran... - [x] `make pr-go` before pushing ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - As there is no end-user accessibility to create repo-specific access tokens, this functionality will not be accessible to end-users yet. But the breaking change in error APIs for public-only access tokens will be visible to end-users. - [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. ## Release notes - Breaking features - [PR](https://codeberg.org/forgejo/forgejo/pulls/11437): implement repo-specific access tokens broadly for universal API permission checks. **Breaking:** API access with a public-only access token would previously return a `403 Forbidden` error when attempting to access a private repository where the repository is on the API path. As part of incorporating the public-only logic into the centralized permission check, these APIs will now return `404 Not Found` instead, consistent with how most permission checks are implemented in order to reduce the risk of data probing through error messages. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11437 Reviewed-by: Andreas Ahlenstorf Co-authored-by: Mathieu Fenniak Co-committed-by: Mathieu Fenniak --- .deadcode-out | 1 - release-notes/11437.md | 1 + routers/api/v1/api.go | 4 +- tests/integration/api_issue_test.go | 2 +- tests/integration/api_repo_branch_test.go | 8 +-- tests/integration/api_repo_test.go | 77 +++++++++++++++++++++++ 6 files changed, 85 insertions(+), 8 deletions(-) create mode 100644 release-notes/11437.md diff --git a/.deadcode-out b/.deadcode-out index b347989b6b..6d2c35e374 100644 --- a/.deadcode-out +++ b/.deadcode-out @@ -49,7 +49,6 @@ forgejo.org/models/organization SearchMembersOptions.ToConds forgejo.org/models/perm/access - GetUserRepoPermissionWithReducer GetRepoWriters forgejo.org/models/user diff --git a/release-notes/11437.md b/release-notes/11437.md new file mode 100644 index 0000000000..a23942d486 --- /dev/null +++ b/release-notes/11437.md @@ -0,0 +1 @@ +feat: implement repo-specific access tokens broadly for universal API permission checks. **Breaking:** API access with a public-only access token would previously return a `403 Forbidden` error when attempting to access a private repository where the repository is on the API path. As part of incorporating the public-only logic into the centralized permission check, these APIs will now return `404 Not Found` instead, consistent with how most permission checks are implemented in order to reduce the risk of data probing through error messages. \ No newline at end of file diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index dbeb916ca6..b680dd50ac 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -209,9 +209,9 @@ func repoAssignment() func(ctx *context.APIContext) { ctx.Repo.UnitsMode[u.Type] = ctx.Repo.AccessMode } } else { - ctx.Repo.Permission, err = access_model.GetUserRepoPermission(ctx, repo, ctx.Doer) + ctx.Repo.Permission, err = access_model.GetUserRepoPermissionWithReducer(ctx, repo, ctx.Doer, ctx.Reducer) if err != nil { - ctx.Error(http.StatusInternalServerError, "GetUserRepoPermission", err) + ctx.Error(http.StatusInternalServerError, "GetUserRepoPermissionWithReducer", err) return } } diff --git a/tests/integration/api_issue_test.go b/tests/integration/api_issue_test.go index 43258562c6..554155e5f7 100644 --- a/tests/integration/api_issue_test.go +++ b/tests/integration/api_issue_test.go @@ -178,7 +178,7 @@ func TestAPIListIssuesPublicOnly(t *testing.T) { publicOnlyToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadIssue, auth_model.AccessTokenScopePublicOnly) req = NewRequest(t, "GET", link.String()).AddTokenAuth(publicOnlyToken) - MakeRequest(t, req, http.StatusForbidden) + MakeRequest(t, req, http.StatusNotFound) } func TestAPICreateIssue(t *testing.T) { diff --git a/tests/integration/api_repo_branch_test.go b/tests/integration/api_repo_branch_test.go index a80ced6f12..dd378dc23c 100644 --- a/tests/integration/api_repo_branch_test.go +++ b/tests/integration/api_repo_branch_test.go @@ -33,7 +33,7 @@ func TestAPIRepoBranchesPlain(t *testing.T) { // public only token should be forbidden publicOnlyToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopePublicOnly, auth_model.AccessTokenScopeWriteRepository) link, _ := url.Parse(fmt.Sprintf("/api/v1/repos/org3/%s/branches", repo3.Name)) // a plain repo - MakeRequest(t, NewRequest(t, "GET", link.String()).AddTokenAuth(publicOnlyToken), http.StatusForbidden) + MakeRequest(t, NewRequest(t, "GET", link.String()).AddTokenAuth(publicOnlyToken), http.StatusNotFound) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) resp := MakeRequest(t, NewRequest(t, "GET", link.String()).AddTokenAuth(token), http.StatusOK) @@ -47,7 +47,7 @@ func TestAPIRepoBranchesPlain(t *testing.T) { assert.Equal(t, "master", branches[1].Name) link2, _ := url.Parse(fmt.Sprintf("/api/v1/repos/org3/%s/branches/test_branch", repo3.Name)) - MakeRequest(t, NewRequest(t, "GET", link2.String()).AddTokenAuth(publicOnlyToken), http.StatusForbidden) + MakeRequest(t, NewRequest(t, "GET", link2.String()).AddTokenAuth(publicOnlyToken), http.StatusNotFound) resp = MakeRequest(t, NewRequest(t, "GET", link2.String()).AddTokenAuth(token), http.StatusOK) bs, err = io.ReadAll(resp.Body) @@ -56,7 +56,7 @@ func TestAPIRepoBranchesPlain(t *testing.T) { require.NoError(t, json.Unmarshal(bs, &branch)) assert.Equal(t, "test_branch", branch.Name) - MakeRequest(t, NewRequest(t, "POST", link.String()).AddTokenAuth(publicOnlyToken), http.StatusForbidden) + MakeRequest(t, NewRequest(t, "POST", link.String()).AddTokenAuth(publicOnlyToken), http.StatusNotFound) req := NewRequest(t, "POST", link.String()).AddTokenAuth(token) req.Header.Add("Content-Type", "application/json") @@ -82,7 +82,7 @@ func TestAPIRepoBranchesPlain(t *testing.T) { link3, _ := url.Parse(fmt.Sprintf("/api/v1/repos/org3/%s/branches/test_branch2", repo3.Name)) MakeRequest(t, NewRequest(t, "DELETE", link3.String()), http.StatusNotFound) - MakeRequest(t, NewRequest(t, "DELETE", link3.String()).AddTokenAuth(publicOnlyToken), http.StatusForbidden) + MakeRequest(t, NewRequest(t, "DELETE", link3.String()).AddTokenAuth(publicOnlyToken), http.StatusNotFound) MakeRequest(t, NewRequest(t, "DELETE", link3.String()).AddTokenAuth(token), http.StatusNoContent) require.NoError(t, err) diff --git a/tests/integration/api_repo_test.go b/tests/integration/api_repo_test.go index 3dd79c8ebe..03d70e0fd6 100644 --- a/tests/integration/api_repo_test.go +++ b/tests/integration/api_repo_test.go @@ -289,6 +289,83 @@ func TestAPIViewRepo(t *testing.T) { assert.Equal(t, 1, repo.Stars) } +// `/repos/{username}/{reponame}` uses repoAssignment() middleware -- this test runs that middleware through all +// variations of access token resource access. +func TestAPIViewRepoAccessTokenResources(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + var repo api.Repository + + t.Run("all access token", func(t *testing.T) { + session := loginUser(t, "user2") + allToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository) + + t.Run("allowed public repo1", func(t *testing.T) { + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1").AddTokenAuth(allToken) + resp := MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &repo) + assert.False(t, repo.Private) + }) + t.Run("allowed private repo2", func(t *testing.T) { + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo2").AddTokenAuth(allToken) + resp := MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &repo) + assert.True(t, repo.Private) + }) + // repo16 is a second repo used in fine-grain testing below, so we include it in other tests as a baseline + t.Run("allowed private repo16", func(t *testing.T) { + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo16").AddTokenAuth(allToken) + resp := MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &repo) + assert.True(t, repo.Private) + }) + }) + + t.Run("public-only access token", func(t *testing.T) { + session := loginUser(t, "user2") + publicOnlyToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopePublicOnly, auth_model.AccessTokenScopeReadRepository) + + t.Run("allowed public repo1", func(t *testing.T) { + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1").AddTokenAuth(publicOnlyToken) + resp := MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &repo) + assert.False(t, repo.Private) + }) + t.Run("denied private repo2", func(t *testing.T) { + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo2").AddTokenAuth(publicOnlyToken) + MakeRequest(t, req, http.StatusNotFound) + }) + t.Run("denied private repo16", func(t *testing.T) { + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo16").AddTokenAuth(publicOnlyToken) + MakeRequest(t, req, http.StatusNotFound) + }) + }) + + t.Run("specific repo access token", func(t *testing.T) { + repo2OnlyToken := createFineGrainedRepoAccessToken(t, "user2", + []auth_model.AccessTokenScope{auth_model.AccessTokenScopeReadRepository}, + []int64{2}, + ) + + t.Run("allowed public repo1", func(t *testing.T) { + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1").AddTokenAuth(repo2OnlyToken) + resp := MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &repo) + assert.False(t, repo.Private) + }) + t.Run("allowed inside fine-grain repo2", func(t *testing.T) { + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo2").AddTokenAuth(repo2OnlyToken) + resp := MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &repo) + assert.True(t, repo.Private) + }) + t.Run("denied private outside fine-grain repo16", func(t *testing.T) { + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo16").AddTokenAuth(repo2OnlyToken) + MakeRequest(t, req, http.StatusNotFound) + }) + }) +} + // Validate that private information on the user profile isn't exposed by way of being an owner of a public repository. func TestAPIViewRepoOwnerSettings(t *testing.T) { defer tests.PrepareTestEnv(t)()