From bbafa75c279c3b549390b5a003f1088537408364 Mon Sep 17 00:00:00 2001 From: Steffen Jacobs Date: Mon, 3 Dec 2018 13:31:18 +0100 Subject: [PATCH 1/4] removed unnecessary duplicate code in UserGroupManagementPanel and UserGroupTableWrapper --- .../component/UserGroupManagementPanel.java | 19 ++---------- .../component/UserGroupTableWrapper.java | 29 ++++--------------- 2 files changed, 8 insertions(+), 40 deletions(-) diff --git a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/client/component/UserGroupManagementPanel.java b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/client/component/UserGroupManagementPanel.java index 1a1f47ac2ef..191da1de252 100755 --- a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/client/component/UserGroupManagementPanel.java +++ b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/client/component/UserGroupManagementPanel.java @@ -1,7 +1,5 @@ package com.sap.sse.security.ui.client.component; -import java.util.Collection; - import com.google.gwt.event.dom.client.ClickEvent; import com.google.gwt.event.dom.client.ClickHandler; import com.google.gwt.user.client.Window; @@ -33,12 +31,10 @@ public class UserGroupManagementPanel extends DockPanel { private final UserGroupTableWrapper userGroupTableWrapper; - private final UserManagementServiceAsync userManagementService; - public UserGroupManagementPanel(final UserService userService, final StringMessages stringMessages, Iterable additionalPermissions, ErrorReporter errorReporter, CellTableWithCheckboxResources tableResources) { - userManagementService = userService.getUserManagementService(); + final UserManagementServiceAsync userManagementService = userService.getUserManagementService(); final VerticalPanel west = new VerticalPanel(); // create button bar @@ -133,18 +129,7 @@ public class UserGroupManagementPanel extends DockPanel { /** Updates the UserGroups. */ public void updateUserGroups() { - userManagementService.getUserGroups(new AsyncCallback>() { - - @Override - public void onFailure(Throwable caught) { - Window.alert(caught.getMessage()); - } - - @Override - public void onSuccess(Collection result) { - userGroupTableWrapper.refreshUserGroups(result); - } - }); + userGroupTableWrapper.refreshUserList(null); userGroupListDataProvider.updateDisplays(); userGroupDetailPanel.updateLists(); } diff --git a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/client/component/UserGroupTableWrapper.java b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/client/component/UserGroupTableWrapper.java index ff487080374..b5634189c81 100644 --- a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/client/component/UserGroupTableWrapper.java +++ b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/client/component/UserGroupTableWrapper.java @@ -26,7 +26,6 @@ import com.sap.sse.security.shared.HasPermissions; import com.sap.sse.security.shared.HasPermissions.DefaultActions; import com.sap.sse.security.shared.dto.UserGroupDTO; import com.sap.sse.security.shared.impl.SecuredSecurityTypes; -import com.sap.sse.security.ui.client.UserManagementServiceAsync; import com.sap.sse.security.ui.client.UserService; import com.sap.sse.security.ui.client.component.editacl.EditACLDialog; import com.sap.sse.security.ui.client.i18n.StringMessages; @@ -57,10 +56,10 @@ public class UserGroupTableWrapper extends } }, tableResources); this.userService = userService; - ListHandler userColumnListHandler = getColumnSortHandler(); + final ListHandler userColumnListHandler = getColumnSortHandler(); // users table - TextColumn UserGroupWithSecurityDTONameColumn = new AbstractSortableTextColumn( + final TextColumn UserGroupWithSecurityDTONameColumn = new AbstractSortableTextColumn( UserGroupDTO -> UserGroupDTO.getName(), userColumnListHandler); final HasPermissions type = SecuredSecurityTypes.USER_GROUP; @@ -69,7 +68,7 @@ public class UserGroupTableWrapper extends new DefaultActionsImagesBarCell(stringMessages), userService, type, idFactory); actionColumn.addAction(ACTION_DELETE, DELETE, userGroupDTO -> { if (Window.confirm(stringMessages.doYouReallyWantToRemoveUserGroup(userGroupDTO.getName()))) { - getUserManagementService().deleteUserGroup(userGroupDTO.getId().toString(), + userService.getUserManagementService().deleteUserGroup(userGroupDTO.getId().toString(), new AsyncCallback() { @Override public void onFailure(Throwable caught) { @@ -101,6 +100,7 @@ public class UserGroupTableWrapper extends final EditACLDialog.DialogConfig configACL = EditACLDialog.create( userService.getUserManagementService(), type, idFactory, user -> user.getAccessControlList(), stringMessages); + actionColumn.addAction(DefaultActionsImagesBarCell.ACTION_CHANGE_OWNERSHIP, DefaultActions.CHANGE_OWNERSHIP, configOwnership::openDialog); actionColumn.addAction(DefaultActionsImagesBarCell.ACTION_CHANGE_ACL, DefaultActions.CHANGE_ACL, @@ -130,18 +130,10 @@ public class UserGroupTableWrapper extends table.ensureDebugId("UserGroupWithSecurityDTOTable"); } - public Iterable getAllUsers() { - return filterField.getAll(); - } - public LabeledAbstractFilterablePanel getFilterField() { return filterField; } - public void refreshUserGroups(Iterable UserGroupWithSecurityDTOs) { - getFilteredUserGroups(UserGroupWithSecurityDTOs); - } - /** * @param callback * optional; may be {@code null} @@ -159,21 +151,12 @@ public class UserGroupTableWrapper extends @Override public void onSuccess(Collection result) { - getFilteredUserGroups(result); - refreshUserGroups(result); + filterField.updateAll(result); if (callback != null) { callback.onSuccess(result); } } }; - getUserManagementService().getUserGroups(myCallback); - } - - private void getFilteredUserGroups(Iterable result) { - filterField.updateAll(result); - } - - private UserManagementServiceAsync getUserManagementService() { - return userService.getUserManagementService(); + userService.getUserManagementService().getUserGroups(myCallback); } } From 6c12bf37cc9bbee8a77f394bbe0a4761373f8828 Mon Sep 17 00:00:00 2001 From: Steffen Jacobs Date: Mon, 3 Dec 2018 14:53:57 +0100 Subject: [PATCH 2/4] take usergroups of alluser into account during pruning --- .../security/ui/server/SecurityDTOFactory.java | 13 ++++++++++++- .../security/ui/server/SecurityDTOUtil.java | 18 ++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOFactory.java b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOFactory.java index 8108aef5013..868fceda30d 100755 --- a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOFactory.java +++ b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOFactory.java @@ -192,6 +192,15 @@ public class SecurityDTOFactory { return result; } + public Iterable createStrippedUserGroupDTOFromUserGroups(Iterable userGroups, + Map fromOriginalToStrippedDownUserGrou) { + Collection result = new ArrayList<>(); + for (UserGroup userGroup : userGroups) { + result.add(createStrippedUserGroupDTOFromUserGroup(userGroup, fromOriginalToStrippedDownUserGrou)); + } + return result; + } + public StrippedUserGroupDTO createStrippedUserGroupDTOFromUserGroup(UserGroup userGroup, Map fromOriginalToStrippedDownUserGroup) { final StrippedUserGroupDTO result; @@ -270,9 +279,11 @@ public class SecurityDTOFactory { * @param fromOriginalToStrippedDownUser * @param fromOriginalToStrippedDownUserGroup */ - public AccessControlListDTO pruneAccessControlListForUser(AccessControlListDTO acl, StrippedUserDTO filterForUser) { + public AccessControlListDTO pruneAccessControlListForUser(AccessControlListDTO acl, StrippedUserDTO filterForUser, + Iterable allUserGroups) { final AccessControlListDTO result; final Collection userGroups = Util.createSet(filterForUser.getUserGroups()); + userGroups.addAll(Util.createSet(allUserGroups)); if (acl != null) { final Map> actionsByUserGroup = new HashMap<>(); diff --git a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOUtil.java b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOUtil.java index 2b97622609c..75afa8768b0 100644 --- a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOUtil.java +++ b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOUtil.java @@ -25,6 +25,8 @@ public abstract class SecurityDTOUtil { private SecurityDTOUtil() { } + private static Iterable allUserGroups; + /** * Adds {@link AccessControlList access control list} and {@link Ownership ownership} information for the given * {@link QualifiedObjectIdentifier qualified object identifier} to the provided {@link NamedSecuredObjectDTO @@ -107,11 +109,13 @@ public abstract class SecurityDTOUtil { } else { User user = securityService.getCurrentUser(); if (user != null) { - // TODO also prune for all user StrippedUserDTO userDTO = new SecurityDTOFactory().createStrippedUserFromUser(user, securityService, fromOriginalToStrippedDownUser, fromOriginalToStrippedDownUserGroup); + final Iterable allUserGroups2 = getAlluserGroups(securityService, + securityDTOFactory); securedObject.setAccessControlList( - securityDTOFactory.pruneAccessControlListForUser(accessControlListDTO, userDTO)); + securityDTOFactory.pruneAccessControlListForUser(accessControlListDTO, userDTO, + allUserGroups2)); } } final OwnershipAnnotation ownership = securityService.getOwnership(objectId); @@ -119,4 +123,14 @@ public abstract class SecurityDTOUtil { securityDTOFactory.createOwnershipDTO(ownership == null ? null : ownership.getAnnotation(), fromOriginalToStrippedDownUser, fromOriginalToStrippedDownUserGroup)); } + + /** Get groups for the user alluser. */ + private static Iterable getAlluserGroups(SecurityService securityService, + SecurityDTOFactory securityDTOFactory) { + if (allUserGroups == null) { + allUserGroups = securityDTOFactory.createStrippedUserGroupDTOFromUserGroups( + securityService.getAllUser().getUserGroups(), new HashMap<>()); + } + return allUserGroups; + } } From 2815513286467c1721cd5814c5c1c642a648b233 Mon Sep 17 00:00:00 2001 From: Steffen Jacobs Date: Mon, 3 Dec 2018 14:58:29 +0100 Subject: [PATCH 3/4] adjusted retrieval of UserGroups of Alluser to avoid possible NullPointerException and renamed method --- .../sap/sse/security/ui/server/SecurityDTOFactory.java | 4 +++- .../com/sap/sse/security/ui/server/SecurityDTOUtil.java | 8 +++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOFactory.java b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOFactory.java index 868fceda30d..1769604af03 100755 --- a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOFactory.java +++ b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOFactory.java @@ -283,7 +283,9 @@ public class SecurityDTOFactory { Iterable allUserGroups) { final AccessControlListDTO result; final Collection userGroups = Util.createSet(filterForUser.getUserGroups()); - userGroups.addAll(Util.createSet(allUserGroups)); + if (allUserGroups != null) { + userGroups.addAll(Util.createSet(allUserGroups)); + } if (acl != null) { final Map> actionsByUserGroup = new HashMap<>(); diff --git a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOUtil.java b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOUtil.java index 75afa8768b0..3ffe0733b11 100644 --- a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOUtil.java +++ b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOUtil.java @@ -111,7 +111,7 @@ public abstract class SecurityDTOUtil { if (user != null) { StrippedUserDTO userDTO = new SecurityDTOFactory().createStrippedUserFromUser(user, securityService, fromOriginalToStrippedDownUser, fromOriginalToStrippedDownUserGroup); - final Iterable allUserGroups2 = getAlluserGroups(securityService, + final Iterable allUserGroups2 = getUserGroupsForAlluser(securityService, securityDTOFactory); securedObject.setAccessControlList( securityDTOFactory.pruneAccessControlListForUser(accessControlListDTO, userDTO, @@ -124,12 +124,14 @@ public abstract class SecurityDTOUtil { fromOriginalToStrippedDownUser, fromOriginalToStrippedDownUserGroup)); } - /** Get groups for the user alluser. */ - private static Iterable getAlluserGroups(SecurityService securityService, + /** Get the UserGroups for the user alluser. */ + private static Iterable getUserGroupsForAlluser(SecurityService securityService, SecurityDTOFactory securityDTOFactory) { if (allUserGroups == null) { + if (securityService.getAllUser() != null) { allUserGroups = securityDTOFactory.createStrippedUserGroupDTOFromUserGroups( securityService.getAllUser().getUserGroups(), new HashMap<>()); + } } return allUserGroups; } From c5935a23504c912f46f39fece648604ca3af9e7a Mon Sep 17 00:00:00 2001 From: Steffen Jacobs Date: Mon, 3 Dec 2018 15:16:59 +0100 Subject: [PATCH 4/4] added null usergroup to pruning whitelist --- .../com/sap/sse/security/ui/server/SecurityDTOFactory.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOFactory.java b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOFactory.java index 1769604af03..20cd32ab7c9 100755 --- a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOFactory.java +++ b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/SecurityDTOFactory.java @@ -282,10 +282,14 @@ public class SecurityDTOFactory { public AccessControlListDTO pruneAccessControlListForUser(AccessControlListDTO acl, StrippedUserDTO filterForUser, Iterable allUserGroups) { final AccessControlListDTO result; + // add user groups of filterForUser user final Collection userGroups = Util.createSet(filterForUser.getUserGroups()); + // add user groups of alluser if (allUserGroups != null) { userGroups.addAll(Util.createSet(allUserGroups)); } + // add null user group + userGroups.add(null); if (acl != null) { final Map> actionsByUserGroup = new HashMap<>();