Merge pull request #597 from catenax-ng/feat/528-Policy-Store-API-Inp…

…ut-Validation-Improve

feat(impl): [#528] Improvements Policy Store Input Validation

CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 _**For better traceability add the corresponding GitHub issue number in each changelog entry, please.**_
 
 ## [Unreleased]
+
+### Changed
+
+- Improved policy store API input validation. #528
+
 
 
 ## Added
@@ -31,7 +36,7 @@ _**For better traceability add the corresponding GitHub issue number in each cha
 
 ### Fixed
 
-- Fixed issue in EDR Token renewal #358
+- Fixed issue in EDR token renewal. #358
 
 ### Added
 

docs/src/api/irs-api.yaml

@@ -815,9 +815,9 @@ paths:
                   payload:
                     '@context':
                       odrl: http://www.w3.org/ns/odrl/2/
-                    '@id': policy-id3
+                    '@id': e917f5f-8dac-49ac-8d10-5b4d254d2b48
                     policy:
-                      policyId: p3
+                      policyId: e917f5f-8dac-49ac-8d10-5b4d254d2b48
                       createdOn: 2024-03-28T03:34:42.9454448Z
                       validUntil: 2025-12-12T23:59:59.999Z
                       permissions:
@@ -1846,7 +1846,7 @@ components:
             This parameter is optional.
             If not set the policy is registered for each existing BPN.
           example: BPNL1234567890AB
-          pattern: "(BPN)[LSA][\\w\\d]{10}[\\w\\d]{2}"
+          pattern: "(BPNL)[\\w\\d]{10}[\\w\\d]{2}"
         payload:
           type: object
           additionalProperties:
@@ -1855,7 +1855,7 @@ components:
           example:
             '@context':
               odrl: http://www.w3.org/ns/odrl/2/
-            '@id': policy-id
+            '@id': e917f5f-8dac-49ac-8d10-5b4d254d2b48
             '@type': PolicyDefinitionRequestDto
             policy:
               '@type': Policy
@@ -2248,7 +2248,7 @@ components:
       example:
         '@context':
           odrl: http://www.w3.org/ns/odrl/2/
-        '@id': policy-id
+        '@id': e917f5f-8dac-49ac-8d10-5b4d254d2b48
         '@type': PolicyDefinitionRequestDto
         policy:
           '@type': Policy

irs-api/src/main/java/org/eclipse/tractusx/irs/controllers/IrsExceptionHandler.java

@@ -25,6 +25,7 @@
 
 import java.util.List;
 import java.util.NoSuchElementException;
+import java.util.UUID;
 
 import jakarta.validation.ConstraintViolationException;
 import lombok.extern.slf4j.Slf4j;
@@ -55,15 +56,18 @@ public class IrsExceptionHandler {
      */
     @ExceptionHandler(ResponseStatusException.class)
     public ResponseEntity<ErrorResponse> handleResponseStatusException(final ResponseStatusException exception) {
-        log.info(exception.getClass().getName(), exception);
+        final UUID errorRef = UUID.randomUUID();
+        final String messageTemplate = "%s (errorRef: %s)";
+        log.info(messageTemplate.formatted(exception.getClass().getName(), errorRef), exception);
 
         final HttpStatus httpStatus = HttpStatus.valueOf(exception.getStatusCode().value());
 
         return ResponseEntity.status(httpStatus)
                              .body(ErrorResponse.builder()
                                                 .withStatusCode(httpStatus)
                                                 .withError(httpStatus.getReasonPhrase())
-                                                .withMessages(List.of(exception.getReason()))
+                                                .withMessages(List.of(messageTemplate.formatted(exception.getReason(),
+                                                        errorRef)))
                                                 .build());
     }
 

irs-edc-client/src/main/java/org/eclipse/tractusx/irs/edc/client/policy/ConstraintConstants.java

@@ -0,0 +1,40 @@
+/********************************************************************************
+ * Copyright (c) 2022,2024 Bayerische Motoren Werke Aktiengesellschaft (BMW AG)
+ * Copyright (c) 2021,2024 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 Apache License, Version 2.0 which is available at
+ * https://www.apache.org/licenses/LICENSE-2.0.
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ ********************************************************************************/
+package org.eclipse.tractusx.irs.edc.client.policy;
+
+/**
+ * Constants for {@link Constraint}s
+ */
+public final class ConstraintConstants {
+
+    public static final Constraint ACTIVE_MEMBERSHIP = new Constraint("Membership", new Operator(OperatorType.EQ),
+            "active");
+
+    public static final Constraint FRAMEWORK_AGREEMENT_TRACEABILITY_ACTIVE = new Constraint(
+            "FrameworkAgreement.traceability", new Operator(OperatorType.EQ), "active");
+
+    public static final Constraint PURPOSE_ID_3_1_TRACE = new Constraint("PURPOSE", new Operator(OperatorType.EQ),
+            "ID 3.1 Trace");
+
+    private ConstraintConstants() {
+        // helper class
+    }
+
+}

irs-edc-client/src/main/java/org/eclipse/tractusx/irs/edc/client/transformer/JsonObjectToIrsPolicyTransformer.java

@@ -56,8 +56,8 @@ protected JsonObjectToIrsPolicyTransformer(final ObjectMapper objectMapper) {
             try {
                 final Object result = objectMapper.readerFor(Policy.class).readValue(v.asJsonObject().toString());
                 builder.permissions(((Policy) result).getPermissions());
-            } catch (JsonProcessingException e) {
-                throw new JsonParseException(e);
+            } catch (ClassCastException | JsonProcessingException e) {
+                throw new JsonParseException("Invalid policy", e);
             }
         });
 

irs-edc-client/src/test/java/org/eclipse/tractusx/irs/edc/client/policy/AcceptedPoliciesProviderTest.java

@@ -49,6 +49,7 @@ void shouldReturnStoredPolicies() {
 
         assertThat(acceptedPolicies).hasSize(1);
     }
+
     @Test
     void shouldRemoveStoredPolicies() {
         testee.addAcceptedPolicies(List.of(policy()));
@@ -60,8 +61,9 @@ void shouldRemoveStoredPolicies() {
 
         assertThat(testee.getAcceptedPolicies("testBpn")).isEmpty();
     }
+
     @NotNull
     private static AcceptedPolicy policy() {
-        return new AcceptedPolicy(new Policy(), OffsetDateTime.now());
+        return new AcceptedPolicy(Policy.builder().build(), OffsetDateTime.now());
     }
 }

irs-edc-client/src/test/java/org/eclipse/tractusx/irs/edc/client/transformer/JsonObjectToIrsPolicyTransformerTest.java

@@ -55,7 +55,7 @@ class JsonObjectToIrsPolicyTransformerTest {
                     "cx-policy": "https://w3id.org/catenax/policy/",
                     "odrl": "http://www.w3.org/ns/odrl/2/"
                 },
-               "@id": "policy-id",
+               "@id": "e917f5f-8dac-49ac-8d10-5b4d254d2b48",
                 "policy": {
                     "odrl:permission": [
                         {
@@ -104,7 +104,7 @@ void shouldTransformJsonObjectToPolicyCorrectly() {
         final Policy transformed = jsonObjectToIrsPolicyTransformer.transform(jsonObject, transformerContext);
 
         // then
-        assertThat(transformed.getPolicyId()).isEqualTo("policy-id");
+        assertThat(transformed.getPolicyId()).isEqualTo("e917f5f-8dac-49ac-8d10-5b4d254d2b48");
         final Permission permission = transformed.getPermissions().get(0);
         assertThat(permission.getAction()).isEqualTo(PolicyType.USE);
         final List<Constraint> and = permission.getConstraint().getAnd();

irs-models/src/main/java/org/eclipse/tractusx/irs/data/JsonParseException.java

@@ -27,6 +27,11 @@
  * Exception when parsing JSON
  */
 public class JsonParseException extends RuntimeException {
+
+    public JsonParseException(final String message, final Throwable throwable) {
+        super(message, throwable);
+    }
+
     public JsonParseException(final Throwable cause) {
         super(cause);
     }

irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/controllers/PolicyStoreController.java

@@ -47,6 +47,7 @@
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.collections.CollectionUtils;
 import org.eclipse.tractusx.irs.common.auth.IrsRoles;
+import org.eclipse.tractusx.irs.data.JsonParseException;
 import org.eclipse.tractusx.irs.dtos.ErrorResponse;
 import org.eclipse.tractusx.irs.edc.client.policy.Policy;
 import org.eclipse.tractusx.irs.policystore.models.CreatePoliciesResponse;
@@ -56,6 +57,7 @@
 import org.eclipse.tractusx.irs.policystore.services.PolicyStoreService;
 import org.eclipse.tractusx.irs.policystore.validators.BusinessPartnerNumberListValidator;
 import org.eclipse.tractusx.irs.policystore.validators.ValidListOfBusinessPartnerNumbers;
+import org.eclipse.tractusx.irs.policystore.validators.ValidPolicyId;
 import org.springframework.http.HttpStatus;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.validation.annotation.Validated;
@@ -128,7 +130,13 @@ public class PolicyStoreController {
     @PreAuthorize("hasAuthority('" + IrsRoles.ADMIN_IRS + "')")
     public CreatePoliciesResponse registerAllowedPolicy(@Valid @RequestBody final CreatePolicyRequest request) {
 
-        final Policy registeredPolicy = service.registerPolicy(request);
+        final Policy registeredPolicy;
+
+        try {
+            registeredPolicy = service.registerPolicy(request);
+        } catch (JsonParseException e) {
+            throw new ResponseStatusException(HttpStatus.BAD_REQUEST, e.getMessage(), e);
+        }
 
         if (registeredPolicy == null) {
             throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Policy was not registered");
@@ -214,7 +222,7 @@ public Map<String, List<PolicyResponse>> getPolicies(//
     @DeleteMapping("/policies/{policyId}")
     @ResponseStatus(HttpStatus.OK)
     @PreAuthorize("hasAuthority('" + IrsRoles.ADMIN_IRS + "')")
-    public void deleteAllowedPolicy(@PathVariable("policyId") final String policyId) {
+    public void deleteAllowedPolicy(@ValidPolicyId @PathVariable("policyId") final String policyId) {
         service.deletePolicy(policyId);
     }
 

irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/models/CreatePolicyRequest.java

@@ -29,19 +29,23 @@
 
 import io.swagger.v3.oas.annotations.media.Schema;
 import jakarta.json.JsonObject;
+import jakarta.validation.constraints.Future;
 import jakarta.validation.constraints.NotNull;
 import jakarta.validation.constraints.Pattern;
+import lombok.Builder;
 
 /**
  * Object for API to create policy
  */
 @SuppressWarnings("FileTabCharacter")
 @Schema(description = "Request to add a policy")
+@Builder
 public record CreatePolicyRequest(
 
         @Schema(description = "Timestamp after which the policy will no longer be accepted in negotiations.",
                 example = "2025-12-12T23:59:59.999Z") //
         @NotNull //
+        @Future(message = "must be in future") //
         OffsetDateTime validUntil, //
 
         @Schema(description = """
@@ -66,7 +70,7 @@ The business partner number (BPN) for which the policy should be registered.
                 "@context": {
                     "odrl": "http://www.w3.org/ns/odrl/2/"
                 },
-                "@id": "policy-id",
+                "@id": "e917f5f-8dac-49ac-8d10-5b4d254d2b48",
                 "@type": "PolicyDefinitionRequestDto",
                 "policy": {
                     "@type": "Policy",

irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/models/PolicyResponse.java

@@ -41,9 +41,9 @@ public record PolicyResponse(OffsetDateTime validUntil, Payload payload) {
                               "@context": {
                                   "odrl": "http://www.w3.org/ns/odrl/2/"
                               },
-                              "@id": "policy-id3",
+                              "@id": "e917f5f-8dac-49ac-8d10-5b4d254d2b48",
                               "policy": {
-                                  "policyId": "p3",
+                                  "policyId": "e917f5f-8dac-49ac-8d10-5b4d254d2b48",
                                   "createdOn": "2024-03-28T03:34:42.9454448Z",
                                   "validUntil": "2025-12-12T23:59:59.999Z",
                                   "permissions": [

irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/models/UpdatePolicyRequest.java

@@ -27,18 +27,23 @@
 import java.util.List;
 
 import io.swagger.v3.oas.annotations.media.Schema;
+import jakarta.validation.constraints.Future;
 import jakarta.validation.constraints.NotEmpty;
 import jakarta.validation.constraints.NotNull;
+import lombok.Builder;
+import org.eclipse.tractusx.irs.policystore.validators.ListOfPolicyIds;
 import org.eclipse.tractusx.irs.policystore.validators.ValidListOfBusinessPartnerNumbers;
 
 /**
  * Request object for policy update
  */
 @Schema(description = "Request to update a policy")
+@Builder
 public record UpdatePolicyRequest(
 
         @Schema(description = "Timestamp after which the policy will no longer be accepted in negotiations.") //
         @NotNull //
+        @Future(message = "must be in future") //
         OffsetDateTime validUntil, //
 
         @Schema(description = "Business Partner Number (BPN).") //
@@ -48,6 +53,7 @@ public record UpdatePolicyRequest(
         @Schema(description = "The IDs of the policies to be updated.") //
         @NotNull //
         @NotEmpty //
+        @ListOfPolicyIds //
         List<String> policyIds //
 ) {
 }

irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/services/PolicyStoreService.java

@@ -56,6 +56,7 @@
 import org.eclipse.tractusx.irs.policystore.models.CreatePolicyRequest;
 import org.eclipse.tractusx.irs.policystore.models.UpdatePolicyRequest;
 import org.eclipse.tractusx.irs.policystore.persistence.PolicyPersistence;
+import org.eclipse.tractusx.irs.policystore.validators.PolicyValidator;
 import org.springframework.http.HttpStatus;
 import org.springframework.stereotype.Service;
 import org.springframework.web.server.ResponseStatusException;
@@ -78,9 +79,6 @@ public class PolicyStoreService implements AcceptedPoliciesProvider {
 
     private final Clock clock;
 
-    private static final String MISSING_REQUEST_FIELD_MESSAGE =
-            "Request does not contain all required fields. " + "Missing: %s";
-
     private static final String DEFAULT = "default";
 
     /**
@@ -100,10 +98,12 @@ private static final class ConfiguredDefaultPolicy {
 
     public PolicyStoreService(final DefaultAcceptedPoliciesConfig defaultAcceptedPoliciesConfig,
             final PolicyPersistence persistence, final EdcTransformer edcTransformer, final Clock clock) {
+
+        this.clock = clock;
+
         this.allowedPoliciesFromConfig = createDefaultPolicyFromConfig(defaultAcceptedPoliciesConfig);
         this.persistence = persistence;
         this.edcTransformer = edcTransformer;
-        this.clock = clock;
     }
 
     /**
@@ -126,8 +126,8 @@ public Policy registerPolicy(final CreatePolicyRequest request) {
         return registeredPolicy;
     }
 
-    public Policy doRegisterPolicy(final Policy policy, final String businessPartnersNumber) {
-        validatePolicy(policy);
+    /* package */ Policy doRegisterPolicy(final Policy policy, final String businessPartnersNumber) {
+        PolicyValidator.validate(policy);
         policy.setCreatedOn(OffsetDateTime.now(clock));
         log.info("Registering new policy with id {}, valid until {}", policy.getPolicyId(), policy.getValidUntil());
         try {
@@ -137,24 +137,6 @@ public Policy doRegisterPolicy(final Policy policy, final String businessPartner
         }
     }
 
-    /**
-     * Checks whether policy from register policy request has all required fields
-     *
-     * @param policy policy to register
-     */
-    private void validatePolicy(final Policy policy) {
-
-        if (policy.getPermissions() == null) {
-            throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
-                    String.format(MISSING_REQUEST_FIELD_MESSAGE, "odrl:permission"));
-        }
-
-        if (policy.getPermissions().stream().anyMatch(p -> p.getConstraint() == null)) {
-            throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
-                    String.format(MISSING_REQUEST_FIELD_MESSAGE, "odrl:constraint"));
-        }
-    }
-
     /**
      * Finds policies by list of BPN.
      *
@@ -321,11 +303,17 @@ private List<Policy> createDefaultPolicyFromConfig(
                                              new Constraint(acceptedPolicy.getLeftOperand(),
                                                      new Operator(OperatorType.fromValue(acceptedPolicy.getOperator())),
                                                      acceptedPolicy.getRightOperand())));
-        final Policy policy = new Policy(ConfiguredDefaultPolicy.DEFAULT_POLICY_ID, OffsetDateTime.now(),
-                OffsetDateTime.now().plusYears(ConfiguredDefaultPolicy.DEFAULT_POLICY_LIFETIME_YEARS),
-                List.of(new Permission(PolicyType.USE, new Constraints(constraints, constraints))));
 
-        return List.of(policy);
+        final OffsetDateTime now = OffsetDateTime.now(clock);
+        return List.of(Policy.builder()
+                             .policyId(ConfiguredDefaultPolicy.DEFAULT_POLICY_ID)
+                             .createdOn(now)
+                             .validUntil(now.plusYears(ConfiguredDefaultPolicy.DEFAULT_POLICY_LIFETIME_YEARS))
+                             .permissions(List.of(Permission.builder()
+                                                            .action(PolicyType.USE)
+                                                            .constraint(new Constraints(constraints, constraints))
+                                                            .build()))
+                             .build());
     }
 
 }