mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-05-12 22:10:25 +00:00
fix: create repo-specific access token unexpected behaviour with "repositories": [] (#11653)
When creating an access token via API, if `"repositories": []`, then it is expected that the intent of the user was to create a repo-specific access token, but the API currently creates an all-access access token instead. `"repositories": []` is expected to be an error, instead of an unexpectedly wide grant. Reported by @aahlenst during testing: https://codeberg.org/forgejo/forgejo/pulls/11604#issuecomment-11569816 ## 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 - [ ] 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. - [x] 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. - Bugfix in unreleased feature. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11653 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
8572835160
commit
ed76e2a114
2 changed files with 18 additions and 6 deletions
|
|
@ -188,7 +188,7 @@ func CreateAccessToken(ctx *context.APIContext) {
|
|||
var resourceRepos []*auth_model.AccessTokenResourceRepo
|
||||
var tokenRepositories []*api.RepositoryMeta
|
||||
|
||||
if len(form.Repositories) != 0 {
|
||||
if form.Repositories != nil {
|
||||
repos := make([]*repo_model.Repository, len(form.Repositories))
|
||||
for i, repoTarget := range form.Repositories {
|
||||
repo, err := repo_model.GetRepositoryByOwnerAndName(ctx, repoTarget.Owner, repoTarget.Name)
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ import (
|
|||
"forgejo.org/modules/log"
|
||||
api "forgejo.org/modules/structs"
|
||||
"forgejo.org/modules/test"
|
||||
"forgejo.org/modules/util"
|
||||
"forgejo.org/tests"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
|
@ -56,7 +57,7 @@ func TestAPIGetTokens(t *testing.T) {
|
|||
assert.Equal(t, []string{""}, at.Scopes)
|
||||
assert.Empty(t, at.Token)
|
||||
assert.Equal(t, "69d28c91", at.TokenLastEight)
|
||||
assert.Nil(t, at.Repositories)
|
||||
assert.Nil(t, at.Repositories) // not repo-specific access token - nil expected, not an empty array
|
||||
})
|
||||
|
||||
t.Run("GET w/ token auth", func(t *testing.T) {
|
||||
|
|
@ -770,7 +771,7 @@ func TestAPITokenCreation(t *testing.T) {
|
|||
t.Run("valid", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
req := NewRequestWithJSON(t, "POST", "/api/v1/users/user2/tokens", &api.CreateAccessTokenOption{
|
||||
Name: "even-newer-token",
|
||||
Name: util.CryptoRandomString(util.RandomStringLow), // avoid false test failures from conflicting names
|
||||
Scopes: []string{string(auth_model.AccessTokenScopeReadRepository)},
|
||||
Repositories: []*api.RepoTargetOption{
|
||||
{
|
||||
|
|
@ -790,7 +791,7 @@ func TestAPITokenCreation(t *testing.T) {
|
|||
t.Run("target other user's private repo", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
req := NewRequestWithJSON(t, "POST", "/api/v1/users/user2/tokens", &api.CreateAccessTokenOption{
|
||||
Name: "not-a-valid-token",
|
||||
Name: util.CryptoRandomString(util.RandomStringLow), // avoid unexpected test impact from conflicting names
|
||||
Scopes: []string{string(auth_model.AccessTokenScopeReadRepository)},
|
||||
Repositories: []*api.RepoTargetOption{
|
||||
{
|
||||
|
|
@ -806,7 +807,7 @@ func TestAPITokenCreation(t *testing.T) {
|
|||
t.Run("target invalid repo", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
req := NewRequestWithJSON(t, "POST", "/api/v1/users/user2/tokens", &api.CreateAccessTokenOption{
|
||||
Name: "not-a-valid-token",
|
||||
Name: util.CryptoRandomString(util.RandomStringLow), // avoid unexpected test impact from conflicting names
|
||||
Scopes: []string{string(auth_model.AccessTokenScopeReadRepository)},
|
||||
Repositories: []*api.RepoTargetOption{
|
||||
{
|
||||
|
|
@ -823,7 +824,7 @@ func TestAPITokenCreation(t *testing.T) {
|
|||
t.Run("invalid scopes", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
req := NewRequestWithJSON(t, "POST", "/api/v1/users/user2/tokens", &api.CreateAccessTokenOption{
|
||||
Name: "not-a-valid-token",
|
||||
Name: util.CryptoRandomString(util.RandomStringLow), // avoid unexpected test impact from conflicting names
|
||||
Scopes: []string{string(auth_model.AccessTokenScopeReadAdmin)},
|
||||
Repositories: []*api.RepoTargetOption{
|
||||
{
|
||||
|
|
@ -835,6 +836,17 @@ func TestAPITokenCreation(t *testing.T) {
|
|||
req.AddBasicAuth("user2")
|
||||
MakeRequest(t, req, http.StatusBadRequest)
|
||||
})
|
||||
|
||||
t.Run("invalid zero repositories", func(t *testing.T) {
|
||||
defer tests.PrintCurrentTest(t)()
|
||||
req := NewRequestWithJSON(t, "POST", "/api/v1/users/user2/tokens", &api.CreateAccessTokenOption{
|
||||
Name: util.CryptoRandomString(util.RandomStringLow), // avoid unexpected test impact from conflicting names
|
||||
Scopes: []string{string(auth_model.AccessTokenScopeReadRepository)},
|
||||
Repositories: []*api.RepoTargetOption{}, // not nil, but not populated
|
||||
})
|
||||
req.AddBasicAuth("user2")
|
||||
MakeRequest(t, req, http.StatusBadRequest)
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue