ilgrosso commented on code in PR #965:
URL: https://github.com/apache/syncope/pull/965#discussion_r1926500811


##########
client/idrepo/console/src/main/java/org/apache/syncope/client/console/panels/RealmChoicePanel.java:
##########
@@ -195,6 +196,15 @@ protected void populateItem(final ListItem<String> item) {
 
                     private static final long serialVersionUID = 
-817438685948164787L;
 
+                    @Override
+                    protected void onInitialize() {
+                        super.onInitialize();
+                        String fullPath = 
RealmsUtils.getFullPath(item.getModelObject());
+                        if (!fullPath.equals("/") && fullPath.lastIndexOf("/") 
== 0) {

Review Comment:
   use `!SyncopeConstants.ROOT_REALM.equals(fullPath)` for first check



##########
fit/core-reference/src/test/java/org/apache/syncope/fit/core/RealmITCase.java:
##########
@@ -401,4 +407,49 @@ public void issueSYNCOPE1472() {
 
         assertFalse(realmTO.getResources().contains("resource-ldap-orgunit"), 
"Should not contain removed resources");
     }
+
+        @Test
+    public void issueSYNCOPE1856() {
+        // CREATE ROLE
+        RoleTO roleTO = new RoleTO();
+        roleTO.getEntitlements()
+                .addAll(List.of(IdRepoEntitlement.REALM_SEARCH, 
IdRepoEntitlement.REALM_CREATE,
+                        IdRepoEntitlement.REALM_UPDATE, 
IdRepoEntitlement.REALM_DELETE));
+        roleTO.getRealms().add("/even");
+        roleTO.setKey("REALM_ADMIN");
+        Response roleResponse = ROLE_SERVICE.create(roleTO);
+        assertEquals(Response.Status.CREATED.getStatusCode(), 
roleResponse.getStatusInfo().getStatusCode());
+
+        // CREATE REALM MANAGER
+        UserCR userCR = 
UserITCase.getUniqueSample("mana...@syncope.apache.org");
+        userCR.setUsername("manager");
+        userCR.setRealm("/even");
+        userCR.getRoles().add(roleTO.getKey());
+        UserTO manager = createUser(userCR).getEntity();
+
+        RealmService managerRealmService = 
CLIENT_FACTORY.create(manager.getUsername(), "password123")
+                .getService(RealmService.class);
+
+        // MANAGER CANNOT CREATE REALM CHILD OF /
+        RealmTO realmTO = new RealmTO();
+        realmTO.setName("child");
+        assertThrows(SyncopeClientException.class, () -> 
managerRealmService.create("/", realmTO));
+
+        Response response = REALM_SERVICE.create("/", realmTO);
+        assertEquals(Response.Status.CREATED.getStatusCode(), 
response.getStatusInfo().getStatusCode());
+        RealmTO childRealm = REALM_SERVICE.search(new 
RealmQuery.Builder().base("/").keyword("child").build())
+                .getResult()
+                .get(0);
+
+        // MANAGER CANNOT UPDATE /child
+        assertThrows(SyncopeClientException.class, () -> 
managerRealmService.update(childRealm));
+
+        // MANAGER CANNOT DELETE /child
+        assertThrows(SyncopeClientException.class, () -> 
managerRealmService.delete(childRealm.getFullPath()));
+
+        //CLEAN
+        deleteUser(manager.getKey());

Review Comment:
   Use
   
   ```java
   try {
   ...
   } finally {
           deleteUser(manager.getKey());
           ROLE_SERVICE.delete("REALM_ADMIN");
           REALM_SERVICE.delete("/child");
   }
   ```



##########
client/idrepo/console/src/main/java/org/apache/syncope/client/console/panels/Realm.java:
##########
@@ -327,5 +337,27 @@ public boolean isVisible() {
             return 
SyncopeWebApplication.get().getSecuritySettings().getAuthorizationStrategy().
                     isActionAuthorized(this, RENDER);
         }
+
+        private boolean securityCheck(final String entitlement) {

Review Comment:
   avoid duplicating this method



##########
fit/core-reference/src/test/java/org/apache/syncope/fit/core/RealmITCase.java:
##########
@@ -401,4 +407,49 @@ public void issueSYNCOPE1472() {
 
         assertFalse(realmTO.getResources().contains("resource-ldap-orgunit"), 
"Should not contain removed resources");
     }
+
+        @Test
+    public void issueSYNCOPE1856() {
+        // CREATE ROLE
+        RoleTO roleTO = new RoleTO();
+        roleTO.getEntitlements()
+                .addAll(List.of(IdRepoEntitlement.REALM_SEARCH, 
IdRepoEntitlement.REALM_CREATE,
+                        IdRepoEntitlement.REALM_UPDATE, 
IdRepoEntitlement.REALM_DELETE));
+        roleTO.getRealms().add("/even");
+        roleTO.setKey("REALM_ADMIN");
+        Response roleResponse = ROLE_SERVICE.create(roleTO);

Review Comment:
   Use `createRole()`



##########
client/idrepo/console/src/main/java/org/apache/syncope/client/console/panels/Realm.java:
##########
@@ -327,5 +337,27 @@ public boolean isVisible() {
             return 
SyncopeWebApplication.get().getSecuritySettings().getAuthorizationStrategy().
                     isActionAuthorized(this, RENDER);
         }
+
+        private boolean securityCheck(final String entitlement) {
+            return securityCheck(Set.of(entitlement));
+        }
+
+        private boolean securityCheck(final Set<String> entitlements) {

Review Comment:
   It is not acceptable to check each single role via REST, you have all 
required information in session



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@syncope.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to