Skip to content

Commit e7fca90

Browse files
GiteaBotlunnyCopilot
authored
Fix get release draft permission check (#36659) (#36715)
Backport #36659 by @lunny Draft release and it's attachments need a write permission to access. Signed-off-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 3422318 commit e7fca90

File tree

5 files changed

+174
-3
lines changed

5 files changed

+174
-3
lines changed

‎models/fixtures/attachment.yml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,3 +153,16 @@
153153
download_count: 0
154154
size: 0
155155
created_unix: 946684800
156+
157+
-
158+
id: 13
159+
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a23
160+
repo_id: 1
161+
issue_id: 0
162+
release_id: 4
163+
uploader_id: 2
164+
comment_id: 0
165+
name: draft-attach
166+
download_count: 0
167+
size: 0
168+
created_unix: 946684800

‎routers/api/v1/repo/release.go‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"fmt"
99
"net/http"
1010

11+
auth_model "code.gitea.io/gitea/models/auth"
1112
"code.gitea.io/gitea/models/db"
1213
"code.gitea.io/gitea/models/perm"
1314
repo_model "code.gitea.io/gitea/models/repo"
@@ -21,6 +22,28 @@ import (
2122
release_service "code.gitea.io/gitea/services/release"
2223
)
2324

25+
func hasRepoWriteScope(ctx *context.APIContext) bool {
26+
scope, ok := ctx.Data["ApiTokenScope"].(auth_model.AccessTokenScope)
27+
if ctx.Data["IsApiToken"] != true || !ok {
28+
return true
29+
}
30+
31+
requiredScopes := auth_model.GetRequiredScopes(auth_model.Write, auth_model.AccessTokenScopeCategoryRepository)
32+
allow, err := scope.HasScope(requiredScopes...)
33+
if err != nil {
34+
ctx.APIError(http.StatusForbidden, "checking scope failed: "+err.Error())
35+
return false
36+
}
37+
return allow
38+
}
39+
40+
func canAccessDraftRelease(ctx *context.APIContext) bool {
41+
if !ctx.IsSigned || !ctx.Repo.CanWrite(unit.TypeReleases) {
42+
return false
43+
}
44+
return hasRepoWriteScope(ctx)
45+
}
46+
2447
// GetRelease get a single release of a repository
2548
func GetRelease(ctx *context.APIContext) {
2649
// swagger:operation GET /repos/{owner}/{repo}/releases/{id} repository repoGetRelease
@@ -62,6 +85,15 @@ func GetRelease(ctx *context.APIContext) {
6285
return
6386
}
6487

88+
if release.IsDraft { // only the users with write access can see draft releases
89+
if !canAccessDraftRelease(ctx) {
90+
if !ctx.Written() {
91+
ctx.APIErrorNotFound()
92+
}
93+
return
94+
}
95+
}
96+
6597
if err := release.LoadAttributes(ctx); err != nil {
6698
ctx.APIErrorInternal(err)
6799
return
@@ -151,9 +183,13 @@ func ListReleases(ctx *context.APIContext) {
151183
// "$ref": "#/responses/notFound"
152184
listOptions := utils.GetListOptions(ctx)
153185

186+
includeDrafts := (ctx.Repo.AccessMode >= perm.AccessModeWrite || ctx.Repo.UnitAccessMode(unit.TypeReleases) >= perm.AccessModeWrite) && hasRepoWriteScope(ctx)
187+
if ctx.Written() {
188+
return
189+
}
154190
opts := repo_model.FindReleasesOptions{
155191
ListOptions: listOptions,
156-
IncludeDrafts: ctx.Repo.AccessMode >= perm.AccessModeWrite || ctx.Repo.UnitAccessMode(unit.TypeReleases) >= perm.AccessModeWrite,
192+
IncludeDrafts: includeDrafts,
157193
IncludeTags: false,
158194
IsDraft: ctx.FormOptionalBool("draft"),
159195
IsPreRelease: ctx.FormOptionalBool("pre-release"),

‎routers/api/v1/repo/release_attachment.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,14 @@ func checkReleaseMatchRepo(ctx *context.APIContext, releaseID int64) bool {
3434
ctx.APIErrorNotFound()
3535
return false
3636
}
37+
if release.IsDraft {
38+
if !canAccessDraftRelease(ctx) {
39+
if !ctx.Written() {
40+
ctx.APIErrorNotFound()
41+
}
42+
return false
43+
}
44+
}
3745
return true
3846
}
3947

@@ -141,6 +149,14 @@ func ListReleaseAttachments(ctx *context.APIContext) {
141149
ctx.APIErrorNotFound()
142150
return
143151
}
152+
if release.IsDraft {
153+
if !canAccessDraftRelease(ctx) {
154+
if !ctx.Written() {
155+
ctx.APIErrorNotFound()
156+
}
157+
return
158+
}
159+
}
144160
if err := release.LoadAttributes(ctx); err != nil {
145161
ctx.APIErrorInternal(err)
146162
return

‎tests/integration/api_releases_attachment_test.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,11 @@ import (
1313
"code.gitea.io/gitea/models/unittest"
1414
user_model "code.gitea.io/gitea/models/user"
1515
"code.gitea.io/gitea/modules/setting"
16+
api "code.gitea.io/gitea/modules/structs"
1617
"code.gitea.io/gitea/modules/test"
1718
"code.gitea.io/gitea/tests"
19+
20+
"github.com/stretchr/testify/assert"
1821
)
1922

2023
func TestAPIEditReleaseAttachmentWithUnallowedFile(t *testing.T) {
@@ -38,3 +41,36 @@ func TestAPIEditReleaseAttachmentWithUnallowedFile(t *testing.T) {
3841

3942
session.MakeRequest(t, req, http.StatusUnprocessableEntity)
4043
}
44+
45+
func TestAPIDraftReleaseAttachmentAccess(t *testing.T) {
46+
defer tests.PrepareTestEnv(t)()
47+
48+
attachment := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 13})
49+
release := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{ID: attachment.ReleaseID})
50+
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: attachment.RepoID})
51+
repoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
52+
reader := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
53+
54+
listURL := fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d/assets", repoOwner.Name, repo.Name, release.ID)
55+
getURL := fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d/assets/%d", repoOwner.Name, repo.Name, release.ID, attachment.ID)
56+
57+
MakeRequest(t, NewRequest(t, "GET", listURL), http.StatusNotFound)
58+
MakeRequest(t, NewRequest(t, "GET", getURL), http.StatusNotFound)
59+
60+
readerToken := getUserToken(t, reader.LowerName, auth_model.AccessTokenScopeReadRepository)
61+
MakeRequest(t, NewRequest(t, "GET", listURL).AddTokenAuth(readerToken), http.StatusNotFound)
62+
MakeRequest(t, NewRequest(t, "GET", getURL).AddTokenAuth(readerToken), http.StatusNotFound)
63+
64+
ownerReadToken := getUserToken(t, repoOwner.LowerName, auth_model.AccessTokenScopeReadRepository)
65+
MakeRequest(t, NewRequest(t, "GET", listURL).AddTokenAuth(ownerReadToken), http.StatusNotFound)
66+
MakeRequest(t, NewRequest(t, "GET", getURL).AddTokenAuth(ownerReadToken), http.StatusNotFound)
67+
ownerToken := getUserToken(t, repoOwner.LowerName, auth_model.AccessTokenScopeWriteRepository)
68+
resp := MakeRequest(t, NewRequest(t, "GET", listURL).AddTokenAuth(ownerToken), http.StatusOK)
69+
var attachments []*api.Attachment
70+
DecodeJSON(t, resp, &attachments)
71+
if assert.Len(t, attachments, 1) {
72+
assert.Equal(t, attachment.ID, attachments[0].ID)
73+
}
74+
75+
MakeRequest(t, NewRequest(t, "GET", getURL).AddTokenAuth(ownerToken), http.StatusOK)
76+
}

‎tests/integration/api_releases_test.go‎

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,12 @@ import (
2929
"github.com/stretchr/testify/assert"
3030
)
3131

32-
func TestAPIListReleases(t *testing.T) {
32+
func TestAPIListReleasesWithWriteToken(t *testing.T) {
3333
defer tests.PrepareTestEnv(t)()
3434

3535
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
3636
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
37-
token := getUserToken(t, user2.LowerName, auth_model.AccessTokenScopeReadRepository)
37+
token := getUserToken(t, user2.LowerName, auth_model.AccessTokenScopeWriteRepository)
3838

3939
link, _ := url.Parse(fmt.Sprintf("/api/v1/repos/%s/%s/releases", user2.Name, repo.Name))
4040
resp := MakeRequest(t, NewRequest(t, "GET", link.String()).AddTokenAuth(token), http.StatusOK)
@@ -81,6 +81,76 @@ func TestAPIListReleases(t *testing.T) {
8181
testFilterByLen(true, url.Values{"draft": {"true"}, "pre-release": {"true"}}, 0, "there is no pre-release draft")
8282
}
8383

84+
func TestAPIListReleasesWithReadToken(t *testing.T) {
85+
defer tests.PrepareTestEnv(t)()
86+
87+
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
88+
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
89+
token := getUserToken(t, user2.LowerName, auth_model.AccessTokenScopeReadRepository)
90+
91+
link, _ := url.Parse(fmt.Sprintf("/api/v1/repos/%s/%s/releases", user2.Name, repo.Name))
92+
resp := MakeRequest(t, NewRequest(t, "GET", link.String()).AddTokenAuth(token), http.StatusOK)
93+
var apiReleases []*api.Release
94+
DecodeJSON(t, resp, &apiReleases)
95+
if assert.Len(t, apiReleases, 2) {
96+
for _, release := range apiReleases {
97+
switch release.ID {
98+
case 1:
99+
assert.False(t, release.IsDraft)
100+
assert.False(t, release.IsPrerelease)
101+
assert.True(t, strings.HasSuffix(release.UploadURL, "/api/v1/repos/user2/repo1/releases/1/assets"), release.UploadURL)
102+
case 5:
103+
assert.False(t, release.IsDraft)
104+
assert.True(t, release.IsPrerelease)
105+
assert.True(t, strings.HasSuffix(release.UploadURL, "/api/v1/repos/user2/repo1/releases/5/assets"), release.UploadURL)
106+
default:
107+
assert.NoError(t, fmt.Errorf("unexpected release: %v", release))
108+
}
109+
}
110+
}
111+
112+
// test filter
113+
testFilterByLen := func(auth bool, query url.Values, expectedLength int, msgAndArgs ...string) {
114+
link.RawQuery = query.Encode()
115+
req := NewRequest(t, "GET", link.String())
116+
if auth {
117+
req.AddTokenAuth(token)
118+
}
119+
resp = MakeRequest(t, req, http.StatusOK)
120+
DecodeJSON(t, resp, &apiReleases)
121+
assert.Len(t, apiReleases, expectedLength, msgAndArgs)
122+
}
123+
124+
testFilterByLen(false, url.Values{"draft": {"true"}}, 0, "anon should not see drafts")
125+
testFilterByLen(true, url.Values{"draft": {"true"}}, 0, "repo owner with read token should not see drafts")
126+
testFilterByLen(true, url.Values{"draft": {"false"}}, 2, "exclude drafts")
127+
testFilterByLen(true, url.Values{"draft": {"false"}, "pre-release": {"false"}}, 1, "exclude drafts and pre-releases")
128+
testFilterByLen(true, url.Values{"pre-release": {"true"}}, 1, "only get pre-release")
129+
testFilterByLen(true, url.Values{"draft": {"true"}, "pre-release": {"true"}}, 0, "there is no pre-release draft")
130+
}
131+
132+
func TestAPIGetDraftRelease(t *testing.T) {
133+
defer tests.PrepareTestEnv(t)()
134+
135+
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
136+
release := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{ID: 4})
137+
owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
138+
reader := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
139+
140+
urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d", owner.Name, repo.Name, release.ID)
141+
142+
MakeRequest(t, NewRequest(t, "GET", urlStr), http.StatusNotFound)
143+
144+
readerToken := getUserToken(t, reader.LowerName, auth_model.AccessTokenScopeReadRepository)
145+
MakeRequest(t, NewRequest(t, "GET", urlStr).AddTokenAuth(readerToken), http.StatusNotFound)
146+
147+
ownerToken := getUserToken(t, owner.LowerName, auth_model.AccessTokenScopeWriteRepository)
148+
resp := MakeRequest(t, NewRequest(t, "GET", urlStr).AddTokenAuth(ownerToken), http.StatusOK)
149+
var apiRelease api.Release
150+
DecodeJSON(t, resp, &apiRelease)
151+
assert.Equal(t, release.Title, apiRelease.Title)
152+
}
153+
84154
func createNewReleaseUsingAPI(t *testing.T, token string, owner *user_model.User, repo *repo_model.Repository, name, target, title, desc string) *api.Release {
85155
urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/releases", owner.Name, repo.Name)
86156
req := NewRequestWithJSON(t, "POST", urlStr, &api.CreateReleaseOption{

0 commit comments

Comments
 (0)