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)()