[Interface] changed all methods which return boolean on action allowed to be void as they must throw exceptions if the privilege is not granted

This commit is contained in:
eitch 2011-07-30 17:34:53 +00:00
parent 6191949c3e
commit e8a7fb5cf9
5 changed files with 119 additions and 53 deletions

View File

@ -563,10 +563,7 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler {
public boolean invalidateSession(Certificate certificate) {
// first validate certificate
if (!isCertificateValid(certificate)) {
logger.info("Certificate is not valid, so no session to invalidate: " + certificate.toString());
return false;
}
isCertificateValid(certificate);
// remove registration
CertificateSessionPair certificateSessionPair = this.sessionMap.remove(certificate.getSessionId());
@ -592,15 +589,12 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler {
* ch.eitchnet.privilege.model.Restrictable)
*/
@Override
public boolean actionAllowed(Certificate certificate, Restrictable restrictable) {
public void actionAllowed(Certificate certificate, Restrictable restrictable) {
// TODO What is better, validate from {@link Restrictable} to {@link User} or the opposite direction?
// first validate certificate
if (!isCertificateValid(certificate)) {
throw new PrivilegeException("Certificate is not valid, so action is not allowed: " + certificate
+ " for restrictable: " + restrictable);
}
isCertificateValid(certificate);
// restrictable must not be null
if (restrictable == null)
@ -613,9 +607,7 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler {
"Oh boy, how did this happen: No User in user map although the certificate is valid!");
}
// default is to not allow the action
// TODO should default deny/allow policy be configurable?
boolean actionAllowed = false;
String privilegeName = restrictable.getPrivilegeName();
// now iterate roles and validate on policies
for (String roleName : user.getRoles()) {
@ -626,14 +618,17 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler {
continue;
}
actionAllowed = actionAllowed(role, restrictable);
// ignore if this role does not have the privilege
if (!role.hasPrivilege(privilegeName))
continue;
// if action is allowed, then break iteration as a privilege match has been made
if (actionAllowed)
break;
if (actionAllowed(role, restrictable))
return;
}
return actionAllowed;
throw new AccessDeniedException("User " + user.getUsername() + " does not have Privilege " + privilegeName
+ " needed for Restrictable " + restrictable.getClass().getName());
}
/**
@ -644,7 +639,7 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler {
* ch.eitchnet.privilege.model.Restrictable)
*/
@Override
public boolean actionAllowed(RoleRep roleRep, Restrictable restrictable) {
public void actionAllowed(RoleRep roleRep, Restrictable restrictable) {
// user and restrictable must not be null
if (roleRep == null)
@ -660,7 +655,8 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler {
throw new PrivilegeException("No Role exists with the name " + roleRep.getName());
}
return actionAllowed(role, restrictable);
// delegate to method with Role
actionAllowed(role, restrictable);
}
/**
@ -669,8 +665,6 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler {
*
* @param role
* @param restrictable
*
* @return true if the privilege is granted, false otherwise
*/
protected boolean actionAllowed(Role role, Restrictable restrictable) {
@ -691,8 +685,9 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler {
Privilege privilege = role.getPrivilegeMap().get(privilegeName);
// check if all is allowed
if (privilege.isAllAllowed())
if (privilege.isAllAllowed()) {
return true;
}
// otherwise delegate checking to the policy configured for this privilege
PrivilegePolicy policy = this.getPolicy(privilege.getPolicy());
@ -702,14 +697,18 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler {
}
// delegate checking to privilege policy
return policy.actionAllowed(role, privilege, restrictable);
policy.actionAllowed(role, privilege, restrictable);
// the policy must throw an exception if action not allowed,
// so return true if we arrive here
return true;
}
/**
* @see ch.eitchnet.privilege.handler.PrivilegeHandler#isCertificateValid(ch.eitchnet.privilege.model.Certificate)
*/
@Override
public boolean isCertificateValid(Certificate certificate) {
public void isCertificateValid(Certificate certificate) {
// certificate must not be null
if (certificate == null)
@ -741,8 +740,7 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler {
"Oh boy, how did this happen: No User in user map although the certificate is valid!");
}
// everything is ok, so return true as the certificate must be valid
return true;
// everything is ok
}
/**
@ -752,9 +750,7 @@ public class DefaultPrivilegeHandler implements PrivilegeHandler {
public void validateIsPrivilegeAdmin(Certificate certificate) throws PrivilegeException {
// validate certificate
if (!this.isCertificateValid(certificate)) {
throw new PrivilegeException("Certificate " + certificate + " is not valid!");
}
this.isCertificateValid(certificate);
// get user object
User user = this.persistenceHandler.getUser(certificate.getUsername());

View File

@ -326,15 +326,13 @@ public interface PrivilegeHandler {
* @param restrictable
* the {@link Restrictable} to which the user wants access
*
* @return true if the access is allowed, false otherwise
*
* @throws AccessDeniedException
* if the user for this certificate may not perform the action defined by the {@link Restrictable}
* implementation
* @throws PrivilegeException
* if there is anything wrong with this certificate
*/
public boolean actionAllowed(Certificate certificate, Restrictable restrictable) throws AccessDeniedException,
public void actionAllowed(Certificate certificate, Restrictable restrictable) throws AccessDeniedException,
PrivilegeException;
/**
@ -345,12 +343,13 @@ public interface PrivilegeHandler {
* @param restrictable
* the {@link Restrictable} to which access is to be checked
*
* @return true if the {@link RoleRep} has access, false otherwise
*
* @throws PrivilegeException
* if the {@link Role} does not exist
* @throws AccessDeniedException
* if the role may not perform the action defined by the {@link Restrictable} implementation
*/
public boolean actionAllowed(RoleRep roleRep, Restrictable restrictable) throws AccessDeniedException;
public void actionAllowed(RoleRep roleRep, Restrictable restrictable) throws PrivilegeException,
AccessDeniedException;
/**
* Checks if the given {@link Certificate} is valid. This means that the certificate is for a valid session and that
@ -359,12 +358,10 @@ public interface PrivilegeHandler {
* @param certificate
* the {@link Certificate} to check
*
* @return true if the {@link Certificate} is valid, false otherwise
*
* @throws PrivilegeException
* if there is anything wrong with this certificate
*/
public boolean isCertificateValid(Certificate certificate) throws PrivilegeException;
public void isCertificateValid(Certificate certificate) throws PrivilegeException;
/**
* <p>

View File

@ -10,6 +10,7 @@
package ch.eitchnet.privilege.policy;
import ch.eitchnet.privilege.i18n.AccessDeniedException;
import ch.eitchnet.privilege.i18n.PrivilegeException;
import ch.eitchnet.privilege.model.Restrictable;
import ch.eitchnet.privilege.model.internal.Privilege;
@ -27,7 +28,7 @@ public class DefaultPrivilege implements PrivilegePolicy {
* ch.eitchnet.privilege.model.internal.Privilege, ch.eitchnet.privilege.model.Restrictable)
*/
@Override
public boolean actionAllowed(Role role, Privilege privilege, Restrictable restrictable) {
public void actionAllowed(Role role, Privilege privilege, Restrictable restrictable) {
// validate user is not null
if (role == null)
@ -39,10 +40,6 @@ public class DefaultPrivilege implements PrivilegePolicy {
throw new PrivilegeException("The PrivilegeName for the Restrictable is null or empty: " + restrictable);
}
// does this role have privilege for any values?
if (privilege.isAllAllowed())
return true;
// get the value on which the action is to be performed
Object object = restrictable.getPrivilegeValue();
@ -56,17 +53,24 @@ public class DefaultPrivilege implements PrivilegePolicy {
// first check values not allowed
for (String denied : privilege.getDenyList()) {
if (denied.equals(privilegeValue))
return false;
// if value in deny list
if (denied.equals(privilegeValue)) {
// then throw access denied
throw new AccessDeniedException("Role " + role.getName() + " does not have Privilege " + privilegeName
+ " needed for Restrictable " + restrictable.getClass().getName());
}
}
// now check values allowed
for (String allowed : privilege.getAllowList()) {
if (allowed.equals(privilegeValue))
return true;
return;
}
// default is not allowed
return false;
throw new AccessDeniedException("Role " + role.getName() + " does not have Privilege " + privilegeName
+ " needed for Restrictable " + restrictable.getClass().getName());
}
}

View File

@ -10,6 +10,7 @@
package ch.eitchnet.privilege.policy;
import ch.eitchnet.privilege.i18n.AccessDeniedException;
import ch.eitchnet.privilege.model.Restrictable;
import ch.eitchnet.privilege.model.internal.Privilege;
import ch.eitchnet.privilege.model.internal.Role;
@ -40,7 +41,8 @@ public interface PrivilegePolicy {
* @param restrictable
* the {@link Restrictable} to which the user wants access
*
* @return return true if the action is allowed, false if not
* @throws AccessDeniedException
* if action not allowed
*/
public boolean actionAllowed(Role role, Privilege privilege, Restrictable restrictable);
public void actionAllowed(Role role, Privilege privilege, Restrictable restrictable) throws AccessDeniedException;
}

View File

@ -47,6 +47,7 @@ public class PrivilegeTest {
private static final String BOB = "bob";
private static final String TED = "ted";
private static final String PASS_BOB = "admin1";
private static final String ROLE_FEATHERLITE_USER = "FeatherliteUser";
private static final String ROLE_USER = "user";
private static final String PASS_BAD = "123";
@ -87,6 +88,7 @@ public class PrivilegeTest {
Certificate certificate = privilegeHandler.authenticate(ADMIN, PASS_ADMIN);
org.junit.Assert.assertTrue("Certificate is null!", certificate != null);
privilegeHandler.invalidateSession(certificate);
}
/**
@ -98,6 +100,7 @@ public class PrivilegeTest {
Certificate certificate = privilegeHandler.authenticate(ADMIN, PASS_BAD);
org.junit.Assert.assertTrue("Certificate is null!", certificate != null);
privilegeHandler.invalidateSession(certificate);
}
/**
@ -109,6 +112,7 @@ public class PrivilegeTest {
Certificate certificate = privilegeHandler.authenticate(ADMIN, null);
org.junit.Assert.assertTrue("Certificate is null!", certificate != null);
privilegeHandler.invalidateSession(certificate);
}
/**
@ -128,6 +132,7 @@ public class PrivilegeTest {
// set bob's password
privilegeHandler.setUserPassword(certificate, BOB, PASS_BOB);
logger.info("Set Bob's password");
privilegeHandler.invalidateSession(certificate);
}
/**
@ -138,7 +143,8 @@ public class PrivilegeTest {
*/
@Test(expected = AccessDeniedException.class)
public void testFailAuthAsBob() throws Exception {
privilegeHandler.authenticate(BOB, PASS_BOB);
Certificate certificate = privilegeHandler.authenticate(BOB, PASS_BOB);
privilegeHandler.invalidateSession(certificate);
}
/**
@ -149,6 +155,7 @@ public class PrivilegeTest {
public void testEnableUserBob() throws Exception {
Certificate certificate = privilegeHandler.authenticate(ADMIN, PASS_ADMIN);
privilegeHandler.setUserState(certificate, BOB, UserState.ENABLED);
privilegeHandler.invalidateSession(certificate);
}
/**
@ -162,6 +169,7 @@ public class PrivilegeTest {
Certificate certificate = privilegeHandler.authenticate(BOB, PASS_BOB);
org.junit.Assert.assertTrue("Certificate is null!", certificate != null);
privilegeHandler.invalidateSession(certificate);
}
/**
@ -176,6 +184,7 @@ public class PrivilegeTest {
RoleRep roleRep = new RoleRep(ROLE_USER, privilegeMap);
privilegeHandler.addOrReplaceRole(certificate, roleRep);
privilegeHandler.invalidateSession(certificate);
}
/**
@ -186,6 +195,7 @@ public class PrivilegeTest {
public void testAddRoleToBob() throws Exception {
Certificate certificate = privilegeHandler.authenticate(ADMIN, PASS_ADMIN);
privilegeHandler.addRoleToUser(certificate, BOB, ROLE_USER);
privilegeHandler.invalidateSession(certificate);
}
/**
@ -194,7 +204,8 @@ public class PrivilegeTest {
*/
@Test
public void testAuthAsBob() throws Exception {
privilegeHandler.authenticate(BOB, PASS_BOB);
Certificate certificate = privilegeHandler.authenticate(BOB, PASS_BOB);
privilegeHandler.invalidateSession(certificate);
}
/**
@ -214,6 +225,7 @@ public class PrivilegeTest {
UserRep userRep = new UserRep("1", TED, "Ted", "And then Some", UserState.NEW, new HashSet<String>(), null);
privilegeHandler.addOrReplaceUser(certificate, userRep, null);
logger.info("Added user " + TED);
privilegeHandler.invalidateSession(certificate);
}
/**
@ -226,6 +238,7 @@ public class PrivilegeTest {
Certificate certificate = privilegeHandler.authenticate(ADMIN, PASS_ADMIN);
privilegeHandler.addRoleToUser(certificate, BOB, PrivilegeHandler.PRIVILEGE_ADMIN_ROLE);
logger.info("Added " + PrivilegeHandler.PRIVILEGE_ADMIN_ROLE + " to " + ADMIN);
privilegeHandler.invalidateSession(certificate);
}
/**
@ -242,6 +255,7 @@ public class PrivilegeTest {
UserRep userRep = new UserRep("2", TED, "Ted", "Newman", UserState.NEW, new HashSet<String>(), null);
privilegeHandler.addOrReplaceUser(certificate, userRep, null);
logger.info("Added user " + TED);
privilegeHandler.invalidateSession(certificate);
}
/**
@ -249,14 +263,67 @@ public class PrivilegeTest {
* if something goes wrong
*/
@Test
public void testPerformRestrictable() throws Exception {
public void testPerformRestrictableAsAdmin() throws Exception {
Certificate certificate = privilegeHandler.authenticate(ADMIN, PASS_ADMIN);
org.junit.Assert.assertTrue("Certificate is null!", certificate != null);
// see if eitch can perform restrictable
Restrictable restrictable = new TestRestrictable();
boolean actionAllowed = privilegeHandler.actionAllowed(certificate, restrictable);
org.junit.Assert.assertTrue(ADMIN + " may not perform restrictable!", actionAllowed);
privilegeHandler.actionAllowed(certificate, restrictable);
privilegeHandler.invalidateSession(certificate);
}
/**
* Tests if the user bob, who does not have FeatherliteUser role can perform restrictable
*
* @throws Exception
* if something goes wrong
*/
@Test(expected = AccessDeniedException.class)
public void testFailPerformRestrictableAsBob() throws Exception {
Certificate certificate = privilegeHandler.authenticate(BOB, PASS_BOB);
org.junit.Assert.assertTrue("Certificate is null!", certificate != null);
// see if bob can perform restrictable
Restrictable restrictable = new TestRestrictable();
try {
privilegeHandler.actionAllowed(certificate, restrictable);
} finally {
privilegeHandler.invalidateSession(certificate);
}
}
/**
* @throws Exception
* if something goes wrong
*/
@Test
public void testAddFeatherliteRoleToBob() throws Exception {
Certificate certificate = privilegeHandler.authenticate(ADMIN, PASS_ADMIN);
privilegeHandler.addRoleToUser(certificate, BOB, ROLE_FEATHERLITE_USER);
logger.info("Added " + ROLE_FEATHERLITE_USER + " to " + BOB);
privilegeHandler.invalidateSession(certificate);
}
/**
* Tests if the user bob, who now has FeatherliteUser role can perform restrictable
*
* @throws Exception
* if something goes wrong
*/
@Test
public void testPerformRestrictableAsBob() throws Exception {
Certificate certificate = privilegeHandler.authenticate(BOB, PASS_BOB);
org.junit.Assert.assertTrue("Certificate is null!", certificate != null);
// see if bob can perform restrictable
Restrictable restrictable = new TestRestrictable();
try {
privilegeHandler.actionAllowed(certificate, restrictable);
} finally {
privilegeHandler.invalidateSession(certificate);
}
}
}