Merge branch 'fix-cyclic' into 'master'

add validation for cyclic membership in AddMember API

See merge request !122

entitlements-v2-core/src/main/java/org/opengroup/osdu/entitlements/v2/service/AddMemberService.java

@@ -59,8 +59,9 @@ public class AddMemberService {
             log.error(String.format("Identity %s already belong to %d groups", addMemberDto.getEmail(), allExistingParents.size()));
             throw new AppException(HttpStatus.PRECONDITION_FAILED.value(), HttpStatus.PRECONDITION_FAILED.getReasonPhrase(), String.format("%s's group quota hit. Identity can't belong to more than %d groups", addMemberDto.getEmail(), EntityNode.MAX_PARENTS));
         }
-        if (existingGroupEntityNode.getNodeId().equalsIgnoreCase(addMemberDto.getEmail())) {
-            throw new AppException(HttpStatus.BAD_REQUEST.value(), HttpStatus.BAD_REQUEST.getReasonPhrase(), "Cyclic membership is not allowed");
+        Set<ParentReference> allParentsOfGroup = retrieveGroupRepo.loadAllParents(existingGroupEntityNode).getParentReferences();
+        Set<String> allParentsOfGroupEmails = allParentsOfGroup.stream().map(ref -> ref.getId()).collect(Collectors.toSet());
+        if (allParentsOfGroupEmails.contains(addMemberDto.getEmail()) || existingGroupEntityNode.getNodeId().equalsIgnoreCase(addMemberDto.getEmail())) {            throw new AppException(HttpStatus.BAD_REQUEST.value(), HttpStatus.BAD_REQUEST.getReasonPhrase(), "Cyclic membership is not allowed");
         }
         AddMemberRepoDto addMemberRepoDto = AddMemberRepoDto.builder().memberNode(memberNode).role(addMemberDto.getRole()).
                 partitionId(addMemberServiceDto.getPartitionId()).existingParents(allExistingParents).build();

entitlements-v2-core/src/test/java/org/opengroup/osdu/entitlements/v2/service/AddMemberServiceTests.java

@@ -84,6 +84,7 @@ public class AddMemberServiceTests {
                 .dataPartitionId("common").description("member@contoso.com").appIds(Collections.emptySet()).build();
         ParentTreeDto parentTreeDto = ParentTreeDto.builder().parentReferences(Collections.emptySet()).maxDepth(2).build();
         when(retrieveGroupRepo.loadAllParents(memberNode)).thenReturn(parentTreeDto);
+        when(retrieveGroupRepo.loadAllParents(groupNode)).thenReturn(parentTreeDto);
 
         AddMemberDto addMemberDto = new AddMemberDto("member@contoso.com", Role.MEMBER);
         AddMemberServiceDto addMemberServiceDto = AddMemberServiceDto.builder()
@@ -118,6 +119,7 @@ public class AddMemberServiceTests {
                 .dataPartitionId("common").description("member@xxx.com").appIds(Collections.emptySet()).build();
         ParentTreeDto parentTreeDto = ParentTreeDto.builder().parentReferences(Collections.emptySet()).maxDepth(2).build();
         when(retrieveGroupRepo.loadAllParents(memberNode)).thenReturn(parentTreeDto);
+        when(retrieveGroupRepo.loadAllParents(groupNode)).thenReturn(parentTreeDto);
 
         AddMemberDto addMemberDto = new AddMemberDto("member@xxx.com", Role.MEMBER);
         AddMemberServiceDto addMemberServiceDto = AddMemberServiceDto.builder()
@@ -181,6 +183,7 @@ public class AddMemberServiceTests {
         when(retrieveGroupRepo.hasDirectChild(rootDataGroupNode, ChildrenReference.createChildrenReference(requesterNode, Role.MEMBER))).thenReturn(Boolean.TRUE);
         ParentTreeDto parentTreeDto = ParentTreeDto.builder().parentReferences(Collections.emptySet()).maxDepth(2).build();
         when(retrieveGroupRepo.loadAllParents(memberNode)).thenReturn(parentTreeDto);
+        when(retrieveGroupRepo.loadAllParents(groupNode)).thenReturn(parentTreeDto);
 
         AddMemberDto addMemberDto = new AddMemberDto("member@xxx.com", Role.MEMBER);
         AddMemberServiceDto addMemberServiceDto = AddMemberServiceDto.builder()
@@ -399,4 +402,41 @@ public class AddMemberServiceTests {
             fail(String.format("should not throw exception: %s", ex.getMessage()));
         }
     }
+
+    @Test
+    public void should_throw400_ifCyclicMembershipExists() {
+        HashSet<ParentReference> parents = new HashSet<>();
+        parents.add(ParentReference.builder().id("users.x@common.contoso.com").dataPartitionId("common").build());
+
+        EntityNode memberNode = EntityNode.builder().nodeId("users.x@common.contoso.com").name("users.x")
+                .type(NodeType.GROUP).dataPartitionId("common").build();
+        EntityNode groupNode = EntityNode.builder().nodeId("data.x@common.contoso.com").name("data.x")
+                .type(NodeType.GROUP).dataPartitionId("common").build();
+        EntityNode requesterNode = EntityNode.builder().nodeId("requesterid").name("requesterid").type(NodeType.USER).dataPartitionId("common").build();
+        when(config.getDomain()).thenReturn("contoso.com");
+        when(retrieveGroupRepo.getEntityNode("users.x@common.contoso.com", "common")).thenReturn(Optional.of(memberNode));
+        when(retrieveGroupRepo.groupExistenceValidation("data.x@common.contoso.com", "common")).thenReturn(groupNode);
+        when(retrieveGroupRepo.getEntityNode("requesterid", "common")).thenReturn(Optional.of(requesterNode));
+        when(retrieveGroupRepo.groupExistenceValidation("users.x@common.contoso.com", "common")).thenReturn(groupNode);
+        when(retrieveGroupRepo.hasDirectChild(groupNode, ChildrenReference.createChildrenReference(requesterNode, Role.OWNER))).thenReturn(Boolean.TRUE);
+        when(retrieveGroupRepo.loadAllParents(memberNode)).thenReturn(ParentTreeDto.builder().parentReferences(Collections.emptySet()).maxDepth(2).build());
+        when(retrieveGroupRepo.loadAllParents(groupNode)).thenReturn(ParentTreeDto.builder().parentReferences(parents).maxDepth(2).build());
+
+        AddMemberDto addMemberDto = new AddMemberDto("users.x@common.contoso.com", Role.MEMBER);
+        AddMemberServiceDto addMemberServiceDto = AddMemberServiceDto.builder()
+                .groupEmail("data.x@common.contoso.com")
+                .requesterId("requesterid")
+                .partitionId("common")
+                .build();
+
+        try {
+            addMemberService.run(addMemberDto, addMemberServiceDto);
+            fail("should throw exception");
+        } catch (AppException ex) {
+            verify(addMemberRepo, never()).addMember(any(), any());
+            assertThat(ex.getError().getCode()).isEqualTo(400);
+        } catch (Exception ex) {
+            fail(String.format("should not throw exception: %s", ex.getMessage()));
+        }
+    }
 }