Skip to content

Commit

Permalink
#1593 added special "merge command" enforcement for patches including…
Browse files Browse the repository at this point in the history 
… a regex: in that case, unrestricted WRITE access on the contained JSON object is required

Signed-off-by: Thomas Jäckle <thomas.jaeckle@beyonnex.io>
  • Loading branch information
@thjaeckle
thjaeckle authored and Stanchev Aleksandar committed Apr 18, 2023
1 parent 75e7280 commit 915e68b
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 2 deletions.
22 changes: 21 additions & 1 deletion ...e/src/main/java/org/eclipse/ditto/things/service/enforcement/ThingCommandEnforcement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.eclipse.ditto.internal.utils.cacheloaders.config.AskWithRetryConfig;
import org.eclipse.ditto.json.JsonFactory;
import org.eclipse.ditto.json.JsonFieldSelector;
import org.eclipse.ditto.json.JsonKey;
import org.eclipse.ditto.json.JsonObject;
import org.eclipse.ditto.json.JsonPointer;
import org.eclipse.ditto.json.JsonPointerInvalidException;
Expand Down Expand Up @@ -503,7 +504,15 @@ private static boolean enforceMergeThingCommand(final Enforcer enforcer,
private static Set<ResourceKey> calculateLeaves(final JsonPointer path, final JsonValue value) {
if (value.isObject()) {
return value.asObject().stream()
.map(f -> calculateLeaves(path.append(f.getKey().asPointer()), f.getValue()))
.map(f -> {
final JsonKey key = f.getKey();
if (isMergeWithNulledKeysByRegex(key, f.getValue())) {
// if regex is contained, writing all below that path must be granted in the policy:
return Set.of(PoliciesResourceType.thingResource(path));
} else {
return calculateLeaves(path.append(key.asPointer()), f.getValue());
}
})
.reduce(new HashSet<>(), ThingCommandEnforcement::addAll, ThingCommandEnforcement::addAll);
} else {
return Set.of(PoliciesResourceType.thingResource(path));
Expand All @@ -515,4 +524,15 @@ private static Set<ResourceKey> addAll(final Set<ResourceKey> result, final Set<
return result;
}

private static boolean isMergeWithNulledKeysByRegex(final JsonKey key, final JsonValue value) {
final String keyString = key.toString();
if (keyString.startsWith("{{") && keyString.endsWith("}}")) {
final String keyRegexWithoutCurly = keyString.substring(2, keyString.length() - 2).trim();
return keyRegexWithoutCurly.startsWith("/") && keyRegexWithoutCurly.endsWith("/") &&
value.isNull();
} else {
return false;
}
}

}
42 changes: 41 additions & 1 deletion ...t/java/org/eclipse/ditto/things/service/enforcement/MergeThingCommandEnforcementTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.eclipse.ditto.json.JsonFactory;
import org.eclipse.ditto.json.JsonObject;
import org.eclipse.ditto.json.JsonPointer;
import org.eclipse.ditto.json.JsonValue;
import org.eclipse.ditto.policies.model.EffectedPermissions;
import org.eclipse.ditto.policies.model.PoliciesModelFactory;
import org.eclipse.ditto.policies.model.Policy;
Expand All @@ -41,6 +42,7 @@
import org.eclipse.ditto.things.api.Permission;
import org.eclipse.ditto.things.model.signals.commands.exceptions.ThingNotModifiableException;
import org.eclipse.ditto.things.model.signals.commands.modify.MergeThing;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
Expand Down Expand Up @@ -119,6 +121,44 @@ void rejectByPolicy(final TestArgument arg) {
() -> ThingCommandEnforcement.authorizeByPolicyOrThrow(policyEnforcer, arg.getMergeThing()));
}

@Test
void acceptUsingRegexInPolicy() {
final TestArgument testArgument = TestArgument.of("/",
Set.of("/", "/attributes/complex"),
Set.of("/attributes/simple")
);
final TrieBasedPolicyEnforcer policyEnforcer = TrieBasedPolicyEnforcer.newInstance(testArgument.getPolicy());
final JsonObject patch = JsonObject.newBuilder()
.set("attributes", JsonObject.newBuilder()
.set("complex", JsonObject.newBuilder()
.set("{{ /.*/ }}", JsonValue.nullLiteral())
.build()
)
.set("another", 42)
.build())
.build();
final MergeThing mergeThing = MergeThing.of(TestSetup.THING_ID, JsonPointer.empty(), patch, TestArgument.headers());
final MergeThing authorizedMergeThing =
ThingCommandEnforcement.authorizeByPolicyOrThrow(policyEnforcer, mergeThing);
assertThat(authorizedMergeThing.getDittoHeaders().getAuthorizationContext()).isNotNull();
}

@Test
void rejectUsingRegexWithRevokesInPolicy() {
final TestArgument testArgument = TestArgument.of("/", Set.of("/"), Set.of("/attributes/complex/nested/secret"));
final TrieBasedPolicyEnforcer policyEnforcer = TrieBasedPolicyEnforcer.newInstance(testArgument.getPolicy());
final JsonObject patch = JsonObject.newBuilder()
.set("attributes", JsonObject.newBuilder()
.set("{{ /.*/ }}", JsonValue.nullLiteral())
.set("simple", "value")
.set("another", 42)
.build())
.build();
final MergeThing mergeThing = MergeThing.of(TestSetup.THING_ID, JsonPointer.empty(), patch, TestArgument.headers());
assertThatExceptionOfType(ThingNotModifiableException.class).isThrownBy(
() -> ThingCommandEnforcement.authorizeByPolicyOrThrow(policyEnforcer, mergeThing));
}

/**
* Generates combinations of a Policy and MergeThing commands that should be <b>accepted</b> by command enforcement.
*/
Expand Down Expand Up @@ -244,7 +284,7 @@ private static MergeThing toMergeCommand(final JsonPointer path, final JsonObjec
return MergeThing.of(TestSetup.THING_ID, path, patch.getValue(path).orElseThrow(), headers());
}

private static DittoHeaders headers() {
static DittoHeaders headers() {
return DittoHeaders.newBuilder()
.authorizationContext(
AuthorizationContext.newInstance(DittoAuthorizationContextType.UNSPECIFIED,
Expand Down

0 comments on commit 915e68b

Please sign in to comment.