Skip to content

Commit 006db4b

Browse files
thjaeckleclaude
thjaeckle
and
claude
committed
fix partial-access filter stripping thingId and audit fields from SSE/WebSocket events
The 3.9.0 partial-events feature (PR #2287) filtered every top-level field of the emitted Thing JSON by exact path match, which dropped entity-identity and audit fields the subscriber needs to make sense of the event. A subscriber with READ on thing:/attributes received SSE events without thingId, breaking correlation; WebSocket subscribers lost the same fields from the payload of full-thing events and from explicitly-requested extraFields. Always preserve thingId, _namespace, _revision, _modified and _created when the subscriber holds at least partial read access on the thing: * ThingsSseRouteBuilder.filterJsonByPartialAccessPaths injects the allowlist into accessiblePaths before invoking JsonPartialAccessFilter. * AdaptablePartialAccessFilter applies the same allowlist via a shared helper in both filterAdaptablePayload (full-thing events) and the extras filter. * THING_QUERY_COMMAND_RESPONSE_ALLOWLIST in ThingCommandEnforcement is widened from thingId alone to the same five fields so RetrieveThing and the streaming filters stay in sync. policyId remains policy-enforced and is intentionally NOT in the allowlist (it identifies the gating Policy entity). _metadata is also intentionally omitted: it is per-path policy-enforced today and auto-including it would expose data RetrieveThing does not. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b2cffa4 commit 006db4b

6 files changed

Lines changed: 205 additions & 11 deletions

File tree

‎gateway/service/src/main/java/org/eclipse/ditto/gateway/service/endpoints/routes/sse/ThingsSseRouteBuilder.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.Collection;
2525
import java.util.Collections;
2626
import java.util.HashMap;
27+
import java.util.HashSet;
2728
import java.util.List;
2829
import java.util.Map;
2930
import java.util.Optional;
@@ -90,6 +91,7 @@
9091
import org.eclipse.ditto.internal.utils.config.ScopedConfig;
9192
import org.eclipse.ditto.internal.utils.metrics.DittoMetrics;
9293
import org.eclipse.ditto.internal.utils.metrics.instruments.counter.Counter;
94+
import org.eclipse.ditto.internal.utils.protocol.AdaptablePartialAccessFilter;
9395
import org.eclipse.ditto.internal.utils.protocol.JsonPartialAccessFilter;
9496
import org.eclipse.ditto.internal.utils.protocol.PartialAccessPathResolver;
9597
import org.eclipse.ditto.internal.utils.pubsub.StreamingType;
@@ -914,7 +916,8 @@ private JsonObject filterJsonByPartialAccessPaths(final JsonObject thingJson, fi
914916
return JsonFactory.newObject();
915917
}
916918

917-
final Set<JsonPointer> accessiblePaths = result.getAccessiblePaths();
919+
final Set<JsonPointer> accessiblePaths = new HashSet<>(result.getAccessiblePaths());
920+
accessiblePaths.addAll(AdaptablePartialAccessFilter.ALWAYS_INCLUDED_THING_FIELDS);
918921
return JsonPartialAccessFilter.filterJsonByPaths(thingJson, accessiblePaths);
919922
}
920923

‎gateway/service/src/test/java/org/eclipse/ditto/gateway/service/endpoints/routes/sse/ThingsSseRouteBuilderTest.java‎

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,12 @@ public void filterJsonByPartialAccessPathsFiltersCorrectlyForPartialReader() thr
365365

366366
final JsonObject filtered = filterJsonByPartialAccessPaths(thing, event, subscriberAuthContext);
367367

368-
assertThat(filtered.getValue("thingId")).isEmpty(); // Not in accessible paths
368+
// thingId is an entity-identity field that must always be visible to any subscriber
369+
// holding at least partial read access. _namespace/_revision/_modified/_created are
370+
// verified separately in filterJsonByPartialAccessPathsPreservesAuditFieldsForPartialReader
371+
// (they only appear in the SSE payload when the user explicitly selects them via
372+
// ?fields=... since they are HIDDEN fields by default).
373+
assertThat(filtered.getValue("thingId")).contains(JsonValue.of("test:thing"));
369374
assertThat(filtered.getValue("attributes")).isPresent();
370375
assertThat(filtered.getValue("attributes").get().asObject().getValue("foo")).isPresent();
371376
assertThat(filtered.getValue("features")).isPresent();
@@ -377,6 +382,49 @@ public void filterJsonByPartialAccessPathsFiltersCorrectlyForPartialReader() thr
377382
assertThat(filtered.getValue("policyId")).isEmpty();
378383
}
379384

385+
@Test
386+
public void filterJsonByPartialAccessPathsPreservesAuditFieldsForPartialReader() throws Exception {
387+
final java.time.Instant created = java.time.Instant.parse("2026-01-01T00:00:00Z");
388+
final java.time.Instant modified = java.time.Instant.parse("2026-06-01T12:00:00Z");
389+
final Thing thing = Thing.newBuilder()
390+
.setId(ThingId.of("test:thing"))
391+
.setPolicyId(org.eclipse.ditto.policies.model.PolicyId.of("test:policy"))
392+
.setRevision(42L)
393+
.setCreated(created)
394+
.setModified(modified)
395+
.setAttributes(Attributes.newBuilder().set("foo", "bar").build())
396+
.build();
397+
final AuthorizationSubject partialReader = AuthorizationSubject.newInstance("test:partial-reader");
398+
399+
final DittoHeaders headers = DittoHeaders.newBuilder()
400+
.putHeader(DittoHeaderDefinition.PARTIAL_ACCESS_PATHS.getKey(),
401+
"{\"subjects\":[\"test:partial-reader\"],\"paths\":{\"attributes\":[0]}}")
402+
.readGrantedSubjects(Collections.singletonList(partialReader))
403+
.build();
404+
405+
final ThingModifiedEvent<?> event = ThingModified.of(thing, 42L, modified, headers, null);
406+
407+
final AuthorizationContext subscriberAuthContext = AuthorizationContext.newInstance(
408+
DittoAuthorizationContextType.UNSPECIFIED, partialReader);
409+
410+
// Build the thing JSON with SPECIAL fields included, mirroring the production path where a
411+
// caller has explicitly opted into hidden identity/audit fields via ?fields=_revision,...
412+
final JsonObject thingJson = thing.toJson(org.eclipse.ditto.base.model.json.JsonSchemaVersion.LATEST,
413+
org.eclipse.ditto.base.model.json.FieldType.regularOrSpecial());
414+
final JsonObject filtered = filterJsonByPartialAccessPaths(thingJson, event, subscriberAuthContext);
415+
416+
// Identity + audit fields must pass through even though the policy only grants /attributes:
417+
assertThat(filtered.getValue("thingId")).contains(JsonValue.of("test:thing"));
418+
assertThat(filtered.getValue("_namespace")).contains(JsonValue.of("test"));
419+
assertThat(filtered.getValue("_revision")).contains(JsonValue.of(42L));
420+
assertThat(filtered.getValue("_modified")).contains(JsonValue.of(modified.toString()));
421+
assertThat(filtered.getValue("_created")).contains(JsonValue.of(created.toString()));
422+
// policyId is not allowlisted and must remain filtered out:
423+
assertThat(filtered.getValue("policyId")).isEmpty();
424+
// The actually-granted resource is present:
425+
assertThat(filtered.getValue("attributes")).isPresent();
426+
}
427+
380428
@Test
381429
public void filterJsonByPartialAccessPathsReturnsFullPayloadForUnrestrictedReader() throws Exception {
382430
final Thing thing = Thing.newBuilder().setId(ThingId.of("test:thing")).build();
@@ -480,6 +528,12 @@ public void filterJsonByPartialAccessPathsReturnsOriginalWhenNoHeader() throws E
480528
private JsonObject filterJsonByPartialAccessPaths(final Thing thing,
481529
final ThingModifiedEvent<?> event,
482530
final AuthorizationContext subscriberAuthContext) throws Exception {
531+
return filterJsonByPartialAccessPaths(thing.toJson(), event, subscriberAuthContext);
532+
}
533+
534+
private JsonObject filterJsonByPartialAccessPaths(final JsonObject thingJson,
535+
final ThingModifiedEvent<?> event,
536+
final AuthorizationContext subscriberAuthContext) throws Exception {
483537
final ThingsSseRouteBuilder routeBuilder = ThingsSseRouteBuilder.getInstance(
484538
actorSystem, streamingActor.ref(), streamingConfig, proxyActor.ref(), HeaderTranslator.empty());
485539
final Method filterMethod = ThingsSseRouteBuilder.class.getDeclaredMethod(
@@ -489,7 +543,6 @@ private JsonObject filterJsonByPartialAccessPaths(final Thing thing,
489543
AuthorizationContext.class);
490544
filterMethod.setAccessible(true);
491545

492-
final JsonObject thingJson = thing.toJson();
493546
return (JsonObject) filterMethod.invoke(routeBuilder, thingJson, event, subscriberAuthContext);
494547
}
495548

‎internal/utils/protocol/src/main/java/org/eclipse/ditto/internal/utils/protocol/AdaptablePartialAccessFilter.java‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*/
1313
package org.eclipse.ditto.internal.utils.protocol;
1414

15+
import java.util.HashSet;
1516
import java.util.List;
1617
import java.util.Map;
1718
import java.util.Optional;
@@ -37,6 +38,22 @@
3738
*/
3839
public final class AdaptablePartialAccessFilter {
3940

41+
/**
42+
* Thing identity and audit fields ({@code thingId}, {@code _namespace}, {@code _revision},
43+
* {@code _modified}, {@code _created}) that must remain visible to any subscriber holding at
44+
* least partial read access on the thing. These are field names from the Ditto Protocol
45+
* (see {@code org.eclipse.ditto.things.model.Thing.JsonFields}) — they are hardcoded here
46+
* rather than imported to keep the {@code internal/utils/protocol} module free of a
47+
* dependency on {@code things/model}.
48+
*/
49+
public static final Set<JsonPointer> ALWAYS_INCLUDED_THING_FIELDS = Set.of(
50+
JsonPointer.of("thingId"),
51+
JsonPointer.of("_namespace"),
52+
JsonPointer.of("_revision"),
53+
JsonPointer.of("_modified"),
54+
JsonPointer.of("_created")
55+
);
56+
4057
private AdaptablePartialAccessFilter() {
4158
// No instantiation
4259
}
@@ -162,8 +179,11 @@ private static Adaptable filterAdaptablePayload(
162179
.map(JsonValue::asObject)
163180
.orElse(JsonFactory.newObject());
164181

165-
final JsonObject filteredPayload = JsonPartialAccessFilter.filterJsonByPaths(
166-
originalPayload, accessiblePaths);
182+
// ThingCreated/ThingModified/ThingMerged events at path "/" carry the full thing JSON
183+
// here; stripping the identity/audit fields would make the protocol message hard to use
184+
// for the partial reader. For events with non-thing-rooted object payloads the allowlist
185+
// pointers simply don't exist in the payload and the addition is a no-op.
186+
final JsonObject filteredPayload = filterJsonByPathsWithAllowlist(originalPayload, accessiblePaths);
167187

168188
final PayloadBuilder payloadBuilder = Payload.newBuilder(adaptable.getPayload())
169189
.withValue(filteredPayload);
@@ -174,6 +194,18 @@ private static Adaptable filterAdaptablePayload(
174194
.build();
175195
}
176196

197+
/**
198+
* Filters {@code jsonObject} by {@code accessiblePaths}, with the thing-identity/audit
199+
* allowlist ({@link #ALWAYS_INCLUDED_THING_FIELDS}) implicitly added. Used for both the
200+
* payload value of full-thing events and for user-requested extra fields.
201+
*/
202+
private static JsonObject filterJsonByPathsWithAllowlist(final JsonObject jsonObject,
203+
final Set<JsonPointer> accessiblePaths) {
204+
final Set<JsonPointer> pathsWithAllowlist = new HashSet<>(accessiblePaths);
205+
pathsWithAllowlist.addAll(ALWAYS_INCLUDED_THING_FIELDS);
206+
return JsonPartialAccessFilter.filterJsonByPaths(jsonObject, pathsWithAllowlist);
207+
}
208+
177209

178210
private static Adaptable filterExtraFields(
179211
final Adaptable adaptable,
@@ -198,8 +230,11 @@ private static void filterExtraFieldsIntoBuilder(
198230
return;
199231
}
200232

201-
final JsonObject filteredExtra = JsonPartialAccessFilter.filterJsonByPaths(
202-
originalExtra.get(), accessiblePaths);
233+
// The user explicitly opted into receiving these extra fields via the extraFields/fields
234+
// parameter; identity and audit fields the subscriber would otherwise lose to per-path
235+
// filtering must still come through (matches the SSE Thing-JSON contract and pre-3.9.0
236+
// behaviour for WebSocket extras).
237+
final JsonObject filteredExtra = filterJsonByPathsWithAllowlist(originalExtra.get(), accessiblePaths);
203238

204239
if (filteredExtra.isEmpty()) {
205240
return;

‎internal/utils/protocol/src/test/java/org/eclipse/ditto/internal/utils/protocol/AdaptablePartialAccessFilterTest.java‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -723,6 +723,96 @@ public void filtersExtraFieldsUsingFilterAdaptableWithResult() {
723723
assertThat(extraObj.getValue(JsonPointer.of("/attributes/private"))).isEmpty();
724724
}
725725

726+
@Test
727+
public void payloadFilterPreservesIdentityAndAuditFieldsForPartialAccess() {
728+
// GIVEN: a ThingMerged-style event at path "/" carrying the full thing in the payload,
729+
// and a partial reader granted only /attributes/public.
730+
final DittoHeaders headers = DittoHeaders.newBuilder()
731+
.putHeader(DittoHeaderDefinition.PARTIAL_ACCESS_PATHS.getKey(), PARTIAL_ACCESS_HEADER)
732+
.readGrantedSubjects(Set.of(SUBJECT_PARTIAL))
733+
.build();
734+
735+
final JsonObject fullThingPayload = JsonFactory.newObjectBuilder()
736+
.set("thingId", JsonValue.of("org.eclipse.ditto.test:thing"))
737+
.set("_namespace", JsonValue.of("org.eclipse.ditto.test"))
738+
.set("_revision", JsonValue.of(7L))
739+
.set("_modified", JsonValue.of("2026-06-01T12:00:00Z"))
740+
.set("_created", JsonValue.of("2026-01-01T00:00:00Z"))
741+
.set("policyId", JsonValue.of("org.eclipse.ditto.test:policy"))
742+
.set("attributes", JsonFactory.newObjectBuilder()
743+
.set("public", JsonValue.of("public-value"))
744+
.set("private", JsonValue.of("private-value"))
745+
.build())
746+
.build();
747+
748+
final Adaptable adaptable = createThingEventAdaptable(fullThingPayload, headers);
749+
final AuthorizationContext context = authContext(SUBJECT_PARTIAL);
750+
751+
// WHEN
752+
final Adaptable result = AdaptablePartialAccessFilter.filterAdaptableForPartialAccess(adaptable, context);
753+
754+
// THEN: identity + audit fields survive in the payload even though the policy only
755+
// grants /attributes/public.
756+
final JsonObject filteredPayload = result.getPayload().getValue()
757+
.filter(JsonValue::isObject)
758+
.map(JsonValue::asObject)
759+
.orElse(JsonFactory.newObject());
760+
assertThat(filteredPayload.getValue("thingId")).contains(JsonValue.of("org.eclipse.ditto.test:thing"));
761+
assertThat(filteredPayload.getValue("_namespace")).contains(JsonValue.of("org.eclipse.ditto.test"));
762+
assertThat(filteredPayload.getValue("_revision")).contains(JsonValue.of(7L));
763+
assertThat(filteredPayload.getValue("_modified")).contains(JsonValue.of("2026-06-01T12:00:00Z"));
764+
assertThat(filteredPayload.getValue("_created")).contains(JsonValue.of("2026-01-01T00:00:00Z"));
765+
// policyId is NOT in the allowlist — must remain filtered out.
766+
assertThat(filteredPayload.getValue("policyId")).isEmpty();
767+
// The actually-granted path is present, the ungranted sibling is dropped.
768+
assertThat(filteredPayload.getValue(JsonPointer.of("/attributes/public"))).isPresent();
769+
assertThat(filteredPayload.getValue(JsonPointer.of("/attributes/private"))).isEmpty();
770+
}
771+
772+
@Test
773+
public void extrasFilterPreservesIdentityAndAuditFieldsForPartialAccess() {
774+
// GIVEN: partial reader is granted only /attributes/public, and explicitly requested
775+
// extraFields covering thingId, audit metadata, the granted path, and a non-granted path.
776+
final DittoHeaders headers = DittoHeaders.newBuilder()
777+
.putHeader(DittoHeaderDefinition.PARTIAL_ACCESS_PATHS.getKey(), PARTIAL_ACCESS_HEADER)
778+
.readGrantedSubjects(Set.of(SUBJECT_PARTIAL))
779+
.build();
780+
781+
final JsonObject extraFields = JsonFactory.newObjectBuilder()
782+
.set("thingId", JsonValue.of("org.eclipse.ditto.test:thing"))
783+
.set("_namespace", JsonValue.of("org.eclipse.ditto.test"))
784+
.set("_revision", JsonValue.of(7L))
785+
.set("_modified", JsonValue.of("2026-06-01T12:00:00Z"))
786+
.set("_created", JsonValue.of("2026-01-01T00:00:00Z"))
787+
.set("policyId", JsonValue.of("org.eclipse.ditto.test:policy"))
788+
.set("attributes", JsonFactory.newObjectBuilder()
789+
.set("public", JsonValue.of("public-value"))
790+
.set("private", JsonValue.of("private-value"))
791+
.build())
792+
.build();
793+
794+
final Adaptable adaptable = createThingEventAdaptableWithExtra(createThingPayload(), extraFields, headers);
795+
final AuthorizationContext context = authContext(SUBJECT_PARTIAL);
796+
797+
// WHEN
798+
final Adaptable result = AdaptablePartialAccessFilter.filterAdaptableForPartialAccess(adaptable, context);
799+
800+
// THEN: identity + audit extras pass through despite the policy only granting /attributes/public.
801+
final Optional<JsonObject> filteredExtra = result.getPayload().getExtra();
802+
assertThat(filteredExtra).isPresent();
803+
final JsonObject extraObj = filteredExtra.get();
804+
assertThat(extraObj.getValue("thingId")).contains(JsonValue.of("org.eclipse.ditto.test:thing"));
805+
assertThat(extraObj.getValue("_namespace")).contains(JsonValue.of("org.eclipse.ditto.test"));
806+
assertThat(extraObj.getValue("_revision")).contains(JsonValue.of(7L));
807+
assertThat(extraObj.getValue("_modified")).contains(JsonValue.of("2026-06-01T12:00:00Z"));
808+
assertThat(extraObj.getValue("_created")).contains(JsonValue.of("2026-01-01T00:00:00Z"));
809+
// policyId is NOT in the allowlist — must remain filtered out.
810+
assertThat(extraObj.getValue("policyId")).isEmpty();
811+
// The actually-granted path is present, the ungranted sibling is dropped.
812+
assertThat(extraObj.getValue(JsonPointer.of("/attributes/public"))).isPresent();
813+
assertThat(extraObj.getValue(JsonPointer.of("/attributes/private"))).isEmpty();
814+
}
815+
726816
private static Adaptable createThingEventAdaptableWithExtra(
727817
final JsonObject payload,
728818
final JsonObject extraFields,

‎things/service/src/main/java/org/eclipse/ditto/things/service/enforcement/ThingCommandEnforcement.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,20 @@ final class ThingCommandEnforcement
100100
DittoLoggerFactory.getThreadSafeLogger(ThingCommandEnforcement.class.getName());
101101

102102
/**
103-
* Json fields that are always shown regardless of authorization.
103+
* Json fields that are always shown regardless of authorization, provided the subject has at
104+
* least partial read permissions on the thing. Covers entity identity ({@code thingId},
105+
* {@code _namespace}) and audit metadata ({@code _revision}, {@code _modified},
106+
* {@code _created}) which do not carry user data and which subscribers need in order to make
107+
* sense of the response.
104108
*/
105109
private static final JsonFieldSelector THING_QUERY_COMMAND_RESPONSE_ALLOWLIST =
106-
JsonFactory.newFieldSelector(Thing.JsonFields.ID);
110+
JsonFactory.newFieldSelector(
111+
Thing.JsonFields.ID,
112+
Thing.JsonFields.NAMESPACE,
113+
Thing.JsonFields.REVISION,
114+
Thing.JsonFields.MODIFIED,
115+
Thing.JsonFields.CREATED
116+
);
107117
private final ActorSystem actorSystem;
108118
private final ActorRef policiesShardRegion;
109119
private final AskWithRetryConfig askWithRetryConfig;

‎things/service/src/test/java/org/eclipse/ditto/things/service/enforcement/ThingCommandEnforcementTest.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,9 +342,12 @@ public void acceptByPolicyWithRevokeOnAttribute() {
342342
thingPersistenceActorProbe.reply(retrieveThingResponseWithAttr);
343343

344344
final JsonObject jsonObjectWithoutAttr = JsonObject.newBuilder()
345-
.set("thingId", "thing:id") // this is re-added as first field being a "special" field always visible after enforcement
346-
.set("_revision", 1)
345+
// thingId, _namespace, _revision are re-added by the entity-identity/audit
346+
// allowlist (THING_QUERY_COMMAND_RESPONSE_ALLOWLIST); ordering follows the
347+
// allowlist declaration order in ThingCommandEnforcement.
348+
.set("thingId", "thing:id")
347349
.set("_namespace", "thing")
350+
.set("_revision", 1)
348351
.set("policyId","policy:id")
349352
.set("attributes",JsonObject.empty())
350353
.build();

0 commit comments

Comments
 (0)