mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-05-12 22:10:25 +00:00
feat: ensure repo-specific access tokens can't perform repo admin operations (#11736)
Last known backend change for #11311, fixing up some loose ends on the repository APIs related to repo-specific access tokens. Adds automated testing, and aligns permissions where necessary, to ensure that repo-specific access tokens can't change the administrative state of the repositories that they are limited to. Repo-specific access tokens cannot be used to: - convert a mirror into a normal repo, - create a new repository from a template, - transfer ownership of a repository - create a new repository (already protected, but test automation added), - delete a repository (already protected, but test automation added), - editing a repository's settings (already protected, but test automation added). **Breaking**: The template generation (`POST /repos/{template_owner}/{template_repo}/generate`) and repository deletion (`DELETE /repos/{username}/{reponame}`) APIs have been updated to require the same permission scope as creating a new repository. Either `write:user` or `write:organization` is required, depending on the owner of the repository being created or deleted. ## 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 - 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. - [ ] 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. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11736 Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org> Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net> Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
This commit is contained in:
parent
dc65408618
commit
a27f9a719e
11 changed files with 264 additions and 53 deletions
|
|
@ -172,7 +172,7 @@ jobs:
|
|||
})
|
||||
}
|
||||
|
||||
httpContext := NewAPITestContext(t, user2.Name, apiRepo.Name, auth_model.AccessTokenScopeWriteRepository)
|
||||
httpContext := NewAPITestContext(t, user2.Name, apiRepo.Name, auth_model.AccessTokenScopeWriteUser)
|
||||
doAPIDeleteRepository(httpContext)(t)
|
||||
})
|
||||
}
|
||||
|
|
@ -357,7 +357,7 @@ jobs:
|
|||
})
|
||||
}
|
||||
|
||||
httpContext := NewAPITestContext(t, user2.Name, apiRepo.Name, auth_model.AccessTokenScopeWriteRepository)
|
||||
httpContext := NewAPITestContext(t, user2.Name, apiRepo.Name, auth_model.AccessTokenScopeWriteUser)
|
||||
doAPIDeleteRepository(httpContext)(t)
|
||||
})
|
||||
}
|
||||
|
|
@ -409,7 +409,7 @@ jobs:
|
|||
|
||||
apiBaseRepo := createActionsTestRepo(t, user2Token, "actions-gitea-context", false)
|
||||
baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: apiBaseRepo.ID})
|
||||
user2APICtx := NewAPITestContext(t, baseRepo.OwnerName, baseRepo.Name, auth_model.AccessTokenScopeWriteRepository)
|
||||
user2APICtx := NewAPITestContext(t, baseRepo.OwnerName, baseRepo.Name, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
|
||||
|
||||
runner := newMockRunner()
|
||||
runner.registerAsRepoRunner(t, baseRepo.OwnerName, baseRepo.Name, "mock-runner", []string{"ubuntu-latest"})
|
||||
|
|
@ -620,7 +620,7 @@ func TestActionsEphemeral(t *testing.T) {
|
|||
|
||||
apiBaseRepo := createActionsTestRepo(t, user2Token, "actions-gitea-context", false)
|
||||
baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: apiBaseRepo.ID})
|
||||
user2APICtx := NewAPITestContext(t, baseRepo.OwnerName, baseRepo.Name, auth_model.AccessTokenScopeWriteRepository)
|
||||
user2APICtx := NewAPITestContext(t, baseRepo.OwnerName, baseRepo.Name, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
|
||||
|
||||
runner := newMockRunner()
|
||||
runner.registerAsEphemeralRepoRunner(t, baseRepo.OwnerName, baseRepo.Name, "mock-runner", []string{"ubuntu-latest"})
|
||||
|
|
|
|||
|
|
@ -159,7 +159,7 @@ jobs:
|
|||
})
|
||||
}
|
||||
|
||||
httpContext := NewAPITestContext(t, user2.Name, repo.Name, auth_model.AccessTokenScopeWriteRepository)
|
||||
httpContext := NewAPITestContext(t, user2.Name, repo.Name, auth_model.AccessTokenScopeWriteUser)
|
||||
doAPIDeleteRepository(httpContext)(t)
|
||||
})
|
||||
}
|
||||
|
|
@ -275,7 +275,7 @@ jobs:
|
|||
)
|
||||
}
|
||||
|
||||
httpContext := NewAPITestContext(t, user2.Name, repo.Name, auth_model.AccessTokenScopeWriteRepository)
|
||||
httpContext := NewAPITestContext(t, user2.Name, repo.Name, auth_model.AccessTokenScopeWriteUser)
|
||||
doAPIDeleteRepository(httpContext)(t)
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -80,7 +80,7 @@ jobs:
|
|||
actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: actionRunJob.RunID})
|
||||
assert.Equal(t, testCase.notifyEmail, actionRun.NotifyEmail)
|
||||
|
||||
httpContext := NewAPITestContext(t, user2.Name, apiRepo.Name, auth_model.AccessTokenScopeWriteRepository)
|
||||
httpContext := NewAPITestContext(t, user2.Name, apiRepo.Name, auth_model.AccessTokenScopeWriteUser)
|
||||
doAPIDeleteRepository(httpContext)(t)
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -64,3 +64,19 @@ func TestAPIConvert(t *testing.T) {
|
|||
repo4edited := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4})
|
||||
assert.False(t, repo4edited.IsMirror)
|
||||
}
|
||||
|
||||
// This test verifies that a repo-specific access token with `write:repository` scope is not a sufficient scope to edit
|
||||
// the settings of a repository that is within its repo-specific list.
|
||||
func TestAPIConvertAccessTokenResources(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
|
||||
repo5 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 5})
|
||||
org3 := "org3"
|
||||
|
||||
repoSpecificToken := createFineGrainedRepoAccessToken(t, "user2",
|
||||
[]auth_model.AccessTokenScope{auth_model.AccessTokenScopeWriteRepository},
|
||||
[]int64{repo5.ID},
|
||||
)
|
||||
req := NewRequest(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/convert", org3, repo5.Name)).AddTokenAuth(repoSpecificToken)
|
||||
MakeRequest(t, req, http.StatusForbidden)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -394,3 +394,21 @@ func TestAPIRepoEdit(t *testing.T) {
|
|||
assert.Equal(t, "rebase", apiRepo.DefaultUpdateStyle)
|
||||
})
|
||||
}
|
||||
|
||||
// This test verifies that a repo-specific access token with `write:repository` scope is not a sufficient scope to edit
|
||||
// the settings of a repository that is within its repo-specific list.
|
||||
func TestAPIRepoEditAccessTokenResources(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
|
||||
repo2OnlyToken := createFineGrainedRepoAccessToken(t, "user2",
|
||||
[]auth_model.AccessTokenScope{auth_model.AccessTokenScopeWriteRepository},
|
||||
[]int64{2},
|
||||
)
|
||||
desc := "here's a new description"
|
||||
req := NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/repos/user2/repo2"),
|
||||
&api.EditRepoOption{
|
||||
Description: &desc,
|
||||
}).
|
||||
AddTokenAuth(repo2OnlyToken)
|
||||
MakeRequest(t, req, http.StatusForbidden)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -824,6 +824,75 @@ func testAPIRepoCreateConflict(t *testing.T, u *url.URL) {
|
|||
})
|
||||
}
|
||||
|
||||
func TestAPIRepoCreateDenied(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
|
||||
// This test verifies that `write:repository` is not a sufficient scope to create a repository. If it was, then
|
||||
// repo-specific access tokens would be able to create new repositories.
|
||||
session := loginUser(t, "user2")
|
||||
writeToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
|
||||
|
||||
req := NewRequestWithJSON(t, "POST", "/api/v1/user/repos",
|
||||
&api.CreateRepoOption{
|
||||
Name: "my-new-repo",
|
||||
}).
|
||||
AddTokenAuth(writeToken)
|
||||
MakeRequest(t, req, http.StatusForbidden)
|
||||
}
|
||||
|
||||
func TestAPIRepoDelete(t *testing.T) {
|
||||
t.Run("permitted to delete user repo w/ user scope", func(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
session := loginUser(t, "user2")
|
||||
writeToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteUser)
|
||||
req := NewRequest(t, "DELETE", "/api/v1/repos/user2/repo2").
|
||||
AddTokenAuth(writeToken)
|
||||
MakeRequest(t, req, http.StatusNoContent)
|
||||
})
|
||||
|
||||
t.Run("denied to delete user repo w/ org scope", func(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
session := loginUser(t, "user2")
|
||||
writeToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteOrganization)
|
||||
req := NewRequest(t, "DELETE", "/api/v1/repos/user2/repo2").
|
||||
AddTokenAuth(writeToken)
|
||||
resp := MakeRequest(t, req, http.StatusForbidden)
|
||||
assert.Contains(t, resp.Body.String(), "token does not have at least one of required scope(s): [write:user]")
|
||||
})
|
||||
|
||||
t.Run("permitted to delete org repo w/ org scope", func(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
session := loginUser(t, "user2")
|
||||
writeToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteOrganization)
|
||||
req := NewRequest(t, "DELETE", "/api/v1/repos/org3/repo3").
|
||||
AddTokenAuth(writeToken)
|
||||
MakeRequest(t, req, http.StatusNoContent)
|
||||
})
|
||||
|
||||
t.Run("denied to delete org repo w/ user scope", func(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
session := loginUser(t, "user2")
|
||||
writeToken := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteUser)
|
||||
req := NewRequest(t, "DELETE", "/api/v1/repos/org3/repo3").
|
||||
AddTokenAuth(writeToken)
|
||||
resp := MakeRequest(t, req, http.StatusForbidden)
|
||||
assert.Contains(t, resp.Body.String(), "token does not have at least one of required scope(s): [write:organization]")
|
||||
})
|
||||
|
||||
t.Run("denied with repo-specific", func(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
// limit ourselves to write:repository -- repo-specific access tokens can't be created with write:user
|
||||
repo2OnlyToken := createFineGrainedRepoAccessToken(t, "user2",
|
||||
[]auth_model.AccessTokenScope{auth_model.AccessTokenScopeWriteRepository},
|
||||
[]int64{2},
|
||||
)
|
||||
req := NewRequest(t, "DELETE", "/api/v1/repos/user2/repo2").
|
||||
AddTokenAuth(repo2OnlyToken)
|
||||
resp := MakeRequest(t, req, http.StatusForbidden)
|
||||
assert.Contains(t, resp.Body.String(), "token does not have at least one of required scope(s): [write:user]")
|
||||
})
|
||||
}
|
||||
|
||||
func TestAPIRepoTransfer(t *testing.T) {
|
||||
testCases := []struct {
|
||||
ctxUserID int64
|
||||
|
|
@ -884,6 +953,23 @@ func TestAPIRepoTransfer(t *testing.T) {
|
|||
_ = repo_service.DeleteRepositoryDirectly(db.DefaultContext, user, repo.ID)
|
||||
}
|
||||
|
||||
// This test verifies that a repo-specific access token with `write:repository` scope is not a sufficient to transfer a
|
||||
// repository to another user.
|
||||
func TestAPIRepoTransferAccessTokenResources(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
|
||||
repo2OnlyToken := createFineGrainedRepoAccessToken(t, "user2",
|
||||
[]auth_model.AccessTokenScope{auth_model.AccessTokenScopeWriteRepository},
|
||||
[]int64{2},
|
||||
)
|
||||
|
||||
req := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo2/transfer", &api.TransferRepoOption{
|
||||
NewOwner: "org3",
|
||||
}).AddTokenAuth(repo2OnlyToken)
|
||||
resp := MakeRequest(t, req, http.StatusForbidden)
|
||||
assert.Contains(t, resp.Body.String(), "user should be an owner or a collaborator with admin write")
|
||||
}
|
||||
|
||||
func transfer(t *testing.T) *repo_model.Repository {
|
||||
// create repo to move
|
||||
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
|
||||
|
|
@ -972,38 +1058,93 @@ func TestAPIRejectTransfer(t *testing.T) {
|
|||
func TestAPIGenerateRepo(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
|
||||
templateRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 44})
|
||||
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
|
||||
session := loginUser(t, user.Name)
|
||||
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
|
||||
|
||||
templateRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 44})
|
||||
// write:repository scope is always required (logically, because we're writing inside the contents of a new
|
||||
// repository) but the need for write:user or write:organization depends on the target owner, so we'll test those
|
||||
// combinations.
|
||||
|
||||
// user
|
||||
repo := new(api.Repository)
|
||||
req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/generate", templateRepo.OwnerName, templateRepo.Name), &api.GenerateRepoOption{
|
||||
Owner: user.Name,
|
||||
Name: "new-repo",
|
||||
Description: "test generate repo",
|
||||
Private: false,
|
||||
GitContent: true,
|
||||
}).AddTokenAuth(token)
|
||||
resp := MakeRequest(t, req, http.StatusCreated)
|
||||
DecodeJSON(t, resp, repo)
|
||||
t.Run("permitted to generate into user with user scope", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
assert.Equal(t, "new-repo", repo.Name)
|
||||
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteUser, auth_model.AccessTokenScopeWriteRepository)
|
||||
repo := new(api.Repository)
|
||||
req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/generate", templateRepo.OwnerName, templateRepo.Name), &api.GenerateRepoOption{
|
||||
Owner: user.Name,
|
||||
Name: "new-repo",
|
||||
Description: "test generate repo",
|
||||
Private: false,
|
||||
GitContent: true,
|
||||
}).AddTokenAuth(token)
|
||||
resp := MakeRequest(t, req, http.StatusCreated)
|
||||
DecodeJSON(t, resp, repo)
|
||||
assert.Equal(t, "new-repo", repo.Name)
|
||||
})
|
||||
|
||||
// org
|
||||
req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/generate", templateRepo.OwnerName, templateRepo.Name), &api.GenerateRepoOption{
|
||||
Owner: "org3",
|
||||
Name: "new-repo",
|
||||
Description: "test generate repo",
|
||||
Private: false,
|
||||
GitContent: true,
|
||||
}).AddTokenAuth(token)
|
||||
resp = MakeRequest(t, req, http.StatusCreated)
|
||||
DecodeJSON(t, resp, repo)
|
||||
t.Run("denied to generate into user without user scope", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
assert.Equal(t, "new-repo", repo.Name)
|
||||
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
|
||||
req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/generate", templateRepo.OwnerName, templateRepo.Name), &api.GenerateRepoOption{
|
||||
Owner: user.Name,
|
||||
Name: "new-repo",
|
||||
Description: "test generate repo",
|
||||
Private: false,
|
||||
GitContent: true,
|
||||
}).AddTokenAuth(token)
|
||||
resp := MakeRequest(t, req, http.StatusForbidden)
|
||||
assert.Contains(t, resp.Body.String(), "token requires scope write:user to create a repository owned by a user")
|
||||
})
|
||||
|
||||
t.Run("permitted to generate into org with org scope", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteOrganization, auth_model.AccessTokenScopeWriteRepository)
|
||||
repo := new(api.Repository)
|
||||
req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/generate", templateRepo.OwnerName, templateRepo.Name), &api.GenerateRepoOption{
|
||||
Owner: "org3",
|
||||
Name: "new-repo",
|
||||
Description: "test generate repo",
|
||||
Private: false,
|
||||
GitContent: true,
|
||||
}).AddTokenAuth(token)
|
||||
resp := MakeRequest(t, req, http.StatusCreated)
|
||||
DecodeJSON(t, resp, repo)
|
||||
|
||||
assert.Equal(t, "new-repo", repo.Name)
|
||||
})
|
||||
|
||||
t.Run("denied to generate into org without org scope", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
|
||||
req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/generate", templateRepo.OwnerName, templateRepo.Name), &api.GenerateRepoOption{
|
||||
Owner: "org3",
|
||||
Name: "new-repo",
|
||||
Description: "test generate repo",
|
||||
Private: false,
|
||||
GitContent: true,
|
||||
}).AddTokenAuth(token)
|
||||
resp := MakeRequest(t, req, http.StatusForbidden)
|
||||
assert.Contains(t, resp.Body.String(), "token requires scope write:organization to create a repository owned by a user")
|
||||
})
|
||||
|
||||
t.Run("denied to generate without write:repository", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
|
||||
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteUser)
|
||||
req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/generate", templateRepo.OwnerName, templateRepo.Name), &api.GenerateRepoOption{
|
||||
Owner: user.Name,
|
||||
Name: "new-repo",
|
||||
Description: "test generate repo",
|
||||
Private: false,
|
||||
GitContent: true,
|
||||
}).AddTokenAuth(token)
|
||||
resp := MakeRequest(t, req, http.StatusForbidden)
|
||||
assert.Contains(t, resp.Body.String(), "token does not have at least one of required scope(s): [write:repository]")
|
||||
})
|
||||
}
|
||||
|
||||
func TestAPIRepoGetReviewers(t *testing.T) {
|
||||
|
|
|
|||
|
|
@ -346,16 +346,6 @@ func TestAPIDeniesPermissionBasedOnTokenScope(t *testing.T) {
|
|||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
"/api/v1/repos/user1/repo1",
|
||||
"DELETE",
|
||||
[]permission{
|
||||
{
|
||||
auth_model.AccessTokenScopeCategoryRepository,
|
||||
auth_model.Write,
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
"/api/v1/repos/user1/repo1/branches",
|
||||
"GET",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue