From c62e07f18a8d0e142424777cac4ec603c5cc022d Mon Sep 17 00:00:00 2001 From: silverwind Date: Sun, 1 Nov 2026 10:46:53 +0000 Subject: [PATCH] fix(projects): only require project write to move board cards (#40161) Follow-up to https://github.com/go-gitea/gitea/pull/30205, per https://github.com/go-gitea/gitea/pull/30205#discussion_r4435046605. Moving a card on a project board is a project operation, so project write is enough again. Repository boards already show every card to project writers, who then got a 404 when moving one. The per-card archived check is removed too. The frontend sends every card of the target column, so one card from an archived repository rejected all drops and reorders into that column, silently. Project workflows already skip archived repositories, and an archived repository's own board is read-only. --------- Co-authored-by: wxiaoguang --- models/unit/unit.go | 4 +- routers/web/shared/project/project.go | 19 ------ routers/web/shared/project/project_test.go | 73 ++++++---------------- 3 files changed, 22 insertions(+), 74 deletions(-) diff --git a/models/unit/unit.go b/models/unit/unit.go index d8d8a4d4202..e39d0d0787c 100644 --- a/models/unit/unit.go +++ b/models/unit/unit.go @@ -30,8 +30,8 @@ const ( TypeWiki // 5 Wiki TypeExternalWiki // 6 ExternalWiki TypeExternalTracker // 7 ExternalTracker - TypeProjects // 8 Projects - TypePackages // 9 Packages + TypeProjects // 8 Projects: implies that the doer can read some information from issue & PR, org-level is not well-designed + TypePackages // 9 Packages: for most cases, it is for owner-level (not well-designed though) TypeActions // 10 Actions ) diff --git a/routers/web/shared/project/project.go b/routers/web/shared/project/project.go index 0dc31db00e6..599544f427e 100644 --- a/routers/web/shared/project/project.go +++ b/routers/web/shared/project/project.go @@ -5,10 +5,8 @@ package project import ( "errors" - "net/http" issues_model "gitea.dev/models/issues" - access_model "gitea.dev/models/perm/access" project_model "gitea.dev/models/project" "gitea.dev/modules/json" "gitea.dev/modules/web" @@ -191,28 +189,11 @@ func MoveIssues(ctx *context.Context) { return } - repoPerms := map[int64]access_model.Permission{} for _, issue := range movedIssues { if !project.CanBeAccessedByOwnerRepo(issue.Repo.OwnerID, issue.Repo) { ctx.ServerError("Some issue's repoID is not equal to project's repoID", errors.New("Some issue's repoID is not equal to project's repoID")) return } - perm, ok := repoPerms[issue.RepoID] - if !ok { - if perm, err = access_model.GetDoerRepoPermission(ctx, issue.Repo, ctx.Doer); err != nil { - ctx.ServerError("GetDoerRepoPermission", err) - return - } - repoPerms[issue.RepoID] = perm - } - if !perm.CanReadIssuesOrPulls(issue.IsPull) { - ctx.NotFound(nil) - return - } - if issue.Repo.IsArchived { - ctx.HTTPError(http.StatusLocked, "repo is archived") - return - } } if err = project_service.MoveIssuesOnProjectColumn(ctx, ctx.Doer, column, sortedIssueIDs); err != nil { diff --git a/routers/web/shared/project/project_test.go b/routers/web/shared/project/project_test.go index 4061a12bd59..33c5eb0bc8b 100644 --- a/routers/web/shared/project/project_test.go +++ b/routers/web/shared/project/project_test.go @@ -11,13 +11,9 @@ import ( "strings" "testing" - "gitea.dev/models/db" issues_model "gitea.dev/models/issues" - "gitea.dev/models/organization" - "gitea.dev/models/perm" project_model "gitea.dev/models/project" repo_model "gitea.dev/models/repo" - "gitea.dev/models/unit" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" "gitea.dev/services/contexttest" @@ -72,54 +68,25 @@ func TestFindColumn(t *testing.T) { } } -func TestMoveIssuesNeedsIssueRead(t *testing.T) { - for _, tc := range []struct { - name string - doerID int64 - repoBoard bool - archived bool - status int - }{ - {"OwnerBoardReader", 2, false, false, http.StatusOK}, - {"OwnerBoardWithoutIssueAccess", 15, false, false, http.StatusNotFound}, - {"OwnerBoardArchivedRepo", 2, false, true, http.StatusLocked}, - {"RepoBoardProjectsWriterWithoutIssueRead", 4, true, false, http.StatusNotFound}, - } { - t.Run(tc.name, func(t *testing.T) { - unittest.PrepareTestEnv(t) - if tc.repoBoard { - _, err := db.Exec(t.Context(), "UPDATE team SET authorize = ? WHERE id = ?", perm.AccessModeNone, 2) - require.NoError(t, err) - _, err = db.DeleteByBean(t.Context(), &organization.TeamUnit{TeamID: 2, Type: unit.TypeIssues}) - require.NoError(t, err) - } - if tc.archived { - require.NoError(t, repo_model.UpdateRepositoryColsNoAutoTime(t.Context(), &repo_model.Repository{ID: 3, IsArchived: true}, "is_archived")) - } - ctx, _ := contexttest.MockContext(t, "POST /org3/-/projects") - contexttest.LoadUser(t, ctx, tc.doerID) - project := unittest.AssertExistsAndLoadBean(t, &project_model.Project{ID: 2}) - if tc.repoBoard { - contexttest.LoadRepo(t, ctx, 3) - } else { - ctx.ContextUser = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) - project = &project_model.Project{Title: "board", OwnerID: 3, CreatorID: 2, Type: project_model.TypeOrganization} - require.NoError(t, project_model.NewProject(ctx, project)) - require.NoError(t, project_model.NewColumn(ctx, &project_model.Column{Title: "Backlog", ProjectID: project.ID, Default: true})) - } - column := &project_model.Column{Title: "Done", ProjectID: project.ID} - require.NoError(t, project_model.NewColumn(ctx, column)) - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 6}) - _, _, err := issues_model.IssueAssignOrRemoveProject(ctx, issue, unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}), []int64{project.ID}) - require.NoError(t, err) - ctx.SetPathParam("id", strconv.FormatInt(project.ID, 10)) - ctx.SetPathParam("columnID", strconv.FormatInt(column.ID, 10)) - ctx.Req.Body = io.NopCloser(strings.NewReader(fmt.Sprintf(`{"issues":[{"issueID":%d,"sorting":0}]}`, issue.ID))) +func TestMoveIssuesNeedsOnlyProjectWrite(t *testing.T) { + unittest.PrepareTestEnv(t) + require.NoError(t, repo_model.UpdateRepositoryColsNoAutoTime(t.Context(), &repo_model.Repository{ID: 3, IsArchived: true}, "is_archived")) + ctx, _ := contexttest.MockContext(t, "POST /org3/-/projects") + contexttest.LoadUser(t, ctx, 15) + ctx.ContextUser = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) + project := &project_model.Project{Title: "board", OwnerID: 3, CreatorID: 2, Type: project_model.TypeOrganization} + require.NoError(t, project_model.NewProject(ctx, project)) + require.NoError(t, project_model.NewColumn(ctx, &project_model.Column{Title: "Backlog", ProjectID: project.ID, Default: true})) + column := &project_model.Column{Title: "Done", ProjectID: project.ID} + require.NoError(t, project_model.NewColumn(ctx, column)) + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 6}) + _, _, err := issues_model.IssueAssignOrRemoveProject(ctx, issue, unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}), []int64{project.ID}) + require.NoError(t, err) + ctx.SetPathParam("id", strconv.FormatInt(project.ID, 10)) + ctx.SetPathParam("columnID", strconv.FormatInt(column.ID, 10)) + ctx.Req.Body = io.NopCloser(strings.NewReader(fmt.Sprintf(`{"issues":[{"issueID":%d,"sorting":0}]}`, issue.ID))) - MoveIssues(ctx) - assert.Equal(t, tc.status, ctx.Resp.WrittenStatus()) - card := unittest.AssertExistsAndLoadBean(t, &project_model.ProjectIssue{IssueID: issue.ID, ProjectID: project.ID}) - assert.Equal(t, tc.status == http.StatusOK, card.ProjectColumnID == column.ID) - }) - } + MoveIssues(ctx) + assert.Equal(t, http.StatusOK, ctx.Resp.WrittenStatus()) + unittest.AssertExistsAndLoadBean(t, &project_model.ProjectIssue{IssueID: issue.ID, ProjectID: project.ID, ProjectColumnID: column.ID}) }