Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A concurrency race can leave successful membership changes unaudited because prepared events may become stale before the actual mutations occur.
1 open issue
What changed in this PR
Adds explicit membership audit events to UAA’s SCIM group APIs while preserving the existing GroupModifiedEvent payload.
Changes:
- Adds membership-added and membership-removed events identifying the affected member.
- Publishes membership events before the final group snapshot for whole-group PUT/PATCH and single-member POST/DELETE operations.
- Adds tests for event ordering, payloads, identity zones, and failed requests.
| File | Description |
|---|---|
uaa/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimGroupEndpointsTests.java |
Updates endpoint construction for publisher injection. |
uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/audit/AuditCheckMockMvcTests.java |
Adds end-to-end membership audit coverage. |
server/src/test/java/org/cloudfoundry/identity/uaa/scim/event/ScimMembershipEventPublisherTests.java |
Tests change detection, name lookup, and fallback behavior. |
server/src/main/java/org/cloudfoundry/identity/uaa/scim/event/ScimEventPublisher.java |
Prepares, enriches, and publishes membership events. |
server/src/main/java/org/cloudfoundry/identity/uaa/scim/event/GroupMembershipEvent.java |
Defines the membership audit payload. |
server/src/main/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimGroupEndpoints.java |
Integrates auditing into membership operations. |
server/src/main/java/org/cloudfoundry/identity/uaa/scim/beans/ScimBeanConfiguration.java |
Injects provisioning services for member-name lookup. |
server/src/main/java/org/cloudfoundry/identity/uaa/scim/beans/ScimAopConfig.java |
Extends group-modified auditing to PATCH. |
server/src/main/java/org/cloudfoundry/identity/uaa/audit/AuditEventType.java |
Adds two membership audit event codes. |
Review effort: Balanced
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A critical data-integrity issue remains because long group or member names can be truncated into invalid JSON audit records, and a test-convention nit also remains unresolved.
2 open issues
1 resolved since last review
Review effort: Lite
strehle
previously approved these changes
Nov 5, 2026
github-project-automation
Bot
moved this from Inbox
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
Emit named membership-added and membership-removed events for group updates and single-member operations. Preserve the existing group-modified event payload and publish it last for both group updates and single-member changes.
Exercise overlapping whole-group and single-member changes with latch-controlled requests. Require success events to wait for commit and failed updates to preserve membership and metadata. These regression tests intentionally fail before transaction coordination is applied.
Coordinate whole-group PUT/PATCH and single-member POST/DELETE with a zone-scoped group lock held in a database transaction. Publish membership audit events after commit and roll back partial updates on failure. Add contract checks for the new locking helper and update endpoint test construction for its dependency.
Replace timed latch and future waits with Awaitility conditions while retaining request sequencing and continuous blocked-state assertions. Preserve worker failure propagation after completion.
duanemay
force-pushed
the
feature/group-membership-audit-events
branch
from
November 6, 2026 19:17
6d7fdb8 to
a07cca5
Compare
fhanik
self-requested a review
November 10, 2026 01:17
fhanik
approved these changes
Nov 10, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make group membership auditing consistent across these three API operations:
PUT /Groups/{groupId}: previously emitted onlyGroupModifiedEvent, listing the members after the update without identifying who was added or removed. Now emitGroupMembershipAddedEventandGroupMembershipRemovedEventas needed for the affected members, followed byGroupModifiedEvent. Whole-groupPATCH /Groups/{groupId}now follows the same behavior.POST /Groups/{groupId}/members: previously emitted no group audit events. Now emitGroupMembershipAddedEvent, followed byGroupModifiedEvent.DELETE /Groups/{groupId}/members/{memberId}: previously emitted no group audit events. Now emitGroupMembershipRemovedEvent, followed byGroupModifiedEvent.The new membership events identify the affected member by ID, name and type. The existing
GroupModifiedEventpayload stays unchanged and contains the complete resulting membership list.Adding a member
Removing a member