From 74bbec3bf9f5e306248bf80808f93e116c232306 Mon Sep 17 00:00:00 2001 From: Ethan Koenig Date: Thu, 19 Jan 2017 22:16:10 -0700 Subject: [PATCH] Fix permission bugs in team API (#647) --- models/user.go | 2 +- routers/api/v1/admin/org_team.go | 88 -------------------- routers/api/v1/api.go | 34 ++++---- routers/api/v1/org/team.go | 134 ++++++++++++++++++++++++++++--- 4 files changed, 137 insertions(+), 121 deletions(-) delete mode 100644 routers/api/v1/admin/org_team.go diff --git a/models/user.go b/models/user.go index 8b20d7e0b..86573dbda 100644 --- a/models/user.go +++ b/models/user.go @@ -470,7 +470,7 @@ func (u *User) IsUserOrgOwner(orgID int64) bool { return IsOrganizationOwner(orgID, u.ID) } -// IsPublicMember returns true if user public his/her membership in give organization. +// IsPublicMember returns true if user public his/her membership in given organization. func (u *User) IsPublicMember(orgID int64) bool { return IsPublicMembership(orgID, u.ID) } diff --git a/routers/api/v1/admin/org_team.go b/routers/api/v1/admin/org_team.go deleted file mode 100644 index 716bd675f..000000000 --- a/routers/api/v1/admin/org_team.go +++ /dev/null @@ -1,88 +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 ( - api "code.gitea.io/sdk/gitea" - - "code.gitea.io/gitea/models" - "code.gitea.io/gitea/modules/context" - "code.gitea.io/gitea/routers/api/v1/convert" - "code.gitea.io/gitea/routers/api/v1/user" -) - -// CreateTeam api for create a team -func CreateTeam(ctx *context.APIContext, form api.CreateTeamOption) { - team := &models.Team{ - OrgID: ctx.Org.Organization.ID, - Name: form.Name, - Description: form.Description, - Authorize: models.ParseAccessMode(form.Permission), - } - if err := models.NewTeam(team); err != nil { - if models.IsErrTeamAlreadyExist(err) { - ctx.Error(422, "", err) - } else { - ctx.Error(500, "NewTeam", err) - } - return - } - - ctx.JSON(201, convert.ToTeam(team)) -} - -// EditTeam api for edit a team -func EditTeam(ctx *context.APIContext, form api.EditTeamOption) { - team := &models.Team{ - ID: ctx.Org.Team.ID, - OrgID: ctx.Org.Team.OrgID, - Name: form.Name, - Description: form.Description, - Authorize: models.ParseAccessMode(form.Permission), - } - if err := models.UpdateTeam(team, true); err != nil { - ctx.Error(500, "EditTeam", err) - return - } - ctx.JSON(200, convert.ToTeam(team)) -} - -// DeleteTeam api for delete a team -func DeleteTeam(ctx *context.APIContext) { - if err := models.DeleteTeam(ctx.Org.Team); err != nil { - ctx.Error(500, "DeleteTeam", err) - return - } - ctx.Status(204) -} - -// AddTeamMember api for add a member to a team -func AddTeamMember(ctx *context.APIContext) { - u := user.GetUserByParams(ctx) - if ctx.Written() { - return - } - if err := ctx.Org.Team.AddMember(u.ID); err != nil { - ctx.Error(500, "AddMember", err) - return - } - - ctx.Status(204) -} - -// RemoveTeamMember api for remove one member from a team -func RemoveTeamMember(ctx *context.APIContext) { - u := user.GetUserByParams(ctx) - if ctx.Written() { - return - } - - if err := ctx.Org.Team.RemoveMember(u.ID); err != nil { - ctx.Error(500, "RemoveMember", err) - return - } - - ctx.Status(204) -} diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 29f268d6b..b83cb36a7 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -405,7 +405,8 @@ func RegisterRoutes(m *macaron.Macaron) { Put(org.PublicizeMember). Delete(org.ConcealMember) }) - m.Combo("/teams").Get(org.ListTeams) + m.Combo("/teams").Get(org.ListTeams). + Post("", bind(api.CreateTeamOption{}), org.CreateTeam) m.Group("/hooks", func() { m.Combo("").Get(org.ListHooks). Post(bind(api.CreateHookOption{}), org.CreateHook) @@ -415,9 +416,19 @@ func RegisterRoutes(m *macaron.Macaron) { }, reqOrgMembership()) }, orgAssignment(true)) m.Group("/teams/:teamid", func() { - m.Get("", org.GetTeam) - m.Get("/members", org.GetTeamMembers) - m.Get("/repos", org.GetTeamRepos) + m.Combo("").Get(org.GetTeam). + Patch(bind(api.EditTeamOption{}), org.EditTeam). + Delete(org.DeleteTeam) + m.Group("/members", func() { + m.Get("", org.GetTeamMembers) + m.Combo("/:username").Put(org.AddTeamMember). + Delete(org.RemoveTeamMember) + }) + m.Group("/repos", func() { + m.Get("", org.GetTeamRepos) + m.Combo("/:reponame").Put(admin.AddTeamRepository). + Delete(admin.RemoveTeamRepository) + }) }, orgAssignment(false, true)) m.Any("/*", func(ctx *context.Context) { @@ -427,7 +438,6 @@ func RegisterRoutes(m *macaron.Macaron) { m.Group("/admin", func() { m.Group("/users", func() { m.Post("", bind(api.CreateUserOption{}), admin.CreateUser) - m.Group("/:username", func() { m.Combo("").Patch(bind(api.EditUserOption{}), admin.EditUser). Delete(admin.DeleteUser) @@ -436,20 +446,6 @@ func RegisterRoutes(m *macaron.Macaron) { m.Post("/repos", bind(api.CreateRepoOption{}), admin.CreateRepo) }) }) - - m.Group("/orgs/:orgname", func() { - m.Group("/teams", func() { - m.Post("", orgAssignment(true), bind(api.CreateTeamOption{}), admin.CreateTeam) - }) - }) - m.Group("/teams", func() { - m.Group("/:teamid", func() { - m.Combo("").Patch(bind(api.EditTeamOption{}), admin.EditTeam). - Delete(admin.DeleteTeam) - m.Combo("/members/:username").Put(admin.AddTeamMember).Delete(admin.RemoveTeamMember) - m.Combo("/repos/:reponame").Put(admin.AddTeamRepository).Delete(admin.RemoveTeamRepository) - }, orgAssignment(false, true)) - }) }, reqAdmin()) }, context.APIContexter()) } diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go index 2f5151887..f87518e25 100644 --- a/routers/api/v1/org/team.go +++ b/routers/api/v1/org/team.go @@ -10,11 +10,16 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/routers/api/v1/convert" + "code.gitea.io/gitea/routers/api/v1/user" ) // 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 @@ -29,26 +34,20 @@ func ListTeams(ctx *context.APIContext) { // GetTeam api for get a team func GetTeam(ctx *context.APIContext) { - ctx.JSON(200, convert.ToTeam(ctx.Org.Team)) -} - -// GetTeamMembers api for get a team's members -func GetTeamMembers(ctx *context.APIContext) { - team := ctx.Org.Team - if err := team.GetMembers(); err != nil { - ctx.Error(500, "GetTeamMembers", err) + if !models.IsOrganizationMember(ctx.Org.Team.OrgID, ctx.User.ID) { + ctx.Status(404) return } - members := make([]*api.User, len(team.Members)) - for i, member := range team.Members { - members[i] = member.APIFormat() - } - ctx.JSON(200, members) + 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) } @@ -63,3 +62,112 @@ func GetTeamRepos(ctx *context.APIContext) { } 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, + Description: form.Description, + Authorize: models.ParseAccessMode(form.Permission), + } + if err := models.NewTeam(team); err != nil { + if models.IsErrTeamAlreadyExist(err) { + ctx.Error(422, "", err) + } else { + ctx.Error(500, "NewTeam", err) + } + return + } + + ctx.JSON(201, convert.ToTeam(team)) +} + +// 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, + Name: form.Name, + Description: form.Description, + Authorize: models.ParseAccessMode(form.Permission), + } + if err := models.UpdateTeam(team, true); err != nil { + ctx.Error(500, "EditTeam", err) + return + } + ctx.JSON(200, convert.ToTeam(team)) +} + +// 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 + } + ctx.Status(204) +} + +// GetTeamMembers api for get a team's members +func GetTeamMembers(ctx *context.APIContext) { + if !models.IsOrganizationMember(ctx.Org.Team.OrgID, ctx.User.ID) { + ctx.Status(404) + return + } + team := ctx.Org.Team + if err := team.GetMembers(); err != nil { + ctx.Error(500, "GetTeamMembers", err) + return + } + members := make([]*api.User, len(team.Members)) + for i, member := range team.Members { + members[i] = member.APIFormat() + } + ctx.JSON(200, members) +} + +// 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 + } + if err := ctx.Org.Team.AddMember(u.ID); err != nil { + ctx.Error(500, "AddMember", err) + return + } + ctx.Status(204) +} + +// 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 + } + + if err := ctx.Org.Team.RemoveMember(u.ID); err != nil { + ctx.Error(500, "RemoveMember", err) + return + } + ctx.Status(204) +}