From 4b83448b7d89726a0fe5baa19cb408cfaf96ec9d Mon Sep 17 00:00:00 2001 From: Mathieu Fenniak Date: Wed, 10 Jun 2026 06:05:01 +0200 Subject: [PATCH] 2026-06-10 security patches (#13001) - fix: prevent stored XSS in user display name on Actions page - fix: LFS locks must belong to the intended repo, port from Gitea - fix: prevent unauthorized access to draft releases via API - fix: prevent writes to OpenID visibility which may affect other users - fix: prevent viewing private PRs that are linked to public issues on public projects Co-authored-by: Lunny Xiao Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13001 Reviewed-by: 0ko <0ko@noreply.codeberg.org> Reviewed-by: Beowulf --- models/git/lfs_lock.go | 8 +- models/git/lfs_lock_test.go | 82 ++++++++++++++ models/user/openid.go | 4 +- models/user/openid_test.go | 25 +++-- release-notes/13002.md | 5 + release-notes/13003.md | 4 + routers/api/v1/repo/release.go | 6 +- routers/api/v1/repo/release_attachment.go | 5 + .../comment.yml | 9 ++ .../project_issue.yml | 6 ++ routers/web/org/projects.go | 6 +- routers/web/org/projects_test.go | 67 ++++++++++++ .../comment.yml | 9 ++ .../TestViewProjectPRLinkVisibility/issue.yml | 13 +++ routers/web/repo/actions/view.go | 5 +- routers/web/repo/projects.go | 6 +- routers/web/repo/projects_test.go | 67 ++++++++++++ routers/web/user/setting/security/openid.go | 2 +- services/lfs/locks.go | 14 +-- tests/integration/api_releases_test.go | 100 ++++++++++++++++++ 20 files changed, 407 insertions(+), 36 deletions(-) create mode 100644 models/git/lfs_lock_test.go create mode 100644 release-notes/13002.md create mode 100644 release-notes/13003.md create mode 100644 routers/web/org/TestViewProjectPRLinkVisibility/comment.yml create mode 100644 routers/web/org/TestViewProjectPRLinkVisibility/project_issue.yml create mode 100644 routers/web/repo/TestViewProjectPRLinkVisibility/comment.yml create mode 100644 routers/web/repo/TestViewProjectPRLinkVisibility/issue.yml diff --git a/models/git/lfs_lock.go b/models/git/lfs_lock.go index 9dc0a947c3..7e44a3111b 100644 --- a/models/git/lfs_lock.go +++ b/models/git/lfs_lock.go @@ -112,10 +112,10 @@ func GetLFSLock(ctx context.Context, repo *repo_model.Repository, path string) ( return rel, nil } -// GetLFSLockByID returns release by given id. -func GetLFSLockByID(ctx context.Context, id int64) (*LFSLock, error) { +// GetLFSLockByIDAndRepo returns lfs lock by given id and repository id. +func GetLFSLockByIDAndRepo(ctx context.Context, id, repoID int64) (*LFSLock, error) { lock := new(LFSLock) - has, err := db.GetEngine(ctx).ID(id).Get(lock) + has, err := db.GetEngine(ctx).ID(id).And("repo_id = ?", repoID).Get(lock) if err != nil { return nil, err } else if !has { @@ -169,7 +169,7 @@ func DeleteLFSLockByID(ctx context.Context, id int64, repo *repo_model.Repositor } defer committer.Close() - lock, err := GetLFSLockByID(dbCtx, id) + lock, err := GetLFSLockByIDAndRepo(ctx, id, repo.ID) if err != nil { return nil, err } diff --git a/models/git/lfs_lock_test.go b/models/git/lfs_lock_test.go new file mode 100644 index 0000000000..a438a785c1 --- /dev/null +++ b/models/git/lfs_lock_test.go @@ -0,0 +1,82 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package git + +import ( + "fmt" + "testing" + "time" + + repo_model "forgejo.org/models/repo" + "forgejo.org/models/unittest" + user_model "forgejo.org/models/user" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func createTestLock(t *testing.T, repo *repo_model.Repository, owner *user_model.User) *LFSLock { + t.Helper() + + path := fmt.Sprintf("%s-%d-%d", t.Name(), repo.ID, time.Now().UnixNano()) + lock, err := CreateLFSLock(t.Context(), repo, &LFSLock{ + OwnerID: owner.ID, + Path: path, + }) + require.NoError(t, err) + return lock +} + +func TestGetLFSLockByIDAndRepo(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + + lockRepo1 := createTestLock(t, repo1, user2) + lockRepo3 := createTestLock(t, repo3, user4) + + fetched, err := GetLFSLockByIDAndRepo(t.Context(), lockRepo1.ID, repo1.ID) + require.NoError(t, err) + assert.Equal(t, lockRepo1.ID, fetched.ID) + assert.Equal(t, repo1.ID, fetched.RepoID) + + _, err = GetLFSLockByIDAndRepo(t.Context(), lockRepo1.ID, repo3.ID) + require.Error(t, err) + assert.True(t, IsErrLFSLockNotExist(err)) + + _, err = GetLFSLockByIDAndRepo(t.Context(), lockRepo3.ID, repo1.ID) + require.Error(t, err) + assert.True(t, IsErrLFSLockNotExist(err)) +} + +func TestDeleteLFSLockByIDRequiresRepoMatch(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + + lockRepo1 := createTestLock(t, repo1, user2) + lockRepo3 := createTestLock(t, repo3, user4) + + _, err := DeleteLFSLockByID(t.Context(), lockRepo3.ID, repo1, user2, true) + require.Error(t, err) + assert.True(t, IsErrLFSLockNotExist(err)) + + existing, err := GetLFSLockByIDAndRepo(t.Context(), lockRepo3.ID, repo3.ID) + require.NoError(t, err) + assert.Equal(t, lockRepo3.ID, existing.ID) + + deleted, err := DeleteLFSLockByID(t.Context(), lockRepo3.ID, repo3, user4, true) + require.NoError(t, err) + assert.Equal(t, lockRepo3.ID, deleted.ID) + + deleted, err = DeleteLFSLockByID(t.Context(), lockRepo1.ID, repo1, user2, false) + require.NoError(t, err) + assert.Equal(t, lockRepo1.ID, deleted.ID) +} diff --git a/models/user/openid.go b/models/user/openid.go index b4d4f175b2..3ad23ebc50 100644 --- a/models/user/openid.go +++ b/models/user/openid.go @@ -105,7 +105,7 @@ func DeleteUserOpenID(ctx context.Context, openid *UserOpenID) (err error) { } // ToggleUserOpenIDVisibility toggles visibility of an openid address of given user. -func ToggleUserOpenIDVisibility(ctx context.Context, id int64) (err error) { - _, err = db.GetEngine(ctx).Exec("update `user_open_id` set `show` = not `show` where `id` = ?", id) +func ToggleUserOpenIDVisibility(ctx context.Context, userID, userOpenID int64) (err error) { + _, err = db.GetEngine(ctx).Exec("update `user_open_id` set `show` = not `show` where `uid` = ? and `id` = ?", userID, userOpenID) return err } diff --git a/models/user/openid_test.go b/models/user/openid_test.go index 3c55891c1f..e6333c433d 100644 --- a/models/user/openid_test.go +++ b/models/user/openid_test.go @@ -40,29 +40,32 @@ func TestToggleUserOpenIDVisibility(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) oids, err := user_model.GetUserOpenIDs(db.DefaultContext, int64(2)) require.NoError(t, err) + require.Len(t, oids, 1) - if !assert.Len(t, oids, 1) { - return - } assert.True(t, oids[0].Show) - err = user_model.ToggleUserOpenIDVisibility(db.DefaultContext, oids[0].ID) + err = user_model.ToggleUserOpenIDVisibility(db.DefaultContext, oids[0].UID, oids[0].ID) require.NoError(t, err) oids, err = user_model.GetUserOpenIDs(db.DefaultContext, int64(2)) require.NoError(t, err) + require.Len(t, oids, 1) - if !assert.Len(t, oids, 1) { - return - } assert.False(t, oids[0].Show) - err = user_model.ToggleUserOpenIDVisibility(db.DefaultContext, oids[0].ID) + err = user_model.ToggleUserOpenIDVisibility(db.DefaultContext, oids[0].UID, oids[0].ID) require.NoError(t, err) oids, err = user_model.GetUserOpenIDs(db.DefaultContext, int64(2)) require.NoError(t, err) + require.Len(t, oids, 1) - if assert.Len(t, oids, 1) { - assert.True(t, oids[0].Show) - } + assert.True(t, oids[0].Show) + + // mismatched UID ineffective + err = user_model.ToggleUserOpenIDVisibility(db.DefaultContext, 999, oids[0].ID) + require.NoError(t, err) + oids, err = user_model.GetUserOpenIDs(db.DefaultContext, int64(2)) + require.NoError(t, err) + require.Len(t, oids, 1) + assert.True(t, oids[0].Show) } diff --git a/release-notes/13002.md b/release-notes/13002.md new file mode 100644 index 0000000000..e0ea3b29d5 --- /dev/null +++ b/release-notes/13002.md @@ -0,0 +1,5 @@ +- fix: prevent stored XSS in user display name on Actions page +- fix: LFS locks must belong to the intended repo, port from Gitea +- fix: prevent unauthorized access to draft releases via API +- fix: prevent writes to OpenID visibility which may affect other users +- fix: prevent viewing private PRs that are linked to public issues on public projects diff --git a/release-notes/13003.md b/release-notes/13003.md new file mode 100644 index 0000000000..ae9c479349 --- /dev/null +++ b/release-notes/13003.md @@ -0,0 +1,4 @@ +- fix: LFS locks must belong to the intended repo, port from Gitea +- fix: prevent unauthorized access to draft releases via API +- fix: prevent writes to OpenID visibility which may affect other users +- fix: prevent viewing private PRs that are linked to public issues on public projects diff --git a/routers/api/v1/repo/release.go b/routers/api/v1/repo/release.go index 0f48c8427e..875add5a4a 100644 --- a/routers/api/v1/repo/release.go +++ b/routers/api/v1/repo/release.go @@ -62,7 +62,10 @@ func GetRelease(ctx *context.APIContext) { ctx.NotFound() return } - + if release.IsDraft && !ctx.Repo.CanWrite(unit.TypeReleases) { + ctx.NotFound() + return + } if err := release.LoadAttributes(ctx); err != nil { ctx.Error(http.StatusInternalServerError, "LoadAttributes", err) return @@ -103,7 +106,6 @@ func GetLatestRelease(ctx *context.APIContext) { ctx.NotFound() return } - if err := release.LoadAttributes(ctx); err != nil { ctx.Error(http.StatusInternalServerError, "LoadAttributes", err) return diff --git a/routers/api/v1/repo/release_attachment.go b/routers/api/v1/repo/release_attachment.go index 211900ec37..0e6b882c3a 100644 --- a/routers/api/v1/repo/release_attachment.go +++ b/routers/api/v1/repo/release_attachment.go @@ -12,6 +12,7 @@ import ( "strings" repo_model "forgejo.org/models/repo" + unit_model "forgejo.org/models/unit" "forgejo.org/modules/log" "forgejo.org/modules/setting" api "forgejo.org/modules/structs" @@ -143,6 +144,10 @@ func ListReleaseAttachments(ctx *context.APIContext) { ctx.NotFound() return } + if release.IsDraft && !ctx.Repo.CanWrite(unit_model.TypeReleases) { + ctx.NotFound() + return + } if err := release.LoadAttributes(ctx); err != nil { ctx.Error(http.StatusInternalServerError, "LoadAttributes", err) return diff --git a/routers/web/org/TestViewProjectPRLinkVisibility/comment.yml b/routers/web/org/TestViewProjectPRLinkVisibility/comment.yml new file mode 100644 index 0000000000..928503251c --- /dev/null +++ b/routers/web/org/TestViewProjectPRLinkVisibility/comment.yml @@ -0,0 +1,9 @@ +- + id: 3000 + type: 6 # pull reference + poster_id: 2 + issue_id: 16 # pull in repo_id 32 + ref_repo_id: 3 + ref_issue_id: 12 # pull in repo_id 3 + ref_is_pull: true + created_unix: 946690100 diff --git a/routers/web/org/TestViewProjectPRLinkVisibility/project_issue.yml b/routers/web/org/TestViewProjectPRLinkVisibility/project_issue.yml new file mode 100644 index 0000000000..62cef343f5 --- /dev/null +++ b/routers/web/org/TestViewProjectPRLinkVisibility/project_issue.yml @@ -0,0 +1,6 @@ +- + id: 10 + issue_id: 16 + project_id: 7 + project_board_id: 0 + sorting: 1 diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index 6e5f4079ac..b1f005ddb8 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -377,8 +377,10 @@ func ViewProject(ctx *context.Context) { if len(referencedIDs) > 0 { if linkedPrs, err := issues_model.Issues(ctx, &issues_model.IssuesOptions{ - IssueIDs: referencedIDs, - IsPull: optional.Some(true), + IssueIDs: referencedIDs, + IsPull: optional.Some(true), + User: ctx.Doer, + AllPublic: !ctx.IsSigned, }); err == nil { linkedPrsMap[issue.ID] = linkedPrs } diff --git a/routers/web/org/projects_test.go b/routers/web/org/projects_test.go index dec78502f2..2cf77f84d4 100644 --- a/routers/web/org/projects_test.go +++ b/routers/web/org/projects_test.go @@ -4,13 +4,17 @@ package org_test import ( + "net/http" "testing" + issues_model "forgejo.org/models/issues" "forgejo.org/models/unittest" + "forgejo.org/models/user" "forgejo.org/routers/web/org" "forgejo.org/services/contexttest" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCheckProjectColumnChangePermissions(t *testing.T) { @@ -26,3 +30,66 @@ func TestCheckProjectColumnChangePermissions(t *testing.T) { assert.NotNil(t, column) assert.False(t, ctx.Written()) } + +func TestViewProjectPRLinkVisibility(t *testing.T) { + defer unittest.OverrideFixtures("routers/web/org/TestViewProjectPRLinkVisibility")() + unittest.PrepareTestEnv(t) + + // When a private PR references a public issue, and the public issue is put onto a project board, then the + // cross-reference from the private PR to the public issue should only be visible if the private PR is visible to + // the logged-in user. + // + // Test requires data fixtures: + // + // private repo, w/ PR in the private repo + // repository.id = 3, org3/repo3 + // PR is issue.ID = 12 + // public repo, public issue + // repository.id = 32, org3/repo21 + // issue.id = 16, org3/repo21#1 + // cross-ref comment from the private PR -> public issue + // comment.id = 3000 + // project board + // project.id = 7, owned by org3 + // put public issue on the project board + // project_issue.id = 10 + + test := func(loggedIn bool) map[int64][]*issues_model.Issue { + ctx, recorder := contexttest.MockContext(t, "org3/-/projects/7") + if loggedIn { + contexttest.LoadUser(t, ctx, 2) + } + contexttest.LoadOrganization(t, ctx, 3) + ctx.ContextUser = unittest.AssertExistsAndLoadBean(t, &user.User{ID: 3}) + ctx.SetParams(":id", "7") + + org.ViewProject(ctx) + assert.Equal(t, http.StatusOK, recorder.Result().StatusCode) // Verify it's a success response + + // Map issue on the project (16) to array of PRs that reference that issue. + linkedPRs, ok := ctx.Data["LinkedPRs"].(map[int64][]*issues_model.Issue) + require.True(t, ok, "LinkedPRs must be map[int64][]*issues_model.Issue") + + return linkedPRs + } + + t.Run("authorized user with visibility", func(t *testing.T) { + linkedPRs := test(true) + + prList, ok := linkedPRs[16] + require.True(t, ok, "linkedPRs must contain ID=16") + assert.Len(t, prList, 1) + + prIssue := prList[0] + assert.True(t, prIssue.IsPull) + assert.EqualValues(t, 12, prIssue.ID) + }) + + t.Run("anonymous user", func(t *testing.T) { + linkedPRs := test(false) + + prList, ok := linkedPRs[16] + require.True(t, ok, "linkedPRs must contain ID=16") + assert.Empty(t, prList) + }) +} diff --git a/routers/web/repo/TestViewProjectPRLinkVisibility/comment.yml b/routers/web/repo/TestViewProjectPRLinkVisibility/comment.yml new file mode 100644 index 0000000000..6b9e647166 --- /dev/null +++ b/routers/web/repo/TestViewProjectPRLinkVisibility/comment.yml @@ -0,0 +1,9 @@ +- + id: 3000 + type: 6 # pull reference + poster_id: 2 + issue_id: 1 # issue in public repo_id 1 + ref_repo_id: 2 # private repo + ref_issue_id: 100 # PR in private repo 2 + ref_is_pull: true + created_unix: 946690100 diff --git a/routers/web/repo/TestViewProjectPRLinkVisibility/issue.yml b/routers/web/repo/TestViewProjectPRLinkVisibility/issue.yml new file mode 100644 index 0000000000..7f976459e4 --- /dev/null +++ b/routers/web/repo/TestViewProjectPRLinkVisibility/issue.yml @@ -0,0 +1,13 @@ +- + id: 100 + repo_id: 2 + index: 100 + poster_id: 2 + original_author_id: 0 + name: pr on user2/repo2 + content: content for the pull request + is_closed: true + is_pull: true + num_comments: 1 + created_unix: 946684830 + updated_unix: 978307200 diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 598d1892c8..173abd6d11 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -7,6 +7,7 @@ package actions import ( "errors" "fmt" + "html" "html/template" "net/http" "net/url" @@ -286,10 +287,10 @@ func getViewResponse(ctx *app_context.Context, req *ViewRequest, runIndex, jobIn base.ShortSha(run.CommitSHA)) } else if run.IsDispatchedRun() { runDescription = ctx.Locale.TrString("actions.runs.workflow_dispatch_description", run.CommitLink(), - base.ShortSha(run.CommitSHA), run.TriggerUser.HomeLink(), run.TriggerUser.GetDisplayName()) + base.ShortSha(run.CommitSHA), run.TriggerUser.HomeLink(), html.EscapeString(run.TriggerUser.GetDisplayName())) } else { runDescription = ctx.Locale.TrString("actions.runs.on_push_description", run.CommitLink(), - base.ShortSha(run.CommitSHA), run.TriggerUser.HomeLink(), run.TriggerUser.GetDisplayName()) + base.ShortSha(run.CommitSHA), run.TriggerUser.HomeLink(), html.EscapeString(run.TriggerUser.GetDisplayName())) } resp.State.Run.Title = run.Title diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index f007dfa7ba..5631027c3c 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -355,8 +355,10 @@ func ViewProject(ctx *context.Context) { if len(referencedIDs) > 0 { if linkedPrs, err := issues_model.Issues(ctx, &issues_model.IssuesOptions{ - IssueIDs: referencedIDs, - IsPull: optional.Some(true), + IssueIDs: referencedIDs, + IsPull: optional.Some(true), + User: ctx.Doer, + AllPublic: !ctx.IsSigned, }); err == nil { linkedPrsMap[issue.ID] = linkedPrs } diff --git a/routers/web/repo/projects_test.go b/routers/web/repo/projects_test.go index bc8b747980..844126c47f 100644 --- a/routers/web/repo/projects_test.go +++ b/routers/web/repo/projects_test.go @@ -4,12 +4,16 @@ package repo import ( + "net/http" "testing" + issues_model "forgejo.org/models/issues" "forgejo.org/models/unittest" + "forgejo.org/models/user" "forgejo.org/services/contexttest" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCheckProjectColumnChangePermissions(t *testing.T) { @@ -25,3 +29,66 @@ func TestCheckProjectColumnChangePermissions(t *testing.T) { assert.NotNil(t, column) assert.False(t, ctx.Written()) } + +func TestViewProjectPRLinkVisibility(t *testing.T) { + defer unittest.OverrideFixtures("routers/web/repo/TestViewProjectPRLinkVisibility")() + unittest.PrepareTestEnv(t) + + // When a private PR references a public issue, and the public issue is put onto a project board, then the + // cross-reference from the private PR to the public issue should only be visible if the private PR is visible to + // the logged-in user. + // + // Test requires data fixtures: + // + // private repo, w/ PR in the private repo + // repository.id = 2, user2/repo2 + // PR is issue.ID = 100 + // public repo, public issue + // repository.id = 1, user2/repo1 + // issue.id = 1, user2/repo1#1 + // cross-ref comment from the private PR -> public issue + // comment.id = 3000 + // project board on a public repo + // project.id = 1, owned by repository.id=1 (user2/repo1) + // put public issue on the project board + // project_issue.id = 1 + + test := func(loggedIn bool) map[int64][]*issues_model.Issue { + ctx, recorder := contexttest.MockContext(t, "user2/repo1/projects/1") + if loggedIn { + contexttest.LoadUser(t, ctx, 2) + } + ctx.ContextUser = unittest.AssertExistsAndLoadBean(t, &user.User{ID: 2}) + contexttest.LoadRepo(t, ctx, 1) + ctx.SetParams(":id", "1") + + ViewProject(ctx) + assert.Equal(t, http.StatusOK, recorder.Result().StatusCode) // Verify it's a success response + + // Map issue on the project (16) to array of PRs that reference that issue. + linkedPRs, ok := ctx.Data["LinkedPRs"].(map[int64][]*issues_model.Issue) + require.True(t, ok, "LinkedPRs must be map[int64][]*issues_model.Issue") + + return linkedPRs + } + + t.Run("authorized user with visibility", func(t *testing.T) { + linkedPRs := test(true) + + prList, ok := linkedPRs[1] + require.True(t, ok, "linkedPRs must contain ID=1") + assert.Len(t, prList, 1) + + prIssue := prList[0] + assert.True(t, prIssue.IsPull) + assert.EqualValues(t, 100, prIssue.ID) + }) + + t.Run("anonymous user", func(t *testing.T) { + linkedPRs := test(false) + + prList, ok := linkedPRs[1] + require.True(t, ok, "linkedPRs must contain ID=1") + assert.Empty(t, prList) + }) +} diff --git a/routers/web/user/setting/security/openid.go b/routers/web/user/setting/security/openid.go index 14660e1646..31143f940e 100644 --- a/routers/web/user/setting/security/openid.go +++ b/routers/web/user/setting/security/openid.go @@ -117,7 +117,7 @@ func DeleteOpenID(ctx *context.Context) { // ToggleOpenIDVisibility response for toggle visibility of user's openid func ToggleOpenIDVisibility(ctx *context.Context) { - if err := user_model.ToggleUserOpenIDVisibility(ctx, ctx.FormInt64("id")); err != nil { + if err := user_model.ToggleUserOpenIDVisibility(ctx, ctx.Doer.ID, ctx.FormInt64("id")); err != nil { ctx.ServerError("ToggleUserOpenIDVisibility", err) return } diff --git a/services/lfs/locks.go b/services/lfs/locks.go index 0a9ddb1ee5..b19c235296 100644 --- a/services/lfs/locks.go +++ b/services/lfs/locks.go @@ -20,7 +20,7 @@ import ( "forgejo.org/services/convert" ) -func handleLockListOut(ctx *context.Context, repo *repo_model.Repository, lock *git_model.LFSLock, err error) { +func handleLockListOut(ctx *context.Context, lock *git_model.LFSLock, err error) { if err != nil { if git_model.IsErrLFSLockNotExist(err) { ctx.JSON(http.StatusOK, api.LFSLockList{ @@ -33,12 +33,6 @@ func handleLockListOut(ctx *context.Context, repo *repo_model.Repository, lock * }) return } - if repo.ID != lock.RepoID { - ctx.JSON(http.StatusOK, api.LFSLockList{ - Locks: []*api.LFSLock{}, - }) - return - } ctx.JSON(http.StatusOK, api.LFSLockList{ Locks: []*api.LFSLock{convert.ToLFSLock(ctx, lock)}, }) @@ -90,11 +84,11 @@ func GetListLockHandler(ctx *context.Context) { }) return } - lock, err := git_model.GetLFSLockByID(ctx, v) + lock, err := git_model.GetLFSLockByIDAndRepo(ctx, v, repository.ID) if err != nil && !git_model.IsErrLFSLockNotExist(err) { log.Error("Unable to get lock with ID[%s]: Error: %v", v, err) } - handleLockListOut(ctx, repository, lock, err) + handleLockListOut(ctx, lock, err) return } @@ -104,7 +98,7 @@ func GetListLockHandler(ctx *context.Context) { if err != nil && !git_model.IsErrLFSLockNotExist(err) { log.Error("Unable to get lock for repository %-v with path %s: Error: %v", repository, path, err) } - handleLockListOut(ctx, repository, lock, err) + handleLockListOut(ctx, lock, err) return } diff --git a/tests/integration/api_releases_test.go b/tests/integration/api_releases_test.go index 9f9ed1cf09..972816c5d7 100644 --- a/tests/integration/api_releases_test.go +++ b/tests/integration/api_releases_test.go @@ -288,6 +288,106 @@ func TestAPIReleaseGetDraftByTag(t *testing.T) { assert.NotEmpty(t, err.Message) } +func TestAPIReleaseGet(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + + session := loginUser(t, "user2") + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository) + + t.Run("draft release, no permission", func(t *testing.T) { + rel := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{ + RepoID: repo.ID, + TagName: "draft-release", + }) + assert.True(t, rel.IsDraft) + assert.False(t, rel.IsTag) // wouldn't test the CanWrite(TypeReleases) check for the draft, if this were the case + + req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d", repo.OwnerName, repo.Name, rel.ID)) + resp := MakeRequest(t, req, http.StatusNotFound) + var err *api.APIError + DecodeJSON(t, resp, &err) + assert.NotEmpty(t, err.Message) + }) + + t.Run("draft release, w/ permission", func(t *testing.T) { + rel := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{ + RepoID: repo.ID, + TagName: "draft-release", + }) + req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d", repo.OwnerName, repo.Name, rel.ID)). + AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + var apiRelease *api.Release + DecodeJSON(t, resp, &apiRelease) + assert.Equal(t, rel.TagName, apiRelease.TagName) + }) + + t.Run("published release", func(t *testing.T) { + rel := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{ + RepoID: repo.ID, + TagName: "v1.1", + }) + assert.False(t, rel.IsDraft) + req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d", repo.OwnerName, repo.Name, rel.ID)) + resp := MakeRequest(t, req, http.StatusOK) + var apiRelease *api.Release + DecodeJSON(t, resp, &apiRelease) + assert.Equal(t, rel.TagName, apiRelease.TagName) + }) +} + +func TestAPIReleaseGetAssets(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + + session := loginUser(t, "user2") + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository) + + t.Run("draft release, no permission", func(t *testing.T) { + rel := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{ + RepoID: repo.ID, + TagName: "draft-release", + }) + assert.True(t, rel.IsDraft) + assert.False(t, rel.IsTag) // wouldn't test the CanWrite(TypeReleases) check for the draft, if this were the case + + req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d/assets", repo.OwnerName, repo.Name, rel.ID)) + resp := MakeRequest(t, req, http.StatusNotFound) + var err *api.APIError + DecodeJSON(t, resp, &err) + assert.NotEmpty(t, err.Message) + }) + + t.Run("draft release, w/ permission", func(t *testing.T) { + rel := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{ + RepoID: repo.ID, + TagName: "draft-release", + }) + req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d/assets", repo.OwnerName, repo.Name, rel.ID)). + AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + var attach []*api.Attachment + DecodeJSON(t, resp, &attach) + assert.Empty(t, attach) + }) + + t.Run("published release", func(t *testing.T) { + rel := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{ + RepoID: repo.ID, + TagName: "v1.1", + }) + assert.False(t, rel.IsDraft) + req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d/assets", repo.OwnerName, repo.Name, rel.ID)) + resp := MakeRequest(t, req, http.StatusOK) + var attach []*api.Attachment + DecodeJSON(t, resp, &attach) + assert.Len(t, attach, 1) + }) +} + func TestAPIReleaseDeleteByTagName(t *testing.T) { defer tests.PrepareTestEnv(t)()