femsci/forgejo
femsci/forgejo
1
0
Fork 0
forked from NYANDEV/forgejo
Code Pull requests Activity

Fix wrong permissions check when issues/prs shared operations (#9885)

Browse source 
* Fix wrong permissions check when issues/prs shared operations

* move redirect to the last of the function

* fix swagger

Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: Lauris BH <lauris@nix.lv>
This commit is contained in:
Lunny Xiao 2020-01-20 20:00:32 +08:00 committed by Antoine GIRARD
parent 81cfe243f9
commit 6d6f1d568e
11 changed files with 43 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
modules/context/repo.go
View file

@ -134,7 +134,7 @@ 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.

4
modules/repofiles/action.go
View file

@ -104,8 +104,8 @@ func UpdateIssuesCommit(doer *models.User, repo *models.Repository, commits []*r
refMarked[key] = true
// FIXME: this kind of condition is all over the code, it should be consolidated in a single place
canclose := perm.IsAdmin() || perm.IsOwner() || perm.CanWrite(models.UnitTypeIssues) || refIssue.PosterID == doer.ID
cancomment := canclose || perm.CanRead(models.UnitTypeIssues)
canclose := perm.IsAdmin() || perm.IsOwner() || perm.CanWriteIssuesOrPulls(refIssue.IsPull) || refIssue.PosterID == doer.ID
cancomment := canclose || perm.CanReadIssuesOrPulls(refIssue.IsPull)
// Don't proceed if the user can't comment
if !cancomment {

26
routers/api/v1/repo/issue.go
View file

@ -206,6 +206,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"
@ -241,6 +245,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 {
@ -251,6 +265,7 @@ func ListIssues(ctx *context.APIContext) {
IsClosed: isClosed,
IssueIDs: issueIDs,
LabelIDs: labelIDs,
IsPull: isPull,
})
}
@ -475,6 +490,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 {
@ -482,7 +498,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(http.StatusForbidden)
return
}
@ -495,7 +511,7 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
}
// Update or remove the deadline, only if set and allowed
if (form.Deadline != nil || form.RemoveDeadline != nil) && ctx.Repo.CanWrite(models.UnitTypeIssues) {
if (form.Deadline != nil || form.RemoveDeadline != nil) && canWrite {
var deadlineUnix timeutil.TimeStamp
if (form.RemoveDeadline == nil || !*form.RemoveDeadline) && !form.Deadline.IsZero() {
@ -519,7 +535,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
@ -532,7 +548,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
@ -618,7 +634,7 @@ func UpdateIssueDeadline(ctx *context.APIContext, form api.EditDeadlineOption) {
return
}
if !ctx.Repo.CanWrite(models.UnitTypeIssues) {
if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
ctx.Error(http.StatusForbidden, "", "Not repo writer")
return
}

2
routers/api/v1/repo/issue_comment.go
View file

@ -204,7 +204,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(http.StatusForbidden, "CreateIssueComment", errors.New(ctx.Tr("repo.issues.comment_on_locked")))
return
}

4
routers/api/v1/repo/issue_reaction.go
View file

@ -179,7 +179,7 @@ func changeIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOp
ctx.Error(http.StatusInternalServerError, "comment.LoadIssue() failed", err)
}
if comment.Issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) {
if comment.Issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull) {
ctx.Error(http.StatusForbidden, "ChangeIssueCommentReaction", errors.New("no permission to change reaction"))
return
}
@ -380,7 +380,7 @@ func changeIssueReaction(ctx *context.APIContext, form api.EditReactionOption, i
return
}
if issue.IsLocked && !ctx.Repo.CanWrite(models.UnitTypeIssues) {
if issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
ctx.Error(http.StatusForbidden, "ChangeIssueCommentReaction", errors.New("no permission to change reaction"))
return
}

2
routers/api/v1/repo/issue_stopwatch.go
View file

@ -170,7 +170,7 @@ func prepareIssueStopwatch(ctx *context.APIContext, shouldExist bool) (*models.I
return nil, err
}
if !ctx.Repo.CanWrite(models.UnitTypeIssues) {
if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
ctx.Status(http.StatusForbidden)
return nil, err
}

9
routers/repo/issue.go
View file

@ -63,13 +63,12 @@ var (
// If locked and user has permissions to write to the repository,
// then the comment is allowed, else it is blocked
func MustAllowUserComment(ctx *context.Context) {
issue := GetActionIssue(ctx)
if ctx.Written() {
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
@ -344,7 +343,7 @@ 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, isPull bool) []*models.Label {
if !ctx.Repo.CanWrite(models.UnitTypeIssues) {
if !ctx.Repo.CanWriteIssuesOrPulls(isPull) {
return nil
}
@ -1022,7 +1021,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)
}
@ -1283,9 +1281,10 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) {
}
ctx.Error(403)
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(), http.StatusSeeOther)
return

6
routers/repo/issue_dependency.go
View file

@ -93,9 +93,6 @@ func RemoveDependency(ctx *context.Context) {
return
}
// Redirect
ctx.Redirect(issue.HTMLURL(), http.StatusSeeOther)
// Dependency Type
depTypeStr := ctx.Req.PostForm.Get("dependencyType")
@ -126,4 +123,7 @@ func RemoveDependency(ctx *context.Context) {
ctx.ServerError("RemoveIssueDependency", err)
return
}
// Redirect
ctx.Redirect(issue.HTMLURL(), http.StatusSeeOther)
}

8
routers/routes/routes.go
View file

@ -508,19 +508,13 @@ func RegisterRoutes(m *macaron.Macaron) {
reqRepoReleaseWriter := context.RequireRepoWriter(models.UnitTypeReleases)
reqRepoReleaseReader := context.RequireRepoReader(models.UnitTypeReleases)
reqRepoWikiWriter := context.RequireRepoWriter(models.UnitTypeWiki)
reqRepoIssueWriter := context.RequireRepoWriter(models.UnitTypeIssues)
reqRepoIssueReader := context.RequireRepoReader(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() {

2
templates/repo/issue/view_content.tmpl
View file

@ -66,7 +66,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 .IsIssueWriter (or (not .Issue.IsLocked))) (not .Repository.IsArchived) }}
<div class="comment form">
<a class="avatar" href="{{.SignedUser.HomeLink}}">
<img src="{{.SignedUser.RelAvatarLink}}">

6
templates/swagger/v1_json.tmpl
View file

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