[v7.0/forgejo] fix(sec): permission check for project issue (#6846) (merge commit)

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6846
Reviewed-by: 0ko <0ko@noreply.codeberg.org>

models/fixtures/PrivateIssueProjects/project.yml

@@ -0,0 +1,23 @@
+-
+  id: 1001
+  title: Org project that contains private issues
+  owner_id: 3
+  repo_id: 0
+  is_closed: false
+  creator_id: 2
+  board_type: 1
+  type: 3
+  created_unix: 1738000000
+  updated_unix: 1738000000
+
+-
+  id: 1002
+  title: User project that contains private issues
+  owner_id: 2
+  repo_id: 0
+  is_closed: false
+  creator_id: 2
+  board_type: 1
+  type: 1
+  created_unix: 1738000000
+  updated_unix: 1738000000

models/fixtures/PrivateIssueProjects/project_board.yml

@@ -0,0 +1,17 @@
+-
+  id: 1001
+  project_id: 1001
+  title: Triage
+  creator_id: 2
+  default: true
+  created_unix: 1738000000
+  updated_unix: 1738000000
+
+-
+  id: 1002
+  project_id: 1002
+  title: Triage
+  creator_id: 2
+  default: true
+  created_unix: 1738000000
+  updated_unix: 1738000000

models/fixtures/PrivateIssueProjects/project_issue.yml

@@ -0,0 +1,11 @@
+-
+  id: 1001
+  issue_id: 6
+  project_id: 1001
+  project_board_id: 1001
+
+-
+  id: 1002
+  issue_id: 7
+  project_id: 1002
+  project_board_id: 1002

models/fixtures/team_unit.yml

@@ -1,42 +1,49 @@
 -
   id: 1
   team_id: 1
+  org_id: 3
   type: 1
   access_mode: 4
 
 -
   id: 2
   team_id: 1
+  org_id: 3
   type: 2
   access_mode: 4
 
 -
   id: 3
   team_id: 1
+  org_id: 3
   type: 3
   access_mode: 4
 
 -
   id: 4
   team_id: 1
+  org_id: 3
   type: 4
   access_mode: 4
 
 -
   id: 5
   team_id: 1
+  org_id: 3
   type: 5
   access_mode: 4
 
 -
   id: 6
   team_id: 1
+  org_id: 3
   type: 6
   access_mode: 4
 
 -
   id: 7
   team_id: 1
+  org_id: 3
   type: 7
   access_mode: 4
 

models/issues/issue_project.go

@@ -7,8 +7,10 @@ import (
 	"context"
 
 	"code.gitea.io/gitea/models/db"
+	org_model "code.gitea.io/gitea/models/organization"
 	project_model "code.gitea.io/gitea/models/project"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/optional"
 	"code.gitea.io/gitea/modules/util"
 )
 
@@ -48,22 +50,28 @@ func (issue *Issue) ProjectBoardID(ctx context.Context) int64 {
 }
 
 // LoadIssuesFromBoard load issues assigned to this board
-func LoadIssuesFromBoard(ctx context.Context, b *project_model.Board) (IssueList, error) {
-	issueList, err := Issues(ctx, &IssuesOptions{
+func LoadIssuesFromBoard(ctx context.Context, b *project_model.Board, doer *user_model.User, org *org_model.Organization, isClosed optional.Option[bool]) (IssueList, error) {
+	issueOpts := &IssuesOptions{
 		ProjectBoardID: b.ID,
 		ProjectID:      b.ProjectID,
 		SortType:       "project-column-sorting",
-	})
+		IsClosed:       isClosed,
+	}
+	if doer != nil {
+		issueOpts.User = doer
+		issueOpts.Org = org
+	} else {
+		issueOpts.AllPublic = true
+	}
+
+	issueList, err := Issues(ctx, issueOpts)
 	if err != nil {
 		return nil, err
 	}
-
 	if b.Default {
-		issues, err := Issues(ctx, &IssuesOptions{
-			ProjectBoardID: db.NoConditionID,
-			ProjectID:      b.ProjectID,
-			SortType:       "project-column-sorting",
-		})
+		issueOpts.ProjectBoardID = db.NoConditionID
+
+		issues, err := Issues(ctx, issueOpts)
 		if err != nil {
 			return nil, err
 		}
@@ -78,10 +86,10 @@ func LoadIssuesFromBoard(ctx context.Context, b *project_model.Board) (IssueList
 }
 
 // LoadIssuesFromBoardList load issues assigned to the boards
-func LoadIssuesFromBoardList(ctx context.Context, bs project_model.BoardList) (map[int64]IssueList, error) {
+func LoadIssuesFromBoardList(ctx context.Context, bs project_model.BoardList, doer *user_model.User, org *org_model.Organization, isClosed optional.Option[bool]) (map[int64]IssueList, error) {
 	issuesMap := make(map[int64]IssueList, len(bs))
 	for i := range bs {
-		il, err := LoadIssuesFromBoard(ctx, bs[i])
+		il, err := LoadIssuesFromBoard(ctx, bs[i], doer, org, isClosed)
 		if err != nil {
 			return nil, err
 		}
@@ -160,3 +168,36 @@ func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_mo
 		})
 	})
 }
+
+// NumIssuesInProjects returns the amount of issues assigned to one of the project
+// in the list which the doer can access.
+func NumIssuesInProjects(ctx context.Context, pl []*project_model.Project, doer *user_model.User, org *org_model.Organization, isClosed optional.Option[bool]) (map[int64]int, error) {
+	numMap := make(map[int64]int, len(pl))
+	for _, p := range pl {
+		num, err := NumIssuesInProject(ctx, p, doer, org, isClosed)
+		if err != nil {
+			return nil, err
+		}
+		numMap[p.ID] = num
+	}
+
+	return numMap, nil
+}
+
+// NumIssuesInProject returns the amount of issues assigned to the project which
+// the doer can access.
+func NumIssuesInProject(ctx context.Context, p *project_model.Project, doer *user_model.User, org *org_model.Organization, isClosed optional.Option[bool]) (int, error) {
+	numIssuesInProject := int(0)
+	bs, err := p.GetBoards(ctx)
+	if err != nil {
+		return 0, err
+	}
+	im, err := LoadIssuesFromBoardList(ctx, bs, doer, org, isClosed)
+	if err != nil {
+		return 0, err
+	}
+	for _, il := range im {
+		numIssuesInProject += len(il)
+	}
+	return numIssuesInProject, nil
+}

models/issues/issue_project_test.go

@@ -0,0 +1,100 @@
+// Copyright 2025 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package issues_test
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/models/issues"
+	"code.gitea.io/gitea/models/organization"
+	"code.gitea.io/gitea/models/project"
+	"code.gitea.io/gitea/models/unittest"
+	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/optional"
+	"code.gitea.io/gitea/tests"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPrivateIssueProjects(t *testing.T) {
+	defer tests.AddFixtures("models/fixtures/PrivateIssueProjects/")()
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	t.Run("Organization project", func(t *testing.T) {
+		org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3})
+		orgProject := unittest.AssertExistsAndLoadBean(t, &project.Project{ID: 1001, OwnerID: org.ID})
+		board := unittest.AssertExistsAndLoadBean(t, &project.Board{ID: 1001, ProjectID: orgProject.ID})
+
+		t.Run("Authenticated user", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+			issueList, err := issues.LoadIssuesFromBoard(db.DefaultContext, board, user2, org, optional.None[bool]())
+			require.NoError(t, err)
+			assert.Len(t, issueList, 1)
+			assert.EqualValues(t, 6, issueList[0].ID)
+
+			issuesNum, err := issues.NumIssuesInProject(db.DefaultContext, orgProject, user2, org, optional.None[bool]())
+			require.NoError(t, err)
+			assert.EqualValues(t, 1, issuesNum)
+
+			issuesNum, err = issues.NumIssuesInProject(db.DefaultContext, orgProject, user2, org, optional.Some(true))
+			require.NoError(t, err)
+			assert.EqualValues(t, 0, issuesNum)
+
+			issuesNum, err = issues.NumIssuesInProject(db.DefaultContext, orgProject, user2, org, optional.Some(false))
+			require.NoError(t, err)
+			assert.EqualValues(t, 1, issuesNum)
+		})
+
+		t.Run("Anonymous user", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+			issueList, err := issues.LoadIssuesFromBoard(db.DefaultContext, board, nil, org, optional.None[bool]())
+			require.NoError(t, err)
+			assert.Empty(t, issueList)
+
+			issuesNum, err := issues.NumIssuesInProject(db.DefaultContext, orgProject, nil, org, optional.None[bool]())
+			require.NoError(t, err)
+			assert.EqualValues(t, 0, issuesNum)
+		})
+	})
+
+	t.Run("User project", func(t *testing.T) {
+		userProject := unittest.AssertExistsAndLoadBean(t, &project.Project{ID: 1002, OwnerID: user2.ID})
+		board := unittest.AssertExistsAndLoadBean(t, &project.Board{ID: 1002, ProjectID: userProject.ID})
+
+		t.Run("Authenticated user", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+			issueList, err := issues.LoadIssuesFromBoard(db.DefaultContext, board, user2, nil, optional.None[bool]())
+			require.NoError(t, err)
+			assert.Len(t, issueList, 1)
+			assert.EqualValues(t, 7, issueList[0].ID)
+
+			issuesNum, err := issues.NumIssuesInProject(db.DefaultContext, userProject, user2, nil, optional.None[bool]())
+			require.NoError(t, err)
+			assert.EqualValues(t, 1, issuesNum)
+
+			issuesNum, err = issues.NumIssuesInProject(db.DefaultContext, userProject, user2, nil, optional.Some(true))
+			require.NoError(t, err)
+			assert.EqualValues(t, 0, issuesNum)
+
+			issuesNum, err = issues.NumIssuesInProject(db.DefaultContext, userProject, user2, nil, optional.Some(false))
+			require.NoError(t, err)
+			assert.EqualValues(t, 1, issuesNum)
+		})
+
+		t.Run("Anonymous user", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			issueList, err := issues.LoadIssuesFromBoard(db.DefaultContext, board, nil, nil, optional.None[bool]())
+			require.NoError(t, err)
+			assert.Empty(t, issueList)
+
+			issuesNum, err := issues.NumIssuesInProject(db.DefaultContext, userProject, nil, nil, optional.None[bool]())
+			require.NoError(t, err)
+			assert.EqualValues(t, 0, issuesNum)
+		})
+	})
+}

models/project/board.go

@@ -70,20 +70,6 @@ func (Board) TableName() string {
 	return "project_board"
 }
 
-// NumIssues return counter of all issues assigned to the board
-func (b *Board) NumIssues(ctx context.Context) int {
-	c, err := db.GetEngine(ctx).Table("project_issue").
-		Where("project_id=?", b.ProjectID).
-		And("project_board_id=?", b.ID).
-		GroupBy("issue_id").
-		Cols("issue_id").
-		Count()
-	if err != nil {
-		return 0
-	}
-	return int(c)
-}
-
 func (b *Board) GetIssues(ctx context.Context) ([]*ProjectIssue, error) {
 	issues := make([]*ProjectIssue, 0, 5)
 	if err := db.GetEngine(ctx).Where("project_id=?", b.ProjectID).

models/project/issue.go

@@ -34,20 +34,6 @@ func deleteProjectIssuesByProjectID(ctx context.Context, projectID int64) error
 	return err
 }
 
-// NumIssues return counter of all issues assigned to a project
-func (p *Project) NumIssues(ctx context.Context) int {
-	c, err := db.GetEngine(ctx).Table("project_issue").
-		Where("project_id=?", p.ID).
-		GroupBy("issue_id").
-		Cols("issue_id").
-		Count()
-	if err != nil {
-		log.Error("NumIssues: %v", err)
-		return 0
-	}
-	return int(c)
-}
-
 // NumClosedIssues return counter of closed issues assigned to a project
 func (p *Project) NumClosedIssues(ctx context.Context) int {
 	c, err := db.GetEngine(ctx).Table("project_issue").

routers/web/org/projects.go

@@ -126,6 +126,19 @@ func Projects(ctx *context.Context) {
 	ctx.Data["PageIsViewProjects"] = true
 	ctx.Data["SortType"] = sortType
 
+	numOpenIssues, err := issues_model.NumIssuesInProjects(ctx, projects, ctx.Doer, ctx.Org.Organization, optional.Some(false))
+	if err != nil {
+		ctx.ServerError("NumIssuesInProjects", err)
+		return
+	}
+	numClosedIssues, err := issues_model.NumIssuesInProjects(ctx, projects, ctx.Doer, ctx.Org.Organization, optional.Some(true))
+	if err != nil {
+		ctx.ServerError("NumIssuesInProjects", err)
+		return
+	}
+	ctx.Data["NumOpenIssuesInProject"] = numOpenIssues
+	ctx.Data["NumClosedIssuesInProject"] = numClosedIssues
+
 	ctx.HTML(http.StatusOK, tplProjects)
 }
 
@@ -332,7 +345,7 @@ func ViewProject(ctx *context.Context) {
 		return
 	}
 
-	issuesMap, err := issues_model.LoadIssuesFromBoardList(ctx, boards)
+	issuesMap, err := issues_model.LoadIssuesFromBoardList(ctx, boards, ctx.Doer, ctx.Org.Organization, optional.None[bool]())
 	if err != nil {
 		ctx.ServerError("LoadIssuesOfBoards", err)
 		return

routers/web/repo/projects.go

@@ -125,6 +125,19 @@ func Projects(ctx *context.Context) {
 	ctx.Data["IsProjectsPage"] = true
 	ctx.Data["SortType"] = sortType
 
+	numOpenIssues, err := issues_model.NumIssuesInProjects(ctx, projects, ctx.Doer, ctx.Org.Organization, optional.Some(false))
+	if err != nil {
+		ctx.ServerError("NumIssuesInProjects", err)
+		return
+	}
+	numClosedIssues, err := issues_model.NumIssuesInProjects(ctx, projects, ctx.Doer, ctx.Org.Organization, optional.Some(true))
+	if err != nil {
+		ctx.ServerError("NumIssuesInProjects", err)
+		return
+	}
+	ctx.Data["NumOpenIssuesInProject"] = numOpenIssues
+	ctx.Data["NumClosedIssuesInProject"] = numClosedIssues
+
 	ctx.HTML(http.StatusOK, tplProjects)
 }
 
@@ -310,7 +323,7 @@ func ViewProject(ctx *context.Context) {
 		return
 	}
 
-	issuesMap, err := issues_model.LoadIssuesFromBoardList(ctx, boards)
+	issuesMap, err := issues_model.LoadIssuesFromBoardList(ctx, boards, ctx.Doer, nil, optional.None[bool]())
 	if err != nil {
 		ctx.ServerError("LoadIssuesOfBoards", err)
 		return

templates/projects/list.tmpl

@@ -49,11 +49,11 @@
 				<div class="group">
 					<div class="flex-text-block">
 						{{svg "octicon-issue-opened" 14}}
-						{{ctx.Locale.PrettyNumber (.NumOpenIssues ctx)}}&nbsp;{{ctx.Locale.Tr "repo.issues.open_title"}}
+						{{ctx.Locale.PrettyNumber (index $.NumOpenIssuesInProject .ID)}}&nbsp;{{ctx.Locale.Tr "repo.issues.open_title"}}
 					</div>
 					<div class="flex-text-block">
 						{{svg "octicon-check" 14}}
-						{{ctx.Locale.PrettyNumber (.NumClosedIssues ctx)}}&nbsp;{{ctx.Locale.Tr "repo.issues.closed_title"}}
+						{{ctx.Locale.PrettyNumber (index $.NumClosedIssuesInProject .ID)}}&nbsp;{{ctx.Locale.Tr "repo.issues.closed_title"}}
 					</div>
 				</div>
 				{{if and $.CanWriteProjects (not $.Repository.IsArchived)}}

templates/projects/view.tmpl

@@ -70,7 +70,7 @@
 				<div class="project-column-header{{if $canWriteProject}} tw-cursor-grab{{end}}">
 					<div class="ui large label project-column-title tw-py-1">
 						<div class="ui small circular grey label project-column-issue-count">
-							{{.NumIssues ctx}}
+							{{len (index $.IssuesMap .ID)}}
 						</div>
 						<span class="project-column-title-label">{{.Title}}</span>
 					</div>

tests/integration/private_project_test.go

@@ -0,0 +1,84 @@
+// Copyright 2025 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package integration
+
+import (
+	"net/http"
+	"strings"
+	"testing"
+
+	org_model "code.gitea.io/gitea/models/organization"
+	project_model "code.gitea.io/gitea/models/project"
+	"code.gitea.io/gitea/models/unittest"
+	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/tests"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPrivateIssueProject(t *testing.T) {
+	defer tests.AddFixtures("models/fixtures/PrivateIssueProjects/")()
+	defer tests.PrepareTestEnv(t)()
+
+	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	sess := loginUser(t, user2.Name)
+
+	test := func(t *testing.T, sess *TestSession, username string, projectID int64, hasAccess bool) {
+		t.Helper()
+		defer tests.PrintCurrentTest(t, 1)()
+
+		// Test that the projects overview page shows the correct open and close issues.
+		req := NewRequestf(t, "GET", "%s/-/projects", username)
+		resp := sess.MakeRequest(t, req, http.StatusOK)
+
+		htmlDoc := NewHTMLParser(t, resp.Body)
+		openCloseStats := htmlDoc.Find(".milestone-toolbar .group").First().Text()
+		if hasAccess {
+			assert.Contains(t, openCloseStats, "1\u00a0Open")
+		} else {
+			assert.Contains(t, openCloseStats, "0\u00a0Open")
+		}
+		assert.Contains(t, openCloseStats, "0\u00a0Closed")
+
+		// Check that on the project itself the issue is not shown.
+		req = NewRequestf(t, "GET", "%s/-/projects/%d", username, projectID)
+		resp = sess.MakeRequest(t, req, http.StatusOK)
+
+		htmlDoc = NewHTMLParser(t, resp.Body)
+		htmlDoc.AssertElement(t, ".project-column .issue-card", hasAccess)
+
+		// And that the issue count is correct.
+		issueCount := strings.TrimSpace(htmlDoc.Find(".project-column-issue-count").Text())
+		if hasAccess {
+			assert.EqualValues(t, "1", issueCount)
+		} else {
+			assert.EqualValues(t, "0", issueCount)
+		}
+	}
+
+	t.Run("Organization project", func(t *testing.T) {
+		org := unittest.AssertExistsAndLoadBean(t, &org_model.Organization{ID: 3})
+		orgProject := unittest.AssertExistsAndLoadBean(t, &project_model.Project{ID: 1001, OwnerID: org.ID})
+
+		t.Run("Authenticated user", func(t *testing.T) {
+			test(t, sess, org.Name, orgProject.ID, true)
+		})
+
+		t.Run("Anonymous user", func(t *testing.T) {
+			test(t, emptyTestSession(t), org.Name, orgProject.ID, false)
+		})
+	})
+
+	t.Run("User project", func(t *testing.T) {
+		userProject := unittest.AssertExistsAndLoadBean(t, &project_model.Project{ID: 1002, OwnerID: user2.ID})
+
+		t.Run("Authenticated user", func(t *testing.T) {
+			test(t, sess, user2.Name, userProject.ID, true)
+		})
+
+		t.Run("Anonymous user", func(t *testing.T) {
+			test(t, emptyTestSession(t), user2.Name, userProject.ID, false)
+		})
+	})
+}