Skip to content

Commit 913e3b5

Browse files
committed
fix(sec): permission check for project issue
- Do an access check when loading issues for a project board, currently this is not done and exposes the title, labels and existence of a private issue that the viewer of the project board may not have access to. - The number of issues cannot be calculated in a efficient manner and stored in the database because their number may vary depending on the visibility of the repositories participating in the project. The previous implementation used the pre-calculated numbers stored in each project, which did not reflect that potential variation. - The code is derived from go-gitea/gitea#22865 (cherry picked from commit 2193afaeb9954a5778f5a47aafd0e6fbbf48d000)
1 parent 0f1cf6d commit 913e3b5

File tree

7 files changed

+83
-44
lines changed

7 files changed

+83
-44
lines changed

models/issues/issue_project.go

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,10 @@ import (
77
"context"
88

99
"code.gitea.io/gitea/models/db"
10+
org_model "code.gitea.io/gitea/models/organization"
1011
project_model "code.gitea.io/gitea/models/project"
1112
user_model "code.gitea.io/gitea/models/user"
13+
"code.gitea.io/gitea/modules/optional"
1214
"code.gitea.io/gitea/modules/util"
1315
)
1416

@@ -48,22 +50,28 @@ func (issue *Issue) ProjectBoardID(ctx context.Context) int64 {
4850
}
4951

5052
// LoadIssuesFromBoard load issues assigned to this board
51-
func LoadIssuesFromBoard(ctx context.Context, b *project_model.Board) (IssueList, error) {
52-
issueList, err := Issues(ctx, &IssuesOptions{
53+
func LoadIssuesFromBoard(ctx context.Context, b *project_model.Board, doer *user_model.User, org *org_model.Organization, isClosed optional.Option[bool]) (IssueList, error) {
54+
issueOpts := &IssuesOptions{
5355
ProjectBoardID: b.ID,
5456
ProjectID: b.ProjectID,
5557
SortType: "project-column-sorting",
56-
})
58+
IsClosed: isClosed,
59+
}
60+
if doer != nil {
61+
issueOpts.User = doer
62+
issueOpts.Org = org
63+
} else {
64+
issueOpts.AllPublic = true
65+
}
66+
67+
issueList, err := Issues(ctx, issueOpts)
5768
if err != nil {
5869
return nil, err
5970
}
60-
6171
if b.Default {
62-
issues, err := Issues(ctx, &IssuesOptions{
63-
ProjectBoardID: db.NoConditionID,
64-
ProjectID: b.ProjectID,
65-
SortType: "project-column-sorting",
66-
})
72+
issueOpts.ProjectBoardID = db.NoConditionID
73+
74+
issues, err := Issues(ctx, issueOpts)
6775
if err != nil {
6876
return nil, err
6977
}
@@ -78,10 +86,10 @@ func LoadIssuesFromBoard(ctx context.Context, b *project_model.Board) (IssueList
7886
}
7987

8088
// LoadIssuesFromBoardList load issues assigned to the boards
81-
func LoadIssuesFromBoardList(ctx context.Context, bs project_model.BoardList) (map[int64]IssueList, error) {
89+
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) {
8290
issuesMap := make(map[int64]IssueList, len(bs))
8391
for i := range bs {
84-
il, err := LoadIssuesFromBoard(ctx, bs[i])
92+
il, err := LoadIssuesFromBoard(ctx, bs[i], doer, org, isClosed)
8593
if err != nil {
8694
return nil, err
8795
}
@@ -160,3 +168,36 @@ func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_mo
160168
})
161169
})
162170
}
171+
172+
// NumIssuesInProjects returns the amount of issues assigned to one of the project
173+
// in the list which the doer can access.
174+
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) {
175+
numMap := make(map[int64]int, len(pl))
176+
for _, p := range pl {
177+
num, err := NumIssuesInProject(ctx, p, doer, org, isClosed)
178+
if err != nil {
179+
return nil, err
180+
}
181+
numMap[p.ID] = num
182+
}
183+
184+
return numMap, nil
185+
}
186+
187+
// NumIssuesInProject returns the amount of issues assigned to the project which
188+
// the doer can access.
189+
func NumIssuesInProject(ctx context.Context, p *project_model.Project, doer *user_model.User, org *org_model.Organization, isClosed optional.Option[bool]) (int, error) {
190+
numIssuesInProject := int(0)
191+
bs, err := p.GetBoards(ctx)
192+
if err != nil {
193+
return 0, err
194+
}
195+
im, err := LoadIssuesFromBoardList(ctx, bs, doer, org, isClosed)
196+
if err != nil {
197+
return 0, err
198+
}
199+
for _, il := range im {
200+
numIssuesInProject += len(il)
201+
}
202+
return numIssuesInProject, nil
203+
}

models/project/board.go

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -70,20 +70,6 @@ func (Board) TableName() string {
7070
return "project_board"
7171
}
7272

73-
// NumIssues return counter of all issues assigned to the board
74-
func (b *Board) NumIssues(ctx context.Context) int {
75-
c, err := db.GetEngine(ctx).Table("project_issue").
76-
Where("project_id=?", b.ProjectID).
77-
And("project_board_id=?", b.ID).
78-
GroupBy("issue_id").
79-
Cols("issue_id").
80-
Count()
81-
if err != nil {
82-
return 0
83-
}
84-
return int(c)
85-
}
86-
8773
func (b *Board) GetIssues(ctx context.Context) ([]*ProjectIssue, error) {
8874
issues := make([]*ProjectIssue, 0, 5)
8975
if err := db.GetEngine(ctx).Where("project_id=?", b.ProjectID).

models/project/issue.go

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,20 +34,6 @@ func deleteProjectIssuesByProjectID(ctx context.Context, projectID int64) error
3434
return err
3535
}
3636

37-
// NumIssues return counter of all issues assigned to a project
38-
func (p *Project) NumIssues(ctx context.Context) int {
39-
c, err := db.GetEngine(ctx).Table("project_issue").
40-
Where("project_id=?", p.ID).
41-
GroupBy("issue_id").
42-
Cols("issue_id").
43-
Count()
44-
if err != nil {
45-
log.Error("NumIssues: %v", err)
46-
return 0
47-
}
48-
return int(c)
49-
}
50-
5137
// NumClosedIssues return counter of closed issues assigned to a project
5238
func (p *Project) NumClosedIssues(ctx context.Context) int {
5339
c, err := db.GetEngine(ctx).Table("project_issue").

routers/web/org/projects.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,19 @@ func Projects(ctx *context.Context) {
126126
ctx.Data["PageIsViewProjects"] = true
127127
ctx.Data["SortType"] = sortType
128128

129+
numOpenIssues, err := issues_model.NumIssuesInProjects(ctx, projects, ctx.Doer, ctx.Org.Organization, optional.Some(false))
130+
if err != nil {
131+
ctx.ServerError("NumIssuesInProjects", err)
132+
return
133+
}
134+
numClosedIssues, err := issues_model.NumIssuesInProjects(ctx, projects, ctx.Doer, ctx.Org.Organization, optional.Some(true))
135+
if err != nil {
136+
ctx.ServerError("NumIssuesInProjects", err)
137+
return
138+
}
139+
ctx.Data["NumOpenIssuesInProject"] = numOpenIssues
140+
ctx.Data["NumClosedIssuesInProject"] = numClosedIssues
141+
129142
ctx.HTML(http.StatusOK, tplProjects)
130143
}
131144

@@ -332,7 +345,7 @@ func ViewProject(ctx *context.Context) {
332345
return
333346
}
334347

335-
issuesMap, err := issues_model.LoadIssuesFromBoardList(ctx, boards)
348+
issuesMap, err := issues_model.LoadIssuesFromBoardList(ctx, boards, ctx.Doer, ctx.Org.Organization, optional.None[bool]())
336349
if err != nil {
337350
ctx.ServerError("LoadIssuesOfBoards", err)
338351
return

routers/web/repo/projects.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,19 @@ func Projects(ctx *context.Context) {
125125
ctx.Data["IsProjectsPage"] = true
126126
ctx.Data["SortType"] = sortType
127127

128+
numOpenIssues, err := issues_model.NumIssuesInProjects(ctx, projects, ctx.Doer, ctx.Org.Organization, optional.Some(false))
129+
if err != nil {
130+
ctx.ServerError("NumIssuesInProjects", err)
131+
return
132+
}
133+
numClosedIssues, err := issues_model.NumIssuesInProjects(ctx, projects, ctx.Doer, ctx.Org.Organization, optional.Some(true))
134+
if err != nil {
135+
ctx.ServerError("NumIssuesInProjects", err)
136+
return
137+
}
138+
ctx.Data["NumOpenIssuesInProject"] = numOpenIssues
139+
ctx.Data["NumClosedIssuesInProject"] = numClosedIssues
140+
128141
ctx.HTML(http.StatusOK, tplProjects)
129142
}
130143

@@ -310,7 +323,7 @@ func ViewProject(ctx *context.Context) {
310323
return
311324
}
312325

313-
issuesMap, err := issues_model.LoadIssuesFromBoardList(ctx, boards)
326+
issuesMap, err := issues_model.LoadIssuesFromBoardList(ctx, boards, ctx.Doer, nil, optional.None[bool]())
314327
if err != nil {
315328
ctx.ServerError("LoadIssuesOfBoards", err)
316329
return

templates/projects/list.tmpl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,11 @@
4949
<div class="group">
5050
<div class="flex-text-block">
5151
{{svg "octicon-issue-opened" 14}}
52-
{{ctx.Locale.PrettyNumber (.NumOpenIssues ctx)}}&nbsp;{{ctx.Locale.Tr "repo.issues.open_title"}}
52+
{{ctx.Locale.PrettyNumber (index $.NumOpenIssuesInProject .ID)}}&nbsp;{{ctx.Locale.Tr "repo.issues.open_title"}}
5353
</div>
5454
<div class="flex-text-block">
5555
{{svg "octicon-check" 14}}
56-
{{ctx.Locale.PrettyNumber (.NumClosedIssues ctx)}}&nbsp;{{ctx.Locale.Tr "repo.issues.closed_title"}}
56+
{{ctx.Locale.PrettyNumber (index $.NumClosedIssuesInProject .ID)}}&nbsp;{{ctx.Locale.Tr "repo.issues.closed_title"}}
5757
</div>
5858
</div>
5959
{{if and $.CanWriteProjects (not $.Repository.IsArchived)}}

templates/projects/view.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@
7070
<div class="project-column-header{{if $canWriteProject}} tw-cursor-grab{{end}}">
7171
<div class="ui large label project-column-title tw-py-1">
7272
<div class="ui small circular grey label project-column-issue-count">
73-
{{.NumIssues ctx}}
73+
{{len (index $.IssuesMap .ID)}}
7474
</div>
7575
<span class="project-column-title-label">{{.Title}}</span>
7676
</div>

0 commit comments

Comments
 (0)