feat: rework always-true policy definition (sovity#1005)

Co-authored-by: Richard Treier <[email protected]>
truzzt · Jul 30, 2024 · 88bdee4 · 88bdee4
1 parent 11cc466
commit 88bdee4
Show file tree

Hide file tree

Showing 18 changed files with 319 additions and 410 deletions.
diff --git a/.env b/.env
@@ -1,5 +1,5 @@
 # Env variables for docker-compose.yaml
-EDC_IMAGE=ghcr.io/sovity/edc-dev:9.0.0
-TEST_BACKEND_IMAGE=ghcr.io/sovity/test-backend:9.0.0
-EDC_UI_IMAGE=ghcr.io/sovity/edc-ui:4.0.0
+EDC_IMAGE=ghcr.io/sovity/edc-dev:10.0.0
+TEST_BACKEND_IMAGE=ghcr.io/sovity/test-backend:10.0.0
+EDC_UI_IMAGE=ghcr.io/sovity/edc-ui:4.1.0
 EDC_UI_ACTIVE_PROFILE=sovity-open-source
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,8 @@ please see [changelog_updates.md](docs/dev/changelog_updates.md).
 
 - Both providers and consumers can now terminate their contracts.
 - Added endpoints for checking ID availability for policies, assets and contract definitions
+- The always-true policy is now created with no constraints instead of the artificial `ALWAYS_TRUE = TRUE` constraint
+  - Existing always-true policy definitions are migrated to the new format - existing contract agreements are not affected
 
 #### Patch Changes
 

diff --git a/docker-compose.yaml b/docker-compose.yaml
@@ -42,6 +42,7 @@ services:
       EDC_WEB_REST_CORS_HEADERS: 'origin,content-type,accept,authorization,X-Api-Key'
       EDC_WEB_REST_CORS_ORIGINS: '*'
       EDC_AGENT_IDENTITY_KEY: 'client_id' # required for Mock IAM to work
+
     ports:
       - '11001:11001'
       - '11002:11002'
@@ -89,6 +90,7 @@ services:
       EDC_WEB_REST_CORS_HEADERS: 'origin,content-type,accept,authorization,X-Api-Key'
       EDC_WEB_REST_CORS_ORIGINS: '*'
       EDC_AGENT_IDENTITY_KEY: 'client_id' # required for Mock IAM to work
+
     ports:
       - '22001:11001'
       - '22002:11002'

diff --git a/.../main/java/de/sovity/edc/extension/policy/services/AlwaysTruePolicyDefinitionService.java b/.../main/java/de/sovity/edc/extension/policy/services/AlwaysTruePolicyDefinitionService.java
@@ -17,14 +17,9 @@
 import org.eclipse.edc.connector.policy.spi.PolicyDefinition;
 import org.eclipse.edc.connector.spi.policydefinition.PolicyDefinitionService;
 import org.eclipse.edc.policy.model.Action;
-import org.eclipse.edc.policy.model.AtomicConstraint;
-import org.eclipse.edc.policy.model.LiteralExpression;
-import org.eclipse.edc.policy.model.Operator;
 import org.eclipse.edc.policy.model.Permission;
 import org.eclipse.edc.policy.model.Policy;
 
-import static de.sovity.edc.extension.policy.AlwaysTruePolicyConstants.EXPRESSION_LEFT_VALUE;
-import static de.sovity.edc.extension.policy.AlwaysTruePolicyConstants.EXPRESSION_RIGHT_VALUE;
 import static de.sovity.edc.extension.policy.AlwaysTruePolicyConstants.POLICY_DEFINITION_ID;
 
 /**
@@ -50,22 +45,16 @@ public boolean exists() {
      * Creates policy definition &quot;always-true&quot;.
      */
     public void create() {
-        var alwaysTrueConstraint = AtomicConstraint.Builder.newInstance()
-                .leftExpression(new LiteralExpression(EXPRESSION_LEFT_VALUE))
-                .operator(Operator.EQ)
-                .rightExpression(new LiteralExpression(EXPRESSION_RIGHT_VALUE))
-                .build();
         var alwaysTruePermission = Permission.Builder.newInstance()
-                .action(Action.Builder.newInstance().type("USE").build())
-                .constraint(alwaysTrueConstraint)
-                .build();
+            .action(Action.Builder.newInstance().type("USE").build())
+            .build();
         var policy = Policy.Builder.newInstance()
-                .permission(alwaysTruePermission)
-                .build();
+            .permission(alwaysTruePermission)
+            .build();
         var policyDefinition = PolicyDefinition.Builder.newInstance()
-                .id(POLICY_DEFINITION_ID)
-                .policy(policy)
-                .build();
+            .id(POLICY_DEFINITION_ID)
+            .policy(policy)
+            .build();
         policyDefinitionService.create(policyDefinition);
     }
 }
diff --git a/...sions/postgres-flyway/src/main/resources/db/migration/V11__migrate_always_true_policy.sql b/...sions/postgres-flyway/src/main/resources/db/migration/V11__migrate_always_true_policy.sql
@@ -0,0 +1,19 @@
+--
+--  Copyright (c) 2024 sovity GmbH
+--
+--  This program and the accompanying materials are made available under the
+--  terms of the Apache License, Version 2.0 which is available at
+--  https://www.apache.org/licenses/LICENSE-2.0
+--
+--  SPDX-License-Identifier: Apache-2.0
+--
+--  Contributors:
+--       sovity GmbH - initial API and implementation
+--
+
+update edc_policydefinitions
+set permissions = jsonb_set(
+    permissions::jsonb,
+    '{0,constraints}',
+    '[]'::jsonb)::json
+where policy_id = 'always-true';
diff --git a/tests/src/test/java/de/sovity/edc/e2e/AlwaysTrueMigrationNewConsumerTest.java b/tests/src/test/java/de/sovity/edc/e2e/AlwaysTrueMigrationNewConsumerTest.java
@@ -0,0 +1,57 @@
+package de.sovity.edc.e2e;
+
+import de.sovity.edc.client.EdcClient;
+import de.sovity.edc.client.gen.model.OperatorDto;
+import de.sovity.edc.client.gen.model.UiPolicyExpressionType;
+import de.sovity.edc.e2e.common.AlwaysTruePolicyMigrationCommonTest;
+import de.sovity.edc.extension.e2e.extension.Consumer;
+import de.sovity.edc.extension.e2e.extension.E2eScenario;
+import de.sovity.edc.extension.e2e.extension.E2eTestExtension;
+import de.sovity.edc.extension.e2e.extension.Provider;
+import de.sovity.edc.extension.policy.AlwaysTruePolicyConstants;
+import de.sovity.edc.extension.utils.junit.DisabledOnGithub;
+import lombok.val;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.mockserver.integration.ClientAndServer;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class AlwaysTrueMigrationNewConsumerTest {
+
+    @RegisterExtension
+    private static final E2eTestExtension E2E_TEST_EXTENSION = E2eTestExtension.builder()
+        .additionalConsumerMigrationLocation("classpath:db/additional-test-data/always-true-policy-migrated")
+        .additionalProviderMigrationLocation("classpath:db/additional-test-data/always-true-policy-legacy")
+        .build();
+
+    @Test
+    @DisabledOnGithub
+    void always_true_agreements_still_work_after_migration(
+        E2eScenario scenario,
+        ClientAndServer mockServer,
+        @Provider EdcClient providerClient,
+        @Consumer EdcClient consumerClient
+    ) {
+        // assert correct policies
+        val oldAlwaysTruePolicyCreatedAfterMigration = providerClient.uiApi().getPolicyDefinitionPage().getPolicies().stream().filter(
+            policy -> policy.getPolicyDefinitionId().equals(AlwaysTruePolicyConstants.POLICY_DEFINITION_ID)
+        ).findFirst().orElseThrow().getPolicy().getExpression();
+
+        val migratedAlwaysTruePolicy = consumerClient.uiApi().getPolicyDefinitionPage().getPolicies().stream().filter(
+            policy -> policy.getPolicyDefinitionId().equals(AlwaysTruePolicyConstants.POLICY_DEFINITION_ID)
+        ).findFirst().orElseThrow().getPolicy().getExpression();
+
+        assertThat(oldAlwaysTruePolicyCreatedAfterMigration.getType()).isEqualTo(UiPolicyExpressionType.CONSTRAINT);
+        assertThat(oldAlwaysTruePolicyCreatedAfterMigration.getConstraint().getLeft()).isEqualTo(
+            AlwaysTruePolicyConstants.EXPRESSION_LEFT_VALUE);
+        assertThat(oldAlwaysTruePolicyCreatedAfterMigration.getConstraint().getRight().getValue()).isEqualTo(
+            AlwaysTruePolicyConstants.EXPRESSION_RIGHT_VALUE);
+        assertThat(oldAlwaysTruePolicyCreatedAfterMigration.getConstraint().getOperator()).isEqualTo(OperatorDto.EQ);
+
+        assertThat(migratedAlwaysTruePolicy.getType()).isEqualTo(UiPolicyExpressionType.EMPTY);
+        assertThat(migratedAlwaysTruePolicy.getConstraint()).isNull();
+
+        AlwaysTruePolicyMigrationCommonTest.alwaysTruePolicyMigrationTest(scenario, mockServer, providerClient, consumerClient);
+    }
+}
diff --git a/tests/src/test/java/de/sovity/edc/e2e/AlwaysTrueMigrationNewProviderTest.java b/tests/src/test/java/de/sovity/edc/e2e/AlwaysTrueMigrationNewProviderTest.java
@@ -0,0 +1,58 @@
+package de.sovity.edc.e2e;
+
+import de.sovity.edc.client.EdcClient;
+import de.sovity.edc.client.gen.model.OperatorDto;
+import de.sovity.edc.client.gen.model.UiPolicyExpressionType;
+import de.sovity.edc.e2e.common.AlwaysTruePolicyMigrationCommonTest;
+import de.sovity.edc.extension.e2e.extension.Consumer;
+import de.sovity.edc.extension.e2e.extension.E2eScenario;
+import de.sovity.edc.extension.e2e.extension.E2eTestExtension;
+import de.sovity.edc.extension.e2e.extension.Provider;
+import de.sovity.edc.extension.policy.AlwaysTruePolicyConstants;
+import de.sovity.edc.extension.utils.junit.DisabledOnGithub;
+import lombok.val;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.mockserver.integration.ClientAndServer;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+
+class AlwaysTrueMigrationNewProviderTest {
+
+    @RegisterExtension
+    private static final E2eTestExtension E2E_TEST_EXTENSION = E2eTestExtension.builder()
+        .additionalConsumerMigrationLocation("classpath:db/additional-test-data/alwaysTruePolicyMigrationTest")
+        .additionalProviderMigrationLocation("")
+        .build();
+
+    @Test
+    @DisabledOnGithub
+    void always_true_agreements_still_work_after_migration(
+        E2eScenario scenario,
+        ClientAndServer mockServer,
+        @Provider EdcClient providerClient,
+        @Consumer EdcClient consumerClient
+    ) {
+        // assert correct policies
+        val newAlwaysTruePolicyOnProvider = providerClient.uiApi().getPolicyDefinitionPage().getPolicies().stream().filter(
+            policy -> policy.getPolicyDefinitionId().equals(AlwaysTruePolicyConstants.POLICY_DEFINITION_ID)
+        ).findFirst().orElseThrow().getPolicy().getExpression();
+
+        val oldAlwaysTruePolicyOnConsumer = consumerClient.uiApi().getPolicyDefinitionPage().getPolicies().stream().filter(
+            policy -> policy.getPolicyDefinitionId().equals(AlwaysTruePolicyConstants.POLICY_DEFINITION_ID)
+        ).findFirst().orElseThrow().getPolicy().getExpression();
+
+        assertThat(oldAlwaysTruePolicyOnConsumer.getType()).isEqualTo(UiPolicyExpressionType.CONSTRAINT);
+        assertThat(oldAlwaysTruePolicyOnConsumer.getConstraint().getLeft()).isEqualTo(AlwaysTruePolicyConstants.EXPRESSION_LEFT_VALUE);
+        assertThat(oldAlwaysTruePolicyOnConsumer.getConstraint().getRight().getValue()).isEqualTo(
+            AlwaysTruePolicyConstants.EXPRESSION_RIGHT_VALUE);
+        assertThat(oldAlwaysTruePolicyOnConsumer.getConstraint().getOperator()).isEqualTo(OperatorDto.EQ);
+
+        assertThat(newAlwaysTruePolicyOnProvider.getType()).isEqualTo(UiPolicyExpressionType.EMPTY);
+        assertThat(newAlwaysTruePolicyOnProvider.getConstraint()).isNull();
+
+        AlwaysTruePolicyMigrationCommonTest.alwaysTruePolicyMigrationTest(scenario, mockServer, providerClient, consumerClient);
+    }
+
+
+}
diff --git a/tests/src/test/java/de/sovity/edc/e2e/ApiWrapperDemoTest.java b/tests/src/test/java/de/sovity/edc/e2e/ApiWrapperDemoTest.java
@@ -41,6 +41,7 @@
 import de.sovity.edc.extension.e2e.connector.MockDataAddressRemote;
 import de.sovity.edc.extension.e2e.db.TestDatabase;
 import de.sovity.edc.extension.e2e.db.TestDatabaseViaTestcontainers;
+import de.sovity.edc.extension.utils.junit.DisabledOnGithub;
 import de.sovity.edc.utils.jsonld.vocab.Prop;
 import org.awaitility.Awaitility;
 import org.eclipse.edc.junit.extensions.EdcExtension;
@@ -108,6 +109,7 @@ void setup() {
     }
 
     @Test
+    @DisabledOnGithub
     void provide_and_consume() {
         // provider: create data offer
         createPolicy();

diff --git a/tests/src/test/java/de/sovity/edc/e2e/ContractTerminationTest.java b/tests/src/test/java/de/sovity/edc/e2e/ContractTerminationTest.java
@@ -27,6 +27,7 @@
 import de.sovity.edc.extension.e2e.extension.E2eTestExtension;
 import de.sovity.edc.extension.e2e.extension.Provider;
 import de.sovity.edc.extension.utils.junit.DisabledOnGithub;
+import de.sovity.edc.utils.jsonld.vocab.Prop;
 import jakarta.ws.rs.HttpMethod;
 import lombok.SneakyThrows;
 import lombok.val;
@@ -61,6 +62,7 @@
 public class ContractTerminationTest {
 
     @Test
+    @DisabledOnGithub
     void canGetAgreementPageForNonTerminatedContract(
         E2eScenario scenario,
         @Consumer EdcClient consumerClient,
@@ -364,9 +366,9 @@ void cantTransferDataAfterTerminated(
             .contractAgreementId(negotiation.getContractAgreementId())
             .dataSinkProperties(
                 Map.of(
-                    EDC_NAMESPACE + "baseUrl", destinationUrl,
-                    EDC_NAMESPACE + "method", HttpMethod.POST,
-                    EDC_NAMESPACE + "type", "HttpData"
+                    Prop.Edc.BASE_URL, destinationUrl,
+                    Prop.Edc.METHOD, HttpMethod.POST,
+                    Prop.Edc.TYPE, "HttpData"
                 )
             )
             .build();

diff --git a/tests/src/test/java/de/sovity/edc/e2e/common/AlwaysTruePolicyMigrationCommonTest.java b/tests/src/test/java/de/sovity/edc/e2e/common/AlwaysTruePolicyMigrationCommonTest.java
@@ -0,0 +1,51 @@
+package de.sovity.edc.e2e.common;
+
+import de.sovity.edc.client.EdcClient;
+import de.sovity.edc.client.gen.model.InitiateTransferRequest;
+import de.sovity.edc.client.gen.model.TransferProcessSimplifiedState;
+import de.sovity.edc.extension.e2e.extension.E2eScenario;
+import de.sovity.edc.utils.jsonld.vocab.Prop;
+import jakarta.ws.rs.HttpMethod;
+import lombok.val;
+import org.mockserver.integration.ClientAndServer;
+import org.mockserver.model.HttpRequest;
+import org.mockserver.model.HttpResponse;
+
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.eclipse.edc.spi.CoreConstants.EDC_NAMESPACE;
+
+public class AlwaysTruePolicyMigrationCommonTest {
+
+    public static void alwaysTruePolicyMigrationTest(E2eScenario scenario, ClientAndServer mockServer, EdcClient providerClient, EdcClient consumerClient) {
+        // arrange
+        val destinationPath = "/destination/some/path/";
+        val destinationUrl = "http://localhost:" + mockServer.getPort() + destinationPath;
+        mockServer.when(HttpRequest.request(destinationPath).withMethod("POST")).respond(it -> HttpResponse.response().withStatusCode(200));
+
+        val asset = scenario.createAsset();
+        scenario.createContractDefinition(asset); //this automatically uses the always-true policy
+        val negotiation = scenario.negotiateAssetAndAwait(asset);
+
+        // act
+        val transfer = scenario.transferAndAwait(
+            InitiateTransferRequest.builder()
+                .contractAgreementId(negotiation.getContractAgreementId())
+                .dataSinkProperties(
+                    Map.of(
+                        Prop.Edc.BASE_URL, destinationUrl,
+                        Prop.Edc.METHOD, HttpMethod.POST,
+                        Prop.Edc.TYPE, "HttpData"
+                    )
+                )
+                .build()
+        );
+        val transferProcess = consumerClient.uiApi().getTransferHistoryPage().getTransferEntries().stream().filter(
+            process -> process.getTransferProcessId().equals(transfer)
+        ).findFirst().orElseThrow();
+
+        // assert
+        assertThat(transferProcess.getState().getSimplifiedState()).isEqualTo(TransferProcessSimplifiedState.OK);
+    }
+}
diff --git a/...ditional-test-data/always-true-policy-legacy/V11_1__test_always_true_policy_migration.sql b/...ditional-test-data/always-true-policy-legacy/V11_1__test_always_true_policy_migration.sql
@@ -0,0 +1,35 @@
+insert into edc.public.edc_policydefinitions
+(policy_id, permissions, prohibitions, duties, extensible_properties, policy_type, created_at)
+values ('always-true', '[
+    {
+        "edctype": "dataspaceconnector:permission",
+        "target": null,
+        "action": {
+            "type": "USE",
+            "includedIn": null,
+            "constraint": null
+        },
+        "assignee": null,
+        "assigner": null,
+        "constraints": [
+            {
+                "edctype": "AtomicConstraint",
+                "leftExpression": {
+                    "edctype": "dataspaceconnector:literalexpression",
+                    "value": "ALWAYS_TRUE"
+                },
+                "rightExpression": {
+                    "edctype": "dataspaceconnector:literalexpression",
+                    "value": "true"
+                },
+                "operator": "EQ"
+            }
+        ],
+        "duties": []
+    }
+]',
+        '[]',
+        '[]',
+        '{}',
+        '{"@policytype":"set"}',
+        1721740661)
diff --git a/...tional-test-data/always-true-policy-migrated/V10_1__test_always_true_policy_migration.sql b/...tional-test-data/always-true-policy-migrated/V10_1__test_always_true_policy_migration.sql
@@ -0,0 +1,35 @@
+insert into edc.public.edc_policydefinitions
+(policy_id, permissions, prohibitions, duties, extensible_properties, policy_type, created_at)
+values ('always-true', '[
+    {
+        "edctype": "dataspaceconnector:permission",
+        "target": null,
+        "action": {
+            "type": "USE",
+            "includedIn": null,
+            "constraint": null
+        },
+        "assignee": null,
+        "assigner": null,
+        "constraints": [
+            {
+                "edctype": "AtomicConstraint",
+                "leftExpression": {
+                    "edctype": "dataspaceconnector:literalexpression",
+                    "value": "ALWAYS_TRUE"
+                },
+                "rightExpression": {
+                    "edctype": "dataspaceconnector:literalexpression",
+                    "value": "true"
+                },
+                "operator": "EQ"
+            }
+        ],
+        "duties": []
+    }
+]',
+        '[]',
+        '[]',
+        '{}',
+        '{"@policytype":"set"}',
+        1721740661)