From 49b2f45f75960a48676c8dd2555d715da1942bd7 Mon Sep 17 00:00:00 2001 From: Vasek Sraier Date: Sun, 7 Apr 2019 22:49:34 +0000 Subject: [PATCH] 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 * PR #6483: helper methods moved to context/context.go, added missing return Signed-off-by: Vasek Sraier * PR #6483: added documentation to new exported helper functions in context/context.go Signed-off-by: Vasek Sraier --- modules/context/context.go | 38 +++++++++++- routers/api/v1/api.go | 121 ++++++++++++++++++++----------------- 2 files changed, 103 insertions(+), 56 deletions(-) diff --git a/modules/context/context.go b/modules/context/context.go index 770d42e2c..07e2b7b61 100644 --- a/modules/context/context.go +++ b/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"] diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 0b5c37a35..02c74e505 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -192,65 +192,102 @@ 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() { - ctx.Error(403) - return - } - } -} - -func reqRepoReader(unitType models.UnitType) macaron.Handler { - return func(ctx *context.Context) { - if !ctx.Repo.CanRead(unitType) { - ctx.Error(403) - return - } - } -} - -func reqAnyRepoReader() macaron.Handler { - return func(ctx *context.Context) { - if !ctx.Repo.HasAccess() { + if !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() { ctx.Error(403) return } } } +// 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) { - for _, unitType := range unitTypes { - if ctx.Repo.CanWrite(unitType) { - return - } + if !ctx.IsUserRepoWriter(unitTypes) && !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() { + ctx.Error(403) + return } - - ctx.Error(403) } } +// 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.IsUserRepoReaderSpecific(unitType) && !ctx.IsUserRepoAdmin() && !ctx.IsUserSiteAdmin() { + ctx.Error(403) + return + } + } +} + +// reqAnyRepoReader user should have any permission to read repository or permissions of site admin +func reqAnyRepoReader() macaron.Handler { + return func(ctx *context.Context) { + if !ctx.IsUserRepoReaderAny() && !ctx.IsUserSiteAdmin() { + ctx.Error(403) + return + } + } +} + +// 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, "", "reqOrgOwnership: unprepared context") + return + } + + isOwner, err := models.IsOrganizationOwner(orgID, ctx.User.ID) + if err != nil { + ctx.Error(500, "IsOrganizationOwner", err) + return + } else if !isOwner { + if ctx.Org.Organization != nil { + ctx.Error(403, "", "Must be an organization owner") + } else { + ctx.NotFound() + } + return + } + } +} + +// 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 @@ -275,32 +312,6 @@ func reqOrgMembership() macaron.Handler { } } -func reqOrgOwnership() macaron.Handler { - return func(ctx *context.APIContext) { - 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") - return - } - - isOwner, err := models.IsOrganizationOwner(orgID, ctx.User.ID) - if err != nil { - ctx.Error(500, "IsOrganizationOwner", err) - } else if !isOwner { - if ctx.Org.Organization != nil { - ctx.Error(403, "", "Must be an organization owner") - } else { - ctx.NotFound() - } - return - } - } -} - func orgAssignment(args ...bool) macaron.Handler { var ( assignOrg bool