Nulo/gitea
Archived
1
0
Fork 0
Code Issues Pull requests Projects Releases Activity

Fix permission checks for close/reopen from commit (#8875) (#9033)

Browse source 
* Fix checks for close/reopen from commit

* Fix permission order
This commit is contained in:
guillep2k 2019-11-15 19:11:40 -03:00 committed by Lauris BH
parent a0e76de75a
commit ecdb4c1750
2 changed files with 32 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
models/action.go
View file

@ -533,34 +533,47 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, bra
} }
refMarked[key] = true refMarked[key] = true
// only create comments for issues if user has permission for it // FIXME: this kind of condition is all over the code, it should be consolidated in a single place
if perm.IsAdmin() || perm.IsOwner() || perm.CanWrite(UnitTypeIssues) { canclose := perm.IsAdmin() || perm.IsOwner() || perm.CanWrite(UnitTypeIssues) || refIssue.PosterID == doer.ID
cancomment := canclose || perm.CanRead(UnitTypeIssues)
// Don't proceed if the user can't comment
if !cancomment {
continue
}
message := fmt.Sprintf(`<a href="%s/commit/%s">%s</a>`, repo.Link(), c.Sha1, html.EscapeString(c.Message)) message := fmt.Sprintf(`<a href="%s/commit/%s">%s</a>`, repo.Link(), c.Sha1, html.EscapeString(c.Message))
if err = CreateRefComment(doer, refRepo, refIssue, message, c.Sha1); err != nil { if err = CreateRefComment(doer, refRepo, refIssue, message, c.Sha1); err != nil {
return err return err
} }
// Only issues can be closed/reopened this way, and user needs the correct permissions
if refIssue.IsPull || !canclose {
continue
} }
// Process closing/reopening keywords // Only process closing/reopening keywords
if ref.Action != references.XRefActionCloses && ref.Action != references.XRefActionReopens { if ref.Action != references.XRefActionCloses && ref.Action != references.XRefActionReopens {
continue continue
} }
// Change issue status only if the commit has been pushed to the default branch. if !repo.CloseIssuesViaCommitInAnyBranch {
// and if the repo is configured to allow only that // If the issue was specified to be in a particular branch, don't allow commits in other branches to close it
// FIXME: we should be using Issue.ref if set instead of repo.DefaultBranch if refIssue.Ref != "" {
if repo.DefaultBranch != branchName && !repo.CloseIssuesViaCommitInAnyBranch { if branchName != refIssue.Ref {
continue continue
} }
// Otherwise, only process commits to the default branch
} else if branchName != repo.DefaultBranch {
continue
}
}
// only close issues in another repo if user has push access
if perm.IsAdmin() || perm.IsOwner() || perm.CanWrite(UnitTypeCode) {
if err := changeIssueStatus(refRepo, refIssue, doer, ref.Action == references.XRefActionCloses); err != nil { if err := changeIssueStatus(refRepo, refIssue, doer, ref.Action == references.XRefActionCloses); err != nil {
return err return err
} }
} }
} }
}
return nil return nil
} }

4
models/action_test.go
View file

@ -219,7 +219,7 @@ func TestUpdateIssuesCommit(t *testing.T) {
PosterID: user.ID, PosterID: user.ID,
IssueID: 1, IssueID: 1,
} }
issueBean := &Issue{RepoID: repo.ID, Index: 2} issueBean := &Issue{RepoID: repo.ID, Index: 4}
AssertNotExistsBean(t, commentBean) AssertNotExistsBean(t, commentBean)
AssertNotExistsBean(t, &Issue{RepoID: repo.ID, Index: 2}, "is_closed=1") AssertNotExistsBean(t, &Issue{RepoID: repo.ID, Index: 2}, "is_closed=1")
@ -273,7 +273,7 @@ func TestUpdateIssuesCommit_Colon(t *testing.T) {
repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
repo.Owner = user repo.Owner = user
issueBean := &Issue{RepoID: repo.ID, Index: 2} issueBean := &Issue{RepoID: repo.ID, Index: 4}
AssertNotExistsBean(t, &Issue{RepoID: repo.ID, Index: 2}, "is_closed=1") AssertNotExistsBean(t, &Issue{RepoID: repo.ID, Index: 2}, "is_closed=1")
assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, repo.DefaultBranch)) assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, repo.DefaultBranch))