Skip to content

Commit 64cef29

Browse files
authored
Merge pull request #534 from slashdevops/feat/scim-listgroups-membersvalue
feat(scim): replace brute-force member sync with members.value pagination
2 parents db655f7 + 91db0fa commit 64cef29

14 files changed

Lines changed: 628 additions & 486 deletions

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,8 @@ For config file examples, environment variable usage, CLI flags, SAM parameter u
164164

165165
## ⚠️ Limitations
166166

167-
* **Group Limit**: The AWS SSO SCIM API has a limit of 50 groups per request. Please support the feature request on the [AWS Support site](https://repost.aws/questions/QUqqnVkIo_SYyF_SlX5LcUjg/aws-sso-scim-api-pagination-for-methods) to help get this limit increased.
168-
* **Throttling**: With a large number of users and groups, you may encounter a `ThrottlingException` from the AWS SSO SCIM API. This project uses the [httpx](https://github.com/slashdevops/httpx) library with automatic retry and jitter backoff to mitigate this, but it's still a possibility.
167+
* **Group Page Size**: The AWS IAM Identity Center SCIM `ListGroups` endpoint returns at most 100 groups per page. Since v0.45.0 this project walks every page via cursor-based pagination, so a larger directory no longer requires manual configuration.
168+
* **Throttling**: With a very large number of users and groups, you may still encounter a `ThrottlingException` from the AWS IAM Identity Center SCIM API. The new member-resolution algorithm (one `members.value` query per user, see [docs/Whats-New.md](docs/Whats-New.md)) is roughly two orders of magnitude lighter than the old brute-force path, but the underlying SCIM endpoint is still rate-limited. This project uses the [httpx](https://github.com/slashdevops/httpx) library with automatic retry and jitter backoff to mitigate this.
169169
* **User Status**: The Google Workspace API doesn't differentiate between normal and guest users except for their status. This project only syncs `ACTIVE` users.
170170

171171
## For `ssosync` Users

docs/Whats-New.md

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,38 @@ This document tracks notable changes, new features, and bug fixes across release
44

55
## Unreleased
66

7+
### SCIM members sync — major security & performance improvement (closes [#520](https://github.com/slashdevops/idp-scim-sync/issues/520))
8+
9+
> [!IMPORTANT]
10+
> This release replaces the brute-force algorithm that reconstructed group memberships on the AWS IAM Identity Center side. The change is **internal-only** (no CLI/config change) but materially improves both the **security posture** and the **runtime cost** of every sync.
11+
12+
#### Security improvements
13+
14+
* **Drastically smaller attack & failure surface per sync.** The number of authenticated SCIM requests issued per sync drops by ~2 orders of magnitude (see *Performance* below). Each request is a credential-bearing call to AWS IAM Identity Center — fewer requests means fewer opportunities for a credential leak, log capture, in-flight tampering, or partial-failure half-state to be observed.
15+
* **Shorter sync window = smaller inconsistency window.** Previously, a sync of a few hundred users could run for many minutes while ~100k requests trickled out behind a hand-rolled 10–150 ms random sleep. During that window, group membership in AWS could be partially reconciled — an externally-observable inconsistent state. The new path completes in a fraction of the time, shrinking that window proportionally.
16+
* **No more time-based throttling band-aid.** The previous `time.Sleep(rand.Intn(...))` jitter existed solely to avoid tripping AWS SCIM throttles under the brute-force call volume. It has been removed: the new call profile is light enough that artificial gapping is no longer required. This eliminates a source of non-determinism in the sync path and removes timing-dependent behavior from a security-sensitive code path.
17+
* **Deterministic pagination.** Cursor-based pagination (`?cursor` + `nextCursor`) walks the full result set deterministically, so memberships can no longer be silently truncated by hitting an undocumented page cap mid-sync.
18+
19+
#### Performance improvements
20+
21+
* **Before:** `internal/scim.GetGroupsMembersBruteForce` issued one `ListGroups` call for *every* (group, user) combination — `O(N_groups × N_users)` requests per sync, throttled by a 10–150 ms random sleep and capped at concurrency 5. For an org with 200 groups and 500 users this is **~100,000 calls per sync run**.
22+
* **Now:** `internal/scim.GetGroupsMembers(ctx, groups, users)` issues one cursor-paginated `?cursor&filter=members.value eq "<user-id>"` request per user (plus one extra request per additional page of memberships, when a single user belongs to more than 100 groups). The result is then inverted into the group → members map the rest of the pipeline expects. For the same 200-group / 500-user org this is **~500 calls per sync — roughly two orders of magnitude fewer requests**.
23+
* **Lower Lambda execution time and cost.** Fewer requests + no random sleeps directly reduces billable Lambda duration on every scheduled invocation.
24+
25+
This is enabled by two AWS IAM Identity Center SCIM features documented at <https://docs.aws.amazon.com/singlesignon/latest/developerguide/listgroups.html>:
26+
27+
* The `members.value eq "<user-id>"` filter, which returns every group containing a given user.
28+
* Cursor-based pagination (`?cursor` + `nextCursor`), which lifts the historical 50-result page cap to 100 results per page and supports walking the full result set deterministically.
29+
30+
**API changes (internal-only — no user-facing CLI/config change):**
31+
32+
* `pkg/aws.SCIMService` gained `ListGroupsWithCursor(ctx, filter, cursor) (*ListGroupsResponse, error)`. `ListGroups` is unchanged and remains the non-paginated single-page call.
33+
* `pkg/aws.ListResponse` gained a `NextCursor string` field.
34+
* `internal/core.SCIMService.GetGroupsMembers` now takes `(ctx, *model.GroupsResult, *model.UsersResult)`. The previous `GetGroupsMembers(ctx, gr)` and `GetGroupsMembersBruteForce(ctx, gr, ur)` methods, plus their AWS-side brute-force scaffolding, have been removed entirely — there is no compatibility shim.
35+
* Memberships pointing at AWS-side groups that are *not* part of the in-scope `gr` (for example AWS-managed groups created outside the sync) are silently ignored, matching prior behavior.
36+
37+
**Tests:** the concurrency cap is now exercised under `testing/synctest` (graduated to the standard library in Go 1.26 — see <https://go.dev/blog/testing-time>) so the test asserts the true peak in-flight count using virtual time, instead of waiting on a wall-clock sleep race.
38+
739
### IAM least-privilege hardening for the state-file Lambda role
840

941
Tightens the Lambda execution role in `template.yaml` so it can only touch the single state object via the single intended path. No behavior change for normal operation; the role is now strictly scoped.

internal/core/actions.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,7 @@ func scimSync(
7272
totalUsersResult = model.MergeUsersResult(usersCreated, usersUpdated, usersEqual)
7373

7474
slog.Info("getting SCIM Groups Members")
75-
// unfortunately, the SCIM service does not support the getGroupsMembers method in and efficient way
76-
// see: "Nor Supported" section in: https://docs.aws.amazon.com/singlesignon/latest/developerguide/listgroups.html
77-
// scimGroupsMembersResult, err := scim.GetGroupsMembers(ctx, &totalGroupsResult) // not supported yet
78-
scimGroupsMembersResult, err := scim.GetGroupsMembersBruteForce(ctx, totalGroupsResult, totalUsersResult)
75+
scimGroupsMembersResult, err := scim.GetGroupsMembers(ctx, totalGroupsResult, totalUsersResult)
7976
if err != nil {
8077
return nil, nil, nil, fmt.Errorf("error getting groups members from the SCIM service: %w", err)
8178
}

internal/core/scim.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ type SCIMService interface {
3535
// DeleteUsers deletes users in the SCIM Service given a list of users.
3636
DeleteUsers(ctx context.Context, ur *model.UsersResult) error
3737

38-
// GetGroupsMembers get the Groups and their Members from the SCIM service.
39-
GetGroupsMembers(ctx context.Context, gr *model.GroupsResult) (*model.GroupsMembersResult, error)
40-
41-
// GetGroupsMembersBruteForce get the Groups and their Members from the SCIM service using brute force.
42-
GetGroupsMembersBruteForce(ctx context.Context, gr *model.GroupsResult, ur *model.UsersResult) (*model.GroupsMembersResult, error)
38+
// GetGroupsMembers gets the in-scope Groups and their Members from the
39+
// SCIM service. The implementation queries AWS with the members.value
40+
// filter for each user in ur and assigns memberships back to groups in
41+
// gr; memberships pointing at groups outside gr are ignored.
42+
GetGroupsMembers(ctx context.Context, gr *model.GroupsResult, ur *model.UsersResult) (*model.GroupsMembersResult, error)
4343

4444
// CreateGroupsMembers create groups members in the SCIM Service given a list of groups members.
4545
CreateGroupsMembers(ctx context.Context, gmr *model.GroupsMembersResult) (*model.GroupsMembersResult, error)

internal/core/sync_test.go

Lines changed: 37 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -309,39 +309,15 @@ func TestSyncService_SyncGroupsAndTheirMembers(t *testing.T) {
309309
createUser2ResponseJSONBytes, err := json.Marshal(createUser2Response)
310310
assert.NoError(t, err)
311311

312-
listGroupsResponseGroup1User1 := &aws.ListGroupsResponse{
313-
ListResponse: aws.ListResponse{
314-
StartIndex: 1,
315-
ItemsPerPage: 1,
316-
TotalResults: 1,
317-
Schemas: []string{"urn:ietf:params:scim:schemas:core:2.0:ListResponse"},
318-
},
319-
Resources: []*aws.Group{
320-
{
321-
ID: "group-1",
322-
Meta: aws.Meta{
323-
ResourceType: "Group",
324-
Created: "2020-01-01T00:00:00Z",
325-
LastModified: "2020-01-01T00:00:00Z",
326-
},
327-
Schemas: []string{"urn:ietf:params:scim:schemas:core:2.0:Group"},
328-
DisplayName: "group 1",
329-
Members: []*aws.Member{}, // AWS SSO SCIM API don't return members for list groups only TotalResults is returned
330-
},
331-
},
332-
}
333-
listGroupsResponseGroup1User1JSONBytes, err := json.Marshal(listGroupsResponseGroup1User1)
334-
assert.NoError(t, err)
335-
336-
listGroupsResponseGroup1User2 := &aws.ListGroupsResponse{
337-
ListResponse: aws.ListResponse{
338-
StartIndex: 1,
339-
ItemsPerPage: 1,
340-
TotalResults: 0,
341-
Schemas: []string{"urn:ietf:params:scim:schemas:core:2.0:ListResponse"},
342-
},
343-
Resources: []*aws.Group{
344-
{
312+
// listGroupsByMember returns the AWS ListGroups response for a
313+
// `members.value eq "<user-id>"` cursor query. The test scenario is:
314+
// group-1 contains user-1; group-2 contains user-2.
315+
listGroupsByMember := func(userSCIMID string) []byte {
316+
schemas := []string{"urn:ietf:params:scim:api:messages:2.0:ListResponse"}
317+
groups := []*aws.Group{}
318+
switch userSCIMID {
319+
case "user-1":
320+
groups = append(groups, &aws.Group{
345321
ID: "group-1",
346322
Meta: aws.Meta{
347323
ResourceType: "Group",
@@ -350,22 +326,10 @@ func TestSyncService_SyncGroupsAndTheirMembers(t *testing.T) {
350326
},
351327
Schemas: []string{"urn:ietf:params:scim:schemas:core:2.0:Group"},
352328
DisplayName: "group 1",
353-
Members: []*aws.Member{}, // AWS SSO SCIM API don't return members for list groups only TotalResults is returned
354-
},
355-
},
356-
}
357-
listGroupsResponseGroup1User2JSONBytes, err := json.Marshal(listGroupsResponseGroup1User2)
358-
assert.NoError(t, err)
359-
360-
listGroupsResponseGroup2User1 := &aws.ListGroupsResponse{
361-
ListResponse: aws.ListResponse{
362-
StartIndex: 1,
363-
ItemsPerPage: 1,
364-
TotalResults: 0,
365-
Schemas: []string{"urn:ietf:params:scim:schemas:core:2.0:ListResponse"},
366-
},
367-
Resources: []*aws.Group{
368-
{
329+
Members: []*aws.Member{},
330+
})
331+
case "user-2":
332+
groups = append(groups, &aws.Group{
369333
ID: "group-2",
370334
Meta: aws.Meta{
371335
ResourceType: "Group",
@@ -374,36 +338,20 @@ func TestSyncService_SyncGroupsAndTheirMembers(t *testing.T) {
374338
},
375339
Schemas: []string{"urn:ietf:params:scim:schemas:core:2.0:Group"},
376340
DisplayName: "group 2",
377-
Members: []*aws.Member{}, // AWS SSO SCIM API don't return members for list groups only TotalResults is returned
378-
},
379-
},
380-
}
381-
listGroupsResponseGroup2User1JSONBytes, err := json.Marshal(listGroupsResponseGroup2User1)
382-
assert.NoError(t, err)
383-
384-
listGroupsResponseGroup2User2 := &aws.ListGroupsResponse{
385-
ListResponse: aws.ListResponse{
386-
StartIndex: 1,
387-
ItemsPerPage: 1,
388-
TotalResults: 1,
389-
Schemas: []string{"urn:ietf:params:scim:schemas:core:2.0:ListResponse"},
390-
},
391-
Resources: []*aws.Group{
392-
{
393-
ID: "group-2",
394-
Meta: aws.Meta{
395-
ResourceType: "Group",
396-
Created: "2020-01-01T00:00:00Z",
397-
LastModified: "2020-01-01T00:00:00Z",
398-
},
399-
Schemas: []string{"urn:ietf:params:scim:schemas:core:2.0:Group"},
400-
DisplayName: "group 2",
401-
Members: []*aws.Member{}, // AWS SSO SCIM API don't return members for list groups only TotalResults is returned
341+
Members: []*aws.Member{},
342+
})
343+
}
344+
resp := &aws.ListGroupsResponse{
345+
ListResponse: aws.ListResponse{
346+
ItemsPerPage: len(groups),
347+
Schemas: schemas,
402348
},
403-
},
349+
Resources: groups,
350+
}
351+
body, err := json.Marshal(resp)
352+
assert.NoError(t, err)
353+
return body
404354
}
405-
listGroupsResponseGroup2User2JSONBytes, err := json.Marshal(listGroupsResponseGroup2User2)
406-
assert.NoError(t, err)
407355

408356
// mock Google Workspace API calls
409357
svrIDP := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -456,21 +404,20 @@ func TestSyncService_SyncGroupsAndTheirMembers(t *testing.T) {
456404
switch r.URL.Path {
457405
case "/Groups":
458406
filter := r.URL.Query().Get("filter")
459-
460-
switch filter {
461-
case "": // first time getting groups
407+
// AWS SCIM cursor-paginated calls include a bare "cursor"
408+
// param; the new GetGroupsMembers algorithm queries one
409+
// `members.value eq "<user-id>"` page per user.
410+
const prefix = `members.value eq "`
411+
if strings.HasPrefix(filter, prefix) && strings.HasSuffix(filter, `"`) {
412+
userSCIMID := filter[len(prefix) : len(filter)-1]
413+
_, _ = w.Write(listGroupsByMember(userSCIMID))
414+
return
415+
}
416+
if filter == "" {
462417
_, _ = w.Write([]byte(`{}`))
463-
case "id eq \"group-1\" and members eq \"user-1\"":
464-
_, _ = w.Write(listGroupsResponseGroup1User1JSONBytes)
465-
case "id eq \"group-1\" and members eq \"user-2\"":
466-
_, _ = w.Write(listGroupsResponseGroup1User2JSONBytes) // user 2 is not in group 1
467-
case "id eq \"group-2\" and members eq \"user-1\"":
468-
_, _ = w.Write(listGroupsResponseGroup2User1JSONBytes) // user 1 is not in group 2
469-
case "id eq \"group-2\" and members eq \"user-2\"":
470-
_, _ = w.Write(listGroupsResponseGroup2User2JSONBytes)
471-
default:
472-
w.WriteHeader(http.StatusBadRequest)
418+
return
473419
}
420+
w.WriteHeader(http.StatusBadRequest)
474421
case "/Users":
475422
_, _ = w.Write([]byte(`{}`))
476423
}

0 commit comments

Comments
 (0)