Skip to content

SSF: Derive emit audit event data from typed events (#50720) - #50728

Merged
ssilvert merged 2 commits into
keycloak:mainfrom
thomasdarimont:issue/gh-50720-ssf-redact-admin-event-payload
Aug 11, 2026
Merged

SSF: Derive emit audit event data from typed events (#50720)#50728
ssilvert merged 2 commits into
keycloak:mainfrom
thomasdarimont:issue/gh-50720-ssf-redact-admin-event-payload

Conversation

@thomasdarimont

@thomasdarimont thomasdarimont commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Instead of persisting the verbatim emit request payload in the admin
event, EmitEventResult now carries the validated typed SsfEvent and the
audit representation is built via the new SsfEvent.createAdminDetails()
hook. Each event class contributes only its audit-safe fields
(e.g. credential_type/change_type for credential-change, status/reason
for stream-updated, the standard CAEP claims on CaepEvent); the default
is an empty map, so free-form payload data and PII never reach the
admin event store. EventEmitterService now also rejects payloads that
do not materialise as a typed SsfEvent.

Fixes #50720

@thomasdarimont
thomasdarimont requested a review from a team as a code owner July 9, 2026 10:41
Copilot AI balanced review requested due to automatic review settings July 9, 2026 10:41
Copilot AI reviewed Jul 9, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses SSF data minimization for the /events/emit admin endpoint by ensuring synthetic SSF event payloads are no longer persisted verbatim in admin event representations (preventing payload PII from being stored beyond SSF delivery), while keeping bounded correlation metadata and validating behavior via integration tests.

Changes:

  • Redacts the audited eventData stored in admin events to a bounded key skeleton with REDACTED values.
  • Introduces bounds for maximum audited payload keys and maximum key length.
  • Adds integration tests asserting redaction, delivery still containing verbatim payload, and bounding behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
ssf/tests/base/src/test/java/org/keycloak/tests/ssf/transmitter/SsfTransmitterEventEmitterTests.java Adds integration coverage to ensure admin event audit data is redacted/bounded while SET delivery remains unredacted.
ssf/services/src/main/java/org/keycloak/ssf/services/admin/SsfAdminResource.java Replaces persisted admin-event payload with a bounded, redacted key skeleton to prevent PII persistence in admin event representations.

Comment thread ssf/services/src/main/java/org/keycloak/ssf/services/admin/SsfAdminResource.java Outdated
Comment thread ssf/services/src/main/java/org/keycloak/ssf/services/admin/SsfAdminResource.java Outdated
Copilot AI review requested due to automatic review settings July 9, 2026 10:58
@thomasdarimont
thomasdarimont force-pushed the issue/gh-50720-ssf-redact-admin-event-payload branch from d4245e1 to 3e9facf Compare July 9, 2026 10:58
Copilot AI reviewed Jul 9, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread ssf/services/src/main/java/org/keycloak/ssf/services/admin/SsfAdminResource.java
Copilot AI review requested due to automatic review settings July 10, 2026 07:47
@thomasdarimont
thomasdarimont force-pushed the issue/gh-50720-ssf-redact-admin-event-payload branch from 3e9facf to c3f4cf4 Compare July 10, 2026 07:47
@thomasdarimont thomasdarimont mentioned this pull request Jul 10, 2026
Closed
Copilot AI reviewed Jul 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread ssf/services/src/main/java/org/keycloak/ssf/services/admin/SsfAdminResource.java Outdated
Copilot AI review requested due to automatic review settings July 10, 2026 08:58
@thomasdarimont thomasdarimont added the area/ssf Indicates an issue on Shared Signals Framework label Jul 10, 2026
Copilot AI reviewed Jul 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comment thread ssf/services/src/main/java/org/keycloak/ssf/services/admin/SsfAdminResource.java Outdated
Comment thread server-spi/src/main/java/org/keycloak/utils/StringUtil.java Outdated
Comment thread ssf/services/src/main/java/org/keycloak/ssf/services/admin/SsfAdminResource.java Outdated
Copilot AI review requested due to automatic review settings July 10, 2026 09:09
Copilot AI reviewed Jul 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread ssf/services/src/main/java/org/keycloak/ssf/services/admin/SsfAdminResource.java Outdated
Copilot AI review requested due to automatic review settings July 10, 2026 15:44
@thomasdarimont
thomasdarimont force-pushed the issue/gh-50720-ssf-redact-admin-event-payload branch 2 times, most recently from 9a3bd2a to 56d3d68 Compare July 10, 2026 15:44
@thomasdarimont thomasdarimont changed the title SSF: Redact event payload in synthetic emit admin events (#50720) SSF: Derive emit audit event data from typed events (#50720) Jul 10, 2026
Copilot AI reviewed Jul 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comment thread ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepEvent.java Outdated
Comment thread ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepCredentialChange.java Outdated
Copilot AI review requested due to automatic review settings July 10, 2026 15:52
@thomasdarimont
thomasdarimont force-pushed the issue/gh-50720-ssf-redact-admin-event-payload branch from 56d3d68 to c635f16 Compare July 10, 2026 15:53
Copilot AI reviewed Jul 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comment thread ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepCredentialChange.java Outdated
Copilot AI reviewed Jul 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comment thread ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepCredentialChange.java Outdated
Comment thread ...s/base/src/test/java/org/keycloak/tests/ssf/transmitter/SsfTransmitterEventEmitterTests.java
Copilot AI review requested due to automatic review settings July 10, 2026 16:29
Copilot AI reviewed Jul 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comment thread ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepCredentialChange.java
Comment thread ...s/base/src/test/java/org/keycloak/tests/ssf/transmitter/SsfTransmitterEventEmitterTests.java
Copilot AI review requested due to automatic review settings July 10, 2026 16:35
@thomasdarimont
thomasdarimont force-pushed the issue/gh-50720-ssf-redact-admin-event-payload branch from 9217237 to 1ad707d Compare July 10, 2026 16:35
Copilot AI reviewed Jul 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comment thread ...s/base/src/test/java/org/keycloak/tests/ssf/transmitter/SsfTransmitterEventEmitterTests.java
Comment thread ssf/services/src/main/java/org/keycloak/ssf/services/admin/SsfAdminResource.java Outdated
Copilot AI review requested due to automatic review settings July 10, 2026 16:42
@thomasdarimont
thomasdarimont force-pushed the issue/gh-50720-ssf-redact-admin-event-payload branch from 1ad707d to 49fdb41 Compare July 10, 2026 16:42
@thomasdarimont
thomasdarimont force-pushed the issue/gh-50720-ssf-redact-admin-event-payload branch from 49fdb41 to b8d63e8 Compare July 10, 2026 16:44
Copilot AI reviewed Jul 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings July 10, 2026 16:45
Copilot AI reviewed Jul 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

ssilvert
ssilvert reviewed Jul 16, 2026
View reviewed changes
Comment thread ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepCredentialChange.java Outdated
@Override
public Map<String, Object> createAdminDetails() {
var adminRepresentation = super.createAdminDetails();
adminRepresentation.put("credential_type", resolvedCredentialType.getType());

@ssilvert ssilvert Jul 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thomasdarimont Claude Fable found this:

The cached resolvedCredentialType is only populated inside setCredentialType(), so anything that sets credentialType without going through the setter — a subclass writing the protected field directly, a future @JsonCreator, a copy path — leaves it null and this line NPEs. It works today only because Jackson happens to prefer the setter over the @JsonProperty-annotated field, and validate() checks the string field, not the resolved enum, so it wouldn't catch the mismatch.

Worse, the audit rep is built after the SET has been dispatched (SsfAdminResource.emitEvent), so the NPE would surface as a 500 for an event that already went out, with no admin event recorded.

The cache doesn't buy anything — deriving it here removes the hidden state entirely, and fromString never returns null (falls back to CUSTOM) while validate() guarantees credentialType is non-null on this path:

Suggested change
adminRepresentation.put("credential_type", resolvedCredentialType.getType());
// fromString() collapses caller-supplied free-text credential types to the
// closed CaepCredentialType vocabulary so free-form values can't leak into
// the admin event store
adminRepresentation.put("credential_type", CaepCredentialType.fromString(credentialType).getType());

Then the resolvedCredentialType field, its getter, and the @JsonIgnore import can be dropped.

@ssilvert

ssilvert commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

@thomasdarimont Please fix conflicts

@thomasdarimont
SSF: Derive emit audit event data from typed events (keycloak#50720)
c8c40cf 
Instead of persisting the verbatim emit request payload in the admin
event, EmitEventResult now carries the validated typed SsfEvent and the
audit representation is built via the new SsfEvent.createAdminDetails()
hook. Each event class contributes only its audit-safe fields
(e.g. credential_type/change_type for credential-change, status/reason
for stream-updated, the standard CAEP claims on CaepEvent); the default
is an empty map, so free-form payload data and PII never reach the
admin event store. EventEmitterService now also rejects payloads that
do not materialise as a typed SsfEvent.

Fixes keycloak#50720

Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>
@thomasdarimont
SSF: Handle SET validation exceptions in push endpoint
0e1f586 
Compute credential_type for adminDetails ad-hoc.

Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>
Copilot AI review requested due to automatic review settings August 11, 2026 12:06
@thomasdarimont
thomasdarimont force-pushed the issue/gh-50720-ssf-redact-admin-event-payload branch from b8d63e8 to 0e1f586 Compare August 11, 2026 12:06
@thomasdarimont

thomasdarimont commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

@ssilvert I just rebased the branch and applied your suggestions while solving the conflicts.

Copilot AI reviewed Aug 11, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

ssilvert
ssilvert approved these changes Aug 11, 2026
View reviewed changes

@ssilvert ssilvert left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ssilvert
ssilvert merged commit f24a1a5 into keycloak:main Aug 11, 2026
92 checks passed
@github-actions github-actions Bot mentioned this pull request Aug 11, 2026
Open
ruchikajha95 pushed a commit to ruchikajha95/keycloak that referenced this pull request Aug 12, 2026
@thomasdarimont @ruchikajha95
SSF: Derive emit audit event data from typed events (keycloak#50720) (k…
16b0c72 
…eycloak#50728)

* SSF: Derive emit audit event data from typed events (keycloak#50720)

Instead of persisting the verbatim emit request payload in the admin
event, EmitEventResult now carries the validated typed SsfEvent and the
audit representation is built via the new SsfEvent.createAdminDetails()
hook. Each event class contributes only its audit-safe fields
(e.g. credential_type/change_type for credential-change, status/reason
for stream-updated, the standard CAEP claims on CaepEvent); the default
is an empty map, so free-form payload data and PII never reach the
admin event store. EventEmitterService now also rejects payloads that
do not materialise as a typed SsfEvent.

Fixes keycloak#50720

Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>

* SSF: Handle SET validation exceptions in push endpoint

Compute credential_type for adminDetails ad-hoc.

Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>

---------

Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@ssilvert ssilvert ssilvert approved these changes
+1 more reviewer
@sguilhen sguilhen sguilhen approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/ssf Indicates an issue on Shared Signals Framework team/core-iam

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Trusted SSF event emission persists verbatim event payload in admin events

4 participants

@thomasdarimont @ssilvert @sguilhen