From d1b5498cc0128b463a04d9e216234d247b0761cd Mon Sep 17 00:00:00 2001 From: Ethan Koenig Date: Thu, 26 Jan 2017 06:54:04 -0500 Subject: [PATCH] Use handlers for API authorization (#723) --- routers/api/v1/admin/org_repo.go | 52 ------------- routers/api/v1/api.go | 40 ++++++---- routers/api/v1/org/member.go | 13 +--- routers/api/v1/org/org.go | 5 -- routers/api/v1/org/team.go | 121 ++++++++++++++++++------------- 5 files changed, 100 insertions(+), 131 deletions(-) delete mode 100644 routers/api/v1/admin/org_repo.go diff --git a/routers/api/v1/admin/org_repo.go b/routers/api/v1/admin/org_repo.go deleted file mode 100644 index 8230c0acc..000000000 --- a/routers/api/v1/admin/org_repo.go +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright 2016 The Gogs Authors. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file. - -package admin - -import ( - "code.gitea.io/gitea/models" - "code.gitea.io/gitea/modules/context" -) - -// GetRepositoryByParams api for getting repository by orgnizition ID and repo name -func GetRepositoryByParams(ctx *context.APIContext) *models.Repository { - repo, err := models.GetRepositoryByName(ctx.Org.Team.OrgID, ctx.Params(":reponame")) - if err != nil { - if models.IsErrRepoNotExist(err) { - ctx.Status(404) - } else { - ctx.Error(500, "GetRepositoryByName", err) - } - return nil - } - return repo -} - -// AddTeamRepository api for adding a repository to a team -func AddTeamRepository(ctx *context.APIContext) { - repo := GetRepositoryByParams(ctx) - if ctx.Written() { - return - } - if err := ctx.Org.Team.AddRepository(repo); err != nil { - ctx.Error(500, "AddRepository", err) - return - } - - ctx.Status(204) -} - -// RemoveTeamRepository api for removing a repository from a team -func RemoveTeamRepository(ctx *context.APIContext) { - repo := GetRepositoryByParams(ctx) - if ctx.Written() { - return - } - if err := ctx.Org.Team.RemoveRepository(repo.ID); err != nil { - ctx.Error(500, "RemoveRepository", err) - return - } - - ctx.Status(204) -} diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 55622da1d..67912e15d 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -132,7 +132,11 @@ func reqOrgMembership() macaron.Handler { } if !models.IsOrganizationMember(orgID, ctx.User.ID) { - ctx.Error(403, "", "Must be an organization member") + if ctx.Org.Organization != nil { + ctx.Error(403, "", "Must be an organization member") + } else { + ctx.Status(404) + } return } } @@ -151,7 +155,11 @@ func reqOrgOwnership() macaron.Handler { } if !models.IsOrganizationOwner(orgID, ctx.User.ID) { - ctx.Error(403, "", "Must be an organization member") + if ctx.Org.Organization != nil { + ctx.Error(403, "", "Must be an organization owner") + } else { + ctx.Status(404) + } return } } @@ -394,18 +402,20 @@ func RegisterRoutes(m *macaron.Macaron) { m.Get("/user/orgs", reqToken(), org.ListMyOrgs) m.Get("/users/:username/orgs", org.ListUserOrgs) m.Group("/orgs/:orgname", func() { - m.Combo("").Get(org.Get).Patch(bind(api.EditOrgOption{}), org.Edit) + m.Combo("").Get(org.Get). + Patch(reqOrgOwnership(), bind(api.EditOrgOption{}), org.Edit) m.Group("/members", func() { m.Get("", org.ListMembers) - m.Combo("/:username").Get(org.IsMember).Delete(org.DeleteMember) + m.Combo("/:username").Get(org.IsMember). + Delete(reqOrgOwnership(), org.DeleteMember) }) m.Group("/public_members", func() { m.Get("", org.ListPublicMembers) m.Combo("/:username").Get(org.IsPublicMember). - Put(org.PublicizeMember). - Delete(org.ConcealMember) + Put(reqOrgMembership(), org.PublicizeMember). + Delete(reqOrgMembership(), org.ConcealMember) }) - m.Combo("/teams").Get(org.ListTeams). + m.Combo("/teams", reqOrgMembership()).Get(org.ListTeams). Post(bind(api.CreateTeamOption{}), org.CreateTeam) m.Group("/hooks", func() { m.Combo("").Get(org.ListHooks). @@ -417,19 +427,21 @@ func RegisterRoutes(m *macaron.Macaron) { }, orgAssignment(true)) m.Group("/teams/:teamid", func() { m.Combo("").Get(org.GetTeam). - Patch(bind(api.EditTeamOption{}), org.EditTeam). - Delete(org.DeleteTeam) + Patch(reqOrgOwnership(), bind(api.EditTeamOption{}), org.EditTeam). + Delete(reqOrgOwnership(), org.DeleteTeam) m.Group("/members", func() { m.Get("", org.GetTeamMembers) - m.Combo("/:username").Put(org.AddTeamMember). - Delete(org.RemoveTeamMember) + m.Combo("/:username"). + Put(reqOrgOwnership(), org.AddTeamMember). + Delete(reqOrgOwnership(), org.RemoveTeamMember) }) m.Group("/repos", func() { m.Get("", org.GetTeamRepos) - m.Combo("/:reponame").Put(admin.AddTeamRepository). - Delete(admin.RemoveTeamRepository) + m.Combo(":orgname/:reponame"). + Put(org.AddTeamRepository). + Delete(org.RemoveTeamRepository) }) - }, orgAssignment(false, true)) + }, reqOrgMembership(), orgAssignment(false, true)) m.Any("/*", func(ctx *context.Context) { ctx.Error(404) diff --git a/routers/api/v1/org/member.go b/routers/api/v1/org/member.go index 2420b9b54..b6b111bcc 100644 --- a/routers/api/v1/org/member.go +++ b/routers/api/v1/org/member.go @@ -97,9 +97,6 @@ func PublicizeMember(ctx *context.APIContext) { if userToPublicize.ID != ctx.User.ID { ctx.Error(403, "", "Cannot publicize another member") return - } else if !ctx.Org.Organization.IsOrgMember(userToPublicize.ID) { - ctx.Error(403, "", "Must be a member of the organization") - return } err := models.ChangeOrgUserStatus(ctx.Org.Organization.ID, userToPublicize.ID, true) if err != nil { @@ -115,9 +112,6 @@ func ConcealMember(ctx *context.APIContext) { if userToConceal.ID != ctx.User.ID { ctx.Error(403, "", "Cannot conceal another member") return - } else if !ctx.Org.Organization.IsOrgMember(userToConceal.ID) { - ctx.Error(403, "", "Must be a member of the organization") - return } err := models.ChangeOrgUserStatus(ctx.Org.Organization.ID, userToConceal.ID, false) if err != nil { @@ -130,11 +124,8 @@ func ConcealMember(ctx *context.APIContext) { // DeleteMember remove a member from an organization func DeleteMember(ctx *context.APIContext) { org := ctx.Org.Organization - if !org.IsOwnedBy(ctx.User.ID) { - ctx.Error(403, "", "You must be an owner of the organization.") - return - } - if err := org.RemoveMember(user.GetUserByParams(ctx).ID); err != nil { + memberID := user.GetUserByParams(ctx).ID + if err := org.RemoveMember(memberID); err != nil { ctx.Error(500, "RemoveMember", err) } ctx.Status(204) diff --git a/routers/api/v1/org/org.go b/routers/api/v1/org/org.go index 328b97a22..e4a36b95c 100644 --- a/routers/api/v1/org/org.go +++ b/routers/api/v1/org/org.go @@ -52,11 +52,6 @@ func Get(ctx *context.APIContext) { // see https://github.com/gogits/go-gogs-client/wiki/Organizations#edit-an-organization func Edit(ctx *context.APIContext, form api.EditOrgOption) { org := ctx.Org.Organization - if !org.IsOwnedBy(ctx.User.ID) { - ctx.Status(403) - return - } - org.FullName = form.FullName org.Description = form.Description org.Website = form.Website diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go index f87518e25..f9d93399a 100644 --- a/routers/api/v1/org/team.go +++ b/routers/api/v1/org/team.go @@ -16,10 +16,6 @@ import ( // ListTeams list all the teams of an organization func ListTeams(ctx *context.APIContext) { org := ctx.Org.Organization - if !org.IsOrgMember(ctx.User.ID) { - ctx.Error(403, "", "Must be a member of the organization") - return - } if err := org.GetTeams(); err != nil { ctx.Error(500, "GetTeams", err) return @@ -34,40 +30,11 @@ func ListTeams(ctx *context.APIContext) { // GetTeam api for get a team func GetTeam(ctx *context.APIContext) { - if !models.IsOrganizationMember(ctx.Org.Team.OrgID, ctx.User.ID) { - ctx.Status(404) - return - } ctx.JSON(200, convert.ToTeam(ctx.Org.Team)) } -// GetTeamRepos api for get a team's repos -func GetTeamRepos(ctx *context.APIContext) { - team := ctx.Org.Team - if !models.IsOrganizationMember(team.OrgID, ctx.User.ID) { - ctx.Status(404) - return - } - if err := team.GetRepositories(); err != nil { - ctx.Error(500, "GetTeamRepos", err) - } - repos := make([]*api.Repository, len(team.Repos)) - for i, repo := range team.Repos { - access, err := models.AccessLevel(ctx.User, repo) - if err != nil { - ctx.Error(500, "GetTeamRepos", err) - return - } - repos[i] = repo.APIFormat(access) - } - ctx.JSON(200, repos) -} - // CreateTeam api for create a team func CreateTeam(ctx *context.APIContext, form api.CreateTeamOption) { - if !ctx.Org.Organization.IsOrgMember(ctx.User.ID) { - ctx.Error(403, "", "Must be an organization member") - } team := &models.Team{ OrgID: ctx.Org.Organization.ID, Name: form.Name, @@ -88,10 +55,6 @@ func CreateTeam(ctx *context.APIContext, form api.CreateTeamOption) { // EditTeam api for edit a team func EditTeam(ctx *context.APIContext, form api.EditTeamOption) { - if !ctx.User.IsUserOrgOwner(ctx.Org.Team.OrgID) { - ctx.Error(403, "", "Must be an organization owner") - return - } team := &models.Team{ ID: ctx.Org.Team.ID, OrgID: ctx.Org.Team.OrgID, @@ -108,10 +71,6 @@ func EditTeam(ctx *context.APIContext, form api.EditTeamOption) { // DeleteTeam api for delete a team func DeleteTeam(ctx *context.APIContext) { - if !ctx.User.IsUserOrgOwner(ctx.Org.Team.OrgID) { - ctx.Error(403, "", "Must be an organization owner") - return - } if err := models.DeleteTeam(ctx.Org.Team); err != nil { ctx.Error(500, "DeleteTeam", err) return @@ -139,10 +98,6 @@ func GetTeamMembers(ctx *context.APIContext) { // AddTeamMember api for add a member to a team func AddTeamMember(ctx *context.APIContext) { - if !ctx.User.IsUserOrgOwner(ctx.Org.Team.OrgID) { - ctx.Error(403, "", "Must be an organization owner") - return - } u := user.GetUserByParams(ctx) if ctx.Written() { return @@ -156,10 +111,6 @@ func AddTeamMember(ctx *context.APIContext) { // RemoveTeamMember api for remove one member from a team func RemoveTeamMember(ctx *context.APIContext) { - if !ctx.User.IsUserOrgOwner(ctx.Org.Team.OrgID) { - ctx.Error(403, "", "Must be an organization owner") - return - } u := user.GetUserByParams(ctx) if ctx.Written() { return @@ -171,3 +122,75 @@ func RemoveTeamMember(ctx *context.APIContext) { } ctx.Status(204) } + +// GetTeamRepos api for get a team's repos +func GetTeamRepos(ctx *context.APIContext) { + team := ctx.Org.Team + if err := team.GetRepositories(); err != nil { + ctx.Error(500, "GetTeamRepos", err) + } + repos := make([]*api.Repository, len(team.Repos)) + for i, repo := range team.Repos { + access, err := models.AccessLevel(ctx.User, repo) + if err != nil { + ctx.Error(500, "GetTeamRepos", err) + return + } + repos[i] = repo.APIFormat(access) + } + ctx.JSON(200, repos) +} + +// getRepositoryByParams get repository by a team's organization ID and repo name +func getRepositoryByParams(ctx *context.APIContext) *models.Repository { + repo, err := models.GetRepositoryByName(ctx.Org.Team.OrgID, ctx.Params(":reponame")) + if err != nil { + if models.IsErrRepoNotExist(err) { + ctx.Status(404) + } else { + ctx.Error(500, "GetRepositoryByName", err) + } + return nil + } + return repo +} + +// AddTeamRepository api for adding a repository to a team +func AddTeamRepository(ctx *context.APIContext) { + repo := getRepositoryByParams(ctx) + if ctx.Written() { + return + } + if access, err := models.AccessLevel(ctx.User, repo); err != nil { + ctx.Error(500, "AccessLevel", err) + return + } else if access < models.AccessModeAdmin { + ctx.Error(403, "", "Must have admin-level access to the repository") + return + } + if err := ctx.Org.Team.AddRepository(repo); err != nil { + ctx.Error(500, "AddRepository", err) + return + } + ctx.Status(204) +} + +// RemoveTeamRepository api for removing a repository from a team +func RemoveTeamRepository(ctx *context.APIContext) { + repo := getRepositoryByParams(ctx) + if ctx.Written() { + return + } + if access, err := models.AccessLevel(ctx.User, repo); err != nil { + ctx.Error(500, "AccessLevel", err) + return + } else if access < models.AccessModeAdmin { + ctx.Error(403, "", "Must have admin-level access to the repository") + return + } + if err := ctx.Org.Team.RemoveRepository(repo.ID); err != nil { + ctx.Error(500, "RemoveRepository", err) + return + } + ctx.Status(204) +}