diff --git a/changes/16691-cross-team-user-list-teams-authz b/changes/16691-cross-team-user-list-teams-authz new file mode 100644 index 00000000000..0ebb6e36070 --- /dev/null +++ b/changes/16691-cross-team-user-list-teams-authz @@ -0,0 +1 @@ +- Fixed fleet-scoped context when retrieving a list of users in a fleet. diff --git a/server/service/users.go b/server/service/users.go index 51b38312daa..d54b3f66e8e 100644 --- a/server/service/users.go +++ b/server/service/users.go @@ -476,6 +476,22 @@ func listUsersEndpoint(ctx context.Context, request interface{}, svc fleet.Servi return resp, nil } +// filterUserTeamsToRequesterScope returns the subset of teams that the +// requester is permitted to see. A requester with any global role sees all +// teams unchanged; a team-scoped requester sees only teams they have a role in. +func filterUserTeamsToRequesterScope(teams []fleet.UserTeam, requester *fleet.User) []fleet.UserTeam { + if requester.HasAnyGlobalRole() { + return teams + } + filtered := make([]fleet.UserTeam, 0, len(teams)) + for _, t := range teams { + if requester.HasAnyRoleInTeam(t.ID) { + filtered = append(filtered, t) + } + } + return filtered +} + func (svc *Service) ListUsers(ctx context.Context, opt fleet.UserListOptions) ([]*fleet.User, error) { user := &fleet.User{} if opt.TeamID != 0 { @@ -485,7 +501,26 @@ func (svc *Service) ListUsers(ctx context.Context, opt fleet.UserListOptions) ([ return nil, err } - return svc.ds.ListUsers(ctx, opt) + vc, ok := viewer.FromContext(ctx) + if !ok { + return nil, fleet.ErrNoContext + } + + users, err := svc.ds.ListUsers(ctx, opt) + if err != nil { + return nil, err + } + + // The datastore loads each user's full team membership. A team-scoped + // requester is only authorized to list users of a team they administer, so + // strip any team memberships (IDs/names/roles) for teams the requester has + // no role in before returning them. Global roles are authorized to see all + // teams and are left untouched. + for _, user := range users { + user.Teams = filterUserTeamsToRequesterScope(user.Teams, vc.User) + } + + return users, nil } func (svc *Service) UsersByIDs(ctx context.Context, ids []uint) ([]*fleet.UserSummary, error) { diff --git a/server/service/users_test.go b/server/service/users_test.go index 76bb14823d9..b81e90208fa 100644 --- a/server/service/users_test.go +++ b/server/service/users_test.go @@ -1907,3 +1907,74 @@ func TestPasswordChangeClearsTokensAndSessions(t *testing.T) { assert.Equal(t, []uint{otherSession.ID}, destroyedSessionIDs, "should only destroy the other session, not the current one") }) } + +// TestListUsersFiltersTeamsToRequesterScope verifies that a team-scoped admin +// listing users does not receive team memberships (IDs/names/roles) for teams +// the requester has no role in. Regression test for the cross-team data +// exposure on GET /api/latest/fleet/users for shared multi-team users. +func TestListUsersFiltersTeamsToRequesterScope(t *testing.T) { + ds := new(mock.Store) + svc, ctx := newTestService(t, ds, nil, nil) + + // A user shared across team 1 and team 2, as returned by the datastore + // (ds.ListUsers always loads the user's full team list). + sharedUserTeams := []fleet.UserTeam{ + {Team: fleet.Team{ID: 1, Name: "Team 1"}, Role: fleet.RoleObserver}, + {Team: fleet.Team{ID: 2, Name: "Team 2"}, Role: fleet.RoleObserver}, + } + ds.ListUsersFunc = func(ctx context.Context, opt fleet.UserListOptions) ([]*fleet.User, error) { + return []*fleet.User{{ + ID: 10, + Teams: append([]fleet.UserTeam{}, sharedUserTeams...), + }}, nil + } + + // Requester is an admin of team 1 only, listing users of team 1. + teamOneAdmin := &fleet.User{ + ID: 1, + Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleAdmin}}, + } + ctx = viewer.NewContext(ctx, viewer.Viewer{User: teamOneAdmin}) + + resp, err := listUsersEndpoint(ctx, &listUsersRequest{ + ListOptions: fleet.UserListOptions{TeamID: 1}, + }, svc) + require.NoError(t, err) + + lr, ok := resp.(listUsersResponse) + require.True(t, ok) + require.NoError(t, lr.Err) + require.Len(t, lr.Users, 1) + + // The requester must only see team 1 (in scope), never team 2. + require.Len(t, lr.Users[0].Teams, 1) + require.Equal(t, uint(1), lr.Users[0].Teams[0].ID) +} + +// TestListUsersGlobalRequesterSeesAllTeams verifies the filter does not strip +// teams for a global-role requester, who is authorized to see all teams. +func TestListUsersGlobalRequesterSeesAllTeams(t *testing.T) { + ds := new(mock.Store) + svc, ctx := newTestService(t, ds, nil, nil) + + ds.ListUsersFunc = func(ctx context.Context, opt fleet.UserListOptions) ([]*fleet.User, error) { + return []*fleet.User{{ + ID: 10, + Teams: []fleet.UserTeam{ + {Team: fleet.Team{ID: 1, Name: "Team 1"}, Role: fleet.RoleObserver}, + {Team: fleet.Team{ID: 2, Name: "Team 2"}, Role: fleet.RoleObserver}, + }, + }}, nil + } + + ctx = viewer.NewContext(ctx, viewer.Viewer{User: test.UserAdmin}) + + resp, err := listUsersEndpoint(ctx, &listUsersRequest{}, svc) + require.NoError(t, err) + + lr, ok := resp.(listUsersResponse) + require.True(t, ok) + require.NoError(t, lr.Err) + require.Len(t, lr.Users, 1) + require.Len(t, lr.Users[0].Teams, 2) +}