From 77fc232e5b4f2e0b7cd39ee3e5d4aa1d550031bd Mon Sep 17 00:00:00 2001 From: Gusted Date: Wed, 29 Jan 2025 09:47:37 +0100 Subject: [PATCH] fix(sec): permission check for project issue - Do an access check when loading issues for a project column, 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 https://github.com/go-gitea/gitea/pull/22865 (cherry picked from commit 2193afaeb9954a5778f5a47aafd0e6fbbf48d000) --- models/issues/issue_project.go | 62 ++++++++++++++++++++++++++++------ models/project/column.go | 14 -------- models/project/issue.go | 14 -------- routers/web/org/projects.go | 15 +++++++- routers/web/repo/projects.go | 15 +++++++- templates/projects/list.tmpl | 4 +-- templates/projects/view.tmpl | 2 +- 7 files changed, 83 insertions(+), 43 deletions(-) diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index 835ea1db52..f606b713cf 100644 --- a/models/issues/issue_project.go +++ b/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,29 @@ func (issue *Issue) ProjectColumnID(ctx context.Context) int64 { } // LoadIssuesFromColumn load issues assigned to this column -func LoadIssuesFromColumn(ctx context.Context, b *project_model.Column) (IssueList, error) { - issueList, err := Issues(ctx, &IssuesOptions{ +func LoadIssuesFromColumn(ctx context.Context, b *project_model.Column, doer *user_model.User, org *org_model.Organization, isClosed optional.Option[bool]) (IssueList, error) { + issueOpts := &IssuesOptions{ ProjectColumnID: 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{ - ProjectColumnID: db.NoConditionID, - ProjectID: b.ProjectID, - SortType: "project-column-sorting", - }) + issueOpts.ProjectColumnID = db.NoConditionID + + issues, err := Issues(ctx, issueOpts) if err != nil { return nil, err } @@ -78,10 +87,10 @@ func LoadIssuesFromColumn(ctx context.Context, b *project_model.Column) (IssueLi } // LoadIssuesFromColumnList load issues assigned to the columns -func LoadIssuesFromColumnList(ctx context.Context, bs project_model.ColumnList) (map[int64]IssueList, error) { +func LoadIssuesFromColumnList(ctx context.Context, bs project_model.ColumnList, 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 := LoadIssuesFromColumn(ctx, bs[i]) + il, err := LoadIssuesFromColumn(ctx, bs[i], doer, org, isClosed) if err != nil { return nil, err } @@ -160,3 +169,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.GetColumns(ctx) + if err != nil { + return 0, err + } + im, err := LoadIssuesFromColumnList(ctx, bs, doer, org, isClosed) + if err != nil { + return 0, err + } + for _, il := range im { + numIssuesInProject += len(il) + } + return numIssuesInProject, nil +} diff --git a/models/project/column.go b/models/project/column.go index 222f448599..f6d6614004 100644 --- a/models/project/column.go +++ b/models/project/column.go @@ -57,20 +57,6 @@ func (Column) TableName() string { return "project_board" // TODO: the legacy table name should be project_column } -// NumIssues return counter of all issues assigned to the column -func (c *Column) NumIssues(ctx context.Context) int { - total, err := db.GetEngine(ctx).Table("project_issue"). - Where("project_id=?", c.ProjectID). - And("project_board_id=?", c.ID). - GroupBy("issue_id"). - Cols("issue_id"). - Count() - if err != nil { - return 0 - } - return int(total) -} - func (c *Column) GetIssues(ctx context.Context) ([]*ProjectIssue, error) { issues := make([]*ProjectIssue, 0, 5) if err := db.GetEngine(ctx).Where("project_id=?", c.ProjectID). diff --git a/models/project/issue.go b/models/project/issue.go index 3361b533b9..984f47ee7c 100644 --- a/models/project/issue.go +++ b/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"). diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index 64d233fc45..6cc2a24c0f 100644 --- a/routers/web/org/projects.go +++ b/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.LoadIssuesFromColumnList(ctx, columns) + issuesMap, err := issues_model.LoadIssuesFromColumnList(ctx, columns, ctx.Doer, ctx.Org.Organization, optional.None[bool]()) if err != nil { ctx.ServerError("LoadIssuesOfColumns", err) return diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index f4b027dae1..5826418b63 100644 --- a/routers/web/repo/projects.go +++ b/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.LoadIssuesFromColumnList(ctx, columns) + issuesMap, err := issues_model.LoadIssuesFromColumnList(ctx, columns, ctx.Doer, nil, optional.None[bool]()) if err != nil { ctx.ServerError("LoadIssuesOfColumns", err) return diff --git a/templates/projects/list.tmpl b/templates/projects/list.tmpl index b892cff996..6139bd4a3f 100644 --- a/templates/projects/list.tmpl +++ b/templates/projects/list.tmpl @@ -49,11 +49,11 @@
{{svg "octicon-issue-opened" 14}} - {{ctx.Locale.PrettyNumber (.NumOpenIssues ctx)}} {{ctx.Locale.Tr "repo.issues.open_title"}} + {{ctx.Locale.PrettyNumber (index $.NumOpenIssuesInProject .ID)}} {{ctx.Locale.Tr "repo.issues.open_title"}}
{{svg "octicon-check" 14}} - {{ctx.Locale.PrettyNumber (.NumClosedIssues ctx)}} {{ctx.Locale.Tr "repo.issues.closed_title"}} + {{ctx.Locale.PrettyNumber (index $.NumClosedIssuesInProject .ID)}} {{ctx.Locale.Tr "repo.issues.closed_title"}}
{{if and $.CanWriteProjects (not $.Repository.IsArchived)}} diff --git a/templates/projects/view.tmpl b/templates/projects/view.tmpl index 08b08b95e5..0e2fc87958 100644 --- a/templates/projects/view.tmpl +++ b/templates/projects/view.tmpl @@ -70,7 +70,7 @@
- {{.NumIssues ctx}} + {{len (index $.IssuesMap .ID)}}
{{.Title}}