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.

10 files changed, 68 insertions, 72 deletions

diff --git a/cmd/web.go b/cmd/web.go
index 3284acb9..8b3b03c4 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 28be3a30..a2661096 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 bd298819..5c863dc0 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 a049904f..73f97b2c 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 bf39d9ab..40e93389 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 591810cc..52d78b19 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 48f7b09b..00537200 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 a0b927be..21f9cea8 100644
--- a/templates/repo/header.tmpl
+++ b/templates/repo/header.tmpl
@@ -49,7 +49,7 @@
                 </a>
             </li>
             <li id="repo-header-fork">
-                <a id="repo-header-fork-btn" {{if or (not $.IsRepositoryTrueOwner) $.Owner.IsOrganization}}href="{{AppSubUrl}}/repo/fork?fork_id={{.Id}}"{{end}}>
+                <a id="repo-header-fork-btn" {{if or (not $.IsRepositoryAdmin) $.Owner.IsOrganization}}href="{{AppSubUrl}}/repo/fork?fork_id={{.Id}}"{{end}}>
                     <button class="btn btn-gray text-bold btn-radius">
                         <i class="octicon octicon-repo-forked"></i>{{$.i18n.Tr "repo.fork"}}
                         <span class="num">{{.NumForks}}</span>
diff --git a/templates/repo/sidebar.tmpl b/templates/repo/sidebar.tmpl
index 9d6abb47..16096038 100644
--- a/templates/repo/sidebar.tmpl
+++ b/templates/repo/sidebar.tmpl
@@ -20,7 +20,7 @@
         <!-- <li>
             <a class="radius" href="#"><i class="octicon octicon-organization"></i>contributors <span class="num right label label-gray label-radius">43</span></a>
         </li> -->
-        {{if .IsRepositoryTrueOwner}}
+        {{if .IsRepositoryAdmin}}
         <li class="border-bottom"></li>
         <li>
             <a class="radius" href="{{.RepoLink}}/settings"><i class="octicon octicon-tools"></i>{{.i18n.Tr "repo.settings"}}</a>
diff --git a/templates/repo/toolbar.tmpl b/templates/repo/toolbar.tmpl
index 6357b3c4..f2254d21 100644
--- a/templates/repo/toolbar.tmpl
+++ b/templates/repo/toolbar.tmpl
@@ -35,7 +35,7 @@
                             <li><a href="#">Pulse</a></li>
                             <li><a href="#">Network</a></li>
                         </ul>
-                    </li> -->{{end}}{{if .IsRepositoryTrueOwner}}
+                    </li> -->{{end}}{{if .IsRepositoryAdmin}}
                     <li class="{{if .IsRepoToolbarSetting}}active{{end}}"><a href="{{.RepoLink}}/settings">Settings</a>
                     </li>{{end}}
                 </ul>