Skip to content

Commit c4e0f71

Browse files
lunnysapk
lunny
authored and
sapk
committed
fix bug about wrong dependencies permissions check and other wr… (#9884)
* fix bug about wrong dependencies permissions check and other wrong permissions check * improve code
1 parent e3e0248 commit c4e0f71

File tree

11 files changed

+73
-48
lines changed

11 files changed

+73
-48
lines changed

‎modules/context/repo.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,12 @@ func (r *Repository) CanUseTimetracker(issue *models.Issue, user *models.User) b
9191
// 2. Is the user a contributor, admin, poster or assignee and do the repository policies require this?
9292
isAssigned, _ := models.IsUserAssignedToIssue(issue, user)
9393
return r.Repository.IsTimetrackerEnabled() && (!r.Repository.AllowOnlyContributorsToTrackTime() ||
94-
r.Permission.CanWrite(models.UnitTypeIssues) || issue.IsPoster(user.ID) || isAssigned)
94+
r.Permission.CanWriteIssuesOrPulls(issue.IsPull) || issue.IsPoster(user.ID) || isAssigned)
9595
}
9696

9797
// CanCreateIssueDependencies returns whether or not a user can create dependencies.
98-
func (r *Repository) CanCreateIssueDependencies(user *models.User) bool {
99-
return r.Permission.CanWrite(models.UnitTypeIssues) && r.Repository.IsDependenciesEnabled()
98+
func (r *Repository) CanCreateIssueDependencies(user *models.User, isPull bool) bool {
99+
return r.Repository.IsDependenciesEnabled() && r.Permission.CanWriteIssuesOrPulls(isPull)
100100
}
101101

102102
// GetCommitsCount returns cached commit count for current view

‎public/js/index.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3134,10 +3134,11 @@ function deleteDependencyModal(id, type) {
31343134

31353135
function initIssueList() {
31363136
const repolink = $('#repolink').val();
3137+
const tp = $('#type').val();
31373138
$('#new-dependency-drop-list')
31383139
.dropdown({
31393140
apiSettings: {
3140-
url: suburl + '/api/v1/repos/' + repolink + '/issues?q={query}',
3141+
url: suburl + '/api/v1/repos/' + repolink + '/issues?q={query}&type='+tp,
31413142
onResponse: function(response) {
31423143
const filteredResponse = {'success': true, 'results': []};
31433144
const currIssueId = $('#new-dependency-drop-list').data('issue-id');

‎routers/api/v1/repo/issue.go‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ func ListIssues(ctx *context.APIContext) {
5757
// in: query
5858
// description: search string
5959
// type: string
60+
// - name: type
61+
// in: query
62+
// description: filter by type (issues / pulls) if set
63+
// type: string
6064
// responses:
6165
// "200":
6266
// "$ref": "#/responses/IssueList"
@@ -91,6 +95,16 @@ func ListIssues(ctx *context.APIContext) {
9195
}
9296
}
9397

98+
var isPull util.OptionalBool
99+
switch ctx.Query("type") {
100+
case "pulls":
101+
isPull = util.OptionalBoolTrue
102+
case "issues":
103+
isPull = util.OptionalBoolFalse
104+
default:
105+
isPull = util.OptionalBoolNone
106+
}
107+
94108
// Only fetch the issues if we either don't have a keyword or the search returned issues
95109
// This would otherwise return all issues if no issues were found by the search.
96110
if len(keyword) == 0 || len(issueIDs) > 0 || len(labelIDs) > 0 {
@@ -101,6 +115,7 @@ func ListIssues(ctx *context.APIContext) {
101115
IsClosed: isClosed,
102116
IssueIDs: issueIDs,
103117
LabelIDs: labelIDs,
118+
IsPull: isPull,
104119
})
105120
}
106121

@@ -292,14 +307,15 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
292307
return
293308
}
294309
issue.Repo = ctx.Repo.Repository
310+
canWrite := ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull)
295311

296312
err = issue.LoadAttributes()
297313
if err != nil {
298314
ctx.Error(500, "LoadAttributes", err)
299315
return
300316
}
301317

302-
if !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanWrite(models.UnitTypeIssues) {
318+
if !issue.IsPoster(ctx.User.ID) && !canWrite {
303319
ctx.Status(403)
304320
return
305321
}
@@ -312,7 +328,7 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
312328
}
313329

314330
// Update the deadline
315-
if form.Deadline != nil && ctx.Repo.CanWrite(models.UnitTypeIssues) {
331+
if form.Deadline != nil && canWrite {
316332
deadlineUnix := timeutil.TimeStamp(form.Deadline.Unix())
317333
if err := models.UpdateIssueDeadline(issue, deadlineUnix, ctx.User); err != nil {
318334
ctx.Error(500, "UpdateIssueDeadline", err)
@@ -329,7 +345,7 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
329345
// Pass one or more user logins to replace the set of assignees on this Issue.
330346
// Send an empty array ([]) to clear all assignees from the Issue.
331347

332-
if ctx.Repo.CanWrite(models.UnitTypeIssues) && (form.Assignees != nil || form.Assignee != nil) {
348+
if canWrite && (form.Assignees != nil || form.Assignee != nil) {
333349
oneAssignee := ""
334350
if form.Assignee != nil {
335351
oneAssignee = *form.Assignee
@@ -342,7 +358,7 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
342358
}
343359
}
344360

345-
if ctx.Repo.CanWrite(models.UnitTypeIssues) && form.Milestone != nil &&
361+
if canWrite && form.Milestone != nil &&
346362
issue.MilestoneID != *form.Milestone {
347363
oldMilestoneID := issue.MilestoneID
348364
issue.MilestoneID = *form.Milestone
@@ -430,7 +446,7 @@ func UpdateIssueDeadline(ctx *context.APIContext, form api.EditDeadlineOption) {
430446
return
431447
}
432448

433-
if !ctx.Repo.CanWrite(models.UnitTypeIssues) {
449+
if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
434450
ctx.Status(403)
435451
return
436452
}
@@ -497,7 +513,7 @@ func StartIssueStopwatch(ctx *context.APIContext) {
497513
return
498514
}
499515

500-
if !ctx.Repo.CanWrite(models.UnitTypeIssues) {
516+
if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
501517
ctx.Status(403)
502518
return
503519
}
@@ -566,7 +582,7 @@ func StopIssueStopwatch(ctx *context.APIContext) {
566582
return
567583
}
568584

569-
if !ctx.Repo.CanWrite(models.UnitTypeIssues) {
585+
if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
570586
ctx.Status(403)
571587
return
572588
}

‎routers/api/v1/repo/issue_comment.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ func CreateIssueComment(ctx *context.APIContext, form api.CreateIssueCommentOpti
185185
return
186186
}
187187

188-
if issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) && !ctx.User.IsAdmin {
188+
if issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) && !ctx.User.IsAdmin {
189189
ctx.Error(403, "CreateIssueComment", errors.New(ctx.Tr("repo.issues.comment_on_locked")))
190190
return
191191
}

‎routers/repo/compare.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ func CompareDiff(ctx *context.Context) {
407407

408408
if !nothingToCompare {
409409
// Setup information for new form.
410-
RetrieveRepoMetas(ctx, ctx.Repo.Repository)
410+
RetrieveRepoMetas(ctx, ctx.Repo.Repository, true)
411411
if ctx.Written() {
412412
return
413413
}

‎routers/repo/issue.go‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func MustAllowUserComment(ctx *context.Context) {
6767
return
6868
}
6969

70-
if issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) && !ctx.User.IsAdmin {
70+
if issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) && !ctx.User.IsAdmin {
7171
ctx.Flash.Error(ctx.Tr("repo.issues.comment_on_locked"))
7272
ctx.Redirect(issue.HTMLURL())
7373
return
@@ -346,8 +346,8 @@ func RetrieveRepoMilestonesAndAssignees(ctx *context.Context, repo *models.Repos
346346
}
347347

348348
// RetrieveRepoMetas find all the meta information of a repository
349-
func RetrieveRepoMetas(ctx *context.Context, repo *models.Repository) []*models.Label {
350-
if !ctx.Repo.CanWrite(models.UnitTypeIssues) {
349+
func RetrieveRepoMetas(ctx *context.Context, repo *models.Repository, isPull bool) []*models.Label {
350+
if !ctx.Repo.CanWriteIssuesOrPulls(isPull) {
351351
return nil
352352
}
353353

@@ -371,7 +371,7 @@ func RetrieveRepoMetas(ctx *context.Context, repo *models.Repository) []*models.
371371
ctx.Data["Branches"] = brs
372372

373373
// Contains true if the user can create issue dependencies
374-
ctx.Data["CanCreateIssueDependencies"] = ctx.Repo.CanCreateIssueDependencies(ctx.User)
374+
ctx.Data["CanCreateIssueDependencies"] = ctx.Repo.CanCreateIssueDependencies(ctx.User, isPull)
375375

376376
return labels
377377
}
@@ -441,7 +441,7 @@ func NewIssue(ctx *context.Context) {
441441
setTemplateIfExists(ctx, issueTemplateKey, IssueTemplateCandidates)
442442
renderAttachmentSettings(ctx)
443443

444-
RetrieveRepoMetas(ctx, ctx.Repo.Repository)
444+
RetrieveRepoMetas(ctx, ctx.Repo.Repository, false)
445445
if ctx.Written() {
446446
return
447447
}
@@ -456,7 +456,7 @@ func ValidateRepoMetas(ctx *context.Context, form auth.CreateIssueForm, isPull b
456456
err error
457457
)
458458

459-
labels := RetrieveRepoMetas(ctx, ctx.Repo.Repository)
459+
labels := RetrieveRepoMetas(ctx, ctx.Repo.Repository, isPull)
460460
if ctx.Written() {
461461
return nil, nil, 0
462462
}
@@ -776,8 +776,16 @@ func ViewIssue(ctx *context.Context) {
776776
}
777777
}
778778

779+
if issue.IsPull && !ctx.Repo.CanRead(models.UnitTypeIssues) {
780+
ctx.Data["IssueType"] = "pulls"
781+
} else if !issue.IsPull && !ctx.Repo.CanRead(models.UnitTypePullRequests) {
782+
ctx.Data["IssueType"] = "issues"
783+
} else {
784+
ctx.Data["IssueType"] = "all"
785+
}
786+
779787
// Check if the user can use the dependencies
780-
ctx.Data["CanCreateIssueDependencies"] = ctx.Repo.CanCreateIssueDependencies(ctx.User)
788+
ctx.Data["CanCreateIssueDependencies"] = ctx.Repo.CanCreateIssueDependencies(ctx.User, issue.IsPull)
781789

782790
// Render comments and and fetch participants.
783791
participants[0] = issue.Poster
@@ -963,7 +971,6 @@ func ViewIssue(ctx *context.Context) {
963971
ctx.Data["IsIssuePoster"] = ctx.IsSigned && issue.IsPoster(ctx.User.ID)
964972
ctx.Data["IsIssueWriter"] = ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull)
965973
ctx.Data["IsRepoAdmin"] = ctx.IsSigned && (ctx.Repo.IsAdmin() || ctx.User.IsAdmin)
966-
ctx.Data["IsRepoIssuesWriter"] = ctx.IsSigned && (ctx.Repo.CanWrite(models.UnitTypeIssues) || ctx.User.IsAdmin)
967974
ctx.Data["LockReasons"] = setting.Repository.Issue.LockReasons
968975
ctx.HTML(200, tplIssueView)
969976
}
@@ -1208,7 +1215,7 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) {
12081215
ctx.Error(403)
12091216
}
12101217

1211-
if issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) && !ctx.User.IsAdmin {
1218+
if issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) && !ctx.User.IsAdmin {
12121219
ctx.Flash.Error(ctx.Tr("repo.issues.comment_on_locked"))
12131220
ctx.Redirect(issue.HTMLURL(), http.StatusSeeOther)
12141221
return

‎routers/repo/issue_dependency.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,21 @@ import (
1414

1515
// AddDependency adds new dependencies
1616
func AddDependency(ctx *context.Context) {
17-
// Check if the Repo is allowed to have dependencies
18-
if !ctx.Repo.CanCreateIssueDependencies(ctx.User) {
19-
ctx.Error(http.StatusForbidden, "CanCreateIssueDependencies")
20-
return
21-
}
22-
23-
depID := ctx.QueryInt64("newDependency")
24-
2517
issueIndex := ctx.ParamsInt64("index")
2618
issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, issueIndex)
2719
if err != nil {
2820
ctx.ServerError("GetIssueByIndex", err)
2921
return
3022
}
3123

24+
// Check if the Repo is allowed to have dependencies
25+
if !ctx.Repo.CanCreateIssueDependencies(ctx.User, issue.IsPull) {
26+
ctx.Error(http.StatusForbidden, "CanCreateIssueDependencies")
27+
return
28+
}
29+
30+
depID := ctx.QueryInt64("newDependency")
31+
3232
// Redirect
3333
defer ctx.Redirect(fmt.Sprintf("%s/issues/%d", ctx.Repo.RepoLink, issueIndex), http.StatusSeeOther)
3434

@@ -68,23 +68,20 @@ func AddDependency(ctx *context.Context) {
6868

6969
// RemoveDependency removes the dependency
7070
func RemoveDependency(ctx *context.Context) {
71-
// Check if the Repo is allowed to have dependencies
72-
if !ctx.Repo.CanCreateIssueDependencies(ctx.User) {
73-
ctx.Error(http.StatusForbidden, "CanCreateIssueDependencies")
74-
return
75-
}
76-
77-
depID := ctx.QueryInt64("removeDependencyID")
78-
7971
issueIndex := ctx.ParamsInt64("index")
8072
issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, issueIndex)
8173
if err != nil {
8274
ctx.ServerError("GetIssueByIndex", err)
8375
return
8476
}
8577

86-
// Redirect
87-
ctx.Redirect(fmt.Sprintf("%s/issues/%d", ctx.Repo.RepoLink, issueIndex), http.StatusSeeOther)
78+
// Check if the Repo is allowed to have dependencies
79+
if !ctx.Repo.CanCreateIssueDependencies(ctx.User, issue.IsPull) {
80+
ctx.Error(http.StatusForbidden, "CanCreateIssueDependencies")
81+
return
82+
}
83+
84+
depID := ctx.QueryInt64("removeDependencyID")
8885

8986
// Dependency Type
9087
depTypeStr := ctx.Req.PostForm.Get("dependencyType")
@@ -116,4 +113,7 @@ func RemoveDependency(ctx *context.Context) {
116113
ctx.ServerError("RemoveIssueDependency", err)
117114
return
118115
}
116+
117+
// Redirect
118+
ctx.Redirect(fmt.Sprintf("%s/issues/%d", ctx.Repo.RepoLink, issueIndex), http.StatusSeeOther)
119119
}

‎routers/routes/routes.go‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -532,18 +532,12 @@ func RegisterRoutes(m *macaron.Macaron) {
532532
reqRepoReleaseReader := context.RequireRepoReader(models.UnitTypeReleases)
533533
reqRepoWikiWriter := context.RequireRepoWriter(models.UnitTypeWiki)
534534
reqRepoIssueReader := context.RequireRepoReader(models.UnitTypeIssues)
535+
reqRepoIssueWriter := context.RequireRepoWriter(models.UnitTypeIssues)
535536
reqRepoPullsWriter := context.RequireRepoWriter(models.UnitTypePullRequests)
536537
reqRepoPullsReader := context.RequireRepoReader(models.UnitTypePullRequests)
537538
reqRepoIssuesOrPullsWriter := context.RequireRepoWriterOr(models.UnitTypeIssues, models.UnitTypePullRequests)
538539
reqRepoIssuesOrPullsReader := context.RequireRepoReaderOr(models.UnitTypeIssues, models.UnitTypePullRequests)
539540

540-
reqRepoIssueWriter := func(ctx *context.Context) {
541-
if !ctx.Repo.CanWrite(models.UnitTypeIssues) {
542-
ctx.Error(403)
543-
return
544-
}
545-
}
546-
547541
// ***** START: Organization *****
548542
m.Group("/org", func() {
549543
m.Group("", func() {

‎templates/repo/issue/view_content.tmpl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@
7878
{{ template "repo/issue/view_content/pull". }}
7979
{{end}}
8080
{{if .IsSigned}}
81-
{{ if and (or .IsRepoAdmin .IsRepoIssuesWriter (or (not .Issue.IsLocked))) (not .Repository.IsArchived) }}
81+
{{ if and (or .IsRepoAdmin .IsIssuesWriter (or (not .Issue.IsLocked))) (not .Repository.IsArchived) }}
8282
<div class="comment form">
8383
<a class="avatar" href="{{.SignedUser.HomeLink}}">
8484
<img src="{{.SignedUser.RelAvatarLink}}">

‎templates/repo/issue/view_content/sidebar.tmpl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,7 @@
426426
<input type="hidden" id="repolink" value="{{$.RepoRelPath}}">
427427
<!-- I know, there is probably a better way to do this -->
428428
<input type="hidden" id="issueIndex" value="{{.Issue.Index}}"/>
429+
<input type="hidden" id="type" value="{{.IssueType}}">
429430

430431
<div class="ui basic modal remove-dependency">
431432
<div class="ui icon header">

0 commit comments

Comments
 (0)