From e26a5479b56dd0aa1e73cabb65c5384b0fdad985 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Fri, 10 Jul 2026 11:55:46 +0200 Subject: [PATCH 1/5] SSF: Revise SSF event structure (#50792) Move the CAEP-specific claims (subject, event_timestamp, initiating_entity, reason_admin, reason_user) from SsfEvent down to CaepEvent so the base event only carries the generic SET fields: event type, alias and the extension attributes map. Introduce shared EVENT_TYPE_BASE_URI constants for the CAEP and SSF stream event type URIs and an (eventType, alias) constructor on SsfEvent. Rework toString across the event hierarchy: SsfEvent renders the fields subclasses contribute to a LinkedHashMap via the new appendFields hook, filtering null values, quoting String values uniformly and appending the extension attributes map only when non-empty. This replaces the per-class string concatenation, which rendered unset fields as null placeholders. SsfEventsTest constructs every registry-contributed event type plus the GenericSsfEvent fallback and asserts toString neither throws nor renders unset fields. Fixes #50792 Signed-off-by: Thomas Darimont --- .../keycloak/ssf/event/GenericSsfEvent.java | 24 ++-- .../java/org/keycloak/ssf/event/SsfEvent.java | 130 +++++++----------- .../ssf/event/caep/CaepCredentialChange.java | 22 +-- .../keycloak/ssf/event/caep/CaepEvent.java | 98 +++++++++++++ .../ssf/event/caep/CaepSessionRevoked.java | 7 +- .../ssf/event/stream/SsfStreamEvent.java | 2 + .../event/stream/SsfStreamUpdatedEvent.java | 13 +- .../stream/SsfStreamVerificationEvent.java | 16 +-- .../org/keycloak/ssf/event/SsfEventsTest.java | 104 ++++++++++++++ 9 files changed, 291 insertions(+), 125 deletions(-) create mode 100644 ssf/core/src/test/java/org/keycloak/ssf/event/SsfEventsTest.java diff --git a/ssf/core/src/main/java/org/keycloak/ssf/event/GenericSsfEvent.java b/ssf/core/src/main/java/org/keycloak/ssf/event/GenericSsfEvent.java index b15bac73bf9b..87fd74172a69 100644 --- a/ssf/core/src/main/java/org/keycloak/ssf/event/GenericSsfEvent.java +++ b/ssf/core/src/main/java/org/keycloak/ssf/event/GenericSsfEvent.java @@ -5,23 +5,19 @@ */ public class GenericSsfEvent extends SsfEvent { + /** + * Required by Jackson: {@code SsfEventMapJsonDeserializer} materialises unknown + * event payloads via {@code treeToValue(eventData, GenericSsfEvent.class)} and + * sets the actual event type URI afterwards through {@link #setEventType}. + */ public GenericSsfEvent() { - super(null); + this(null); + } + + public GenericSsfEvent(String eventType) { + super(eventType); // Generic events don't have an alias by default setAlias(null); } - - @Override - public String toString() { - return "GenericSecurityEvent{" + - "subjectId=" + subjectId + - ", eventType='" + eventType + '\'' + - ", eventTimestamp=" + eventTimestamp + - ", initiatingEntity=" + initiatingEntity + - ", reasonAdmin=" + reasonAdmin + - ", reasonUser=" + reasonUser + - ", attributes=" + attributes + - '}'; - } } diff --git a/ssf/core/src/main/java/org/keycloak/ssf/event/SsfEvent.java b/ssf/core/src/main/java/org/keycloak/ssf/event/SsfEvent.java index 7695b8b14951..c38a1f66b197 100644 --- a/ssf/core/src/main/java/org/keycloak/ssf/event/SsfEvent.java +++ b/ssf/core/src/main/java/org/keycloak/ssf/event/SsfEvent.java @@ -3,117 +3,55 @@ import java.lang.reflect.Field; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; +import java.util.StringJoiner; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import org.keycloak.ssf.subject.SubjectId; -import org.keycloak.ssf.subject.SubjectIdJsonDeserializer; - import com.fasterxml.jackson.annotation.JsonAnyGetter; import com.fasterxml.jackson.annotation.JsonAnySetter; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; /** * Represents a generic SSF event. - * + *

* See: https://datatracker.ietf.org/doc/html/rfc8417 */ @JsonInclude(JsonInclude.Include.NON_NULL) public abstract class SsfEvent { + private static final ConcurrentMap, Set> DECLARED_JSON_PROPERTIES = new ConcurrentHashMap<>(); + /** * Internal (shorter) alias for the event type. */ @JsonIgnore protected String alias; - @JsonProperty("subject") - @JsonDeserialize(using = SubjectIdJsonDeserializer.class) - protected SubjectId subjectId; - - @JsonIgnore - protected String eventType; - - /** - * The time of the event (UNIX timestamp). Nullable so events that do - * not carry a timestamp — e.g. {@code ssf/event-type/verification} - * (SSF §8.1.4 carries only {@code state}) and other stream-management - * events — are omitted from the wire JSON instead of being serialized - * as {@code "event_timestamp": 0} (the default value of a primitive - * {@code long}, which Jackson always emits). - */ - @JsonProperty("event_timestamp") - protected Long eventTimestamp; - - /** - * The entity that initiated the event - */ - @JsonProperty("initiating_entity") - protected InitiatingEntity initiatingEntity; - /** - * A localized administrative message intended for logging and auditing. - * key is language code, value is message. + * The event type URI */ - @JsonProperty("reason_admin") - protected Map reasonAdmin; + @JsonIgnore + protected String eventType; /** - * A localized message intended for the end user. - * key is language code, value is message. + * Additional unmapped event-specific fields. */ - @JsonProperty("reason_user") - protected Map reasonUser; - @JsonIgnore protected Map attributes = new HashMap<>(); public SsfEvent(String eventType) { - this.eventType = eventType; - - // use the simple class name as the default alias - this.alias = getClass().getSimpleName(); - } - - public SubjectId getSubjectId() { - return subjectId; - } - - public Long getEventTimestamp() { - return eventTimestamp; - } - - public void setEventTimestamp(long eventTimestamp) { - this.eventTimestamp = eventTimestamp; + this(eventType, null); } - public InitiatingEntity getInitiatingEntity() { - return initiatingEntity; - } - - public void setInitiatingEntity(InitiatingEntity initiatingEntity) { - this.initiatingEntity = initiatingEntity; - } - - public Map getReasonAdmin() { - return reasonAdmin; - } - - public void setReasonAdmin(Map reasonAdmin) { - this.reasonAdmin = reasonAdmin; - } - - public Map getReasonUser() { - return reasonUser; - } - - public void setReasonUser(Map reasonUser) { - this.reasonUser = reasonUser; + public SsfEvent(String eventType, String alias) { + this.eventType = eventType; + // use the simple class name as the default alias + this.alias = alias == null ? getClass().getSimpleName() : alias; } public String getEventType() { @@ -141,13 +79,11 @@ public void setAttributeValue(String key, Object value) { if (declaredJsonPropertyNames(getClass()).contains(key)) { throw new IllegalArgumentException( "Custom attribute key '" + key + "' collides with a declared @JsonProperty on " - + getClass().getName()); + + getClass().getName()); } attributes.put(key, value); } - private static final ConcurrentMap, Set> DECLARED_JSON_PROPERTIES = new ConcurrentHashMap<>(); - private static Set declaredJsonPropertyNames(Class type) { return DECLARED_JSON_PROPERTIES.computeIfAbsent(type, t -> { Set names = new HashSet<>(); @@ -167,10 +103,6 @@ public void setEventType(String eventType) { this.eventType = eventType; } - public void setSubjectId(SubjectId subjectId) { - this.subjectId = subjectId; - } - public String getAlias() { return alias; } @@ -202,4 +134,36 @@ public void setAlias(String alias) { public void validate() { // no-op — overridden by event subclasses that have spec-required fields } + + @Override + public String toString() { + Map fields = new LinkedHashMap<>(); + appendFields(fields); + if (attributes != null && !attributes.isEmpty()) { + fields.putIfAbsent("attributes", attributes); + } + StringJoiner rendered = new StringJoiner(", "); + for (Map.Entry entry : fields.entrySet()) { + Object value = entry.getValue(); + if (value == null) { + continue; + } + rendered.add(entry.getKey() + "=" + (value instanceof String ? "'" + value + '\'' : value)); + } + String name = alias != null ? alias : getClass().getSimpleName(); + return rendered.length() == 0 ? name : name + "::" + rendered; + } + + /** + * Contributes the fields of this level of the event hierarchy to the + * {@link #toString()} output; insertion order is the render order. + * Subclasses override this (calling {@code super.appendFields(fields)} first) + * instead of {@code toString()} itself. Values may be put unconditionally — + * {@code null} entries are filtered and {@link String} values quoted centrally + * when rendering, and the extension {@link #attributes} map is appended + * automatically when non-empty. + */ + protected void appendFields(Map fields) { + fields.put("eventType", eventType); + } } diff --git a/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepCredentialChange.java b/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepCredentialChange.java index 7174059f222d..60f9b02be8d0 100644 --- a/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepCredentialChange.java +++ b/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepCredentialChange.java @@ -1,5 +1,7 @@ package org.keycloak.ssf.event.caep; +import java.util.Map; + import org.keycloak.ssf.event.SsfEventValidationException; import com.fasterxml.jackson.annotation.JsonCreator; @@ -15,8 +17,7 @@ public class CaepCredentialChange extends CaepEvent { /** * See: https://openid.github.io/sharedsignals/openid-caep-1_0.html#name-credential-change */ - public static final String TYPE = "https://schemas.openid.net/secevent/caep/event-type/credential-change"; - + public static final String TYPE = CaepEvent.EVENT_TYPE_BASE_URI + "credential-change"; /** * This MUST be one of the following strings, or any other credential type supported mutually by the Transmitter and the Receiver. @@ -205,14 +206,13 @@ public String getType() { } @Override - public String toString() { - return "CredentialChange{" + - "credentialType=" + credentialType + - ", changeType=" + changeType + - ", friendlyName='" + friendlyName + '\'' + - ", x509Issuer='" + x509Issuer + '\'' + - ", x509Serial='" + x509Serial + '\'' + - ", fido2Aaguid='" + fido2Aaguid + '\'' + - '}'; + protected void appendFields(Map fields) { + super.appendFields(fields); + fields.put("credentialType", credentialType); + fields.put("changeType", changeType); + fields.put("friendlyName", friendlyName); + fields.put("x509Issuer", x509Issuer); + fields.put("x509Serial", x509Serial); + fields.put("fido2Aaguid", fido2Aaguid); } } diff --git a/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepEvent.java b/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepEvent.java index 437f58855cc1..3d9f99aa9f73 100644 --- a/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepEvent.java +++ b/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepEvent.java @@ -1,6 +1,14 @@ package org.keycloak.ssf.event.caep; +import java.util.Map; + +import org.keycloak.ssf.event.InitiatingEntity; import org.keycloak.ssf.event.SsfEvent; +import org.keycloak.ssf.subject.SubjectId; +import org.keycloak.ssf.subject.SubjectIdJsonDeserializer; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; /** * Generic CaepEvent. @@ -9,7 +17,97 @@ */ public abstract class CaepEvent extends SsfEvent { + /** + * See: https://openid.net/specs/openid-caep-1_0-final.html#section-3 + */ + public static final String EVENT_TYPE_BASE_URI = "https://schemas.openid.net/secevent/caep/event-type/"; + + @JsonProperty("subject") + @JsonDeserialize(using = SubjectIdJsonDeserializer.class) + protected SubjectId subjectId; + + /** + * The time of the event (UNIX timestamp). Nullable so events that do + * not carry a timestamp — e.g. {@code ssf/event-type/verification} + * (SSF §8.1.4 carries only {@code state}) and other stream-management + * events — are omitted from the wire JSON instead of being serialized + * as {@code "event_timestamp": 0} (the default value of a primitive + * {@code long}, which Jackson always emits). + */ + @JsonProperty("event_timestamp") + protected Long eventTimestamp; + + /** + * The entity that initiated the event + */ + @JsonProperty("initiating_entity") + protected InitiatingEntity initiatingEntity; + + /** + * A localized administrative message intended for logging and auditing. + * key is language code, value is message. + */ + @JsonProperty("reason_admin") + protected Map reasonAdmin; + + /** + * A localized message intended for the end user. + * key is language code, value is message. + */ + @JsonProperty("reason_user") + protected Map reasonUser; + public CaepEvent(String type) { super(type); } + + public SubjectId getSubjectId() { + return subjectId; + } + + public void setSubjectId(SubjectId subjectId) { + this.subjectId = subjectId; + } + + public Long getEventTimestamp() { + return eventTimestamp; + } + + public void setEventTimestamp(long eventTimestamp) { + this.eventTimestamp = eventTimestamp; + } + + public InitiatingEntity getInitiatingEntity() { + return initiatingEntity; + } + + public void setInitiatingEntity(InitiatingEntity initiatingEntity) { + this.initiatingEntity = initiatingEntity; + } + + public Map getReasonAdmin() { + return reasonAdmin; + } + + public void setReasonAdmin(Map reasonAdmin) { + this.reasonAdmin = reasonAdmin; + } + + public Map getReasonUser() { + return reasonUser; + } + + public void setReasonUser(Map reasonUser) { + this.reasonUser = reasonUser; + } + + @Override + protected void appendFields(Map fields) { + super.appendFields(fields); + fields.put("subjectId", subjectId); + fields.put("eventTimestamp", eventTimestamp); + fields.put("initiatingEntity", initiatingEntity); + fields.put("reasonAdmin", reasonAdmin); + fields.put("reasonUser", reasonUser); + } } diff --git a/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepSessionRevoked.java b/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepSessionRevoked.java index 9f59bf3c45ba..68a36aa12778 100644 --- a/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepSessionRevoked.java +++ b/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepSessionRevoked.java @@ -8,14 +8,9 @@ public class CaepSessionRevoked extends CaepEvent { /** * See: https://openid.github.io/sharedsignals/openid-caep-1_0.html#name-session-revoked */ - public static final String TYPE = "https://schemas.openid.net/secevent/caep/event-type/session-revoked"; + public static final String TYPE = CaepEvent.EVENT_TYPE_BASE_URI + "session-revoked"; public CaepSessionRevoked() { super(TYPE); } - - @Override - public String toString() { - return "SessionRevoked{}"; - } } diff --git a/ssf/core/src/main/java/org/keycloak/ssf/event/stream/SsfStreamEvent.java b/ssf/core/src/main/java/org/keycloak/ssf/event/stream/SsfStreamEvent.java index 621ecb665ea0..215032acd7a1 100644 --- a/ssf/core/src/main/java/org/keycloak/ssf/event/stream/SsfStreamEvent.java +++ b/ssf/core/src/main/java/org/keycloak/ssf/event/stream/SsfStreamEvent.java @@ -7,6 +7,8 @@ */ public abstract class SsfStreamEvent extends SsfEvent { + public static final String EVENT_TYPE_BASE_URI = "https://schemas.openid.net/secevent/ssf/event-type/"; + public SsfStreamEvent(String eventType) { super(eventType); } diff --git a/ssf/core/src/main/java/org/keycloak/ssf/event/stream/SsfStreamUpdatedEvent.java b/ssf/core/src/main/java/org/keycloak/ssf/event/stream/SsfStreamUpdatedEvent.java index becb070af158..ad5800004d9e 100644 --- a/ssf/core/src/main/java/org/keycloak/ssf/event/stream/SsfStreamUpdatedEvent.java +++ b/ssf/core/src/main/java/org/keycloak/ssf/event/stream/SsfStreamUpdatedEvent.java @@ -1,17 +1,19 @@ package org.keycloak.ssf.event.stream; +import java.util.Map; + import org.keycloak.ssf.stream.StreamStatus; import com.fasterxml.jackson.annotation.JsonProperty; /** * SSF Stream status updated event. - * + *

* See: https://openid.net/specs/openid-sharedsignals-framework-1_0-final.html#name-stream-updated-event */ public class SsfStreamUpdatedEvent extends SsfStreamEvent { - public static final String TYPE = "https://schemas.openid.net/secevent/ssf/event-type/stream-updated"; + public static final String TYPE = SsfStreamEvent.EVENT_TYPE_BASE_URI + "stream-updated"; /** * REQUIRED. Defines the new status of the stream. @@ -44,4 +46,11 @@ public String getReason() { public void setReason(String reason) { this.reason = reason; } + + @Override + protected void appendFields(Map fields) { + super.appendFields(fields); + fields.put("status", status); + fields.put("reason", reason); + } } diff --git a/ssf/core/src/main/java/org/keycloak/ssf/event/stream/SsfStreamVerificationEvent.java b/ssf/core/src/main/java/org/keycloak/ssf/event/stream/SsfStreamVerificationEvent.java index 8cce81fc14ef..ec31635e2d1a 100644 --- a/ssf/core/src/main/java/org/keycloak/ssf/event/stream/SsfStreamVerificationEvent.java +++ b/ssf/core/src/main/java/org/keycloak/ssf/event/stream/SsfStreamVerificationEvent.java @@ -1,15 +1,17 @@ package org.keycloak.ssf.event.stream; +import java.util.Map; + import com.fasterxml.jackson.annotation.JsonProperty; /** * SSF Verification event. - * + *

* See: https://openid.net/specs/openid-sharedsignals-framework-1_0-final.html#name-verification */ public class SsfStreamVerificationEvent extends SsfStreamEvent { - public static final String TYPE = "https://schemas.openid.net/secevent/ssf/event-type/verification"; + public static final String TYPE = SsfStreamEvent.EVENT_TYPE_BASE_URI + "verification"; @JsonProperty("state") protected String state; @@ -27,12 +29,8 @@ public void setState(String state) { } @Override - public String toString() { - // Render absent state as an empty object rather than "state='null'" - // so the log representation matches what Jackson actually puts on - // the wire (omitted thanks to @JsonInclude(NON_NULL)). - return state == null - ? "VerificationEvent{}" - : "VerificationEvent{state='" + state + "'}"; + protected void appendFields(Map fields) { + super.appendFields(fields); + fields.put("state", state); } } diff --git a/ssf/core/src/test/java/org/keycloak/ssf/event/SsfEventsTest.java b/ssf/core/src/test/java/org/keycloak/ssf/event/SsfEventsTest.java new file mode 100644 index 000000000000..3cf9a817f961 --- /dev/null +++ b/ssf/core/src/test/java/org/keycloak/ssf/event/SsfEventsTest.java @@ -0,0 +1,104 @@ +package org.keycloak.ssf.event; + +import java.util.Map; +import java.util.function.Supplier; + +import org.keycloak.ssf.event.caep.CaepCredentialChange; +import org.keycloak.ssf.event.token.SsfSecurityEventToken; +import org.keycloak.util.JsonSerialization; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class SsfEventsTest { + + @Test + void builtInEvents_toStringOnFreshInstanceDoesNotThrow() { + Map> factories = + new DefaultSsfEventProviderFactory().getContributedEventFactories(); + assertFalse(factories.isEmpty(), "factory should contribute the built-in events"); + + for (Map.Entry> entry : factories.entrySet()) { + SsfEvent event = entry.getValue().get(); + String rendered = assertDoesNotThrow(event::toString, + () -> "toString() must not throw for fresh " + entry.getKey()); + assertNotNull(rendered, () -> "toString() must not return null for " + entry.getKey()); + assertFalse(rendered.isBlank(), () -> "toString() must not be blank for " + entry.getKey()); + assertFalse(rendered.contains("null"), + () -> "unset fields must be omitted, not rendered as null, for " + + entry.getKey() + ": " + rendered); + } + } + + @Test + void unknownEventType_isRepresentedAsGenericSsfEvent() throws Exception { + // A SET carrying an event type URI that is not in the registry: + // SsfEventMapJsonDeserializer falls back to GenericSsfEvent, so the + // full payload lands in the @JsonAnySetter extension attributes and + // the real type URI is set after materialisation. + String unknownType = "https://example.com/secevent/custom/unknown-event"; + String setJson = """ + { + "jti": "jti-123", + "iss": "https://issuer.example.test", + "events": { + "%s": { + "foo": "bar", + "count": 3, + "nested": { "a": 1 } + } + } + } + """.formatted(unknownType); + + SsfSecurityEventToken token = JsonSerialization.readValue(setJson, SsfSecurityEventToken.class); + + Object event = token.getEvents().get(unknownType); + GenericSsfEvent generic = assertInstanceOf(GenericSsfEvent.class, event, + "unknown event types must degrade to GenericSsfEvent, not fail parsing"); + assertEquals(unknownType, generic.getEventType(), + "the deserializer must preserve the original event type URI"); + assertEquals("bar", generic.getAttributes().get("foo"), + "payload fields must be preserved in the extension attributes"); + assertEquals(3, generic.getAttributes().get("count")); + assertEquals(Map.of("a", 1), generic.getAttributes().get("nested"), + "nested payload objects must be preserved as maps"); + + String rendered = assertDoesNotThrow(generic::toString); + assertTrue(rendered.contains(unknownType), + "toString should render the preserved event type URI: " + rendered); + assertTrue(rendered.contains("foo=bar"), + "toString should render the preserved attributes: " + rendered); + } + + @Test + void genericEvent_toStringOnFreshInstanceDoesNotThrow() { + // GenericSsfEvent is the unknown-type fallback and is not part of the + // factory contributions, so it is covered explicitly. + GenericSsfEvent event = new GenericSsfEvent(); + String rendered = assertDoesNotThrow(event::toString); + assertNotNull(rendered); + assertFalse(rendered.isBlank()); + } + + @Test + void caepEvent_toStringRendersOnlySetFields() { + CaepCredentialChange event = new CaepCredentialChange(); + event.setCredentialType("password"); + event.setChangeType(CaepCredentialChange.ChangeType.UPDATE); + + String rendered = event.toString(); + assertTrue(rendered.contains("credentialType='password'"), + "set String fields must be rendered quoted: " + rendered); + assertTrue(rendered.contains("changeType="), + "set fields must be rendered: " + rendered); + assertFalse(rendered.contains("null"), + "unset optional fields (friendlyName, x509*, …) must be omitted: " + rendered); + } +} From d235bc917d7bb1df4e1e8c50e99174a68b8ac134 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Fri, 10 Jul 2026 16:33:37 +0200 Subject: [PATCH 2/5] SSF: Make spotless happy Signed-off-by: Thomas Darimont --- .../src/main/java/org/keycloak/ssf/event/SsfEvent.java | 10 +++++++++- .../java/org/keycloak/ssf/event/caep/CaepEvent.java | 9 +++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/ssf/core/src/main/java/org/keycloak/ssf/event/SsfEvent.java b/ssf/core/src/main/java/org/keycloak/ssf/event/SsfEvent.java index c38a1f66b197..93bd62dc5f6f 100644 --- a/ssf/core/src/main/java/org/keycloak/ssf/event/SsfEvent.java +++ b/ssf/core/src/main/java/org/keycloak/ssf/event/SsfEvent.java @@ -140,7 +140,15 @@ public String toString() { Map fields = new LinkedHashMap<>(); appendFields(fields); if (attributes != null && !attributes.isEmpty()) { - fields.putIfAbsent("attributes", attributes); + Map renderedAttributes = new LinkedHashMap<>(); + for (var entry : attributes.entrySet()) { + if (entry.getValue() != null) { + renderedAttributes.put(entry.getKey(), entry.getValue()); + } + } + if (!renderedAttributes.isEmpty()) { + fields.putIfAbsent("attributes", renderedAttributes); + } } StringJoiner rendered = new StringJoiner(", "); for (Map.Entry entry : fields.entrySet()) { diff --git a/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepEvent.java b/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepEvent.java index 3d9f99aa9f73..9d330c9ea05f 100644 --- a/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepEvent.java +++ b/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepEvent.java @@ -27,12 +27,9 @@ public abstract class CaepEvent extends SsfEvent { protected SubjectId subjectId; /** - * The time of the event (UNIX timestamp). Nullable so events that do - * not carry a timestamp — e.g. {@code ssf/event-type/verification} - * (SSF §8.1.4 carries only {@code state}) and other stream-management - * events — are omitted from the wire JSON instead of being serialized - * as {@code "event_timestamp": 0} (the default value of a primitive - * {@code long}, which Jackson always emits). + * The time of the event (UNIX timestamp). Nullable so an absent timestamp + * is omitted from wire JSON rather than serialized as + * {@code "event_timestamp": 0}. */ @JsonProperty("event_timestamp") protected Long eventTimestamp; From 96a49bda44e7fcc654c52084aee198bb5edc4c11 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Fri, 10 Jul 2026 18:32:09 +0200 Subject: [PATCH 3/5] SSF: Add additional test to verify null-valued attributes in GenericEvents Signed-off-by: Thomas Darimont --- .../src/test/java/org/keycloak/ssf/event/SsfEventsTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ssf/core/src/test/java/org/keycloak/ssf/event/SsfEventsTest.java b/ssf/core/src/test/java/org/keycloak/ssf/event/SsfEventsTest.java index 3cf9a817f961..08da67e9484c 100644 --- a/ssf/core/src/test/java/org/keycloak/ssf/event/SsfEventsTest.java +++ b/ssf/core/src/test/java/org/keycloak/ssf/event/SsfEventsTest.java @@ -70,11 +70,15 @@ void unknownEventType_isRepresentedAsGenericSsfEvent() throws Exception { assertEquals(Map.of("a", 1), generic.getAttributes().get("nested"), "nested payload objects must be preserved as maps"); + generic.setAttributeValue("unset", null); + String rendered = assertDoesNotThrow(generic::toString); assertTrue(rendered.contains(unknownType), "toString should render the preserved event type URI: " + rendered); assertTrue(rendered.contains("foo=bar"), "toString should render the preserved attributes: " + rendered); + assertFalse(rendered.contains("unset="), + "toString should omit null-valued attributes: " + rendered); } @Test From a9b8f2d7f3120477a9559ea4e771ec0c570d6f1c Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Thu, 6 Aug 2026 09:56:23 +0200 Subject: [PATCH 4/5] SSF: Add javadoc on compatibility to CaepEvent.subjectId. Signed-off-by: Thomas Darimont --- .../main/java/org/keycloak/ssf/event/caep/CaepEvent.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepEvent.java b/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepEvent.java index 9d330c9ea05f..6738196e99ce 100644 --- a/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepEvent.java +++ b/ssf/core/src/main/java/org/keycloak/ssf/event/caep/CaepEvent.java @@ -22,6 +22,14 @@ public abstract class CaepEvent extends SsfEvent { */ public static final String EVENT_TYPE_BASE_URI = "https://schemas.openid.net/secevent/caep/event-type/"; + /** + * The legacy CAEP SSE subject attribute for backwards compatibility. In the OpenID Shared Signals and Events Framework Specification 1.0 - draft 01 + * the subject attribute was used to identify the subject of the event. + * See: https://openid.net/specs/openid-sse-framework-1_0-ID1.html#rfc.section.3.3 + * + * In SSF 1.0 the subject is encoded as the "sub_id" claim in the Security Event Token (SET). + * See: https://openid.github.io/sharedsignals/openid-sharedsignals-framework-1_0.html#section-3.1 + */ @JsonProperty("subject") @JsonDeserialize(using = SubjectIdJsonDeserializer.class) protected SubjectId subjectId; From 9165e81572240a90b70a5be4bf9777657d317aef Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Thu, 6 Aug 2026 10:27:01 +0200 Subject: [PATCH 5/5] SSF: Address review feedback on event structure - Render toString in the conventional Name{...} shape and quote String values recursively at every nesting level, including extension attributes and reason_admin/reason_user entries - Tighten toString test assertions to "=null" and genuinely cover the unknown-type registry fallback via a stubbed resolveRegistry(), renaming the no-session degradation test to what it actually asserts Signed-off-by: Thomas Darimont --- .../java/org/keycloak/ssf/event/SsfEvent.java | 38 +++++++++-- .../org/keycloak/ssf/event/SsfEventsTest.java | 66 ++++++++++++++++--- 2 files changed, 90 insertions(+), 14 deletions(-) diff --git a/ssf/core/src/main/java/org/keycloak/ssf/event/SsfEvent.java b/ssf/core/src/main/java/org/keycloak/ssf/event/SsfEvent.java index 93bd62dc5f6f..66afdf461d5d 100644 --- a/ssf/core/src/main/java/org/keycloak/ssf/event/SsfEvent.java +++ b/ssf/core/src/main/java/org/keycloak/ssf/event/SsfEvent.java @@ -151,15 +151,43 @@ public String toString() { } } StringJoiner rendered = new StringJoiner(", "); - for (Map.Entry entry : fields.entrySet()) { + for (var entry : fields.entrySet()) { Object value = entry.getValue(); if (value == null) { continue; } - rendered.add(entry.getKey() + "=" + (value instanceof String ? "'" + value + '\'' : value)); + rendered.add(entry.getKey() + "=" + render(value)); } String name = alias != null ? alias : getClass().getSimpleName(); - return rendered.length() == 0 ? name : name + "::" + rendered; + return rendered.length() == 0 ? name : name + "{" + rendered + "}"; + } + + /** + * Renders a field value for {@link #toString()}, quoting {@link String} + * values at every nesting level so entries inside maps (extension + * {@link #attributes}, {@code reason_admin} / {@code reason_user}) read the + * same as top-level fields. Values originate from JSON payloads, which + * cannot form cycles, so the recursion is bounded. + */ + private static String render(Object value) { + if (value instanceof String) { + return "'" + value + '\''; + } + if (value instanceof Map map) { + StringJoiner joiner = new StringJoiner(", ", "{", "}"); + for (var entry : map.entrySet()) { + joiner.add(entry.getKey() + "=" + render(entry.getValue())); + } + return joiner.toString(); + } + if (value instanceof Iterable iterable) { + StringJoiner joiner = new StringJoiner(", ", "[", "]"); + for (var element : iterable) { + joiner.add(render(element)); + } + return joiner.toString(); + } + return String.valueOf(value); } /** @@ -168,8 +196,8 @@ public String toString() { * Subclasses override this (calling {@code super.appendFields(fields)} first) * instead of {@code toString()} itself. Values may be put unconditionally — * {@code null} entries are filtered and {@link String} values quoted centrally - * when rendering, and the extension {@link #attributes} map is appended - * automatically when non-empty. + * at every nesting level when rendering, and the extension {@link #attributes} + * map is appended automatically when non-empty. */ protected void appendFields(Map fields) { fields.put("eventType", eventType); diff --git a/ssf/core/src/test/java/org/keycloak/ssf/event/SsfEventsTest.java b/ssf/core/src/test/java/org/keycloak/ssf/event/SsfEventsTest.java index 08da67e9484c..81aa15d5650d 100644 --- a/ssf/core/src/test/java/org/keycloak/ssf/event/SsfEventsTest.java +++ b/ssf/core/src/test/java/org/keycloak/ssf/event/SsfEventsTest.java @@ -1,12 +1,16 @@ package org.keycloak.ssf.event; +import java.util.List; import java.util.Map; import java.util.function.Supplier; import org.keycloak.ssf.event.caep.CaepCredentialChange; +import org.keycloak.ssf.event.token.SsfEventMapJsonDeserializer; import org.keycloak.ssf.event.token.SsfSecurityEventToken; import org.keycloak.util.JsonSerialization; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; @@ -30,18 +34,21 @@ void builtInEvents_toStringOnFreshInstanceDoesNotThrow() { () -> "toString() must not throw for fresh " + entry.getKey()); assertNotNull(rendered, () -> "toString() must not return null for " + entry.getKey()); assertFalse(rendered.isBlank(), () -> "toString() must not be blank for " + entry.getKey()); - assertFalse(rendered.contains("null"), + assertFalse(rendered.contains("=null"), () -> "unset fields must be omitted, not rendered as null, for " + entry.getKey() + ": " + rendered); } } @Test - void unknownEventType_isRepresentedAsGenericSsfEvent() throws Exception { - // A SET carrying an event type URI that is not in the registry: - // SsfEventMapJsonDeserializer falls back to GenericSsfEvent, so the - // full payload lands in the @JsonAnySetter extension attributes and - // the real type URI is set after materialisation. + void withoutBoundSession_setParsingDegradesToGenericSsfEvent() throws Exception { + // Outside a request scope no KeycloakSession is bound, so + // SsfEventMapJsonDeserializer.resolveRegistry() returns null and every + // event type degrades to GenericSsfEvent (documented contract of + // resolveRegistry): the full payload lands in the @JsonAnySetter + // extension attributes and the real type URI is set after + // materialisation. The registry-bound unknown-type fallback is covered + // separately below. String unknownType = "https://example.com/secevent/custom/unknown-event"; String setJson = """ { @@ -75,12 +82,53 @@ void unknownEventType_isRepresentedAsGenericSsfEvent() throws Exception { String rendered = assertDoesNotThrow(generic::toString); assertTrue(rendered.contains(unknownType), "toString should render the preserved event type URI: " + rendered); - assertTrue(rendered.contains("foo=bar"), - "toString should render the preserved attributes: " + rendered); + assertTrue(rendered.contains("foo='bar'"), + "toString should render attribute Strings quoted like top-level fields: " + rendered); + assertTrue(rendered.contains("nested={a=1}"), + "toString should render nested maps recursively: " + rendered); assertFalse(rendered.contains("unset="), "toString should omit null-valued attributes: " + rendered); } + @Test + void registryBound_unknownTypeFallsBackToGenericWhileKnownTypeResolvesTyped() throws Exception { + // With a registry bound (stubbed resolveRegistry, same hook the + // per-session provider uses), a contributed type must resolve to its + // typed event class and only genuinely unknown types may fall back to + // GenericSsfEvent — the branch the no-session test above cannot cover. + SsfEventRegistry registry = SsfEventRegistry.from(List.of(new DefaultSsfEventProviderFactory())); + SsfEventMapJsonDeserializer deserializer = new SsfEventMapJsonDeserializer() { + @Override + protected SsfEventRegistry resolveRegistry() { + return registry; + } + }; + + String unknownType = "https://example.com/secevent/custom/unknown-event"; + String eventsJson = """ + { + "%s": { "credential_type": "password" }, + "%s": { "foo": "bar" } + } + """.formatted(CaepCredentialChange.TYPE, unknownType); + + ObjectMapper mapper = new ObjectMapper(); + Map events; + try (JsonParser parser = mapper.createParser(eventsJson)) { + events = deserializer.deserialize(parser, null); + } + + CaepCredentialChange known = assertInstanceOf(CaepCredentialChange.class, + events.get(CaepCredentialChange.TYPE), + "registry-contributed types must resolve to their typed event class"); + assertEquals("password", known.getCredentialType()); + + GenericSsfEvent generic = assertInstanceOf(GenericSsfEvent.class, events.get(unknownType), + "only unknown event types may fall back to GenericSsfEvent"); + assertEquals(unknownType, generic.getEventType()); + assertEquals("bar", generic.getAttributes().get("foo")); + } + @Test void genericEvent_toStringOnFreshInstanceDoesNotThrow() { // GenericSsfEvent is the unknown-type fallback and is not part of the @@ -102,7 +150,7 @@ void caepEvent_toStringRendersOnlySetFields() { "set String fields must be rendered quoted: " + rendered); assertTrue(rendered.contains("changeType="), "set fields must be rendered: " + rendered); - assertFalse(rendered.contains("null"), + assertFalse(rendered.contains("=null"), "unset optional fields (friendlyName, x509*, …) must be omitted: " + rendered); } }