From b330981b408ea7ea9e7861fa043aaf95e67b6a0c Mon Sep 17 00:00:00 2001 From: Steffen Schaefer Date: Wed, 29 May 2019 10:00:53 +0200 Subject: [PATCH] Bug 5047: Simplified the various map and filter methods in SecurityService by removing the redundant HasPermissions parameter. This can now be obtained from the given WithQualifiedObjectIdentifier values. --- .../gwt/ui/server/SailingServiceImpl.java | 25 +++++++++---------- .../ui/server/UserManagementServiceImpl.java | 3 +-- .../com/sap/sse/security/SecurityService.java | 10 ++++---- .../security/impl/SecurityServiceImpl.java | 24 +++++++++--------- 4 files changed, 30 insertions(+), 32 deletions(-) diff --git a/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/server/SailingServiceImpl.java b/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/server/SailingServiceImpl.java index a4ee5565d2f..ed1757b16b0 100644 --- a/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/server/SailingServiceImpl.java +++ b/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/server/SailingServiceImpl.java @@ -1024,13 +1024,13 @@ public class SailingServiceImpl extends ResultCachingProxiedRemoteServiceServlet @Override public List getRegattas() { - return getSecurityService().mapAndFilterByReadPermissionForCurrentUser(SecuredDomainType.REGATTA, + return getSecurityService().mapAndFilterByReadPermissionForCurrentUser( getService().getAllRegattas(), this::convertToRegattaDTO); } @Override public List getRegattasWithUpdatePermission() { - return getSecurityService().mapAndFilterByExplicitPermissionForCurrentUser(SecuredDomainType.REGATTA, + return getSecurityService().mapAndFilterByExplicitPermissionForCurrentUser( new HasPermissions.Action[] { DefaultActions.READ, DefaultActions.UPDATE }, getService().getAllRegattas(), this::convertToRegattaDTO); } @@ -1413,7 +1413,7 @@ public class SailingServiceImpl extends ResultCachingProxiedRemoteServiceServlet @Override public List getPreviousTracTracConfigurations() throws Exception { final Iterable configs = tractracDomainObjectFactory.getTracTracConfigurations(); - return getSecurityService().mapAndFilterByReadPermissionForCurrentUser(SecuredDomainType.TRACTRAC_ACCOUNT, + return getSecurityService().mapAndFilterByReadPermissionForCurrentUser( configs, ttConfig -> { TracTracConfigurationWithSecurityDTO config = new TracTracConfigurationWithSecurityDTO( @@ -2712,7 +2712,7 @@ public class SailingServiceImpl extends ResultCachingProxiedRemoteServiceServlet private ArrayList getLeaderboardNamesFilteredForCurrentUser() { final ArrayList result = new ArrayList<>(); - getSecurityService().filterObjectsWithPermissionForCurrentUser(SecuredDomainType.LEADERBOARD, + getSecurityService().filterObjectsWithPermissionForCurrentUser( DefaultActions.READ, getService().getLeaderboards().values(), l -> result.add(l.getName())); return result; } @@ -2782,7 +2782,7 @@ public class SailingServiceImpl extends ResultCachingProxiedRemoteServiceServlet @Override public List getLeaderboardsWithSecurity() { final Map leaderboards = getService().getLeaderboards(); - return getSecurityService().mapAndFilterByReadPermissionForCurrentUser(SecuredDomainType.LEADERBOARD, + return getSecurityService().mapAndFilterByReadPermissionForCurrentUser( leaderboards.values(), leaderboard -> createStrippedLeaderboardDTOWithSecurity(leaderboard, false, false)); } @@ -3241,7 +3241,7 @@ public class SailingServiceImpl extends ResultCachingProxiedRemoteServiceServlet @Override public List getPreviousSwissTimingConfigurations() { Iterable configs = swissTimingAdapterPersistence.getSwissTimingConfigurations(); - return getSecurityService().mapAndFilterByReadPermissionForCurrentUser(SecuredDomainType.SWISS_TIMING_ACCOUNT, configs, + return getSecurityService().mapAndFilterByReadPermissionForCurrentUser(configs, stConfig -> { final SwissTimingConfigurationWithSecurityDTO config = new SwissTimingConfigurationWithSecurityDTO( stConfig.getName(), stConfig.getJsonURL(), @@ -4179,7 +4179,7 @@ public class SailingServiceImpl extends ResultCachingProxiedRemoteServiceServlet @Override public List getLeaderboardGroups(boolean withGeoLocationData) { final Map leaderboardGroups = getService().getLeaderboardGroups(); - return getSecurityService().mapAndFilterByReadPermissionForCurrentUser(SecuredDomainType.LEADERBOARD_GROUP, + return getSecurityService().mapAndFilterByReadPermissionForCurrentUser( leaderboardGroups.values(), leaderboardGroup -> convertToLeaderboardGroupDTO(leaderboardGroup, withGeoLocationData, false)); } @@ -4349,7 +4349,7 @@ public class SailingServiceImpl extends ResultCachingProxiedRemoteServiceServlet @Override public List getEvents() throws MalformedURLException { - return getSecurityService().mapAndFilterByReadPermissionForCurrentUser(SecuredDomainType.EVENT, + return getSecurityService().mapAndFilterByReadPermissionForCurrentUser( getService().getAllEvents(), event -> { EventDTO eventDTO = convertToEventDTO(event, false); try { @@ -5285,8 +5285,7 @@ public class SailingServiceImpl extends ResultCachingProxiedRemoteServiceServlet public List getPreviousSwissTimingArchiveConfigurations() { Iterable configs = swissTimingAdapterPersistence .getSwissTimingArchiveConfigurations(); - return getSecurityService().mapAndFilterByReadPermissionForCurrentUser( - SecuredDomainType.SWISS_TIMING_ARCHIVE_ACCOUNT, configs, + return getSecurityService().mapAndFilterByReadPermissionForCurrentUser(configs, stArchiveConfig -> { SwissTimingArchiveConfigurationWithSecurityDTO config = new SwissTimingArchiveConfigurationWithSecurityDTO( stArchiveConfig.getJsonURL(), stArchiveConfig.getCreatorName()); @@ -6059,7 +6058,7 @@ public class SailingServiceImpl extends ResultCachingProxiedRemoteServiceServlet @Override public Iterable getStandaloneBoats() { List result = new ArrayList<>(); - getSecurityService().filterObjectsWithAnyPermissionForCurrentUser(SecuredDomainType.BOAT, + getSecurityService().filterObjectsWithAnyPermissionForCurrentUser( SecuredSecurityTypes.PublicReadableActions.READ_AND_READ_PUBLIC_ACTIONS, getService().getBaseDomainFactory().getCompetitorAndBoatStore().getStandaloneBoats(), filteredObject -> result.add(convertToBoatDTO(filteredObject))); @@ -6481,13 +6480,13 @@ public class SailingServiceImpl extends ResultCachingProxiedRemoteServiceServlet @Override public Iterable getAllIgtimiAccountEmailAddresses() { - return getSecurityService().mapAndFilterByReadPermissionForCurrentUser(SecuredDomainType.IGTIMI_ACCOUNT, + return getSecurityService().mapAndFilterByReadPermissionForCurrentUser( getIgtimiConnectionFactory().getAllAccounts(), acc -> acc.getUser().getEmail()); } @Override public Iterable getAllIgtimiAccountsWithSecurity() { - return getSecurityService().mapAndFilterByReadPermissionForCurrentUser(SecuredDomainType.IGTIMI_ACCOUNT, + return getSecurityService().mapAndFilterByReadPermissionForCurrentUser( getIgtimiConnectionFactory().getAllAccounts(), this::toSecuredIgtimiAccountDTO); } diff --git a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/UserManagementServiceImpl.java b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/UserManagementServiceImpl.java index 16628d1e23c..e55dcd973d0 100644 --- a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/UserManagementServiceImpl.java +++ b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/server/UserManagementServiceImpl.java @@ -284,8 +284,7 @@ public class UserManagementServiceImpl extends RemoteServiceServlet implements U public Collection getUserGroups() { Map fromOriginalToStrippedDownUser = new HashMap<>(); Map fromOriginalToStrippedDownUserGroup = new HashMap<>(); - return getSecurityService().mapAndFilterByReadPermissionForCurrentUser(SecuredSecurityTypes.USER_GROUP, - getSecurityService().getUserGroupList(), + return getSecurityService().mapAndFilterByReadPermissionForCurrentUser(getSecurityService().getUserGroupList(), ug -> securityDTOFactory.createUserGroupDTOFromUserGroup(ug, fromOriginalToStrippedDownUser, fromOriginalToStrippedDownUserGroup, getSecurityService())); } diff --git a/java/com.sap.sse.security/src/com/sap/sse/security/SecurityService.java b/java/com.sap.sse.security/src/com/sap/sse/security/SecurityService.java index 441ef4719ed..1a04b54b777 100644 --- a/java/com.sap.sse.security/src/com/sap/sse/security/SecurityService.java +++ b/java/com.sap.sse.security/src/com/sap/sse/security/SecurityService.java @@ -391,11 +391,11 @@ public interface SecurityService extends ReplicableWithObjectInputStream void filterObjectsWithPermissionForCurrentUser(HasPermissions permittedObject, + void filterObjectsWithPermissionForCurrentUser( com.sap.sse.security.shared.HasPermissions.Action action, Iterable objectsToFilter, Consumer filteredObjectsConsumer); - void filterObjectsWithPermissionForCurrentUser(HasPermissions permittedObject, + void filterObjectsWithPermissionForCurrentUser( com.sap.sse.security.shared.HasPermissions.Action[] actions, Iterable objectsToFilter, Consumer filteredObjectsConsumer); @@ -403,13 +403,13 @@ public interface SecurityService extends ReplicableWithObjectInputStream void filterObjectsWithAnyPermissionForCurrentUser( - HasPermissions permittedObject, com.sap.sse.security.shared.HasPermissions.Action[] actions, + com.sap.sse.security.shared.HasPermissions.Action[] actions, Iterable objectsToFilter, Consumer filteredObjectsConsumer); - List mapAndFilterByReadPermissionForCurrentUser(HasPermissions permittedObject, + List mapAndFilterByReadPermissionForCurrentUser( Iterable objectsToFilter, Function filteredObjectsMapper); - List mapAndFilterByExplicitPermissionForCurrentUser(HasPermissions permittedObject, + List mapAndFilterByExplicitPermissionForCurrentUser( HasPermissions.Action[] actions, Iterable objectsToFilter, Function filteredObjectsMapper); diff --git a/java/com.sap.sse.security/src/com/sap/sse/security/impl/SecurityServiceImpl.java b/java/com.sap.sse.security/src/com/sap/sse/security/impl/SecurityServiceImpl.java index ad0a471bdac..b90b25dd91c 100644 --- a/java/com.sap.sse.security/src/com/sap/sse/security/impl/SecurityServiceImpl.java +++ b/java/com.sap.sse.security/src/com/sap/sse/security/impl/SecurityServiceImpl.java @@ -1082,7 +1082,7 @@ public class SecurityServiceImpl implements ReplicableSecurityService, ClearStat @Override public Iterable getRoleDefinitions() { Collection result = new ArrayList<>(); - filterObjectsWithPermissionForCurrentUser(SecuredSecurityTypes.ROLE_DEFINITION, DefaultActions.READ, + filterObjectsWithPermissionForCurrentUser(DefaultActions.READ, store.getRoleDefinitions(), t -> result.add(t)); return result; } @@ -1822,26 +1822,26 @@ public class SecurityServiceImpl implements ReplicableSecurityService, ClearStat } @Override - public void filterObjectsWithPermissionForCurrentUser(HasPermissions permittedObject, + public void filterObjectsWithPermissionForCurrentUser( HasPermissions.Action action, Iterable objectsToFilter, Consumer filteredObjectsConsumer) { objectsToFilter.forEach(objectToCheck -> { if (SecurityUtils.getSubject().isPermitted( - permittedObject.getStringPermissionForObject(action, objectToCheck))) { + objectToCheck.getIdentifier().getStringPermission(action))) { filteredObjectsConsumer.accept(objectToCheck); } }); } @Override - public void filterObjectsWithPermissionForCurrentUser(HasPermissions permittedObject, + public void filterObjectsWithPermissionForCurrentUser( HasPermissions.Action[] actions, Iterable objectsToFilter, Consumer filteredObjectsConsumer) { objectsToFilter.forEach(objectToCheck -> { boolean isPermitted = actions.length > 0; for (int i = 0; i < actions.length; i++) { isPermitted &= SecurityUtils.getSubject().isPermitted( - permittedObject.getStringPermissionForObject(actions[i], objectToCheck)); + objectToCheck.getIdentifier().getStringPermission(actions[i])); } if (isPermitted) { filteredObjectsConsumer.accept(objectToCheck); @@ -1852,13 +1852,13 @@ public class SecurityServiceImpl implements ReplicableSecurityService, ClearStat /** Filters the objects with any of the given permissions for the current user */ @Override public void filterObjectsWithAnyPermissionForCurrentUser( - HasPermissions permittedObject, HasPermissions.Action[] actions, Iterable objectsToFilter, + HasPermissions.Action[] actions, Iterable objectsToFilter, Consumer filteredObjectsConsumer) { objectsToFilter.forEach(objectToCheck -> { boolean isPermitted = false; for (int i = 0; i < actions.length; i++) { if (SecurityUtils.getSubject() - .isPermitted(permittedObject.getStringPermissionForObject(actions[i], objectToCheck))) { + .isPermitted(objectToCheck.getIdentifier().getStringPermission(actions[i]))) { isPermitted = true; break; } @@ -1870,20 +1870,20 @@ public class SecurityServiceImpl implements ReplicableSecurityService, ClearStat } @Override - public List mapAndFilterByReadPermissionForCurrentUser(HasPermissions permittedObject, + public List mapAndFilterByReadPermissionForCurrentUser( Iterable objectsToFilter, Function filteredObjectsMapper) { final List result = new ArrayList<>(); - filterObjectsWithPermissionForCurrentUser(permittedObject, DefaultActions.READ, objectsToFilter, + filterObjectsWithPermissionForCurrentUser(DefaultActions.READ, objectsToFilter, filteredObject -> result.add(filteredObjectsMapper.apply(filteredObject))); return result; } @Override - public List mapAndFilterByExplicitPermissionForCurrentUser(HasPermissions permittedObject, + public List mapAndFilterByExplicitPermissionForCurrentUser( HasPermissions.Action[] actions, Iterable objectsToFilter, Function filteredObjectsMapper) { final List result = new ArrayList<>(); - filterObjectsWithPermissionForCurrentUser(permittedObject, actions, objectsToFilter, + filterObjectsWithPermissionForCurrentUser(actions, objectsToFilter, filteredObject -> result.add(filteredObjectsMapper.apply(filteredObject))); return result; } @@ -1893,7 +1893,7 @@ public class SecurityServiceImpl implements ReplicableSecurityService, ClearStat HasPermissions permittedObject, HasPermissions.Action[] actions, Iterable objectsToFilter, Function filteredObjectsMapper) { final List result = new ArrayList<>(); - filterObjectsWithAnyPermissionForCurrentUser(permittedObject, actions, objectsToFilter, + filterObjectsWithAnyPermissionForCurrentUser(actions, objectsToFilter, filteredObject -> result.add(filteredObjectsMapper.apply(filteredObject))); return result; }