From 05ec3f1ee94baef8065d157a6613782df24d0b9a Mon Sep 17 00:00:00 2001 From: Gusted Date: Thu, 18 Nov 2021 12:55:24 +0100 Subject: [PATCH 01/71] WIP --- models/issue.go | 12 ++++++++++++ models/repo_permission.go | 5 +++++ options/locale/locale_en-US.ini | 1 + routers/web/repo/issue.go | 4 ++++ templates/repo/issue/new_form.tmpl | 1 + 5 files changed, 23 insertions(+) diff --git a/models/issue.go b/models/issue.go index 288163707a466..9240186e29827 100644 --- a/models/issue.go +++ b/models/issue.go @@ -69,6 +69,8 @@ type Issue struct { // IsLocked limits commenting abilities to users on an issue // with write access IsLocked bool `xorm:"NOT NULL DEFAULT false"` + // IsPrivate limits who can see the issue. + IsPrivate bool `xorm:"NOT NULL DEFAULT false"` // For view issue page. ShowRole RoleDescriptor `xorm:"-"` @@ -1154,6 +1156,7 @@ type IssuesOptions struct { // prioritize issues from this repo PriorityRepoID int64 IsArchived util.OptionalBool + CanSeePrivate bool } // sortIssuesSession sort an issues-related session based on the provided @@ -1259,6 +1262,10 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { } } + if !opts.CanSeePrivate { + sess.And("issue.is_private=?", false) + } + switch opts.IsPull { case util.OptionalBoolTrue: sess.And("issue.is_pull=?", true) @@ -1496,6 +1503,7 @@ type IssueStatsOptions struct { ReviewRequestedID int64 IsPull util.OptionalBool IssueIDs []int64 + CanSeePrivate bool } const ( @@ -1583,6 +1591,10 @@ func getIssueStatsChunk(opts *IssueStatsOptions, issueIDs []int64) (*IssueStats, applyReviewRequestedCondition(sess, opts.ReviewRequestedID) } + if !opts.CanSeePrivate { + sess.And("issue.is_private=?", false) + } + switch opts.IsPull { case util.OptionalBoolTrue: sess.And("issue.is_pull=?", true) diff --git a/models/repo_permission.go b/models/repo_permission.go index 3db7200470581..031e08a35c701 100644 --- a/models/repo_permission.go +++ b/models/repo_permission.go @@ -98,6 +98,11 @@ func (p *Permission) CanWriteIssuesOrPulls(isPull bool) bool { return p.CanWrite(unit.TypeIssues) } +// CanSeePrivateIssues returns true if the user is allowed to see private issues on the repo. +func (p *Permission) CanSeePrivateIssues() bool { + return p.CanWrite(unit.TypeIssues) +} + // ColorFormat writes a colored string for these Permissions func (p *Permission) ColorFormat(s fmt.State) { noColor := log.ColorBytes(log.Reset) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 66996b2014567..1f27d854acd87 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1395,6 +1395,7 @@ issues.content_history.created = created issues.content_history.delete_from_history = Delete from history issues.content_history.delete_from_history_confirm = Delete from history? issues.content_history.options = Options +issues.confidential = Confidential compare.compare_base = base compare.compare_head = compare diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 95363258e9d55..2324cfd72a5f5 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -169,6 +169,8 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti } } + canSeePrivateIssues := ctx.Repo.CanSeePrivateIssues() + var issueStats *models.IssueStats if forceEmpty { issueStats = &models.IssueStats{} @@ -183,6 +185,7 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti ReviewRequestedID: reviewRequestedID, IsPull: isPullOption, IssueIDs: issueIDs, + CanSeePrivate: canSeePrivateIssues, }) if err != nil { ctx.ServerError("GetIssueStats", err) @@ -235,6 +238,7 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti LabelIDs: labelIDs, SortType: sortType, IssueIDs: issueIDs, + CanSeePrivate: canSeePrivateIssues, }) if err != nil { ctx.ServerError("Issues", err) diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index 1089c82415ab8..443ad9de09d41 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -235,6 +235,7 @@ {{end}} +
From 5652ea17ce08a3301cf74e91f83f53f6f673998b Mon Sep 17 00:00:00 2001 From: Gusted Date: Thu, 18 Nov 2021 15:46:43 +0100 Subject: [PATCH 02/71] Add UI/backend-end handling --- models/error.go | 19 +++++++++++++++++++ routers/web/repo/issue.go | 20 ++++++++++++++++++++ services/forms/repo_form.go | 19 ++++++++++--------- templates/repo/issue/new_form.tmpl | 8 ++++++++ 4 files changed, 57 insertions(+), 9 deletions(-) diff --git a/models/error.go b/models/error.go index 7d9b2ae65bc7e..19be0c366ebda 100644 --- a/models/error.go +++ b/models/error.go @@ -1331,6 +1331,25 @@ func (err ErrReactionAlreadyExist) Error() string { return fmt.Sprintf("reaction '%s' already exists", err.Reaction) } +// ErrCannotSeePrivateIssue is used when a user tries to view a issue +// which they don't have permission for. +type ErrCannotSeePrivateIssue struct { + UserID int64 + ID int64 + RepoID int64 + Index int64 +} + +// IsErrCannotSeePrivateIssue checks if an error is a ErrCannotSeePrivateIssue. +func IsErrCannotSeePrivateIssue(err error) bool { + _, ok := err.(ErrCannotSeePrivateIssue) + return ok +} + +func (err ErrCannotSeePrivateIssue) Error() string { + return fmt.Sprintf("issue is private but user hasn't permission to view it [id: %d, repo_id: %d, index: %d, user_id: %d]", err.ID, err.RepoID, err.Index, err.UserID) +} + // __________ .__ .__ __________ __ // \______ \__ __| | | |\______ \ ____ ________ __ ____ _______/ |_ // | ___/ | \ | | | | _// __ \/ ____/ | \_/ __ \ / ___/\ __\ diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 2324cfd72a5f5..4289be29493a5 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -995,6 +995,7 @@ func NewIssuePost(ctx *context.Context) { MilestoneID: milestoneID, Content: form.Content, Ref: form.Ref, + IsPrivate: form.IsConfidential, } if err := issue_service.NewIssue(repo, issue, labelIDs, attachments, assigneeIDs); err != nil { @@ -1107,6 +1108,25 @@ func ViewIssue(ctx *context.Context) { issue.Repo = ctx.Repo.Repository } + // Check if the issue is private, if so check if the user has enough + // permission to view the issue. + if issue.IsPrivate { + if !ctx.Repo.CanSeePrivateIssues() { + var userID int64 + if ctx.User != nil { + userID = ctx.User.ID + } else { + userID = -1 + } + ctx.NotFound("CanSeePrivateIssues", models.ErrCannotSeePrivateIssue{ + UserID: userID, + ID: issue.ID, + RepoID: ctx.Repo.Repository.ID, + Index: ctx.ParamsInt64(":index"), + }) + } + } + // Make sure type and URL matches. if ctx.Params(":type") == "issues" && issue.IsPull { ctx.Redirect(issue.Link()) diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 7c61be5e2221d..870b73f91300c 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -404,15 +404,16 @@ func (f *NewWechatWorkHookForm) Validate(req *http.Request, errs binding.Errors) // CreateIssueForm form for creating issue type CreateIssueForm struct { - Title string `binding:"Required;MaxSize(255)"` - LabelIDs string `form:"label_ids"` - AssigneeIDs string `form:"assignee_ids"` - Ref string `form:"ref"` - MilestoneID int64 - ProjectID int64 - AssigneeID int64 - Content string - Files []string + Title string `binding:"Required;MaxSize(255)"` + LabelIDs string `form:"label_ids"` + AssigneeIDs string `form:"assignee_ids"` + Ref string `form:"ref"` + MilestoneID int64 + ProjectID int64 + AssigneeID int64 + Content string + Files []string + IsConfidential bool `form:"is_confidential"` } // Validate validates the fields diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index 443ad9de09d41..16f1ff753d57d 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -236,6 +236,14 @@ {{end}}
+
+ + +
From 46f1cc7392038a883bc8d5802eb60467cd8ca5db Mon Sep 17 00:00:00 2001 From: Gusted Date: Thu, 18 Nov 2021 17:14:32 +0100 Subject: [PATCH 03/71] Add toggle --- models/issue.go | 34 +++++++++++++ models/issue_comment.go | 6 ++- options/locale/locale_en-US.ini | 2 + routers/web/repo/issue.go | 50 +++++++++++++++++++ routers/web/web.go | 1 + services/issue/issue.go | 13 +++++ .../repo/issue/view_content/sidebar.tmpl | 23 +++++++++ 7 files changed, 128 insertions(+), 1 deletion(-) diff --git a/models/issue.go b/models/issue.go index 9240186e29827..1097093f15e44 100644 --- a/models/issue.go +++ b/models/issue.go @@ -743,6 +743,40 @@ func (issue *Issue) ChangeTitle(doer *User, oldTitle string) (err error) { return committer.Commit() } +// ChangeConfidential changes the confidential of this issue, as the given user. +func (issue *Issue) ChangeConfidential(doer *User) (err error) { + ctx, committer, err := db.TxContext() + if err != nil { + return err + } + defer committer.Close() + + if err = updateIssueCols(db.GetEngine(ctx), issue, "is_private"); err != nil { + return fmt.Errorf("updateIssueCols: %v", err) + } + + if err = issue.loadRepo(db.GetEngine(ctx)); err != nil { + return fmt.Errorf("loadRepo: %v", err) + } + + opts := &CreateCommentOptions{ + Type: CommenTypeConfidentialChanged, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + OldConfidential: !issue.IsPrivate, + NewConfidential: issue.IsPrivate, + } + if _, err = createComment(db.GetEngine(ctx), opts); err != nil { + return fmt.Errorf("createComment: %v", err) + } + if err = issue.addCrossReferences(db.GetEngine(ctx), doer, true); err != nil { + return err + } + + return committer.Commit() +} + // ChangeRef changes the branch of this issue, as the given user. func (issue *Issue) ChangeRef(doer *User, oldRef string) (err error) { ctx, committer, err := db.TxContext() diff --git a/models/issue_comment.go b/models/issue_comment.go index a41f4cb298a46..64d8ab8007994 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -101,8 +101,10 @@ const ( CommentTypeProject // 31 Project board changed CommentTypeProjectBoard - // Dismiss Review + // 32 Dismiss Review CommentTypeDismissReview + // 33 Change confidential + CommenTypeConfidentialChanged ) // RoleDescriptor defines comment tag type @@ -907,6 +909,8 @@ type CreateCommentOptions struct { NewTitle string OldRef string NewRef string + OldConfidential bool + NewConfidential bool CommitID int64 CommitSHA string Patch string diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 1f27d854acd87..c477052af9a2c 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1396,6 +1396,8 @@ issues.content_history.delete_from_history = Delete from history issues.content_history.delete_from_history_confirm = Delete from history? issues.content_history.options = Options issues.confidential = Confidential +issues.confidential.off = Make issue public +issues.confidential.on = Make issue confidential compare.compare_base = base compare.compare_head = compare diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 4289be29493a5..099285190f358 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1752,6 +1752,28 @@ func UpdateIssueTitle(ctx *context.Context) { }) } +// IssuePrivate sets the issue confidential +func IssuePrivate(ctx *context.Context) { + issue := GetActionIssue(ctx) + if ctx.Written() { + return + } + + if !ctx.IsSigned || (!issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull)) { + ctx.Error(http.StatusForbidden) + return + } + + isConfidential := ctx.FormBool("is_confidential") + + if err := issue_service.ChangeConfidential(issue, ctx.User, isConfidential); err != nil { + ctx.ServerError("ChangeTitle", err) + return + } + + ctx.Redirect(issue.HTMLURL(), http.StatusSeeOther) +} + // UpdateIssueRef change issue's ref (branch) func UpdateIssueRef(ctx *context.Context) { issue := GetActionIssue(ctx) @@ -1889,6 +1911,34 @@ func UpdateIssueAssignee(ctx *context.Context) { }) } +// UpdateIssueConfidential change issue's confidential +func UpdateIssueConfidential(ctx *context.Context) { + issue := GetActionIssue(ctx) + if ctx.Written() { + return + } + + if !ctx.IsSigned || (!issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull)) { + ctx.Error(http.StatusForbidden) + return + } + + title := ctx.FormTrim("title") + if len(title) == 0 { + ctx.Error(http.StatusNoContent) + return + } + + if err := issue_service.ChangeTitle(issue, ctx.User, title); err != nil { + ctx.ServerError("ChangeTitle", err) + return + } + + ctx.JSON(http.StatusOK, map[string]interface{}{ + "title": issue.Title, + }) +} + // UpdatePullReviewRequest add or remove review request func UpdatePullReviewRequest(ctx *context.Context) { issues := getActionIssues(ctx) diff --git a/routers/web/web.go b/routers/web/web.go index f84d357bb126e..3802a0a52149c 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -715,6 +715,7 @@ func RegisterRoutes(m *web.Route) { m.Post("/title", repo.UpdateIssueTitle) m.Post("/content", repo.UpdateIssueContent) m.Post("/watch", repo.IssueWatch) + m.Post("/private", repo.IssuePrivate) m.Post("/ref", repo.UpdateIssueRef) m.Group("/dependency", func() { m.Post("/add", repo.AddDependency) diff --git a/services/issue/issue.go b/services/issue/issue.go index e3571bd396f69..45a51b15cbdf4 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -54,6 +54,19 @@ func ChangeTitle(issue *models.Issue, doer *models.User, title string) (err erro return nil } +// ChangeConfidential changes the confidential of this issue, as the given user. +func ChangeConfidential(issue *models.Issue, doer *models.User, isConfidential bool) (err error) { + issue.IsPrivate = isConfidential + + if err = issue.ChangeConfidential(doer); err != nil { + return + } + + // TODO: add a notification (possible)? + + return nil +} + // ChangeIssueRef changes the branch of this issue, as the given user. func ChangeIssueRef(issue *models.Issue, doer *models.User, ref string) error { oldRef := issue.Ref diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index 6198b6a6210b4..6a69e7b4c78ef 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -302,6 +302,29 @@ + {{if or .HasIssuesOrPullsWritePermission .Issue.IsPrivate .IsPoster}} +
+
+ {{.i18n.Tr "repo.issues.confidential"}} +
+
+ + {{$.CsrfTokenHtml}} + +
+
+
+ {{end}}
{{if .Participants}} From 0e1e744973f97bcb8f3279c0faed97812ff17479 Mon Sep 17 00:00:00 2001 From: Gusted Date: Thu, 18 Nov 2021 21:12:51 +0100 Subject: [PATCH 04/71] Add NumPrivate{Closed,Open,}Issues (BEGIN) --- models/issue.go | 28 +++++++++++++++++++++------- models/repo.go | 35 ++++++++++++++++++++--------------- templates/repo/header.tmpl | 6 +++++- 3 files changed, 46 insertions(+), 23 deletions(-) diff --git a/models/issue.go b/models/issue.go index 1097093f15e44..6f8060b3beac7 100644 --- a/models/issue.go +++ b/models/issue.go @@ -975,7 +975,11 @@ func newIssue(e db.Engine, doer *User, opts NewIssueOptions) (err error) { if opts.IsPull { _, err = e.Exec("UPDATE `repository` SET num_pulls = num_pulls + 1 WHERE id = ?", opts.Issue.RepoID) } else { - _, err = e.Exec("UPDATE `repository` SET num_issues = num_issues + 1 WHERE id = ?", opts.Issue.RepoID) + if opts.Issue.IsPrivate { + _, err = e.Exec("UPDATE `repository` SET num_private_issues = num_private_issues + 1 WHERE id = ?", opts.Issue.RepoID) + } else { + _, err = e.Exec("UPDATE `repository` SET num_issues = num_issues + 1 WHERE id = ?", opts.Issue.RepoID) + } } if err != nil { return err @@ -2027,12 +2031,22 @@ func (issue *Issue) updateClosedNum(e db.Engine) (err error) { issue.RepoID, ) } else { - _, err = e.Exec("UPDATE `repository` SET num_closed_issues=(SELECT count(*) FROM issue WHERE repo_id=? AND is_pull=? AND is_closed=?) WHERE id=?", - issue.RepoID, - false, - true, - issue.RepoID, - ) + if issue.IsPrivate { + _, err = e.Exec("UPDATE `repository` SET num_closed_private_issues=(SELECT count(*) FROM issue WHERE repo_id=? AND is_pull=? AND is_closed=? AND is_private=?) WHERE id=?", + issue.RepoID, + false, + true, + true, + issue.RepoID, + ) + } else { + _, err = e.Exec("UPDATE `repository` SET num_closed_issues=(SELECT count(*) FROM issue WHERE repo_id=? AND is_pull=? AND is_closed=?) WHERE id=?", + issue.RepoID, + false, + true, + issue.RepoID, + ) + } } return } diff --git a/models/repo.go b/models/repo.go index 57806a86f7b5f..96259127669b8 100644 --- a/models/repo.go +++ b/models/repo.go @@ -202,21 +202,24 @@ type Repository struct { OriginalURL string `xorm:"VARCHAR(2048)"` DefaultBranch string - NumWatches int - NumStars int - NumForks int - NumIssues int - NumClosedIssues int - NumOpenIssues int `xorm:"-"` - NumPulls int - NumClosedPulls int - NumOpenPulls int `xorm:"-"` - NumMilestones int `xorm:"NOT NULL DEFAULT 0"` - NumClosedMilestones int `xorm:"NOT NULL DEFAULT 0"` - NumOpenMilestones int `xorm:"-"` - NumProjects int `xorm:"NOT NULL DEFAULT 0"` - NumClosedProjects int `xorm:"NOT NULL DEFAULT 0"` - NumOpenProjects int `xorm:"-"` + NumWatches int + NumStars int + NumForks int + NumIssues int + NumClosedIssues int + NumOpenIssues int `xorm:"-"` + NumPrivateIssues int + NumOpenPrivateIssues int `xorm:"-"` + NumClosedPrivateIssues int + NumPulls int + NumClosedPulls int + NumOpenPulls int `xorm:"-"` + NumMilestones int `xorm:"NOT NULL DEFAULT 0"` + NumClosedMilestones int `xorm:"NOT NULL DEFAULT 0"` + NumOpenMilestones int `xorm:"-"` + NumProjects int `xorm:"NOT NULL DEFAULT 0"` + NumClosedProjects int `xorm:"NOT NULL DEFAULT 0"` + NumOpenProjects int `xorm:"-"` IsPrivate bool `xorm:"INDEX"` IsEmpty bool `xorm:"INDEX"` @@ -296,6 +299,8 @@ func (repo *Repository) AfterLoad() { } repo.NumOpenIssues = repo.NumIssues - repo.NumClosedIssues + repo.NumOpenPrivateIssues = repo.NumPrivateIssues - repo.NumClosedPrivateIssues + fmt.Println(repo.NumPrivateIssues, repo.NumClosedPrivateIssues) repo.NumOpenPulls = repo.NumPulls - repo.NumClosedPulls repo.NumOpenMilestones = repo.NumMilestones - repo.NumClosedMilestones repo.NumOpenProjects = repo.NumProjects - repo.NumClosedProjects diff --git a/templates/repo/header.tmpl b/templates/repo/header.tmpl index 554456d23048a..09552369d023c 100644 --- a/templates/repo/header.tmpl +++ b/templates/repo/header.tmpl @@ -112,7 +112,11 @@ {{svg "octicon-issue-opened"}} {{.i18n.Tr "repo.issues"}} {{if .Repository.NumOpenIssues}} - {{CountFmt .Repository.NumOpenIssues}} + {{if .Permission.CanSeePrivateIssues}} + {{CountFmt (Add .Repository.NumOpenIssues .Repository.NumOpenPrivateIssues)}} + {{else}} + {{CountFmt .Repository.NumOpenIssues}} + {{end}} {{end}} {{end}} From 8e63544b551d707612d36ae89d8999a89f12ea6d Mon Sep 17 00:00:00 2001 From: Gusted Date: Thu, 18 Nov 2021 22:19:30 +0100 Subject: [PATCH 05/71] Respect issue's private on API --- models/issue.go | 13 +++++++++++++ models/repo.go | 1 - routers/api/v1/repo/issue.go | 5 +++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/models/issue.go b/models/issue.go index 6f8060b3beac7..4be620a49b178 100644 --- a/models/issue.go +++ b/models/issue.go @@ -192,6 +192,15 @@ func (issue *Issue) loadPoster(e db.Engine) (err error) { return } +func (issue *Issue) loadIsPrivate(e db.Engine) (err error) { + var isPrivate bool + if _, err = e.Table("issue").Where("id=?", issue.ID).Cols("is_private").Get(&isPrivate); err != nil { + return + } + issue.IsPrivate = isPrivate + return +} + func (issue *Issue) loadPullRequest(e db.Engine) (err error) { if issue.IsPull && issue.PullRequest == nil { issue.PullRequest, err = getPullRequestByIssueID(e, issue.ID) @@ -325,6 +334,10 @@ func (issue *Issue) loadAttributes(e db.Engine) (err error) { } } + if err = issue.loadIsPrivate(e); err != nil { + return err + } + return issue.loadReactions(e) } diff --git a/models/repo.go b/models/repo.go index 96259127669b8..330c342e0f664 100644 --- a/models/repo.go +++ b/models/repo.go @@ -300,7 +300,6 @@ func (repo *Repository) AfterLoad() { repo.NumOpenIssues = repo.NumIssues - repo.NumClosedIssues repo.NumOpenPrivateIssues = repo.NumPrivateIssues - repo.NumClosedPrivateIssues - fmt.Println(repo.NumPrivateIssues, repo.NumClosedPrivateIssues) repo.NumOpenPulls = repo.NumPulls - repo.NumClosedPulls repo.NumOpenMilestones = repo.NumMilestones - repo.NumClosedMilestones repo.NumOpenProjects = repo.NumProjects - repo.NumClosedProjects diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index a2454b8618382..4b67c4a638ed5 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -545,6 +545,11 @@ func GetIssue(ctx *context.APIContext) { } return } + + if issue.IsPrivate && !ctx.Repo.CanSeePrivateIssues() { + ctx.NotFound() + } + ctx.JSON(http.StatusOK, convert.ToAPIIssue(issue)) } From 14bfc692c9b20967f593b759f727bc23000eab7f Mon Sep 17 00:00:00 2001 From: Gusted Date: Thu, 18 Nov 2021 22:34:47 +0100 Subject: [PATCH 06/71] Fix issue Right checking --- routers/web/repo/issue.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 099285190f358..44381debb1f2b 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1684,7 +1684,8 @@ func GetActionIssue(ctx *context.Context) *models.Issue { func checkIssueRights(ctx *context.Context, issue *models.Issue) { if issue.IsPull && !ctx.Repo.CanRead(unit.TypePullRequests) || - !issue.IsPull && !ctx.Repo.CanRead(unit.TypeIssues) { + !issue.IsPull && !ctx.Repo.CanRead(unit.TypeIssues) || + !issue.IsPull && (issue.IsPrivate && !(ctx.Repo.CanSeePrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID)))) { ctx.NotFound("IssueOrPullRequestUnitNotAllowed", nil) } } From a81083392c5b808e83cdcf80b009e0150471d6eb Mon Sep 17 00:00:00 2001 From: Gusted Date: Thu, 18 Nov 2021 23:26:53 +0100 Subject: [PATCH 07/71] Fix issue list on poster --- models/issue.go | 41 ++++++++++++++++++++++++++++++++------- models/issue_test.go | 2 +- routers/web/repo/issue.go | 12 +++++++++--- 3 files changed, 44 insertions(+), 11 deletions(-) diff --git a/models/issue.go b/models/issue.go index 4be620a49b178..ce4a960d9aef4 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1207,6 +1207,7 @@ type IssuesOptions struct { // prioritize issues from this repo PriorityRepoID int64 IsArchived util.OptionalBool + UserID int64 CanSeePrivate bool } @@ -1314,7 +1315,20 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { } if !opts.CanSeePrivate { - sess.And("issue.is_private=?", false) + if opts.UserID == 0 { + sess.And("issue.is_private=?", false) + } else { + // Allow to see private issues if the user is the poster of it. + sess.And( + builder.Or( + builder.Eq{"`issue`.is_private": false}, + builder.And( + builder.Eq{"`issue`.is_private": true}, + builder.In("`issue`.poster_id", opts.UserID), + ), + ), + ) + } } switch opts.IsPull { @@ -1563,10 +1577,10 @@ const ( maxQueryParameters = 300 ) -// GetIssueStats returns issue statistic information by given conditions. -func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) { +// GetIssueStats returns issue statistic information by given conditions, as User +func GetIssueStats(opts *IssueStatsOptions, userID int64) (*IssueStats, error) { if len(opts.IssueIDs) <= maxQueryParameters { - return getIssueStatsChunk(opts, opts.IssueIDs) + return getIssueStatsChunk(opts, opts.IssueIDs, userID) } // If too long a list of IDs is provided, we get the statistics in @@ -1579,7 +1593,7 @@ func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) { if chunk > len(opts.IssueIDs) { chunk = len(opts.IssueIDs) } - stats, err := getIssueStatsChunk(opts, opts.IssueIDs[i:chunk]) + stats, err := getIssueStatsChunk(opts, opts.IssueIDs[i:chunk], userID) if err != nil { return nil, err } @@ -1595,7 +1609,7 @@ func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) { return accum, nil } -func getIssueStatsChunk(opts *IssueStatsOptions, issueIDs []int64) (*IssueStats, error) { +func getIssueStatsChunk(opts *IssueStatsOptions, issueIDs []int64, userID int64) (*IssueStats, error) { stats := &IssueStats{} countSession := func(opts *IssueStatsOptions, issueIDs []int64) *xorm.Session { @@ -1643,7 +1657,20 @@ func getIssueStatsChunk(opts *IssueStatsOptions, issueIDs []int64) (*IssueStats, } if !opts.CanSeePrivate { - sess.And("issue.is_private=?", false) + if userID == 0 { + sess.And("issue.is_private=?", false) + } else { + // Allow to see private issues if the user is the poster of it. + sess.And( + builder.Or( + builder.Eq{"`issue`.is_private": false}, + builder.And( + builder.Eq{"`issue`.is_private": true}, + builder.In("`issue`.poster_id", userID), + ), + ), + ) + } } switch opts.IsPull { diff --git a/models/issue_test.go b/models/issue_test.go index 7942b7e78539a..90314e47c6cf9 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -471,7 +471,7 @@ func TestCorrectIssueStats(t *testing.T) { issueStats, err := GetIssueStats(&IssueStatsOptions{ RepoID: 1, IssueIDs: ids, - }) + }, 0) // Now check the values. assert.NoError(t, err) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 44381debb1f2b..a211436d487c3 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -171,6 +171,11 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti canSeePrivateIssues := ctx.Repo.CanSeePrivateIssues() + var userID int64 + if ctx.IsSigned { + userID = ctx.User.ID + } + var issueStats *models.IssueStats if forceEmpty { issueStats = &models.IssueStats{} @@ -186,7 +191,7 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti IsPull: isPullOption, IssueIDs: issueIDs, CanSeePrivate: canSeePrivateIssues, - }) + }, userID) if err != nil { ctx.ServerError("GetIssueStats", err) return @@ -239,6 +244,7 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti SortType: sortType, IssueIDs: issueIDs, CanSeePrivate: canSeePrivateIssues, + UserID: userID, }) if err != nil { ctx.ServerError("Issues", err) @@ -1111,9 +1117,9 @@ func ViewIssue(ctx *context.Context) { // Check if the issue is private, if so check if the user has enough // permission to view the issue. if issue.IsPrivate { - if !ctx.Repo.CanSeePrivateIssues() { + if !(ctx.Repo.CanSeePrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { var userID int64 - if ctx.User != nil { + if ctx.IsSigned { userID = ctx.User.ID } else { userID = -1 From faadf71983b82f7919297f3c488618bbab981ba2 Mon Sep 17 00:00:00 2001 From: Gusted Date: Thu, 18 Nov 2021 23:39:48 +0100 Subject: [PATCH 08/71] Code review feedback Co-authored-by: delvh --- models/error.go | 2 +- routers/web/repo/issue.go | 37 ++----------------- .../repo/issue/view_content/sidebar.tmpl | 2 - 3 files changed, 5 insertions(+), 36 deletions(-) diff --git a/models/error.go b/models/error.go index 19be0c366ebda..9d7dc4af8b1ca 100644 --- a/models/error.go +++ b/models/error.go @@ -1347,7 +1347,7 @@ func IsErrCannotSeePrivateIssue(err error) bool { } func (err ErrCannotSeePrivateIssue) Error() string { - return fmt.Sprintf("issue is private but user hasn't permission to view it [id: %d, repo_id: %d, index: %d, user_id: %d]", err.ID, err.RepoID, err.Index, err.UserID) + return fmt.Sprintf("issue is private but user has no permission to view it [id: %d, repo_id: %d, index: %d, user_id: %d]", err.ID, err.RepoID, err.Index, err.UserID) } // __________ .__ .__ __________ __ diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index a211436d487c3..bcf1d1ddf71d2 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1116,8 +1116,7 @@ func ViewIssue(ctx *context.Context) { // Check if the issue is private, if so check if the user has enough // permission to view the issue. - if issue.IsPrivate { - if !(ctx.Repo.CanSeePrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { + if issue.IsPrivate && !(ctx.Repo.CanSeePrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { var userID int64 if ctx.IsSigned { userID = ctx.User.ID @@ -1690,8 +1689,8 @@ func GetActionIssue(ctx *context.Context) *models.Issue { func checkIssueRights(ctx *context.Context, issue *models.Issue) { if issue.IsPull && !ctx.Repo.CanRead(unit.TypePullRequests) || - !issue.IsPull && !ctx.Repo.CanRead(unit.TypeIssues) || - !issue.IsPull && (issue.IsPrivate && !(ctx.Repo.CanSeePrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID)))) { + (!issue.IsPull && !ctx.Repo.CanRead(unit.TypeIssues) && + (issue.IsPrivate && !(ctx.Repo.CanSeePrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))))) { ctx.NotFound("IssueOrPullRequestUnitNotAllowed", nil) } } @@ -1774,7 +1773,7 @@ func IssuePrivate(ctx *context.Context) { isConfidential := ctx.FormBool("is_confidential") if err := issue_service.ChangeConfidential(issue, ctx.User, isConfidential); err != nil { - ctx.ServerError("ChangeTitle", err) + ctx.ServerError("ChangeConfidential", err) return } @@ -1918,34 +1917,6 @@ func UpdateIssueAssignee(ctx *context.Context) { }) } -// UpdateIssueConfidential change issue's confidential -func UpdateIssueConfidential(ctx *context.Context) { - issue := GetActionIssue(ctx) - if ctx.Written() { - return - } - - if !ctx.IsSigned || (!issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull)) { - ctx.Error(http.StatusForbidden) - return - } - - title := ctx.FormTrim("title") - if len(title) == 0 { - ctx.Error(http.StatusNoContent) - return - } - - if err := issue_service.ChangeTitle(issue, ctx.User, title); err != nil { - ctx.ServerError("ChangeTitle", err) - return - } - - ctx.JSON(http.StatusOK, map[string]interface{}{ - "title": issue.Title, - }) -} - // UpdatePullReviewRequest add or remove review request func UpdatePullReviewRequest(ctx *context.Context) { issues := getActionIssues(ctx) diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index 6a69e7b4c78ef..c9a4536ff762a 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -312,8 +312,6 @@ {{$.CsrfTokenHtml}} From 4706958a4ff11f45e1ba5e8753cdba67d35e41bc Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 20 Nov 2021 00:52:52 +0100 Subject: [PATCH 12/71] Add team permission --- models/org.go | 1 + models/org_team.go | 3 ++- models/repo_permission.go | 17 +++++++++++------ options/locale/locale_en-US.ini | 2 ++ routers/api/v1/repo/issue.go | 2 +- routers/web/org/teams.go | 3 +++ routers/web/repo/issue.go | 6 +++--- services/forms/org.go | 13 +++++++------ templates/org/team/new.tmpl | 7 +++++++ templates/repo/header.tmpl | 2 +- 10 files changed, 38 insertions(+), 18 deletions(-) diff --git a/models/org.go b/models/org.go index e0b4d272459cd..61510d4af6a4e 100644 --- a/models/org.go +++ b/models/org.go @@ -197,6 +197,7 @@ func CreateOrganization(org, owner *User) (err error) { NumMembers: 1, IncludesAllRepositories: true, CanCreateOrgRepo: true, + CanSeePrivateIssues: true, } if err = db.Insert(ctx, t); err != nil { return fmt.Errorf("insert owner team: %v", err) diff --git a/models/org_team.go b/models/org_team.go index 10178ec88aee9..d2b591c2e52c2 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -37,6 +37,7 @@ type Team struct { Units []*TeamUnit `xorm:"-"` IncludesAllRepositories bool `xorm:"NOT NULL DEFAULT false"` CanCreateOrgRepo bool `xorm:"NOT NULL DEFAULT false"` + CanSeePrivateIssues bool `xorm:"NOT NULL DEFAULT false"` } func init() { @@ -643,7 +644,7 @@ func UpdateTeam(t *Team, authChanged, includeAllChanged bool) (err error) { } if _, err = sess.ID(t.ID).Cols("name", "lower_name", "description", - "can_create_org_repo", "authorize", "includes_all_repositories").Update(t); err != nil { + "can_create_org_repo", "authorize", "includes_all_repositories", "can_see_private_issues").Update(t); err != nil { return fmt.Errorf("update: %v", err) } diff --git a/models/repo_permission.go b/models/repo_permission.go index 031e08a35c701..bd4dbbe6c867b 100644 --- a/models/repo_permission.go +++ b/models/repo_permission.go @@ -14,9 +14,10 @@ import ( // Permission contains all the permissions related variables to a repository for a user type Permission struct { - AccessMode AccessMode - Units []*RepoUnit - UnitsMode map[unit.Type]AccessMode + AccessMode AccessMode + Units []*RepoUnit + UnitsMode map[unit.Type]AccessMode + SeePrivateIssue bool } // IsOwner returns true if current user is the owner of repository. @@ -98,9 +99,9 @@ func (p *Permission) CanWriteIssuesOrPulls(isPull bool) bool { return p.CanWrite(unit.TypeIssues) } -// CanSeePrivateIssues returns true if the user is allowed to see private issues on the repo. -func (p *Permission) CanSeePrivateIssues() bool { - return p.CanWrite(unit.TypeIssues) +// CanReadPrivateIssues returns true if the user is allowed to see private issues on the repo. +func (p *Permission) CanReadPrivateIssues() bool { + return p.SeePrivateIssue || p.IsAdmin() } // ColorFormat writes a colored string for these Permissions @@ -243,6 +244,10 @@ func getUserRepoPermission(e db.Engine, repo *Repository, user *User) (perm Perm perm.UnitsMode = nil return } + // Check if the team has acces to private issues. + if team.CanSeePrivateIssues { + perm.SeePrivateIssue = true + } } for _, u := range repo.Units { diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 2b0b810e5bdf8..84401b83e0e05 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2264,6 +2264,8 @@ teams.all_repositories_helper = Team has access to all repositories. Selecting t teams.all_repositories_read_permission_desc = This team grants Read access to all repositories: members can view and clone repositories. teams.all_repositories_write_permission_desc = This team grants Write access to all repositories: members can read from and push to repositories. teams.all_repositories_admin_permission_desc = This team grants Admin access to all repositories: members can read from, push to and add collaborators to repositories. +teams.can_see_private_issues = Access private issues +teams.can_see_private_issues_helper = Members can access the private issues in team repositories. [admin] dashboard = Dashboard diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 4b67c4a638ed5..64b17a5efd635 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -546,7 +546,7 @@ func GetIssue(ctx *context.APIContext) { return } - if issue.IsPrivate && !ctx.Repo.CanSeePrivateIssues() { + if issue.IsPrivate && !ctx.Repo.CanReadPrivateIssues() { ctx.NotFound() } diff --git a/routers/web/org/teams.go b/routers/web/org/teams.go index 28ffac4dd39bd..0cd770e08f21f 100644 --- a/routers/web/org/teams.go +++ b/routers/web/org/teams.go @@ -237,6 +237,7 @@ func NewTeamPost(ctx *context.Context) { Authorize: models.ParseAccessMode(form.Permission), IncludesAllRepositories: includesAllRepositories, CanCreateOrgRepo: form.CanCreateOrgRepo, + CanSeePrivateIssues: form.CanSeePrivateIssues, } if t.Authorize < models.AccessModeOwner { @@ -353,7 +354,9 @@ func EditTeamPost(ctx *context.Context) { return } } + t.CanCreateOrgRepo = form.CanCreateOrgRepo + t.CanSeePrivateIssues = form.CanSeePrivateIssues if ctx.HasError() { ctx.HTML(http.StatusOK, tplTeamNew) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 13e995f10c607..b53e1d4c5c931 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -169,7 +169,7 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti } } - canSeePrivateIssues := ctx.Repo.CanSeePrivateIssues() + canSeePrivateIssues := ctx.Repo.CanReadPrivateIssues() var userID int64 if ctx.IsSigned { @@ -1116,7 +1116,7 @@ func ViewIssue(ctx *context.Context) { // Check if the issue is private, if so check if the user has enough // permission to view the issue. - if issue.IsPrivate && !(ctx.Repo.CanSeePrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { var userID int64 if ctx.IsSigned { userID = ctx.User.ID @@ -1689,7 +1689,7 @@ func GetActionIssue(ctx *context.Context) *models.Issue { func checkIssueRights(ctx *context.Context, issue *models.Issue) { if issue.IsPull && !ctx.Repo.CanRead(unit.TypePullRequests) || (!issue.IsPull && !ctx.Repo.CanRead(unit.TypeIssues) && - (issue.IsPrivate && !(ctx.Repo.CanSeePrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))))) { + (issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))))) { ctx.NotFound("IssueOrPullRequestUnitNotAllowed", nil) } } diff --git a/services/forms/org.go b/services/forms/org.go index 7c8f17f95ee60..3ba319418abfe 100644 --- a/services/forms/org.go +++ b/services/forms/org.go @@ -63,12 +63,13 @@ func (f *UpdateOrgSettingForm) Validate(req *http.Request, errs binding.Errors) // CreateTeamForm form for creating team type CreateTeamForm struct { - TeamName string `binding:"Required;AlphaDashDot;MaxSize(30)"` - Description string `binding:"MaxSize(255)"` - Permission string - Units []unit.Type - RepoAccess string - CanCreateOrgRepo bool + TeamName string `binding:"Required;AlphaDashDot;MaxSize(30)"` + Description string `binding:"MaxSize(255)"` + Permission string + Units []unit.Type + RepoAccess string + CanCreateOrgRepo bool + CanSeePrivateIssues bool } // Validate validates the fields diff --git a/templates/org/team/new.tmpl b/templates/org/team/new.tmpl index 40c3ed99c3891..35aded1ecab3e 100644 --- a/templates/org/team/new.tmpl +++ b/templates/org/team/new.tmpl @@ -74,6 +74,13 @@ {{.i18n.Tr "org.teams.admin_access_helper"}} +
+
can_see_private_issues + + + {{.i18n.Tr "org.teams.can_see_private_issues_helper"}} +
+
diff --git a/templates/repo/header.tmpl b/templates/repo/header.tmpl index 09552369d023c..56d08bc2b2ba5 100644 --- a/templates/repo/header.tmpl +++ b/templates/repo/header.tmpl @@ -112,7 +112,7 @@ {{svg "octicon-issue-opened"}} {{.i18n.Tr "repo.issues"}} {{if .Repository.NumOpenIssues}} - {{if .Permission.CanSeePrivateIssues}} + {{if .Permission.CanReadPrivateIssues}} {{CountFmt (Add .Repository.NumOpenIssues .Repository.NumOpenPrivateIssues)}} {{else}} {{CountFmt .Repository.NumOpenIssues}} From b728100f757c80fba662de5bc30a794f8be523fb Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 20 Nov 2021 00:58:08 +0100 Subject: [PATCH 13/71] Fix comments --- models/issue.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/issue.go b/models/issue.go index 4c223b9189938..f9c0f508f8d77 100644 --- a/models/issue.go +++ b/models/issue.go @@ -783,10 +783,10 @@ func (issue *Issue) ChangeConfidential(doer *User) (err error) { OldConfidential: !issue.IsPrivate, NewConfidential: issue.IsPrivate, } - if _, err = createComment(db.GetEngine(ctx), opts); err != nil { + if _, err = createComment(ctx, opts); err != nil { return fmt.Errorf("createComment: %v", err) } - if err = issue.addCrossReferences(db.GetEngine(ctx), doer, true); err != nil { + if err = issue.addCrossReferences(ctx, doer, true); err != nil { return err } From 108ae1f309768ff8b09f95d0065065d275f91a80 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 20 Nov 2021 21:39:28 +0100 Subject: [PATCH 14/71] Add migration --- models/migrations/migrations.go | 2 ++ models/migrations/v202.go | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 models/migrations/v202.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index c0d8f111d3764..2b87a38cd62c2 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -357,6 +357,8 @@ var migrations = []Migration{ NewMigration("Add table app_state", addTableAppState), // v201 -> v202 NewMigration("Drop table remote_version (if exists)", dropTableRemoteVersion), + // v202 -> v203 + NewMigration("Add private issues to Repository table", addPrivateIssuesToRepo), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v202.go b/models/migrations/v202.go new file mode 100644 index 0000000000000..803ea23ecdd43 --- /dev/null +++ b/models/migrations/v202.go @@ -0,0 +1,23 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "fmt" + + "xorm.io/xorm" +) + +func addPrivateIssuesToRepo(x *xorm.Engine) error { + type Repository struct { + NumPrivateIssues int + NumClosedPrivateIssues int + } + + if err := x.Sync2(new(Repository)); err != nil { + return fmt.Errorf("Sync2: %v", err) + } + return nil +} From 75c8a346dc161c3e4902d330a3909b057ae38526 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 20 Nov 2021 21:39:33 +0100 Subject: [PATCH 15/71] Fix header calculation --- templates/repo/header.tmpl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/templates/repo/header.tmpl b/templates/repo/header.tmpl index 56d08bc2b2ba5..6a3c17ee4ef3b 100644 --- a/templates/repo/header.tmpl +++ b/templates/repo/header.tmpl @@ -111,10 +111,10 @@ {{if .Permission.CanRead $.UnitTypeIssues}} {{svg "octicon-issue-opened"}} {{.i18n.Tr "repo.issues"}} - {{if .Repository.NumOpenIssues}} - {{if .Permission.CanReadPrivateIssues}} - {{CountFmt (Add .Repository.NumOpenIssues .Repository.NumOpenPrivateIssues)}} - {{else}} + {{if or .Repository.NumOpenIssues .Repository.NumOpenPrivateIssues}} + {{if and .Permission.CanReadPrivateIssues .Repository.NumOpenPrivateIssues}} + {{CountFmt (Add .Repository.NumOpenPrivateIssues .Repository.NumOpenIssues)}} + {{else if .Repository.NumOpenIssues}} {{CountFmt .Repository.NumOpenIssues}} {{end}} {{end}} From fe48cfb823578d807155e74f171a4f36fc0f1665 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 20 Nov 2021 21:53:34 +0100 Subject: [PATCH 16/71] Fix showing confidential button. --- templates/repo/issue/view_content/sidebar.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index a6047d9ea53c0..ce1eff3347aa4 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -302,7 +302,7 @@ - {{if or (.IsRepoAdmin) (.Issue.IsPrivate) (.IsIssuePoster)}} + {{if or (.IsRepoAdmin) (.IsIssuePoster)}}
{{.i18n.Tr "repo.issues.confidential"}} From de881716106371681d6a7ae45fe81c90a1a618ca Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 20 Nov 2021 22:05:35 +0100 Subject: [PATCH 17/71] Fix listing issues on other places --- routers/api/v1/repo/issue.go | 11 ++++++++++- routers/web/user/home.go | 1 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 64b17a5efd635..752ad3d613805 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -223,10 +223,13 @@ func SearchIssues(ctx *context.APIContext) { } else if limit > setting.API.MaxResponseItems { limit = setting.API.MaxResponseItems } - // Only fetch the issues if we either don't have a keyword or the search returned issues // This would otherwise return all issues if no issues were found by the search. if len(keyword) == 0 || len(issueIDs) > 0 || len(includedLabelNames) > 0 || len(includedMilestones) > 0 { + var userID int64 + if ctx.IsSigned { + userID = ctx.User.ID + } issuesOpt := &models.IssuesOptions{ ListOptions: db.ListOptions{ Page: ctx.FormInt("page"), @@ -242,6 +245,7 @@ func SearchIssues(ctx *context.APIContext) { IsPull: isPull, UpdatedBeforeUnix: before, UpdatedAfterUnix: since, + UserID: userID, } // Filter for: Created by User, Assigned to User, Mentioning User, Review of User Requested @@ -452,6 +456,10 @@ func ListIssues(ctx *context.APIContext) { // Only fetch the issues if we either don't have a keyword or the search returned issues // This would otherwise return all issues if no issues were found by the search. if len(keyword) == 0 || len(issueIDs) > 0 || len(labelIDs) > 0 { + var userID int64 + if ctx.IsSigned { + userID = ctx.User.ID + } issuesOpt := &models.IssuesOptions{ ListOptions: listOptions, RepoIDs: []int64{ctx.Repo.Repository.ID}, @@ -465,6 +473,7 @@ func ListIssues(ctx *context.APIContext) { PosterID: createdByID, AssigneeID: assignedByID, MentionedID: mentionedByID, + UserID: userID, } if issues, err = models.Issues(issuesOpt); err != nil { diff --git a/routers/web/user/home.go b/routers/web/user/home.go index c0ecd0c2a0f11..ca55334cf85e0 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -403,6 +403,7 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { IsPull: util.OptionalBoolOf(isPullList), SortType: sortType, IsArchived: util.OptionalBoolFalse, + UserID: ctxUser.ID, } // Get repository IDs where User/Org/Team has access. From 777694a7fa94f7de5a2586f2a8c86c6f5713af50 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 20 Nov 2021 23:09:13 +0100 Subject: [PATCH 18/71] Add permission checking to api --- models/issue.go | 5 +++ models/issue_comment.go | 32 ++++++++++---- routers/api/v1/repo/issue.go | 16 ++++++- routers/api/v1/repo/issue_comment.go | 54 ++++++++++++++++++++--- routers/api/v1/repo/issue_label.go | 20 +++++++++ routers/api/v1/repo/issue_reaction.go | 19 ++++++++ routers/api/v1/repo/issue_stopwatch.go | 5 +++ routers/api/v1/repo/issue_subscription.go | 15 +++++++ routers/api/v1/repo/issue_tracked_time.go | 20 +++++++++ 9 files changed, 171 insertions(+), 15 deletions(-) diff --git a/models/issue.go b/models/issue.go index f9c0f508f8d77..7781f8d203d7b 100644 --- a/models/issue.go +++ b/models/issue.go @@ -193,6 +193,11 @@ func (issue *Issue) loadPoster(e db.Engine) (err error) { return } +// LoadIsPrivate loads isPrivate +func (issue *Issue) LoadIsPrivate() error { + return issue.loadIsPrivate(db.GetEngine(db.DefaultContext)) +} + func (issue *Issue) loadIsPrivate(e db.Engine) (err error) { var isPrivate bool if _, err = e.Table("issue").Where("id=?", issue.ID).Cols("is_private").Get(&isPrivate); err != nil { diff --git a/models/issue_comment.go b/models/issue_comment.go index 418f606a68c0c..bf5095ef753e8 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -1003,14 +1003,16 @@ func getCommentByID(e db.Engine, id int64) (*Comment, error) { // FindCommentsOptions describes the conditions to Find comments type FindCommentsOptions struct { db.ListOptions - RepoID int64 - IssueID int64 - ReviewID int64 - Since int64 - Before int64 - Line int64 - TreePath string - Type CommentType + RepoID int64 + IssueID int64 + ReviewID int64 + UserID int64 + Since int64 + Before int64 + Line int64 + TreePath string + Type CommentType + CanSeePrivate bool } func (opts *FindCommentsOptions) toConds() builder.Cond { @@ -1039,6 +1041,20 @@ func (opts *FindCommentsOptions) toConds() builder.Cond { if len(opts.TreePath) > 0 { cond = cond.And(builder.Eq{"comment.tree_path": opts.TreePath}) } + if opts.CanSeePrivate { + // Allow to see comments on private issues + cond = cond.And( + builder.Or( + builder.Eq{"`issue`.is_private": false}, + builder.And( + builder.Eq{"`issue`.is_private": true}, + builder.In("`issue`.poster_id", opts.UserID), + ), + ), + ) + } else { + cond = cond.And(builder.Eq{"issue.is_private": false}) + } return cond } diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 752ad3d613805..3272a7f89f238 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -555,8 +555,9 @@ func GetIssue(ctx *context.APIContext) { return } - if issue.IsPrivate && !ctx.Repo.CanReadPrivateIssues() { - ctx.NotFound() + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return } ctx.JSON(http.StatusOK, convert.ToAPIIssue(issue)) @@ -729,6 +730,12 @@ func EditIssue(ctx *context.APIContext) { return } issue.Repo = ctx.Repo.Repository + + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + canWrite := ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) err = issue.LoadAttributes() @@ -895,6 +902,11 @@ func UpdateIssueDeadline(ctx *context.APIContext) { return } + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) { ctx.Error(http.StatusForbidden, "", "Not repo writer") return diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go index 13e7de46b1f12..82d2f4be41e52 100644 --- a/routers/api/v1/repo/issue_comment.go +++ b/routers/api/v1/repo/issue_comment.go @@ -68,6 +68,19 @@ func ListIssueComments(ctx *context.APIContext) { } issue.Repo = ctx.Repo.Repository + if err = issue.LoadPoster(); err != nil { + ctx.InternalServerError(err) + return + } + if err = issue.LoadIsPrivate(); err != nil { + ctx.InternalServerError(err) + return + } + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + opts := &models.FindCommentsOptions{ IssueID: issue.ID, Since: since, @@ -148,12 +161,19 @@ func ListRepoIssueComments(ctx *context.APIContext) { return } + var userID int64 + if ctx.IsSigned { + userID = ctx.User.ID + } + opts := &models.FindCommentsOptions{ - ListOptions: utils.GetListOptions(ctx), - RepoID: ctx.Repo.Repository.ID, - Type: models.CommentTypeComment, - Since: since, - Before: before, + ListOptions: utils.GetListOptions(ctx), + RepoID: ctx.Repo.Repository.ID, + Type: models.CommentTypeComment, + Since: since, + Before: before, + CanSeePrivate: ctx.Repo.CanReadPrivateIssues(), + UserID: userID, } comments, err := models.FindComments(opts) @@ -300,6 +320,12 @@ func GetIssueComment(ctx *context.APIContext) { ctx.InternalServerError(err) return } + + if comment.Issue.IsPrivate && (!ctx.IsSigned || !comment.Issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + if comment.Issue.RepoID != ctx.Repo.Repository.ID { ctx.Status(http.StatusNotFound) return @@ -423,6 +449,19 @@ func editIssueComment(ctx *context.APIContext, form api.EditIssueCommentOption) return } + if comment.Issue.IsPrivate { + canSeePrivateIssues := ctx.Repo.CanReadPrivateIssues() + var userID int64 + if ctx.IsSigned { + userID = ctx.User.ID + } + + if !(comment.Issue.PosterID == userID || canSeePrivateIssues) { + ctx.Status(http.StatusNotFound) + return + } + } + if !ctx.IsSigned || (ctx.User.ID != comment.PosterID && !ctx.Repo.IsAdmin()) { ctx.Status(http.StatusForbidden) return @@ -524,6 +563,11 @@ func deleteIssueComment(ctx *context.APIContext) { return } + if comment.Issue.IsPrivate && (!ctx.IsSigned || !comment.Issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + if !ctx.IsSigned || (ctx.User.ID != comment.PosterID && !ctx.Repo.IsAdmin()) { ctx.Status(http.StatusForbidden) return diff --git a/routers/api/v1/repo/issue_label.go b/routers/api/v1/repo/issue_label.go index 0469ae247c3b4..80d9b64c42e11 100644 --- a/routers/api/v1/repo/issue_label.go +++ b/routers/api/v1/repo/issue_label.go @@ -61,6 +61,11 @@ func ListIssueLabels(ctx *context.APIContext) { return } + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + ctx.JSON(http.StatusOK, convert.ToLabelList(issue.Labels, ctx.Repo.Repository, ctx.Repo.Owner)) } @@ -168,6 +173,11 @@ func DeleteIssueLabel(ctx *context.APIContext) { return } + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) { ctx.Status(http.StatusForbidden) return @@ -286,6 +296,11 @@ func ClearIssueLabels(ctx *context.APIContext) { return } + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) { ctx.Status(http.StatusForbidden) return @@ -310,6 +325,11 @@ func prepareForReplaceOrAdd(ctx *context.APIContext, form api.IssueLabelsOption) return } + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + labels, err = models.GetLabelsByIDs(form.Labels) if err != nil { ctx.Error(http.StatusInternalServerError, "GetLabelsByIDs", err) diff --git a/routers/api/v1/repo/issue_reaction.go b/routers/api/v1/repo/issue_reaction.go index d0ba8dac65835..e094595949c17 100644 --- a/routers/api/v1/repo/issue_reaction.go +++ b/routers/api/v1/repo/issue_reaction.go @@ -61,6 +61,10 @@ func GetIssueCommentReactions(ctx *context.APIContext) { if err := comment.LoadIssue(); err != nil { ctx.Error(http.StatusInternalServerError, "comment.LoadIssue", err) } + if comment.Issue.IsPrivate && (!ctx.IsSigned || !comment.Issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) { ctx.Error(http.StatusForbidden, "GetIssueCommentReactions", errors.New("no permission to get reactions")) @@ -190,6 +194,11 @@ func changeIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOp ctx.Error(http.StatusInternalServerError, "comment.LoadIssue() failed", err) } + if comment.Issue.IsPrivate && (!ctx.IsSigned || !comment.Issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + if comment.Issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull) { ctx.Error(http.StatusForbidden, "ChangeIssueCommentReaction", errors.New("no permission to change reaction")) return @@ -280,6 +289,11 @@ func GetIssueReactions(ctx *context.APIContext) { return } + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + if !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull) { ctx.Error(http.StatusForbidden, "GetIssueReactions", errors.New("no permission to get reactions")) return @@ -399,6 +413,11 @@ func changeIssueReaction(ctx *context.APIContext, form api.EditReactionOption, i return } + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + if issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) { ctx.Error(http.StatusForbidden, "ChangeIssueCommentReaction", errors.New("no permission to change reaction")) return diff --git a/routers/api/v1/repo/issue_stopwatch.go b/routers/api/v1/repo/issue_stopwatch.go index 82a9ffe10bb73..46d547c3bbd85 100644 --- a/routers/api/v1/repo/issue_stopwatch.go +++ b/routers/api/v1/repo/issue_stopwatch.go @@ -173,6 +173,11 @@ func prepareIssueStopwatch(ctx *context.APIContext, shouldExist bool) (*models.I return nil, err } + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return nil, errors.New("Not enough permission to see issue") + } + if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) { ctx.Status(http.StatusForbidden) return nil, errors.New("Unable to write to PRs") diff --git a/routers/api/v1/repo/issue_subscription.go b/routers/api/v1/repo/issue_subscription.go index 8acd378cc5e03..9453bafc367eb 100644 --- a/routers/api/v1/repo/issue_subscription.go +++ b/routers/api/v1/repo/issue_subscription.go @@ -114,6 +114,11 @@ func setIssueSubscription(ctx *context.APIContext, watch bool) { return } + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + user, err := models.GetUserByName(ctx.Params(":user")) if err != nil { if models.IsErrUserNotExist(err) { @@ -195,6 +200,11 @@ func CheckIssueSubscription(ctx *context.APIContext) { return } + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + watching, err := models.CheckIssueWatch(ctx.User, issue) if err != nil { ctx.InternalServerError(err) @@ -261,6 +271,11 @@ func GetIssueSubscribers(ctx *context.APIContext) { return } + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + iwl, err := models.GetIssueWatchers(issue.ID, utils.GetListOptions(ctx)) if err != nil { ctx.Error(http.StatusInternalServerError, "GetIssueWatchers", err) diff --git a/routers/api/v1/repo/issue_tracked_time.go b/routers/api/v1/repo/issue_tracked_time.go index f73e63547b0ce..48e342d578df5 100644 --- a/routers/api/v1/repo/issue_tracked_time.go +++ b/routers/api/v1/repo/issue_tracked_time.go @@ -84,6 +84,11 @@ func ListTrackedTimes(ctx *context.APIContext) { return } + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + opts := &models.FindTrackedTimesOptions{ ListOptions: utils.GetListOptions(ctx), RepositoryID: ctx.Repo.Repository.ID, @@ -188,6 +193,11 @@ func AddTime(ctx *context.APIContext) { return } + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + if !ctx.Repo.CanUseTimetracker(issue, ctx.User) { if !ctx.Repo.Repository.IsTimetrackerEnabled() { ctx.Error(http.StatusBadRequest, "", "time tracking disabled") @@ -269,6 +279,11 @@ func ResetIssueTime(ctx *context.APIContext) { return } + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + if !ctx.Repo.CanUseTimetracker(issue, ctx.User) { if !ctx.Repo.Repository.IsTimetrackerEnabled() { ctx.JSON(http.StatusBadRequest, struct{ Message string }{Message: "time tracking disabled"}) @@ -340,6 +355,11 @@ func DeleteTime(ctx *context.APIContext) { return } + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.Status(http.StatusNotFound) + return + } + if !ctx.Repo.CanUseTimetracker(issue, ctx.User) { if !ctx.Repo.Repository.IsTimetrackerEnabled() { ctx.JSON(http.StatusBadRequest, struct{ Message string }{Message: "time tracking disabled"}) From aca5ad2d99244759cdc1fab8bfa2623fb5f7926b Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 20 Nov 2021 23:45:35 +0100 Subject: [PATCH 19/71] Fix database errors --- models/issue.go | 19 +------------------ models/issue_comment.go | 13 ++++++------- 2 files changed, 7 insertions(+), 25 deletions(-) diff --git a/models/issue.go b/models/issue.go index 7781f8d203d7b..6e7fc1b2af763 100644 --- a/models/issue.go +++ b/models/issue.go @@ -193,20 +193,6 @@ func (issue *Issue) loadPoster(e db.Engine) (err error) { return } -// LoadIsPrivate loads isPrivate -func (issue *Issue) LoadIsPrivate() error { - return issue.loadIsPrivate(db.GetEngine(db.DefaultContext)) -} - -func (issue *Issue) loadIsPrivate(e db.Engine) (err error) { - var isPrivate bool - if _, err = e.Table("issue").Where("id=?", issue.ID).Cols("is_private").Get(&isPrivate); err != nil { - return - } - issue.IsPrivate = isPrivate - return -} - func (issue *Issue) loadPullRequest(e db.Engine) (err error) { if issue.IsPull && issue.PullRequest == nil { issue.PullRequest, err = getPullRequestByIssueID(e, issue.ID) @@ -329,6 +315,7 @@ func (issue *Issue) loadAttributes(ctx context.Context) (err error) { } if err = issue.loadComments(e); err != nil { + return err } @@ -341,10 +328,6 @@ func (issue *Issue) loadAttributes(ctx context.Context) (err error) { } } - if err = issue.loadIsPrivate(e); err != nil { - return err - } - return issue.loadReactions(e) } diff --git a/models/issue_comment.go b/models/issue_comment.go index bf5095ef753e8..b1fae730f2113 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -1053,17 +1053,17 @@ func (opts *FindCommentsOptions) toConds() builder.Cond { ), ) } else { - cond = cond.And(builder.Eq{"issue.is_private": false}) + cond = cond.And(builder.Eq{"`issue`.is_private": false}) } + return cond } func findComments(e db.Engine, opts *FindCommentsOptions) ([]*Comment, error) { comments := make([]*Comment, 0, 10) sess := e.Where(opts.toConds()) - if opts.RepoID > 0 { - sess.Join("INNER", "issue", "issue.id = comment.issue_id") - } + + sess.Join("INNER", "issue", "issue.id = comment.issue_id") if opts.Page != 0 { sess = db.SetSessionPagination(sess, opts) @@ -1085,9 +1085,8 @@ func FindComments(opts *FindCommentsOptions) ([]*Comment, error) { // CountComments count all comments according options by ignoring pagination func CountComments(opts *FindCommentsOptions) (int64, error) { sess := db.GetEngine(db.DefaultContext).Where(opts.toConds()) - if opts.RepoID > 0 { - sess.Join("INNER", "issue", "issue.id = comment.issue_id") - } + sess.Join("INNER", "issue", "issue.id = comment.issue_id") + return sess.Count(&Comment{}) } From cb5ebf6c32bf98bdeea39ea1d29076bc6f9e9ebb Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 20 Nov 2021 23:45:46 +0100 Subject: [PATCH 20/71] Fix api not found --- routers/api/v1/repo/issue.go | 8 +++---- routers/api/v1/repo/issue_comment.go | 29 +++++++++++++---------- routers/api/v1/repo/issue_label.go | 8 +++---- routers/api/v1/repo/issue_reaction.go | 4 ++-- routers/api/v1/repo/issue_subscription.go | 6 ++--- routers/api/v1/repo/issue_tracked_time.go | 16 ++++++------- 6 files changed, 38 insertions(+), 33 deletions(-) diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 3272a7f89f238..1e480cb55f214 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -556,7 +556,7 @@ func GetIssue(ctx *context.APIContext) { } if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } @@ -729,13 +729,13 @@ func EditIssue(ctx *context.APIContext) { } return } - issue.Repo = ctx.Repo.Repository if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } + issue.Repo = ctx.Repo.Repository canWrite := ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) err = issue.LoadAttributes() @@ -903,7 +903,7 @@ func UpdateIssueDeadline(ctx *context.APIContext) { } if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go index 82d2f4be41e52..c0e4bfb7ba7e7 100644 --- a/routers/api/v1/repo/issue_comment.go +++ b/routers/api/v1/repo/issue_comment.go @@ -63,24 +63,20 @@ func ListIssueComments(ctx *context.APIContext) { } issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { - ctx.Error(http.StatusInternalServerError, "GetRawIssueByIndex", err) + if models.IsErrIssueNotExist(err) { + ctx.NotFound() + } else { + ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err) + } return } - issue.Repo = ctx.Repo.Repository - if err = issue.LoadPoster(); err != nil { - ctx.InternalServerError(err) - return - } - if err = issue.LoadIsPrivate(); err != nil { - ctx.InternalServerError(err) - return - } if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } + issue.Repo = ctx.Repo.Repository opts := &models.FindCommentsOptions{ IssueID: issue.ID, Since: since, @@ -252,7 +248,16 @@ func CreateIssueComment(ctx *context.APIContext) { form := web.GetForm(ctx).(*api.CreateIssueCommentOption) issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { - ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err) + if models.IsErrIssueNotExist(err) { + ctx.NotFound() + } else { + ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err) + } + return + } + + if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + ctx.NotFound() return } diff --git a/routers/api/v1/repo/issue_label.go b/routers/api/v1/repo/issue_label.go index 80d9b64c42e11..df406e19fba91 100644 --- a/routers/api/v1/repo/issue_label.go +++ b/routers/api/v1/repo/issue_label.go @@ -62,7 +62,7 @@ func ListIssueLabels(ctx *context.APIContext) { } if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } @@ -174,7 +174,7 @@ func DeleteIssueLabel(ctx *context.APIContext) { } if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } @@ -297,7 +297,7 @@ func ClearIssueLabels(ctx *context.APIContext) { } if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } @@ -326,7 +326,7 @@ func prepareForReplaceOrAdd(ctx *context.APIContext, form api.IssueLabelsOption) } if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } diff --git a/routers/api/v1/repo/issue_reaction.go b/routers/api/v1/repo/issue_reaction.go index e094595949c17..d1a295029d6b1 100644 --- a/routers/api/v1/repo/issue_reaction.go +++ b/routers/api/v1/repo/issue_reaction.go @@ -290,7 +290,7 @@ func GetIssueReactions(ctx *context.APIContext) { } if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } @@ -414,7 +414,7 @@ func changeIssueReaction(ctx *context.APIContext, form api.EditReactionOption, i } if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } diff --git a/routers/api/v1/repo/issue_subscription.go b/routers/api/v1/repo/issue_subscription.go index 9453bafc367eb..c49b312afa530 100644 --- a/routers/api/v1/repo/issue_subscription.go +++ b/routers/api/v1/repo/issue_subscription.go @@ -115,7 +115,7 @@ func setIssueSubscription(ctx *context.APIContext, watch bool) { } if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } @@ -201,7 +201,7 @@ func CheckIssueSubscription(ctx *context.APIContext) { } if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } @@ -272,7 +272,7 @@ func GetIssueSubscribers(ctx *context.APIContext) { } if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } diff --git a/routers/api/v1/repo/issue_tracked_time.go b/routers/api/v1/repo/issue_tracked_time.go index 48e342d578df5..6ff8adb304376 100644 --- a/routers/api/v1/repo/issue_tracked_time.go +++ b/routers/api/v1/repo/issue_tracked_time.go @@ -77,7 +77,7 @@ func ListTrackedTimes(ctx *context.APIContext) { issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { if models.IsErrIssueNotExist(err) { - ctx.NotFound(err) + ctx.NotFound() } else { ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err) } @@ -85,7 +85,7 @@ func ListTrackedTimes(ctx *context.APIContext) { } if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } @@ -186,7 +186,7 @@ func AddTime(ctx *context.APIContext) { issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { if models.IsErrIssueNotExist(err) { - ctx.NotFound(err) + ctx.NotFound() } else { ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err) } @@ -194,7 +194,7 @@ func AddTime(ctx *context.APIContext) { } if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } @@ -272,7 +272,7 @@ func ResetIssueTime(ctx *context.APIContext) { issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { if models.IsErrIssueNotExist(err) { - ctx.NotFound(err) + ctx.NotFound() } else { ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err) } @@ -280,7 +280,7 @@ func ResetIssueTime(ctx *context.APIContext) { } if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } @@ -348,7 +348,7 @@ func DeleteTime(ctx *context.APIContext) { issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { if models.IsErrIssueNotExist(err) { - ctx.NotFound(err) + ctx.NotFound() } else { ctx.Error(http.StatusInternalServerError, "GetIssueByIndex", err) } @@ -356,7 +356,7 @@ func DeleteTime(ctx *context.APIContext) { } if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { - ctx.Status(http.StatusNotFound) + ctx.NotFound() return } From ba762be6fc70d58e534df14da943c4612e2e8d97 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 00:39:30 +0100 Subject: [PATCH 21/71] Allow API to create confidential issues --- modules/structs/issue.go | 2 ++ routers/api/v1/repo/issue.go | 1 + templates/swagger/v1_json.tmpl | 5 +++++ 3 files changed, 8 insertions(+) diff --git a/modules/structs/issue.go b/modules/structs/issue.go index a4a5baa90fdb9..406ac713227b6 100644 --- a/modules/structs/issue.go +++ b/modules/structs/issue.go @@ -89,6 +89,8 @@ type CreateIssueOption struct { // list of label ids Labels []int64 `json:"labels"` Closed bool `json:"closed"` + // mark if the issue is confidential + IsConfidential bool `json:"is_confidential"` } // EditIssueOption options for editing an issue diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 1e480cb55f214..873e4d64e49a1 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -611,6 +611,7 @@ func CreateIssue(ctx *context.APIContext) { Content: form.Body, Ref: form.Ref, DeadlineUnix: deadlineUnix, + IsPrivate: form.IsConfidential, } var assigneeIDs = make([]int64, 0) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 6bbc0934811b6..a4f3693aa90da 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -13419,6 +13419,11 @@ "format": "date-time", "x-go-name": "Deadline" }, + "is_confidential": { + "description": "mark if the issue is confidential", + "type": "boolean", + "x-go-name": "IsConfidential" + }, "labels": { "description": "list of label ids", "type": "array", From c784db7cf961af832006211047496775fbce1554 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 04:57:22 +0100 Subject: [PATCH 22/71] Fix copy pasta --- templates/org/team/new.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/org/team/new.tmpl b/templates/org/team/new.tmpl index 35aded1ecab3e..27b8bb1c34d4a 100644 --- a/templates/org/team/new.tmpl +++ b/templates/org/team/new.tmpl @@ -75,7 +75,7 @@
-
can_see_private_issues +
{{.i18n.Tr "org.teams.can_see_private_issues_helper"}} From 23be89dd5c65cfbe1678730ffd492529e87928fb Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 04:57:55 +0100 Subject: [PATCH 23/71] Fix notifications checking (I think?) --- models/action.go | 29 +++++++++++++------------- models/notification.go | 10 +++++---- models/repo_watch.go | 7 +++++++ modules/notification/action/action.go | 30 ++++++++++++++------------- 4 files changed, 44 insertions(+), 32 deletions(-) diff --git a/models/action.go b/models/action.go index d790cd6678f5c..ca137239bf31c 100644 --- a/models/action.go +++ b/models/action.go @@ -60,20 +60,21 @@ const ( // repository. It implemented interface base.Actioner so that can be // used in template render. type Action struct { - ID int64 `xorm:"pk autoincr"` - UserID int64 `xorm:"INDEX"` // Receiver user id. - OpType ActionType - ActUserID int64 `xorm:"INDEX"` // Action user id. - ActUser *User `xorm:"-"` - RepoID int64 `xorm:"INDEX"` - Repo *Repository `xorm:"-"` - CommentID int64 `xorm:"INDEX"` - Comment *Comment `xorm:"-"` - IsDeleted bool `xorm:"INDEX NOT NULL DEFAULT false"` - RefName string - IsPrivate bool `xorm:"INDEX NOT NULL DEFAULT false"` - Content string `xorm:"TEXT"` - CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"INDEX"` // Receiver user id. + OpType ActionType + ActUserID int64 `xorm:"INDEX"` // Action user id. + ActUser *User `xorm:"-"` + RepoID int64 `xorm:"INDEX"` + Repo *Repository `xorm:"-"` + CommentID int64 `xorm:"INDEX"` + Comment *Comment `xorm:"-"` + IsDeleted bool `xorm:"INDEX NOT NULL DEFAULT false"` + RefName string + IsPrivate bool `xorm:"INDEX NOT NULL DEFAULT false"` + Content string `xorm:"TEXT"` + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + IsIssuePrivate bool `xorm:"INDEX NOT NULL DEFAULT false"` } func init() { diff --git a/models/notification.go b/models/notification.go index 3c252d123d3ec..bf07765340eb1 100644 --- a/models/notification.go +++ b/models/notification.go @@ -11,7 +11,6 @@ import ( "strconv" "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" @@ -265,10 +264,13 @@ func createOrUpdateIssueNotifications(e db.Engine, issueID, commentID, notificat return err } - if issue.IsPull && !issue.Repo.checkUnitUser(e, user, unit.TypePullRequests) { - continue + perm, err := getUserRepoPermission(e, issue.Repo, user) + if err != nil { + log.Error("getUserRepoPermission(): %v", err) + return err } - if !issue.IsPull && !issue.Repo.checkUnitUser(e, user, unit.TypeIssues) { + + if !perm.CanReadIssuesOrPulls(issue.IsPull) || (issue.IsPrivate && !(issue.PosterID == userID || perm.CanReadPrivateIssues())) { continue } diff --git a/models/repo_watch.go b/models/repo_watch.go index 55fb08a426532..7b36f21839b32 100644 --- a/models/repo_watch.go +++ b/models/repo_watch.go @@ -188,6 +188,7 @@ func notifyWatchers(e db.Engine, actions ...*Action) error { var permCode []bool var permIssue []bool var permPR []bool + var permPrivateIssue []bool for _, act := range actions { repoChanged := repo == nil || repo.ID != act.RepoID @@ -231,6 +232,7 @@ func notifyWatchers(e db.Engine, actions ...*Action) error { permCode = make([]bool, len(watchers)) permIssue = make([]bool, len(watchers)) permPR = make([]bool, len(watchers)) + permPrivateIssue = make([]bool, len(watchers)) for i, watcher := range watchers { user, err := getUserByID(e, watcher.UserID) if err != nil { @@ -249,6 +251,7 @@ func notifyWatchers(e db.Engine, actions ...*Action) error { permCode[i] = perm.CanRead(unit.TypeCode) permIssue[i] = perm.CanRead(unit.TypeIssues) permPR[i] = perm.CanRead(unit.TypePullRequests) + permPrivateIssue[i] = perm.CanReadPrivateIssues() } } @@ -260,6 +263,10 @@ func notifyWatchers(e db.Engine, actions ...*Action) error { act.UserID = watcher.UserID act.Repo.Units = nil + if act.IsIssuePrivate && !permPrivateIssue[i] { + continue + } + switch act.OpType { case ActionCommitRepo, ActionPushTag, ActionDeleteTag, ActionPublishRelease, ActionDeleteBranch: if !permCode[i] { diff --git a/modules/notification/action/action.go b/modules/notification/action/action.go index 772e7be13705d..a6b223068ff0d 100644 --- a/modules/notification/action/action.go +++ b/modules/notification/action/action.go @@ -41,13 +41,14 @@ func (a *actionNotifier) NotifyNewIssue(issue *models.Issue, mentions []*models. repo := issue.Repo if err := models.NotifyWatchers(&models.Action{ - ActUserID: issue.Poster.ID, - ActUser: issue.Poster, - OpType: models.ActionCreateIssue, - Content: fmt.Sprintf("%d|%s", issue.Index, issue.Title), - RepoID: repo.ID, - Repo: repo, - IsPrivate: repo.IsPrivate, + ActUserID: issue.Poster.ID, + ActUser: issue.Poster, + OpType: models.ActionCreateIssue, + Content: fmt.Sprintf("%d|%s", issue.Index, issue.Title), + RepoID: repo.ID, + Repo: repo, + IsPrivate: repo.IsPrivate, + IsIssuePrivate: issue.IsPrivate, }); err != nil { log.Error("NotifyWatchers: %v", err) } @@ -90,13 +91,14 @@ func (a *actionNotifier) NotifyIssueChangeStatus(doer *models.User, issue *model func (a *actionNotifier) NotifyCreateIssueComment(doer *models.User, repo *models.Repository, issue *models.Issue, comment *models.Comment, mentions []*models.User) { act := &models.Action{ - ActUserID: doer.ID, - ActUser: doer, - RepoID: issue.Repo.ID, - Repo: issue.Repo, - Comment: comment, - CommentID: comment.ID, - IsPrivate: issue.Repo.IsPrivate, + ActUserID: doer.ID, + ActUser: doer, + RepoID: issue.Repo.ID, + Repo: issue.Repo, + Comment: comment, + CommentID: comment.ID, + IsPrivate: issue.Repo.IsPrivate, + IsIssuePrivate: issue.IsPrivate, } content := "" From 9fd110f59687bb642c89020cb4e9200c304345bd Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 04:58:13 +0100 Subject: [PATCH 24/71] Fix permission checking --- models/issue.go | 67 +++++++++++++++++++++++++++------------ models/issue_comment.go | 26 ++++++++------- routers/web/repo/issue.go | 14 +++++--- 3 files changed, 70 insertions(+), 37 deletions(-) diff --git a/models/issue.go b/models/issue.go index 6e7fc1b2af763..b0762704ce4e9 100644 --- a/models/issue.go +++ b/models/issue.go @@ -315,7 +315,6 @@ func (issue *Issue) loadAttributes(ctx context.Context) (err error) { } if err = issue.loadComments(e); err != nil { - return err } @@ -336,6 +335,25 @@ func (issue *Issue) LoadAttributes() error { return issue.loadAttributes(db.DefaultContext) } +// LoadCommentsAsUser loads the comment of the issue, as the user. +func (issue *Issue) LoadCommentsAsUser(user *User, canSeePrivateIssues bool) error { + return issue.loadCommentsAsUser(db.GetEngine(db.DefaultContext), user, canSeePrivateIssues) +} + +func (issue *Issue) loadCommentsAsUser(e db.Engine, user *User, canSeePrivateIssues bool) (err error) { + var userID int64 + if user != nil { + userID = user.ID + } + issue.Comments, err = findComments(e, &FindCommentsOptions{ + IssueID: issue.ID, + Type: CommentTypeUnknown, + UserID: userID, + CanSeePrivate: canSeePrivateIssues, + }) + return err +} + // LoadMilestone load milestone of this issue. func (issue *Issue) LoadMilestone() error { return issue.loadMilestone(db.GetEngine(db.DefaultContext)) @@ -1218,7 +1236,6 @@ type IssuesOptions struct { PriorityRepoID int64 IsArchived util.OptionalBool UserID int64 - CanSeePrivate bool } // sortIssuesSession sort an issues-related session based on the provided @@ -1324,21 +1341,20 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { } } - if !opts.CanSeePrivate { - if opts.UserID == 0 { - sess.And("issue.is_private=?", false) - } else { - // Allow to see private issues if the user is the poster of it. - sess.And( - builder.Or( - builder.Eq{"`issue`.is_private": false}, - builder.And( - builder.Eq{"`issue`.is_private": true}, - builder.In("`issue`.poster_id", opts.UserID), - ), + if opts.UserID == 0 { + sess.And("issue.is_private=?", false) + } else { + // Allow to see private issues if the user is the poster of it. + sess.And( + builder.Or( + builder.Eq{"`issue`.is_private": false}, + builder.And( + builder.Eq{"`issue`.is_private": true}, + builder.In("`issue`.poster_id", opts.UserID), ), - ) - } + ), + ) + } switch opts.IsPull { @@ -1667,9 +1683,7 @@ func getIssueStatsChunk(opts *IssueStatsOptions, issueIDs []int64, userID int64) } if !opts.CanSeePrivate { - if userID == 0 { - sess.And("issue.is_private=?", false) - } else { + if userID != 0 { // Allow to see private issues if the user is the poster of it. sess.And( builder.Or( @@ -1680,6 +1694,9 @@ func getIssueStatsChunk(opts *IssueStatsOptions, issueIDs []int64, userID int64) ), ), ) + + } else { + sess.And("issue.is_private=?", false) } } @@ -1743,6 +1760,16 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) { s.Join("INNER", "repository", "issue.repo_id = repository.id"). And(builder.Eq{"repository.is_archived": opts.IsArchived.IsTrue()}) } + // Allow to see comments on private issues + s.And( + builder.Or( + builder.Eq{"`issue`.is_private": false}, + builder.And( + builder.Eq{"`issue`.is_private": true}, + builder.In("`issue`.poster_id", opts.UserID), + ), + ), + ) return s } @@ -2241,7 +2268,7 @@ func (issue *Issue) ResolveMentionsByVisibility(ctx context.Context, doer *User, if err != nil { return nil, fmt.Errorf("getUserRepoPermission [%d]: %v", user.ID, err) } - if !perm.CanReadIssuesOrPulls(issue.IsPull) { + if !perm.CanReadIssuesOrPulls(issue.IsPull) || (issue.IsPrivate && !(issue.IsPoster(user.ID) || perm.CanReadPrivateIssues())) { continue } users = append(users, user) diff --git a/models/issue_comment.go b/models/issue_comment.go index b1fae730f2113..20a9cfdcbfd21 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -1041,19 +1041,21 @@ func (opts *FindCommentsOptions) toConds() builder.Cond { if len(opts.TreePath) > 0 { cond = cond.And(builder.Eq{"comment.tree_path": opts.TreePath}) } - if opts.CanSeePrivate { - // Allow to see comments on private issues - cond = cond.And( - builder.Or( - builder.Eq{"`issue`.is_private": false}, - builder.And( - builder.Eq{"`issue`.is_private": true}, - builder.In("`issue`.poster_id", opts.UserID), + if !opts.CanSeePrivate { + if opts.UserID != 0 { + // Allow to see comments on private issues + cond = cond.And( + builder.Or( + builder.Eq{"`issue`.is_private": false}, + builder.And( + builder.Eq{"`issue`.is_private": true}, + builder.In("`issue`.poster_id", opts.UserID), + ), ), - ), - ) - } else { - cond = cond.And(builder.Eq{"`issue`.is_private": false}) + ) + } else { + cond = cond.And(builder.Eq{"`issue`.is_private": false}) + } } return cond diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 76b628b87458c..f29c89a9f103d 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -170,8 +170,6 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti } } - canSeePrivateIssues := ctx.Repo.CanReadPrivateIssues() - var userID int64 if ctx.IsSigned { userID = ctx.User.ID @@ -191,7 +189,7 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti ReviewRequestedID: reviewRequestedID, IsPull: isPullOption, IssueIDs: issueIDs, - CanSeePrivate: canSeePrivateIssues, + CanSeePrivate: ctx.Repo.CanReadPrivateIssues(), }, userID) if err != nil { ctx.ServerError("GetIssueStats", err) @@ -244,7 +242,6 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti LabelIDs: labelIDs, SortType: sortType, IssueIDs: issueIDs, - CanSeePrivate: canSeePrivateIssues, UserID: userID, }) if err != nil { @@ -1177,6 +1174,13 @@ func ViewIssue(ctx *context.Context) { return } + if issue.IsPrivate { + if err = issue.LoadCommentsAsUser(ctx.User, ctx.Repo.CanReadPrivateIssues()); err != nil { + ctx.ServerError("LoadCommentsAsUser", err) + return + } + } + if err = filterXRefComments(ctx, issue); err != nil { ctx.ServerError("filterXRefComments", err) return @@ -2090,7 +2094,7 @@ func NewComment(ctx *context.Context) { return } - if !ctx.IsSigned || (ctx.User.ID != issue.PosterID && !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull)) { + if !ctx.IsSigned || (ctx.User.ID != issue.PosterID && !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull)) || issue.IsPrivate && !(ctx.User.ID != issue.PosterID || ctx.Repo.CanReadPrivateIssues()) { if log.IsTrace() { if ctx.IsSigned { issueType := "issues" From 8e2b43baa5513e2ddc055ed4ddc06c9e30f7c15c Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 04:58:18 +0100 Subject: [PATCH 25/71] Simplify --- routers/api/v1/repo/issue_comment.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go index c0e4bfb7ba7e7..be7b7ca93e674 100644 --- a/routers/api/v1/repo/issue_comment.go +++ b/routers/api/v1/repo/issue_comment.go @@ -455,13 +455,12 @@ func editIssueComment(ctx *context.APIContext, form api.EditIssueCommentOption) } if comment.Issue.IsPrivate { - canSeePrivateIssues := ctx.Repo.CanReadPrivateIssues() var userID int64 if ctx.IsSigned { userID = ctx.User.ID } - if !(comment.Issue.PosterID == userID || canSeePrivateIssues) { + if !(comment.Issue.PosterID == userID || ctx.Repo.CanReadPrivateIssues()) { ctx.Status(http.StatusNotFound) return } From 87792d350cd4b15e7ee7cbc9d057f2ad665f5e82 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 05:05:38 +0100 Subject: [PATCH 26/71] Fix mail permission checking --- services/mailer/mail_issue.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/services/mailer/mail_issue.go b/services/mailer/mail_issue.go index 6e631627136c8..4a5451feeff48 100644 --- a/services/mailer/mail_issue.go +++ b/services/mailer/mail_issue.go @@ -8,7 +8,6 @@ import ( "fmt" "code.gitea.io/gitea/models" - "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" ) @@ -115,11 +114,6 @@ func mailIssueCommentToParticipants(ctx *mailCommentContext, mentions []*models. } func mailIssueCommentBatch(ctx *mailCommentContext, users []*models.User, visited map[int64]bool, fromMention bool) error { - checkUnit := unit.TypeIssues - if ctx.Issue.IsPull { - checkUnit = unit.TypePullRequests - } - langMap := make(map[string][]*models.User) for _, user := range users { // At this point we exclude: @@ -138,7 +132,13 @@ func mailIssueCommentBatch(ctx *mailCommentContext, users []*models.User, visite visited[user.ID] = true // test if this user is allowed to see the issue/pull - if !ctx.Issue.Repo.CheckUnitUser(user, checkUnit) { + perm, err := models.GetUserRepoPermission(ctx.Issue.Repo, user) + if err != nil { + log.Error("getUserRepoPermission(): %v", err) + return err + } + + if !perm.CanReadIssuesOrPulls(ctx.Issue.IsPull) || (ctx.Issue.IsPrivate && !(ctx.Issue.PosterID == user.ID || perm.CanReadPrivateIssues())) { continue } From 9c396b8ad7596695b0eba652514f00e2bdbd3075 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 05:14:04 +0100 Subject: [PATCH 27/71] Disallow private issues on webhooks --- modules/notification/webhook/webhook.go | 40 +++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index de6e19a06551c..ac0fe2a2b4d67 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -42,6 +42,10 @@ func (m *webhookNotifier) NotifyIssueClearLabels(doer *models.User, issue *model return } + if issue.IsPrivate { + return + } + mode, _ := models.AccessLevel(issue.Poster, issue.Repo) var err error if issue.IsPull { @@ -137,6 +141,10 @@ func (m *webhookNotifier) NotifyMigrateRepository(doer *models.User, u *models.U } func (m *webhookNotifier) NotifyIssueChangeAssignee(doer *models.User, issue *models.Issue, assignee *models.User, removed bool, comment *models.Comment) { + if issue.IsPrivate { + return + } + if issue.IsPull { mode, _ := models.AccessLevelUnit(doer, issue.Repo, unit.TypePullRequests) @@ -183,6 +191,10 @@ func (m *webhookNotifier) NotifyIssueChangeAssignee(doer *models.User, issue *mo } func (m *webhookNotifier) NotifyIssueChangeTitle(doer *models.User, issue *models.Issue, oldTitle string) { + if issue.IsPrivate { + return + } + mode, _ := models.AccessLevel(issue.Poster, issue.Repo) var err error if issue.IsPull { @@ -224,6 +236,10 @@ func (m *webhookNotifier) NotifyIssueChangeTitle(doer *models.User, issue *model } func (m *webhookNotifier) NotifyIssueChangeStatus(doer *models.User, issue *models.Issue, actionComment *models.Comment, isClosed bool) { + if issue.IsPrivate { + return + } + mode, _ := models.AccessLevel(issue.Poster, issue.Repo) var err error if issue.IsPull { @@ -272,6 +288,9 @@ func (m *webhookNotifier) NotifyNewIssue(issue *models.Issue, mentions []*models log.Error("issue.LoadPoster: %v", err) return } + if issue.IsPrivate { + return + } mode, _ := models.AccessLevel(issue.Poster, issue.Repo) if err := webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventIssues, &api.IssuePayload{ @@ -314,6 +333,9 @@ func (m *webhookNotifier) NotifyNewPullRequest(pull *models.PullRequest, mention func (m *webhookNotifier) NotifyIssueChangeContent(doer *models.User, issue *models.Issue, oldContent string) { mode, _ := models.AccessLevel(issue.Poster, issue.Repo) var err error + if issue.IsPrivate { + return + } if issue.IsPull { issue.PullRequest.Issue = issue err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventPullRequest, &api.PullRequestPayload{ @@ -363,6 +385,9 @@ func (m *webhookNotifier) NotifyUpdateComment(doer *models.User, c *models.Comme log.Error("LoadAttributes: %v", err) return } + if c.Issue.IsPrivate { + return + } mode, _ := models.AccessLevel(doer, c.Issue.Repo) if c.Issue.IsPull { @@ -404,6 +429,10 @@ func (m *webhookNotifier) NotifyCreateIssueComment(doer *models.User, repo *mode issue *models.Issue, comment *models.Comment, mentions []*models.User) { mode, _ := models.AccessLevel(doer, repo) + if issue.IsPrivate { + return + } + var err error if issue.IsPull { err = webhook_services.PrepareWebhooks(issue.Repo, webhook.HookEventPullRequestComment, &api.IssueCommentPayload{ @@ -447,6 +476,10 @@ func (m *webhookNotifier) NotifyDeleteComment(doer *models.User, comment *models return } + if comment.Issue.IsPrivate { + return + } + mode, _ := models.AccessLevel(doer, comment.Issue.Repo) if comment.Issue.IsPull { @@ -489,6 +522,10 @@ func (m *webhookNotifier) NotifyIssueChangeLabels(doer *models.User, issue *mode return } + if issue.IsPrivate { + return + } + mode, _ := models.AccessLevel(issue.Poster, issue.Repo) if issue.IsPull { if err = issue.LoadPullRequest(); err != nil { @@ -529,6 +566,9 @@ func (m *webhookNotifier) NotifyIssueChangeMilestone(doer *models.User, issue *m hookAction = api.HookIssueDemilestoned } + if issue.IsPrivate { + return + } if err = issue.LoadAttributes(); err != nil { log.Error("issue.LoadAttributes failed: %v", err) return From d4ebb38a2e19beb9afa69c5f67951019c81cc0d5 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 05:25:47 +0100 Subject: [PATCH 28/71] Migration: set owner's team to see private issues --- models/migrations/migrations.go | 2 ++ models/migrations/v203.go | 15 +++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 models/migrations/v203.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 2b87a38cd62c2..78aa0b641e31a 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -359,6 +359,8 @@ var migrations = []Migration{ NewMigration("Drop table remote_version (if exists)", dropTableRemoteVersion), // v202 -> v203 NewMigration("Add private issues to Repository table", addPrivateIssuesToRepo), + // v203 -> v204 + NewMigration("Set Owners team to acess private issues", setOwnersTeamToSeePrivateIssues), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v203.go b/models/migrations/v203.go new file mode 100644 index 0000000000000..de44c1b9d5362 --- /dev/null +++ b/models/migrations/v203.go @@ -0,0 +1,15 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "xorm.io/xorm" +) + +func setOwnersTeamToSeePrivateIssues(x *xorm.Engine) error { + _, err := x.Exec("UPDATE `team` SET `can_see_private_issues` = ? WHERE `name`=?", + true, "Owners") + return err +} From 8f5890831ffcd6e1bdf7a08889fa4917625ba2f0 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 05:35:36 +0100 Subject: [PATCH 29/71] Fix unit tests --- models/issue_comment.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index 20a9cfdcbfd21..83fed270d91ca 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -1064,7 +1064,6 @@ func (opts *FindCommentsOptions) toConds() builder.Cond { func findComments(e db.Engine, opts *FindCommentsOptions) ([]*Comment, error) { comments := make([]*Comment, 0, 10) sess := e.Where(opts.toConds()) - sess.Join("INNER", "issue", "issue.id = comment.issue_id") if opts.Page != 0 { @@ -1202,7 +1201,9 @@ func findCodeComments(e db.Engine, opts FindCommentsOptions, issue *Issue, curre if review.ID == 0 { conds = conds.And(builder.Eq{"invalidated": false}) } + if err := e.Where(conds). + Join("INNER", "issue", "issue.id = comment.issue_id"). Asc("comment.created_unix"). Asc("comment.id"). Find(&comments); err != nil { From 1714bf5659e2731ed074d132dd6d6e1581b070cb Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 06:11:27 +0100 Subject: [PATCH 30/71] Fix migration --- models/migrations/v203.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/models/migrations/v203.go b/models/migrations/v203.go index de44c1b9d5362..7a2610b4cd228 100644 --- a/models/migrations/v203.go +++ b/models/migrations/v203.go @@ -9,6 +9,15 @@ import ( ) func setOwnersTeamToSeePrivateIssues(x *xorm.Engine) error { + type Team struct { + ID int64 `xorm:"pk autoincr"` + IncludesAllRepositories bool `xorm:"NOT NULL DEFAULT false"` + } + + if err := x.Sync2(new(Team)); err != nil { + return err + } + _, err := x.Exec("UPDATE `team` SET `can_see_private_issues` = ? WHERE `name`=?", true, "Owners") return err From 6c1f279cd86e20c49030ba5e5fe0ad1a9b357b4f Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 14:33:21 +0100 Subject: [PATCH 31/71] Typo in field --- models/migrations/v203.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/migrations/v203.go b/models/migrations/v203.go index 7a2610b4cd228..c9806a6939c4a 100644 --- a/models/migrations/v203.go +++ b/models/migrations/v203.go @@ -10,8 +10,8 @@ import ( func setOwnersTeamToSeePrivateIssues(x *xorm.Engine) error { type Team struct { - ID int64 `xorm:"pk autoincr"` - IncludesAllRepositories bool `xorm:"NOT NULL DEFAULT false"` + ID int64 `xorm:"pk autoincr"` + CanSeePrivateIssues bool `xorm:"NOT NULL DEFAULT false"` } if err := x.Sync2(new(Team)); err != nil { From 790975f8103e219826355f2209a43668dbb7c478 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 21:21:03 +0100 Subject: [PATCH 32/71] Fix issues loading for admins --- models/issue.go | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/models/issue.go b/models/issue.go index b0762704ce4e9..070b7256151c2 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1222,6 +1222,7 @@ type IssuesOptions struct { MilestoneIDs []int64 ProjectID int64 ProjectBoardID int64 + UserID int64 IsClosed util.OptionalBool IsPull util.OptionalBool LabelIDs []int64 @@ -1235,7 +1236,7 @@ type IssuesOptions struct { // prioritize issues from this repo PriorityRepoID int64 IsArchived util.OptionalBool - UserID int64 + CanSeePrivate bool } // sortIssuesSession sort an issues-related session based on the provided @@ -1341,20 +1342,21 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { } } - if opts.UserID == 0 { - sess.And("issue.is_private=?", false) - } else { - // Allow to see private issues if the user is the poster of it. - sess.And( - builder.Or( - builder.Eq{"`issue`.is_private": false}, - builder.And( - builder.Eq{"`issue`.is_private": true}, - builder.In("`issue`.poster_id", opts.UserID), + if !opts.CanSeePrivate { + if opts.UserID == 0 { + sess.And("issue.is_private=?", false) + } else { + // Allow to see private issues if the user is the poster of it. + sess.And( + builder.Or( + builder.Eq{"`issue`.is_private": false}, + builder.And( + builder.Eq{"`issue`.is_private": true}, + builder.In("`issue`.poster_id", opts.UserID), + ), ), - ), - ) - + ) + } } switch opts.IsPull { From 44a4b63521afb1f5186834764600c15ce8496b1d Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 21:21:19 +0100 Subject: [PATCH 33/71] Load correct issues on projects --- models/project_board.go | 18 ++++++++++++++---- routers/web/repo/issue.go | 2 -- routers/web/repo/projects.go | 11 +++++++++-- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/models/project_board.go b/models/project_board.go index e6c9f0338632b..bb5d1b02c8cc9 100644 --- a/models/project_board.go +++ b/models/project_board.go @@ -52,7 +52,8 @@ type ProjectBoard struct { CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` - Issues []*Issue `xorm:"-"` + Issues []*Issue `xorm:"-"` + Repo *Repository `xorm:"-"` } func init() { @@ -259,13 +260,15 @@ func SetDefaultBoard(projectID, boardID int64) error { } // LoadIssues load issues assigned to this board -func (b *ProjectBoard) LoadIssues() (IssueList, error) { +func (b *ProjectBoard) LoadIssues(opts *LoadIssuesOpts) (IssueList, error) { issueList := make([]*Issue, 0, 10) if b.ID != 0 { issues, err := Issues(&IssuesOptions{ ProjectBoardID: b.ID, ProjectID: b.ProjectID, + CanSeePrivate: opts.CanSeePrivateIssues, + UserID: opts.UserID, }) if err != nil { return nil, err @@ -277,6 +280,8 @@ func (b *ProjectBoard) LoadIssues() (IssueList, error) { issues, err := Issues(&IssuesOptions{ ProjectBoardID: -1, // Issues without ProjectBoardID ProjectID: b.ProjectID, + CanSeePrivate: opts.CanSeePrivateIssues, + UserID: opts.UserID, }) if err != nil { return nil, err @@ -292,11 +297,16 @@ func (b *ProjectBoard) LoadIssues() (IssueList, error) { return issueList, nil } +type LoadIssuesOpts struct { + UserID int64 + CanSeePrivateIssues bool +} + // LoadIssues load issues assigned to the boards -func (bs ProjectBoardList) LoadIssues() (IssueList, error) { +func (bs ProjectBoardList) LoadIssues(opts *LoadIssuesOpts) (IssueList, error) { issues := make(IssueList, 0, len(bs)*10) for i := range bs { - il, err := bs[i].LoadIssues() + il, err := bs[i].LoadIssues(opts) if err != nil { return nil, err } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index f29c89a9f103d..265f2e889df5b 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1118,8 +1118,6 @@ func ViewIssue(ctx *context.Context) { var userID int64 if ctx.IsSigned { userID = ctx.User.ID - } else { - userID = -1 } ctx.NotFound("CanSeePrivateIssues", models.ErrCannotSeePrivateIssue{ UserID: userID, diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index 437da14d45dcf..db2a731433c3d 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -269,7 +269,6 @@ func EditProjectPost(ctx *context.Context) { // ViewProject renders the project board for a project func ViewProject(ctx *context.Context) { - project, err := models.GetProjectByID(ctx.ParamsInt64(":id")) if err != nil { if models.IsErrProjectNotExist(err) { @@ -294,7 +293,15 @@ func ViewProject(ctx *context.Context) { boards[0].Title = ctx.Tr("repo.projects.type.uncategorized") } - issueList, err := boards.LoadIssues() + var userID int64 + if ctx.IsSigned { + userID = ctx.User.ID + } + + issueList, err := boards.LoadIssues(&models.LoadIssuesOpts{ + UserID: userID, + CanSeePrivateIssues: ctx.Repo.CanReadPrivateIssues(), + }) if err != nil { ctx.ServerError("LoadIssuesOfBoards", err) return From 45dcc9891656120b1125b427dafca9bc34729817 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 21:40:02 +0100 Subject: [PATCH 34/71] Actually fixup permissions --- models/issue.go | 13 +++++++------ models/issue_test.go | 2 +- routers/web/repo/issue.go | 4 +++- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/models/issue.go b/models/issue.go index 070b7256151c2..68f8fd888e30d 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1594,6 +1594,7 @@ type IssueStatsOptions struct { MentionedID int64 PosterID int64 ReviewRequestedID int64 + UserID int64 IsPull util.OptionalBool IssueIDs []int64 CanSeePrivate bool @@ -1606,9 +1607,9 @@ const ( ) // GetIssueStats returns issue statistic information by given conditions, as User -func GetIssueStats(opts *IssueStatsOptions, userID int64) (*IssueStats, error) { +func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) { if len(opts.IssueIDs) <= maxQueryParameters { - return getIssueStatsChunk(opts, opts.IssueIDs, userID) + return getIssueStatsChunk(opts, opts.IssueIDs) } // If too long a list of IDs is provided, we get the statistics in @@ -1621,7 +1622,7 @@ func GetIssueStats(opts *IssueStatsOptions, userID int64) (*IssueStats, error) { if chunk > len(opts.IssueIDs) { chunk = len(opts.IssueIDs) } - stats, err := getIssueStatsChunk(opts, opts.IssueIDs[i:chunk], userID) + stats, err := getIssueStatsChunk(opts, opts.IssueIDs[i:chunk]) if err != nil { return nil, err } @@ -1637,7 +1638,7 @@ func GetIssueStats(opts *IssueStatsOptions, userID int64) (*IssueStats, error) { return accum, nil } -func getIssueStatsChunk(opts *IssueStatsOptions, issueIDs []int64, userID int64) (*IssueStats, error) { +func getIssueStatsChunk(opts *IssueStatsOptions, issueIDs []int64) (*IssueStats, error) { stats := &IssueStats{} countSession := func(opts *IssueStatsOptions, issueIDs []int64) *xorm.Session { @@ -1685,14 +1686,14 @@ func getIssueStatsChunk(opts *IssueStatsOptions, issueIDs []int64, userID int64) } if !opts.CanSeePrivate { - if userID != 0 { + if opts.UserID != 0 { // Allow to see private issues if the user is the poster of it. sess.And( builder.Or( builder.Eq{"`issue`.is_private": false}, builder.And( builder.Eq{"`issue`.is_private": true}, - builder.In("`issue`.poster_id", userID), + builder.In("`issue`.poster_id", opts.UserID), ), ), ) diff --git a/models/issue_test.go b/models/issue_test.go index 90314e47c6cf9..7942b7e78539a 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -471,7 +471,7 @@ func TestCorrectIssueStats(t *testing.T) { issueStats, err := GetIssueStats(&IssueStatsOptions{ RepoID: 1, IssueIDs: ids, - }, 0) + }) // Now check the values. assert.NoError(t, err) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 265f2e889df5b..304f9c7e3a60f 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -189,8 +189,9 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti ReviewRequestedID: reviewRequestedID, IsPull: isPullOption, IssueIDs: issueIDs, + UserID: userID, CanSeePrivate: ctx.Repo.CanReadPrivateIssues(), - }, userID) + }) if err != nil { ctx.ServerError("GetIssueStats", err) return @@ -243,6 +244,7 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti SortType: sortType, IssueIDs: issueIDs, UserID: userID, + CanSeePrivate: ctx.Repo.CanReadPrivateIssues(), }) if err != nil { ctx.ServerError("Issues", err) From b1a1972733a5b93a582622c84b2799e829ff4d42 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 21:40:10 +0100 Subject: [PATCH 35/71] Fix projects with private issues --- models/project_issue.go | 56 +++++++++++++++++++++++++++++-- routers/web/repo/projects.go | 7 ++++ templates/repo/projects/list.tmpl | 12 +++++-- 3 files changed, 71 insertions(+), 4 deletions(-) diff --git a/models/project_issue.go b/models/project_issue.go index 75e74295b57e6..ea5070fc95ef2 100644 --- a/models/project_issue.go +++ b/models/project_issue.go @@ -103,11 +103,37 @@ func (p *Project) NumIssues() int { return int(c) } +// NumClosedPrivateOwnIssues return counter of closed private issues of the user, assigned to a project +func (p *Project) NumClosedPrivateOwnIssues(userID int64) int { + c, err := db.GetEngine(db.DefaultContext).Table("project_issue"). + Join("INNER", "issue", "project_issue.issue_id=issue.id"). + Where("project_issue.project_id=? AND issue.is_closed=? AND issue.is_private=? AND issue.poster_id=?", p.ID, true, true, userID). + Cols("issue_id"). + Count() + if err != nil { + return 0 + } + return int(c) +} + +// NumClosedPrivateIssues return counter of closed private issues assigned to a project +func (p *Project) NumClosedPrivateIssues() int { + c, err := db.GetEngine(db.DefaultContext).Table("project_issue"). + Join("INNER", "issue", "project_issue.issue_id=issue.id"). + Where("project_issue.project_id=? AND issue.is_closed=? AND issue.is_private=?", p.ID, true, true). + Cols("issue_id"). + Count() + if err != nil { + return 0 + } + return int(c) +} + // NumClosedIssues return counter of closed issues assigned to a project func (p *Project) NumClosedIssues() int { c, err := db.GetEngine(db.DefaultContext).Table("project_issue"). Join("INNER", "issue", "project_issue.issue_id=issue.id"). - Where("project_issue.project_id=? AND issue.is_closed=?", p.ID, true). + Where("project_issue.project_id=? AND issue.is_closed=? AND issue.is_private=?", p.ID, true, false). Cols("issue_id"). Count() if err != nil { @@ -120,7 +146,33 @@ func (p *Project) NumClosedIssues() int { func (p *Project) NumOpenIssues() int { c, err := db.GetEngine(db.DefaultContext).Table("project_issue"). Join("INNER", "issue", "project_issue.issue_id=issue.id"). - Where("project_issue.project_id=? AND issue.is_closed=?", p.ID, false). + Where("project_issue.project_id=? AND issue.is_closed=? AND issue.is_private=?", p.ID, false, false). + Cols("issue_id"). + Count() + if err != nil { + return 0 + } + return int(c) +} + +// NumOpenPrivateIssues return counter of open private issues assigned to a project +func (p *Project) NumOpenPrivateIssues() int { + c, err := db.GetEngine(db.DefaultContext).Table("project_issue"). + Join("INNER", "issue", "project_issue.issue_id=issue.id"). + Where("project_issue.project_id=? AND issue.is_closed=? AND issue.is_private=?", p.ID, false, true). + Cols("issue_id"). + Count() + if err != nil { + return 0 + } + return int(c) +} + +// NumOpenPrivateOwnIssues return counter of open private issues of the user, assigned to a project +func (p *Project) NumOpenPrivateOwnIssues(userID int64) int { + c, err := db.GetEngine(db.DefaultContext).Table("project_issue"). + Join("INNER", "issue", "project_issue.issue_id=issue.id"). + Where("project_issue.project_id=? AND issue.is_closed=? AND issue.is_private=? AND issue.poster_id=?", p.ID, false, true, userID). Cols("issue_id"). Count() if err != nil { diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index db2a731433c3d..ab0abe54943b4 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -110,10 +110,17 @@ func Projects(ctx *context.Context) { ctx.Data["Page"] = pager ctx.Data["CanWriteProjects"] = ctx.Repo.Permission.CanWrite(unit.TypeProjects) + ctx.Data["CanSeePrivateIssues"] = ctx.Repo.Permission.CanReadPrivateIssues() ctx.Data["IsShowClosed"] = isShowClosed ctx.Data["IsProjectsPage"] = true ctx.Data["SortType"] = sortType + var userID int64 + if ctx.IsSigned { + userID = ctx.User.ID + } + ctx.Data["UserID"] = userID + ctx.HTML(http.StatusOK, tplProjects) } diff --git a/templates/repo/projects/list.tmpl b/templates/repo/projects/list.tmpl index f152d20915499..bac2148e06080 100644 --- a/templates/repo/projects/list.tmpl +++ b/templates/repo/projects/list.tmpl @@ -47,8 +47,16 @@ {{svg "octicon-clock"}} {{$.i18n.Tr "repo.milestones.closed" $closedDate|Str2html}} {{end}} - {{svg "octicon-issue-opened"}} {{$.i18n.Tr "repo.issues.open_tab" .NumOpenIssues}} - {{svg "octicon-issue-closed"}} {{$.i18n.Tr "repo.issues.close_tab" .NumClosedIssues}} + {{if $.CanSeePrivateIssues}} + {{svg "octicon-issue-opened"}} {{$.i18n.Tr "repo.issues.open_tab" (Add .NumOpenIssues .NumOpenPrivateIssues)}} + {{svg "octicon-issue-closed"}} {{$.i18n.Tr "repo.issues.close_tab" (Add .NumClosedIssues .NumClosedPrivateIssues)}} + {{else if (gt $.UserID 0)}} + {{svg "octicon-issue-opened"}} {{$.i18n.Tr "repo.issues.open_tab" (Add .NumOpenIssues (.NumOpenPrivateOwnIssues $.UserID))}} + {{svg "octicon-issue-closed"}} {{$.i18n.Tr "repo.issues.close_tab" (Add .NumClosedIssues (.NumClosedPrivateOwnIssues $.UserID))}} + {{else}} + {{svg "octicon-issue-opened"}} {{$.i18n.Tr "repo.issues.open_tab" .NumOpenIssues}} + {{svg "octicon-issue-closed"}} {{$.i18n.Tr "repo.issues.close_tab" .NumClosedIssues}} + {{end}}
{{if and (or $.CanWriteIssues $.CanWritePulls) (not $.Repository.IsArchived)}} From 8461e94dbfcfffa1863c4eec3d8d88a99457e009 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 21:43:52 +0100 Subject: [PATCH 36/71] Make linter happy --- models/project_board.go | 1 + 1 file changed, 1 insertion(+) diff --git a/models/project_board.go b/models/project_board.go index bb5d1b02c8cc9..1755c94907cc1 100644 --- a/models/project_board.go +++ b/models/project_board.go @@ -297,6 +297,7 @@ func (b *ProjectBoard) LoadIssues(opts *LoadIssuesOpts) (IssueList, error) { return issueList, nil } +// LoadIssuesOpts list the options that can be given to load the issues. type LoadIssuesOpts struct { UserID int64 CanSeePrivateIssues bool From b1915c08412e698980558e2ebcce59d1758e7242 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 22:31:42 +0100 Subject: [PATCH 37/71] Load issue on comments on API --- routers/api/v1/repo/issue_comment.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go index be7b7ca93e674..f5b0dd640e1d6 100644 --- a/routers/api/v1/repo/issue_comment.go +++ b/routers/api/v1/repo/issue_comment.go @@ -322,7 +322,7 @@ func GetIssueComment(ctx *context.APIContext) { } if err = comment.LoadIssue(); err != nil { - ctx.InternalServerError(err) + ctx.Error(http.StatusInternalServerError, "LoadIssue", err) return } @@ -454,6 +454,11 @@ func editIssueComment(ctx *context.APIContext, form api.EditIssueCommentOption) return } + if err = comment.LoadIssue(); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadIssue", err) + return + } + if comment.Issue.IsPrivate { var userID int64 if ctx.IsSigned { @@ -567,6 +572,11 @@ func deleteIssueComment(ctx *context.APIContext) { return } + if err = comment.LoadIssue(); err != nil { + ctx.Error(http.StatusInternalServerError, "LoadIssue", err) + return + } + if comment.Issue.IsPrivate && (!ctx.IsSigned || !comment.Issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { ctx.Status(http.StatusNotFound) return From ec998e8a8a357e22b08c531004f8c842af7f5c47 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 21 Nov 2021 22:32:38 +0100 Subject: [PATCH 38/71] Fix GetCodeCommentsCount --- models/review.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/models/review.go b/models/review.go index 00906c7a2730e..b727f0facc56b 100644 --- a/models/review.go +++ b/models/review.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/timeutil" "xorm.io/builder" @@ -970,8 +971,12 @@ func (r *Review) GetCodeCommentsCount() int { conds = conds.And(builder.Eq{"invalidated": false}) } - count, err := db.GetEngine(db.DefaultContext).Where(conds).Count(new(Comment)) + count, err := db.GetEngine(db.DefaultContext). + Where(conds). + Join("INNER", "issue", "issue.id = comment.issue_id"). + Count(new(Comment)) if err != nil { + log.Error("GetCodeCommentsCount: %v", err) return 0 } return int(count) From 51ea1a2b62ad0f6b6c0c2ae04ddf9ca644b4cd52 Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 22 Nov 2021 03:06:39 +0100 Subject: [PATCH 39/71] Fix tests --- models/issue_comment.go | 23 ++++++++++++----------- models/review.go | 19 ++++++++++++------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index cfb703543abb7..8cc578d1916de 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -1003,16 +1003,17 @@ func getCommentByID(e db.Engine, id int64) (*Comment, error) { // FindCommentsOptions describes the conditions to Find comments type FindCommentsOptions struct { db.ListOptions - RepoID int64 - IssueID int64 - ReviewID int64 - UserID int64 - Since int64 - Before int64 - Line int64 - TreePath string - Type CommentType - CanSeePrivate bool + RepoID int64 + IssueID int64 + ReviewID int64 + UserID int64 + Since int64 + Before int64 + Line int64 + TreePath string + Type CommentType + CanSeePrivate bool + DisablePrivateIssues bool } func (opts *FindCommentsOptions) toConds() builder.Cond { @@ -1041,7 +1042,7 @@ func (opts *FindCommentsOptions) toConds() builder.Cond { if len(opts.TreePath) > 0 { cond = cond.And(builder.Eq{"comment.tree_path": opts.TreePath}) } - if !opts.CanSeePrivate { + if !opts.CanSeePrivate && !opts.DisablePrivateIssues { if opts.UserID != 0 { // Allow to see comments on private issues cond = cond.And( diff --git a/models/review.go b/models/review.go index b727f0facc56b..bfab81a74041c 100644 --- a/models/review.go +++ b/models/review.go @@ -933,9 +933,10 @@ func DeleteReview(r *Review) error { } opts := FindCommentsOptions{ - Type: CommentTypeCode, - IssueID: r.IssueID, - ReviewID: r.ID, + Type: CommentTypeCode, + IssueID: r.IssueID, + ReviewID: r.ID, + DisablePrivateIssues: true, } if _, err := sess.Where(opts.toConds()).Delete(new(Comment)); err != nil { @@ -943,9 +944,10 @@ func DeleteReview(r *Review) error { } opts = FindCommentsOptions{ - Type: CommentTypeReview, - IssueID: r.IssueID, - ReviewID: r.ID, + Type: CommentTypeReview, + IssueID: r.IssueID, + ReviewID: r.ID, + DisablePrivateIssues: true, } if _, err := sess.Where(opts.toConds()).Delete(new(Comment)); err != nil { @@ -990,7 +992,10 @@ func (r *Review) HTMLURL() string { ReviewID: r.ID, } comment := new(Comment) - has, err := db.GetEngine(db.DefaultContext).Where(opts.toConds()).Get(comment) + has, err := db.GetEngine(db.DefaultContext). + Where(opts.toConds()). + Join("INNER", "issue", "issue.id = comment.issue_id"). + Get(comment) if err != nil || !has { return "" } From 69851a97146b373a565e785bd1a994f1c9141dee Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 22 Nov 2021 03:06:47 +0100 Subject: [PATCH 40/71] More verbose error --- routers/api/v1/repo/pull_review.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index 55b5178305edf..0afbc15f9dcd6 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/routers/api/v1/repo/pull_review.go @@ -259,7 +259,7 @@ func DeletePullReview(ctx *context.APIContext) { } if err := models.DeleteReview(review); err != nil { - ctx.Error(http.StatusInternalServerError, "DeleteReview", fmt.Errorf("can not delete ReviewID: %d", review.ID)) + ctx.Error(http.StatusInternalServerError, "DeleteReview", fmt.Errorf("can not delete ReviewID: %d, %v", review.ID, err)) return } From f8feef7140b9495fa78497b07a915bfa4cc0519f Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 22 Nov 2021 03:27:20 +0100 Subject: [PATCH 41/71] Add some more locks --- routers/web/repo/issue.go | 1 + routers/web/repo/issue_dependency.go | 40 ++++++++++++++++++++++++++-- routers/web/repo/pull.go | 15 +++++++++++ services/issue/commit.go | 5 ++++ 4 files changed, 59 insertions(+), 2 deletions(-) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 304f9c7e3a60f..0a53ba58008ad 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1127,6 +1127,7 @@ func ViewIssue(ctx *context.Context) { RepoID: ctx.Repo.Repository.ID, Index: ctx.ParamsInt64(":index"), }) + return } // Make sure type and URL matches. diff --git a/routers/web/repo/issue_dependency.go b/routers/web/repo/issue_dependency.go index 015f31d8304ff..80cc98d0f2ac3 100644 --- a/routers/web/repo/issue_dependency.go +++ b/routers/web/repo/issue_dependency.go @@ -17,7 +17,25 @@ func AddDependency(ctx *context.Context) { issueIndex := ctx.ParamsInt64("index") issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, issueIndex) if err != nil { - ctx.ServerError("GetIssueByIndex", err) + if models.IsErrIssueNotExist(err) { + ctx.NotFound("GetIssueByIndex", err) + } else { + ctx.ServerError("GetIssueByIndex", err) + } + return + } + + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { + var userID int64 + if ctx.IsSigned { + userID = ctx.User.ID + } + ctx.NotFound("CanSeePrivateIssues", models.ErrCannotSeePrivateIssue{ + UserID: userID, + ID: issue.ID, + RepoID: ctx.Repo.Repository.ID, + Index: ctx.ParamsInt64(":index"), + }) return } @@ -76,7 +94,25 @@ func RemoveDependency(ctx *context.Context) { issueIndex := ctx.ParamsInt64("index") issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, issueIndex) if err != nil { - ctx.ServerError("GetIssueByIndex", err) + if models.IsErrIssueNotExist(err) { + ctx.NotFound("GetIssueByIndex", err) + } else { + ctx.ServerError("GetIssueByIndex", err) + } + return + } + + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { + var userID int64 + if ctx.IsSigned { + userID = ctx.User.ID + } + ctx.NotFound("CanSeePrivateIssues", models.ErrCannotSeePrivateIssue{ + UserID: userID, + ID: issue.ID, + RepoID: ctx.Repo.Repository.ID, + Index: ctx.ParamsInt64(":index"), + }) return } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index a7afc3a05c11d..e05f3503013bc 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -261,6 +261,21 @@ func checkPullInfo(ctx *context.Context) *models.Issue { } return nil } + + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { + var userID int64 + if ctx.IsSigned { + userID = ctx.User.ID + } + ctx.NotFound("CanSeePrivateIssues", models.ErrCannotSeePrivateIssue{ + UserID: userID, + ID: issue.ID, + RepoID: ctx.Repo.Repository.ID, + Index: ctx.ParamsInt64(":index"), + }) + return nil + } + if err = issue.LoadPoster(); err != nil { ctx.ServerError("LoadPoster", err) return nil diff --git a/services/issue/commit.go b/services/issue/commit.go index 401084639d630..6daba9315e660 100644 --- a/services/issue/commit.go +++ b/services/issue/commit.go @@ -124,6 +124,7 @@ func UpdateIssuesCommit(doer *models.User, repo *models.Repository, commits []*r if refIssue, err = getIssueFromRef(refRepo, ref.Index); err != nil { return err } + if refIssue == nil { continue } @@ -133,6 +134,10 @@ func UpdateIssuesCommit(doer *models.User, repo *models.Repository, commits []*r return err } + if refIssue.IsPrivate && !(perm.CanReadPrivateIssues() || refIssue.IsPoster(doer.ID)) { + continue + } + key := markKey{ID: refIssue.ID, Action: ref.Action} if refMarked[key] { continue From 312a5b4e176c020bdaec9e0be53a50b4989b4a62 Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 22 Nov 2021 03:57:37 +0100 Subject: [PATCH 42/71] More locks! --- modules/context/repo.go | 4 ++ routers/web/repo/attachment.go | 9 ++++ routers/web/repo/issue_dependency.go | 70 +++++++++++++++++++++++++--- routers/web/repo/issue_stopwatch.go | 10 ++++ routers/web/repo/projects.go | 5 ++ 5 files changed, 91 insertions(+), 7 deletions(-) diff --git a/modules/context/repo.go b/modules/context/repo.go index e3d66fc3b98ac..37df700accd98 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -149,6 +149,10 @@ func (r *Repository) CanUseTimetracker(issue *models.Issue, user *models.User) b // Checking for following: // 1. Is timetracker enabled // 2. Is the user a contributor, admin, poster or assignee and do the repository policies require this? + // 3. Is on a private issue, check if they can access that private issue. + if issue.IsPrivate && !(r.CanReadPrivateIssues() || issue.IsPoster(user.ID)) { + return false + } isAssigned, _ := models.IsUserAssignedToIssue(issue, user) return r.Repository.IsTimetrackerEnabled() && (!r.Repository.AllowOnlyContributorsToTrackTime() || r.Permission.CanWriteIssuesOrPulls(issue.IsPull) || issue.IsPoster(user.ID) || isAssigned) diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index 303eee24d17db..328994674046e 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -115,6 +115,15 @@ func GetAttachment(ctx *context.Context) { ctx.Error(http.StatusNotFound) return } + issue, err := models.GetIssueByID(attach.IssueID) + if err != nil { + ctx.Error(http.StatusInternalServerError, "GetIssueByID", err.Error()) + return + } + if issue.IsPrivate && !(perm.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { + ctx.Error(http.StatusNotFound) + return + } } if err := attach.IncreaseDownloadCount(); err != nil { diff --git a/routers/web/repo/issue_dependency.go b/routers/web/repo/issue_dependency.go index 80cc98d0f2ac3..029ee3ed68220 100644 --- a/routers/web/repo/issue_dependency.go +++ b/routers/web/repo/issue_dependency.go @@ -25,16 +25,12 @@ func AddDependency(ctx *context.Context) { return } - if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { - var userID int64 - if ctx.IsSigned { - userID = ctx.User.ID - } + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || issue.IsPoster(ctx.User.ID)) { ctx.NotFound("CanSeePrivateIssues", models.ErrCannotSeePrivateIssue{ - UserID: userID, + UserID: ctx.User.ID, ID: issue.ID, RepoID: ctx.Repo.Repository.ID, - Index: ctx.ParamsInt64(":index"), + Index: ctx.ParamsInt64("index"), }) return } @@ -62,6 +58,36 @@ func AddDependency(ctx *context.Context) { return } + if issue.RepoID == dep.RepoID { + if dep.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || dep.IsPoster(ctx.User.ID)) { + ctx.NotFound("CanSeePrivateIssues", models.ErrCannotSeePrivateIssue{ + UserID: ctx.User.ID, + ID: dep.ID, + RepoID: dep.Repo.ID, + Index: ctx.ParamsInt64("index"), + }) + return + } + } else { + if err := dep.LoadRepo(); err != nil { + ctx.ServerError("LoadRepo", err) + } + + perm, err := models.GetUserRepoPermission(dep.Repo, ctx.User) + if err != nil { + ctx.ServerError("GetUserRepoPermission", err) + } + + if dep.IsPrivate && !(perm.CanReadPrivateIssues() || dep.IsPoster(ctx.User.ID)) { + ctx.NotFound("CanSeePrivateIssues", models.ErrCannotSeePrivateIssue{ + UserID: ctx.User.ID, + ID: dep.ID, + RepoID: dep.Repo.ID, + }) + return + } + } + // Check if both issues are in the same repo if cross repository dependencies is not enabled if issue.RepoID != dep.RepoID && !setting.Service.AllowCrossRepositoryDependencies { ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_not_same_repo")) @@ -151,6 +177,36 @@ func RemoveDependency(ctx *context.Context) { return } + if issue.RepoID == dep.RepoID { + if dep.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || dep.IsPoster(ctx.User.ID)) { + ctx.NotFound("CanSeePrivateIssues", models.ErrCannotSeePrivateIssue{ + UserID: ctx.User.ID, + ID: dep.ID, + RepoID: dep.Repo.ID, + Index: ctx.ParamsInt64("index"), + }) + return + } + } else { + if err := dep.LoadRepo(); err != nil { + ctx.ServerError("LoadRepo", err) + } + + perm, err := models.GetUserRepoPermission(dep.Repo, ctx.User) + if err != nil { + ctx.ServerError("GetUserRepoPermission", err) + } + + if dep.IsPrivate && !(perm.CanReadPrivateIssues() || dep.IsPoster(ctx.User.ID)) { + ctx.NotFound("CanSeePrivateIssues", models.ErrCannotSeePrivateIssue{ + UserID: ctx.User.ID, + ID: dep.ID, + RepoID: dep.Repo.ID, + }) + return + } + } + if err = models.RemoveIssueDependency(ctx.User, issue, dep, depType); err != nil { if models.IsErrDependencyNotExists(err) { ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_not_exist")) diff --git a/routers/web/repo/issue_stopwatch.go b/routers/web/repo/issue_stopwatch.go index 0e9405fde4dbd..6b4c560125902 100644 --- a/routers/web/repo/issue_stopwatch.go +++ b/routers/web/repo/issue_stopwatch.go @@ -93,6 +93,16 @@ func GetActiveStopwatch(c *context.Context) { return } + perm, err := models.GetUserRepoPermission(issue.Repo, c.User) + if err != nil { + c.ServerError("GetUserRepoPermission", err) + return + } + + if issue.IsPrivate && !(perm.CanReadPrivateIssues() || issue.IsPoster(c.User.ID)) { + return + } + c.Data["ActiveStopwatch"] = StopwatchTmplInfo{ issue.Link(), issue.Repo.FullName(), diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index ab0abe54943b4..569f0d2f8216e 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -628,6 +628,11 @@ func MoveIssueAcrossBoards(ctx *context.Context) { return } + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || issue.IsPoster(ctx.User.ID)) { + ctx.NotFound("", nil) + return + } + if err := models.MoveIssueAcrossProjectBoards(issue, board); err != nil { ctx.ServerError("MoveIssueAcrossProjectBoards", err) return From 1ffb2f3de3d1b7e779b059b03768d0fa1a0764b1 Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 22 Nov 2021 03:59:31 +0100 Subject: [PATCH 43/71] Simplify --- routers/api/v1/repo/issue.go | 6 +++--- routers/api/v1/repo/issue_comment.go | 4 ++-- routers/api/v1/repo/issue_label.go | 8 ++++---- routers/api/v1/repo/issue_reaction.go | 4 ++-- routers/api/v1/repo/issue_stopwatch.go | 2 +- routers/api/v1/repo/issue_subscription.go | 6 +++--- routers/api/v1/repo/issue_tracked_time.go | 8 ++++---- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 873e4d64e49a1..454b6a273353a 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -555,7 +555,7 @@ func GetIssue(ctx *context.APIContext) { return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } @@ -731,7 +731,7 @@ func EditIssue(ctx *context.APIContext) { return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } @@ -903,7 +903,7 @@ func UpdateIssueDeadline(ctx *context.APIContext) { return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go index f5b0dd640e1d6..0fb58ba8c5d70 100644 --- a/routers/api/v1/repo/issue_comment.go +++ b/routers/api/v1/repo/issue_comment.go @@ -71,7 +71,7 @@ func ListIssueComments(ctx *context.APIContext) { return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } @@ -256,7 +256,7 @@ func CreateIssueComment(ctx *context.APIContext) { return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } diff --git a/routers/api/v1/repo/issue_label.go b/routers/api/v1/repo/issue_label.go index df406e19fba91..2e7015e39aa43 100644 --- a/routers/api/v1/repo/issue_label.go +++ b/routers/api/v1/repo/issue_label.go @@ -61,7 +61,7 @@ func ListIssueLabels(ctx *context.APIContext) { return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } @@ -173,7 +173,7 @@ func DeleteIssueLabel(ctx *context.APIContext) { return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } @@ -296,7 +296,7 @@ func ClearIssueLabels(ctx *context.APIContext) { return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } @@ -325,7 +325,7 @@ func prepareForReplaceOrAdd(ctx *context.APIContext, form api.IssueLabelsOption) return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } diff --git a/routers/api/v1/repo/issue_reaction.go b/routers/api/v1/repo/issue_reaction.go index d1a295029d6b1..19b5ed2e6c908 100644 --- a/routers/api/v1/repo/issue_reaction.go +++ b/routers/api/v1/repo/issue_reaction.go @@ -289,7 +289,7 @@ func GetIssueReactions(ctx *context.APIContext) { return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } @@ -413,7 +413,7 @@ func changeIssueReaction(ctx *context.APIContext, form api.EditReactionOption, i return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } diff --git a/routers/api/v1/repo/issue_stopwatch.go b/routers/api/v1/repo/issue_stopwatch.go index b4d7f6f622604..2b782bdf2e329 100644 --- a/routers/api/v1/repo/issue_stopwatch.go +++ b/routers/api/v1/repo/issue_stopwatch.go @@ -174,7 +174,7 @@ func prepareIssueStopwatch(ctx *context.APIContext, shouldExist bool) (*models.I return nil, err } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.Status(http.StatusNotFound) return nil, errors.New("Not enough permission to see issue") } diff --git a/routers/api/v1/repo/issue_subscription.go b/routers/api/v1/repo/issue_subscription.go index c49b312afa530..d1555ce095f79 100644 --- a/routers/api/v1/repo/issue_subscription.go +++ b/routers/api/v1/repo/issue_subscription.go @@ -114,7 +114,7 @@ func setIssueSubscription(ctx *context.APIContext, watch bool) { return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } @@ -200,7 +200,7 @@ func CheckIssueSubscription(ctx *context.APIContext) { return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } @@ -271,7 +271,7 @@ func GetIssueSubscribers(ctx *context.APIContext) { return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } diff --git a/routers/api/v1/repo/issue_tracked_time.go b/routers/api/v1/repo/issue_tracked_time.go index 6ff8adb304376..fc255c75b6a07 100644 --- a/routers/api/v1/repo/issue_tracked_time.go +++ b/routers/api/v1/repo/issue_tracked_time.go @@ -84,7 +84,7 @@ func ListTrackedTimes(ctx *context.APIContext) { return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } @@ -193,7 +193,7 @@ func AddTime(ctx *context.APIContext) { return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } @@ -279,7 +279,7 @@ func ResetIssueTime(ctx *context.APIContext) { return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } @@ -355,7 +355,7 @@ func DeleteTime(ctx *context.APIContext) { return } - if issue.IsPrivate && (!ctx.IsSigned || !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanReadPrivateIssues()) { + if issue.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { ctx.NotFound() return } From 69abea3976b08871f54fe38fb027d97467b46309 Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 22 Nov 2021 04:02:08 +0100 Subject: [PATCH 44/71] Add a lock --- routers/web/repo/issue.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 0a53ba58008ad..a3a327dcdad30 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1722,11 +1722,16 @@ func getActionIssues(ctx *context.Context) []*models.Issue { // Check access rights for all issues issueUnitEnabled := ctx.Repo.CanRead(unit.TypeIssues) prUnitEnabled := ctx.Repo.CanRead(unit.TypePullRequests) + canReadPrivateIssues := ctx.Repo.CanReadPrivateIssues() for _, issue := range issues { if issue.IsPull && !prUnitEnabled || !issue.IsPull && !issueUnitEnabled { ctx.NotFound("IssueOrPullRequestUnitNotAllowed", nil) return nil } + if issue.IsPrivate && !(canReadPrivateIssues || (ctx.IsSigned && issue.IsPoster(ctx.User.ID))) { + ctx.NotFound("IsPrivate", nil) + return nil + } if err = issue.LoadAttributes(); err != nil { ctx.ServerError("LoadAttributes", err) return nil From 254c50026f041505ba3c06ca2d5419315af5284e Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 22 Nov 2021 04:08:31 +0100 Subject: [PATCH 45/71] Make linter happy --- routers/web/repo/issue_dependency.go | 88 +++++++++++----------------- 1 file changed, 35 insertions(+), 53 deletions(-) diff --git a/routers/web/repo/issue_dependency.go b/routers/web/repo/issue_dependency.go index 029ee3ed68220..3c8451e4a278b 100644 --- a/routers/web/repo/issue_dependency.go +++ b/routers/web/repo/issue_dependency.go @@ -58,34 +58,14 @@ func AddDependency(ctx *context.Context) { return } - if issue.RepoID == dep.RepoID { - if dep.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || dep.IsPoster(ctx.User.ID)) { - ctx.NotFound("CanSeePrivateIssues", models.ErrCannotSeePrivateIssue{ - UserID: ctx.User.ID, - ID: dep.ID, - RepoID: dep.Repo.ID, - Index: ctx.ParamsInt64("index"), - }) - return - } - } else { - if err := dep.LoadRepo(); err != nil { - ctx.ServerError("LoadRepo", err) - } - - perm, err := models.GetUserRepoPermission(dep.Repo, ctx.User) - if err != nil { - ctx.ServerError("GetUserRepoPermission", err) - } - - if dep.IsPrivate && !(perm.CanReadPrivateIssues() || dep.IsPoster(ctx.User.ID)) { - ctx.NotFound("CanSeePrivateIssues", models.ErrCannotSeePrivateIssue{ - UserID: ctx.User.ID, - ID: dep.ID, - RepoID: dep.Repo.ID, - }) - return - } + if !canDepBeLoaded(issue, dep, ctx) { + ctx.NotFound("CanSeePrivateIssues", models.ErrCannotSeePrivateIssue{ + UserID: ctx.User.ID, + ID: dep.ID, + RepoID: dep.Repo.ID, + Index: ctx.ParamsInt64("index"), + }) + return } // Check if both issues are in the same repo if cross repository dependencies is not enabled @@ -177,15 +157,33 @@ func RemoveDependency(ctx *context.Context) { return } + if !canDepBeLoaded(issue, dep, ctx) { + ctx.NotFound("CanSeePrivateIssues", models.ErrCannotSeePrivateIssue{ + UserID: ctx.User.ID, + ID: dep.ID, + RepoID: dep.Repo.ID, + Index: ctx.ParamsInt64("index"), + }) + return + } + + if err = models.RemoveIssueDependency(ctx.User, issue, dep, depType); err != nil { + if models.IsErrDependencyNotExists(err) { + ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_not_exist")) + return + } + ctx.ServerError("RemoveIssueDependency", err) + return + } + + // Redirect + ctx.Redirect(issue.HTMLURL(), http.StatusSeeOther) +} + +func canDepBeLoaded(issue *models.Issue, dep *models.Issue, ctx *context.Context) bool { if issue.RepoID == dep.RepoID { if dep.IsPrivate && !(ctx.Repo.CanReadPrivateIssues() || dep.IsPoster(ctx.User.ID)) { - ctx.NotFound("CanSeePrivateIssues", models.ErrCannotSeePrivateIssue{ - UserID: ctx.User.ID, - ID: dep.ID, - RepoID: dep.Repo.ID, - Index: ctx.ParamsInt64("index"), - }) - return + return false } } else { if err := dep.LoadRepo(); err != nil { @@ -198,24 +196,8 @@ func RemoveDependency(ctx *context.Context) { } if dep.IsPrivate && !(perm.CanReadPrivateIssues() || dep.IsPoster(ctx.User.ID)) { - ctx.NotFound("CanSeePrivateIssues", models.ErrCannotSeePrivateIssue{ - UserID: ctx.User.ID, - ID: dep.ID, - RepoID: dep.Repo.ID, - }) - return - } - } - - if err = models.RemoveIssueDependency(ctx.User, issue, dep, depType); err != nil { - if models.IsErrDependencyNotExists(err) { - ctx.Flash.Error(ctx.Tr("repo.issues.dependency.add_error_dep_not_exist")) - return + return false } - ctx.ServerError("RemoveIssueDependency", err) - return } - - // Redirect - ctx.Redirect(issue.HTMLURL(), http.StatusSeeOther) + return true } From 211804b2d6da8226a75ee381d42851f236f19de6 Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 22 Nov 2021 13:03:37 +0100 Subject: [PATCH 46/71] PR's are not supported --- templates/repo/issue/new_form.tmpl | 20 ++++++++++--------- .../repo/issue/view_content/sidebar.tmpl | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index 9688207fd3760..48880ba08a83b 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -235,15 +235,17 @@ {{end}}
-
-
- - -
+ {{if not .PageIsComparePull}} +
+
+ + +
+ {{end}}
diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index ce1eff3347aa4..d6b1fb2e5b022 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -302,7 +302,7 @@ - {{if or (.IsRepoAdmin) (.IsIssuePoster)}} + {{if and (or (.IsRepoAdmin) (.IsIssuePoster)) (not .Issue.IsPull)}}
{{.i18n.Tr "repo.issues.confidential"}} From 58de70413a7fc2aa97d80557cb3fdd7281601cdf Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 22 Nov 2021 13:19:33 +0100 Subject: [PATCH 47/71] Add tooltip data to sidebars --- options/locale/locale_en-US.ini | 2 ++ templates/repo/issue/new_form.tmpl | 2 +- templates/repo/issue/view_content/sidebar.tmpl | 4 +++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index af3f23c9fd9af..858aa31b83727 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1399,7 +1399,9 @@ issues.content_history.delete_from_history_confirm = Delete from history? issues.content_history.options = Options issues.confidential = Confidential issues.confidential.off = Make issue public +issues.confidential.off_popup = By clicking this button, you will make this issue and comments public to everyone who can read this repo. issues.confidential.on = Make issue confidential +issues.confidential.on_popup = By turning on the confidential mode, only the poster, repo admins and team members who have "Access private issues" permission will be able to access this issue. compare.compare_base = base compare.compare_head = compare diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index 48880ba08a83b..eb1700e10e5ba 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -237,7 +237,7 @@
{{if not .PageIsComparePull}}
-
+
From 82f14ecd9af0d92bc954b863f53bb87c55d0c5d7 Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 22 Nov 2021 13:32:43 +0100 Subject: [PATCH 48/71] Don't leak private issues via activity tab --- models/repo_activity.go | 66 +++++++++++++++++++++++++----------- routers/web/repo/activity.go | 19 ++++++++--- 2 files changed, 61 insertions(+), 24 deletions(-) diff --git a/models/repo_activity.go b/models/repo_activity.go index 5986da7e77ae9..6d6f0fd881efb 100644 --- a/models/repo_activity.go +++ b/models/repo_activity.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/git" + "xorm.io/builder" "xorm.io/xorm" ) @@ -40,35 +41,45 @@ type ActivityStats struct { Code *git.CodeActivityStats } -// GetActivityStats return stats for repository at given time range -func GetActivityStats(repo *Repository, timeFrom time.Time, releases, issues, prs, code bool) (*ActivityStats, error) { +type GetActivityStatsOpts struct { + TimeFrom time.Time + UserID int64 + ShowReleases bool + ShowIssues bool + ShowPRs bool + ShowCode bool + CanReadPrivateIssues bool +} + +// GetActivityStats return stats for repository at given time range, as user +func GetActivityStats(repo *Repository, opts *GetActivityStatsOpts) (*ActivityStats, error) { stats := &ActivityStats{Code: &git.CodeActivityStats{}} - if releases { - if err := stats.FillReleases(repo.ID, timeFrom); err != nil { + if opts.ShowReleases { + if err := stats.FillReleases(repo.ID, opts.TimeFrom); err != nil { return nil, fmt.Errorf("FillReleases: %v", err) } } - if prs { - if err := stats.FillPullRequests(repo.ID, timeFrom); err != nil { + if opts.ShowPRs { + if err := stats.FillPullRequests(repo.ID, opts.TimeFrom); err != nil { return nil, fmt.Errorf("FillPullRequests: %v", err) } } - if issues { - if err := stats.FillIssues(repo.ID, timeFrom); err != nil { + if opts.ShowIssues { + if err := stats.FillIssues(repo.ID, opts.TimeFrom, opts.CanReadPrivateIssues, opts.UserID); err != nil { return nil, fmt.Errorf("FillIssues: %v", err) } } - if err := stats.FillUnresolvedIssues(repo.ID, timeFrom, issues, prs); err != nil { + if err := stats.FillUnresolvedIssues(repo.ID, opts.TimeFrom, opts.ShowIssues, opts.ShowPRs, opts.CanReadPrivateIssues, opts.UserID); err != nil { return nil, fmt.Errorf("FillUnresolvedIssues: %v", err) } - if code { + if opts.ShowCode { gitRepo, err := git.OpenRepository(repo.RepoPath()) if err != nil { return nil, fmt.Errorf("OpenRepository: %v", err) } defer gitRepo.Close() - code, err := gitRepo.GetCodeActivityStats(timeFrom, repo.DefaultBranch) + code, err := gitRepo.GetCodeActivityStats(opts.TimeFrom, repo.DefaultBranch) if err != nil { return nil, fmt.Errorf("FillFromGit: %v", err) } @@ -261,12 +272,12 @@ func pullRequestsForActivityStatement(repoID int64, fromTime time.Time, merged b } // FillIssues returns issue information for activity page -func (stats *ActivityStats) FillIssues(repoID int64, fromTime time.Time) error { +func (stats *ActivityStats) FillIssues(repoID int64, fromTime time.Time, canReadPrivateIssues bool, userID int64) error { var err error var count int64 // Closed issues - sess := issuesForActivityStatement(repoID, fromTime, true, false) + sess := issuesForActivityStatement(repoID, fromTime, true, false, canReadPrivateIssues, userID) sess.OrderBy("issue.closed_unix DESC") stats.ClosedIssues = make(IssueList, 0) if err = sess.Find(&stats.ClosedIssues); err != nil { @@ -274,14 +285,14 @@ func (stats *ActivityStats) FillIssues(repoID int64, fromTime time.Time) error { } // Closed issue authors - sess = issuesForActivityStatement(repoID, fromTime, true, false) + sess = issuesForActivityStatement(repoID, fromTime, true, false, canReadPrivateIssues, userID) if _, err = sess.Select("count(distinct issue.poster_id) as `count`").Table("issue").Get(&count); err != nil { return err } stats.ClosedIssueAuthorCount = count // New issues - sess = issuesForActivityStatement(repoID, fromTime, false, false) + sess = issuesForActivityStatement(repoID, fromTime, false, false, canReadPrivateIssues, userID) sess.OrderBy("issue.created_unix ASC") stats.OpenedIssues = make(IssueList, 0) if err = sess.Find(&stats.OpenedIssues); err != nil { @@ -289,7 +300,7 @@ func (stats *ActivityStats) FillIssues(repoID int64, fromTime time.Time) error { } // Opened issue authors - sess = issuesForActivityStatement(repoID, fromTime, false, false) + sess = issuesForActivityStatement(repoID, fromTime, false, false, canReadPrivateIssues, userID) if _, err = sess.Select("count(distinct issue.poster_id) as `count`").Table("issue").Get(&count); err != nil { return err } @@ -299,12 +310,12 @@ func (stats *ActivityStats) FillIssues(repoID int64, fromTime time.Time) error { } // FillUnresolvedIssues returns unresolved issue and pull request information for activity page -func (stats *ActivityStats) FillUnresolvedIssues(repoID int64, fromTime time.Time, issues, prs bool) error { +func (stats *ActivityStats) FillUnresolvedIssues(repoID int64, fromTime time.Time, issues, prs bool, canReadPrivateIssues bool, userID int64) error { // Check if we need to select anything if !issues && !prs { return nil } - sess := issuesForActivityStatement(repoID, fromTime, false, true) + sess := issuesForActivityStatement(repoID, fromTime, false, true, canReadPrivateIssues, userID) if !issues || !prs { sess.And("issue.is_pull = ?", prs) } @@ -313,7 +324,7 @@ func (stats *ActivityStats) FillUnresolvedIssues(repoID int64, fromTime time.Tim return sess.Find(&stats.UnresolvedIssues) } -func issuesForActivityStatement(repoID int64, fromTime time.Time, closed, unresolved bool) *xorm.Session { +func issuesForActivityStatement(repoID int64, fromTime time.Time, closed, unresolved bool, canReadPrivateIssues bool, userID int64) *xorm.Session { sess := db.GetEngine(db.DefaultContext).Where("issue.repo_id = ?", repoID). And("issue.is_closed = ?", closed) @@ -329,6 +340,23 @@ func issuesForActivityStatement(repoID int64, fromTime time.Time, closed, unreso sess.And("issue.updated_unix >= ?", fromTime.Unix()) } + if !canReadPrivateIssues { + if userID == 0 { + sess.And("issue.is_private = ?", false) + } else { + // Allow to see private issues if the user is the poster of it. + sess.And( + builder.Or( + builder.Eq{"`issue`.is_private": false}, + builder.And( + builder.Eq{"`issue`.is_private": true}, + builder.In("`issue`.poster_id", userID), + ), + ), + ) + } + } + return sess } diff --git a/routers/web/repo/activity.go b/routers/web/repo/activity.go index ae37b51403fe0..043f06c36c1f0 100644 --- a/routers/web/repo/activity.go +++ b/routers/web/repo/activity.go @@ -51,12 +51,21 @@ func Activity(ctx *context.Context) { ctx.Data["DateUntil"] = timeUntil.Format("January 2, 2006") ctx.Data["PeriodText"] = ctx.Tr("repo.activity.period." + ctx.Data["Period"].(string)) + var userID int64 + if ctx.IsSigned { + userID = ctx.User.ID + } + var err error - if ctx.Data["Activity"], err = models.GetActivityStats(ctx.Repo.Repository, timeFrom, - ctx.Repo.CanRead(unit.TypeReleases), - ctx.Repo.CanRead(unit.TypeIssues), - ctx.Repo.CanRead(unit.TypePullRequests), - ctx.Repo.CanRead(unit.TypeCode)); err != nil { + if ctx.Data["Activity"], err = models.GetActivityStats(ctx.Repo.Repository, &models.GetActivityStatsOpts{ + TimeFrom: timeFrom, + UserID: userID, + ShowReleases: ctx.Repo.CanRead(unit.TypeReleases), + ShowIssues: ctx.Repo.CanRead(unit.TypeIssues), + ShowPRs: ctx.Repo.CanRead(unit.TypePullRequests), + ShowCode: ctx.Repo.CanRead(unit.TypeCode), + CanReadPrivateIssues: ctx.Repo.CanReadPrivateIssues(), + }); err != nil { ctx.ServerError("GetActivityStats", err) return } From 27fe481232a5f81a2d2b27c46baff2e55e9885d1 Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 22 Nov 2021 13:41:55 +0100 Subject: [PATCH 49/71] Make linter happy --- models/repo_activity.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/models/repo_activity.go b/models/repo_activity.go index 6d6f0fd881efb..fddd13b919e92 100644 --- a/models/repo_activity.go +++ b/models/repo_activity.go @@ -41,6 +41,8 @@ type ActivityStats struct { Code *git.CodeActivityStats } +// GetActivityStatsOpts represents the possible options to GetActivityStats + type GetActivityStatsOpts struct { TimeFrom time.Time UserID int64 From e9db7efb6578157c083573eb537f5538fcdae8e9 Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 22 Nov 2021 16:09:10 +0100 Subject: [PATCH 50/71] Remove redunant space --- models/repo_activity.go | 1 - 1 file changed, 1 deletion(-) diff --git a/models/repo_activity.go b/models/repo_activity.go index fddd13b919e92..a2d0bbca71a7e 100644 --- a/models/repo_activity.go +++ b/models/repo_activity.go @@ -42,7 +42,6 @@ type ActivityStats struct { } // GetActivityStatsOpts represents the possible options to GetActivityStats - type GetActivityStatsOpts struct { TimeFrom time.Time UserID int64 From 74bc87a8c6294e9cac165a3258e2ecf7cf4d662d Mon Sep 17 00:00:00 2001 From: Gusted Date: Thu, 25 Nov 2021 00:26:10 +0100 Subject: [PATCH 51/71] Fix build errors --- models/issue.go | 6 +++--- services/issue/issue.go | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/models/issue.go b/models/issue.go index 93b03713a4df7..26387e2c0a3f7 100644 --- a/models/issue.go +++ b/models/issue.go @@ -337,11 +337,11 @@ func (issue *Issue) LoadAttributes() error { } // LoadCommentsAsUser loads the comment of the issue, as the user. -func (issue *Issue) LoadCommentsAsUser(user *User, canSeePrivateIssues bool) error { +func (issue *Issue) LoadCommentsAsUser(user *user_model.User, canSeePrivateIssues bool) error { return issue.loadCommentsAsUser(db.GetEngine(db.DefaultContext), user, canSeePrivateIssues) } -func (issue *Issue) loadCommentsAsUser(e db.Engine, user *User, canSeePrivateIssues bool) (err error) { +func (issue *Issue) loadCommentsAsUser(e db.Engine, user *user_model.User, canSeePrivateIssues bool) (err error) { var userID int64 if user != nil { userID = user.ID @@ -767,7 +767,7 @@ func (issue *Issue) ChangeTitle(doer *user_model.User, oldTitle string) (err err } // ChangeConfidential changes the confidential of this issue, as the given user. -func (issue *Issue) ChangeConfidential(doer *User) (err error) { +func (issue *Issue) ChangeConfidential(doer *user_model.User) (err error) { ctx, committer, err := db.TxContext() if err != nil { return err diff --git a/services/issue/issue.go b/services/issue/issue.go index 216cd0c58de2f..4b966b332da79 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -56,7 +56,7 @@ func ChangeTitle(issue *models.Issue, doer *user_model.User, title string) (err } // ChangeConfidential changes the confidential of this issue, as the given user. -func ChangeConfidential(issue *models.Issue, doer *models.User, isConfidential bool) (err error) { +func ChangeConfidential(issue *models.Issue, doer *user_model.User, isConfidential bool) (err error) { if doer == nil { return models.ErrCannotSeePrivateIssue{ UserID: -1, @@ -65,6 +65,7 @@ func ChangeConfidential(issue *models.Issue, doer *models.User, isConfidential b Index: issue.Index, } } + // TODO: remove existing watcher, notifications, etc etc. isUserAdmin, err := models.IsUserRealRepoAdmin(issue.Repo, doer) if err != nil { return From 51a7ea1c7d34c4f7841bb397b5bf73f1280cd10f Mon Sep 17 00:00:00 2001 From: Gusted Date: Thu, 25 Nov 2021 00:40:42 +0100 Subject: [PATCH 52/71] Replac with better icons --- templates/repo/issue/new_form.tmpl | 3 +-- templates/repo/issue/view_content/sidebar.tmpl | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/templates/repo/issue/new_form.tmpl b/templates/repo/issue/new_form.tmpl index eb1700e10e5ba..7602debf5e627 100644 --- a/templates/repo/issue/new_form.tmpl +++ b/templates/repo/issue/new_form.tmpl @@ -240,8 +240,7 @@
diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index e9845ecc061e3..9a45fb60c740f 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -313,8 +313,7 @@