Bug 5195: Several adjustments to ensure consistent usage of

RoleDefinitions for RolePrototypes in the security model to prevent
incompatible usages of objects that effectively represent the same
RoleDefinition.
This commit is contained in:
Steffen Schaefer
2020-02-11 13:04:30 +01:00
parent ac0552f98d
commit 341abf655f
6 changed files with 46 additions and 46 deletions
@@ -4,12 +4,13 @@ import java.util.HashSet;
import java.util.Set;
import java.util.UUID;
import com.sap.sse.common.NamedWithID;
import com.sap.sse.common.Util;
import com.sap.sse.security.shared.impl.SecuredSecurityTypes;
public abstract class RolePrototype implements RoleDefinition {
private static final long serialVersionUID = 3291793984984443193L;
public abstract class RolePrototype implements NamedWithID {
private static final long serialVersionUID = -3911998376131317304L;
private final UUID id;
private final String name;
private final Set<WildcardPermission> permissions;
@@ -46,17 +47,14 @@ public abstract class RolePrototype implements RoleDefinition {
return id;
}
@Override
public Set<WildcardPermission> getPermissions() {
return permissions;
}
@Override
public void setName(String newName) {
throw new UnsupportedOperationException("Cannot change the name of role " + getName());
}
@Override
public void setPermissions(Iterable<WildcardPermission> permissions) {
throw new UnsupportedOperationException("Cannot change the permissions of role " + getName());
}
@@ -85,22 +83,4 @@ public abstract class RolePrototype implements RoleDefinition {
return false;
return true;
}
@Override
public HasPermissions getPermissionType() {
return SecuredSecurityTypes.ROLE_DEFINITION;
}
@Override
public QualifiedObjectIdentifier getIdentifier() {
return getPermissionType().getQualifiedObjectIdentifier(getTypeRelativeObjectIdentifier());
}
public TypeRelativeObjectIdentifier getTypeRelativeObjectIdentifier() {
return getTypeRelativeObjectIdentifier(this);
}
public static TypeRelativeObjectIdentifier getTypeRelativeObjectIdentifier(RoleDefinition roleDefinition) {
return new TypeRelativeObjectIdentifier(roleDefinition.getIdAsString());
}
}
@@ -1,6 +1,8 @@
package com.sap.sse.security.interfaces;
import com.sap.sse.security.shared.BasicUserStore;
import com.sap.sse.security.shared.RoleDefinition;
import com.sap.sse.security.shared.RolePrototype;
import com.sap.sse.security.shared.UserGroupManagementException;
import com.sap.sse.security.shared.UserManagementException;
import com.sap.sse.security.shared.impl.Role;
@@ -104,4 +106,6 @@ public interface UserStore extends BasicUserStore {
void removeAllQualifiedRolesForUser(User user);
void removeAllQualifiedRolesForUserGroup(UserGroup userGroup);
RoleDefinition getRoleDefinitionByPrototype(RolePrototype rolePrototype);
}
@@ -109,7 +109,8 @@ public class PermissionCheckerTest {
assertFalse(PermissionChecker.isPermitted(eventReadPermission, user, tenants, null, null,
ownership, acl));
userStore.addRoleForUser(user.getName(),
new Role(AdminRole.getInstance(), /* qualified for userTenant */ null, /* qualified for user */ user));
new Role(userStore.getRoleDefinitionByPrototype(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, null, null,
ownership, acl));
@@ -145,7 +146,8 @@ public class PermissionCheckerTest {
/* tenantOwner */ null, regattaName);
// grant user the admin role, but only for objects owned by the user (leaderboard, but not regatta)
userStore.addRoleForUser(user.getName(),
new Role(AdminRole.getInstance(), /* qualifiedForTenant */ null, /* qualifiedForUser */ user));
new Role(userStore.getRoleDefinitionByPrototype(AdminRole.getInstance()), /* qualifiedForTenant */ null,
/* qualifiedForUser */ user));
assertTrue(realm.isPermitted(principalCollection, leaderboardPermission.toString()));
assertFalse(realm.isPermitted(principalCollection, regattaPermission.toString()));
accessControlStore.setOwnership(SecuredDomainType.REGATTA.getQualifiedObjectIdentifier(regattaIdentifier), /* userOwner */ null,
@@ -155,7 +157,8 @@ public class PermissionCheckerTest {
assertFalse(realm.isPermitted(principalCollection, regattaPermission.toString()));
// but now we assign the admin role to the user, qualified for objects owned by the group owner:
userStore.addRoleForUser(user.getName(),
new Role(AdminRole.getInstance(), /* qualifiedForTenant */ userTenant, /* qualifiedForUser */ null));
new Role(userStore.getRoleDefinitionByPrototype(AdminRole.getInstance()),
/* qualifiedForTenant */ userTenant, /* qualifiedForUser */ null));
assertTrue(realm.isPermitted(principalCollection, leaderboardPermission.toString()));
// now the user should be granted permission because admin gets *, and the user gets admin on all objects owned by userTenant
assertTrue(realm.isPermitted(principalCollection, regattaPermission.toString()));
@@ -2,6 +2,7 @@ package com.sap.sse.security.test;
import static org.junit.Assert.assertEquals;
import java.util.Collections;
import java.util.UUID;
import org.junit.Test;
@@ -10,7 +11,6 @@ import com.sap.sse.security.shared.HasPermissions.DefaultActions;
import com.sap.sse.security.shared.QualifiedObjectIdentifier;
import com.sap.sse.security.shared.RoleDefinition;
import com.sap.sse.security.shared.RoleDefinitionImpl;
import com.sap.sse.security.shared.RolePrototype;
import com.sap.sse.security.shared.TypeRelativeObjectIdentifier;
/**
@@ -39,8 +39,10 @@ public class RoleIdentifierTest {
@Test
public void testRolePrototypeIdentifier() {
final RoleDefinition roleDefinition = new RolePrototype("Test Role", UUID.randomUUID().toString()) {
final RoleDefinition roleDefinition = new RoleDefinitionImpl(UUID.randomUUID(), "Test Role",
Collections.emptySet()) {
private static final long serialVersionUID = 6504908715414284115L;
@Override
public String getIdAsString() {
return "abc'::\\\\://|\\?/\\///\\";
@@ -28,6 +28,7 @@ import com.sap.sse.security.shared.AdminRole;
import com.sap.sse.security.shared.PredefinedRoles;
import com.sap.sse.security.shared.RoleDefinition;
import com.sap.sse.security.shared.RoleDefinitionImpl;
import com.sap.sse.security.shared.RolePrototype;
import com.sap.sse.security.shared.SecurityUser;
import com.sap.sse.security.shared.UserGroupManagementException;
import com.sap.sse.security.shared.UserManagementException;
@@ -266,20 +267,8 @@ public class UserStoreImpl implements UserStore {
@Override
public void ensureDefaultRolesExist() {
final AdminRole adminRolePrototype = AdminRole.getInstance();
if (getRoleDefinition(adminRolePrototype.getId()) == null) {
logger.info("No admin role found. Creating default role \"" + adminRolePrototype.getName()
+ "\" with permission \"" + AdminRole.getInstance().getPermissions() + "\"");
createRoleDefinition((UUID) adminRolePrototype.getId(), adminRolePrototype.getName(),
adminRolePrototype.getPermissions());
}
final UserRole userRolePrototype = UserRole.getInstance();
if (getRoleDefinition(userRolePrototype.getId()) == null) {
logger.info("No user role found. Creating default role \"" + userRolePrototype.getName()
+ "\" with permission \"" + userRolePrototype.getPermissions() + "\"");
createRoleDefinition((UUID) userRolePrototype.getId(), userRolePrototype.getName(),
userRolePrototype.getPermissions());
}
getOrCreateRoleDefinitionByPrototype(AdminRole.getInstance());
getOrCreateRoleDefinitionByPrototype(UserRole.getInstance());
for (final PredefinedRoles otherPredefinedRole : PredefinedRoles.values()) {
if (getRoleDefinition(otherPredefinedRole.getId()) == null) {
logger.info("Predefined role definition " + otherPredefinedRole + " not found; creating");
@@ -291,6 +280,27 @@ public class UserStoreImpl implements UserStore {
}
}
}
private RoleDefinition getOrCreateRoleDefinitionByPrototype(RolePrototype rolePrototype) {
RoleDefinition roleDefinition = getRoleDefinition(rolePrototype.getId());
if (roleDefinition == null) {
logger.info("No " + rolePrototype.getName() + " role found. Creating default role \""
+ rolePrototype.getName() + "\" with permission \"" + rolePrototype.getPermissions() + "\"");
roleDefinition = createRoleDefinition(rolePrototype.getId(), rolePrototype.getName(), rolePrototype.getPermissions());
}
return roleDefinition;
}
public RoleDefinition getRoleDefinitionByPrototype(RolePrototype rolePrototype) {
final RoleDefinition roleDefinition = getRoleDefinition(rolePrototype.getId());
if (roleDefinition == null) {
final String errorMsg = "No " + rolePrototype.getName() + " role definition found by ID "
+ rolePrototype.getId() + "." + "RoleDefinitions for prototypes are required to exist on usage.";
logger.severe(errorMsg);
throw new IllegalStateException(errorMsg);
}
return roleDefinition;
}
@Override
public String getServerGroupName() {
@@ -863,12 +863,13 @@ public class SecurityServiceImpl implements ReplicableSecurityService, ClearStat
}
private void addUserRoleToUser(final User user) {
addRoleForUserAndSetUserAsOwner(user,
new Role(UserRole.getInstance(), /* tenant qualifier */ null, /* user qualifier */ user));
addRoleForUserAndSetUserAsOwner(user, new Role(store.getRoleDefinitionByPrototype(UserRole.getInstance()),
/* tenant qualifier */ null, /* user qualifier */ user));
}
private void addUserRoleForGroupToUser(final UserGroup group, final User user) {
addRoleForUserAndSetUserAsOwner(user, new Role(UserRole.getInstance(), /* tenant qualifier */ group, /* user qualifier */ null));
addRoleForUserAndSetUserAsOwner(user, new Role(store.getRoleDefinitionByPrototype(UserRole.getInstance()),
/* tenant qualifier */ group, /* user qualifier */ null));
}
private UserGroup getOrCreateTenantForUser(User user) throws UserGroupManagementException {