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

modules/context/repo.go

@@ -91,12 +91,12 @@ func (r *Repository) CanUseTimetracker(issue *models.Issue, user *models.User) b
 	// 2. Is the user a contributor, admin, poster or assignee and do the repository policies require this?
 	isAssigned, _ := models.IsUserAssignedToIssue(issue, user)
 	return r.Repository.IsTimetrackerEnabled() && (!r.Repository.AllowOnlyContributorsToTrackTime() ||
-		r.Permission.CanWrite(models.UnitTypeIssues) || issue.IsPoster(user.ID) || isAssigned)
+		r.Permission.CanWriteIssuesOrPulls(issue.IsPull) || issue.IsPoster(user.ID) || isAssigned)
 }
 
 // CanCreateIssueDependencies returns whether or not a user can create dependencies.
-func (r *Repository) CanCreateIssueDependencies(user *models.User) bool {
+func (r *Repository) CanCreateIssueDependencies(user *models.User, isPull bool) bool {
-	return r.Permission.CanWrite(models.UnitTypeIssues) && r.Repository.IsDependenciesEnabled()
+	return r.Repository.IsDependenciesEnabled() && r.Permission.CanWriteIssuesOrPulls(isPull)
 }
 
 // GetCommitsCount returns cached commit count for current view

public/js/index.js

@@ -3134,10 +3134,11 @@ function deleteDependencyModal(id, type) {
 
 function initIssueList() {
     const repolink = $('#repolink').val();
+    const tp = $('#type').val();
     $('#new-dependency-drop-list')
         .dropdown({
             apiSettings: {
-                url: suburl + '/api/v1/repos/' + repolink + '/issues?q={query}',
+                url: suburl + '/api/v1/repos/' + repolink + '/issues?q={query}&type='+tp,
                 onResponse: function(response) {
                     const filteredResponse = {'success': true, 'results': []};
                     const currIssueId = $('#new-dependency-drop-list').data('issue-id');

routers/api/v1/repo/issue.go

@@ -57,6 +57,10 @@ func ListIssues(ctx *context.APIContext) {
 	//   in: query
 	//   description: search string
 	//   type: string
+	// - name: type
+	//   in: query
+	//   description: filter by type (issues / pulls) if set
+	//   type: string
 	// responses:
 	//   "200":
 	//     "$ref": "#/responses/IssueList"
@@ -91,6 +95,16 @@ func ListIssues(ctx *context.APIContext) {
 		}
 	}
 
+	var isPull util.OptionalBool
+	switch ctx.Query("type") {
+	case "pulls":
+		isPull = util.OptionalBoolTrue
+	case "issues":
+		isPull = util.OptionalBoolFalse
+	default:
+		isPull = util.OptionalBoolNone
+	}
+
 	// 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 {
@@ -101,6 +115,7 @@ func ListIssues(ctx *context.APIContext) {
 			IsClosed: isClosed,
 			IssueIDs: issueIDs,
 			LabelIDs: labelIDs,
+			IsPull:   isPull,
 		})
 	}
 
@@ -292,6 +307,7 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
 		return
 	}
 	issue.Repo = ctx.Repo.Repository
+	canWrite := ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull)
 
 	err = issue.LoadAttributes()
 	if err != nil {
@@ -299,7 +315,7 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
 		return
 	}
 
-	if !issue.IsPoster(ctx.User.ID) && !ctx.Repo.CanWrite(models.UnitTypeIssues) {
+	if !issue.IsPoster(ctx.User.ID) && !canWrite {
 		ctx.Status(403)
 		return
 	}
@@ -312,7 +328,7 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
 	}
 
 	// Update the deadline
-	if form.Deadline != nil && ctx.Repo.CanWrite(models.UnitTypeIssues) {
+	if form.Deadline != nil && canWrite {
 		deadlineUnix := timeutil.TimeStamp(form.Deadline.Unix())
 		if err := models.UpdateIssueDeadline(issue, deadlineUnix, ctx.User); err != nil {
 			ctx.Error(500, "UpdateIssueDeadline", err)
@@ -329,7 +345,7 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
 	// Pass one or more user logins to replace the set of assignees on this Issue.
 	// Send an empty array ([]) to clear all assignees from the Issue.
 
-	if ctx.Repo.CanWrite(models.UnitTypeIssues) && (form.Assignees != nil || form.Assignee != nil) {
+	if canWrite && (form.Assignees != nil || form.Assignee != nil) {
 		oneAssignee := ""
 		if form.Assignee != nil {
 			oneAssignee = *form.Assignee
@@ -342,7 +358,7 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
 		}
 	}
 
-	if ctx.Repo.CanWrite(models.UnitTypeIssues) && form.Milestone != nil &&
+	if canWrite && form.Milestone != nil &&
 		issue.MilestoneID != *form.Milestone {
 		oldMilestoneID := issue.MilestoneID
 		issue.MilestoneID = *form.Milestone
@@ -430,7 +446,7 @@ func UpdateIssueDeadline(ctx *context.APIContext, form api.EditDeadlineOption) {
 		return
 	}
 
-	if !ctx.Repo.CanWrite(models.UnitTypeIssues) {
+	if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
 		ctx.Status(403)
 		return
 	}
@@ -497,7 +513,7 @@ func StartIssueStopwatch(ctx *context.APIContext) {
 		return
 	}
 
-	if !ctx.Repo.CanWrite(models.UnitTypeIssues) {
+	if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
 		ctx.Status(403)
 		return
 	}
@@ -566,7 +582,7 @@ func StopIssueStopwatch(ctx *context.APIContext) {
 		return
 	}
 
-	if !ctx.Repo.CanWrite(models.UnitTypeIssues) {
+	if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
 		ctx.Status(403)
 		return
 	}

routers/api/v1/repo/issue_comment.go

@@ -185,7 +185,7 @@ func CreateIssueComment(ctx *context.APIContext, form api.CreateIssueCommentOpti
 		return
 	}
 
-	if issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) && !ctx.User.IsAdmin {
+	if issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) && !ctx.User.IsAdmin {
 		ctx.Error(403, "CreateIssueComment", errors.New(ctx.Tr("repo.issues.comment_on_locked")))
 		return
 	}

routers/repo/compare.go

@@ -407,7 +407,7 @@ func CompareDiff(ctx *context.Context) {
 
 		if !nothingToCompare {
 			// Setup information for new form.
-			RetrieveRepoMetas(ctx, ctx.Repo.Repository)
+			RetrieveRepoMetas(ctx, ctx.Repo.Repository, true)
 			if ctx.Written() {
 				return
 			}

routers/repo/issue.go

@@ -67,7 +67,7 @@ func MustAllowUserComment(ctx *context.Context) {
 		return
 	}
 
-	if issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) && !ctx.User.IsAdmin {
+	if issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) && !ctx.User.IsAdmin {
 		ctx.Flash.Error(ctx.Tr("repo.issues.comment_on_locked"))
 		ctx.Redirect(issue.HTMLURL())
 		return
@@ -346,8 +346,8 @@ func RetrieveRepoMilestonesAndAssignees(ctx *context.Context, repo *models.Repos
 }
 
 // RetrieveRepoMetas find all the meta information of a repository
-func RetrieveRepoMetas(ctx *context.Context, repo *models.Repository) []*models.Label {
+func RetrieveRepoMetas(ctx *context.Context, repo *models.Repository, isPull bool) []*models.Label {
-	if !ctx.Repo.CanWrite(models.UnitTypeIssues) {
+	if !ctx.Repo.CanWriteIssuesOrPulls(isPull) {
 		return nil
 	}
 
@@ -371,7 +371,7 @@ func RetrieveRepoMetas(ctx *context.Context, repo *models.Repository) []*models.
 	ctx.Data["Branches"] = brs
 
 	// Contains true if the user can create issue dependencies
-	ctx.Data["CanCreateIssueDependencies"] = ctx.Repo.CanCreateIssueDependencies(ctx.User)
+	ctx.Data["CanCreateIssueDependencies"] = ctx.Repo.CanCreateIssueDependencies(ctx.User, isPull)
 
 	return labels
 }
@@ -441,7 +441,7 @@ func NewIssue(ctx *context.Context) {
 	setTemplateIfExists(ctx, issueTemplateKey, IssueTemplateCandidates)
 	renderAttachmentSettings(ctx)
 
-	RetrieveRepoMetas(ctx, ctx.Repo.Repository)
+	RetrieveRepoMetas(ctx, ctx.Repo.Repository, false)
 	if ctx.Written() {
 		return
 	}
@@ -456,7 +456,7 @@ func ValidateRepoMetas(ctx *context.Context, form auth.CreateIssueForm, isPull b
 		err  error
 	)
 
-	labels := RetrieveRepoMetas(ctx, ctx.Repo.Repository)
+	labels := RetrieveRepoMetas(ctx, ctx.Repo.Repository, isPull)
 	if ctx.Written() {
 		return nil, nil, 0
 	}
@@ -776,8 +776,16 @@ func ViewIssue(ctx *context.Context) {
 		}
 	}
 
+	if issue.IsPull && !ctx.Repo.CanRead(models.UnitTypeIssues) {
+		ctx.Data["IssueType"] = "pulls"
+	} else if !issue.IsPull && !ctx.Repo.CanRead(models.UnitTypePullRequests) {
+		ctx.Data["IssueType"] = "issues"
+	} else {
+		ctx.Data["IssueType"] = "all"
+	}
+
 	// Check if the user can use the dependencies
-	ctx.Data["CanCreateIssueDependencies"] = ctx.Repo.CanCreateIssueDependencies(ctx.User)
+	ctx.Data["CanCreateIssueDependencies"] = ctx.Repo.CanCreateIssueDependencies(ctx.User, issue.IsPull)
 
 	// Render comments and and fetch participants.
 	participants[0] = issue.Poster
@@ -963,7 +971,6 @@ func ViewIssue(ctx *context.Context) {
 	ctx.Data["IsIssuePoster"] = ctx.IsSigned && issue.IsPoster(ctx.User.ID)
 	ctx.Data["IsIssueWriter"] = ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull)
 	ctx.Data["IsRepoAdmin"] = ctx.IsSigned && (ctx.Repo.IsAdmin() || ctx.User.IsAdmin)
-	ctx.Data["IsRepoIssuesWriter"] = ctx.IsSigned && (ctx.Repo.CanWrite(models.UnitTypeIssues) || ctx.User.IsAdmin)
 	ctx.Data["LockReasons"] = setting.Repository.Issue.LockReasons
 	ctx.HTML(200, tplIssueView)
 }
@@ -1208,7 +1215,7 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) {
 		ctx.Error(403)
 	}
 
-	if issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) && !ctx.User.IsAdmin {
+	if issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) && !ctx.User.IsAdmin {
 		ctx.Flash.Error(ctx.Tr("repo.issues.comment_on_locked"))
 		ctx.Redirect(issue.HTMLURL(), http.StatusSeeOther)
 		return

routers/repo/issue_dependency.go

@@ -14,14 +14,6 @@ import (
 
 // AddDependency adds new dependencies
 func AddDependency(ctx *context.Context) {
-	// Check if the Repo is allowed to have dependencies
-	if !ctx.Repo.CanCreateIssueDependencies(ctx.User) {
-		ctx.Error(http.StatusForbidden, "CanCreateIssueDependencies")
-		return
-	}
-
-	depID := ctx.QueryInt64("newDependency")
-
 	issueIndex := ctx.ParamsInt64("index")
 	issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, issueIndex)
 	if err != nil {
@@ -29,6 +21,14 @@ func AddDependency(ctx *context.Context) {
 		return
 	}
 
+	// Check if the Repo is allowed to have dependencies
+	if !ctx.Repo.CanCreateIssueDependencies(ctx.User, issue.IsPull) {
+		ctx.Error(http.StatusForbidden, "CanCreateIssueDependencies")
+		return
+	}
+
+	depID := ctx.QueryInt64("newDependency")
+
 	// Redirect
 	defer ctx.Redirect(fmt.Sprintf("%s/issues/%d", ctx.Repo.RepoLink, issueIndex), http.StatusSeeOther)
 
@@ -68,14 +68,6 @@ func AddDependency(ctx *context.Context) {
 
 // RemoveDependency removes the dependency
 func RemoveDependency(ctx *context.Context) {
-	// Check if the Repo is allowed to have dependencies
-	if !ctx.Repo.CanCreateIssueDependencies(ctx.User) {
-		ctx.Error(http.StatusForbidden, "CanCreateIssueDependencies")
-		return
-	}
-
-	depID := ctx.QueryInt64("removeDependencyID")
-
 	issueIndex := ctx.ParamsInt64("index")
 	issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, issueIndex)
 	if err != nil {
@@ -83,8 +75,13 @@ func RemoveDependency(ctx *context.Context) {
 		return
 	}
 
-	// Redirect
+	// Check if the Repo is allowed to have dependencies
-	ctx.Redirect(fmt.Sprintf("%s/issues/%d", ctx.Repo.RepoLink, issueIndex), http.StatusSeeOther)
+	if !ctx.Repo.CanCreateIssueDependencies(ctx.User, issue.IsPull) {
+		ctx.Error(http.StatusForbidden, "CanCreateIssueDependencies")
+		return
+	}
+
+	depID := ctx.QueryInt64("removeDependencyID")
 
 	// Dependency Type
 	depTypeStr := ctx.Req.PostForm.Get("dependencyType")
@@ -116,4 +113,7 @@ func RemoveDependency(ctx *context.Context) {
 		ctx.ServerError("RemoveIssueDependency", err)
 		return
 	}
+
+	// Redirect
+	ctx.Redirect(fmt.Sprintf("%s/issues/%d", ctx.Repo.RepoLink, issueIndex), http.StatusSeeOther)
 }

routers/routes/routes.go

@@ -532,18 +532,12 @@ func RegisterRoutes(m *macaron.Macaron) {
 	reqRepoReleaseReader := context.RequireRepoReader(models.UnitTypeReleases)
 	reqRepoWikiWriter := context.RequireRepoWriter(models.UnitTypeWiki)
 	reqRepoIssueReader := context.RequireRepoReader(models.UnitTypeIssues)
+	reqRepoIssueWriter := context.RequireRepoWriter(models.UnitTypeIssues)
 	reqRepoPullsWriter := context.RequireRepoWriter(models.UnitTypePullRequests)
 	reqRepoPullsReader := context.RequireRepoReader(models.UnitTypePullRequests)
 	reqRepoIssuesOrPullsWriter := context.RequireRepoWriterOr(models.UnitTypeIssues, models.UnitTypePullRequests)
 	reqRepoIssuesOrPullsReader := context.RequireRepoReaderOr(models.UnitTypeIssues, models.UnitTypePullRequests)
 
-	reqRepoIssueWriter := func(ctx *context.Context) {
-		if !ctx.Repo.CanWrite(models.UnitTypeIssues) {
-			ctx.Error(403)
-			return
-		}
-	}
-
 	// ***** START: Organization *****
 	m.Group("/org", func() {
 		m.Group("", func() {

templates/repo/issue/view_content.tmpl

@@ -78,7 +78,7 @@
 				{{ template "repo/issue/view_content/pull". }}
 			{{end}}
 			{{if .IsSigned}}
-				{{ if and (or .IsRepoAdmin .IsRepoIssuesWriter (or (not .Issue.IsLocked))) (not .Repository.IsArchived) }}
+				{{ if and (or .IsRepoAdmin .IsIssuesWriter (or (not .Issue.IsLocked))) (not .Repository.IsArchived) }}
 				<div class="comment form">
 					<a class="avatar" href="{{.SignedUser.HomeLink}}">
 						<img src="{{.SignedUser.RelAvatarLink}}">

templates/repo/issue/view_content/sidebar.tmpl

@@ -426,6 +426,7 @@
 	<input type="hidden" id="repolink" value="{{$.RepoRelPath}}">
 	<!-- I know, there is probably a better way to do this -->
 	<input type="hidden" id="issueIndex" value="{{.Issue.Index}}"/>
+	<input type="hidden" id="type" value="{{.IssueType}}">
 
 	<div class="ui basic modal remove-dependency">
 		<div class="ui icon header">

templates/swagger/v1_json.tmpl

@@ -2777,6 +2777,12 @@
             "description": "search string",
             "name": "q",
             "in": "query"
+          },
+          {
+            "type": "string",
+            "description": "filter by type (issues / pulls) if set",
+            "name": "type",
+            "in": "query"
           }
         ],
         "responses": {