Fix permission bugs in team API (#647)

4 files changed, 137 insertions, 121 deletions

diff --git a/models/user.go b/models/user.go
index 8b20d7e0ba..86573dbda4 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 716bd675fa..0000000000
--- 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 29f268d6bc..b83cb36a70 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 2f51518875..f87518e256 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)
+}