From 3428a1aa5a49d507532416850f6afaed8da24d45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20J=C3=A4ckle?= Date: Tue, 17 Feb 2026 16:15:38 +0100 Subject: [PATCH] Optimize ThingEventEnricher -> TreeBasedPolicyEnforcer hot path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reduce redundant policy tree walks and thing.toJson() serializations in the per-ThingEvent enrichment path, which JFR profiling identified as the #1 CPU hotspot on production things nodes. - Add ClassifySubjectsVisitor that replaces 3 separate tree walks (partial/unrestricted/effected) with a single pass, returning a SubjectClassification result. Expose as classifySubjects() on the Enforcer interface with an optimized override in TreeBasedPolicyEnforcer. - Add batch getAccessiblePathsForSubjects() that flattens Thing JSON into leaf pointers once per batch instead of once per subject. Default method on Enforcer with an optimized override in TreeBasedPolicyEnforcer. - Cache thing.toJson() once at the top of enrichWithPredefinedExtraFields() and thread the JsonObject through ReadGrantCollector, PartialAccessPathCalculator, and the extra fields header builder, eliminating redundant serializations per event. - Extract ROOT_RESOURCE_POINTER constant in TreeBasedPolicyEnforcer to avoid repeated JsonFactory.newPointer("/") allocations. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Thomas Jäckle --- .../policies/model/enforcers/Enforcer.java | 50 +++ .../enforcers/SubjectClassification.java | 127 ++++++ .../tree/ClassifySubjectsVisitor.java | 254 ++++++++++++ .../tree/TreeBasedPolicyEnforcer.java | 61 ++- .../tree/ClassifySubjectsVisitorTest.java | 363 ++++++++++++++++++ .../actors/enrichment/ThingEventEnricher.java | 38 +- .../utils/PartialAccessPathCalculator.java | 116 ++---- .../service/utils/ReadGrantCollector.java | 77 ++-- 8 files changed, 950 insertions(+), 136 deletions(-) create mode 100644 policies/model/src/main/java/org/eclipse/ditto/policies/model/enforcers/SubjectClassification.java create mode 100644 policies/model/src/main/java/org/eclipse/ditto/policies/model/enforcers/tree/ClassifySubjectsVisitor.java create mode 100644 policies/model/src/test/java/org/eclipse/ditto/policies/model/enforcers/tree/ClassifySubjectsVisitorTest.java diff --git a/policies/model/src/main/java/org/eclipse/ditto/policies/model/enforcers/Enforcer.java b/policies/model/src/main/java/org/eclipse/ditto/policies/model/enforcers/Enforcer.java index 563a5d0cb0..42e3a1ac80 100755 --- a/policies/model/src/main/java/org/eclipse/ditto/policies/model/enforcers/Enforcer.java +++ b/policies/model/src/main/java/org/eclipse/ditto/policies/model/enforcers/Enforcer.java @@ -12,6 +12,9 @@ */ package org.eclipse.ditto.policies.model.enforcers; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; import java.util.Set; import org.eclipse.ditto.base.model.auth.AuthorizationContext; @@ -355,4 +358,51 @@ default Set getAccessiblePaths(ResourceKey resourceKey, Iterable> getAccessiblePathsForSubjects( + final ResourceKey resourceKey, final Iterable jsonFields, + final Set authorizationSubjects, final Permissions permissions) { + final Map> result = new HashMap<>(); + for (final AuthorizationSubject subject : authorizationSubjects) { + final Set paths = getAccessiblePaths(resourceKey, jsonFields, subject, permissions); + if (!paths.isEmpty()) { + result.put(subject, paths); + } + } + return result; + } + + /** + * Classifies all authorization subjects into three categories based on their permissions on the given resource: + * unrestricted, partial-only, and effected granted. This performs a single classification pass instead of + * requiring separate calls to {@link #getSubjectsWithPartialPermission}, {@link #getSubjectsWithUnrestrictedPermission}, + * and {@link #getSubjectsWithPermission}. + * + * @param resourceKey the ResourceKey (containing Resource type and path) to classify subjects for. + * @param permissions the permissions to check. + * @return the classification of subjects. + * @throws NullPointerException if any argument is {@code null}. + * @since 3.9.0 + */ + default SubjectClassification classifySubjects(final ResourceKey resourceKey, final Permissions permissions) { + final Set partial = getSubjectsWithPartialPermission(resourceKey, permissions); + final Set unrestricted = getSubjectsWithUnrestrictedPermission(resourceKey, permissions); + final EffectedSubjects effected = getSubjectsWithPermission(resourceKey, permissions); + final Set partialOnly = new HashSet<>(partial); + partialOnly.removeAll(unrestricted); + return SubjectClassification.of(unrestricted, partialOnly, effected.getGranted()); + } + } diff --git a/policies/model/src/main/java/org/eclipse/ditto/policies/model/enforcers/SubjectClassification.java b/policies/model/src/main/java/org/eclipse/ditto/policies/model/enforcers/SubjectClassification.java new file mode 100644 index 0000000000..90ed9f5f6a --- /dev/null +++ b/policies/model/src/main/java/org/eclipse/ditto/policies/model/enforcers/SubjectClassification.java @@ -0,0 +1,127 @@ +/* + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.eclipse.ditto.policies.model.enforcers; + +import java.util.Collections; +import java.util.Objects; +import java.util.Set; + +import org.eclipse.ditto.base.model.auth.AuthorizationSubject; + +/** + * Immutable result of classifying authorization subjects into three categories based on their permissions + * on a specific resource: + *
    + *
  • {@code unrestricted} - subjects with full access, no revocations below
    • + *
  • {@code partialOnly} - subjects with some grants but not unrestricted (partial minus unrestricted)
    • + *
  • {@code effectedGranted} - subjects with grants exactly at the resource level
    • + *
+ * + * @since 3.9.0 + */ +public final class SubjectClassification { + + private static final SubjectClassification EMPTY = new SubjectClassification( + Collections.emptySet(), Collections.emptySet(), Collections.emptySet()); + + private final Set unrestricted; + private final Set partialOnly; + private final Set effectedGranted; + + private SubjectClassification(final Set unrestricted, + final Set partialOnly, + final Set effectedGranted) { + this.unrestricted = Collections.unmodifiableSet(unrestricted); + this.partialOnly = Collections.unmodifiableSet(partialOnly); + this.effectedGranted = Collections.unmodifiableSet(effectedGranted); + } + + /** + * Creates a new {@code SubjectClassification} from the given sets. + * + * @param unrestricted subjects with full access, no revocations below. + * @param partialOnly subjects with some grants but not unrestricted. + * @param effectedGranted subjects with grants exactly at the resource level. + * @return the new SubjectClassification. + */ + public static SubjectClassification of(final Set unrestricted, + final Set partialOnly, + final Set effectedGranted) { + return new SubjectClassification(unrestricted, partialOnly, effectedGranted); + } + + /** + * Returns an empty {@code SubjectClassification} with no subjects in any category. + * + * @return the empty SubjectClassification. + */ + public static SubjectClassification empty() { + return EMPTY; + } + + /** + * Returns the subjects with unrestricted (full) access. + * + * @return an unmodifiable set of unrestricted subjects. + */ + public Set getUnrestricted() { + return unrestricted; + } + + /** + * Returns the subjects with partial access only (not unrestricted). + * + * @return an unmodifiable set of partial-only subjects. + */ + public Set getPartialOnly() { + return partialOnly; + } + + /** + * Returns the subjects with grants exactly at the resource level. + * + * @return an unmodifiable set of effected granted subjects. + */ + public Set getEffectedGranted() { + return effectedGranted; + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + final SubjectClassification that = (SubjectClassification) o; + return Objects.equals(unrestricted, that.unrestricted) && + Objects.equals(partialOnly, that.partialOnly) && + Objects.equals(effectedGranted, that.effectedGranted); + } + + @Override + public int hashCode() { + return Objects.hash(unrestricted, partialOnly, effectedGranted); + } + + @Override + public String toString() { + return getClass().getSimpleName() + " [" + + "unrestricted=" + unrestricted + + ", partialOnly=" + partialOnly + + ", effectedGranted=" + effectedGranted + + "]"; + } + +} diff --git a/policies/model/src/main/java/org/eclipse/ditto/policies/model/enforcers/tree/ClassifySubjectsVisitor.java b/policies/model/src/main/java/org/eclipse/ditto/policies/model/enforcers/tree/ClassifySubjectsVisitor.java new file mode 100644 index 0000000000..06d0a0730f --- /dev/null +++ b/policies/model/src/main/java/org/eclipse/ditto/policies/model/enforcers/tree/ClassifySubjectsVisitor.java @@ -0,0 +1,254 @@ +/* + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.eclipse.ditto.policies.model.enforcers.tree; + +import java.util.Collection; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; + +import javax.annotation.Nullable; +import javax.annotation.concurrent.NotThreadSafe; + +import org.eclipse.ditto.base.model.auth.AuthorizationModelFactory; +import org.eclipse.ditto.base.model.auth.AuthorizationSubject; +import org.eclipse.ditto.json.JsonPointer; +import org.eclipse.ditto.policies.model.EffectedPermissions; +import org.eclipse.ditto.policies.model.Permissions; +import org.eclipse.ditto.policies.model.enforcers.SubjectClassification; + +/** + * A single visitor that walks the policy tree once and classifies all subjects into three categories: + * partial, unrestricted, and effected granted. This replaces the need for three separate tree walks + * using {@code CollectPartialGrantedSubjectsVisitor}, {@code CollectUnrestrictedSubjectsVisitor}, + * and {@code CollectEffectedSubjectsVisitor}. + * + *

Per subject, maintains 3 {@link WeightedPermissions} instances tracking different aggregation strategies:

+ * + * + * + * + * + * + *
WeightedPermissionsABOVE/SAME grantsABOVE/SAME revokesBELOW grantsBELOW revokes
partialWeightsyesyesyesno
unrestrictedWeightsyesyesnoyes
effectedWeightsyesyesnono
+ * + * @since 3.9.0 + */ +@NotThreadSafe +final class ClassifySubjectsVisitor implements Visitor { + + private final Permissions expectedPermissions; + private final Function pointerLocationEvaluator; + private final Collection evaluators; + @Nullable private ResourceNodeEvaluator currentEvaluator; + + /** + * Constructs a new {@code ClassifySubjectsVisitor} object. + * + * @param resourcePointer the resource pointer to classify subjects for. + * @param expectedPermissions the permissions to check. + */ + ClassifySubjectsVisitor(final JsonPointer resourcePointer, final Permissions expectedPermissions) { + this.expectedPermissions = expectedPermissions; + this.pointerLocationEvaluator = new PointerLocationEvaluator(resourcePointer); + this.evaluators = new HashSet<>(); + this.currentEvaluator = null; + } + + @Override + public void visitTreeNode(final PolicyTreeNode node) { + if (PolicyTreeNode.Type.SUBJECT == node.getType()) { + visitSubjectNode(node); + } else { + visitResourceNode((ResourceNode) node); + } + } + + private void visitSubjectNode(final PolicyTreeNode subjectNode) { + final String currentSubjectId = subjectNode.getName(); + currentEvaluator = new ResourceNodeEvaluator(currentSubjectId); + evaluators.add(currentEvaluator); + } + + private void visitResourceNode(final ResourceNode resourceNode) { + if (null != currentEvaluator) { + currentEvaluator.aggregateWeightedPermissions(resourceNode); + } + } + + @Override + public SubjectClassification get() { + final Set partialSubjects = new HashSet<>(); + final Set unrestrictedSubjects = new HashSet<>(); + final EffectedSubjectsBuilder effectedSubjectsBuilder = new EffectedSubjectsBuilder(); + + for (final ResourceNodeEvaluator evaluator : evaluators) { + evaluator.evaluate(partialSubjects, unrestrictedSubjects, effectedSubjectsBuilder); + } + + final Set partialOnly = new HashSet<>(partialSubjects); + partialOnly.removeAll(unrestrictedSubjects); + + return SubjectClassification.of( + unrestrictedSubjects, + partialOnly, + effectedSubjectsBuilder.build().getGranted() + ); + } + + /** + * Aggregates and evaluates permissions for a single subject using three weight sets. + */ + @NotThreadSafe + private final class ResourceNodeEvaluator { + + private final AuthorizationSubject authorizationSubject; + private final WeightedPermissions partialWeights; + private final WeightedPermissions unrestrictedWeights; + private final WeightedPermissions effectedWeights; + + private ResourceNodeEvaluator(final CharSequence subjectId) { + this.authorizationSubject = AuthorizationModelFactory.newAuthSubject(subjectId); + this.partialWeights = new WeightedPermissions(); + this.unrestrictedWeights = new WeightedPermissions(); + this.effectedWeights = new WeightedPermissions(); + } + + private void aggregateWeightedPermissions(final ResourceNode resourceNode) { + final PointerLocation pointerLocation = + pointerLocationEvaluator.apply(resourceNode.getAbsolutePointer()); + final EffectedPermissions effectedPermissions = resourceNode.getPermissions(); + final Permissions grantedPermissions = effectedPermissions.getGrantedPermissions(); + final Permissions revokedPermissions = effectedPermissions.getRevokedPermissions(); + final int level = resourceNode.getLevel(); + + if (PointerLocation.ABOVE == pointerLocation || PointerLocation.SAME == pointerLocation) { + // All three weight sets get ABOVE/SAME grants and revokes + partialWeights.addGranted(grantedPermissions, level); + partialWeights.addRevoked(revokedPermissions, level); + + unrestrictedWeights.addGranted(grantedPermissions, level); + unrestrictedWeights.addRevoked(revokedPermissions, level); + + effectedWeights.addGranted(grantedPermissions, level); + effectedWeights.addRevoked(revokedPermissions, level); + } else if (PointerLocation.BELOW == pointerLocation) { + // Partial: BELOW grants only (no BELOW revokes) + partialWeights.addGranted(grantedPermissions, level); + + // Unrestricted: BELOW revokes only (no BELOW grants) + unrestrictedWeights.addRevoked(revokedPermissions, level); + + // Effected: nothing for BELOW + } + } + + private void evaluate(final Set partialSubjects, + final Set unrestrictedSubjects, + final EffectedSubjectsBuilder effectedSubjectsBuilder) { + + // Evaluate partial (same logic as CollectPartialGrantedSubjectsVisitor) + evaluatePartial(partialSubjects); + + // Evaluate unrestricted (same logic as CollectUnrestrictedSubjectsVisitor) + evaluateUnrestricted(unrestrictedSubjects); + + // Evaluate effected (same logic as CollectEffectedSubjectsVisitor) + evaluateEffected(effectedSubjectsBuilder); + } + + private void evaluatePartial(final Set partialSubjects) { + final Map revoked = + partialWeights.getRevokedWithHighestWeight(expectedPermissions); + final Map granted = + partialWeights.getGrantedWithHighestWeight(expectedPermissions); + if (areExpectedPermissionsEffectivelyRevoked(revoked, granted)) { + partialSubjects.remove(authorizationSubject); + } else if (areExpectedPermissionsEffectivelyGranted(granted, revoked)) { + partialSubjects.add(authorizationSubject); + } + } + + private void evaluateUnrestricted(final Set unrestrictedSubjects) { + final Map revoked = + unrestrictedWeights.getRevokedWithHighestWeight(expectedPermissions); + final Map granted = + unrestrictedWeights.getGrantedWithHighestWeight(expectedPermissions); + if (areExpectedPermissionsEffectivelyRevoked(revoked, granted)) { + unrestrictedSubjects.remove(authorizationSubject); + } else if (areExpectedPermissionsEffectivelyGranted(granted, revoked)) { + unrestrictedSubjects.add(authorizationSubject); + } + } + + private void evaluateEffected(final EffectedSubjectsBuilder effectedSubjectsBuilder) { + final Map revoked = + effectedWeights.getRevokedWithHighestWeight(expectedPermissions); + final Map granted = + effectedWeights.getGrantedWithHighestWeight(expectedPermissions); + if (areExpectedPermissionsEffectivelyRevoked(revoked, granted)) { + effectedSubjectsBuilder.withRevoked(authorizationSubject); + } else if (areExpectedPermissionsEffectivelyGranted(granted, revoked)) { + effectedSubjectsBuilder.withGranted(authorizationSubject); + } + } + + @SuppressWarnings("MethodWithMultipleReturnPoints") + private boolean areExpectedPermissionsEffectivelyRevoked(final Map revoked, + final Map granted) { + + if (revoked.size() != expectedPermissions.size()) { + return false; + } + + for (final String expectedPermission : expectedPermissions) { + final WeightedPermission revokedPermission = revoked.get(expectedPermission); + final WeightedPermission grantedPermission = granted.get(expectedPermission); + if (null != grantedPermission) { + final int grantedPermissionWeight = grantedPermission.getWeight(); + final int revokedPermissionWeight = revokedPermission.getWeight(); + if (grantedPermissionWeight > revokedPermissionWeight) { + return false; + } + } + } + + return true; + } + + @SuppressWarnings("MethodWithMultipleReturnPoints") + private boolean areExpectedPermissionsEffectivelyGranted(final Map granted, + final Map revoked) { + + if (granted.size() != expectedPermissions.size()) { + return false; + } + + for (final String expectedPermission : expectedPermissions) { + final WeightedPermission grantedPermission = granted.get(expectedPermission); + final WeightedPermission revokedPermission = revoked.get(expectedPermission); + if (null != revokedPermission) { + final int revokedPermissionWeight = revokedPermission.getWeight(); + final int grantedPermissionWeight = grantedPermission.getWeight(); + if (revokedPermissionWeight >= grantedPermissionWeight) { + return false; + } + } + } + + return true; + } + } + +} diff --git a/policies/model/src/main/java/org/eclipse/ditto/policies/model/enforcers/tree/TreeBasedPolicyEnforcer.java b/policies/model/src/main/java/org/eclipse/ditto/policies/model/enforcers/tree/TreeBasedPolicyEnforcer.java index 7dc3444f3f..93811b71ea 100755 --- a/policies/model/src/main/java/org/eclipse/ditto/policies/model/enforcers/tree/TreeBasedPolicyEnforcer.java +++ b/policies/model/src/main/java/org/eclipse/ditto/policies/model/enforcers/tree/TreeBasedPolicyEnforcer.java @@ -49,6 +49,7 @@ import org.eclipse.ditto.policies.model.Subjects; import org.eclipse.ditto.policies.model.enforcers.EffectedSubjects; import org.eclipse.ditto.policies.model.enforcers.Enforcer; +import org.eclipse.ditto.policies.model.enforcers.SubjectClassification; /** * Holds Algorithms to create a policy tree and to perform different policy checks on this tree. @@ -56,6 +57,7 @@ public final class TreeBasedPolicyEnforcer implements Enforcer { private static final String ROOT_RESOURCE = "/"; + private static final JsonPointer ROOT_RESOURCE_POINTER = JsonFactory.newPointer(ROOT_RESOURCE); /** * Maps subject ID to {@link org.eclipse.ditto.policies.model.enforcers.tree.SubjectNode} whose children are {@link org.eclipse.ditto.policies.model.enforcers.tree.ResourceNode} for which the subject is granted @@ -227,6 +229,14 @@ public Set getSubjectsWithUnrestrictedPermission(final Res return visitTree(new CollectUnrestrictedSubjectsVisitor(resourcePointer, permissions)); } + @Override + public SubjectClassification classifySubjects(final ResourceKey resourceKey, final Permissions permissions) { + checkResourceKey(resourceKey); + checkPermissions(permissions); + final JsonPointer resourcePointer = createAbsoluteResourcePointer(resourceKey); + return visitTree(new ClassifySubjectsVisitor(resourcePointer, permissions)); + } + @Override public JsonObject buildJsonView( final ResourceKey resourceKey, @@ -240,7 +250,7 @@ public JsonObject buildJsonView( final Collection authorizationSubjectIds = getAuthorizationSubjectIds(authorizationContext); final EffectedResources effectedResources = getGrantedAndRevokedSubResource( - JsonFactory.newPointer(ROOT_RESOURCE), resourceKey.getResourceType(), authorizationSubjectIds, + ROOT_RESOURCE_POINTER, resourceKey.getResourceType(), authorizationSubjectIds, permissions); if (jsonFields instanceof JsonObject && ((JsonObject) jsonFields).isNull()) { @@ -272,7 +282,7 @@ public Set getAccessiblePaths( final Collection authorizationSubjectIds = getAuthorizationSubjectIds(authorizationContext); final EffectedResources effectedResources = getGrantedAndRevokedSubResource( - JsonFactory.newPointer(ROOT_RESOURCE), resourceKey.getResourceType(), authorizationSubjectIds, + ROOT_RESOURCE_POINTER, resourceKey.getResourceType(), authorizationSubjectIds, permissions); if (jsonFields instanceof JsonObject && ((JsonObject) jsonFields).isNull()) { @@ -291,6 +301,46 @@ public Set getAccessiblePaths( return extractAccessiblePaths(prefixedPointers, grantedResources, revokedResources, resourcePath); } + @Override + public Map> getAccessiblePathsForSubjects( + final ResourceKey resourceKey, final Iterable jsonFields, + final Set authorizationSubjects, final Permissions permissions) { + + checkResourceKey(resourceKey); + checkNotNull(jsonFields, "JSON fields"); + checkPermissions(permissions); + + if (jsonFields instanceof JsonObject && ((JsonObject) jsonFields).isNull()) { + return Collections.emptyMap(); + } + + // 1. Flatten once + final List flatPointers = new ArrayList<>(); + jsonFields.forEach(jf -> collectFlatPointers(jf.getKey().asPointer(), jf, flatPointers)); + final JsonPointer resourcePath = resourceKey.getResourcePath(); + final List prefixedPointers = flatPointers.stream() + .map(pv -> new PointerAndValue(resourcePath.append(pv.pointer), pv.value)) + .collect(Collectors.toList()); + + // 2. Per-subject: tree walk + filter (reusing prefixedPointers) + final Map> result = new HashMap<>(); + for (final AuthorizationSubject subject : authorizationSubjects) { + final Collection subjectIds = Collections.singleton(subject.getId()); + final EffectedResources effectedResources = getGrantedAndRevokedSubResource( + ROOT_RESOURCE_POINTER, resourceKey.getResourceType(), subjectIds, permissions); + + final Set grantedResources = extractJsonPointers(effectedResources.getGrantedResources()); + final Set revokedResources = extractJsonPointers(effectedResources.getRevokedResources()); + + final Set paths = extractAccessiblePaths( + prefixedPointers, grantedResources, revokedResources, resourcePath); + if (!paths.isEmpty()) { + result.put(subject, paths); + } + } + return result; + } + private static Set extractJsonPointers(final Collection resources) { return resources.stream() .map(pointerAndPermission -> pointerAndPermission.pointer) @@ -385,9 +435,8 @@ private static boolean pointerStartsWith(final JsonPointer pointer, final JsonPo private static boolean isAccessible(final JsonPointer pointer, final Collection grantedResources, final Collection revokedResources) { - final JsonPointer rootResourcePointer = JsonFactory.newPointer(ROOT_RESOURCE); - boolean accessible = grantedResources.contains(rootResourcePointer) && - !revokedResources.contains(rootResourcePointer); + boolean accessible = grantedResources.contains(ROOT_RESOURCE_POINTER) && + !revokedResources.contains(ROOT_RESOURCE_POINTER); final int levelCount = pointer.getLevelCount(); if (levelCount > 0) { @@ -597,7 +646,7 @@ private static void addPermission(final String permission, final ResourceNode resourceNode) { final JsonPointer resourceToAdd = ROOT_RESOURCE.equals(resource.toString()) - ? JsonFactory.newPointer(ROOT_RESOURCE) + ? ROOT_RESOURCE_POINTER : getPrefixPointerOrThrow(resource, level); final EffectedPermissions effectedPermissions = resourceNode.getPermissions(); if (effectedPermissions.getGrantedPermissions().contains(permission)) { diff --git a/policies/model/src/test/java/org/eclipse/ditto/policies/model/enforcers/tree/ClassifySubjectsVisitorTest.java b/policies/model/src/test/java/org/eclipse/ditto/policies/model/enforcers/tree/ClassifySubjectsVisitorTest.java new file mode 100644 index 0000000000..4133c5b46a --- /dev/null +++ b/policies/model/src/test/java/org/eclipse/ditto/policies/model/enforcers/tree/ClassifySubjectsVisitorTest.java @@ -0,0 +1,363 @@ +/* + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.eclipse.ditto.policies.model.enforcers.tree; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.HashSet; +import java.util.Set; + +import org.eclipse.ditto.base.model.auth.AuthorizationSubject; +import org.eclipse.ditto.policies.model.Permissions; +import org.eclipse.ditto.policies.model.PoliciesResourceType; +import org.eclipse.ditto.policies.model.Policy; +import org.eclipse.ditto.policies.model.PolicyId; +import org.eclipse.ditto.policies.model.ResourceKey; +import org.eclipse.ditto.policies.model.SubjectType; +import org.eclipse.ditto.policies.model.enforcers.EffectedSubjects; +import org.eclipse.ditto.policies.model.enforcers.SubjectClassification; +import org.junit.Test; + +/** + * Unit test for {@link ClassifySubjectsVisitor}. + *

+ * Verifies that the combined single-pass visitor produces the same results as + * the three individual visitors ({@code CollectPartialGrantedSubjectsVisitor}, + * {@code CollectUnrestrictedSubjectsVisitor}, {@code CollectEffectedSubjectsVisitor}). + *

+ */ +public final class ClassifySubjectsVisitorTest { + + private static final Permissions READ = Permissions.newInstance("READ"); + private static final Permissions READ_WRITE = Permissions.newInstance("READ", "WRITE"); + + @Test + public void emptyPolicyReturnsEmptyClassification() { + final PolicyId policyId = PolicyId.of("ns", "empty"); + final TreeBasedPolicyEnforcer enforcer = + TreeBasedPolicyEnforcer.createInstance(Policy.newBuilder(policyId).build()); + + final ResourceKey resourceKey = PoliciesResourceType.thingResource("/"); + final SubjectClassification classification = enforcer.classifySubjects(resourceKey, READ); + + assertThat(classification.getUnrestricted()).isEmpty(); + assertThat(classification.getPartialOnly()).isEmpty(); + assertThat(classification.getEffectedGranted()).isEmpty(); + } + + @Test + public void singleSubjectFullGrantMatchesIndividualVisitors() { + final AuthorizationSubject subject = AuthorizationSubject.newInstance("test:full"); + final PolicyId policyId = PolicyId.of("ns", "single-full"); + final Policy policy = Policy.newBuilder(policyId) + .forLabel("grant-all") + .setSubject(subject.getId(), SubjectType.GENERATED) + .setGrantedPermissions(PoliciesResourceType.thingResource("/"), READ) + .build(); + + final TreeBasedPolicyEnforcer enforcer = TreeBasedPolicyEnforcer.createInstance(policy); + final ResourceKey resourceKey = PoliciesResourceType.thingResource("/"); + + final SubjectClassification classification = enforcer.classifySubjects(resourceKey, READ); + assertMatchesIndividualVisitors(enforcer, resourceKey, READ, classification); + + assertThat(classification.getUnrestricted()).containsExactly(subject); + assertThat(classification.getPartialOnly()).isEmpty(); + assertThat(classification.getEffectedGranted()).containsExactly(subject); + } + + @Test + public void subjectWithRevokedChildIsNotUnrestricted() { + final AuthorizationSubject subject = AuthorizationSubject.newInstance("test:revoked-child"); + final PolicyId policyId = PolicyId.of("ns", "revoked-child"); + final Policy policy = Policy.newBuilder(policyId) + .forLabel("grant") + .setSubject(subject.getId(), SubjectType.GENERATED) + .setGrantedPermissions(PoliciesResourceType.thingResource("/"), READ) + .forLabel("revoke") + .setSubject(subject.getId(), SubjectType.GENERATED) + .setRevokedPermissions(PoliciesResourceType.thingResource("/attributes/secret"), READ) + .build(); + + final TreeBasedPolicyEnforcer enforcer = TreeBasedPolicyEnforcer.createInstance(policy); + final ResourceKey resourceKey = PoliciesResourceType.thingResource("/"); + + final SubjectClassification classification = enforcer.classifySubjects(resourceKey, READ); + assertMatchesIndividualVisitors(enforcer, resourceKey, READ, classification); + + // Has grants at root + below, so partial. But has revoke below -> not unrestricted. + assertThat(classification.getUnrestricted()).doesNotContain(subject); + assertThat(classification.getPartialOnly()).contains(subject); + assertThat(classification.getEffectedGranted()).contains(subject); + } + + @Test + public void multipleSubjectsWithMixedPermissions() { + final AuthorizationSubject fullAccess = AuthorizationSubject.newInstance("test:full"); + final AuthorizationSubject partialAccess = AuthorizationSubject.newInstance("test:partial"); + final AuthorizationSubject noAccess = AuthorizationSubject.newInstance("test:none"); + + final PolicyId policyId = PolicyId.of("ns", "multi"); + final Policy policy = Policy.newBuilder(policyId) + .forLabel("full-grant") + .setSubject(fullAccess.getId(), SubjectType.GENERATED) + .setGrantedPermissions(PoliciesResourceType.thingResource("/"), READ) + .forLabel("partial-grant") + .setSubject(partialAccess.getId(), SubjectType.GENERATED) + .setGrantedPermissions(PoliciesResourceType.thingResource("/attributes/location"), READ) + .forLabel("no-access") + .setSubject(noAccess.getId(), SubjectType.GENERATED) + .setGrantedPermissions(PoliciesResourceType.thingResource("/"), Permissions.newInstance("WRITE")) + .build(); + + final TreeBasedPolicyEnforcer enforcer = TreeBasedPolicyEnforcer.createInstance(policy); + final ResourceKey resourceKey = PoliciesResourceType.thingResource("/"); + + final SubjectClassification classification = enforcer.classifySubjects(resourceKey, READ); + assertMatchesIndividualVisitors(enforcer, resourceKey, READ, classification); + + assertThat(classification.getUnrestricted()).containsExactly(fullAccess); + assertThat(classification.getPartialOnly()).containsExactly(partialAccess); + assertThat(classification.getEffectedGranted()).contains(fullAccess); + assertThat(classification.getEffectedGranted()).doesNotContain(partialAccess); + assertThat(classification.getEffectedGranted()).doesNotContain(noAccess); + } + + @Test + public void subjectWithOnlyBelowGrantIsPartialOnly() { + final AuthorizationSubject subject = AuthorizationSubject.newInstance("test:below-only"); + final PolicyId policyId = PolicyId.of("ns", "below-only"); + final Policy policy = Policy.newBuilder(policyId) + .forLabel("below-grant") + .setSubject(subject.getId(), SubjectType.GENERATED) + .setGrantedPermissions( + PoliciesResourceType.thingResource("/features/sensor/properties/value"), READ) + .build(); + + final TreeBasedPolicyEnforcer enforcer = TreeBasedPolicyEnforcer.createInstance(policy); + final ResourceKey resourceKey = PoliciesResourceType.thingResource("/"); + + final SubjectClassification classification = enforcer.classifySubjects(resourceKey, READ); + assertMatchesIndividualVisitors(enforcer, resourceKey, READ, classification); + + assertThat(classification.getUnrestricted()).isEmpty(); + assertThat(classification.getPartialOnly()).containsExactly(subject); + assertThat(classification.getEffectedGranted()).isEmpty(); + } + + @Test + public void revokedAtRootMeansNotPartialAndNotUnrestricted() { + final AuthorizationSubject subject = AuthorizationSubject.newInstance("test:revoked-root"); + final PolicyId policyId = PolicyId.of("ns", "revoked-root"); + final Policy policy = Policy.newBuilder(policyId) + .forLabel("revoke") + .setSubject(subject.getId(), SubjectType.GENERATED) + .setRevokedPermissions(PoliciesResourceType.thingResource("/"), READ) + .build(); + + final TreeBasedPolicyEnforcer enforcer = TreeBasedPolicyEnforcer.createInstance(policy); + final ResourceKey resourceKey = PoliciesResourceType.thingResource("/"); + + final SubjectClassification classification = enforcer.classifySubjects(resourceKey, READ); + assertMatchesIndividualVisitors(enforcer, resourceKey, READ, classification); + + assertThat(classification.getUnrestricted()).isEmpty(); + assertThat(classification.getPartialOnly()).isEmpty(); + assertThat(classification.getEffectedGranted()).isEmpty(); + } + + @Test + public void nestedResourceQueryMatchesIndividualVisitors() { + final AuthorizationSubject subjectA = AuthorizationSubject.newInstance("test:a"); + final AuthorizationSubject subjectB = AuthorizationSubject.newInstance("test:b"); + + final PolicyId policyId = PolicyId.of("ns", "nested"); + final Policy policy = Policy.newBuilder(policyId) + .forLabel("a-full") + .setSubject(subjectA.getId(), SubjectType.GENERATED) + .setGrantedPermissions(PoliciesResourceType.thingResource("/"), READ) + .forLabel("b-features") + .setSubject(subjectB.getId(), SubjectType.GENERATED) + .setGrantedPermissions(PoliciesResourceType.thingResource("/features"), READ) + .setRevokedPermissions( + PoliciesResourceType.thingResource("/features/private"), READ) + .build(); + + final TreeBasedPolicyEnforcer enforcer = TreeBasedPolicyEnforcer.createInstance(policy); + + // Query at /features level + final ResourceKey featuresKey = PoliciesResourceType.thingResource("/features"); + final SubjectClassification classification = enforcer.classifySubjects(featuresKey, READ); + assertMatchesIndividualVisitors(enforcer, featuresKey, READ, classification); + } + + @Test + public void multiplePermissionsReadWriteMatchesIndividualVisitors() { + final AuthorizationSubject rwSubject = AuthorizationSubject.newInstance("test:rw"); + final AuthorizationSubject rOnlySubject = AuthorizationSubject.newInstance("test:r-only"); + + final PolicyId policyId = PolicyId.of("ns", "multi-perm"); + final Policy policy = Policy.newBuilder(policyId) + .forLabel("rw-full") + .setSubject(rwSubject.getId(), SubjectType.GENERATED) + .setGrantedPermissions(PoliciesResourceType.thingResource("/"), READ_WRITE) + .forLabel("r-only") + .setSubject(rOnlySubject.getId(), SubjectType.GENERATED) + .setGrantedPermissions(PoliciesResourceType.thingResource("/"), READ) + .build(); + + final TreeBasedPolicyEnforcer enforcer = TreeBasedPolicyEnforcer.createInstance(policy); + final ResourceKey resourceKey = PoliciesResourceType.thingResource("/"); + + // Check READ_WRITE: rOnlySubject should NOT appear as it only has READ + final SubjectClassification classification = enforcer.classifySubjects(resourceKey, READ_WRITE); + assertMatchesIndividualVisitors(enforcer, resourceKey, READ_WRITE, classification); + + assertThat(classification.getUnrestricted()).containsExactly(rwSubject); + assertThat(classification.getEffectedGranted()).containsExactly(rwSubject); + } + + @Test + public void complexScenarioWithGrantRevokeReGrant() { + final AuthorizationSubject subject = AuthorizationSubject.newInstance("test:complex"); + + final PolicyId policyId = PolicyId.of("ns", "complex"); + final Policy policy = Policy.newBuilder(policyId) + .forLabel("root-grant") + .setSubject(subject.getId(), SubjectType.GENERATED) + .setGrantedPermissions(PoliciesResourceType.thingResource("/"), READ) + .forLabel("attributes-revoke") + .setSubject(subject.getId(), SubjectType.GENERATED) + .setRevokedPermissions(PoliciesResourceType.thingResource("/attributes"), READ) + .forLabel("location-re-grant") + .setSubject(subject.getId(), SubjectType.GENERATED) + .setGrantedPermissions( + PoliciesResourceType.thingResource("/attributes/location"), READ) + .build(); + + final TreeBasedPolicyEnforcer enforcer = TreeBasedPolicyEnforcer.createInstance(policy); + final ResourceKey resourceKey = PoliciesResourceType.thingResource("/"); + + final SubjectClassification classification = enforcer.classifySubjects(resourceKey, READ); + assertMatchesIndividualVisitors(enforcer, resourceKey, READ, classification); + + // Has revoke below root -> not unrestricted, but partial (has grants at/below root) + assertThat(classification.getUnrestricted()).doesNotContain(subject); + assertThat(classification.getPartialOnly()).contains(subject); + } + + @Test + public void differentResourceTypesAreIsolated() { + final AuthorizationSubject subject = AuthorizationSubject.newInstance("test:policy-only"); + + final PolicyId policyId = PolicyId.of("ns", "type-isolation"); + final Policy policy = Policy.newBuilder(policyId) + .forLabel("policy-grant") + .setSubject(subject.getId(), SubjectType.GENERATED) + .setGrantedPermissions(PoliciesResourceType.policyResource("/"), READ) + .build(); + + final TreeBasedPolicyEnforcer enforcer = TreeBasedPolicyEnforcer.createInstance(policy); + + // Query thing resource -> should be empty + final ResourceKey thingKey = PoliciesResourceType.thingResource("/"); + final SubjectClassification thingClassification = enforcer.classifySubjects(thingKey, READ); + assertMatchesIndividualVisitors(enforcer, thingKey, READ, thingClassification); + + assertThat(thingClassification.getUnrestricted()).isEmpty(); + assertThat(thingClassification.getPartialOnly()).isEmpty(); + assertThat(thingClassification.getEffectedGranted()).isEmpty(); + + // Query policy resource -> should have the subject + final ResourceKey policyKey = PoliciesResourceType.policyResource("/"); + final SubjectClassification policyClassification = enforcer.classifySubjects(policyKey, READ); + assertMatchesIndividualVisitors(enforcer, policyKey, READ, policyClassification); + + assertThat(policyClassification.getUnrestricted()).containsExactly(subject); + } + + @Test + public void manySubjectsWithOverlappingPermissions() { + final AuthorizationSubject s1 = AuthorizationSubject.newInstance("test:s1"); + final AuthorizationSubject s2 = AuthorizationSubject.newInstance("test:s2"); + final AuthorizationSubject s3 = AuthorizationSubject.newInstance("test:s3"); + final AuthorizationSubject s4 = AuthorizationSubject.newInstance("test:s4"); + + final PolicyId policyId = PolicyId.of("ns", "many"); + final Policy policy = Policy.newBuilder(policyId) + // s1: full unrestricted + .forLabel("s1") + .setSubject(s1.getId(), SubjectType.GENERATED) + .setGrantedPermissions(PoliciesResourceType.thingResource("/"), READ) + // s2: partial only (grant below) + .forLabel("s2") + .setSubject(s2.getId(), SubjectType.GENERATED) + .setGrantedPermissions( + PoliciesResourceType.thingResource("/features/public"), READ) + // s3: grant at root but revoked below -> partial + .forLabel("s3-grant") + .setSubject(s3.getId(), SubjectType.GENERATED) + .setGrantedPermissions(PoliciesResourceType.thingResource("/"), READ) + .forLabel("s3-revoke") + .setSubject(s3.getId(), SubjectType.GENERATED) + .setRevokedPermissions( + PoliciesResourceType.thingResource("/features/private"), READ) + // s4: only WRITE, no READ + .forLabel("s4") + .setSubject(s4.getId(), SubjectType.GENERATED) + .setGrantedPermissions(PoliciesResourceType.thingResource("/"), + Permissions.newInstance("WRITE")) + .build(); + + final TreeBasedPolicyEnforcer enforcer = TreeBasedPolicyEnforcer.createInstance(policy); + final ResourceKey resourceKey = PoliciesResourceType.thingResource("/"); + + final SubjectClassification classification = enforcer.classifySubjects(resourceKey, READ); + assertMatchesIndividualVisitors(enforcer, resourceKey, READ, classification); + + assertThat(classification.getUnrestricted()).containsExactly(s1); + assertThat(classification.getPartialOnly()).containsExactlyInAnyOrder(s2, s3); + assertThat(classification.getEffectedGranted()).containsExactlyInAnyOrder(s1, s3); + } + + /** + * Asserts that the classification result matches what the three individual visitor methods return. + */ + private static void assertMatchesIndividualVisitors( + final TreeBasedPolicyEnforcer enforcer, + final ResourceKey resourceKey, + final Permissions permissions, + final SubjectClassification classification) { + + final Set partial = + enforcer.getSubjectsWithPartialPermission(resourceKey, permissions); + final Set unrestricted = + enforcer.getSubjectsWithUnrestrictedPermission(resourceKey, permissions); + final EffectedSubjects effected = enforcer.getSubjectsWithPermission(resourceKey, permissions); + + // partialOnly = partial - unrestricted + final Set expectedPartialOnly = new HashSet<>(partial); + expectedPartialOnly.removeAll(unrestricted); + + assertThat(classification.getUnrestricted()) + .as("unrestricted subjects") + .containsExactlyInAnyOrderElementsOf(unrestricted); + assertThat(classification.getPartialOnly()) + .as("partialOnly subjects") + .containsExactlyInAnyOrderElementsOf(expectedPartialOnly); + assertThat(classification.getEffectedGranted()) + .as("effectedGranted subjects") + .containsExactlyInAnyOrderElementsOf(effected.getGranted()); + } + +} diff --git a/things/service/src/main/java/org/eclipse/ditto/things/service/persistence/actors/enrichment/ThingEventEnricher.java b/things/service/src/main/java/org/eclipse/ditto/things/service/persistence/actors/enrichment/ThingEventEnricher.java index 27a7da9bf6..cf949fc395 100644 --- a/things/service/src/main/java/org/eclipse/ditto/things/service/persistence/actors/enrichment/ThingEventEnricher.java +++ b/things/service/src/main/java/org/eclipse/ditto/things/service/persistence/actors/enrichment/ThingEventEnricher.java @@ -108,8 +108,11 @@ public > CompletionStage enrichWi @Nullable final PolicyId policyId, final T withDittoHeaders ) { + // Cache thing.toJson() once to avoid redundant serialization across all enrichment steps + final JsonObject thingJson = thing != null ? thing.toJson() : null; + final CompletionStage partialAccessPathsStage = - enrichWithPartialAccessPathsIfNecessary(withDittoHeaders, thing, policyId); + enrichWithPartialAccessPathsIfNecessary(withDittoHeaders, thing, policyId, thingJson); if (null != thing && !preDefinedExtraFieldsConfigs.isEmpty()) { final List matchingPreDefinedFieldsConfigs = @@ -130,28 +133,28 @@ public > CompletionStage enrichWi combinedPointerSet.addAll(b.getPointers()); return JsonFactory.newFieldSelector(combinedPointerSet); }); - + if (combinedPredefinedExtraFields.isEmpty()) { return partialAccessPathsStage; } - + return partialAccessPathsStage.thenCompose(enrichedWithPartialAccessPaths -> { final DittoHeaders dittoHeaders = enrichedWithPartialAccessPaths.getDittoHeaders(); return buildPredefinedExtraFieldsHeaderReadGrantObject( - policyId, combinedPredefinedExtraFields, thing, dittoHeaders) + policyId, combinedPredefinedExtraFields, thingJson, dittoHeaders) .thenApply(indexedGrants -> { final String extraFieldsKey = DittoHeaderDefinition.PRE_DEFINED_EXTRA_FIELDS.getKey(); final String readGrantKey = DittoHeaderDefinition.PRE_DEFINED_EXTRA_FIELDS_READ_GRANT_OBJECT.getKey(); final String extraFieldsObjectKey = DittoHeaderDefinition.PRE_DEFINED_EXTRA_FIELDS_OBJECT.getKey(); - + final DittoHeadersBuilder headersBuilder = dittoHeaders.toBuilder() .putHeader(extraFieldsKey, buildPredefinedExtraFieldsHeaderList(combinedPredefinedExtraFields)) .putHeader(readGrantKey, indexedGrants.pathsToJson().toString()) .putHeader(extraFieldsObjectKey, - buildPredefinedExtraFieldsHeaderObject(thing, + buildPredefinedExtraFieldsHeaderObject(thingJson, combinedPredefinedExtraFields).toString()); - + return enrichedWithPartialAccessPaths.setDittoHeaders(headersBuilder.build()); }); }); @@ -163,7 +166,8 @@ public > CompletionStage enrichWi private > CompletionStage enrichWithPartialAccessPathsIfNecessary( final T withDittoHeaders, @Nullable final Thing thing, - @Nullable final PolicyId policyId) { + @Nullable final PolicyId policyId, + @Nullable final JsonObject thingJson) { if (!partialAccessEventsEnabled) { return CompletableFuture.completedStage(withDittoHeaders); @@ -173,7 +177,7 @@ private > CompletionStage enrichW return CompletableFuture.completedStage(withDittoHeaders); } - return enrichWithPartialAccessPaths(thingEvent, thing, policyId) + return enrichWithPartialAccessPaths(thingEvent, thing, policyId, thingJson) .thenApply(partialAccessPathsJson -> { if (hasNoPartialSubjects(partialAccessPathsJson)) { return withDittoHeaders; @@ -254,7 +258,7 @@ private CompletionStage> getPolicyEnforcerSafely(@Nulla private CompletionStage buildPredefinedExtraFieldsHeaderReadGrantObject( @Nullable final PolicyId policyId, final JsonFieldSelector preDefinedExtraFields, - final Thing thing, + final JsonObject thingJson, final DittoHeaders dittoHeaders ) { return getPolicyEnforcerSafely(policyId).thenApply(policyEnforcerOpt -> { @@ -268,7 +272,7 @@ private CompletionStage buildPredefinedExtraFieldsHeaderReadGr final ReadGrant readGrant = ReadGrantCollector.collect( preDefinedExtraFields, - thing, + thingJson, policyEnforcer ); @@ -287,12 +291,14 @@ private CompletionStage buildPredefinedExtraFieldsHeaderReadGr private CompletionStage enrichWithPartialAccessPaths( final ThingEvent thingEvent, final Thing thing, - @Nullable final PolicyId policyId + @Nullable final PolicyId policyId, + @Nullable final JsonObject thingJson ) { final DittoHeaders dittoHeaders = thingEvent.getDittoHeaders(); LOGGER.withCorrelationId(dittoHeaders) .debug("Enriching event '{}' (thingId: {}) with partial access paths, policyId: {}", thingEvent.getType(), thingEvent.getEntityId(), policyId); + final JsonObject effectiveThingJson = thingJson != null ? thingJson : thing.toJson(); return getPolicyEnforcerSafely(policyId).thenApply(policyEnforcerOpt -> { if (policyEnforcerOpt.isEmpty()) { LOGGER.withCorrelationId(dittoHeaders) @@ -302,7 +308,7 @@ private CompletionStage enrichWithPartialAccessPaths( } final Map> partialAccessPaths = PartialAccessPathCalculator.calculatePartialAccessPaths( - thing, policyEnforcerOpt.get()); + thing, policyEnforcerOpt.get(), effectiveThingJson); // Return consistent empty structure (will be filtered out in caller) if (partialAccessPaths.isEmpty()) { return createEmptyPartialAccessPathsJson(); @@ -365,15 +371,13 @@ private > T addPartialAccessPathsHea } /** - * Builds JSON object for predefined extra fields, optimized to avoid full thing.toJson() when possible. - * For large Things, this reduces memory allocation significantly. + * Builds JSON object for predefined extra fields from the pre-computed thing JSON. */ private static JsonObject buildPredefinedExtraFieldsHeaderObject( - final Thing thing, + final JsonObject thingJson, final JsonFieldSelector preDefinedExtraFields ) { final JsonObjectBuilder builder = JsonObject.newBuilder(); - final JsonObject thingJson = thing.toJson(); preDefinedExtraFields.getPointers().forEach(pointer -> thingJson.getValue(pointer).ifPresent(thingValue -> builder.set(pointer, thingValue)) ); diff --git a/things/service/src/main/java/org/eclipse/ditto/things/service/utils/PartialAccessPathCalculator.java b/things/service/src/main/java/org/eclipse/ditto/things/service/utils/PartialAccessPathCalculator.java index 888a8b4b0c..66bda74456 100644 --- a/things/service/src/main/java/org/eclipse/ditto/things/service/utils/PartialAccessPathCalculator.java +++ b/things/service/src/main/java/org/eclipse/ditto/things/service/utils/PartialAccessPathCalculator.java @@ -21,9 +21,7 @@ import javax.annotation.Nullable; -import org.eclipse.ditto.base.model.auth.AuthorizationContext; import org.eclipse.ditto.base.model.auth.AuthorizationSubject; -import org.eclipse.ditto.base.model.auth.DittoAuthorizationContextType; import org.eclipse.ditto.base.model.json.FieldType; import org.eclipse.ditto.json.JsonArray; import org.eclipse.ditto.json.JsonArrayBuilder; @@ -38,6 +36,7 @@ import org.eclipse.ditto.policies.model.PoliciesResourceType; import org.eclipse.ditto.policies.model.ResourceKey; import org.eclipse.ditto.policies.model.enforcers.Enforcer; +import org.eclipse.ditto.policies.model.enforcers.SubjectClassification; import org.eclipse.ditto.things.model.Thing; /** @@ -94,106 +93,73 @@ public static Map> calculatePartialAccessPaths( return Map.of(); } - final Enforcer enforcer = policyEnforcer.getEnforcer(); - final var rootResourceKey = PoliciesResourceType.thingResource(ROOT_RESOURCE_POINTER); - - final Set subjectsWithPartialPermission = - enforcer.getSubjectsWithPartialPermission(rootResourceKey, READ_PERMISSIONS); - - if (subjectsWithPartialPermission.isEmpty()) { - return Map.of(); - } - - final Set subjectsWithRestrictedAccess = - filterSubjectsWithRestrictedAccess(subjectsWithPartialPermission, enforcer, rootResourceKey); - - if (subjectsWithRestrictedAccess.isEmpty()) { - return Map.of(); - } - - return calculateAccessiblePathsForSubjects(subjectsWithRestrictedAccess, thing, enforcer); + return calculatePartialAccessPaths(thing, policyEnforcer, thing.toJson()); } /** - * Filters out subjects that have full access to the root resource. - * Only subjects with truly restricted (partial) access are returned. + * Calculates which JSON paths each subject with partial READ permissions can access, + * using a pre-computed {@code thingJson} to avoid redundant serialization. * - * @param subjectsWithPartialPermission all subjects with partial permission - * @param enforcer the Enforcer to query - * @param rootResourceKey the root resource key - * @return set of subjects with restricted access (excluding those with full root access) + * @param thing the current Thing state (may be null for ThingDeleted events) + * @param policyEnforcer the PolicyEnforcer to use for permission checks (must not be null) + * @param thingJson the pre-computed JSON representation of the Thing + * @return a map from subject ID to list of accessible JsonPointer paths, empty map if no partial subjects exist + * @throws NullPointerException if policyEnforcer is null */ - private static Set filterSubjectsWithRestrictedAccess( - final Set subjectsWithPartialPermission, - final Enforcer enforcer, - final ResourceKey rootResourceKey) { + public static Map> calculatePartialAccessPaths( + @Nullable final Thing thing, + final PolicyEnforcer policyEnforcer, + final JsonObject thingJson) { - final var effectedSubjects = enforcer.getSubjectsWithPermission(rootResourceKey, READ_PERMISSIONS); - final Set subjectsWithPermissionOnRoot = effectedSubjects.getGranted(); + if (thing == null) { + return Map.of(); + } - final Set subjectsWithUnrestrictedPermission = - enforcer.getSubjectsWithUnrestrictedPermission(rootResourceKey, READ_PERMISSIONS); + final Enforcer enforcer = policyEnforcer.getEnforcer(); + final ResourceKey rootResourceKey = PoliciesResourceType.thingResource(ROOT_RESOURCE_POINTER); - final Set subjectsWithFullAccessOnRoot = new LinkedHashSet<>(subjectsWithPermissionOnRoot); - subjectsWithFullAccessOnRoot.retainAll(subjectsWithUnrestrictedPermission); + final SubjectClassification classification = enforcer.classifySubjects(rootResourceKey, READ_PERMISSIONS); - final Set subjectsWithRestrictedAccess = - new LinkedHashSet<>(subjectsWithPartialPermission); - subjectsWithRestrictedAccess.removeAll(subjectsWithFullAccessOnRoot); + // Reconstruct "restricted" = partial - (effectedGranted ∩ unrestricted) + final Set allPartial = new LinkedHashSet<>(classification.getPartialOnly()); + allPartial.addAll(classification.getUnrestricted()); + final Set fullAccessOnRoot = new LinkedHashSet<>(classification.getEffectedGranted()); + fullAccessOnRoot.retainAll(classification.getUnrestricted()); + final Set subjectsWithRestrictedAccess = new LinkedHashSet<>(allPartial); + subjectsWithRestrictedAccess.removeAll(fullAccessOnRoot); - return subjectsWithRestrictedAccess; + if (subjectsWithRestrictedAccess.isEmpty()) { + return Map.of(); + } + + return calculateAccessiblePathsForSubjects(subjectsWithRestrictedAccess, thingJson, enforcer); } /** - * Calculates accessible paths for a set of subjects with restricted access. + * Calculates accessible paths for a set of subjects with restricted access using a batch call. * * @param subjectsWithRestrictedAccess subjects to calculate paths for - * @param thing the Thing entity + * @param thingJson the Thing JSON representation * @param enforcer the Enforcer to use * @return map of subject ID to their accessible paths */ private static Map> calculateAccessiblePathsForSubjects( final Set subjectsWithRestrictedAccess, - final Thing thing, + final JsonObject thingJson, final Enforcer enforcer) { - final Map> result = new LinkedHashMap<>(); - final JsonObject thingJson = thing.toJson(); + final ResourceKey resourceKey = PoliciesResourceType.thingResource(ROOT_RESOURCE_POINTER); + final Map> batchResult = + enforcer.getAccessiblePathsForSubjects(resourceKey, thingJson, + subjectsWithRestrictedAccess, READ_PERMISSIONS); - for (final AuthorizationSubject subject : subjectsWithRestrictedAccess) { - final List accessiblePaths = calculateAccessiblePathsForSubject( - subject, thingJson, enforcer); - if (!accessiblePaths.isEmpty()) { - result.put(subject.getId(), accessiblePaths); - } + final Map> result = new LinkedHashMap<>(); + for (final Map.Entry> entry : batchResult.entrySet()) { + result.put(entry.getKey().getId(), new ArrayList<>(entry.getValue())); } - return result; } - /** - * Calculates which paths a specific subject can access. - * - * @param subject the subject to check - * @param thingJson the Thing JSON representation - * @param enforcer the Enforcer to use for permission checks - * @return list of accessible JsonPointer paths for the subject - */ - private static List calculateAccessiblePathsForSubject( - final AuthorizationSubject subject, - final JsonObject thingJson, - final Enforcer enforcer) { - - final Set accessiblePaths = enforcer.getAccessiblePaths( - PoliciesResourceType.thingResource(ROOT_RESOURCE_POINTER), - thingJson, - subject, - READ_PERMISSIONS - ); - - return new ArrayList<>(accessiblePaths); - } - /** * Converts a map of subject IDs to accessible paths into an indexed JSON object format. *

diff --git a/things/service/src/main/java/org/eclipse/ditto/things/service/utils/ReadGrantCollector.java b/things/service/src/main/java/org/eclipse/ditto/things/service/utils/ReadGrantCollector.java index 2ff9b2f018..f42e0838dd 100644 --- a/things/service/src/main/java/org/eclipse/ditto/things/service/utils/ReadGrantCollector.java +++ b/things/service/src/main/java/org/eclipse/ditto/things/service/utils/ReadGrantCollector.java @@ -12,16 +12,13 @@ */ package org.eclipse.ditto.things.service.utils; -import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; -import org.eclipse.ditto.base.model.auth.AuthorizationContext; import org.eclipse.ditto.base.model.auth.AuthorizationSubject; -import org.eclipse.ditto.base.model.auth.DittoAuthorizationContextType; import org.eclipse.ditto.json.JsonFieldSelector; import org.eclipse.ditto.json.JsonObject; import org.eclipse.ditto.json.JsonPointer; @@ -32,6 +29,7 @@ import org.eclipse.ditto.policies.model.PoliciesResourceType; import org.eclipse.ditto.policies.model.ResourceKey; import org.eclipse.ditto.policies.model.enforcers.Enforcer; +import org.eclipse.ditto.policies.model.enforcers.SubjectClassification; import org.eclipse.ditto.things.model.Thing; /** @@ -57,13 +55,30 @@ public static ReadGrant collect( final Thing thing, final PolicyEnforcer policyEnforcer ) { + return collect(fields, thing.toJson(), policyEnforcer); + } + /** + * Collects read grants for the specified fields from the enforcer, using a pre-computed {@code thingJson} + * to avoid redundant serialization. + * + * @param fields the fields to collect grants for + * @param thingJson the pre-computed JSON representation of the Thing + * @param policyEnforcer the PolicyEnforcer to query + * @return ReadGrant mapping paths to sets of subject IDs + */ + public static ReadGrant collect( + final JsonFieldSelector fields, + final JsonObject thingJson, + final PolicyEnforcer policyEnforcer + ) { final Enforcer enforcer = policyEnforcer.getEnforcer(); final Map> pointerToSubjects = new LinkedHashMap<>(); for (final JsonPointer pointer : fields.getPointers()) { - final Map> pathsForPointer = collectSubjectsForPointer(pointer, thing, enforcer); - + final Map> pathsForPointer = + collectSubjectsForPointer(pointer, thingJson, enforcer); + for (final Map.Entry> entry : pathsForPointer.entrySet()) { pointerToSubjects.merge(entry.getKey(), entry.getValue(), (existing, newSet) -> { final Set merged = new LinkedHashSet<>(existing); @@ -78,29 +93,24 @@ public static ReadGrant collect( /** * Collects all subject IDs and their accessible nested paths for the specified pointer. - * For partial subjects, collects all nested paths they can access. + * Uses {@code classifySubjects()} for a single tree walk and batch path calculation. * * @param pointer the pointer to check - * @param thing the Thing entity + * @param thingJson the Thing JSON representation * @param enforcer the Enforcer to query * @return map of JsonPointer paths to sets of subject IDs */ private static Map> collectSubjectsForPointer( final JsonPointer pointer, - final Thing thing, + final JsonObject thingJson, final Enforcer enforcer ) { - final Set subjectsWithUnrestrictedPermission = - enforcer.getSubjectsWithUnrestrictedPermission( - PoliciesResourceType.thingResource(pointer), - Permissions.newInstance(Permission.READ) - ); - - final Set subjectsWithPartialPermission = - enforcer.getSubjectsWithPartialPermission( - PoliciesResourceType.thingResource(pointer), - Permissions.newInstance(Permission.READ) - ); + final ResourceKey resourceKey = PoliciesResourceType.thingResource(pointer); + final Permissions readPermissions = Permissions.newInstance(Permission.READ); + + final SubjectClassification classification = enforcer.classifySubjects(resourceKey, readPermissions); + final Set subjectsWithUnrestrictedPermission = classification.getUnrestricted(); + final Set partialOnly = classification.getPartialOnly(); final Map> result = new LinkedHashMap<>(); @@ -111,28 +121,19 @@ private static Map> collectSubjectsForPointer( result.put(pointer, unrestrictedIds); } - if (!subjectsWithPartialPermission.equals(subjectsWithUnrestrictedPermission)) { - final Set partialOnly = new HashSet<>(subjectsWithPartialPermission); - partialOnly.removeAll(subjectsWithUnrestrictedPermission); - - final JsonObject thingJson = thing.toJson(); + if (!partialOnly.isEmpty()) { final JsonValue pointerValue = thingJson.getValue(pointer).orElse(null); - + if (pointerValue != null && pointerValue.isObject()) { final JsonObject pointerObject = pointerValue.asObject(); - final ResourceKey resourceKey = PoliciesResourceType.thingResource(pointer); - final Permissions readPermissions = Permissions.newInstance(Permission.READ); - - for (final AuthorizationSubject subject : partialOnly) { - final Set accessiblePaths = enforcer.getAccessiblePaths( - resourceKey, - pointerObject, - subject, - readPermissions - ); - - for (final JsonPointer accessiblePath : accessiblePaths) { - result.computeIfAbsent(accessiblePath, k -> new LinkedHashSet<>()).add(subject.getId()); + + final Map> batchResult = + enforcer.getAccessiblePathsForSubjects(resourceKey, pointerObject, + partialOnly, readPermissions); + for (final Map.Entry> entry : batchResult.entrySet()) { + for (final JsonPointer accessiblePath : entry.getValue()) { + result.computeIfAbsent(accessiblePath, k -> new LinkedHashSet<>()) + .add(entry.getKey().getId()); } } }