From b51b30ed016dd42ffc2a0f03a32bdeceb64d313e Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 4 Jun 2015 15:18:28 -0400 Subject: [PATCH] Upgrade to Spring Security 4 and Spring Framework 4.1 Issue: SWF-1657, SWF-1663 --- build.gradle | 4 +- .../faces/security/FaceletsAuthorizeTag.java | 52 +++++++++++++- .../security/FaceletsAuthorizeTagUtils.java | 8 +-- .../security/FaceletsAuthorizeTagTests.java | 70 +++++++++++++++++++ .../SecurityFlowExecutionListener.java | 58 +++++++++++---- .../SecurityFlowExecutionListenerTests.java | 6 +- 6 files changed, 173 insertions(+), 25 deletions(-) create mode 100644 spring-faces/src/test/java/org/springframework/faces/security/FaceletsAuthorizeTagTests.java diff --git a/build.gradle b/build.gradle index 65da9185..1b96273c 100644 --- a/build.gradle +++ b/build.gradle @@ -71,8 +71,8 @@ configure(subprojects.findAll { } subproject.ext { - springVersion = "4.0.6.RELEASE" - springSecurityVersion = "3.2.4.RELEASE" + springVersion = "4.1.6.RELEASE" + springSecurityVersion = "4.0.1.RELEASE" slf4jVersion = "1.7.5" log4jVersion = "1.2.17" } diff --git a/spring-faces/src/main/java/org/springframework/faces/security/FaceletsAuthorizeTag.java b/spring-faces/src/main/java/org/springframework/faces/security/FaceletsAuthorizeTag.java index e38eae8d..bdff4508 100644 --- a/spring-faces/src/main/java/org/springframework/faces/security/FaceletsAuthorizeTag.java +++ b/spring-faces/src/main/java/org/springframework/faces/security/FaceletsAuthorizeTag.java @@ -1,5 +1,5 @@ /* - * Copyright 2004-2013 the original author or authors. + * Copyright 2004-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,6 +25,8 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import org.springframework.security.taglibs.authz.AbstractAuthorizeTag; +import org.springframework.util.ObjectUtils; +import org.springframework.util.StringUtils; /** * A concrete implementation of {@link AbstractAuthorizeTag} for use with standard Facelets rendering technology. @@ -76,7 +78,53 @@ public class FaceletsAuthorizeTag extends AbstractAuthorizeTag { return (ServletContext) FacesContext.getCurrentInstance().getExternalContext().getContext(); } - /*---- Pirvate helper methods ----*/ + void setIfAllGranted(String ifAllGranted) { + String[] roles = StringUtils.tokenizeToStringArray(ifAllGranted, ","); + if (!ObjectUtils.isEmpty(roles)) { + String expression = toHasRoleExpression(roles); + setAccess(getAccess() != null ? getAccess() + " and " + expression: expression); + } + } + + void setIfAnyGranted(String ifAnyGranted) { + String[] roles = StringUtils.tokenizeToStringArray(ifAnyGranted, ","); + if (!ObjectUtils.isEmpty(roles)) { + String expression = toHasAnyRoleExpression(roles, false); + setAccess(getAccess() != null ? getAccess() + " and " + expression: expression); + } + } + + void setIfNotGranted(String ifNotGranted) { + String[] roles = StringUtils.tokenizeToStringArray(ifNotGranted, ","); + if (!ObjectUtils.isEmpty(roles)) { + String expression = toHasAnyRoleExpression(roles, true); + setAccess(getAccess() != null ? getAccess() + " and " + expression: expression); + } + } + + private static String toHasRoleExpression(String[] roles) { + StringBuilder expression = new StringBuilder(); + boolean insertSeparator = false; + for (String role : roles) { + expression.append(insertSeparator ? " and " : ""); + expression.append("hasRole('").append(role).append("')"); + insertSeparator = true; + } + return expression.toString(); + } + + private static String toHasAnyRoleExpression(String[] roles, boolean negate) { + StringBuilder expression = new StringBuilder(); + expression.append(negate ? "!" : ""); + expression.append("hasAnyRole("); + boolean insertSeparator = false; + for (String role : roles) { + expression.append(insertSeparator ? "," : ""); + expression.append("'").append(role).append("'"); + insertSeparator = true; + } + return expression.append(")").toString(); + } private String getAttributeValue(FaceletContext faceletContext, TagAttribute tagAttribute, boolean evaluate) { String value = null; diff --git a/spring-faces/src/main/java/org/springframework/faces/security/FaceletsAuthorizeTagUtils.java b/spring-faces/src/main/java/org/springframework/faces/security/FaceletsAuthorizeTagUtils.java index 565d4367..e5f80473 100644 --- a/spring-faces/src/main/java/org/springframework/faces/security/FaceletsAuthorizeTagUtils.java +++ b/spring-faces/src/main/java/org/springframework/faces/security/FaceletsAuthorizeTagUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2004-2010 the original author or authors. + * Copyright 2004-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -34,7 +34,7 @@ public abstract class FaceletsAuthorizeTagUtils { public static boolean areAllGranted(String authorities) throws IOException { FaceletsAuthorizeTag authorizeTag = new FaceletsAuthorizeTag(); authorizeTag.setIfAllGranted(authorities); - return authorizeTag.authorizeUsingGrantedAuthorities(); + return authorizeTag.authorize(); } /** @@ -45,7 +45,7 @@ public abstract class FaceletsAuthorizeTagUtils { public static boolean areAnyGranted(String authorities) throws IOException { FaceletsAuthorizeTag authorizeTag = new FaceletsAuthorizeTag(); authorizeTag.setIfAnyGranted(authorities); - return authorizeTag.authorizeUsingGrantedAuthorities(); + return authorizeTag.authorize(); } /** @@ -56,7 +56,7 @@ public abstract class FaceletsAuthorizeTagUtils { public static boolean areNotGranted(String authorities) throws IOException { FaceletsAuthorizeTag authorizeTag = new FaceletsAuthorizeTag(); authorizeTag.setIfNotGranted(authorities); - return authorizeTag.authorizeUsingGrantedAuthorities(); + return authorizeTag.authorize(); } /** diff --git a/spring-faces/src/test/java/org/springframework/faces/security/FaceletsAuthorizeTagTests.java b/spring-faces/src/test/java/org/springframework/faces/security/FaceletsAuthorizeTagTests.java new file mode 100644 index 00000000..c9cd3ad5 --- /dev/null +++ b/spring-faces/src/test/java/org/springframework/faces/security/FaceletsAuthorizeTagTests.java @@ -0,0 +1,70 @@ +/* + * Copyright 2002-2015 the original author or authors. + * + * 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 org.springframework.faces.security; + +import junit.framework.TestCase; + +/** + * Unit tests for {@link FaceletsAuthorizeTag}. + * @author Rossen Stoyanchev + */ +public class FaceletsAuthorizeTagTests extends TestCase { + + public void testIfAllGrantedWithOneRole() throws Exception { + FaceletsAuthorizeTag tag = new FaceletsAuthorizeTag(); + tag.setIfAllGranted("ROLE_A"); + assertEquals("hasRole('ROLE_A')", tag.getAccess()); + } + + public void testIfAllGrantedWithMultipleRoles() throws Exception { + FaceletsAuthorizeTag tag = new FaceletsAuthorizeTag(); + tag.setIfAllGranted("ROLE_A, ROLE_B, ROLE_C"); + assertEquals("hasRole('ROLE_A') and hasRole('ROLE_B') and hasRole('ROLE_C')", tag.getAccess()); + } + + public void testIfAnyGrantedWithOneRole() throws Exception { + FaceletsAuthorizeTag tag = new FaceletsAuthorizeTag(); + tag.setIfAnyGranted("ROLE_A"); + assertEquals("hasAnyRole('ROLE_A')", tag.getAccess()); + } + + public void testIfAnyGrantedWithMultipleRole() throws Exception { + FaceletsAuthorizeTag tag = new FaceletsAuthorizeTag(); + tag.setIfAnyGranted("ROLE_A, ROLE_B, ROLE_C"); + assertEquals("hasAnyRole('ROLE_A','ROLE_B','ROLE_C')", tag.getAccess()); + } + + public void testIfNoneGrantedWithOneRole() throws Exception { + FaceletsAuthorizeTag tag = new FaceletsAuthorizeTag(); + tag.setIfNotGranted("ROLE_A"); + assertEquals("!hasAnyRole('ROLE_A')", tag.getAccess()); + } + + public void testIfNoneGrantedWithMultipleRole() throws Exception { + FaceletsAuthorizeTag tag = new FaceletsAuthorizeTag(); + tag.setIfNotGranted("ROLE_A, ROLE_B, ROLE_C"); + assertEquals("!hasAnyRole('ROLE_A','ROLE_B','ROLE_C')", tag.getAccess()); + } + + public void testIfAllAnyNotGranted() throws Exception { + FaceletsAuthorizeTag tag = new FaceletsAuthorizeTag(); + tag.setIfAllGranted("ROLE_A"); + tag.setIfAnyGranted("ROLE_B"); + tag.setIfNotGranted("ROLE_C"); + assertEquals("hasRole('ROLE_A') and hasAnyRole('ROLE_B') and !hasAnyRole('ROLE_C')", tag.getAccess()); + } + +} 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 e35d4618..2583e0db 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,5 +1,5 @@ /* - * Copyright 2004-2012 the original author or authors. + * Copyright 2004-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,10 +15,12 @@ */ package org.springframework.webflow.security; +import java.lang.reflect.Constructor; import java.util.ArrayList; import java.util.Collection; import java.util.List; +import org.springframework.beans.DirectFieldAccessor; import org.springframework.security.access.AccessDecisionManager; import org.springframework.security.access.AccessDecisionVoter; import org.springframework.security.access.ConfigAttribute; @@ -29,6 +31,7 @@ import org.springframework.security.access.vote.RoleVoter; import org.springframework.security.access.vote.UnanimousBased; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.util.ClassUtils; import org.springframework.webflow.definition.FlowDefinition; import org.springframework.webflow.definition.StateDefinition; import org.springframework.webflow.definition.TransitionDefinition; @@ -43,6 +46,8 @@ import org.springframework.webflow.execution.RequestContext; */ public class SecurityFlowExecutionListener extends FlowExecutionListenerAdapter { + private static final boolean SPRING_SECURITY_3_PRESENT = ClassUtils.hasConstructor(AffirmativeBased.class); + private AccessDecisionManager accessDecisionManager; /** @@ -50,7 +55,7 @@ public class SecurityFlowExecutionListener extends FlowExecutionListenerAdapter * @return the decision manager */ public AccessDecisionManager getAccessDecisionManager() { - return accessDecisionManager; + return this.accessDecisionManager; } /** @@ -95,18 +100,43 @@ public class SecurityFlowExecutionListener extends FlowExecutionListenerAdapter if (accessDecisionManager != null) { accessDecisionManager.decide(authentication, object, configAttributes); } else { - 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); - abstractAccessDecisionManager.decide(authentication, object, configAttributes); + AccessDecisionManager manager = (SPRING_SECURITY_3_PRESENT ? + createManagerWithSpringSecurity3(rule) : createManager(rule)); + manager.decide(authentication, object, configAttributes); + } + } + + private AbstractAccessDecisionManager createManager(SecurityRule rule) { + List> voters = new ArrayList>(); + voters.add(new RoleVoter()); + if (rule.getComparisonType() == SecurityRule.COMPARISON_ANY) { + return new AffirmativeBased(voters); + } else if (rule.getComparisonType() == SecurityRule.COMPARISON_ALL) { + return new UnanimousBased(voters); + } else { + throw new IllegalStateException("Unknown SecurityRule match type: " + rule.getComparisonType()); + } + } + + private AbstractAccessDecisionManager createManagerWithSpringSecurity3(SecurityRule rule) { + List voters = new ArrayList(); + voters.add(new RoleVoter()); + Class managerType; + if (rule.getComparisonType() == SecurityRule.COMPARISON_ANY) { + managerType = AffirmativeBased.class; + } else if (rule.getComparisonType() == SecurityRule.COMPARISON_ALL) { + managerType = UnanimousBased.class; + } else { + throw new IllegalStateException("Unknown SecurityRule match type: " + rule.getComparisonType()); + } + try { + Constructor constructor = managerType.getConstructor(); + AbstractAccessDecisionManager manager = (AbstractAccessDecisionManager) constructor.newInstance(); + new DirectFieldAccessor(manager).setPropertyValue("decisionVoters", voters); + return manager; + } + catch (Throwable ex) { + throw new IllegalStateException("Failed to initialize AccessDecisionManager", ex); } } 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 bd308e92..b64c0b09 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 @@ -11,7 +11,7 @@ import org.springframework.security.access.AccessDeniedException; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; -import org.springframework.security.core.authority.GrantedAuthorityImpl; +import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.context.SecurityContextImpl; @@ -156,8 +156,8 @@ public class SecurityFlowExecutionListenerTests extends TestCase { } private Authentication getAuthentication() { - List authorities = Arrays. asList(new GrantedAuthorityImpl("ROLE_1"), - new GrantedAuthorityImpl("ROLE_2"), new GrantedAuthorityImpl("ROLE_3")); + List authorities = Arrays. asList(new SimpleGrantedAuthority("ROLE_1"), + new SimpleGrantedAuthority("ROLE_2"), new SimpleGrantedAuthority("ROLE_3")); return new UsernamePasswordAuthenticationToken("test", "", authorities); } }