Skip to content

Adding private issues functionality #17711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 85 commits into from
Closed
Changes from all commits
Commits
Show all changes
85 commits
Select commit Hold shift + click to select a range
05ec3f1
WIP
Nov 18, 2021
5652ea1
Add UI/backend-end handling
Nov 18, 2021
46f1cc7
Add toggle
Nov 18, 2021
0e1e744
Add NumPrivate{Closed,Open,}Issues (BEGIN)
Nov 18, 2021
8e63544
Respect issue's private on API
Nov 18, 2021
14bfc69
Fix issue Right checking
Nov 18, 2021
a810833
Fix issue list on poster
Nov 18, 2021
faadf71
Code review feedback
Nov 18, 2021
f7da3f2
Fix compiling
Nov 19, 2021
ea6a63f
Restrict changing state to poster + admin
Nov 19, 2021
535cec3
Fixing up some permissions
Nov 19, 2021
045374c
Merge branch 'main' into private-comments
Nov 19, 2021
4706958
Add team permission
Nov 19, 2021
cef707a
Merge branch 'main' into private-comments
Nov 19, 2021
b728100
Fix comments
Nov 19, 2021
108ae1f
Add migration
Nov 20, 2021
75c8a34
Fix header calculation
Nov 20, 2021
fe48cfb
Fix showing confidential button.
Nov 20, 2021
de88171
Fix listing issues on other places
Nov 20, 2021
777694a
Add permission checking to api
Nov 20, 2021
aca5ad2
Fix database errors
Nov 20, 2021
cb5ebf6
Fix api not found
Nov 20, 2021
ba762be
Allow API to create confidential issues
Nov 20, 2021
c784db7
Fix copy pasta
Nov 21, 2021
23be89d
Fix notifications checking (I think?)
Nov 21, 2021
9fd110f
Fix permission checking
Nov 21, 2021
8e2b43b
Simplify
Nov 21, 2021
87792d3
Fix mail permission checking
Nov 21, 2021
9c396b8
Disallow private issues on webhooks
Nov 21, 2021
d6edd41
Merge branch 'main' into private-comments
Nov 21, 2021
d4ebb38
Migration: set owner's team to see private issues
Nov 21, 2021
8f58908
Fix unit tests
Nov 21, 2021
1714bf5
Fix migration
Nov 21, 2021
6c1f279
Typo in field
Nov 21, 2021
790975f
Fix issues loading for admins
Nov 21, 2021
44a4b63
Load correct issues on projects
Nov 21, 2021
45dcc98
Actually fixup permissions
Nov 21, 2021
b1a1972
Fix projects with private issues
Nov 21, 2021
8461e94
Make linter happy
Nov 21, 2021
04efd7f
Merge branch 'main' into private-comments
Nov 21, 2021
b1915c0
Load issue on comments on API
Nov 21, 2021
ec998e8
Fix GetCodeCommentsCount
Nov 21, 2021
51ea1a2
Fix tests
Nov 22, 2021
69851a9
More verbose error
Nov 22, 2021
f8feef7
Add some more locks
Nov 22, 2021
312a5b4
More locks!
Nov 22, 2021
1ffb2f3
Simplify
Nov 22, 2021
69abea3
Add a lock
Nov 22, 2021
254c500
Make linter happy
Nov 22, 2021
211804b
PR's are not supported
Nov 22, 2021
58de704
Add tooltip data to sidebars
Nov 22, 2021
82f14ec
Don't leak private issues via activity tab
Nov 22, 2021
1fa3372
Merge branch 'main' into private-comments
Nov 22, 2021
27fe481
Make linter happy
Nov 22, 2021
e9db7ef
Remove redunant space
Nov 22, 2021
9a67e55
Merge branch 'main' into private-comments
Nov 24, 2021
74bc87a
Fix build errors
Nov 24, 2021
51a7ea1
Replac with better icons
Nov 24, 2021
9971830
Add icon into issue list
Nov 25, 2021
bced477
Attract some attention
Nov 25, 2021
cf28438
Best-effort
Nov 25, 2021
29323ce
Fix attachment fixtures
Nov 25, 2021
60a5a3c
Merge branch 'main' into private-comments
Nov 25, 2021
596916f
Simplify migration
Nov 25, 2021
dcdf3c7
Fix attachment fixtures
Nov 25, 2021
69bdec9
Simplify condition
Nov 25, 2021
8cd0fbb
Use private
Nov 25, 2021
6cb1a5d
Use options
Nov 25, 2021
e1db0e5
Merge branch 'main' into private-comments
Nov 25, 2021
eaa5c54
Fixup error
Nov 25, 2021
c082f66
Remove TODO
Nov 30, 2021
a1deafb
Merge branch 'main' into private-comments
Nov 30, 2021
3bfa133
Refactor into `CanSeeIssue` function
Dec 8, 2021
07d252a
Apply code review suggestions
Dec 8, 2021
37f0a21
Merge branch 'main' into private-comments
Dec 8, 2021
7253c73
Merge branch 'main' into private-comments
Dec 15, 2021
0436cd7
Fix errors
Dec 15, 2021
241d735
Apply suggestion from code-Review
Dec 15, 2021
9b93d30
Merge branch 'main' into private-comments
Mar 5, 2022
c560ddb
Merge branch 'main' into private-comments
Mar 15, 2022
52b58b4
Merge branch 'main' into private-comments
May 2, 2022
8a6fb51
Fix panic on projects page
May 2, 2022
d268277
Fix some more problems
May 2, 2022
f581410
Fix projects with confidential Issues
May 3, 2022
0ed31e2
Gofumpt code
May 3, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 22 additions & 14 deletions models/action.go
Original file line number Diff line number Diff line change
@@ -67,20 +67,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_model.User `xorm:"-"`
RepoID int64 `xorm:"INDEX"`
Repo *repo_model.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_model.User `xorm:"-"`
RepoID int64 `xorm:"INDEX"`
Repo *repo_model.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() {
@@ -455,6 +456,7 @@ func notifyWatchers(ctx context.Context, actions ...*Action) error {
var permCode []bool
var permIssue []bool
var permPR []bool
var permPrivateIssue []bool

e := db.GetEngine(ctx)

@@ -500,6 +502,7 @@ func notifyWatchers(ctx context.Context, 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 := user_model.GetUserByIDEngine(e, watcher.UserID)
if err != nil {
@@ -518,6 +521,7 @@ func notifyWatchers(ctx context.Context, 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()
}
}

@@ -529,6 +533,10 @@ func notifyWatchers(ctx context.Context, 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] {
19 changes: 19 additions & 0 deletions models/error.go
Original file line number Diff line number Diff line change
@@ -761,6 +761,25 @@ func (err ErrPullWasClosed) Error() string {
return fmt.Sprintf("Pull request [%d] %d was already closed", err.ID, err.Index)
}

// 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 has no permission to view it [id: %d, repo_id: %d, index: %d, user_id: %d]", err.ID, err.RepoID, err.Index, err.UserID)
}

// __________ .__ .__ __________ __
// \______ \__ __| | | |\______ \ ____ ________ __ ____ _______/ |_
// | ___/ | \ | | | | _// __ \/ ____/ | \_/ __ \ / ___/\ __\
2 changes: 2 additions & 0 deletions models/fixtures/attachment.yml
Original file line number Diff line number Diff line change
@@ -91,6 +91,7 @@
id: 10
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a20
repo_id: 0 # TestGetAttachment/NotLinked
issue_id: 0
uploader_id: 8
name: attach1
download_count: 0
@@ -100,6 +101,7 @@
id: 11
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a21
repo_id: 40
issue_id: 16
release_id: 2
name: attach1
download_count: 0
12 changes: 12 additions & 0 deletions models/fixtures/issue.yml
Original file line number Diff line number Diff line change
@@ -184,3 +184,15 @@
is_pull: false
created_unix: 1602935696
updated_unix: 1602935696

-
id: 16
repo_id: 40
index: 1
poster_id: 2
name: issue in active repo
content: we are a no-op issue for some attachment, pretty special
is_closed: false
is_pull: false
created_unix: 1602935696
updated_unix: 1602935696
135 changes: 129 additions & 6 deletions models/issue.go
Original file line number Diff line number Diff line change
@@ -80,6 +80,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:"-"`
@@ -362,6 +364,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_model.User, canSeePrivateIssues bool) error {
return issue.loadCommentsAsUser(db.GetEngine(db.DefaultContext), user, canSeePrivateIssues)
}

func (issue *Issue) loadCommentsAsUser(e db.Engine, user *user_model.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.DefaultContext)
@@ -448,6 +469,14 @@ func (issue *Issue) IsPoster(uid int64) bool {
return issue.OriginalAuthorID == 0 && issue.PosterID == uid
}

// CanSeeIssue returns true when a given user can view the issue.
func (issue *Issue) CanSeeIssue(userID int64, repoPermission *Permission) bool {
if issue.IsPrivate {
return repoPermission.CanReadPrivateIssues() || issue.IsPoster(userID)
}
return true
}

func (issue *Issue) getLabels(e db.Engine) (err error) {
if len(issue.Labels) > 0 {
return nil
@@ -753,6 +782,49 @@ func ChangeIssueTitle(issue *Issue, doer *user_model.User, oldTitle string) (err
return committer.Commit()
}

// ChangePrivate changes the private status of the issue, as the given user.
func (issue *Issue) ChangePrivate(doer *user_model.User, isConfidential bool) (err error) {
ctx, committer, err := db.TxContext()
if err != nil {
return err
}
defer committer.Close()

if err = UpdateIssueCols(ctx, issue, "is_private"); err != nil {
return fmt.Errorf("updateIssueCols: %v", err)
}

if err = issue.LoadRepo(ctx); err != nil {
return fmt.Errorf("loadRepo: %v", err)
}

opts := &CreateCommentOptions{
Type: CommentTypeConfidentialChanged,
Doer: doer,
Repo: issue.Repo,
Issue: issue,
OldConfidential: !issue.IsPrivate,
NewConfidential: issue.IsPrivate,
}
if _, err = CreateCommentCtx(ctx, opts); err != nil {
return fmt.Errorf("createComment: %v", err)
}
if err = issue.addCrossReferences(ctx, doer, true); err != nil {
return err
}

engine := db.GetEngine(ctx)
if isConfidential {
_, err = engine.Exec("UPDATE `repository` SET num_private_issues = num_private_issues + 1 WHERE id = ?", opts.Issue.RepoID)
_, err = engine.Exec("UPDATE `repository` SET num_issues = num_issues - 1 WHERE id = ?", opts.Issue.RepoID)
} else {
_, err = engine.Exec("UPDATE `repository` SET num_private_issues = num_private_issues - 1 WHERE id = ?", opts.Issue.RepoID)
_, err = engine.Exec("UPDATE `repository` SET num_issues = num_issues + 1 WHERE id = ?", opts.Issue.RepoID)
}

return committer.Commit()
}

// ChangeIssueRef changes the branch of this issue, as the given user.
func ChangeIssueRef(issue *Issue, doer *user_model.User, oldRef string) (err error) {
ctx, committer, err := db.TxContext()
@@ -981,7 +1053,11 @@ func newIssue(ctx context.Context, doer *user_model.User, opts NewIssueOptions)
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
@@ -1199,6 +1275,7 @@ type IssuesOptions struct {
MilestoneIDs []int64
ProjectID int64
ProjectBoardID int64
UserID int64
IsClosed util.OptionalBool
IsPull util.OptionalBool
LabelIDs []int64
@@ -1212,6 +1289,7 @@ type IssuesOptions struct {
// prioritize issues from this repo
PriorityRepoID int64
IsArchived util.OptionalBool
CanSeePrivate bool
Org *organization.Organization // issues permission scope
Team *organization.Team // issues permission scope
User *user_model.User // issues permission scope
@@ -1333,6 +1411,10 @@ func (opts *IssuesOptions) setupSessionNoLimit(sess *xorm.Session) {
}
}

if !opts.CanSeePrivate {
applyPosterPrivateIssues(sess, opts.UserID)
}

switch opts.IsPull {
case util.OptionalBoolTrue:
sess.And("issue.is_pull=?", true)
@@ -1433,6 +1515,27 @@ func applyReviewRequestedCondition(sess *xorm.Session, reviewRequestedID int64)
reviewRequestedID, ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest, reviewRequestedID)
}

func applyPosterPrivateIssues(sess *xorm.Session, userID int64) *xorm.Session {
if userID == 0 {
return sess.And("issue.is_private=?", false)
}
// OR:
// All non-private issues by default
// All issues that satisfy the condtion
// AND:
// All private issues
// That are in the group of where the user is the poster.
return sess.And(
builder.Or(
builder.Eq{"`issue`.is_private": false},
builder.And(
builder.Eq{"`issue`.is_private": true},
builder.In("`issue`.poster_id", userID),
),
),
)
}

// CountIssuesByRepo map from repoID to number of issues matching the options
func CountIssuesByRepo(opts *IssuesOptions) (map[int64]int64, error) {
e := db.GetEngine(db.DefaultContext)
@@ -1487,7 +1590,7 @@ func Issues(opts *IssuesOptions) ([]*Issue, error) {
opts.setupSessionWithLimit(sess)
sortIssuesSession(sess, opts.SortType, opts.PriorityRepoID)

issues := make([]*Issue, 0, opts.ListOptions.PageSize)
issues := make([]*Issue, 0, opts.PageSize)
if err := sess.Find(&issues); err != nil {
return nil, fmt.Errorf("unable to query Issues: %w", err)
}
@@ -1587,8 +1690,10 @@ type IssueStatsOptions struct {
MentionedID int64
PosterID int64
ReviewRequestedID int64
UserID int64
IsPull util.OptionalBool
IssueIDs []int64
CanSeePrivate bool
}

const (
@@ -1597,7 +1702,7 @@ const (
maxQueryParameters = 300
)

// GetIssueStats returns issue statistic information by given conditions.
// GetIssueStats returns issue statistic information by given conditions, as User
func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) {
if len(opts.IssueIDs) <= maxQueryParameters {
return getIssueStatsChunk(opts, opts.IssueIDs)
@@ -1676,6 +1781,10 @@ func getIssueStatsChunk(opts *IssueStatsOptions, issueIDs []int64) (*IssueStats,
applyReviewRequestedCondition(sess, opts.ReviewRequestedID)
}

if !opts.CanSeePrivate {
applyPosterPrivateIssues(sess, opts.UserID)
}

switch opts.IsPull {
case util.OptionalBoolTrue:
sess.And("issue.is_pull=?", true)
@@ -1747,6 +1856,16 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) {
s.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
}

@@ -2196,9 +2315,13 @@ func (issue *Issue) BlockingDependencies() ([]*DependencyInfo, error) {

func updateIssueClosedNum(ctx context.Context, issue *Issue) (err error) {
if issue.IsPull {
err = repoStatsCorrectNumClosed(ctx, issue.RepoID, true, "num_closed_pulls")
err = repoStatsCorrectNumClosed(ctx, issue.RepoID, true, false, "num_closed_pulls")
} else {
err = repoStatsCorrectNumClosed(ctx, issue.RepoID, false, "num_closed_issues")
if issue.IsPrivate {
err = repoStatsCorrectNumClosed(ctx, issue.RepoID, false, true, "num_private_issues")
} else {
err = repoStatsCorrectNumClosed(ctx, issue.RepoID, false, false, "num_closed_issues")
}
}
return
}
@@ -2341,7 +2464,7 @@ func ResolveIssueMentionsByVisibility(ctx context.Context, issue *Issue, doer *u
if err != nil {
return nil, fmt.Errorf("GetUserRepoPermission [%d]: %v", user.ID, err)
}
if !perm.CanReadIssuesOrPulls(issue.IsPull) {
if !perm.CanReadIssuesOrPulls(issue.IsPull) || !issue.CanSeeIssue(user.ID, &perm) {
continue
}
users = append(users, user)
Loading