diff --git a/config/PrivilegeModel.xml b/config/PrivilegeModel.xml index 070e087d7..68ef7222f 100644 --- a/config/PrivilegeModel.xml +++ b/config/PrivilegeModel.xml @@ -81,6 +81,17 @@ true + + true + + + ENABLED + DISABLED + SYSTEM + + + true + diff --git a/src/main/java/ch/eitchnet/privilege/handler/DefaultPrivilegeHandler.java b/src/main/java/ch/eitchnet/privilege/handler/DefaultPrivilegeHandler.java index a91a018cd..1737181c4 100644 --- a/src/main/java/ch/eitchnet/privilege/handler/DefaultPrivilegeHandler.java +++ b/src/main/java/ch/eitchnet/privilege/handler/DefaultPrivilegeHandler.java @@ -651,18 +651,9 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler { @Override public UserRep setUserLocale(Certificate certificate, String username, Locale locale) { - // check if certificate is for same user, in which case user is changing their own locale - if (certificate.getUsername().equals(username)) { - - // validate the certificate - isCertificateValid(certificate); - - } else { - - // validate user actually has this type of privilege - PrivilegeContext prvCtx = getPrivilegeContext(certificate); - prvCtx.assertHasPrivilege(PRIVILEGE_MODIFY_USER); - } + // validate user actually has this type of privilege + PrivilegeContext prvCtx = getPrivilegeContext(certificate); + prvCtx.assertHasPrivilege(PRIVILEGE_SET_USER_LOCALE); // get User User existingUser = this.persistenceHandler.getUser(username); @@ -677,8 +668,7 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler { // if the user is not setting their own locale, then make sure this user may set this user's locale if (!certificate.getUsername().equals(username)) { - PrivilegeContext prvCtx = getPrivilegeContext(certificate); - prvCtx.validateAction(new SimpleRestrictable(PRIVILEGE_MODIFY_USER, new Tuple(existingUser, newUser))); + prvCtx.validateAction(new SimpleRestrictable(PRIVILEGE_SET_USER_LOCALE, new Tuple(existingUser, newUser))); } // delegate user replacement to persistence handler @@ -696,18 +686,9 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler { public void setUserPassword(Certificate certificate, String username, byte[] password) { try { - // check if certificate is for same user, in which case user is changing their own password - if (certificate.getUsername().equals(username)) { - - // validate the certificate - isCertificateValid(certificate); - - } else { - - // validate user actually has this type of privilege - PrivilegeContext prvCtx = getPrivilegeContext(certificate); - prvCtx.assertHasPrivilege(PRIVILEGE_MODIFY_USER); - } + // validate user actually has this type of privilege + PrivilegeContext prvCtx = getPrivilegeContext(certificate); + prvCtx.assertHasPrivilege(PRIVILEGE_SET_USER_PASSWORD); // get User User existingUser = this.persistenceHandler.getUser(username); @@ -732,8 +713,8 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler { // if the user is not setting their own password, then make sure this user may set this user's password if (!certificate.getUsername().equals(username)) { - PrivilegeContext prvCtx = getPrivilegeContext(certificate); - prvCtx.validateAction(new SimpleRestrictable(PRIVILEGE_MODIFY_USER, new Tuple(existingUser, newUser))); + prvCtx.validateAction(new SimpleRestrictable(PRIVILEGE_SET_USER_PASSWORD, new Tuple(existingUser, + newUser))); } // delegate user replacement to persistence handler @@ -754,7 +735,7 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler { // validate user actually has this type of privilege PrivilegeContext prvCtx = getPrivilegeContext(certificate); - prvCtx.assertHasPrivilege(PRIVILEGE_MODIFY_USER); + prvCtx.assertHasPrivilege(PRIVILEGE_SET_USER_STATE); // get User User existingUser = this.persistenceHandler.getUser(username); @@ -768,7 +749,7 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler { existingUser.getLocale(), existingUser.getProperties()); // validate that this user may modify this user's state - prvCtx.validateAction(new SimpleRestrictable(PRIVILEGE_MODIFY_USER, new Tuple(existingUser, newUser))); + prvCtx.validateAction(new SimpleRestrictable(PRIVILEGE_SET_USER_STATE, new Tuple(existingUser, newUser))); // delegate user replacement to persistence handler this.persistenceHandler.replaceUser(newUser); diff --git a/src/main/java/ch/eitchnet/privilege/handler/PrivilegeHandler.java b/src/main/java/ch/eitchnet/privilege/handler/PrivilegeHandler.java index 5b6d81ad1..4e6b848d9 100644 --- a/src/main/java/ch/eitchnet/privilege/handler/PrivilegeHandler.java +++ b/src/main/java/ch/eitchnet/privilege/handler/PrivilegeHandler.java @@ -114,6 +114,20 @@ public interface PrivilegeHandler { * specific user */ public static final String PRIVILEGE_REMOVE_ROLE_FROM_USER = "PrivilegeRemoveRoleFromUser"; + /** + * Privilege "PRIVILEGE_SET_USER_LOCALE" which is used to validate that a user can set the locale of a user, or + * their own + */ + public static final String PRIVILEGE_SET_USER_LOCALE = "PrivilegeSetUserLocale"; + /** + * Privilege "PRIVILEGE_SET_USER_STATE" which is used to validate that a user can set the state of a user + */ + public static final String PRIVILEGE_SET_USER_STATE = "PrivilegeSetUserState"; + /** + * Privilege "PRIVILEGE_SET_USER_PASSWORD" which is used to validate that a user can set the password of a user, or + * their own + */ + public static final String PRIVILEGE_SET_USER_PASSWORD = "PrivilegeSetUserPassword"; /// diff --git a/src/main/java/ch/eitchnet/privilege/policy/RoleAccessPrivilege.java b/src/main/java/ch/eitchnet/privilege/policy/RoleAccessPrivilege.java index d5b94f859..d82f3576d 100644 --- a/src/main/java/ch/eitchnet/privilege/policy/RoleAccessPrivilege.java +++ b/src/main/java/ch/eitchnet/privilege/policy/RoleAccessPrivilege.java @@ -44,6 +44,10 @@ public class RoleAccessPrivilege implements PrivilegePolicy { // get the value on which the action is to be performed Object object = restrictable.getPrivilegeValue(); + // if the object is null, then it means the validation is that the privilege must exist + if (object == null) + return; + // RoleAccessPrivilege policy expects the privilege value to be a role if (!(object instanceof Tuple)) { String msg = Restrictable.class.getName() diff --git a/src/main/java/ch/eitchnet/privilege/policy/UserAccessPrivilege.java b/src/main/java/ch/eitchnet/privilege/policy/UserAccessPrivilege.java index 1b92f19cf..cafbd37c5 100644 --- a/src/main/java/ch/eitchnet/privilege/policy/UserAccessPrivilege.java +++ b/src/main/java/ch/eitchnet/privilege/policy/UserAccessPrivilege.java @@ -43,6 +43,10 @@ public class UserAccessPrivilege implements PrivilegePolicy { // get the value on which the action is to be performed Object object = restrictable.getPrivilegeValue(); + // if the object is null, then it means the validation is that the privilege must exist + if (object == null) + return; + // RoleAccessPrivilege policy expects the privilege value to be a role if (!(object instanceof Tuple)) { String msg = Restrictable.class.getName() @@ -130,11 +134,56 @@ public class UserAccessPrivilege implements PrivilegePolicy { break; } + case PrivilegeHandler.PRIVILEGE_SET_USER_STATE: { + User oldUser = tuple.getFirst(); + User newUser = tuple.getSecond(); + + DBC.INTERIM.assertNotNull("For " + privilegeName + " first must not be null!", oldUser); + DBC.INTERIM.assertNotNull("For " + privilegeName + " second must not be null!", newUser); + + String privilegeValue = newUser.getUserState().name(); + PrivilegePolicyHelper.checkByAllowDenyValues(ctx, privilege, restrictable, privilegeValue); + + break; + } + case PrivilegeHandler.PRIVILEGE_SET_USER_LOCALE: { + User oldUser = tuple.getFirst(); + User newUser = tuple.getSecond(); + + DBC.INTERIM.assertNotNull("For " + privilegeName + " first must not be null!", oldUser); + DBC.INTERIM.assertNotNull("For " + privilegeName + " second must not be null!", newUser); + + String privilegeValue = newUser.getUsername(); + + // user can set their own locale + if (ctx.getUsername().equals(privilegeValue)) + return; + + PrivilegePolicyHelper.checkByAllowDenyValues(ctx, privilege, restrictable, privilegeValue); + + break; + } + case PrivilegeHandler.PRIVILEGE_SET_USER_PASSWORD: { + User oldUser = tuple.getFirst(); + User newUser = tuple.getSecond(); + + DBC.INTERIM.assertNotNull("For " + privilegeName + " first must not be null!", oldUser); + DBC.INTERIM.assertNotNull("For " + privilegeName + " second must not be null!", newUser); + + String privilegeValue = newUser.getUsername(); + + // user can set their own password + if (ctx.getUsername().equals(privilegeValue)) + return; + + PrivilegePolicyHelper.checkByAllowDenyValues(ctx, privilege, restrictable, privilegeValue); + + break; + } default: - String msg = Restrictable.class.getName() - + PrivilegeMessages.getString("Privilege.userAccessPrivilege.unknownPrivilege"); //$NON-NLS-1$ - msg = MessageFormat.format(msg, privilegeName); + String msg = PrivilegeMessages.getString("Privilege.userAccessPrivilege.unknownPrivilege"); //$NON-NLS-1$ + msg = MessageFormat.format(msg, privilegeName, this.getClass().getName()); throw new PrivilegeException(msg); } } diff --git a/src/main/resources/PrivilegeMessages.properties b/src/main/resources/PrivilegeMessages.properties index 5c291a359..a92f0f135 100644 --- a/src/main/resources/PrivilegeMessages.properties +++ b/src/main/resources/PrivilegeMessages.properties @@ -4,10 +4,11 @@ Privilege.illegalArgument.nonrole=\ {0} did not return a Role privilege value\! Privilege.illegalArgument.noncertificate=\ {0} did not return a Certificate privilege value\! Privilege.illegalArgument.nonuser=\ {0} did not return a User privilege value\! Privilege.illegalArgument.privilegeNameMismatch=The passed privilege has the name {0} but the restrictable is referencing privilege {1} +Privilege.illegalArgument.nontuple=\ {0} did not return a Tuple privilege value\! Privilege.privilegeNameEmpty=The PrivilegeName for the Restrictable is null or empty: {0} Privilege.privilegeNull=Privilege may not be null\! Privilege.restrictableNull=Restrictable may not be null\! Privilege.noprivilege=No Privilege exists with name {0} Privilege.noprivilege.user=User {0} does not have the privilege {1} -Privilege.roleAccessPrivilege.unknownPrivilege=Unhandled RoleAccessPrivilege {0} -Privilege.userAccessPrivilege.unknownPrivilege=Unhandled UserAccessPrivilege {0} +Privilege.roleAccessPrivilege.unknownPrivilege=Unhandled privilege {0} for policy {1} +Privilege.userAccessPrivilege.unknownPrivilege=Unhandled privilege {0} for policy {1} diff --git a/src/test/java/ch/eitchnet/privilege/test/PrivilegeTest.java b/src/test/java/ch/eitchnet/privilege/test/PrivilegeTest.java index 3b7a35411..37d159165 100644 --- a/src/test/java/ch/eitchnet/privilege/test/PrivilegeTest.java +++ b/src/test/java/ch/eitchnet/privilege/test/PrivilegeTest.java @@ -28,6 +28,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Locale; import org.junit.AfterClass; import org.junit.Before; @@ -67,6 +68,7 @@ import ch.eitchnet.utils.helper.FileHelper; public class PrivilegeTest { private static final String ROLE_PRIVILEGE_ADMIN = "PrivilegeAdmin"; + private static final String PRIVILEGE_USER_ACCESS = "UserAccessPrivilege"; private static final String ADMIN = "admin"; private static final byte[] PASS_ADMIN = "admin".getBytes(); private static final String BOB = "bob"; @@ -77,6 +79,7 @@ public class PrivilegeTest { 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_CHANGE_PW = "changePw"; private static final String ROLE_TEMP = "temp"; private static final String ROLE_USER = "user"; private static final byte[] PASS_DEF = "def".getBytes(); @@ -487,13 +490,121 @@ public class PrivilegeTest { addTedAsBob(); failAuthAsTedNoPass(); setPassForTedAsBob(); - tedChangesOwnPass(); + failSetSystemStateTed(); + failTedChangesPwAndLocale(); + addChangePwRoleToTed(); + tedChangesOwnPassAndLocale(); + failTedChangesOtherPwStateAndLocale(); authAsTed(); failPerformRestrictableAsBobNoRoleApp(); addRoleAppToBob(); performRestrictableAsBob(); } + private void failSetSystemStateTed() { + try { + // testEnableUserBob + login(BOB, ArraysHelper.copyOf(PASS_BOB)); + Certificate certificate = this.ctx.getCertificate(); + + try { + privilegeHandler.setUserState(certificate, TED, UserState.SYSTEM); + fail("Should not be able to set user state to SYSTEM"); + } catch (AccessDeniedException e) { + // ok + } + + } finally { + logout(); + } + } + + private void failTedChangesOtherPwStateAndLocale() { + try { + // testTedChangesOwnPwd + login(TED, ArraysHelper.copyOf(PASS_TED)); + Certificate certificate = this.ctx.getCertificate(); + + try { + privilegeHandler.setUserPassword(certificate, BOB, ArraysHelper.copyOf(PASS_TED)); + fail("Should not be able to set password of other user, as missing privilege"); + } catch (AccessDeniedException e) { + // ok + } + + try { + privilegeHandler.setUserLocale(certificate, BOB, Locale.FRENCH); + fail("Should not be able to set locale of other user, as missing privilege"); + } catch (AccessDeniedException e) { + // ok + } + + try { + privilegeHandler.setUserState(certificate, BOB, UserState.DISABLED); + fail("Should not be able to set state of other user, as missing privilege"); + } catch (AccessDeniedException e) { + // ok + } + + } finally { + logout(); + } + } + + private void failTedChangesPwAndLocale() { + try { + // testTedChangesOwnPwd + login(TED, ArraysHelper.copyOf(PASS_DEF)); + Certificate certificate = this.ctx.getCertificate(); + + try { + privilegeHandler.setUserPassword(certificate, TED, ArraysHelper.copyOf(PASS_TED)); + fail("Should not be able to set password, as missing privilege"); + } catch (AccessDeniedException e) { + // ok + } + + try { + privilegeHandler.setUserLocale(certificate, TED, Locale.FRENCH); + fail("Should not be able to set locale, as missing privilege"); + } catch (AccessDeniedException e) { + // ok + } + + try { + privilegeHandler.setUserState(certificate, TED, UserState.ENABLED); + fail("Should not be able to set state, as missing privilege"); + } catch (AccessDeniedException e) { + // ok + } + + } finally { + logout(); + } + } + + private void addChangePwRoleToTed() { + try { + // add role user + login(ADMIN, ArraysHelper.copyOf(PASS_ADMIN)); + + PrivilegeRep passwordRep = new PrivilegeRep(PrivilegeHandler.PRIVILEGE_SET_USER_PASSWORD, + PRIVILEGE_USER_ACCESS, false, Collections.emptySet(), Collections.emptySet()); + PrivilegeRep localeRep = new PrivilegeRep(PrivilegeHandler.PRIVILEGE_SET_USER_LOCALE, + PRIVILEGE_USER_ACCESS, false, Collections.emptySet(), Collections.emptySet()); + + RoleRep roleRep = new RoleRep(ROLE_CHANGE_PW, Arrays.asList(passwordRep, localeRep)); + + Certificate certificate = this.ctx.getCertificate(); + privilegeHandler.addRole(certificate, roleRep); + privilegeHandler.addRoleToUser(certificate, TED, ROLE_CHANGE_PW); + logger.info("Added " + ROLE_CHANGE_PW + " to " + TED); + privilegeHandler.persist(certificate); + } finally { + logout(); + } + } + private void performRestrictableAsBob() { try { // testPerformRestrictableAsBob @@ -547,12 +658,13 @@ public class PrivilegeTest { } } - private void tedChangesOwnPass() { + private void tedChangesOwnPassAndLocale() { try { // testTedChangesOwnPwd login(TED, ArraysHelper.copyOf(PASS_DEF)); Certificate certificate = this.ctx.getCertificate(); privilegeHandler.setUserPassword(certificate, TED, ArraysHelper.copyOf(PASS_TED)); + privilegeHandler.setUserLocale(certificate, TED, Locale.FRENCH); } finally { logout(); } @@ -666,7 +778,7 @@ public class PrivilegeTest { try { // add role user login(ADMIN, ArraysHelper.copyOf(PASS_ADMIN)); - RoleRep roleRep = new RoleRep(ROLE_USER, new ArrayList()); + RoleRep roleRep = new RoleRep(ROLE_USER, new ArrayList<>()); Certificate certificate = this.ctx.getCertificate(); privilegeHandler.addRole(certificate, roleRep); privilegeHandler.persist(certificate); diff --git a/src/test/java/ch/eitchnet/privilege/test/XmlTest.java b/src/test/java/ch/eitchnet/privilege/test/XmlTest.java index ba39eb0e2..637c43901 100644 --- a/src/test/java/ch/eitchnet/privilege/test/XmlTest.java +++ b/src/test/java/ch/eitchnet/privilege/test/XmlTest.java @@ -212,7 +212,7 @@ public class XmlTest { // PrivilegeAdmin Role privilegeAdmin = findRole("PrivilegeAdmin", roles); assertEquals("PrivilegeAdmin", privilegeAdmin.getName()); - assertEquals(11, privilegeAdmin.getPrivilegeNames().size()); + assertEquals(14, privilegeAdmin.getPrivilegeNames().size()); IPrivilege privilegeAction = privilegeAdmin.getPrivilege(PrivilegeHandler.PRIVILEGE_ACTION); assertFalse(privilegeAction.isAllAllowed()); assertEquals(3, privilegeAction.getAllowList().size());