diff --git a/routers/api/v1/repo/release.go b/routers/api/v1/repo/release.go index 4f17590abd..ff43628fa5 100644 --- a/routers/api/v1/repo/release.go +++ b/routers/api/v1/repo/release.go @@ -10,7 +10,6 @@ import ( auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/models/perm" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/git" @@ -22,26 +21,19 @@ import ( release_service "code.gitea.io/gitea/services/release" ) -func hasRepoWriteScope(ctx *context.APIContext) bool { - scope, ok := ctx.Data["ApiTokenScope"].(auth_model.AccessTokenScope) - if ctx.Data["IsApiToken"] != true || !ok { - return true - } - - requiredScopes := auth_model.GetRequiredScopes(auth_model.Write, auth_model.AccessTokenScopeCategoryRepository) - allow, err := scope.HasScope(requiredScopes...) - if err != nil { - ctx.APIError(http.StatusForbidden, "checking scope failed: "+err.Error()) - return false - } - return allow -} - -func canAccessDraftRelease(ctx *context.APIContext) bool { +func canAccessReleaseDraft(ctx *context.APIContext) bool { if !ctx.IsSigned || !ctx.Repo.CanWrite(unit.TypeReleases) { return false } - return hasRepoWriteScope(ctx) + if ctx.Data["IsApiToken"] != true { + // not API token request, the request is from a user session with write access + return true + } + // the request is from an access token with scope + scope := ctx.Data["ApiTokenScope"].(auth_model.AccessTokenScope) + requiredScopes := auth_model.GetRequiredScopes(auth_model.Write, auth_model.AccessTokenScopeCategoryRepository) + allow, _ := scope.HasScope(requiredScopes...) // err (invalid token) can be safely ignored + return allow } // GetRelease get a single release of a repository @@ -85,13 +77,9 @@ func GetRelease(ctx *context.APIContext) { return } - if release.IsDraft { // only the users with write access can see draft releases - if !canAccessDraftRelease(ctx) { - if !ctx.Written() { - ctx.APIErrorNotFound() - } - return - } + if release.IsDraft && !canAccessReleaseDraft(ctx) { // only the users with write access can see draft releases + ctx.APIErrorNotFound() + return } if err := release.LoadAttributes(ctx); err != nil { @@ -182,14 +170,12 @@ func ListReleases(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" listOptions := utils.GetListOptions(ctx) - - includeDrafts := (ctx.Repo.AccessMode >= perm.AccessModeWrite || ctx.Repo.UnitAccessMode(unit.TypeReleases) >= perm.AccessModeWrite) && hasRepoWriteScope(ctx) if ctx.Written() { return } opts := repo_model.FindReleasesOptions{ ListOptions: listOptions, - IncludeDrafts: includeDrafts, + IncludeDrafts: canAccessReleaseDraft(ctx), IncludeTags: false, IsDraft: ctx.FormOptionalBool("draft"), IsPreRelease: ctx.FormOptionalBool("pre-release"), diff --git a/routers/api/v1/repo/release_attachment.go b/routers/api/v1/repo/release_attachment.go index 6b30070db8..19075961f3 100644 --- a/routers/api/v1/repo/release_attachment.go +++ b/routers/api/v1/repo/release_attachment.go @@ -34,13 +34,9 @@ func checkReleaseMatchRepo(ctx *context.APIContext, releaseID int64) bool { ctx.APIErrorNotFound() return false } - if release.IsDraft { - if !canAccessDraftRelease(ctx) { - if !ctx.Written() { - ctx.APIErrorNotFound() - } - return false - } + if release.IsDraft && !canAccessReleaseDraft(ctx) { + ctx.APIErrorNotFound() + return false } return true } @@ -149,13 +145,9 @@ func ListReleaseAttachments(ctx *context.APIContext) { ctx.APIErrorNotFound() return } - if release.IsDraft { - if !canAccessDraftRelease(ctx) { - if !ctx.Written() { - ctx.APIErrorNotFound() - } - return - } + if release.IsDraft && !canAccessReleaseDraft(ctx) { + ctx.APIErrorNotFound() + return } if err := release.LoadAttributes(ctx); err != nil { ctx.APIErrorInternal(err) diff --git a/tests/integration/api_releases_attachment_test.go b/tests/integration/api_releases_attachment_test.go index e859b23c72..3f2592e331 100644 --- a/tests/integration/api_releases_attachment_test.go +++ b/tests/integration/api_releases_attachment_test.go @@ -15,16 +15,13 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" - "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" ) -func TestAPIEditReleaseAttachmentWithUnallowedFile(t *testing.T) { +func testAPIEditReleaseAttachmentWithUnallowedFile(t *testing.T) { // Limit the allowed release types (since by default there is no restriction) defer test.MockVariableValue(&setting.Repository.Release.AllowedTypes, ".exe")() - defer tests.PrepareTestEnv(t)() - attachment := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 9}) release := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{ID: attachment.ReleaseID}) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: attachment.RepoID}) @@ -42,9 +39,7 @@ func TestAPIEditReleaseAttachmentWithUnallowedFile(t *testing.T) { session.MakeRequest(t, req, http.StatusUnprocessableEntity) } -func TestAPIDraftReleaseAttachmentAccess(t *testing.T) { - defer tests.PrepareTestEnv(t)() - +func testAPIDraftReleaseAttachmentAccess(t *testing.T) { attachment := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 13}) release := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{ID: attachment.ReleaseID}) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: attachment.RepoID}) diff --git a/tests/integration/api_releases_test.go b/tests/integration/api_releases_test.go index c7200129dd..c7f1343dde 100644 --- a/tests/integration/api_releases_test.go +++ b/tests/integration/api_releases_test.go @@ -29,9 +29,19 @@ import ( "github.com/stretchr/testify/assert" ) -func TestAPIListReleasesWithWriteToken(t *testing.T) { +func TestAPIReleaseRead(t *testing.T) { defer tests.PrepareTestEnv(t)() + t.Run("DraftReleaseAttachmentAccess", testAPIDraftReleaseAttachmentAccess) + t.Run("ListReleasesWithWriteToken", testAPIListReleasesWithWriteToken) + t.Run("ListReleasesWithReadToken", testAPIListReleasesWithReadToken) + t.Run("GetDraftRelease", testAPIGetDraftRelease) + t.Run("GetLatestRelease", testAPIGetLatestRelease) + t.Run("GetReleaseByTag", testAPIGetReleaseByTag) + t.Run("GetDraftReleaseByTag", testAPIGetDraftReleaseByTag) + t.Run("EditReleaseAttachmentWithUnallowedFile", testAPIEditReleaseAttachmentWithUnallowedFile) // failed attempt, so it is also a read test +} +func testAPIListReleasesWithWriteToken(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) token := getUserToken(t, user2.LowerName, auth_model.AccessTokenScopeWriteRepository) @@ -81,9 +91,7 @@ func TestAPIListReleasesWithWriteToken(t *testing.T) { testFilterByLen(true, url.Values{"draft": {"true"}, "pre-release": {"true"}}, 0, "there is no pre-release draft") } -func TestAPIListReleasesWithReadToken(t *testing.T) { - defer tests.PrepareTestEnv(t)() - +func testAPIListReleasesWithReadToken(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) token := getUserToken(t, user2.LowerName, auth_model.AccessTokenScopeReadRepository) @@ -129,9 +137,7 @@ func TestAPIListReleasesWithReadToken(t *testing.T) { testFilterByLen(true, url.Values{"draft": {"true"}, "pre-release": {"true"}}, 0, "there is no pre-release draft") } -func TestAPIGetDraftRelease(t *testing.T) { - defer tests.PrepareTestEnv(t)() - +func testAPIGetDraftRelease(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) release := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{ID: 4}) owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) @@ -300,9 +306,7 @@ func TestAPICreateReleaseGivenInvalidTarget(t *testing.T) { MakeRequest(t, req, http.StatusNotFound) } -func TestAPIGetLatestRelease(t *testing.T) { - defer tests.PrepareTestEnv(t)() - +func testAPIGetLatestRelease(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) @@ -315,9 +319,7 @@ func TestAPIGetLatestRelease(t *testing.T) { assert.Equal(t, "testing-release", release.Title) } -func TestAPIGetReleaseByTag(t *testing.T) { - defer tests.PrepareTestEnv(t)() - +func testAPIGetReleaseByTag(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) @@ -341,9 +343,7 @@ func TestAPIGetReleaseByTag(t *testing.T) { assert.NotEmpty(t, err.Message) } -func TestAPIGetDraftReleaseByTag(t *testing.T) { - defer tests.PrepareTestEnv(t)() - +func testAPIGetDraftReleaseByTag(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})