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 3284acb9df..8b3b03c45e 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 28be3a3025..a266109691 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 bd298819d7..5c863dc01f 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 a049904f95..73f97b2cae 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 bf39d9aba6..40e9338970 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 591810cc5f..52d78b1967 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 48f7b09bc0..0053720030 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 a0b927be60..21f9cea882 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 9d6abb47bc..1609603821 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 6357b3c4c0..f2254d21dd 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>