Cleaned permission checks for API -> site admin can now do anything (#6483)

* cleaned permission checks for API -> site admin can now do anything

Signed-off-by: Vasek Sraier <git@vakabus.cz>

* PR #6483: helper methods moved to context/context.go, added missing return

Signed-off-by: Vasek Sraier <git@vakabus.cz>

* PR #6483: added documentation to new exported helper functions in context/context.go

Signed-off-by: Vasek Sraier <git@vakabus.cz>

modules/context/context.go

@@ -25,7 +25,7 @@ import (
 	"github.com/go-macaron/csrf"
 	"github.com/go-macaron/i18n"
 	"github.com/go-macaron/session"
-	macaron "gopkg.in/macaron.v1"
+	"gopkg.in/macaron.v1"
 )
 
 // Context represents context of a request.
@@ -46,6 +46,42 @@ type Context struct {
 	Org  *Organization
 }
 
+// IsUserSiteAdmin returns true if current user is a site admin
+func (ctx *Context) IsUserSiteAdmin() bool {
+	return ctx.IsSigned && ctx.User.IsAdmin
+}
+
+// IsUserRepoOwner returns true if current user owns current repo
+func (ctx *Context) IsUserRepoOwner() bool {
+	return ctx.Repo.IsOwner()
+}
+
+// IsUserRepoAdmin returns true if current user is admin in current repo
+func (ctx *Context) IsUserRepoAdmin() bool {
+	return ctx.Repo.IsAdmin()
+}
+
+// IsUserRepoWriter returns true if current user has write privilege in current repo
+func (ctx *Context) IsUserRepoWriter(unitTypes []models.UnitType) bool {
+	for _, unitType := range unitTypes {
+		if ctx.Repo.CanWrite(unitType) {
+			return true
+		}
+	}
+
+	return false
+}
+
+// IsUserRepoReaderSpecific returns true if current user can read current repo's specific part
+func (ctx *Context) IsUserRepoReaderSpecific(unitType models.UnitType) bool {
+	return ctx.Repo.CanRead(unitType)
+}
+
+// IsUserRepoReaderAny returns true if current user can read any part of current repo
+func (ctx *Context) IsUserRepoReaderAny() bool {
+	return ctx.Repo.HasAccess()
+}
+
 // HasAPIError returns true if error occurs in form validation.
 func (ctx *Context) HasAPIError() bool {
 	hasErr, ok := ctx.Data["HasError"]

routers/api/v1/api.go

@@ -192,81 +192,87 @@ func reqBasicAuth() macaron.Handler {
 // reqSiteAdmin user should be the site admin
 func reqSiteAdmin() macaron.Handler {
 	return func(ctx *context.Context) {
-		if !ctx.IsSigned || !ctx.User.IsAdmin {
+		if !ctx.IsUserSiteAdmin() {
 			ctx.Error(403)
 			return
 		}
 	}
 }
 
-// reqOwner user should be the owner of the repo.
+// reqOwner user should be the owner of the repo or site admin.
 func reqOwner() macaron.Handler {
 	return func(ctx *context.Context) {
-		if !ctx.Repo.IsOwner() {
+		if !ctx.IsUserRepoOwner() && !ctx.IsUserSiteAdmin() {
 			ctx.Error(403)
 			return
 		}
 	}
 }
 
-// reqAdmin user should be an owner or a collaborator with admin write of a repository
+// reqAdmin user should be an owner or a collaborator with admin write of a repository, or site admin
 func reqAdmin() macaron.Handler {
 	return func(ctx *context.Context) {
-		if !ctx.Repo.IsAdmin() {
+		if !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() {
 			ctx.Error(403)
 			return
 		}
 	}
 }
 
-func reqRepoReader(unitType models.UnitType) macaron.Handler {
+// reqRepoWriter user should have a permission to write to a repo, or be a site admin
+func reqRepoWriter(unitTypes ...models.UnitType) macaron.Handler {
 	return func(ctx *context.Context) {
-		if !ctx.Repo.CanRead(unitType) {
+		if !ctx.IsUserRepoWriter(unitTypes) && !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() {
 			ctx.Error(403)
 			return
 		}
 	}
 }
 
-func reqAnyRepoReader() macaron.Handler {
+// reqRepoReader user should have specific read permission or be a repo admin or a site admin
+func reqRepoReader(unitType models.UnitType) macaron.Handler {
 	return func(ctx *context.Context) {
-		if !ctx.Repo.HasAccess() {
+		if !ctx.IsUserRepoReaderSpecific(unitType) && !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() {
 			ctx.Error(403)
 			return
 		}
 	}
 }
 
-func reqRepoWriter(unitTypes ...models.UnitType) macaron.Handler {
+// reqAnyRepoReader user should have any permission to read repository or permissions of site admin
+func reqAnyRepoReader() macaron.Handler {
 	return func(ctx *context.Context) {
-		for _, unitType := range unitTypes {
+		if !ctx.IsUserRepoReaderAny() && !ctx.IsUserSiteAdmin() {
-			if ctx.Repo.CanWrite(unitType) {
+			ctx.Error(403)
 			return
 		}
 	}
-
-		ctx.Error(403)
-	}
 }
 
-func reqOrgMembership() macaron.Handler {
+// reqOrgOwnership user should be an organization owner, or a site admin
+func reqOrgOwnership() macaron.Handler {
 	return func(ctx *context.APIContext) {
+		if ctx.Context.IsUserSiteAdmin() {
+			return
+		}
+
 		var orgID int64
 		if ctx.Org.Organization != nil {
 			orgID = ctx.Org.Organization.ID
 		} else if ctx.Org.Team != nil {
 			orgID = ctx.Org.Team.OrgID
 		} else {
-			ctx.Error(500, "", "reqOrgMembership: unprepared context")
+			ctx.Error(500, "", "reqOrgOwnership: unprepared context")
 			return
 		}
 
-		if isMember, err := models.IsOrganizationMember(orgID, ctx.User.ID); err != nil {
+		isOwner, err := models.IsOrganizationOwner(orgID, ctx.User.ID)
-			ctx.Error(500, "IsOrganizationMember", err)
+		if err != nil {
+			ctx.Error(500, "IsOrganizationOwner", err)
 			return
-		} else if !isMember {
+		} else if !isOwner {
 			if ctx.Org.Organization != nil {
-				ctx.Error(403, "", "Must be an organization member")
+				ctx.Error(403, "", "Must be an organization owner")
 			} else {
 				ctx.NotFound()
 			}
@@ -275,24 +281,29 @@ func reqOrgMembership() macaron.Handler {
 	}
 }
 
-func reqOrgOwnership() macaron.Handler {
+// reqOrgMembership user should be an organization member, or a site admin
+func reqOrgMembership() macaron.Handler {
 	return func(ctx *context.APIContext) {
+		if ctx.Context.IsUserSiteAdmin() {
+			return
+		}
+
 		var orgID int64
 		if ctx.Org.Organization != nil {
 			orgID = ctx.Org.Organization.ID
 		} else if ctx.Org.Team != nil {
 			orgID = ctx.Org.Team.OrgID
 		} else {
-			ctx.Error(500, "", "reqOrgOwnership: unprepared context")
+			ctx.Error(500, "", "reqOrgMembership: unprepared context")
 			return
 		}
 
-		isOwner, err := models.IsOrganizationOwner(orgID, ctx.User.ID)
+		if isMember, err := models.IsOrganizationMember(orgID, ctx.User.ID); err != nil {
-		if err != nil {
+			ctx.Error(500, "IsOrganizationMember", err)
-			ctx.Error(500, "IsOrganizationOwner", err)
+			return
-		} else if !isOwner {
+		} else if !isMember {
 			if ctx.Org.Organization != nil {
-				ctx.Error(403, "", "Must be an organization owner")
+				ctx.Error(403, "", "Must be an organization member")
 			} else {
 				ctx.NotFound()
 			}