improve permission check

routers/api/v1/api.go

@@ -189,7 +189,8 @@ func reqBasicAuth() macaron.Handler {
 	}
 }
 
-func reqAdmin() macaron.Handler {
+// reqSiteAdmin user should be the site admin
+func reqSiteAdmin() macaron.Handler {
 	return func(ctx *context.Context) {
 		if !ctx.IsSigned || !ctx.User.IsAdmin {
 			ctx.Error(403)
@@ -198,7 +199,18 @@ func reqAdmin() macaron.Handler {
 	}
 }
 
+// reqOwner user should be the owner of the repo.
 func reqOwner() macaron.Handler {
+	return func(ctx *context.Context) {
+		if !ctx.Repo.IsOwner() {
+			ctx.Error(403)
+			return
+		}
+	}
+}
+
+// reqAdmin user should be an owner or a collaborator with admin write of a repository
+func reqAdmin() macaron.Handler {
 	return func(ctx *context.Context) {
 		if !ctx.Repo.IsAdmin() {
 			ctx.Error(403)
@@ -216,6 +228,15 @@ func reqRepoReader(unitType models.UnitType) macaron.Handler {
 	}
 }
 
+func reqAnyRepoReader() macaron.Handler {
+	return func(ctx *context.Context) {
+		if !ctx.Repo.HasAccess() {
+			ctx.Error(403)
+			return
+		}
+	}
+}
+
 func reqRepoWriter(unitTypes ...models.UnitType) macaron.Handler {
 	return func(ctx *context.Context) {
 		for _, unitType := range unitTypes {
@@ -457,7 +478,8 @@ func RegisterRoutes(m *macaron.Macaron) {
 			m.Post("/migrate", reqToken(), bind(auth.MigrateRepoForm{}), repo.Migrate)
 
 			m.Group("/:username/:reponame", func() {
-				m.Combo("").Get(repo.Get).Delete(reqToken(), repo.Delete)
+				m.Combo("").Get(reqAnyRepoReader(), repo.Get).
+					Delete(reqToken(), reqOwner(), repo.Delete)
 				m.Group("/hooks", func() {
 					m.Combo("").Get(repo.ListHooks).
 						Post(bind(api.CreateHookOption{}), repo.CreateHook)
@@ -467,21 +489,21 @@ func RegisterRoutes(m *macaron.Macaron) {
 							Delete(repo.DeleteHook)
 						m.Post("/tests", context.RepoRef(), repo.TestHook)
 					})
-				}, reqToken(), reqRepoWriter(models.UnitTypeCode))
+				}, reqToken(), reqAdmin())
 				m.Group("/collaborators", func() {
 					m.Get("", repo.ListCollaborators)
 					m.Combo("/:collaborator").Get(repo.IsCollaborator).
 						Put(bind(api.AddCollaboratorOption{}), repo.AddCollaborator).
 						Delete(repo.DeleteCollaborator)
-				}, reqToken())
-				m.Get("/raw/*", context.RepoRefByType(context.RepoRefAny), repo.GetRawFile)
-				m.Get("/archive/*", repo.GetArchive)
+				}, reqToken(), reqAdmin())
+				m.Get("/raw/*", context.RepoRefByType(context.RepoRefAny), reqRepoReader(models.UnitTypeCode), repo.GetRawFile)
+				m.Get("/archive/*", reqRepoReader(models.UnitTypeCode), repo.GetArchive)
 				m.Combo("/forks").Get(repo.ListForks).
-					Post(reqToken(), bind(api.CreateForkOption{}), repo.CreateFork)
+					Post(reqToken(), reqRepoReader(models.UnitTypeCode), bind(api.CreateForkOption{}), repo.CreateFork)
 				m.Group("/branches", func() {
 					m.Get("", repo.ListBranches)
 					m.Get("/*", context.RepoRefByType(context.RepoRefBranch), repo.GetBranch)
-				})
+				}, reqRepoReader(models.UnitTypeCode))
 				m.Group("/keys", func() {
 					m.Combo("").Get(repo.ListDeployKeys).
 						Post(bind(api.CreateKeyOption{}), repo.CreateDeployKey)
@@ -491,7 +513,6 @@ func RegisterRoutes(m *macaron.Macaron) {
 				m.Group("/times", func() {
 					m.Combo("").Get(repo.ListTrackedTimesByRepository)
 					m.Combo("/:timetrackingusername").Get(repo.ListTrackedTimesByUser)
-
 				}, mustEnableIssues)
 				m.Group("/issues", func() {
 					m.Combo("").Get(repo.ListIssues).
@@ -567,7 +588,7 @@ func RegisterRoutes(m *macaron.Macaron) {
 					})
 				}, reqRepoReader(models.UnitTypeReleases))
 				m.Post("/mirror-sync", reqToken(), reqRepoWriter(models.UnitTypeCode), repo.MirrorSync)
-				m.Get("/editorconfig/:filename", context.RepoRef(), repo.GetEditorconfig)
+				m.Get("/editorconfig/:filename", context.RepoRef(), reqRepoReader(models.UnitTypeCode), repo.GetEditorconfig)
 				m.Group("/pulls", func() {
 					m.Combo("").Get(bind(api.ListPullRequestsOptions{}), repo.ListPullRequests).
 						Post(reqToken(), bind(api.CreatePullRequestOption{}), repo.CreatePullRequest)
@@ -582,15 +603,15 @@ func RegisterRoutes(m *macaron.Macaron) {
 				m.Group("/statuses", func() {
 					m.Combo("/:sha").Get(repo.GetCommitStatuses).
 						Post(reqToken(), bind(api.CreateStatusOption{}), repo.NewCommitStatus)
-				})
+				}, reqRepoReader(models.UnitTypeCode))
 				m.Group("/commits/:ref", func() {
 					m.Get("/status", repo.GetCombinedCommitStatusByRef)
 					m.Get("/statuses", repo.GetCommitStatusesByRef)
-				})
+				}, reqRepoReader(models.UnitTypeCode))
 				m.Group("/git", func() {
 					m.Get("/refs", repo.GetGitAllRefs)
 					m.Get("/refs/*", repo.GetGitRefs)
-				})
+				}, reqRepoReader(models.UnitTypeCode))
 			}, repoAssignment())
 		})
 
@@ -659,7 +680,7 @@ func RegisterRoutes(m *macaron.Macaron) {
 					m.Post("/repos", bind(api.CreateRepoOption{}), admin.CreateRepo)
 				})
 			})
-		}, reqToken(), reqAdmin())
+		}, reqToken(), reqSiteAdmin())
 
 		m.Group("/topics", func() {
 			m.Get("/search", repo.TopicSearch)

routers/api/v1/repo/collaborators.go

@@ -35,10 +35,6 @@ func ListCollaborators(ctx *context.APIContext) {
 	// responses:
 	//   "200":
 	//     "$ref": "#/responses/UserList"
-	if !ctx.Repo.CanWrite(models.UnitTypeCode) {
-		ctx.Error(403, "", "User does not have push access")
-		return
-	}
 	collaborators, err := ctx.Repo.Repository.GetCollaborators()
 	if err != nil {
 		ctx.Error(500, "ListCollaborators", err)
@@ -79,10 +75,6 @@ func IsCollaborator(ctx *context.APIContext) {
 	//     "$ref": "#/responses/empty"
 	//   "404":
 	//     "$ref": "#/responses/empty"
-	if !ctx.Repo.CanWrite(models.UnitTypeCode) {
-		ctx.Error(403, "", "User does not have push access")
-		return
-	}
 	user, err := models.GetUserByName(ctx.Params(":collaborator"))
 	if err != nil {
 		if models.IsErrUserNotExist(err) {
@@ -134,10 +126,6 @@ func AddCollaborator(ctx *context.APIContext, form api.AddCollaboratorOption) {
 	// responses:
 	//   "204":
 	//     "$ref": "#/responses/empty"
-	if !ctx.Repo.CanWrite(models.UnitTypeCode) {
-		ctx.Error(403, "", "User does not have push access")
-		return
-	}
 	collaborator, err := models.GetUserByName(ctx.Params(":collaborator"))
 	if err != nil {
 		if models.IsErrUserNotExist(err) {
@@ -194,11 +182,6 @@ func DeleteCollaborator(ctx *context.APIContext) {
 	// responses:
 	//   "204":
 	//     "$ref": "#/responses/empty"
-	if !ctx.Repo.CanWrite(models.UnitTypeCode) {
-		ctx.Error(403, "", "User does not have push access")
-		return
-	}
-
 	collaborator, err := models.GetUserByName(ctx.Params(":collaborator"))
 	if err != nil {
 		if models.IsErrUserNotExist(err) {

routers/api/v1/repo/file.go

@@ -39,11 +39,6 @@ func GetRawFile(ctx *context.APIContext) {
 	// responses:
 	//   200:
 	//     description: success
-	if !ctx.Repo.CanAccess(models.UnitTypeCode) {
-		ctx.Status(404)
-		return
-	}
-
 	if ctx.Repo.Repository.IsBare {
 		ctx.Status(404)
 		return

routers/api/v1/repo/label.go

@@ -124,11 +124,6 @@ func CreateLabel(ctx *context.APIContext, form api.CreateLabelOption) {
 	// responses:
 	//   "201":
 	//     "$ref": "#/responses/Label"
-	if !ctx.Repo.CanWrite(models.UnitTypeIssues) && !ctx.Repo.CanWrite(models.UnitTypePullRequests) {
-		ctx.Status(403)
-		return
-	}
-
 	label := &models.Label{
 		Name:   form.Name,
 		Color:  form.Color,
@@ -174,11 +169,6 @@ func EditLabel(ctx *context.APIContext, form api.EditLabelOption) {
 	// responses:
 	//   "200":
 	//     "$ref": "#/responses/Label"
-	if !ctx.Repo.CanWrite(models.UnitTypeIssues) && !ctx.Repo.CanWrite(models.UnitTypePullRequests) {
-		ctx.Status(403)
-		return
-	}
-
 	label, err := models.GetLabelInRepoByID(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id"))
 	if err != nil {
 		if models.IsErrLabelNotExist(err) {
@@ -227,11 +217,6 @@ func DeleteLabel(ctx *context.APIContext) {
 	// responses:
 	//   "204":
 	//     "$ref": "#/responses/empty"
-	if !ctx.Repo.CanWrite(models.UnitTypeIssues) && !ctx.Repo.CanWrite(models.UnitTypePullRequests) {
-		ctx.Status(403)
-		return
-	}
-
 	if err := models.DeleteLabel(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id")); err != nil {
 		ctx.Error(500, "DeleteLabel", err)
 		return

routers/api/v1/repo/release.go

@@ -122,10 +122,6 @@ func CreateRelease(ctx *context.APIContext, form api.CreateReleaseOption) {
 	// responses:
 	//   "201":
 	//     "$ref": "#/responses/Release"
-	if ctx.Repo.AccessMode < models.AccessModeWrite {
-		ctx.Status(403)
-		return
-	}
 	rel, err := models.GetRelease(ctx.Repo.Repository.ID, form.TagName)
 	if err != nil {
 		if !models.IsErrReleaseNotExist(err) {
@@ -209,10 +205,6 @@ func EditRelease(ctx *context.APIContext, form api.EditReleaseOption) {
 	// responses:
 	//   "200":
 	//     "$ref": "#/responses/Release"
-	if ctx.Repo.AccessMode < models.AccessModeWrite {
-		ctx.Status(403)
-		return
-	}
 	id := ctx.ParamsInt64(":id")
 	rel, err := models.GetReleaseByID(id)
 	if err != nil && !models.IsErrReleaseNotExist(err) {
@@ -285,10 +277,6 @@ func DeleteRelease(ctx *context.APIContext) {
 	// responses:
 	//   "204":
 	//     "$ref": "#/responses/empty"
-	if ctx.Repo.AccessMode < models.AccessModeWrite {
-		ctx.Status(403)
-		return
-	}
 	id := ctx.ParamsInt64(":id")
 	rel, err := models.GetReleaseByID(id)
 	if err != nil && !models.IsErrReleaseNotExist(err) {

routers/api/v1/repo/repo.go

@@ -470,15 +470,15 @@ func GetByID(ctx *context.APIContext) {
 		return
 	}
 
-	access, err := models.AccessLevel(ctx.User.ID, repo)
+	perm, err := models.GetUserRepoPermission(repo, ctx.User)
 	if err != nil {
 		ctx.Error(500, "AccessLevel", err)
 		return
-	} else if access < models.AccessModeRead {
+	} else if !perm.HasAccess() {
 		ctx.Status(404)
 		return
 	}
-	ctx.JSON(200, repo.APIFormat(access))
+	ctx.JSON(200, repo.APIFormat(perm.AccessMode))
 }
 
 // Delete one repository
@@ -504,10 +504,6 @@ func Delete(ctx *context.APIContext) {
 	//     "$ref": "#/responses/empty"
 	//   "403":
 	//     "$ref": "#/responses/forbidden"
-	if !ctx.Repo.IsAdmin() {
-		ctx.Error(403, "", "Must have admin rights")
-		return
-	}
 	owner := ctx.Repo.Owner
 	repo := ctx.Repo.Repository
 

routers/repo/activity.go

@@ -45,9 +45,9 @@ func Activity(ctx *context.Context) {
 
 	var err error
 	if ctx.Data["Activity"], err = models.GetActivityStats(ctx.Repo.Repository.ID, timeFrom,
-		ctx.Repo.Repository.UnitEnabled(models.UnitTypeReleases),
-		ctx.Repo.Repository.UnitEnabled(models.UnitTypeIssues),
-		ctx.Repo.Repository.UnitEnabled(models.UnitTypePullRequests)); err != nil {
+		ctx.Repo.CanAccess(models.UnitTypeReleases),
+		ctx.Repo.CanAccess(models.UnitTypeIssues),
+		ctx.Repo.CanAccess(models.UnitTypePullRequests)); err != nil {
 		ctx.ServerError("GetActivityStats", err)
 		return
 	}

routers/repo/issue.go

@@ -844,8 +844,8 @@ func GetActionIssue(ctx *context.Context) *models.Issue {
 }
 
 func checkIssueRights(ctx *context.Context, issue *models.Issue) {
-	if issue.IsPull && !ctx.Repo.Repository.UnitEnabled(models.UnitTypePullRequests) ||
-		!issue.IsPull && !ctx.Repo.Repository.UnitEnabled(models.UnitTypeIssues) {
+	if issue.IsPull && !ctx.Repo.CanAccess(models.UnitTypePullRequests) ||
+		!issue.IsPull && !ctx.Repo.CanAccess(models.UnitTypeIssues) {
 		ctx.NotFound("IssueOrPullRequestUnitNotAllowed", nil)
 	}
 }
@@ -870,8 +870,8 @@ func getActionIssues(ctx *context.Context) []*models.Issue {
 		return nil
 	}
 	// Check access rights for all issues
-	issueUnitEnabled := ctx.Repo.Repository.UnitEnabled(models.UnitTypeIssues)
-	prUnitEnabled := ctx.Repo.Repository.UnitEnabled(models.UnitTypePullRequests)
+	issueUnitEnabled := ctx.Repo.CanAccess(models.UnitTypeIssues)
+	prUnitEnabled := ctx.Repo.CanAccess(models.UnitTypePullRequests)
 	for _, issue := range issues {
 		if issue.IsPull && !prUnitEnabled || !issue.IsPull && !issueUnitEnabled {
 			ctx.NotFound("IssueOrPullRequestUnitNotAllowed", nil)

routers/repo/wiki.go

@@ -31,8 +31,8 @@ const (
 
 // MustEnableWiki check if wiki is enabled, if external then redirect
 func MustEnableWiki(ctx *context.Context) {
-	if !ctx.Repo.Repository.UnitEnabled(models.UnitTypeWiki) &&
-		!ctx.Repo.Repository.UnitEnabled(models.UnitTypeExternalWiki) {
+	if !ctx.Repo.CanAccess(models.UnitTypeWiki) &&
+		!ctx.Repo.CanAccess(models.UnitTypeExternalWiki) {
 		ctx.NotFound("MustEnableWiki", nil)
 		return
 	}