Skip to content
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

Adding private issues functionality #17711

Closed
wants to merge 85 commits into from
Closed
Show file tree
Hide file tree
Changes from 80 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
Expand Up @@ -66,20 +66,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() {
Expand Down Expand Up @@ -454,6 +455,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)

Expand Down Expand Up @@ -499,6 +501,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 {
Expand All @@ -517,6 +520,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()
}
}

Expand All @@ -528,6 +532,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] {
Expand Down
19 changes: 19 additions & 0 deletions models/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -847,6 +847,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 has no permission to view it [id: %d, repo_id: %d, index: %d, user_id: %d]", err.ID, err.RepoID, err.Index, err.UserID)
}

// __________ .__ .__ __________ __
// \______ \__ __| | | |\______ \ ____ ________ __ ____ _______/ |_
// | ___/ | \ | | | | _// __ \/ ____/ | \_/ __ \ / ___/\ __\
Expand Down
2 changes: 2 additions & 0 deletions models/fixtures/attachment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
12 changes: 12 additions & 0 deletions models/fixtures/issue.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
133 changes: 128 additions & 5 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,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"`
Gusted marked this conversation as resolved.
Show resolved Hide resolved

// For view issue page.
ShowRole RoleDescriptor `xorm:"-"`
Expand Down Expand Up @@ -340,6 +342,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.GetEngine(db.DefaultContext))
Expand Down Expand Up @@ -426,6 +447,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)
6543 marked this conversation as resolved.
Show resolved Hide resolved
}
return true
}

func (issue *Issue) hasLabel(e db.Engine, labelID int64) bool {
return hasIssueLabel(e, issue.ID, labelID)
}
Expand Down Expand Up @@ -751,6 +780,49 @@ func (issue *Issue) ChangeTitle(doer *user_model.User, oldTitle string) (err 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 = createComment(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()
}

// ChangeRef changes the branch of this issue, as the given user.
func (issue *Issue) ChangeRef(doer *user_model.User, oldRef string) (err error) {
ctx, committer, err := db.TxContext()
Expand Down Expand Up @@ -979,7 +1051,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)
Gusted marked this conversation as resolved.
Show resolved Hide resolved
} else {
_, err = e.Exec("UPDATE `repository` SET num_issues = num_issues + 1 WHERE id = ?", opts.Issue.RepoID)
Gusted marked this conversation as resolved.
Show resolved Hide resolved
}
}
if err != nil {
return err
Expand Down Expand Up @@ -1176,6 +1252,7 @@ type IssuesOptions struct {
MilestoneIDs []int64
ProjectID int64
ProjectBoardID int64
UserID int64
IsClosed util.OptionalBool
IsPull util.OptionalBool
LabelIDs []int64
Expand All @@ -1189,6 +1266,7 @@ type IssuesOptions struct {
// prioritize issues from this repo
PriorityRepoID int64
IsArchived util.OptionalBool
CanSeePrivate bool
Org *Organization // issues permission scope
Team *Team // issues permission scope
User *user_model.User // issues permission scope
Expand Down Expand Up @@ -1307,6 +1385,10 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) {
}
}

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

switch opts.IsPull {
case util.OptionalBoolTrue:
sess.And("issue.is_pull=?", true)
Expand Down Expand Up @@ -1411,6 +1493,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)
Expand Down Expand Up @@ -1576,8 +1679,10 @@ type IssueStatsOptions struct {
MentionedID int64
PosterID int64
ReviewRequestedID int64
UserID int64
IsPull util.OptionalBool
IssueIDs []int64
CanSeePrivate bool
}

const (
Expand All @@ -1586,7 +1691,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)
Expand Down Expand Up @@ -1665,6 +1770,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)
Expand Down Expand Up @@ -1732,6 +1841,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
}

Expand Down Expand Up @@ -2180,9 +2299,13 @@ func (issue *Issue) BlockingDependencies() ([]*DependencyInfo, error) {

func (issue *Issue) updateClosedNum(ctx context.Context) (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
}
Expand Down Expand Up @@ -2325,7 +2448,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.CanSeeIssue(user.ID, &perm) {
continue
}
users = append(users, user)
Expand Down
Loading