moved anonymous checks to PermissionChecker

This commit is contained in:
Kai Börnert
2018-10-15 11:37:17 +02:00
parent dcd809206c
commit 000ac30a95
9 changed files with 93 additions and 91 deletions
@@ -107,7 +107,7 @@ public class AdminConsolePanel extends HeaderPanel implements HandleTabSelectabl
* admin console and its panels.
*/
private final Iterable<? extends WildcardPermission> 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;
}
@@ -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<UserGroup> groupsOfWhichUserIsMember,
Ownership ownership, AccessControlList acl) {
public static boolean isPermitted(WildcardPermission permission, SecurityUser user,
Iterable<UserGroup> groupsOfWhichUserIsMember, SecurityUser allUser,
Iterable<UserGroup> allUserGroupsOfWhichUserIsMember, Ownership ownership, AccessControlList acl) {
List<Set<String>> 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;
}
/**
@@ -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<UserGroup> 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
@@ -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<UserGroup> groupsThisUserIsPartOf, AccessControlList acl) {
return PermissionChecker.isPermitted(permission, this, groupsThisUserIsPartOf, ownership, acl);
}
public void addRole(Role role) {
roles.add(role);
}
@@ -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));
}
}
@@ -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<UserGroup> 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<String> 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));
}
}
@@ -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;
}
}
@@ -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<AccountDTO> getAccounts() {
return accounts;
}
@@ -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