From 55e6297f55cdf1ad1ad0626832bef1be3f2c3928 Mon Sep 17 00:00:00 2001 From: Axel Uhl Date: Tue, 12 Dec 2017 18:25:41 +0100 Subject: [PATCH] more consistent user/group/tenant handling and serialization Change-Id: Ib7621e2cbddb913db0e32e2607d122e38505d87b --- .../domain/common/security/Permission.java | 13 +++- .../ui/adminconsole/EventListComposite.java | 11 +-- .../sailing/gwt/ui/client/RaceTimePanel.java | 2 +- .../media/MediaPlayerManagerComponent.java | 4 +- .../raceboard/SideBySideComponentViewer.java | 4 +- .../gateway/jaxrs/api/EventsResource.java | 2 +- .../gwt/adminconsole/AdminConsolePanel.java | 2 +- .../gwt}/adminconsole/AdminConsoleTable.css | 0 .../security/shared/DefaultPermissions.java | 11 +++ .../sap/sse/security/shared/Permission.java | 30 ++++++--- .../security/shared/PermissionChecker.java | 11 ++- .../sap/sse/security/shared/SecurityUser.java | 25 +++++++ .../shared/impl/SecurityUserImpl.java | 17 ++++- .../app/AuthenticationContextImpl.java | 2 +- .../ui/client/component/UserList.java | 3 +- .../ui/server/UserManagementServiceImpl.java | 67 ++++++++++++++++--- .../sap/sse/security/ui/shared/UserDTO.java | 28 ++++---- .../userstore/mongodb/UserStoreImpl.java | 4 +- .../mongodb/impl/DomainObjectFactoryImpl.java | 5 +- .../com/sap/sse/security/SecurityService.java | 2 + .../security/impl/SecurityServiceImpl.java | 15 ++++- 21 files changed, 201 insertions(+), 57 deletions(-) rename java/{com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui => com.sap.sse.gwt.adminconsole/src/com/sap/sse/gwt}/adminconsole/AdminConsoleTable.css (100%) diff --git a/java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/security/Permission.java b/java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/security/Permission.java index 40a5d7e7893..f1b5934cc89 100755 --- a/java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/security/Permission.java +++ b/java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/security/Permission.java @@ -1,7 +1,6 @@ package com.sap.sailing.domain.common.security; - - +import com.sap.sse.security.shared.WildcardPermission; public enum Permission implements com.sap.sse.security.shared.Permission { // AdminConsole permissions @@ -63,6 +62,11 @@ public enum Permission implements com.sap.sse.security.shared.Permission { return result; } + @Override + public WildcardPermission getPermission(com.sap.sse.security.shared.Permission.Mode... modes) { + return new WildcardPermission(getStringPermission(modes)); + } + // TODO once we can use Java8 here, move this up into a "default" method on the Permission interface @Override public String getStringPermissionForObjects(com.sap.sse.security.shared.Permission.Mode mode, String... objectIdentifiers) { @@ -82,6 +86,11 @@ public enum Permission implements com.sap.sse.security.shared.Permission { return result.toString(); } + @Override + public WildcardPermission getPermissionForObjects(com.sap.sse.security.shared.Permission.Mode mode, String... objectIdentifiers) { + return new WildcardPermission(getStringPermissionForObjects(mode, objectIdentifiers)); + } + /** * The mode of interaction with a resource; used as the second element of a wildcard permission * diff --git a/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/adminconsole/EventListComposite.java b/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/adminconsole/EventListComposite.java index 2b177eeba8d..d53cd0e46a2 100755 --- a/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/adminconsole/EventListComposite.java +++ b/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/adminconsole/EventListComposite.java @@ -352,11 +352,12 @@ public class EventListComposite extends Composite implements EventsRefresher, Le public Iterable getAllowedActions(EventDTO event) { ArrayList allowedActions = new ArrayList<>(); for (Action action : Arrays.asList(DefaultActions.EDIT, DefaultActions.REMOVE)) { - if (user.hasPermission( - PermissionBuilderImpl.getInstance().getPermission("com.sap.sailing.domain.base.Event", action, event.id.toString()), - event.getAcl(), event.getOwnership())) { - allowedActions.add(action); - } + if (user.hasPermission( + PermissionBuilderImpl.getInstance().getPermission( + "com.sap.sailing.domain.base.Event", action, event.id.toString()), + event.getOwnership(), event.getAcl())) { + allowedActions.add(action); + } } return allowedActions; } diff --git a/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/client/RaceTimePanel.java b/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/client/RaceTimePanel.java index 43d31e5c469..878cd5ed226 100644 --- a/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/client/RaceTimePanel.java +++ b/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/client/RaceTimePanel.java @@ -60,7 +60,7 @@ public class RaceTimePanel extends TimePanel implements R @Override public void onUserStatusChange(UserDTO user, boolean preAuthenticated) { RaceTimePanel.this.hasCanReplayDuringLiveRacesPermission = user != null && user.hasPermission( - Permission.CAN_REPLAY_DURING_LIVE_RACES.getStringPermission()); + Permission.CAN_REPLAY_DURING_LIVE_RACES.getPermission(), /* TODO race ownership */ null, /* TODO race acl */ null); } }; diff --git a/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/client/media/MediaPlayerManagerComponent.java b/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/client/media/MediaPlayerManagerComponent.java index 4d6a6001b90..58480b44038 100755 --- a/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/client/media/MediaPlayerManagerComponent.java +++ b/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/client/media/MediaPlayerManagerComponent.java @@ -489,7 +489,7 @@ public class MediaPlayerManagerComponent extends AbstractComponentmode specified as the second wildcard permission - * segment, and the objectIdentifier as the third wildcard permission segment. + * Same as {@link #getStringPermission(Mode...)}, only that the result is a {@link WildcardPermission} instead of a + * {@link String} + */ + WildcardPermission getPermission(Mode... modes); + + /** + * Produces a string permission for this permission, the mode specified as the second wildcard + * permission segment, and the objectIdentifier as the third wildcard permission segment. */ String getStringPermissionForObjects(Mode mode, String... objectIdentifiers); - + + /** + * Same as {@link #getStringPermissionForObjects(Mode, String...)}, only that the result is a + * {@link WildcardPermission} instead of a {@link String} + */ + WildcardPermission getPermissionForObjects(Mode mode, String... objectIdentifiers); + public static interface Mode { String name(); - + int ordinal(); - + String getStringPermission(); } - + public enum DefaultModes implements Mode { CREATE, READ, UPDATE, DELETE; diff --git a/java/com.sap.sse.security.common/src/com/sap/sse/security/shared/PermissionChecker.java b/java/com.sap.sse.security.common/src/com/sap/sse/security/shared/PermissionChecker.java index dfafcfc5bf5..a3364de2dca 100755 --- a/java/com.sap.sse.security.common/src/com/sap/sse/security/shared/PermissionChecker.java +++ b/java/com.sap.sse.security.common/src/com/sap/sse/security/shared/PermissionChecker.java @@ -27,9 +27,14 @@ public class PermissionChecker { } /** - * @param permission Permission of the form "data_object_type:action:instance_id". - * The instance id can be omitted when a general permission for the data - * object type is asked after (e.g. "event:create"). + * @param permission + * Permission of the form "data_object_type:action:instance_id". The instance id can be omitted when a + * general permission for the data object type is asked after (e.g. "event:create"). + * @param ownership + * may be {@code null}, causing user- or tenant-parameterized roles and no user ownership override to be + * applied + * @param acl + * may be {@code null} in which case no ACL-specific checks are performed */ public static boolean isPermitted(WildcardPermission permission, SecurityUser user, Iterable groupsOfWhichUserIsMember, Iterable roles, Ownership ownership, diff --git a/java/com.sap.sse.security.common/src/com/sap/sse/security/shared/SecurityUser.java b/java/com.sap.sse.security.common/src/com/sap/sse/security/shared/SecurityUser.java index d651a31f4e5..b5bcac29973 100755 --- a/java/com.sap.sse.security.common/src/com/sap/sse/security/shared/SecurityUser.java +++ b/java/com.sap.sse.security.common/src/com/sap/sse/security/shared/SecurityUser.java @@ -15,8 +15,33 @@ import com.sap.sse.common.WithID; public interface SecurityUser extends NamedWithID { Tenant getDefaultTenant(); + /** + * Checks whether this user has the {@code permission} requested. For this, the {@link #getRoles() roles} + * and {@link #getPermissions() permissions} are checked; however, since this method does not accept + * {@link Ownership} or {@link AccessControlList} parameters, no further inferences are made. + */ boolean hasPermission(WildcardPermission permission); + /** + * Checks whether this user has the {@code permission} requested. For this, the {@link #getRoles() roles} and + * {@link #getPermissions() permissions} are checked, furthermore if this user is the + * {@link Ownership#getUserOwner() user owner} as per the {@code ownership} information, the permission will be + * granted because users have all rights to the objects they own. Furthermore, tenant and user parameterized roles + * will be applied based on the {@code ownership} information. No {@link AccessControlList} rules are applied here. + */ + boolean hasPermission(WildcardPermission permission, Ownership ownership); + + /** + * Checks whether this user has the {@code permission} requested. For this, the {@link #getRoles() roles} and + * {@link #getPermissions() permissions} are checked, furthermore if this user is the + * {@link Ownership#getUserOwner() user owner} as per the {@code ownership} information, the permission will be + * granted because users have all rights to the objects they own. Furthermore, tenant and user parameterized roles + * will be applied based on the {@code ownership} information. If the user belongs to one or more groups + * ({@code groupsThisUserIsPartOf}) and a non-{@code null} {@code acl} is provided, the access control list + * permissions are applied accordingly. + */ + boolean hasPermission(WildcardPermission permission, Ownership ownership, Iterable groupsThisUserIsPartOf, AccessControlList acl); + /** * Returns the "raw" permissions explicitly set for this user. This does not include permissions * inferred by any {@link PermissionsForRoleProvider} for the {@link #getRoles() roles} that this diff --git a/java/com.sap.sse.security.common/src/com/sap/sse/security/shared/impl/SecurityUserImpl.java b/java/com.sap.sse.security.common/src/com/sap/sse/security/shared/impl/SecurityUserImpl.java index afc250e3400..607d5b5c374 100755 --- a/java/com.sap.sse.security.common/src/com/sap/sse/security/shared/impl/SecurityUserImpl.java +++ b/java/com.sap.sse.security.common/src/com/sap/sse/security/shared/impl/SecurityUserImpl.java @@ -5,9 +5,13 @@ import java.util.HashSet; import java.util.Set; import com.sap.sse.common.Util; +import com.sap.sse.security.shared.AccessControlList; +import com.sap.sse.security.shared.Ownership; +import com.sap.sse.security.shared.PermissionChecker; import com.sap.sse.security.shared.Role; import com.sap.sse.security.shared.SecurityUser; import com.sap.sse.security.shared.Tenant; +import com.sap.sse.security.shared.UserGroup; import com.sap.sse.security.shared.WildcardPermission; public class SecurityUserImpl implements SecurityUser { @@ -76,9 +80,20 @@ public class SecurityUserImpl implements SecurityUser { @Override public boolean hasPermission(WildcardPermission permission) { - return permissions.contains(permission); + return hasPermission(permission, /* ownership */ null); } + @Override + public boolean hasPermission(WildcardPermission permission, Ownership ownership) { + return hasPermission(permission, ownership, /* user groups */ null, /* ACL */ null); + } + + @Override + public boolean hasPermission(WildcardPermission permission, Ownership ownership, + Iterable groupsThisUserIsPartOf, AccessControlList acl) { + return PermissionChecker.isPermitted(permission, this, groupsThisUserIsPartOf, getRoles(), ownership, acl); + } + public void addRole(Role role) { roles.add(role); } diff --git a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/authentication/app/AuthenticationContextImpl.java b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/authentication/app/AuthenticationContextImpl.java index f1772e3270c..c733bedc140 100644 --- a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/authentication/app/AuthenticationContextImpl.java +++ b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/authentication/app/AuthenticationContextImpl.java @@ -14,7 +14,7 @@ public class AuthenticationContextImpl implements AuthenticationContext { private final UserDTO currentUser; private final static UserDTO ANONYMOUS = new UserDTO("Anonymous", "", "", "", null, false, new ArrayList(), - new ArrayList(), /* default tenant */ null, new ArrayList()); + new ArrayList(), /* default tenant */ null, new ArrayList(), /* groups */ null); /** * Creating an {@link AuthenticationContextImpl} containing an anonymous {@link UserDTO} object. diff --git a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/client/component/UserList.java b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/client/component/UserList.java index 8c76f16684c..7023fa77147 100644 --- a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/client/component/UserList.java +++ b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/client/component/UserList.java @@ -5,6 +5,7 @@ import com.google.gwt.resources.client.ImageResource; import com.google.gwt.safehtml.shared.SafeHtmlBuilder; import com.google.gwt.user.cellview.client.CellList; import com.google.gwt.user.client.ui.ImageResourceRenderer; +import com.sap.sse.common.Util; import com.sap.sse.security.ui.client.IconResources; import com.sap.sse.security.ui.shared.UserDTO; @@ -28,7 +29,7 @@ public class UserList extends CellList { sb.appendHtmlConstant(""); sb.appendHtmlConstant(""); sb.appendHtmlConstant("
"); - sb.appendEscaped(value.getName()); + sb.appendEscaped(value.getName()+" ("+Util.join(", ", value.getUserGroups())+")"); sb.appendHtmlConstant("
"); sb.appendHtmlConstant(""); sb.appendHtmlConstant(""); 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 0d37eed21c8..ddc4c4fee94 100755 --- 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 @@ -55,6 +55,7 @@ import com.sap.sse.security.shared.UsernamePasswordAccount; import com.sap.sse.security.shared.WildcardPermission; import com.sap.sse.security.shared.impl.SecurityUserImpl; import com.sap.sse.security.shared.impl.TenantImpl; +import com.sap.sse.security.shared.impl.UserGroupImpl; import com.sap.sse.security.ui.client.UserManagementService; import com.sap.sse.security.ui.oauth.client.CredentialDTO; import com.sap.sse.security.ui.oauth.client.SocialUserDTO; @@ -100,13 +101,14 @@ public class UserManagementServiceImpl extends RemoteServiceServlet implements U }.start(); } - private SecurityUser createUserDTOFromUser(SecurityUser user, Map fromOriginalToStrippedDownTenant, Map fromOriginalToStrippedDownUser) { + private SecurityUser createUserDTOFromUser(SecurityUser user, Map fromOriginalToStrippedDownTenant, Map fromOriginalToStrippedDownUser, + Map fromOriginalToStrippedDownUserGroup) { SecurityUser result = fromOriginalToStrippedDownUser.get(user); if (result == null) { final SecurityUserImpl preResult = new SecurityUserImpl(user.getName(), /* default tenant to be set later: */ null); result = preResult; fromOriginalToStrippedDownUser.put(user, result); - preResult.setDefaultTenant(createTenantDTOFromTenant(user.getDefaultTenant(), fromOriginalToStrippedDownTenant, fromOriginalToStrippedDownUser)); + preResult.setDefaultTenant(createTenantDTOFromTenant(user.getDefaultTenant(), fromOriginalToStrippedDownTenant, fromOriginalToStrippedDownUser, fromOriginalToStrippedDownUserGroup)); } return result; } @@ -125,10 +127,12 @@ public class UserManagementServiceImpl extends RemoteServiceServlet implements U * as will for tenants. */ private Tenant createTenantDTOFromTenant(Tenant tenant) { - return createTenantDTOFromTenant(tenant, new HashMap<>(), new HashMap<>()); + return createTenantDTOFromTenant(tenant, new HashMap<>(), new HashMap<>(), new HashMap<>()); } - private Tenant createTenantDTOFromTenant(Tenant tenant, Map fromOriginalToStrippedDownTenant, Map fromOriginalToStrippedDownUser) { + private Tenant createTenantDTOFromTenant(Tenant tenant, Map fromOriginalToStrippedDownTenant, + Map fromOriginalToStrippedDownUser, + Map fromOriginalToStrippedDownUserGroup) { final Tenant result; if (tenant == null) { result = null; @@ -138,7 +142,8 @@ public class UserManagementServiceImpl extends RemoteServiceServlet implements U result = new TenantImpl(tenant.getId(), tenant.getName()); fromOriginalToStrippedDownTenant.put(tenant, result); for (final SecurityUser user : tenant.getUsers()) { - result.add(createUserDTOFromUser(user, fromOriginalToStrippedDownTenant, fromOriginalToStrippedDownUser)); + result.add(createUserDTOFromUser(user, fromOriginalToStrippedDownTenant, fromOriginalToStrippedDownUser, + fromOriginalToStrippedDownUserGroup)); } } return result; @@ -261,10 +266,11 @@ public class UserManagementServiceImpl extends RemoteServiceServlet implements U if (SecurityUtils.getSubject().isPermitted("tenant:delete:" + tenantIdAsString)) { try { UUID tenantId = UUID.fromString(tenantIdAsString); - getSecurityService().deleteTenant(getSecurityService().getTenant(tenantId)); + final Tenant tenant = getSecurityService().getTenant(tenantId); + getSecurityService().deleteTenant(tenant); getSecurityService().deleteACL(tenantIdAsString); getSecurityService().deleteOwnership(tenantIdAsString); - return new SuccessInfo(true, "Deleted tenant: " + tenantIdAsString + ".", /* redirectURL */ null, null); + return new SuccessInfo(true, "Deleted tenant: " + tenant.getName() + ".", /* redirectURL */ null, null); } catch (UserGroupManagementException e) { return new SuccessInfo(false, "Could not delete tenant.", /* redirectURL */ null, null); } @@ -518,10 +524,11 @@ public class UserManagementServiceImpl extends RemoteServiceServlet implements U } private UserDTO createUserDTOFromUser(User user) { - return createUserDTOFromUser(user, new HashMap<>(), new HashMap<>()); + return createUserDTOFromUser(user, new HashMap<>(), new HashMap<>(), new HashMap<>()); } - private UserDTO createUserDTOFromUser(User user, Map fromOriginalToStrippedDownTenant, Map fromOriginalToStrippedDownUser) { + private UserDTO createUserDTOFromUser(User user, Map fromOriginalToStrippedDownTenant, Map fromOriginalToStrippedDownUser, + Map fromOriginalToStrippedDownUserGroup) { UserDTO userDTO; Map accounts = user.getAllAccounts(); List accountDTOs = new ArrayList<>(); @@ -544,12 +551,50 @@ public class UserManagementServiceImpl extends RemoteServiceServlet implements U userDTO = new UserDTO(user.getName(), user.getEmail(), user.getFullName(), user.getCompany(), user.getLocale() != null ? user.getLocale().toLanguageTag() : null, user.isEmailValidated(), accountDTOs, user.getRoles(), /* default tenant filled in later */ null, - user.getPermissions()); + user.getPermissions(), + createUserGroupDTOsFromUserGroups(getSecurityService().getUserGroupsOfUser(user), + fromOriginalToStrippedDownTenant, fromOriginalToStrippedDownUser, + fromOriginalToStrippedDownUserGroup)); fromOriginalToStrippedDownUser.put(user, userDTO); - userDTO.setDefaultTenant(createTenantDTOFromTenant(user.getDefaultTenant(), fromOriginalToStrippedDownTenant, fromOriginalToStrippedDownUser)); + userDTO.setDefaultTenant(createTenantDTOFromTenant(user.getDefaultTenant(), fromOriginalToStrippedDownTenant, + fromOriginalToStrippedDownUser, fromOriginalToStrippedDownUserGroup)); return userDTO; } + private Iterable createUserGroupDTOsFromUserGroups(Iterable userGroups, + Map fromOriginalToStrippedDownTenant, + Map fromOriginalToStrippedDownUser, + Map fromOriginalToStrippedDownUserGroup) { + final List result; + if (userGroups == null) { + result = null; + } else { + result = new ArrayList<>(); + for (final UserGroup userGroup : userGroups) { + result.add(createUserDTOFromUserGroup(userGroup, fromOriginalToStrippedDownTenant, + fromOriginalToStrippedDownUser, fromOriginalToStrippedDownUserGroup)); + } + } + return result; + } + + private UserGroup createUserDTOFromUserGroup(UserGroup userGroup, + Map fromOriginalToStrippedDownTenant, + Map fromOriginalToStrippedDownUser, + Map fromOriginalToStrippedDownUserGroup) { + final UserGroup result; + if (fromOriginalToStrippedDownUserGroup.containsKey(userGroup)) { + result = fromOriginalToStrippedDownUserGroup.get(userGroup); + } else { + result = new UserGroupImpl(userGroup.getId(), userGroup.getName()); + fromOriginalToStrippedDownUserGroup.put(userGroup, result); + for (final SecurityUser user : userGroup.getUsers()) { + result.add(createUserDTOFromUser(user, fromOriginalToStrippedDownTenant, fromOriginalToStrippedDownUser, fromOriginalToStrippedDownUserGroup)); + } + } + return result; + } + @Override public Map getSettings() { Map settings = new TreeMap(); diff --git a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/shared/UserDTO.java b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/shared/UserDTO.java index 33cbf96ae1e..8a879f85977 100644 --- a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/shared/UserDTO.java +++ b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/shared/UserDTO.java @@ -10,7 +10,6 @@ import com.google.gwt.user.client.rpc.IsSerializable; import com.sap.sse.common.Util; import com.sap.sse.security.shared.AccessControlList; import com.sap.sse.security.shared.Ownership; -import com.sap.sse.security.shared.PermissionChecker; import com.sap.sse.security.shared.Role; import com.sap.sse.security.shared.Tenant; import com.sap.sse.security.shared.UserGroup; @@ -26,6 +25,7 @@ public class UserDTO extends SecurityUserImpl implements IsSerializable { private String locale; private List accounts; private boolean emailValidated; + private List groups; // for GWT serialization only @Deprecated @@ -33,8 +33,12 @@ public class UserDTO extends SecurityUserImpl implements IsSerializable { super(); } + /** + * @param groups may be {@code null} which is equivalent to passing an empty groups collection + */ public UserDTO(String name, String email, String fullName, String company, String locale, boolean emailValidated, - List accounts, Iterable roles, Tenant defaultTenant, Iterable permissions) { + List accounts, Iterable roles, Tenant defaultTenant, Iterable permissions, + Iterable groups) { super(name, roles, defaultTenant, permissions); this.email = email; this.fullName = fullName; @@ -42,6 +46,8 @@ public class UserDTO extends SecurityUserImpl implements IsSerializable { this.locale = locale; this.emailValidated = emailValidated; this.accounts = accounts; + this.groups = new ArrayList<>(); + Util.addAll(groups, this.groups); } public String getFullName() { @@ -105,20 +111,16 @@ public class UserDTO extends SecurityUserImpl implements IsSerializable { return result; } + public List getUserGroups() { + return groups; + } + public boolean hasPermission(String permission) { return hasPermission(new WildcardPermission(permission)); } - - public boolean hasPermission(String permission, AccessControlList acl, Ownership ownership) { - return hasPermission(new WildcardPermission(permission), acl, ownership); - } - - public boolean hasPermission(WildcardPermission permission, AccessControlList acl, Ownership ownership) { - ArrayList groupsTheUserBelongsTo = new ArrayList<>(); - if (acl != null) { - groupsTheUserBelongsTo = new ArrayList<>(acl.getActionsByUserGroup().keySet()); - } - return PermissionChecker.isPermitted(permission, this, groupsTheUserBelongsTo, getRoles(), ownership, acl); + + public boolean hasPermission(WildcardPermission permission, Ownership ownership, AccessControlList acl) { + return hasPermission(permission, ownership, getUserGroups(), acl); } public List getAccounts() { diff --git a/java/com.sap.sse.security.userstore.mongodb/src/com/sap/sse/security/userstore/mongodb/UserStoreImpl.java b/java/com.sap.sse.security.userstore.mongodb/src/com/sap/sse/security/userstore/mongodb/UserStoreImpl.java index 9bbe7a33764..f1f58219cc4 100644 --- a/java/com.sap.sse.security.userstore.mongodb/src/com/sap/sse/security/userstore/mongodb/UserStoreImpl.java +++ b/java/com.sap.sse.security.userstore.mongodb/src/com/sap/sse/security/userstore/mongodb/UserStoreImpl.java @@ -72,7 +72,9 @@ public class UserStoreImpl implements UserStore { /** * Protects access to the two maps {@link #userGroupsContainingUser} and {@link #usersInUserGroups} which implement - * an efficient lookup for the m:n association between {@link UserGroup#getUsers()} and {@link SecurityUser}. + * an efficient lookup for the m:n association between {@link UserGroup#getUsers()} and {@link SecurityUser}. The + * collections also contain the relationships for the specialized {@link Tenant} objects which are not part of + * {@link #userGroups} but of {@link #tenants}. */ private final NamedReentrantReadWriteLock userGroupsUserCacheLock = new NamedReentrantReadWriteLock("User Groups Cache", /* fair */ false); private final ConcurrentHashMap> userGroupsContainingUser; diff --git a/java/com.sap.sse.security.userstore.mongodb/src/com/sap/sse/security/userstore/mongodb/impl/DomainObjectFactoryImpl.java b/java/com.sap.sse.security.userstore.mongodb/src/com/sap/sse/security/userstore/mongodb/impl/DomainObjectFactoryImpl.java index 4fff1e27d92..403b7ec9d3e 100644 --- a/java/com.sap.sse.security.userstore.mongodb/src/com/sap/sse/security/userstore/mongodb/impl/DomainObjectFactoryImpl.java +++ b/java/com.sap.sse.security.userstore.mongodb/src/com/sap/sse/security/userstore/mongodb/impl/DomainObjectFactoryImpl.java @@ -170,7 +170,10 @@ public class DomainObjectFactoryImpl implements DomainObjectFactory { BasicDBList usersO = (BasicDBList) groupDBObject.get(FieldNames.UserGroup.USERNAMES.name()); if (usersO != null) { for (Object o : usersO) { - users.add(usersByName.get((String) o)); + final UserImpl user = usersByName.get((String) o); + if (user != null) { + users.add(user); + } } } UserGroup result = new UserGroupImpl(id, name, users); 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 d98184ace71..183b8425bf6 100755 --- 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 @@ -104,6 +104,8 @@ public interface SecurityService extends ReplicableWithObjectInputStream getUserGroupsOfUser(SecurityUser user); Iterable getTenants(); 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 f73e705b8cd..ff789a7a836 100755 --- 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 @@ -490,6 +490,11 @@ public class SecurityServiceImpl implements ReplicableSecurityService, ClearStat return userStore.getUserGroupByName(name); } + @Override + public Iterable getUserGroupsOfUser(SecurityUser user) { + return userStore.getUserGroupsOfUser(user); + } + @Override public Iterable getTenants() { return userStore.getTenants(); @@ -552,7 +557,8 @@ public class SecurityServiceImpl implements ReplicableSecurityService, ClearStat @Override public void deleteTenant(Tenant tenant) throws TenantManagementException, UserGroupManagementException { for (Ownership ownership : accessControlStore.getOwnerships()) { - if (ownership.getTenantOwner().equals(tenant)) { + if (!Util.equalsWithNull(ownership.getIdOfOwnedObjectAsString(), tenant.getId().toString()) && + Util.equalsWithNull(ownership.getTenantOwner(), tenant)) { throw new TenantManagementException("The tenant "+tenant.getName()+ " is still used as tenant owner and therefore cannot be removed"); } @@ -620,7 +626,12 @@ public class SecurityServiceImpl implements ReplicableSecurityService, ClearStat @Override public Void internalDeleteUserGroup(UUID groupId) throws UserGroupManagementException { - userStore.deleteUserGroup(getUserGroup(groupId)); + final UserGroup userGroup = getUserGroup(groupId); + if (userGroup == null) { + logger.warning("Strange: the user group with ID "+groupId+" which is about to be deleted couldn't be found"); + } else { + userStore.deleteUserGroup(userGroup); + } return null; }