Team: Add an endpoint for bulk team membership updates (#87441)

* add an endpoint for bulk team membership updates

* update comment

* schema gen

* test fix

* add swagger parameter definition
This commit is contained in:
Ieva
2024-05-17 11:41:41 +01:00
committed by GitHub
parent 7726631fe8
commit da1a99d729
9 changed files with 407 additions and 1 deletions

View File

@@ -126,6 +126,7 @@ type SyncUserRolesCommand struct {
type TeamPermissionsService interface {
GetPermissions(ctx context.Context, user identity.Requester, resourceID string) ([]ResourcePermission, error)
SetUserPermission(ctx context.Context, orgID int64, user User, resourceID, permission string) (*ResourcePermission, error)
SetPermissions(ctx context.Context, orgID int64, resourceID string, commands ...SetResourcePermissionCommand) ([]ResourcePermission, error)
}
type FolderPermissionsService interface {

View File

@@ -22,6 +22,7 @@ var (
)
const MemberPermissionName = "Member"
const AdminPermissionName = "Admin"
// Team model
type Team struct {
@@ -133,6 +134,11 @@ type UpdateTeamMemberCommand struct {
Permission dashboardaccess.PermissionType `json:"permission"`
}
type SetTeamMembershipsCommand struct {
Members []string `json:"members"`
Admins []string `json:"admins"`
}
type RemoveTeamMemberCommand struct {
OrgID int64 `json:"-"`
UserID int64

View File

@@ -2,12 +2,14 @@ package teamapi
import (
"github.com/grafana/grafana/pkg/api/routing"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/middleware/requestmeta"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/licensing"
pref "github.com/grafana/grafana/pkg/services/preference"
"github.com/grafana/grafana/pkg/services/team"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
)
@@ -15,10 +17,12 @@ type TeamAPI struct {
teamService team.Service
ac accesscontrol.Service
teamPermissionsService accesscontrol.TeamPermissionsService
userService user.Service
license licensing.Licensing
cfg *setting.Cfg
preferenceService pref.Service
ds dashboards.DashboardService
logger log.Logger
}
func ProvideTeamAPI(
@@ -27,6 +31,7 @@ func ProvideTeamAPI(
ac accesscontrol.Service,
acEvaluator accesscontrol.AccessControl,
teamPermissionsService accesscontrol.TeamPermissionsService,
userService user.Service,
license licensing.Licensing,
cfg *setting.Cfg,
preferenceService pref.Service,
@@ -36,10 +41,12 @@ func ProvideTeamAPI(
teamService: teamService,
ac: ac,
teamPermissionsService: teamPermissionsService,
userService: userService,
license: license,
cfg: cfg,
preferenceService: preferenceService,
ds: ds,
logger: log.New("team-api"),
}
tapi.registerRoutes(routeRegister, acEvaluator)
@@ -63,6 +70,8 @@ func (tapi *TeamAPI) registerRoutes(router routing.RouteRegister, ac accesscontr
accesscontrol.ScopeTeamsID)), routing.Wrap(tapi.addTeamMember))
teamsRoute.Put("/:teamId/members/:userId", authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsPermissionsWrite,
accesscontrol.ScopeTeamsID)), routing.Wrap(tapi.updateTeamMember))
teamsRoute.Put("/:teamId/members", authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsPermissionsWrite,
accesscontrol.ScopeTeamsID)), routing.Wrap(tapi.setTeamMemberships))
teamsRoute.Delete("/:teamId/members/:userId", authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsPermissionsWrite,
accesscontrol.ScopeTeamsID)), routing.Wrap(tapi.removeTeamMember))
teamsRoute.Get("/:teamId/preferences", authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsRead,

View File

@@ -10,10 +10,12 @@ import (
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/auth/identity"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/team"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/web"
)
@@ -140,12 +142,122 @@ func (tapi *TeamAPI) updateTeamMember(c *contextmodel.ReqContext) response.Respo
return response.Success("Team member updated")
}
// swagger:route PUT /teams/{team_id}/members teams setTeamMemberships
//
// Set team memberships.
//
// Takes user emails, and updates team members and admins to the provided lists of users.
// Any current team members and admins not in the provided lists will be removed.
//
// Responses:
// 200: okResponse
// 401: unauthorisedError
// 403: forbiddenError
// 404: notFoundError
// 500: internalServerError
func (tapi *TeamAPI) setTeamMemberships(c *contextmodel.ReqContext) response.Response {
cmd := team.SetTeamMembershipsCommand{}
if err := web.Bind(c.Req, &cmd); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
}
teamId, err := strconv.ParseInt(web.Params(c.Req)[":teamId"], 10, 64)
if err != nil {
return response.Error(http.StatusBadRequest, "teamId is invalid", err)
}
orgId := c.SignedInUser.GetOrgID()
teamMemberships, err := tapi.getTeamMembershipUpdates(c.Req.Context(), orgId, teamId, cmd, c.SignedInUser)
if err != nil {
if errors.Is(err, user.ErrUserNotFound) || errors.Is(err, team.ErrTeamNotFound) {
return response.Error(http.StatusNotFound, err.Error(), nil)
}
return response.Error(http.StatusInternalServerError, "Failed to parse team membership updates", err)
}
_, err = tapi.teamPermissionsService.SetPermissions(c.Req.Context(), orgId, strconv.FormatInt(teamId, 10), teamMemberships...)
if err != nil {
if errors.Is(err, user.ErrUserNotFound) || errors.Is(err, team.ErrTeamNotFound) {
return response.Error(http.StatusNotFound, err.Error(), nil)
}
return response.Error(http.StatusInternalServerError, "Failed to update team memberships", err)
}
return response.Success("Team memberships have been updated")
}
func (tapi *TeamAPI) getTeamMembershipUpdates(ctx context.Context, orgID, teamID int64, cmd team.SetTeamMembershipsCommand, signedInUser identity.Requester) ([]accesscontrol.SetResourcePermissionCommand, error) {
adminEmails := make(map[string]struct{}, len(cmd.Admins))
for _, admin := range cmd.Admins {
adminEmails[admin] = struct{}{}
}
memberEmails := make(map[string]struct{}, len(cmd.Members))
for _, member := range cmd.Members {
memberEmails[member] = struct{}{}
}
currentMemberships, err := tapi.teamService.GetTeamMembers(ctx, &team.GetTeamMembersQuery{OrgID: orgID, TeamID: teamID, SignedInUser: signedInUser})
if err != nil {
return nil, err
}
membersToRemove := make([]int64, 0)
for _, member := range currentMemberships {
if _, ok := adminEmails[member.Email]; ok {
if getPermissionName(member.Permission) == team.AdminPermissionName {
delete(adminEmails, member.Email)
}
continue
}
if _, ok := memberEmails[member.Email]; ok {
if getPermissionName(member.Permission) == team.MemberPermissionName {
delete(memberEmails, member.Email)
}
continue
}
membersToRemove = append(membersToRemove, member.UserID)
}
adminIDs, err := tapi.getUserIDs(ctx, adminEmails)
if err != nil {
return nil, err
}
memberIDs, err := tapi.getUserIDs(ctx, memberEmails)
if err != nil {
return nil, err
}
teamMemberships := make([]accesscontrol.SetResourcePermissionCommand, 0, len(adminIDs)+len(memberIDs)+len(membersToRemove))
for _, admin := range adminIDs {
teamMemberships = append(teamMemberships, accesscontrol.SetResourcePermissionCommand{Permission: team.AdminPermissionName, UserID: admin})
}
for _, member := range memberIDs {
teamMemberships = append(teamMemberships, accesscontrol.SetResourcePermissionCommand{Permission: team.MemberPermissionName, UserID: member})
}
for _, member := range membersToRemove {
teamMemberships = append(teamMemberships, accesscontrol.SetResourcePermissionCommand{Permission: "", UserID: member})
}
return teamMemberships, nil
}
func (tapi *TeamAPI) getUserIDs(ctx context.Context, emails map[string]struct{}) ([]int64, error) {
userIDs := make([]int64, 0, len(emails))
for email := range emails {
user, err := tapi.userService.GetByEmail(ctx, &user.GetUserByEmailQuery{Email: email})
if err != nil {
tapi.logger.Error("failed to find user", "email", email, "error", err)
return nil, err
}
userIDs = append(userIDs, user.ID)
}
return userIDs, nil
}
func getPermissionName(permission dashboardaccess.PermissionType) string {
permissionName := permission.String()
// Team member permission is 0, which maps to an empty string.
// However, we want the team permission service to display "Member" for team members. This is a hack to make it work.
if permissionName == "" {
permissionName = "Member"
permissionName = team.MemberPermissionName
}
return permissionName
}
@@ -227,6 +339,16 @@ type UpdateTeamMemberParams struct {
UserID int64 `json:"user_id"`
}
// swagger:parameters setTeamMemberships
type SetTeamMembershipsParams struct {
// in:body
// required:true
Body team.SetTeamMembershipsCommand `json:"body"`
// in:path
// required:true
TeamID string `json:"team_id"`
}
// swagger:parameters removeTeamMember
type RemoveTeamMemberParams struct {
// in:path

View File

@@ -1,7 +1,9 @@
package teamapi
import (
"context"
"net/http"
"strconv"
"strings"
"testing"
@@ -13,12 +15,15 @@ import (
"github.com/grafana/grafana/pkg/services/accesscontrol/acimpl"
"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/licensing"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/preference/preftest"
"github.com/grafana/grafana/pkg/services/team"
"github.com/grafana/grafana/pkg/services/team/teamtest"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/web/webtest"
)
@@ -34,6 +39,7 @@ func SetupAPITestServer(t *testing.T, opts ...func(a *TeamAPI)) *webtest.Server
actest.FakeService{},
acimpl.ProvideAccessControl(featuremgmt.WithFeatures()),
&actest.FakePermissionsService{},
&usertest.FakeUserService{},
&licensing.OSSLicensingService{},
cfg,
preftest.NewPreferenceServiceFake(),
@@ -154,6 +160,126 @@ func TestDeleteTeamMembersAPIEndpoint(t *testing.T) {
})
}
func Test_getTeamMembershipUpdates(t *testing.T) {
type testCase struct {
description string
input team.SetTeamMembershipsCommand
currentMembers []*team.TeamMemberDTO
expectedUpdates []accesscontrol.SetResourcePermissionCommand
expectErr bool
}
testCases := []testCase{
{
description: "should correctly list members and admins for a team with no current members or admins",
input: team.SetTeamMembershipsCommand{
Members: []string{"user1", "user2"},
Admins: []string{"user3"},
},
expectedUpdates: []accesscontrol.SetResourcePermissionCommand{
{UserID: 1, Permission: team.MemberPermissionName},
{UserID: 2, Permission: team.MemberPermissionName},
{UserID: 3, Permission: team.AdminPermissionName},
},
},
{
description: "should correctly diff the member updates for a team with existing members and admins",
input: team.SetTeamMembershipsCommand{
Members: []string{"user1", "user2"},
Admins: []string{"user3"},
},
currentMembers: []*team.TeamMemberDTO{
{Email: "user1", Permission: 0},
{Email: "user3", Permission: dashboardaccess.PERMISSION_ADMIN},
},
expectedUpdates: []accesscontrol.SetResourcePermissionCommand{
{UserID: 2, Permission: team.MemberPermissionName},
},
},
{
description: "should correctly update membership type for the existing members and admins",
input: team.SetTeamMembershipsCommand{
Members: []string{"user1", "user2"},
Admins: []string{"user3"},
},
currentMembers: []*team.TeamMemberDTO{
{Email: "user1", Permission: 0},
{Email: "user2", Permission: dashboardaccess.PERMISSION_ADMIN},
{Email: "user3", Permission: 0},
},
expectedUpdates: []accesscontrol.SetResourcePermissionCommand{
{UserID: 2, Permission: team.MemberPermissionName},
{UserID: 3, Permission: team.AdminPermissionName},
},
},
{
description: "should correctly remove current members and admins that are not in the new list",
input: team.SetTeamMembershipsCommand{
Members: []string{"user1"},
Admins: []string{"user3"},
},
currentMembers: []*team.TeamMemberDTO{
{Email: "user1", UserID: 1, Permission: 0},
{Email: "user2", UserID: 2, Permission: 0},
{Email: "user3", UserID: 3, Permission: dashboardaccess.PERMISSION_ADMIN},
{Email: "user4", UserID: 4, Permission: dashboardaccess.PERMISSION_ADMIN},
},
expectedUpdates: []accesscontrol.SetResourcePermissionCommand{
{UserID: 2, Permission: ""},
{UserID: 4, Permission: ""},
},
},
{
description: "should error if a non-existent user is provided",
input: team.SetTeamMembershipsCommand{
Members: []string{"non-existent-user", "user2"},
},
expectErr: true,
},
}
for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) {
cfg := setting.NewCfg()
userService := &usertest.FakeUserService{
GetByEmailFn: func(ctx context.Context, query *user.GetUserByEmailQuery) (*user.User, error) {
id, err := strconv.Atoi(strings.TrimPrefix(query.Email, "user"))
if err != nil {
return nil, err
}
user := &user.User{
ID: int64(id),
Email: query.Email,
}
return user, nil
},
}
teamSvc := teamtest.NewFakeService()
teamSvc.ExpectedMembers = tc.currentMembers
tapi := ProvideTeamAPI(routing.NewRouteRegister(),
teamSvc,
actest.FakeService{},
acimpl.ProvideAccessControl(featuremgmt.WithFeatures()),
&actest.FakePermissionsService{},
userService,
&licensing.OSSLicensingService{},
cfg,
preftest.NewPreferenceServiceFake(),
dashboards.NewFakeDashboardService(t),
)
user := &user.SignedInUser{UserID: 1, OrgID: 1, OrgRole: org.RoleAdmin, Permissions: map[int64]map[string][]string{1: {accesscontrol.ActionOrgUsersRead: {"users:id:*"}}}}
updates, err := tapi.getTeamMembershipUpdates(context.Background(), 1, 1, tc.input, user)
if tc.expectErr {
require.Error(t, err)
return
}
require.NoError(t, err)
require.ElementsMatch(t, tc.expectedUpdates, updates)
})
}
}
func authedUserWithPermissions(userID, orgID int64, permissions []accesscontrol.Permission) *user.SignedInUser {
return &user.SignedInUser{UserID: userID, OrgID: orgID, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction(permissions)}}
}

View File

@@ -20,6 +20,7 @@ type FakeUserService struct {
GetSignedInUserFn func(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error)
CreateFn func(ctx context.Context, cmd *user.CreateUserCommand) (*user.User, error)
BatchDisableUsersFn func(ctx context.Context, cmd *user.BatchDisableUsersCommand) error
GetByEmailFn func(ctx context.Context, query *user.GetUserByEmailQuery) (*user.User, error)
counter int
}
@@ -57,6 +58,9 @@ func (f *FakeUserService) GetByLogin(ctx context.Context, query *user.GetUserByL
}
func (f *FakeUserService) GetByEmail(ctx context.Context, query *user.GetUserByEmailQuery) (*user.User, error) {
if f.GetByEmailFn != nil {
return f.GetByEmailFn(ctx, query)
}
return f.ExpectedUser, f.ExpectedError
}

View File

@@ -6872,6 +6872,23 @@
}
}
},
"SetTeamMembershipsCommand": {
"type": "object",
"properties": {
"admins": {
"type": "array",
"items": {
"type": "string"
}
},
"members": {
"type": "array",
"items": {
"type": "string"
}
}
}
},
"SetUserRolesCommand": {
"type": "object",
"properties": {

View File

@@ -9121,6 +9121,47 @@
}
}
},
"put": {
"description": "Takes user emails, and updates team members and admins to the provided lists of users.\nAny current team members and admins not in the provided lists will be removed.",
"tags": [
"teams"
],
"summary": "Set team memberships.",
"operationId": "setTeamMemberships",
"parameters": [
{
"name": "body",
"in": "body",
"required": true,
"schema": {
"$ref": "#/definitions/SetTeamMembershipsCommand"
}
},
{
"type": "string",
"name": "team_id",
"in": "path",
"required": true
}
],
"responses": {
"200": {
"$ref": "#/responses/okResponse"
},
"401": {
"$ref": "#/responses/unauthorisedError"
},
"403": {
"$ref": "#/responses/forbiddenError"
},
"404": {
"$ref": "#/responses/notFoundError"
},
"500": {
"$ref": "#/responses/internalServerError"
}
}
},
"post": {
"tags": [
"teams"
@@ -19418,6 +19459,23 @@
}
}
},
"SetTeamMembershipsCommand": {
"type": "object",
"properties": {
"admins": {
"type": "array",
"items": {
"type": "string"
}
},
"members": {
"type": "array",
"items": {
"type": "string"
}
}
}
},
"SetUserRolesCommand": {
"type": "object",
"properties": {

View File

@@ -9979,6 +9979,23 @@
},
"type": "object"
},
"SetTeamMembershipsCommand": {
"properties": {
"admins": {
"items": {
"type": "string"
},
"type": "array"
},
"members": {
"items": {
"type": "string"
},
"type": "array"
}
},
"type": "object"
},
"SetUserRolesCommand": {
"properties": {
"global": {
@@ -22323,6 +22340,52 @@
"tags": [
"teams"
]
},
"put": {
"description": "Takes user emails, and updates team members and admins to the provided lists of users.\nAny current team members and admins not in the provided lists will be removed.",
"operationId": "setTeamMemberships",
"parameters": [
{
"in": "path",
"name": "team_id",
"required": true,
"schema": {
"type": "string"
}
}
],
"requestBody": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/SetTeamMembershipsCommand"
}
}
},
"required": true,
"x-originalParamName": "body"
},
"responses": {
"200": {
"$ref": "#/components/responses/okResponse"
},
"401": {
"$ref": "#/components/responses/unauthorisedError"
},
"403": {
"$ref": "#/components/responses/forbiddenError"
},
"404": {
"$ref": "#/components/responses/notFoundError"
},
"500": {
"$ref": "#/components/responses/internalServerError"
}
},
"summary": "Set team memberships.",
"tags": [
"teams"
]
}
},
"/teams/{team_id}/members/{user_id}": {