JAMES-2428 Reject DLP rules when duplicated ids

Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/f1ca57cf
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/f1ca57cf
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/f1ca57cf

Branch: refs/heads/master
Commit: f1ca57cffb10ca80a641e1bb784b5086fa670ae4
Parents: 2139e38
Author: Benoit Tellier <[email protected]>
Authored: Fri Aug 31 11:24:26 2018 +0700
Committer: Antoine Duprat <[email protected]>
Committed: Thu Sep 6 09:54:19 2018 +0200

----------------------------------------------------------------------
 .../dlp/api/DLPConfigurationStoreContract.java  |  9 +-----
 .../aggregates/DLPDomainConfiguration.java      | 12 +++++++
 .../webadmin/routes/DLPConfigurationRoutes.java | 26 ++++++++++++++-
 .../routes/DLPConfigurationRoutesTest.java      | 33 ++++++++++++++++++++
 4 files changed, 71 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/james-project/blob/f1ca57cf/server/data/data-api/src/test/java/org/apache/james/dlp/api/DLPConfigurationStoreContract.java
----------------------------------------------------------------------
diff --git 
a/server/data/data-api/src/test/java/org/apache/james/dlp/api/DLPConfigurationStoreContract.java
 
b/server/data/data-api/src/test/java/org/apache/james/dlp/api/DLPConfigurationStoreContract.java
index 990d724..e2e7251 100644
--- 
a/server/data/data-api/src/test/java/org/apache/james/dlp/api/DLPConfigurationStoreContract.java
+++ 
b/server/data/data-api/src/test/java/org/apache/james/dlp/api/DLPConfigurationStoreContract.java
@@ -109,14 +109,7 @@ public interface DLPConfigurationStoreContract {
     }
 
     @Test
-    default void storeShouldRemoveDuplicates(DLPConfigurationStore 
dlpConfigurationStore) {
-        dlpConfigurationStore.store(Domain.LOCALHOST, RULE, RULE);
-
-        
assertThat(dlpConfigurationStore.list(Domain.LOCALHOST)).containsOnly(RULE);
-    }
-
-    @Test
-    default void storeShouldRejectIdDuplicates(DLPConfigurationStore 
dlpConfigurationStore) {
+    default void storeShouldRejectDuplicateIds(DLPConfigurationStore 
dlpConfigurationStore) {
         assertThatThrownBy(() -> dlpConfigurationStore.store(Domain.LOCALHOST, 
RULE, RULE))
             .isInstanceOf(IllegalArgumentException.class);
     }

http://git-wip-us.apache.org/repos/asf/james-project/blob/f1ca57cf/server/data/data-library/src/main/java/org/apache/james/dlp/eventsourcing/aggregates/DLPDomainConfiguration.java
----------------------------------------------------------------------
diff --git 
a/server/data/data-library/src/main/java/org/apache/james/dlp/eventsourcing/aggregates/DLPDomainConfiguration.java
 
b/server/data/data-library/src/main/java/org/apache/james/dlp/eventsourcing/aggregates/DLPDomainConfiguration.java
index 003a5cc..0666fbd 100644
--- 
a/server/data/data-library/src/main/java/org/apache/james/dlp/eventsourcing/aggregates/DLPDomainConfiguration.java
+++ 
b/server/data/data-library/src/main/java/org/apache/james/dlp/eventsourcing/aggregates/DLPDomainConfiguration.java
@@ -19,6 +19,7 @@
 
 package org.apache.james.dlp.eventsourcing.aggregates;
 
+import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
@@ -33,6 +34,7 @@ import org.apache.james.eventsourcing.eventstore.History;
 import org.apache.james.util.OptionalUtils;
 
 import com.github.steveash.guavate.Guavate;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
@@ -93,6 +95,8 @@ public class DLPDomainConfiguration {
     }
 
     public List<Event> store(List<DLPConfigurationItem> updatedRules) {
+        Preconditions.checkArgument(shouldNotContainDuplicates(updatedRules));
+
         ImmutableSet<DLPConfigurationItem> existingRules = 
retrieveRules().collect(Guavate.toImmutableSet());
         ImmutableSet<DLPConfigurationItem> updatedRulesSet = 
ImmutableSet.copyOf(updatedRules);
 
@@ -108,6 +112,14 @@ public class DLPDomainConfiguration {
         return events;
     }
 
+    private boolean 
shouldNotContainDuplicates(Collection<DLPConfigurationItem> items) {
+        long uniqueIdCount = items.stream()
+            .map(DLPConfigurationItem::getId)
+            .distinct()
+            .count();
+        return uniqueIdCount == items.size();
+    }
+
     private EventId computeNextEventId(Optional<Event> removedRulesEvent) {
         return removedRulesEvent
             .map(Event::eventId)

http://git-wip-us.apache.org/repos/asf/james-project/blob/f1ca57cf/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/DLPConfigurationRoutes.java
----------------------------------------------------------------------
diff --git 
a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/DLPConfigurationRoutes.java
 
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/DLPConfigurationRoutes.java
index cd78646..bb257db 100644
--- 
a/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/DLPConfigurationRoutes.java
+++ 
b/server/protocols/webadmin/webadmin-data/src/main/java/org/apache/james/webadmin/routes/DLPConfigurationRoutes.java
@@ -24,6 +24,8 @@ import static org.apache.james.webadmin.Constants.EMPTY_BODY;
 import static org.apache.james.webadmin.Constants.JSON_CONTENT_TYPE;
 import static org.apache.james.webadmin.Constants.SEPARATOR;
 
+import java.util.Collection;
+import java.util.List;
 import java.util.stream.Stream;
 
 import javax.inject.Inject;
@@ -127,13 +129,35 @@ public class DLPConfigurationRoutes implements Routes {
             Domain senderDomain = parseDomain(request);
             DLPConfigurationDTO dto = jsonExtractor.parse(request.body());
 
-            dlpConfigurationStore.store(senderDomain, 
dto.toDLPConfigurations());
+            List<DLPConfigurationItem> rules = dto.toDLPConfigurations();
+            shouldNotContainDuplicates(rules);
+
+            dlpConfigurationStore.store(senderDomain, rules);
 
             response.status(HttpStatus.NO_CONTENT_204);
             return EMPTY_BODY;
         });
     }
 
+    private void shouldNotContainDuplicates(Collection<DLPConfigurationItem> 
items) {
+        if (containsDuplicate(items)) {
+            ErrorResponder.builder()
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .type(ErrorType.INVALID_ARGUMENT)
+                .message("'id' duplicates are not allowed in DLP rules")
+                .haltError();
+        }
+    }
+
+    private boolean containsDuplicate(Collection<DLPConfigurationItem> items) {
+        long uniqueIdCount = items.stream()
+            .map(DLPConfigurationItem::getId)
+            .distinct()
+            .count();
+
+        return uniqueIdCount != items.size();
+    }
+
     @GET
     @Path("/{senderDomain}")
     @ApiOperation(value = "Return a DLP configuration for a given 
senderDomain")

http://git-wip-us.apache.org/repos/asf/james-project/blob/f1ca57cf/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DLPConfigurationRoutesTest.java
----------------------------------------------------------------------
diff --git 
a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DLPConfigurationRoutesTest.java
 
b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DLPConfigurationRoutesTest.java
index 6887cc9..2abefc8 100644
--- 
a/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DLPConfigurationRoutesTest.java
+++ 
b/server/protocols/webadmin/webadmin-data/src/test/java/org/apache/james/webadmin/routes/DLPConfigurationRoutesTest.java
@@ -411,6 +411,39 @@ class DLPConfigurationRoutesTest {
 
             assertThatJson(retrievedBody).isEqualTo(updatedBody);
         }
+        
+        @Test
+        void putShouldRejectDuplicatedIds() {
+            String storeBody =
+                "{\"rules\": [" +
+                    "  {" +
+                    "    \"id\": \"1\"," +
+                    "    \"expression\": \"expression 1\"," +
+                    "    \"explanation\": \"explanation 1\"," +
+                    "    \"targetsSender\": true," +
+                    "    \"targetsRecipients\": true," +
+                    "    \"targetsContent\": true" +
+                    "  }," +
+                    "  {" +
+                    "    \"id\": \"1\"," +
+                    "    \"expression\": \"expression 3\"," +
+                    "    \"explanation\": \"explanation 3\"," +
+                    "    \"targetsSender\": false," +
+                    "    \"targetsRecipients\": false," +
+                    "    \"targetsContent\": true" +
+                    "  }]}";
+
+            given()
+                .body(storeBody).log().ifValidationFails()
+            .when()
+                .put(DEFAULT_DOMAIN)
+            .then()
+                .statusCode(HttpStatus.BAD_REQUEST_400)
+                .contentType(JSON_CONTENT_TYPE)
+                .body("statusCode", is(HttpStatus.BAD_REQUEST_400))
+                .body("type", is("InvalidArgument"))
+                .body("message", is("'id' duplicates are not allowed in DLP 
rules"));
+        }
 
         @Test
         void putShouldClearTheConfigurationsWhenNoRule() {


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to