diff --git a/config/PrivilegeConfig.xml b/config/PrivilegeConfig.xml index 17fb4ad27..9dd3e2b11 100644 --- a/config/PrivilegeConfig.xml +++ b/config/PrivilegeConfig.xml @@ -6,6 +6,7 @@ + diff --git a/config/PrivilegeConfigMerge.xml b/config/PrivilegeConfigMerge.xml new file mode 100644 index 000000000..923189b53 --- /dev/null +++ b/config/PrivilegeConfigMerge.xml @@ -0,0 +1,33 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/config/PrivilegeModel.xml b/config/PrivilegeModel.xml index 104b28130..070e087d7 100644 --- a/config/PrivilegeModel.xml +++ b/config/PrivilegeModel.xml @@ -89,7 +89,17 @@ - + + + true + + + + + + true + + diff --git a/config/PrivilegeModelMerge.xml b/config/PrivilegeModelMerge.xml new file mode 100644 index 000000000..97fb7065d --- /dev/null +++ b/config/PrivilegeModelMerge.xml @@ -0,0 +1,54 @@ + + + + + + System User + Administrator + ENABLED + en_GB + + RoleA1 + RoleA2 + + + + System User + Administrator + ENABLED + en_GB + + RoleB1 + RoleB2 + + + + + + + + + allow1 + + + + + true + + + + + + allow1 + deny1 + + + + + allow2 + deny2 + + + + + \ No newline at end of file diff --git a/src/main/java/ch/eitchnet/privilege/base/PrivilegeConflictResolution.java b/src/main/java/ch/eitchnet/privilege/base/PrivilegeConflictResolution.java new file mode 100644 index 000000000..0d60fb28d --- /dev/null +++ b/src/main/java/ch/eitchnet/privilege/base/PrivilegeConflictResolution.java @@ -0,0 +1,52 @@ +/* + * Copyright 2013 Robert von Burg + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package ch.eitchnet.privilege.base; + +import ch.eitchnet.privilege.model.internal.Role; +import ch.eitchnet.privilege.model.internal.User; + +/** + * The {@link PrivilegeConflictResolution} defines what should be done if a {@link User} has {@link Role Roles} which + * have Privileges with conflicting names. + * + * @author Robert von Burg + */ +public enum PrivilegeConflictResolution { + + /** + * STRICT requires that a User may not have conflicting Privileges throug multiple Roles. In this case an Exception + * is thrown. + */ + STRICT { + @Override + public boolean isStrict() { + return true; + } + }, + + /** + * MERGE defines that if conflicting privileges are encountered then a merge is to take place. A merge means that if + * all is allowed, then that wins. Otherwise any existing allow and deny lists are merged + */ + MERGE { + @Override + public boolean isStrict() { + return false; + } + }; + + public abstract boolean isStrict(); +} diff --git a/src/main/java/ch/eitchnet/privilege/handler/DefaultPrivilegeHandler.java b/src/main/java/ch/eitchnet/privilege/handler/DefaultPrivilegeHandler.java index 89fc92884..26eb14d20 100644 --- a/src/main/java/ch/eitchnet/privilege/handler/DefaultPrivilegeHandler.java +++ b/src/main/java/ch/eitchnet/privilege/handler/DefaultPrivilegeHandler.java @@ -33,6 +33,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import ch.eitchnet.privilege.base.AccessDeniedException; +import ch.eitchnet.privilege.base.PrivilegeConflictResolution; import ch.eitchnet.privilege.base.PrivilegeException; import ch.eitchnet.privilege.model.Certificate; import ch.eitchnet.privilege.model.IPrivilege; @@ -111,6 +112,8 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler { */ private boolean autoPersistOnUserChangesData; + private PrivilegeConflictResolution privilegeConflictResolution; + @Override public RoleRep getRole(Certificate certificate, String roleName) { @@ -397,6 +400,9 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler { // create new user User newUser = createUser(userRep, passwordHash); + // detect privilege conflicts + assertNoPrivilegeConflict(newUser); + // validate this user may create such a user prvCtx.validateAction(new SimpleRestrictable(PRIVILEGE_ADD_USER, new Tuple(null, newUser))); @@ -449,6 +455,9 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler { User newUser = createUser(userRep, passwordHash); + // detect privilege conflicts + assertNoPrivilegeConflict(newUser); + // validate this user may modify this user prvCtx.validateAction(new SimpleRestrictable(PRIVILEGE_MODIFY_USER, new Tuple(existingUser, newUser))); @@ -525,6 +534,9 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler { // create new user User newUser = new User(userId, username, password, firstname, lastname, userState, roles, locale, propertyMap); + // detect privilege conflicts + assertNoPrivilegeConflict(newUser); + // validate this user may modify this user prvCtx.validateAction(new SimpleRestrictable(PRIVILEGE_MODIFY_USER, new Tuple(existingUser, newUser))); @@ -594,6 +606,9 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler { existingUser.getFirstname(), existingUser.getLastname(), existingUser.getUserState(), newRoles, existingUser.getLocale(), existingUser.getProperties()); + // detect privilege conflicts + assertNoPrivilegeConflict(newUser); + // delegate user replacement to persistence handler this.persistenceHandler.replaceUser(newUser); @@ -815,6 +830,9 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler { // create new role from RoleRep Role newRole = new Role(roleRep); + // detect privilege conflicts + assertNoPrivilegeConflict(newRole); + // validate that this user may modify this role prvCtx.validateAction(new SimpleRestrictable(PRIVILEGE_MODIFY_ROLE, new Tuple(existingRole, newRole))); @@ -903,6 +921,9 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler { // create new role Role newRole = new Role(existingRole.getName(), privilegeMap); + // detect privilege conflicts + assertNoPrivilegeConflict(newRole); + // validate that this user may modify this role prvCtx.validateAction(new SimpleRestrictable(PRIVILEGE_MODIFY_ROLE, new Tuple(existingRole, newRole))); @@ -1078,16 +1099,40 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler { Set privilegeNames = role.getPrivilegeNames(); for (String privilegeName : privilegeNames) { - // cache the privilege - if (privileges.containsKey(privilegeName)) - continue; - IPrivilege privilege = role.getPrivilege(privilegeName); if (privilege == null) { String msg = "The Privilege {0} does not exist for role {1}"; //$NON-NLS-1$ msg = MessageFormat.format(msg, privilegeName, roleName); throw new PrivilegeException(msg); } + + // cache the privilege + if (privileges.containsKey(privilegeName)) { + if (this.privilegeConflictResolution.isStrict()) { + String msg = "User has conflicts for privilege {0} with role {1}"; + msg = MessageFormat.format(msg, privilegeName, roleName); + throw new PrivilegeException(msg); + } + + IPrivilege priv = privileges.get(privilegeName); + boolean allAllowed = priv.isAllAllowed() || privilege.isAllAllowed(); + Set allowList; + Set denyList; + if (allAllowed) { + allowList = Collections.emptySet(); + denyList = Collections.emptySet(); + } else { + allowList = new HashSet<>(priv.getAllowList()); + allowList.addAll(privilege.getAllowList()); + denyList = new HashSet<>(priv.getDenyList()); + denyList.addAll(privilege.getDenyList()); + } + priv = new PrivilegeImpl(priv.getName(), priv.getPolicy(), allAllowed, denyList, allowList); + + privileges.put(privilegeName, priv); + continue; + } + privileges.put(privilegeName, privilege); // cache the policy for the privilege @@ -1258,16 +1303,110 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler { logger.error(msg); } + String privilegeConflictResolutionS = parameterMap.get(PARAM_PRIVILEGE_CONFLICT_RESOLUTION); + if (privilegeConflictResolutionS == null) { + this.privilegeConflictResolution = PrivilegeConflictResolution.STRICT; + String msg = "No {0} parameter defined. Using {1}"; + msg = MessageFormat.format(msg, PARAM_PRIVILEGE_CONFLICT_RESOLUTION, this.privilegeConflictResolution); + logger.info(msg); + } else { + try { + this.privilegeConflictResolution = PrivilegeConflictResolution.valueOf(privilegeConflictResolutionS); + } catch (Exception e) { + String msg = "Parameter {0} has illegal value {1}."; //$NON-NLS-1$ + msg = MessageFormat.format(msg, PARAM_PRIVILEGE_CONFLICT_RESOLUTION, privilegeConflictResolutionS); + throw new PrivilegeException(msg); + } + } + logger.info("Privilege conflict resolution set to " + this.privilegeConflictResolution); //$NON-NLS-1$ + // validate policies on privileges of Roles for (Role role : persistenceHandler.getAllRoles()) { validatePolicies(role); } + // validate privilege conflicts + validatePrivilegeConflicts(); + this.lastSessionId = 0l; this.privilegeContextMap = Collections.synchronizedMap(new HashMap()); this.initialized = true; } + private void validatePrivilegeConflicts() { + if (!this.privilegeConflictResolution.isStrict()) { + return; + } + + List conflicts = new ArrayList<>(); + List users = this.persistenceHandler.getAllUsers(); + for (User user : users) { + Map privilegeNames = new HashMap<>(); + conflicts.addAll(detectPrivilegeConflicts(privilegeNames, user)); + } + + if (!conflicts.isEmpty()) { + for (String conflict : conflicts) { + logger.error(conflict); + } + throw new PrivilegeException("There are " + conflicts.size() + " privilege conflicts!"); + } + } + + private void assertNoPrivilegeConflict(User user) { + if (this.privilegeConflictResolution.isStrict()) { + Map privilegeNames = new HashMap<>(); + List conflicts = detectPrivilegeConflicts(privilegeNames, user); + if (!conflicts.isEmpty()) { + String msg = conflicts.stream().collect(Collectors.joining("\n")); + throw new PrivilegeException(msg); + } + } + } + + private void assertNoPrivilegeConflict(Role role) { + if (!this.privilegeConflictResolution.isStrict()) + return; + + Map privilegeNames = new HashMap<>(); + for (String privilegeName : role.getPrivilegeNames()) { + privilegeNames.put(privilegeName, role.getName()); + } + + List conflicts = new ArrayList<>(); + List users = this.persistenceHandler.getAllUsers(); + for (User user : users) { + if (user.hasRole(role.getName())) + conflicts.addAll(detectPrivilegeConflicts(privilegeNames, user)); + } + + if (!conflicts.isEmpty()) { + String msg = conflicts.stream().collect(Collectors.joining("\n")); + throw new PrivilegeException(msg); + } + } + + private List detectPrivilegeConflicts(Map privilegeNames, User user) { + List conflicts = new ArrayList<>(); + + Set userRoles = user.getRoles(); + for (String roleName : userRoles) { + Role role = this.persistenceHandler.getRole(roleName); + for (String privilegeName : role.getPrivilegeNames()) { + if (!privilegeNames.containsKey(privilegeName)) { + privilegeNames.put(privilegeName, roleName); + } else { + String roleOrigin = privilegeNames.get(privilegeName); + String msg = "User has conflicts for privilege {0} on roles {1} and {2}"; + msg = MessageFormat.format(msg, privilegeName, roleOrigin, roleName); + conflicts.add(msg); + } + } + } + + return conflicts; + } + /** * Validates that the policies which are not null on the privileges of the role exist * diff --git a/src/main/java/ch/eitchnet/privilege/handler/PrivilegeHandler.java b/src/main/java/ch/eitchnet/privilege/handler/PrivilegeHandler.java index 9aea7c272..5b6d81ad1 100644 --- a/src/main/java/ch/eitchnet/privilege/handler/PrivilegeHandler.java +++ b/src/main/java/ch/eitchnet/privilege/handler/PrivilegeHandler.java @@ -20,6 +20,7 @@ import java.util.Locale; import java.util.Map; import ch.eitchnet.privilege.base.AccessDeniedException; +import ch.eitchnet.privilege.base.PrivilegeConflictResolution; import ch.eitchnet.privilege.base.PrivilegeException; import ch.eitchnet.privilege.model.Certificate; import ch.eitchnet.privilege.model.IPrivilege; @@ -121,6 +122,11 @@ public interface PrivilegeHandler { */ public static final String PARAM_AUTO_PERSIST_ON_USER_CHANGES_DATA = "autoPersistOnUserChangesData"; //$NON-NLS-1$ + /** + * configuration parameter to define {@link PrivilegeConflictResolution} + */ + public static final String PARAM_PRIVILEGE_CONFLICT_RESOLUTION = "privilegeConflictResolution"; + /** * Returns a {@link UserRep} for the given username * diff --git a/src/test/java/ch/eitchnet/privilege/test/PrivilegeConflictMergeTest.java b/src/test/java/ch/eitchnet/privilege/test/PrivilegeConflictMergeTest.java new file mode 100644 index 000000000..ab9a352e1 --- /dev/null +++ b/src/test/java/ch/eitchnet/privilege/test/PrivilegeConflictMergeTest.java @@ -0,0 +1,164 @@ +/* + * Copyright 2013 Robert von Burg + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package ch.eitchnet.privilege.test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.File; + +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import ch.eitchnet.privilege.base.PrivilegeException; +import ch.eitchnet.privilege.handler.PrivilegeHandler; +import ch.eitchnet.privilege.helper.PrivilegeInitializationHelper; +import ch.eitchnet.privilege.model.Certificate; +import ch.eitchnet.privilege.model.IPrivilege; +import ch.eitchnet.privilege.model.PrivilegeContext; +import ch.eitchnet.utils.helper.FileHelper; + +/** + * @author Robert von Burg + */ +public class PrivilegeConflictMergeTest { + + private static final Logger logger = LoggerFactory.getLogger(PrivilegeConflictMergeTest.class); + + @BeforeClass + public static void init() throws Exception { + try { + destroy(); + + // copy configuration to tmp + String pwd = System.getProperty("user.dir"); + + File origPrivilegeModelFile = new File(pwd + "/config/PrivilegeModelMerge.xml"); + File tmpPrivilegeModelFile = new File(pwd + "/target/testPrivilege/PrivilegeModelMerge.xml"); + if (tmpPrivilegeModelFile.exists() && !tmpPrivilegeModelFile.delete()) { + throw new RuntimeException("Tmp configuration still exists and can not be deleted at " + + tmpPrivilegeModelFile.getAbsolutePath()); + } + + File parentFile = tmpPrivilegeModelFile.getParentFile(); + if (!parentFile.exists()) { + if (!parentFile.mkdirs()) + throw new RuntimeException("Could not create parent for tmp " + tmpPrivilegeModelFile); + } + + if (!FileHelper.copy(origPrivilegeModelFile, tmpPrivilegeModelFile, true)) + throw new RuntimeException("Failed to copy " + origPrivilegeModelFile + " to " + tmpPrivilegeModelFile); + + } catch (Exception e) { + logger.error(e.getMessage(), e); + + throw new RuntimeException("Initialization failed: " + e.getLocalizedMessage(), e); + } + } + + @AfterClass + public static void destroy() throws Exception { + + // delete temporary file + String pwd = System.getProperty("user.dir"); + + File tmpPrivilegeModelFile = new File(pwd + "/target/testPrivilege/PrivilegeModelMerge.xml"); + if (tmpPrivilegeModelFile.exists() && !tmpPrivilegeModelFile.delete()) { + throw new RuntimeException("Tmp configuration still exists and can not be deleted at " + + tmpPrivilegeModelFile.getAbsolutePath()); + } + + // and temporary parent + File parentFile = tmpPrivilegeModelFile.getParentFile(); + if (parentFile.exists() && !parentFile.delete()) { + throw new RuntimeException("Could not remove temporary parent for tmp " + tmpPrivilegeModelFile); + } + } + + private PrivilegeHandler privilegeHandler; + private PrivilegeContext ctx; + + @Before + public void setup() throws Exception { + try { + + String pwd = System.getProperty("user.dir"); + + File privilegeConfigFile = new File(pwd + "/config/PrivilegeConfigMerge.xml"); + + // initialize privilege + privilegeHandler = PrivilegeInitializationHelper.initializeFromXml(privilegeConfigFile); + + } catch (Exception e) { + logger.error(e.getMessage(), e); + + throw new RuntimeException("Setup failed: " + e.getLocalizedMessage(), e); + } + } + + private void login(String username, byte[] password) { + Certificate certificate = privilegeHandler.authenticate(username, password); + assertTrue("Certificate is null!", certificate != null); + PrivilegeContext privilegeContext = privilegeHandler.getPrivilegeContext(certificate); + this.ctx = privilegeContext; + } + + private void logout() { + if (this.ctx != null) { + try { + PrivilegeContext privilegeContext = this.ctx; + this.ctx = null; + privilegeHandler.invalidateSession(privilegeContext.getCertificate()); + } catch (PrivilegeException e) { + String msg = "There is no PrivilegeContext currently bound to the ThreadLocal!"; + if (!e.getMessage().equals(msg)) + throw e; + } + } + } + + @Test + public void shouldMergePrivileges1() { + try { + login("userA", "admin".getBytes()); + IPrivilege privilege = this.ctx.getPrivilege("Foo"); + assertTrue(privilege.isAllAllowed()); + assertTrue(privilege.getAllowList().isEmpty()); + assertTrue(privilege.getDenyList().isEmpty()); + + } finally { + logout(); + } + } + + @Test + public void shouldMergePrivileges2() { + try { + login("userB", "admin".getBytes()); + IPrivilege privilege = this.ctx.getPrivilege("Bar"); + assertFalse(privilege.isAllAllowed()); + assertEquals(2, privilege.getAllowList().size()); + assertEquals(2, privilege.getDenyList().size()); + } finally { + logout(); + } + } +} diff --git a/src/test/java/ch/eitchnet/privilege/test/PrivilegeTest.java b/src/test/java/ch/eitchnet/privilege/test/PrivilegeTest.java index 310c88ad4..f23293162 100644 --- a/src/test/java/ch/eitchnet/privilege/test/PrivilegeTest.java +++ b/src/test/java/ch/eitchnet/privilege/test/PrivilegeTest.java @@ -24,6 +24,7 @@ import java.io.File; import java.text.MessageFormat; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -75,6 +76,7 @@ public class PrivilegeTest { private static final byte[] PASS_BOB = "admin1".getBytes(); private static final String ROLE_APP_USER = "AppUser"; private static final String ROLE_MY = "MyRole"; + private static final String ROLE_MY2 = "MyRole2"; private static final String ROLE_TEMP = "temp"; private static final String ROLE_USER = "user"; private static final byte[] PASS_DEF = "def".getBytes(); @@ -89,10 +91,6 @@ public class PrivilegeTest { private static PrivilegeHandler privilegeHandler; private PrivilegeContext ctx; - /** - * @throws Exception - * if something goes wrong - */ @BeforeClass public static void init() throws Exception { try { @@ -425,6 +423,35 @@ public class PrivilegeTest { } } + @Test + public void shouldDetectPrivilegeConflict1() { + exception.expect(PrivilegeException.class); + exception.expectMessage("User has conflicts for privilege "); + try { + login(ADMIN, ArraysHelper.copyOf(PASS_ADMIN)); + Certificate certificate = this.ctx.getCertificate(); + PrivilegeRep privilegeRep = new PrivilegeRep(PrivilegeHandler.PRIVILEGE_ACTION, "DefaultPrivilege", true, + Collections.emptySet(), Collections.emptySet()); + privilegeHandler.addOrReplacePrivilegeOnRole(certificate, ROLE_APP_USER, privilegeRep); + } finally { + logout(); + } + } + + @Test + public void shouldDetectPrivilegeConflict2() { + exception.expect(PrivilegeException.class); + exception.expectMessage("User has conflicts for privilege "); + try { + login(ADMIN, ArraysHelper.copyOf(PASS_ADMIN)); + Certificate certificate = this.ctx.getCertificate(); + privilegeHandler.addRoleToUser(certificate, ADMIN, ROLE_MY); + privilegeHandler.addRoleToUser(certificate, ADMIN, ROLE_MY2); + } finally { + logout(); + } + } + /** * This test performs multiple tests which are dependent on each other as the following is done: *
    diff --git a/src/test/java/ch/eitchnet/privilege/test/XmlTest.java b/src/test/java/ch/eitchnet/privilege/test/XmlTest.java index 6526e913a..ba39eb0e2 100644 --- a/src/test/java/ch/eitchnet/privilege/test/XmlTest.java +++ b/src/test/java/ch/eitchnet/privilege/test/XmlTest.java @@ -120,7 +120,7 @@ public class XmlTest { assertNotNull(containerModel.getPersistenceHandlerClassName()); assertNotNull(containerModel.getPersistenceHandlerParameterMap()); - assertEquals(1, containerModel.getParameterMap().size()); + assertEquals(2, containerModel.getParameterMap().size()); assertEquals(3, containerModel.getPolicies().size()); assertEquals(1, containerModel.getEncryptionHandlerParameterMap().size()); assertEquals(2, containerModel.getPersistenceHandlerParameterMap().size()); @@ -170,7 +170,7 @@ public class XmlTest { assertNotNull(roles); assertEquals(3, users.size()); - assertEquals(6, roles.size()); + assertEquals(7, roles.size()); // assert model