Merge pull request #5385 from enonic/issue-5283

Add selection of Role in the User Group wizard #5285

modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/security/SecurityResource.java

@@ -290,7 +290,16 @@ public PrincipalJson getPrincipalByKey( @PathParam("key") final String keyParam,
 
             case GROUP:
                 final PrincipalKeys groupMembers = getMembers( principalKey );
-                return new GroupJson( (Group) principal, groupMembers );
+                if ( resolveMemberships )
+                {
+                    final PrincipalKeys membershipKeys = securityService.getMemberships( principalKey );
+                    final Principals memberships = securityService.getPrincipals( membershipKeys );
+                    return new GroupJson( (Group) principal, groupMembers, memberships );
+                }
+                else
+                {
+                    return new GroupJson( (Group) principal, groupMembers );
+                }
 
             case ROLE:
                 final PrincipalKeys roleMembers = getMembers( principalKey );
@@ -353,7 +362,15 @@ public GroupJson createGroup( final CreateGroupJson params )
             securityService.addRelationship( rel );
         }
 
-        return new GroupJson( group, members );
+        for ( final PrincipalKey membershipToAdd : params.toMembershipKeys() )
+        {
+            final PrincipalRelationship rel = PrincipalRelationship.from( membershipToAdd ).to( groupKey );
+            this.securityService.addRelationship( rel );
+        }
+
+        final Principals memberships = this.securityService.getPrincipals( this.securityService.getMemberships( groupKey ) );
+
+        return new GroupJson( group, members, memberships );
     }
 
     @POST
@@ -380,16 +397,8 @@ public UserJson updateUser( final UpdateUserJson params )
         final User user = securityService.updateUser( params.getUpdateUserParams() );
 
         final PrincipalKey userKey = user.getKey();
-        for ( PrincipalKey membershipToAdd : params.getAddMemberships() )
-        {
-            final PrincipalRelationship rel = PrincipalRelationship.from( membershipToAdd ).to( userKey );
-            securityService.addRelationship( rel );
-        }
-        for ( PrincipalKey membershipToRemove : params.getRemoveMemberships() )
-        {
-            final PrincipalRelationship rel = PrincipalRelationship.from( membershipToRemove ).to( userKey );
-            securityService.removeRelationship( rel );
-        }
+
+        updateMemberships( userKey, params.getAddMemberships(), params.getRemoveMemberships() );
 
         final Principals memberships = securityService.getPrincipals( securityService.getMemberships( userKey ) );
         return new UserJson( user, memberships );
@@ -417,10 +426,14 @@ public GroupJson updateGroup( final UpdateGroupJson params )
         final Group group = securityService.updateGroup( params.getUpdateGroupParams() );
         final PrincipalKey groupKey = group.getKey();
 
-        updateMemberships( groupKey, params.getRemoveMembers(), params.getAddMembers() );
+        updateMembers( groupKey, params.getAddMembers(), params.getRemoveMembers() );
+
+        updateMemberships( groupKey, params.getAddMemberships(), params.getRemoveMemberships() );
+
+        final Principals memberships = securityService.getPrincipals( securityService.getMemberships( groupKey ) );
 
         final PrincipalKeys groupMembers = getMembers( groupKey );
-        return new GroupJson( group, groupMembers );
+        return new GroupJson( group, groupMembers, memberships );
     }
 
     @POST
@@ -430,7 +443,7 @@ public RoleJson updateRole( final UpdateRoleJson params )
         final Role role = securityService.updateRole( params.getUpdateRoleParams() );
         final PrincipalKey roleKey = role.getKey();
 
-        updateMemberships( roleKey, params.getRemoveMembers(), params.getAddMembers() );
+        updateMembers( roleKey, params.getAddMembers(), params.getRemoveMembers() );
 
         final PrincipalKeys roleMembers = getMembers( roleKey );
         return new RoleJson( role, roleMembers );
@@ -455,7 +468,7 @@ public DeletePrincipalsResultJson deletePrincipals( final DeletePrincipalJson pr
         return resultsJson;
     }
 
-    private void updateMemberships( final PrincipalKey target, PrincipalKeys membersToRemove, PrincipalKeys membersToAdd )
+    private void updateMembers( final PrincipalKey target, PrincipalKeys membersToAdd, PrincipalKeys membersToRemove )
     {
         for ( PrincipalKey memberToAdd : membersToAdd )
         {
@@ -470,6 +483,21 @@ private void updateMemberships( final PrincipalKey target, PrincipalKeys members
         }
     }
 
+    private void updateMemberships( final PrincipalKey source, PrincipalKeys membershipsToAdd, PrincipalKeys membershipsToRemove )
+    {
+        for ( PrincipalKey membershipToAdd : membershipsToAdd )
+        {
+            final PrincipalRelationship rel = PrincipalRelationship.from( membershipToAdd ).to( source );
+            securityService.addRelationship( rel );
+        }
+
+        for ( PrincipalKey membershipToRemove : membershipsToRemove )
+        {
+            final PrincipalRelationship rel = PrincipalRelationship.from( membershipToRemove ).to( source );
+            securityService.removeRelationship( rel );
+        }
+    }
+
     private PrincipalKeys getMembers( final PrincipalKey principal )
     {
         final PrincipalRelationships relationships = this.securityService.getRelationships( principal );

modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/security/json/CreateGroupJson.java

@@ -1,7 +1,9 @@
 package com.enonic.xp.admin.impl.rest.resource.security.json;
 
+import java.util.Collections;
 import java.util.List;
 
+import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonProperty;
 
 import com.enonic.xp.security.CreateGroupParams;
@@ -21,6 +23,9 @@ public final class CreateGroupJson
     @JsonProperty("members")
     public List<String> members;
 
+    @JsonProperty("memberships")
+    public List<String> memberships = Collections.emptyList();
+
     @JsonProperty("description")
     public String description;
 
@@ -34,8 +39,15 @@ public CreateGroupParams toCreateGroupParams()
             build();
     }
 
+    @JsonIgnore
     public PrincipalKeys toMemberKeys()
     {
         return PrincipalKeys.from( this.members.stream().map( PrincipalKey::from ).collect( toList() ) );
     }
+
+    @JsonIgnore
+    public PrincipalKeys toMembershipKeys()
+    {
+        return PrincipalKeys.from( this.memberships.stream().map( PrincipalKey::from ).collect( toList() ) );
+    }
 }

modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/security/json/GroupJson.java

@@ -6,6 +6,7 @@
 import com.enonic.xp.security.Group;
 import com.enonic.xp.security.PrincipalKey;
 import com.enonic.xp.security.PrincipalKeys;
+import com.enonic.xp.security.Principals;
 
 import static java.util.stream.Collectors.toList;
 
@@ -16,15 +17,28 @@ public final class GroupJson
 
     private final List<String> members;
 
+    private final List<PrincipalJson> memberships;
+
     public GroupJson( final Group group, final PrincipalKeys members )
+    {
+        this( group, members, Principals.empty() );
+    }
+
+    public GroupJson( final Group group, final PrincipalKeys members, final Principals memberships )
     {
         super( group );
         this.group = group;
         this.members = members.stream().map( PrincipalKey::toString ).collect( toList() );
+        this.memberships = new PrincipalsJson( memberships ).getPrincipals();
     }
 
     public List<String> getMembers()
     {
         return members;
     }
+
+    public List<PrincipalJson> getMemberships()
+    {
+        return memberships;
+    }
 }

modules/admin/admin-impl/src/main/java/com/enonic/xp/admin/impl/rest/resource/security/json/UpdateGroupJson.java

@@ -21,12 +21,18 @@ public final class UpdateGroupJson
 
     private final PrincipalKeys removeMembers;
 
+    private final PrincipalKeys addMemberships;
+
+    private final PrincipalKeys removeMemberships;
+
     private final String description;
 
     @JsonCreator
     public UpdateGroupJson( @JsonProperty("key") final String userKey, @JsonProperty("displayName") final String displayName,
                             @JsonProperty("addMembers") final List<String> addMembers,
                             @JsonProperty("removeMembers") final List<String> removeMembers,
+                            @JsonProperty("addMemberships") final List<String> addMemberships,
+                            @JsonProperty("removeMemberships") final List<String> removeMemberships,
                             @JsonProperty("description") final String description )
     {
         final PrincipalKey principalKey = PrincipalKey.from( userKey );
@@ -37,6 +43,8 @@ public UpdateGroupJson( @JsonProperty("key") final String userKey, @JsonProperty
             build();
         this.addMembers = PrincipalKeys.from( addMembers.stream().map( PrincipalKey::from ).collect( toList() ) );
         this.removeMembers = PrincipalKeys.from( removeMembers.stream().map( PrincipalKey::from ).collect( toList() ) );
+        this.addMemberships = PrincipalKeys.from( addMemberships.stream().map( PrincipalKey::from ).collect( toList() ) );
+        this.removeMemberships = PrincipalKeys.from( removeMemberships.stream().map( PrincipalKey::from ).collect( toList() ) );
         this.description = description;
     }
 
@@ -57,4 +65,16 @@ public PrincipalKeys getRemoveMembers()
     {
         return removeMembers;
     }
+
+    @JsonIgnore
+    public PrincipalKeys getAddMemberships()
+    {
+        return addMemberships;
+    }
+
+    @JsonIgnore
+    public PrincipalKeys getRemoveMemberships()
+    {
+        return removeMemberships;
+    }
 }

modules/admin/admin-impl/src/test/java/com/enonic/xp/admin/impl/rest/resource/security/SecurityResourceTest.java

@@ -315,10 +315,10 @@ public void getPrincipalGroupById()
         Mockito.<Optional<? extends Principal>>when(
             securityService.getPrincipal( PrincipalKey.from( "group:system:group-a" ) ) ).thenReturn( userRes );
 
-        PrincipalRelationship membership1 = from( group.getKey() ).to( PrincipalKey.from( "user:system:user1" ) );
-        PrincipalRelationship membership2 = from( group.getKey() ).to( PrincipalKey.from( "user:system:user2" ) );
-        PrincipalRelationships memberships = PrincipalRelationships.from( membership1, membership2 );
-        Mockito.when( securityService.getRelationships( PrincipalKey.from( "group:system:group-a" ) ) ).thenReturn( memberships );
+        PrincipalRelationship member1 = from( group.getKey() ).to( PrincipalKey.from( "user:system:user1" ) );
+        PrincipalRelationship member2 = from( group.getKey() ).to( PrincipalKey.from( "user:system:user2" ) );
+        PrincipalRelationships members = PrincipalRelationships.from( member1, member2 );
+        Mockito.when( securityService.getRelationships( PrincipalKey.from( "group:system:group-a" ) ) ).thenReturn( members );
 
         String jsonString = request().
             path( "security/principals/" + URLEncoder.encode( "group:system:group-a", "UTF-8" ) ).
@@ -327,6 +327,46 @@ public void getPrincipalGroupById()
         assertJson( "getPrincipalGroupById.json", jsonString );
     }
 
+    @Test
+    public void getPrincipalGroupByIdWithMemberships()
+        throws Exception
+    {
+        final Group group = Group.create().
+            key( PrincipalKey.ofGroup( UserStoreKey.system(), "group-a" ) ).
+            displayName( "Group A" ).
+            description( "group a" ).
+            modifiedTime( Instant.now( clock ) ).
+            build();
+
+        final Role role = Role.create().
+            key( PrincipalKey.ofRole( "superuser" ) ).
+            displayName( "Super user role" ).
+            modifiedTime( Instant.now( clock ) ).
+            description( "super u" ).
+            build();
+
+        final Optional<? extends Principal> userRes = Optional.of( group );
+        Mockito.<Optional<? extends Principal>>when(
+            securityService.getPrincipal( PrincipalKey.from( "group:system:group-a" ) ) ).thenReturn( userRes );
+
+        PrincipalRelationship member1 = from( group.getKey() ).to( PrincipalKey.from( "user:system:user1" ) );
+        PrincipalRelationship member2 = from( group.getKey() ).to( PrincipalKey.from( "user:system:user2" ) );
+        PrincipalRelationships members = PrincipalRelationships.from( member1, member2 );
+        Mockito.when( securityService.getRelationships( PrincipalKey.from( "group:system:group-a" ) ) ).thenReturn( members );
+
+        final PrincipalKeys membershipKeys = PrincipalKeys.from( role.getKey() );
+        Mockito.when( securityService.getMemberships( PrincipalKey.from( "group:system:group-a" ) ) ).thenReturn( membershipKeys );
+        final Principals memberships = Principals.from( role );
+        Mockito.when( securityService.getPrincipals( membershipKeys ) ).thenReturn( memberships );
+
+        String jsonString = request().
+            path( "security/principals/" + URLEncoder.encode( "group:system:group-a", "UTF-8" ) ).
+            queryParam( "memberships", "true" ).
+            get().getAsString();
+
+        assertJson( "getPrincipalGroupByIdWithMemberships.json", jsonString );
+    }
+
     @Test
     public void getPrincipalRoleById()
         throws Exception
@@ -493,10 +533,10 @@ public void updateGroup()
             build();
 
         Mockito.when( securityService.updateGroup( Mockito.any( UpdateGroupParams.class ) ) ).thenReturn( group );
-        PrincipalRelationship membership1 = from( group.getKey() ).to( PrincipalKey.from( "user:system:user1" ) );
-        PrincipalRelationship membership2 = from( group.getKey() ).to( PrincipalKey.from( "user:system:user2" ) );
-        PrincipalRelationships memberships = PrincipalRelationships.from( membership1, membership2 );
-        Mockito.when( securityService.getRelationships( group.getKey() ) ).thenReturn( memberships );
+        PrincipalRelationship members1 = from( group.getKey() ).to( PrincipalKey.from( "user:system:user1" ) );
+        PrincipalRelationship members2 = from( group.getKey() ).to( PrincipalKey.from( "user:system:user2" ) );
+        PrincipalRelationships members = PrincipalRelationships.from( members1, members2 );
+        Mockito.when( securityService.getRelationships( group.getKey() ) ).thenReturn( members );
 
         String jsonString = request().
             path( "security/principals/updateGroup" ).

modules/admin/admin-impl/src/test/resources/com/enonic/xp/admin/impl/rest/resource/security/createGroupSuccess.json

@@ -6,5 +6,6 @@
     "user:system:user1",
     "user:system:user2"
   ],
+  "memberships": [],
   "modifiedTime": "1970-01-01T00:00:00Z"
-}
+}

modules/admin/admin-impl/src/test/resources/com/enonic/xp/admin/impl/rest/resource/security/getPrincipalGroupById.json

@@ -6,5 +6,6 @@
     "user:system:user1",
     "user:system:user2"
   ],
+  "memberships": [],
   "modifiedTime": "1970-01-01T00:00:00Z"
-}
+}

modules/admin/admin-impl/src/test/resources/com/enonic/xp/admin/impl/rest/resource/security/getPrincipalGroupByIdWithMemberships.json

@@ -0,0 +1,18 @@
+{
+  "description": "group a",
+  "displayName": "Group A",
+  "key": "group:system:group-a",
+  "members": [
+    "user:system:user1",
+    "user:system:user2"
+  ],
+  "memberships": [
+    {
+      "description": "super u",
+      "displayName": "Super user role",
+      "key": "role:superuser",
+      "modifiedTime": "1970-01-01T00:00:00Z"
+    }
+  ],
+  "modifiedTime": "1970-01-01T00:00:00Z"
+}

modules/admin/admin-impl/src/test/resources/com/enonic/xp/admin/impl/rest/resource/security/updateGroupParams.json

@@ -7,5 +7,7 @@
   ],
   "removeMembers": [
     "user:system:user3"
-  ]
+  ],
+  "addMemberships": [],
+  "removeMemberships": []
 }

modules/admin/admin-ui/src/main/resources/web/admin/common/js/app/view/ItemDataGroup.ts

@@ -1,5 +1,8 @@
 module api.app.view {
 
+    import SpanEl = api.dom.SpanEl;
+    import StringHelper = api.util.StringHelper;
+
     export class ItemDataGroup extends api.dom.DivEl {
 
         private header: api.dom.H2El;
@@ -20,31 +23,23 @@ module api.app.view {
         }
 
         addDataArray(header: string, datas: string[]) {
-            let dataList = new api.dom.UlEl('data-list');
-
-            if (header) {
-                this.addHeader(header, dataList);
-            }
-
-            datas.forEach((data) => {
-                let dataElement = new api.dom.LiEl();
-                dataElement.getEl().setInnerHtml(data, false);
-                dataList.appendChild(dataElement);
-                this.empty = false;
-            });
-
-            this.appendChild(dataList);
+            const elements = datas.filter(text => !StringHelper.isBlank(text)).map(text => new SpanEl().setHtml(text, false));
+            this.addDataElements(header, elements);
         }
 
         addDataElements(header:string, datas:api.dom.Element[]) {
             let dataList = new api.dom.UlEl('data-list');
 
+            if (!datas || datas.length === 0) {
+                return;
+            }
+
             if (header) {
                 this.addHeader(header, dataList);
             }
 
             datas.forEach((data) => {
-                let dataElement = new api.dom.LiEl();
+                const dataElement = new api.dom.LiEl();
                 dataElement.appendChild(data);
                 dataList.appendChild(dataElement);
                 this.empty = false;