Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/16691-cross-team-user-list-teams-authz
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fixed fleet-scoped context when retrieving a list of users in a fleet.
37 changes: 36 additions & 1 deletion server/service/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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) {
Expand Down
71 changes: 71 additions & 0 deletions server/service/users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Loading