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 <wxiaoguang@gmail.com>

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
 )
 

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 {

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})
 }