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