From 3cd253b9966e60fb29312f2b711ac7f38aac6e2d Mon Sep 17 00:00:00 2001 From: Scott Andrews Date: Thu, 6 Mar 2008 20:14:13 +0000 Subject: [PATCH] refactored security integration to use AccessDecisionManagers provided by Spring Security. SWF-93 --- .../SecurityFlowExecutionListener.java | 97 +++++++++---- .../webflow/security/SecurityRule.java | 59 +------- .../SecurityFlowExecutionListenerTests.java | 136 ++++++++++-------- .../webflow/security/SecurityRuleTests.java | 63 +------- 4 files changed, 145 insertions(+), 210 deletions(-) diff --git a/spring-webflow/src/main/java/org/springframework/webflow/security/SecurityFlowExecutionListener.java b/spring-webflow/src/main/java/org/springframework/webflow/security/SecurityFlowExecutionListener.java index 0900cd09..3f6c3608 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/security/SecurityFlowExecutionListener.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/security/SecurityFlowExecutionListener.java @@ -1,12 +1,18 @@ package org.springframework.webflow.security; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; -import org.springframework.security.AccessDeniedException; +import org.springframework.security.AccessDecisionManager; import org.springframework.security.Authentication; +import org.springframework.security.ConfigAttributeDefinition; +import org.springframework.security.SecurityConfig; import org.springframework.security.context.SecurityContextHolder; +import org.springframework.security.vote.AbstractAccessDecisionManager; +import org.springframework.security.vote.AffirmativeBased; +import org.springframework.security.vote.RoleVoter; +import org.springframework.security.vote.UnanimousBased; import org.springframework.webflow.definition.FlowDefinition; import org.springframework.webflow.definition.StateDefinition; import org.springframework.webflow.definition.TransitionDefinition; @@ -21,6 +27,8 @@ import org.springframework.webflow.execution.RequestContext; */ public class SecurityFlowExecutionListener extends FlowExecutionListenerAdapter { + AccessDecisionManager accessDecisionManager; + /** * Check security authorization when flow session starts */ @@ -28,12 +36,7 @@ public class SecurityFlowExecutionListener extends FlowExecutionListenerAdapter SecurityRule rule = (SecurityRule) definition.getAttributes().get( SecurityRule.SECURITY_AUTHORITY_ATTRIBUTE_NAME); if (rule != null) { - Collection principalAuthorities = getPrincipalAuthorities(); - if (!rule.isAuthorized(principalAuthorities)) { - throw new AccessDeniedException("Required authority not found: " - + SecurityRule.convertAuthoritiesToCommaSeparatedString(rule - .getNonGrantedAuthorities(principalAuthorities))); - } + decide(rule, definition); } } @@ -43,12 +46,7 @@ public class SecurityFlowExecutionListener extends FlowExecutionListenerAdapter public void stateEntering(RequestContext context, StateDefinition state) throws EnterStateVetoException { SecurityRule rule = (SecurityRule) state.getAttributes().get(SecurityRule.SECURITY_AUTHORITY_ATTRIBUTE_NAME); if (rule != null) { - Collection principalAuthorities = getPrincipalAuthorities(); - if (!rule.isAuthorized(principalAuthorities)) { - throw new AccessDeniedException("Required authority not found: " - + SecurityRule.convertAuthoritiesToCommaSeparatedString(rule - .getNonGrantedAuthorities(principalAuthorities))); - } + decide(rule, state); } } @@ -59,26 +57,65 @@ public class SecurityFlowExecutionListener extends FlowExecutionListenerAdapter SecurityRule rule = (SecurityRule) transition.getAttributes().get( SecurityRule.SECURITY_AUTHORITY_ATTRIBUTE_NAME); if (rule != null) { - Collection principalAuthorities = getPrincipalAuthorities(); - if (!rule.isAuthorized(principalAuthorities)) { - throw new AccessDeniedException("Required authority not found: " - + SecurityRule.convertAuthoritiesToCommaSeparatedString(rule - .getNonGrantedAuthorities(principalAuthorities))); - } + decide(rule, transition); } } /** - * Get Spring Security authorities for the principal - * @return granted authorities for the principal + * Performs a Spring Security authorization decision. Decision will use the provided AccessDecisionManager. If no + * AccessDecisionManager is provided a role based manager will be selected according to the comparison type of the + * rule. + * @param rule the rule to base the decision + * @param object the execution listener phase */ - protected Collection getPrincipalAuthorities() { - Authentication currentUser = SecurityContextHolder.getContext().getAuthentication(); - if ((null == currentUser) || (null == currentUser.getAuthorities()) - || (currentUser.getAuthorities().length < 1)) { - return Collections.EMPTY_LIST; + public void decide(SecurityRule rule, Object object) { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + ConfigAttributeDefinition config = new ConfigAttributeDefinition(getConfigAttributes(rule)); + if (accessDecisionManager == null) { + AbstractAccessDecisionManager abstractAccessDecisionManager; + List voters = new ArrayList(); + voters.add(new RoleVoter()); + if (rule.getComparisonType() == SecurityRule.COMPARISON_ANY) { + abstractAccessDecisionManager = new AffirmativeBased(); + } else if (rule.getComparisonType() == SecurityRule.COMPARISON_ALL) { + abstractAccessDecisionManager = new UnanimousBased(); + } else { + throw new IllegalStateException("Unknown SecurityRule match type: " + rule.getComparisonType()); + } + abstractAccessDecisionManager.setDecisionVoters(voters); + accessDecisionManager = abstractAccessDecisionManager; } - return Arrays.asList(currentUser.getAuthorities()); + accessDecisionManager.decide(authentication, object, config); + } + + /** + * Convert SecurityRule into a form understood by Spring Security + * @param rule the rule to convert + * @return list of ConfigAttributes for Spring Security + */ + protected List getConfigAttributes(SecurityRule rule) { + List configAttributes = new ArrayList(); + Iterator requiredAuthorityIt = rule.getRequiredAuthorities().iterator(); + while (requiredAuthorityIt.hasNext()) { + configAttributes.add(new SecurityConfig((String) requiredAuthorityIt.next())); + } + return configAttributes; + } + + /** + * Get decision manager + * @return the decision manager + */ + public AccessDecisionManager getAccessDecisionManager() { + return accessDecisionManager; + } + + /** + * Set decision manager + * @param accessDecisionManager the decision manager to user + */ + public void setAccessDecisionManager(AccessDecisionManager accessDecisionManager) { + this.accessDecisionManager = accessDecisionManager; } } diff --git a/spring-webflow/src/main/java/org/springframework/webflow/security/SecurityRule.java b/spring-webflow/src/main/java/org/springframework/webflow/security/SecurityRule.java index 0c0d23ed..9ba3e528 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/security/SecurityRule.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/security/SecurityRule.java @@ -28,39 +28,7 @@ public class SecurityRule { public static final short COMPARISON_ALL = 2; private Collection requiredAuthorities; - private short comparisonType; - - /** - * Test the required authorities against the principal's authorities - * @param principalAuthorities the principal's granted authorities - * @return true if authorized - */ - public boolean isAuthorized(Collection principalAuthorities) { - if (getComparisonType() == COMPARISON_ANY) { - return isAuthorizedAny(principalAuthorities); - } else if (getComparisonType() == COMPARISON_ALL) { - return isAuthorizedAll(principalAuthorities); - } else { - throw new IllegalStateException("Unknow comparisonType"); - } - } - - /** - * Get authorities that are required but not granted - * @param principalAuthorities the principal's granted authorities - * @return non granted authorities - */ - public Collection getNonGrantedAuthorities(Collection principalAuthorities) { - Collection nonGrantedAuthorities = new HashSet(); - Iterator authorityIt = getRequiredAuthorities().iterator(); - while (authorityIt.hasNext()) { - String authority = (String) authorityIt.next(); - if (!principalAuthorities.contains(authority)) { - nonGrantedAuthorities.add(authority); - } - } - return nonGrantedAuthorities; - } + private short comparisonType = COMPARISON_ANY; /** * Convert authorities to comma separated String @@ -96,31 +64,6 @@ public class SecurityRule { return auths; } - /** - * Test that any of the required authorities match the principal's authorities - * @param principalAuthorities the principal's granted authorities - * @return true if authorized - */ - private boolean isAuthorizedAny(Collection principalAuthorities) { - boolean authorized = false; - Iterator authorityIt = principalAuthorities.iterator(); - while (!authorized && authorityIt.hasNext()) { - if (getRequiredAuthorities().contains(authorityIt.next())) { - authorized = true; - } - } - return authorized; - } - - /** - * Test that all of the required authorities match the principal's authorities - * @param principalAuthorities the principal's granted authorities - * @return true if authorized - */ - private boolean isAuthorizedAll(Collection principalAuthorities) { - return principalAuthorities.containsAll(getRequiredAuthorities()); - } - /** * Gets required authorities * @return required authorities diff --git a/spring-webflow/src/test/java/org/springframework/webflow/security/SecurityFlowExecutionListenerTests.java b/spring-webflow/src/test/java/org/springframework/webflow/security/SecurityFlowExecutionListenerTests.java index 1f9973ec..daedb38b 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/security/SecurityFlowExecutionListenerTests.java +++ b/spring-webflow/src/test/java/org/springframework/webflow/security/SecurityFlowExecutionListenerTests.java @@ -32,31 +32,16 @@ public class SecurityFlowExecutionListenerTests extends TestCase { listener.sessionCreating(context, definition); } - public void testSessionCreatingAuthorized() { + public void testSessionCreatingWithSecurity() { SecurityFlowExecutionListener listener = new SecurityFlowExecutionListener(); RequestContext context = new MockRequestContext(); Flow flow = new Flow("flow"); - SecurityRule rule = getSecurityRuleAuthorized(); + SecurityRule rule = getSecurityRuleAnyAuthorized(); ((LocalAttributeMap) flow.getAttributes()).put(SecurityRule.SECURITY_AUTHORITY_ATTRIBUTE_NAME, rule); configureSecurityContext(); listener.sessionCreating(context, flow); } - public void testSessionCreatingDenied() { - SecurityFlowExecutionListener listener = new SecurityFlowExecutionListener(); - RequestContext context = new MockRequestContext(); - Flow flow = new Flow("flow"); - SecurityRule rule = getSecurityRuleDenied(); - ((LocalAttributeMap) flow.getAttributes()).put(SecurityRule.SECURITY_AUTHORITY_ATTRIBUTE_NAME, rule); - configureSecurityContext(); - try { - listener.sessionCreating(context, flow); - fail("expected AccessDeniedException"); - } catch (AccessDeniedException e) { - // success - } - } - public void testStateEnteringNoSecurity() { SecurityFlowExecutionListener listener = new SecurityFlowExecutionListener(); RequestContext context = new MockRequestContext(); @@ -65,33 +50,17 @@ public class SecurityFlowExecutionListenerTests extends TestCase { listener.stateEntering(context, state); } - public void testStateEnteringAuthorized() { + public void testStateEnteringWithSecurity() { SecurityFlowExecutionListener listener = new SecurityFlowExecutionListener(); RequestContext context = new MockRequestContext(); Flow flow = new Flow("flow"); ViewState state = new ViewState(flow, "view", new StubViewFactory()); - SecurityRule rule = getSecurityRuleAuthorized(); + SecurityRule rule = getSecurityRuleAllAuthorized(); ((LocalAttributeMap) state.getAttributes()).put(SecurityRule.SECURITY_AUTHORITY_ATTRIBUTE_NAME, rule); configureSecurityContext(); listener.stateEntering(context, state); } - public void testStateEnteringDenied() { - SecurityFlowExecutionListener listener = new SecurityFlowExecutionListener(); - RequestContext context = new MockRequestContext(); - Flow flow = new Flow("flow"); - ViewState state = new ViewState(flow, "view", new StubViewFactory()); - SecurityRule rule = getSecurityRuleDenied(); - ((LocalAttributeMap) state.getAttributes()).put(SecurityRule.SECURITY_AUTHORITY_ATTRIBUTE_NAME, rule); - configureSecurityContext(); - try { - listener.stateEntering(context, state); - fail("expected AccessDeniedException"); - } catch (AccessDeniedException e) { - // success - } - } - public void testTransitionExecutingNoSecurity() { SecurityFlowExecutionListener listener = new SecurityFlowExecutionListener(); RequestContext context = new MockRequestContext(); @@ -99,54 +68,95 @@ public class SecurityFlowExecutionListenerTests extends TestCase { listener.transitionExecuting(context, transition); } - public void testTransitionExecutingAuthorized() { + public void testTransitionExecutingWithSecurity() { SecurityFlowExecutionListener listener = new SecurityFlowExecutionListener(); RequestContext context = new MockRequestContext(); Transition transition = new Transition(new DefaultTargetStateResolver("target")); - SecurityRule rule = getSecurityRuleAuthorized(); + SecurityRule rule = getSecurityRuleAnyAuthorized(); ((LocalAttributeMap) transition.getAttributes()).put(SecurityRule.SECURITY_AUTHORITY_ATTRIBUTE_NAME, rule); configureSecurityContext(); listener.transitionExecuting(context, transition); } - public void testTransitionExecutingDenied() { - SecurityFlowExecutionListener listener = new SecurityFlowExecutionListener(); - RequestContext context = new MockRequestContext(); - Transition transition = new Transition(new DefaultTargetStateResolver("target")); - SecurityRule rule = getSecurityRuleDenied(); - ((LocalAttributeMap) transition.getAttributes()).put(SecurityRule.SECURITY_AUTHORITY_ATTRIBUTE_NAME, rule); + public void testDecideAnyAuthorized() { + configureSecurityContext(); + new SecurityFlowExecutionListener().decide(getSecurityRuleAnyAuthorized(), this); + } + + public void testDecideAnyDenied() { configureSecurityContext(); try { - listener.transitionExecuting(context, transition); - fail("expected AccessDeniedException"); + new SecurityFlowExecutionListener().decide(getSecurityRuleAnyDenied(), this); + fail("expected AccessDeniedExpetion"); } catch (AccessDeniedException e) { - // success + // we want this } } - private SecurityRule getSecurityRuleAuthorized() { - SecurityRule rule = new SecurityRule(); - rule.setComparisonType(SecurityRule.COMPARISON_ANY); - Collection authorities = new HashSet(); - authorities.add("ROLE_USER"); - rule.setRequiredAuthorities(authorities); - return rule; + public void testDecideAllAuthorized() { + configureSecurityContext(); + new SecurityFlowExecutionListener().decide(getSecurityRuleAllAuthorized(), this); } - private SecurityRule getSecurityRuleDenied() { - SecurityRule rule = new SecurityRule(); - rule.setComparisonType(SecurityRule.COMPARISON_ANY); - Collection authorities = new HashSet(); - authorities.add("ROLE_ANONYMOUS"); - rule.setRequiredAuthorities(authorities); - return rule; + public void testDecideAllDenied() { + configureSecurityContext(); + try { + new SecurityFlowExecutionListener().decide(getSecurityRuleAllDenied(), this); + fail("expected AccessDeniedExpetion"); + } catch (AccessDeniedException e) { + // we want this + } } private void configureSecurityContext() { - GrantedAuthority[] authorities = { new GrantedAuthorityImpl("ROLE_USER") }; - Authentication authentication = new TestingAuthenticationToken("test", "", authorities); SecurityContext sc = new SecurityContextImpl(); - sc.setAuthentication(authentication); + sc.setAuthentication(getAuthentication()); SecurityContextHolder.setContext(sc); } + + private SecurityRule getSecurityRuleAnyAuthorized() { + SecurityRule rule = new SecurityRule(); + rule.setComparisonType(SecurityRule.COMPARISON_ANY); + Collection authorities = new HashSet(); + authorities.add("ROLE_1"); + authorities.add("ROLE_A"); + rule.setRequiredAuthorities(authorities); + return rule; + } + + private SecurityRule getSecurityRuleAnyDenied() { + SecurityRule rule = new SecurityRule(); + rule.setComparisonType(SecurityRule.COMPARISON_ANY); + Collection authorities = new HashSet(); + authorities.add("ROLE_A"); + authorities.add("ROLE_B"); + rule.setRequiredAuthorities(authorities); + return rule; + } + + private SecurityRule getSecurityRuleAllAuthorized() { + SecurityRule rule = new SecurityRule(); + rule.setComparisonType(SecurityRule.COMPARISON_ALL); + Collection authorities = new HashSet(); + authorities.add("ROLE_1"); + authorities.add("ROLE_3"); + rule.setRequiredAuthorities(authorities); + return rule; + } + + private SecurityRule getSecurityRuleAllDenied() { + SecurityRule rule = new SecurityRule(); + rule.setComparisonType(SecurityRule.COMPARISON_ALL); + Collection authorities = new HashSet(); + authorities.add("ROLE_1"); + authorities.add("ROLE_A"); + rule.setRequiredAuthorities(authorities); + return rule; + } + + private Authentication getAuthentication() { + GrantedAuthority[] authorities = { new GrantedAuthorityImpl("ROLE_1"), new GrantedAuthorityImpl("ROLE_2"), + new GrantedAuthorityImpl("ROLE_3") }; + return new TestingAuthenticationToken("test", "", authorities); + } } diff --git a/spring-webflow/src/test/java/org/springframework/webflow/security/SecurityRuleTests.java b/spring-webflow/src/test/java/org/springframework/webflow/security/SecurityRuleTests.java index 83a0c008..90c58da9 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/security/SecurityRuleTests.java +++ b/spring-webflow/src/test/java/org/springframework/webflow/security/SecurityRuleTests.java @@ -2,65 +2,12 @@ package org.springframework.webflow.security; import java.util.ArrayList; import java.util.Collection; -import java.util.HashSet; import junit.framework.Assert; import junit.framework.TestCase; public class SecurityRuleTests extends TestCase { - public void testAuthorizedAll() { - SecurityRule rule = new SecurityRule(); - rule.setComparisonType(SecurityRule.COMPARISON_ALL); - Collection requiredAuthorities = new HashSet(); - requiredAuthorities.add("ROLE_USER"); - requiredAuthorities.add("ROLE_SUPERVISOR"); - rule.setRequiredAuthorities(requiredAuthorities); - Assert.assertTrue(rule.isAuthorized(getPrincipalAuthorities())); - } - - public void testAuthorizedAllFail() { - SecurityRule rule = new SecurityRule(); - rule.setComparisonType(SecurityRule.COMPARISON_ALL); - Collection requiredAuthorities = new HashSet(); - requiredAuthorities.add("ROLE_USER"); - requiredAuthorities.add("ROLE_ANONYMOUS"); - rule.setRequiredAuthorities(requiredAuthorities); - Assert.assertFalse(rule.isAuthorized(getPrincipalAuthorities())); - } - - public void testAuthorizedAny() { - SecurityRule rule = new SecurityRule(); - rule.setComparisonType(SecurityRule.COMPARISON_ANY); - Collection requiredAuthorities = new HashSet(); - requiredAuthorities.add("ROLE_USER"); - requiredAuthorities.add("ROLE_ANONYMOUS"); - rule.setRequiredAuthorities(requiredAuthorities); - Assert.assertTrue(rule.isAuthorized(getPrincipalAuthorities())); - } - - public void testAuthorizedAnyFail() { - SecurityRule rule = new SecurityRule(); - rule.setComparisonType(SecurityRule.COMPARISON_ANY); - Collection requiredAuthorities = new HashSet(); - requiredAuthorities.add("ROLE_NONE"); - requiredAuthorities.add("ROLE_ANONYMOUS"); - rule.setRequiredAuthorities(requiredAuthorities); - Assert.assertFalse(rule.isAuthorized(getPrincipalAuthorities())); - } - - public void testNonGrantedAuthorities() { - SecurityRule rule = new SecurityRule(); - rule.setComparisonType(SecurityRule.COMPARISON_ALL); - Collection requiredAuthorities = new HashSet(); - requiredAuthorities.add("ROLE_USER"); - requiredAuthorities.add("ROLE_ANONYMOUS"); - rule.setRequiredAuthorities(requiredAuthorities); - Collection nonGrantedAuthorities = rule.getNonGrantedAuthorities(getPrincipalAuthorities()); - Assert.assertEquals(1, nonGrantedAuthorities.size()); - Assert.assertTrue(nonGrantedAuthorities.contains("ROLE_ANONYMOUS")); - } - public void testConvertAuthoritiesToCommaSeparatedString() { Collection authorities = new ArrayList(); authorities.add("ROLE_USER"); @@ -77,11 +24,9 @@ public class SecurityRuleTests extends TestCase { Assert.assertTrue(authorities.contains("ROLE_ANONYMOUS")); } - private Collection getPrincipalAuthorities() { - Collection principalAuthorities = new HashSet(); - principalAuthorities.add("ROLE_USER"); - principalAuthorities.add("ROLE_SUPERVISOR"); - principalAuthorities.add("ROLE_NEVERTOBEHAD"); - return principalAuthorities; + public void testDefaultComparisonType() { + SecurityRule rule = new SecurityRule(); + Assert.assertTrue(rule.getComparisonType() == SecurityRule.COMPARISON_ANY); } + }