Skip to content

Commit 9add82e

Browse files
authored
Merge pull request #2384 from beyonnex-io/fix-mising-missing-extra-fields
use string subject IDs in PRE_DEFINED_EXTRA_FIELDS_READ_GRANT_OBJECT …
2 parents 25fabcc + 5036e30 commit 9add82e

File tree

6 files changed

+140
-166
lines changed

6 files changed

+140
-166
lines changed

things/service/src/main/java/org/eclipse/ditto/things/service/persistence/actors/enrichment/ThingEventEnricher.java

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,9 @@
5454
import org.eclipse.ditto.things.model.ThingId;
5555
import org.eclipse.ditto.things.model.signals.events.ThingEvent;
5656
import org.eclipse.ditto.things.service.common.config.PreDefinedExtraFieldsConfig;
57-
import org.eclipse.ditto.things.service.utils.IndexedReadGrant;
5857
import org.eclipse.ditto.things.service.utils.PartialAccessPathCalculator;
5958
import org.eclipse.ditto.things.service.utils.ReadGrant;
6059
import org.eclipse.ditto.things.service.utils.ReadGrantCollector;
61-
import org.eclipse.ditto.things.service.utils.ReadGrantIndexer;
6260

6361
/**
6462
* Encapsulates functionality in order to enrich ThingEvents with pre-defined {@code extraFields} via DittoHeaders
@@ -142,15 +140,15 @@ public <T extends DittoHeadersSettable<? extends T>> CompletionStage<T> enrichWi
142140
final DittoHeaders dittoHeaders = enrichedWithPartialAccessPaths.getDittoHeaders();
143141
return buildPredefinedExtraFieldsHeaderReadGrantObject(
144142
policyId, combinedPredefinedExtraFields, thingJson, dittoHeaders)
145-
.thenApply(indexedGrants -> {
143+
.thenApply(readGrant -> {
146144
final String extraFieldsKey = DittoHeaderDefinition.PRE_DEFINED_EXTRA_FIELDS.getKey();
147145
final String readGrantKey = DittoHeaderDefinition.PRE_DEFINED_EXTRA_FIELDS_READ_GRANT_OBJECT.getKey();
148146
final String extraFieldsObjectKey = DittoHeaderDefinition.PRE_DEFINED_EXTRA_FIELDS_OBJECT.getKey();
149147

150148
final DittoHeadersBuilder<?, ?> headersBuilder = dittoHeaders.toBuilder()
151149
.putHeader(extraFieldsKey,
152150
buildPredefinedExtraFieldsHeaderList(combinedPredefinedExtraFields))
153-
.putHeader(readGrantKey, indexedGrants.pathsToJson().toString())
151+
.putHeader(readGrantKey, readGrant.toJson().toString())
154152
.putHeader(extraFieldsObjectKey,
155153
buildPredefinedExtraFieldsHeaderObject(thingJson,
156154
combinedPredefinedExtraFields).toString());
@@ -253,9 +251,11 @@ private CompletionStage<Optional<PolicyEnforcer>> getPolicyEnforcerSafely(@Nulla
253251

254252
/**
255253
* Builds the read grant object using the new helper classes.
256-
* Returns indexed format to reduce header size.
254+
* Returns the ReadGrant with string subject IDs (not indexed integers), because
255+
* DittoCachingSignalEnrichmentFacade.filterAskedForFieldSelectorToGrantedFields() matches
256+
* authorization subject IDs directly against the array values in this header.
257257
*/
258-
private CompletionStage<IndexedReadGrant> buildPredefinedExtraFieldsHeaderReadGrantObject(
258+
private CompletionStage<ReadGrant> buildPredefinedExtraFieldsHeaderReadGrantObject(
259259
@Nullable final PolicyId policyId,
260260
final JsonFieldSelector preDefinedExtraFields,
261261
final JsonObject thingJson,
@@ -265,18 +265,16 @@ private CompletionStage<IndexedReadGrant> buildPredefinedExtraFieldsHeaderReadGr
265265
if (policyEnforcerOpt.isEmpty()) {
266266
LOGGER.withCorrelationId(dittoHeaders)
267267
.warn("No policy enforcer found for policyId: {}, returning empty read grant object", policyId);
268-
return IndexedReadGrant.empty();
268+
return ReadGrant.empty();
269269
}
270270

271271
final PolicyEnforcer policyEnforcer = policyEnforcerOpt.get();
272272

273-
final ReadGrant readGrant = ReadGrantCollector.collect(
273+
return ReadGrantCollector.collect(
274274
preDefinedExtraFields,
275275
thingJson,
276276
policyEnforcer
277277
);
278-
279-
return ReadGrantIndexer.index(readGrant);
280278
});
281279
}
282280

things/service/src/main/java/org/eclipse/ditto/things/service/utils/IndexedReadGrant.java

Lines changed: 0 additions & 72 deletions
This file was deleted.

things/service/src/main/java/org/eclipse/ditto/things/service/utils/ReadGrant.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,14 @@
1515
import java.util.Map;
1616
import java.util.Set;
1717

18+
import org.eclipse.ditto.json.JsonArray;
19+
import org.eclipse.ditto.json.JsonCollectors;
20+
import org.eclipse.ditto.json.JsonFactory;
21+
import org.eclipse.ditto.json.JsonKey;
22+
import org.eclipse.ditto.json.JsonObject;
23+
import org.eclipse.ditto.json.JsonObjectBuilder;
1824
import org.eclipse.ditto.json.JsonPointer;
25+
import org.eclipse.ditto.json.JsonValue;
1926

2027
/**
2128
* Immutable model representing read grants: which subjects can read which paths.
@@ -40,5 +47,31 @@ public static ReadGrant empty() {
4047
public boolean isEmpty() {
4148
return pointerToSubjects.isEmpty();
4249
}
50+
51+
/**
52+
* Converts this read grant to a JSON object mapping paths to arrays of subject ID strings.
53+
* <p>
54+
* Example output: {@code {"/attributes": ["connection:foo"], "/definition": ["connection:foo", "connection:bar"]}}
55+
* <p>
56+
* This format is required by
57+
* {@code DittoCachingSignalEnrichmentFacade.filterAskedForFieldSelectorToGrantedFields()},
58+
* which matches authorization subject IDs directly against the array values in the
59+
* {@code PRE_DEFINED_EXTRA_FIELDS_READ_GRANT_OBJECT} header.
60+
*
61+
* @return JSON object with path keys and string subject ID arrays as values
62+
* @since 3.9.0
63+
*/
64+
public JsonObject toJson() {
65+
final JsonObjectBuilder builder = JsonFactory.newObjectBuilder();
66+
for (final Map.Entry<JsonPointer, Set<String>> entry : pointerToSubjects.entrySet()) {
67+
final JsonKey pathKey = JsonKey.of(entry.getKey().toString());
68+
final JsonArray subjectsArray = entry.getValue().stream()
69+
.sorted()
70+
.map(JsonValue::of)
71+
.collect(JsonCollectors.valuesToArray());
72+
builder.set(pathKey, subjectsArray);
73+
}
74+
return builder.build();
75+
}
4376
}
4477

things/service/src/main/java/org/eclipse/ditto/things/service/utils/ReadGrantIndexer.java

Lines changed: 0 additions & 72 deletions
This file was deleted.

things/service/src/test/java/org/eclipse/ditto/things/service/persistence/actors/enrichment/ThingEventEnricherTest.java

Lines changed: 53 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ public void ensureDefinitionIsEnrichedAsPreDefinedFromConfiguration() {
132132
predefinedExtraFields -> predefinedExtraFields.add("/definition"),
133133
preDefinedExtraFieldsReadGrantObject -> preDefinedExtraFieldsReadGrantObject
134134
.set(JsonKey.of("/definition"), JsonArray.newBuilder()
135-
.add(0)
135+
.add(KNOWN_ISSUER_FULL_SUBJECT)
136136
.build()
137137
),
138138
preDefinedFieldsObject -> preDefinedFieldsObject.set("definition", KNOWN_DEFINITION)
@@ -157,17 +157,17 @@ public void ensureDefinitionAndAdditionalNamespaceSpecificIsEnrichedAsPreDefined
157157
.add("/attributes/public1")
158158
.add("/attributes/private"),
159159
preDefinedExtraFieldsReadGrantObject -> preDefinedExtraFieldsReadGrantObject
160-
.set(JsonKey.of("/attributes/public1"), JsonArray.newBuilder()
161-
.add(0)
162-
.add(1)
160+
.set(JsonKey.of("/definition"), JsonArray.newBuilder()
161+
.add(KNOWN_ISSUER_FULL_SUBJECT)
163162
.build()
164163
)
165-
.set(JsonKey.of("/definition"), JsonArray.newBuilder()
166-
.add(0)
164+
.set(JsonKey.of("/attributes/public1"), JsonArray.newBuilder()
165+
.add(KNOWN_ISSUER_FULL_SUBJECT)
166+
.add(KNOWN_ISSUER_RESTRICTED_SUBJECT)
167167
.build()
168168
)
169169
.set(JsonKey.of("/attributes/private"), JsonArray.newBuilder()
170-
.add(0)
170+
.add(KNOWN_ISSUER_FULL_SUBJECT)
171171
.build()
172172
),
173173
preDefinedFieldsObject -> preDefinedFieldsObject
@@ -269,17 +269,17 @@ public void ensureConditionBasedEnrichmentAsPreDefinedFromConfiguration() {
269269
.add("/attributes/folder"),
270270
preDefinedExtraFieldsReadGrantObject -> preDefinedExtraFieldsReadGrantObject
271271
.set(JsonKey.of("/attributes/public1"), JsonArray.newBuilder()
272-
.add(0)
273-
.add(1)
272+
.add(KNOWN_ISSUER_FULL_SUBJECT)
273+
.add(KNOWN_ISSUER_RESTRICTED_SUBJECT)
274274
.build()
275275
)
276276
.set(JsonKey.of("/attributes/folder"), JsonArray.newBuilder()
277-
.add(0)
277+
.add(KNOWN_ISSUER_FULL_SUBJECT)
278278
.build()
279279
)
280280
.set(JsonKey.of("/attributes/folder/public"), JsonArray.newBuilder()
281-
.add(2)
282-
.add(1)
281+
.add(KNOWN_ISSUER_ANOTHER_SUBJECT)
282+
.add(KNOWN_ISSUER_RESTRICTED_SUBJECT)
283283
.build()
284284
),
285285
preDefinedFieldsObject -> preDefinedFieldsObject
@@ -289,6 +289,47 @@ public void ensureConditionBasedEnrichmentAsPreDefinedFromConfiguration() {
289289
);
290290
}
291291

292+
/**
293+
* Reproduces the production scenario: connection with {@code thing:/ READ} requesting
294+
* {@code attributes} as extraFields. Verifies that the read grant header contains string
295+
* subject IDs that {@code DittoCachingSignalEnrichmentFacade} can match against.
296+
*/
297+
@Test
298+
public void readGrantHeaderContainsStringSubjectIdsForRootReadPolicy() {
299+
// GIVEN: a policy with only thing:/ READ (like lora-downlink-processor in production)
300+
final PolicyEnforcerProvider fullAccessProvider = mock(PolicyEnforcerProvider.class);
301+
when(fullAccessProvider.getPolicyEnforcer(KNOWN_POLICY_ID))
302+
.thenReturn(CompletableFuture.completedFuture(Optional.of(PolicyEnforcer.of(FULL_ACCESS_POLICY))));
303+
final ThingEventEnricher sut = new ThingEventEnricher(fullAccessProvider, true);
304+
final var configs = getPreDefinedExtraFieldsConfigs(
305+
"namespaces = [\"org.eclipse.ditto.some\"]\n" +
306+
"extra-fields = [\"attributes\"]"
307+
);
308+
309+
// WHEN: enriched headers are calculated
310+
final CompletionStage<DittoHeaders> resultHeadersStage = calculateEnrichedSignalHeaders(sut, configs);
311+
final DittoHeaders headers = resultHeadersStage.toCompletableFuture().join();
312+
313+
// THEN: the read grant header must contain string subject IDs, not integer indices
314+
final String readGrantHeader = headers.get(
315+
DittoHeaderDefinition.PRE_DEFINED_EXTRA_FIELDS_READ_GRANT_OBJECT.getKey());
316+
assertThat(readGrantHeader)
317+
.as("read grant header should be present")
318+
.isNotNull();
319+
final JsonObject readGrant = JsonFactory.readFrom(readGrantHeader).asObject();
320+
// Verify it contains string subject IDs (e.g. "foo-issuer:full-subject"), not integers
321+
readGrant.forEach(field -> {
322+
final JsonArray subjects = field.getValue().asArray();
323+
subjects.forEach(value ->
324+
assertThat(value.isString())
325+
.as("read grant array values must be strings, not integers: %s", value)
326+
.isTrue()
327+
);
328+
});
329+
// Verify the specific subject is present for /attributes
330+
assertThat(readGrantHeader).contains(KNOWN_ISSUER_FULL_SUBJECT);
331+
}
332+
292333
private List<PreDefinedExtraFieldsConfig> getPreDefinedExtraFieldsConfigs(final String... configurations) {
293334
return Arrays.stream(configurations)
294335
.map(configString ->

0 commit comments

Comments
 (0)