From ed89b39984a9191380263eaf357c3a9c71770674 Mon Sep 17 00:00:00 2001 From: Peter Smit Date: Mon, 16 Feb 2015 12:51:56 +0200 Subject: [PATCH] Updating context and fixing permission issues The boolean flags in the repo context have been replaced with mode and two methods Also, the permissions have been brought more in line with https://help.github.com/articles/permission-levels-for-an-organization-repository/ , Admin Team members are able to change settings of their repositories. --- cmd/web.go | 4 +-- modules/middleware/context.go | 55 ++++++++++++++++++++--------------- modules/middleware/repo.go | 47 +++++++++++------------------- routers/api/v1/repo_file.go | 2 +- routers/repo/issue.go | 14 ++++----- routers/repo/release.go | 10 +++---- routers/repo/repo.go | 2 +- templates/repo/header.tmpl | 2 +- templates/repo/sidebar.tmpl | 2 +- templates/repo/toolbar.tmpl | 2 +- 10 files changed, 68 insertions(+), 72 deletions(-) diff --git a/cmd/web.go b/cmd/web.go index 3284acb9d..8b3b03c45 100644 --- a/cmd/web.go +++ b/cmd/web.go @@ -319,7 +319,7 @@ func runWeb(ctx *cli.Context) { m.Get("/template/*", dev.TemplatePreview) } - reqTrueOwner := middleware.RequireTrueOwner() + reqAdmin := middleware.RequireAdmin() // Organization. m.Group("/org", func() { @@ -394,7 +394,7 @@ func runWeb(ctx *cli.Context) { m.Post("/:name", repo.GitHooksEditPost) }, middleware.GitHookService()) }) - }, reqSignIn, middleware.RepoAssignment(true), reqTrueOwner) + }, reqSignIn, middleware.RepoAssignment(true), reqAdmin) m.Group("/:username/:reponame", func() { m.Get("/action/:action", repo.Action) diff --git a/modules/middleware/context.go b/modules/middleware/context.go index 28be3a302..a26610969 100644 --- a/modules/middleware/context.go +++ b/modules/middleware/context.go @@ -38,29 +38,7 @@ type Context struct { IsSigned bool IsBasicAuth bool - Repo struct { - IsOwner bool - IsTrueOwner bool - IsWatching bool - IsBranch bool - IsTag bool - IsCommit bool - IsAdmin bool // Current user is admin level. - HasAccess bool - Repository *models.Repository - Owner *models.User - Commit *git.Commit - Tag *git.Tag - GitRepo *git.Repository - BranchName string - TagName string - TreeName string - CommitId string - RepoLink string - CloneLink models.CloneLink - CommitsCount int - Mirror *models.Mirror - } + Repo RepoContext Org struct { IsOwner bool @@ -73,6 +51,37 @@ type Context struct { } } +type RepoContext struct { + AccessMode models.AccessMode + IsWatching bool + IsBranch bool + IsTag bool + IsCommit bool + Repository *models.Repository + Owner *models.User + Commit *git.Commit + Tag *git.Tag + GitRepo *git.Repository + BranchName string + TagName string + TreeName string + CommitId string + RepoLink string + CloneLink models.CloneLink + CommitsCount int + Mirror *models.Mirror +} + +// Return if the current user has write access for this repository +func (r RepoContext) IsOwner() bool { + return r.AccessMode >= models.ACCESS_MODE_WRITE +} + +// Return if the current user has read access for this repository +func (r RepoContext) HasAccess() bool { + return r.AccessMode >= models.ACCESS_MODE_READ +} + // HasError returns true if error occurs in form validation. func (ctx *Context) HasApiError() bool { hasErr, ok := ctx.Data["HasError"] diff --git a/modules/middleware/repo.go b/modules/middleware/repo.go index bd298819d..5c863dc01 100644 --- a/modules/middleware/repo.go +++ b/modules/middleware/repo.go @@ -58,24 +58,19 @@ func ApiRepoAssignment() macaron.Handler { return } - if ctx.IsSigned { - mode, err := models.AccessLevel(ctx.User, repo) - if err != nil { - ctx.JSON(500, &base.ApiJsonErr{"AccessLevel: " + err.Error(), base.DOC_URL}) - return - } - - ctx.Repo.IsOwner = mode >= models.ACCESS_MODE_WRITE - ctx.Repo.IsAdmin = mode >= models.ACCESS_MODE_READ - ctx.Repo.IsTrueOwner = mode >= models.ACCESS_MODE_OWNER + mode, err := models.AccessLevel(ctx.User, repo) + if err != nil { + ctx.JSON(500, &base.ApiJsonErr{"AccessLevel: " + err.Error(), base.DOC_URL}) + return } + ctx.Repo.AccessMode = mode + // Check access. - if repo.IsPrivate && !ctx.Repo.IsOwner { + if ctx.Repo.AccessMode == models.ACCESS_MODE_NONE { ctx.Error(404) return } - ctx.Repo.HasAccess = true ctx.Repo.Repository = repo } @@ -239,26 +234,18 @@ func RepoAssignment(redirect bool, args ...bool) macaron.Handler { return } - if ctx.IsSigned { - mode, err := models.AccessLevel(ctx.User, repo) - if err != nil { - ctx.Handle(500, "AccessLevel", err) - return - } - ctx.Repo.IsOwner = mode >= models.ACCESS_MODE_WRITE - ctx.Repo.IsAdmin = mode >= models.ACCESS_MODE_READ - ctx.Repo.IsTrueOwner = mode >= models.ACCESS_MODE_OWNER - if !ctx.Repo.IsTrueOwner && ctx.Repo.Owner.IsOrganization() { - ctx.Repo.IsTrueOwner = ctx.Repo.Owner.IsOwnedBy(ctx.User.Id) - } + mode, err := models.AccessLevel(ctx.User, repo) + if err != nil { + ctx.Handle(500, "AccessLevel", err) + return } + ctx.Repo.AccessMode = mode // Check access. - if repo.IsPrivate && !ctx.Repo.IsOwner { + if ctx.Repo.AccessMode == models.ACCESS_MODE_NONE { ctx.Handle(404, "no access right", err) return } - ctx.Repo.HasAccess = true ctx.Data["HasAccess"] = true @@ -306,8 +293,8 @@ func RepoAssignment(redirect bool, args ...bool) macaron.Handler { ctx.Data["Title"] = u.Name + "/" + repo.Name ctx.Data["Repository"] = repo ctx.Data["Owner"] = ctx.Repo.Repository.Owner - ctx.Data["IsRepositoryOwner"] = ctx.Repo.IsOwner - ctx.Data["IsRepositoryTrueOwner"] = ctx.Repo.IsTrueOwner + ctx.Data["IsRepositoryOwner"] = ctx.Repo.AccessMode >= models.ACCESS_MODE_WRITE + ctx.Data["IsRepositoryAdmin"] = ctx.Repo.AccessMode >= models.ACCESS_MODE_ADMIN ctx.Data["DisableSSH"] = setting.DisableSSH ctx.Repo.CloneLink, err = repo.CloneLink() @@ -362,9 +349,9 @@ func RepoAssignment(redirect bool, args ...bool) macaron.Handler { } } -func RequireTrueOwner() macaron.Handler { +func RequireAdmin() macaron.Handler { return func(ctx *Context) { - if !ctx.Repo.IsTrueOwner && !ctx.Repo.IsAdmin { + if ctx.Repo.AccessMode < models.ACCESS_MODE_ADMIN { if !ctx.IsSigned { ctx.SetCookie("redirect_to", "/"+url.QueryEscape(setting.AppSubUrl+ctx.Req.RequestURI), 0, setting.AppSubUrl) ctx.Redirect(setting.AppSubUrl + "/user/login") diff --git a/routers/api/v1/repo_file.go b/routers/api/v1/repo_file.go index a049904f9..73f97b2ca 100644 --- a/routers/api/v1/repo_file.go +++ b/routers/api/v1/repo_file.go @@ -12,7 +12,7 @@ import ( ) func GetRepoRawFile(ctx *middleware.Context) { - if ctx.Repo.Repository.IsPrivate && !ctx.Repo.HasAccess { + if !ctx.Repo.HasAccess() { ctx.Error(404) return } diff --git a/routers/repo/issue.go b/routers/repo/issue.go index bf39d9aba..40e933897 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -230,7 +230,7 @@ func CreateIssuePost(ctx *middleware.Context, form auth.CreateIssueForm) { } // Only collaborators can assign. - if !ctx.Repo.IsOwner { + if !ctx.Repo.IsOwner() { form.AssigneeId = 0 } issue := &models.Issue{ @@ -434,7 +434,7 @@ func ViewIssue(ctx *middleware.Context) { ctx.Data["Title"] = issue.Name ctx.Data["Issue"] = issue ctx.Data["Comments"] = comments - ctx.Data["IsIssueOwner"] = ctx.Repo.IsOwner || (ctx.IsSigned && issue.PosterId == ctx.User.Id) + ctx.Data["IsIssueOwner"] = ctx.Repo.IsOwner() || (ctx.IsSigned && issue.PosterId == ctx.User.Id) ctx.Data["IsRepoToolbarIssues"] = true ctx.Data["IsRepoToolbarIssuesList"] = false ctx.HTML(200, ISSUE_VIEW) @@ -457,7 +457,7 @@ func UpdateIssue(ctx *middleware.Context, form auth.CreateIssueForm) { return } - if ctx.User.Id != issue.PosterId && !ctx.Repo.IsOwner { + if ctx.User.Id != issue.PosterId && !ctx.Repo.IsOwner() { ctx.Error(403) return } @@ -484,7 +484,7 @@ func UpdateIssue(ctx *middleware.Context, form auth.CreateIssueForm) { } func UpdateIssueLabel(ctx *middleware.Context) { - if !ctx.Repo.IsOwner { + if !ctx.Repo.IsOwner() { ctx.Error(403) return } @@ -560,7 +560,7 @@ func UpdateIssueLabel(ctx *middleware.Context) { } func UpdateIssueMilestone(ctx *middleware.Context) { - if !ctx.Repo.IsOwner { + if !ctx.Repo.IsOwner() { ctx.Error(403) return } @@ -606,7 +606,7 @@ func UpdateIssueMilestone(ctx *middleware.Context) { } func UpdateAssignee(ctx *middleware.Context) { - if !ctx.Repo.IsOwner { + if !ctx.Repo.IsOwner() { ctx.Error(403) return } @@ -752,7 +752,7 @@ func Comment(ctx *middleware.Context) { // Check if issue owner changes the status of issue. var newStatus string - if ctx.Repo.IsOwner || issue.PosterId == ctx.User.Id { + if ctx.Repo.IsOwner() || issue.PosterId == ctx.User.Id { newStatus = ctx.Query("change_status") } if len(newStatus) > 0 { diff --git a/routers/repo/release.go b/routers/repo/release.go index 591810cc5..52d78b196 100644 --- a/routers/repo/release.go +++ b/routers/repo/release.go @@ -41,7 +41,7 @@ func Releases(ctx *middleware.Context) { tags := make([]*models.Release, len(rawTags)) for i, rawTag := range rawTags { for j, rel := range rels { - if rel == nil || (rel.IsDraft && !ctx.Repo.IsOwner) { + if rel == nil || (rel.IsDraft && !ctx.Repo.IsOwner()) { continue } if rel.TagName == rawTag { @@ -140,7 +140,7 @@ func Releases(ctx *middleware.Context) { } func NewRelease(ctx *middleware.Context) { - if !ctx.Repo.IsOwner { + if !ctx.Repo.IsOwner() { ctx.Handle(403, "release.ReleasesNew", nil) return } @@ -153,7 +153,7 @@ func NewRelease(ctx *middleware.Context) { } func NewReleasePost(ctx *middleware.Context, form auth.NewReleaseForm) { - if !ctx.Repo.IsOwner { + if !ctx.Repo.IsOwner() { ctx.Handle(403, "release.ReleasesNew", nil) return } @@ -211,7 +211,7 @@ func NewReleasePost(ctx *middleware.Context, form auth.NewReleaseForm) { } func EditRelease(ctx *middleware.Context) { - if !ctx.Repo.IsOwner { + if !ctx.Repo.IsOwner() { ctx.Handle(403, "release.ReleasesEdit", nil) return } @@ -234,7 +234,7 @@ func EditRelease(ctx *middleware.Context) { } func EditReleasePost(ctx *middleware.Context, form auth.EditReleaseForm) { - if !ctx.Repo.IsOwner { + if !ctx.Repo.IsOwner() { ctx.Handle(403, "release.EditReleasePost", nil) return } diff --git a/routers/repo/repo.go b/routers/repo/repo.go index 48f7b09bc..005372003 100644 --- a/routers/repo/repo.go +++ b/routers/repo/repo.go @@ -343,7 +343,7 @@ func Action(ctx *middleware.Context) { case "unstar": err = models.StarRepo(ctx.User.Id, ctx.Repo.Repository.Id, false) case "desc": - if !ctx.Repo.IsOwner { + if !ctx.Repo.IsOwner() { ctx.Error(404) return } diff --git a/templates/repo/header.tmpl b/templates/repo/header.tmpl index a0b927be6..21f9cea88 100644 --- a/templates/repo/header.tmpl +++ b/templates/repo/header.tmpl @@ -49,7 +49,7 @@
  • - +
    • -->{{end}}{{if .IsRepositoryTrueOwner}} + -->{{end}}{{if .IsRepositoryAdmin}}
  • Settings
    • {{end}}