diff --git a/java/com.sap.sse.gwt.adminconsole/src/com/sap/sse/gwt/adminconsole/AdminConsolePanel.java b/java/com.sap.sse.gwt.adminconsole/src/com/sap/sse/gwt/adminconsole/AdminConsolePanel.java index bbcf9630c45..76cbe7ef467 100755 --- a/java/com.sap.sse.gwt.adminconsole/src/com/sap/sse/gwt/adminconsole/AdminConsolePanel.java +++ b/java/com.sap.sse.gwt.adminconsole/src/com/sap/sse/gwt/adminconsole/AdminConsolePanel.java @@ -107,7 +107,7 @@ public class AdminConsolePanel extends HeaderPanel implements HandleTabSelectabl * admin console and its panels. */ private final Iterable acceptablePermissionsRequiredToSeeWidgets; - + /** * Generic selection handler that forwards selected tabs to a refresher that ensures that data gets reloaded. If * you add a new tab then make sure to have a look at #refreshDataFor(Widget widget) to ensure that upon @@ -160,11 +160,12 @@ public class AdminConsolePanel extends HeaderPanel implements HandleTabSelectabl return target; } - public AdminConsolePanel(UserService userService, - ServerInfoDTO serverInfo, String releaseNotesAnchorLabel, - String releaseNotesURL, ErrorReporter errorReporter, LoginPanelCss loginPanelCss, StringMessages stringMessages) { - this(userService, serverInfo, releaseNotesAnchorLabel, releaseNotesURL, errorReporter, loginPanelCss, - stringMessages, /* acceptablePermissionsRequiredToSeeWidgets==null means accept any permission */ null); + public AdminConsolePanel(UserService userService, ServerInfoDTO serverInfo, + String releaseNotesAnchorLabel, String releaseNotesURL, ErrorReporter errorReporter, + LoginPanelCss loginPanelCss, StringMessages stringMessages) { + this(userService, serverInfo, releaseNotesAnchorLabel, releaseNotesURL, errorReporter, + loginPanelCss, stringMessages, + /* acceptablePermissionsRequiredToSeeWidgets==null means accept any permission */ null); } public AdminConsolePanel(UserService userService, @@ -454,7 +455,7 @@ public class AdminConsolePanel extends HeaderPanel implements HandleTabSelectabl widgetForSetup = currentWidget; } } - panelsByWidget.get(unwrapScrollPanel(widgetForSetup)).setupWidgetByParams(params);; + panelsByWidget.get(unwrapScrollPanel(widgetForSetup)).setupWidgetByParams(params); } /** @@ -477,7 +478,9 @@ public class AdminConsolePanel extends HeaderPanel implements HandleTabSelectabl hasPermission = false; for (WildcardPermission requiredPermission : permissionsRequired) { // TODO bug4763: obtain ownership and ACL through a provider pattern; providers may be passed to this panel's constructor - if (PermissionChecker.isPermitted(requiredPermission, user, user.getUserGroups(), /* ownership */ null, /* acl */ null)) { + UserDTO anonymous = userService.getAnonymousUser(); + if (PermissionChecker.isPermitted(requiredPermission, user, user.getUserGroups(), anonymous, + anonymous.getUserGroups(), null, null)) { hasPermission = true; break; } 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 e0ec27cdf74..9fa2420f0f1 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 @@ -35,8 +35,9 @@ public class PermissionChecker { * @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, - Ownership ownership, AccessControlList acl) { + public static boolean isPermitted(WildcardPermission permission, SecurityUser user, + Iterable groupsOfWhichUserIsMember, SecurityUser allUser, + Iterable allUserGroupsOfWhichUserIsMember, Ownership ownership, AccessControlList acl) { List> parts = permission.getParts(); // permission has at least data object type and action as parts // and data object part only has one sub-part @@ -59,6 +60,22 @@ public class PermissionChecker { } result = acl.hasPermission(action, groupsOfWhichUserIsMember); } + + // anonymous can only grant it if not already decided by acl + if (result == PermissionState.NONE) { + PermissionState anonymous = checkUserPermissions(permission, allUser, ownership, result); + if (anonymous == PermissionState.GRANTED) { + result = anonymous; + } + } + if (result == PermissionState.NONE) { + result = checkUserPermissions(permission, allUser, ownership, result); + } + return result == PermissionState.GRANTED; + } + + private static PermissionState checkUserPermissions(WildcardPermission permission, SecurityUser user, + Ownership ownership, PermissionState result) { // 2. check direct permissions if (result == PermissionState.NONE && user != null) { // no direct permissions for anonymous users for (WildcardPermission directPermission : user.getPermissions()) { @@ -77,7 +94,7 @@ public class PermissionChecker { } } } - return result == PermissionState.GRANTED; + return result; } /** 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 62062564a88..b563d0ad2a1 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 @@ -18,33 +18,6 @@ public interface SecurityUser extends NamedWithID { */ UserGroup 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 f95c221dbca..9709891de90 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 @@ -11,9 +11,7 @@ import java.util.Set; import com.sap.sse.common.Util; import com.sap.sse.common.settings.GwtIncompatible; -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.UserGroup; @@ -110,22 +108,6 @@ public class SecurityUserImpl implements SecurityUser { return permissions; } - @Override - public boolean hasPermission(WildcardPermission permission) { - return hasPermission(permission, /* ownership */ null); - } - - @Override - public boolean hasPermission(WildcardPermission permission, Ownership ownership) { - return hasPermission(permission, ownership, getUserGroups(), /* ACL */ null); - } - - @Override - public boolean hasPermission(WildcardPermission permission, Ownership ownership, - Iterable groupsThisUserIsPartOf, AccessControlList acl) { - return PermissionChecker.isPermitted(permission, this, groupsThisUserIsPartOf, ownership, acl); - } - public void addRole(Role role) { roles.add(role); } diff --git a/java/com.sap.sse.security.test/src/com/sap/sse/security/test/LoginTest.java b/java/com.sap.sse.security.test/src/com/sap/sse/security/test/LoginTest.java index 67b584905d0..68015872160 100755 --- a/java/com.sap.sse.security.test/src/com/sap/sse/security/test/LoginTest.java +++ b/java/com.sap.sse.security.test/src/com/sap/sse/security/test/LoginTest.java @@ -20,8 +20,10 @@ import com.sap.sse.common.Util; import com.sap.sse.mongodb.MongoDBConfiguration; import com.sap.sse.mongodb.MongoDBService; import com.sap.sse.security.AccessControlStore; +import com.sap.sse.security.SecurityService; import com.sap.sse.security.impl.Activator; import com.sap.sse.security.impl.SecurityServiceImpl; +import com.sap.sse.security.shared.PermissionChecker; import com.sap.sse.security.shared.Role; import com.sap.sse.security.shared.RoleDefinition; import com.sap.sse.security.shared.RoleImpl; @@ -107,7 +109,10 @@ public class LoginTest { userStore.createUser("me", "me@sap.com", new UserGroupImpl(UUID.randomUUID(), "me-tenant")); userStore.addPermissionForUser("me", new WildcardPermission("a:b:c")); UserStoreImpl store2 = new UserStoreImpl(DEFAULT_TENANT_NAME); - assertTrue(store2.getUserByName("me").hasPermission(new WildcardPermission("a:b:c"))); + User allUser = userStore.getUserByName(SecurityService.ALL_USERNAME); + User user = store2.getUserByName("me"); + assertTrue(PermissionChecker.isPermitted(new WildcardPermission("a:b:c"), user, user.getUserGroups(), allUser, + allUser.getUserGroups(), null, null)); } } diff --git a/java/com.sap.sse.security.test/src/com/sap/sse/security/test/PermissionCheckerTest.java b/java/com.sap.sse.security.test/src/com/sap/sse/security/test/PermissionCheckerTest.java index e509637ae91..8555b97b828 100755 --- a/java/com.sap.sse.security.test/src/com/sap/sse/security/test/PermissionCheckerTest.java +++ b/java/com.sap.sse.security.test/src/com/sap/sse/security/test/PermissionCheckerTest.java @@ -17,6 +17,7 @@ import org.junit.Test; import com.sap.sailing.domain.common.security.SecuredDomainType; import com.sap.sse.security.AbstractCompositeAuthorizingRealm; import com.sap.sse.security.AccessControlStore; +import com.sap.sse.security.SecurityService; import com.sap.sse.security.UserStore; import com.sap.sse.security.UsernamePasswordRealm; import com.sap.sse.security.shared.AccessControlList; @@ -46,6 +47,7 @@ public class PermissionCheckerTest { private SecurityUser adminUser; private UserGroup userTenant; private User user; + private User allUser; private ArrayList tenants; private Ownership ownership; private Ownership adminOwnership; @@ -61,6 +63,7 @@ public class PermissionCheckerTest { public void setUp() throws UserGroupManagementException, UserManagementException { final String adminTenantName = "admin-tenant"; userStore = new UserStoreImpl(adminTenantName); + allUser = userStore.getUserByName(SecurityService.ALL_USERNAME); accessControlStore = new AccessControlStoreImpl(userStore); AbstractCompositeAuthorizingRealm.setTestStores(userStore, accessControlStore); realm = new UsernamePasswordRealm(); @@ -88,13 +91,17 @@ public class PermissionCheckerTest { @Test public void testOwnership() throws UserManagementException { - assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, null, acl)); - assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, adminOwnership, acl)); + assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + null, acl)); + assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + adminOwnership, acl)); // being the owning user does not imply any permissions per se - assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, ownership, acl)); + assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + ownership, acl)); userStore.addRoleForUser(user.getName(), new RoleImpl(AdminRole.getInstance(), /* qualified for userTenant */ null, /* qualified for user */ user)); // having the admin role qualified for objects owned by user should help - assertTrue(PermissionChecker.isPermitted(eventReadPermission, user, tenants, ownership, acl)); + assertTrue(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + ownership, acl)); } /** @@ -139,46 +146,62 @@ public class PermissionCheckerTest { @Test public void testAccessControlList() { - assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, adminOwnership, null)); - assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, adminOwnership, acl)); + assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + adminOwnership, null)); + assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + adminOwnership, acl)); acl.addPermission(userTenant, DefaultActions.READ.name()); - assertTrue(PermissionChecker.isPermitted(eventReadPermission, user, tenants, adminOwnership, acl)); + assertTrue(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + adminOwnership, acl)); // ensure that anonymous users don't have access because they don't belong to any group - assertFalse(PermissionChecker.isPermitted(eventReadPermission, /* user */ null, /* groups */ new HashSet<>(), adminOwnership, acl)); + assertFalse(PermissionChecker.isPermitted(eventReadPermission, /* user */ null, /* groups */ new HashSet<>(), + allUser, allUser.getUserGroups(), adminOwnership, acl)); user.addPermission(eventReadPermission); - assertTrue(PermissionChecker.isPermitted(eventReadPermission, user, tenants, adminOwnership, acl)); + assertTrue(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + adminOwnership, acl)); final Set permissionSet = new HashSet<>(); permissionSet.add("!" + DefaultActions.READ.name()); acl.setPermissions(userTenant, permissionSet); - assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, adminOwnership, acl)); + assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + adminOwnership, acl)); // User ownership shall NOT imply permissions; the revoking ACL still takes precedence - assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, ownership, acl)); + assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + ownership, acl)); // now add "public read" permission to ACL: acl.addPermission(null, DefaultActions.READ.name()); - assertTrue(PermissionChecker.isPermitted(eventReadPermission, /* user */ null, /* groups */ new HashSet<>(), adminOwnership, acl)); + assertTrue(PermissionChecker.isPermitted(eventReadPermission, /* user */ null, /* groups */ new HashSet<>(), + allUser, allUser.getUserGroups(), adminOwnership, acl)); // now deny "public read" permission in ACL which is expected to supersede the granting from above: acl.denyPermission(null, DefaultActions.READ.name()); - assertFalse(PermissionChecker.isPermitted(eventReadPermission, /* user */ null, /* groups */ new HashSet<>(), adminOwnership, acl)); + assertFalse(PermissionChecker.isPermitted(eventReadPermission, /* user */ null, /* groups */ new HashSet<>(), + allUser, allUser.getUserGroups(), adminOwnership, acl)); } @Test public void testDirectPermission() { - assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, adminOwnership, acl)); + assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + adminOwnership, acl)); user.addPermission(eventReadPermission); - assertTrue(PermissionChecker.isPermitted(eventReadPermission, user, tenants, adminOwnership, acl)); + assertTrue(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + adminOwnership, acl)); } @Test public void testRole() { - assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, adminOwnership, acl)); + assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + adminOwnership, acl)); final RoleImpl globalRole = new RoleImpl(globalRoleDefinition); user.addRole(globalRole); - assertTrue(PermissionChecker.isPermitted(eventReadPermission, user, tenants, adminOwnership, acl)); + assertTrue(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + adminOwnership, acl)); user.removeRole(globalRole); user.addRole(new RoleImpl(globalRoleDefinition, this.userTenant, /* user qualifier */ null)); - assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, adminOwnership, acl)); + assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + adminOwnership, acl)); Ownership testOwnership = new OwnershipImpl(adminUser, userTenant); - assertTrue(PermissionChecker.isPermitted(eventReadPermission, user, tenants, testOwnership, acl)); - assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, null, acl)); + assertTrue(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + testOwnership, acl)); + assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, allUser, allUser.getUserGroups(), + null, acl)); } } \ No newline at end of file diff --git a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/client/UserService.java b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/client/UserService.java index 569a767abe4..56b85c47b29 100755 --- a/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/client/UserService.java +++ b/java/com.sap.sse.security.ui/src/main/java/com/sap/sse/security/ui/client/UserService.java @@ -25,6 +25,7 @@ import com.sap.sse.security.shared.AccessControlList; import com.sap.sse.security.shared.HasPermissions; import com.sap.sse.security.shared.HasPermissions.DefaultActions; import com.sap.sse.security.shared.Ownership; +import com.sap.sse.security.shared.PermissionChecker; import com.sap.sse.security.shared.WildcardPermission; import com.sap.sse.security.shared.impl.OwnershipImpl; import com.sap.sse.security.ui.client.i18n.StringMessages; @@ -382,10 +383,8 @@ public class UserService { if (anonymousUser == null) { return false; } - if (anonymousUser.hasPermission(permission, ownership, acl)) { - return true; - } - return currentUser != null && currentUser.hasPermission(permission, ownership, acl); + return PermissionChecker.isPermitted(permission, currentUser, currentUser.getUserGroups(), anonymousUser, + anonymousUser.getUserGroups(), ownership, acl); } /** @@ -400,4 +399,8 @@ public class UserService { return hasPermission(logicalSecuredObjectType.getPermission(DefaultActions.CREATE), new OwnershipImpl(currentUser, currentUser.getDefaultTenant())); } + + public UserDTO getAnonymousUser() { + return anonymousUser; + } } 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 26716a4f3aa..c659daf6988 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 @@ -5,8 +5,6 @@ import java.util.List; 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.Role; import com.sap.sse.security.shared.UserGroup; import com.sap.sse.security.shared.WildcardPermission; @@ -82,10 +80,6 @@ public class UserDTO extends SecurityUserImpl implements IsSerializable { return groups; } - public boolean hasPermission(WildcardPermission permission, Ownership ownership, AccessControlList acl) { - return hasPermission(permission, ownership, getUserGroups(), acl); - } - public List getAccounts() { return accounts; } diff --git a/java/com.sap.sse.security/src/com/sap/sse/security/AbstractCompositeAuthorizingRealm.java b/java/com.sap.sse.security/src/com/sap/sse/security/AbstractCompositeAuthorizingRealm.java index 33fbd706529..3f327ac3c52 100755 --- a/java/com.sap.sse.security/src/com/sap/sse/security/AbstractCompositeAuthorizingRealm.java +++ b/java/com.sap.sse.security/src/com/sap/sse/security/AbstractCompositeAuthorizingRealm.java @@ -194,10 +194,12 @@ public abstract class AbstractCompositeAuthorizingRealm extends AuthorizingRealm getUserStore().getUserByName(SecurityService.ALL_USERNAME), ownership, acl); } - private boolean isPermittedForUser(WildcardPermission wildcardPermission, User user, OwnershipAnnotation ownership, AccessControlListAnnotation acl) { - return PermissionChecker.isPermitted(wildcardPermission, - user, getUserStore().getUserGroupsOfUser(user), ownership==null?null:ownership.getAnnotation(), - acl==null?null:acl.getAnnotation()); + private boolean isPermittedForUser(WildcardPermission wildcardPermission, User user, OwnershipAnnotation ownership, + AccessControlListAnnotation acl) { + User allUser = getUserStore().getUserByName(SecurityService.ALL_USERNAME); + return PermissionChecker.isPermitted(wildcardPermission, user, user.getUserGroups(), allUser, + allUser.getUserGroups(), ownership == null ? null : ownership.getAnnotation(), + acl == null ? null : acl.getAnnotation()); } @Override