diff --git a/core/src/main/java/org/springframework/security/userdetails/hierarchicalroles/UserDetailsServiceWrapper.java b/core/src/main/java/org/springframework/security/userdetails/hierarchicalroles/UserDetailsServiceWrapper.java index d512818d16..97c2e990a9 100755 --- a/core/src/main/java/org/springframework/security/userdetails/hierarchicalroles/UserDetailsServiceWrapper.java +++ b/core/src/main/java/org/springframework/security/userdetails/hierarchicalroles/UserDetailsServiceWrapper.java @@ -25,6 +25,8 @@ import org.springframework.dao.DataAccessException; * instead of only the directly assigned authorities. * * @author Michael Mayr + * @deprecated use a {@link RoleHierarchyVoter} instead of populating the user Authentication object + * with the additional authorities. */ public class UserDetailsServiceWrapper implements UserDetailsService { diff --git a/core/src/main/java/org/springframework/security/userdetails/hierarchicalroles/UserDetailsWrapper.java b/core/src/main/java/org/springframework/security/userdetails/hierarchicalroles/UserDetailsWrapper.java index 3ca0d3ec60..0471c4c3ca 100755 --- a/core/src/main/java/org/springframework/security/userdetails/hierarchicalroles/UserDetailsWrapper.java +++ b/core/src/main/java/org/springframework/security/userdetails/hierarchicalroles/UserDetailsWrapper.java @@ -23,6 +23,7 @@ import org.springframework.security.userdetails.UserDetails; * delegated to the UserDetails implementation. * * @author Michael Mayr + * @deprecated use a {@link RoleHierarchyVoter} instead. */ public class UserDetailsWrapper implements UserDetails { diff --git a/core/src/main/java/org/springframework/security/vote/RoleHierarchyVoter.java b/core/src/main/java/org/springframework/security/vote/RoleHierarchyVoter.java new file mode 100644 index 0000000000..1ce679da21 --- /dev/null +++ b/core/src/main/java/org/springframework/security/vote/RoleHierarchyVoter.java @@ -0,0 +1,29 @@ +package org.springframework.security.vote; + +import org.springframework.security.Authentication; +import org.springframework.security.GrantedAuthority; +import org.springframework.security.userdetails.hierarchicalroles.RoleHierarchy; +import org.springframework.util.Assert; + +/** + * Extended RoleVoter which uses a {@link RoleHierarchy} definition to determine the + * roles allocated to the current user before voting. + * + * @author Luke Taylor + * @since 2.0.4 + */ +public class RoleHierarchyVoter extends RoleVoter { + private RoleHierarchy roleHierarchy = null; + + public RoleHierarchyVoter(RoleHierarchy roleHierarchy) { + Assert.notNull(roleHierarchy, "RoleHierarchy must not be null"); + this.roleHierarchy = roleHierarchy; + } + + /** + * Calls the RoleHierarchy to obtain the complete set of user authorities. + */ + GrantedAuthority[] extractAuthorities(Authentication authentication) { + return roleHierarchy.getReachableGrantedAuthorities(authentication.getAuthorities()); + } +} diff --git a/core/src/main/java/org/springframework/security/vote/RoleVoter.java b/core/src/main/java/org/springframework/security/vote/RoleVoter.java index d36c2d75a9..ddddbcf2e6 100644 --- a/core/src/main/java/org/springframework/security/vote/RoleVoter.java +++ b/core/src/main/java/org/springframework/security/vote/RoleVoter.java @@ -95,6 +95,7 @@ public class RoleVoter implements AccessDecisionVoter { public int vote(Authentication authentication, Object object, ConfigAttributeDefinition config) { int result = ACCESS_ABSTAIN; Iterator iter = config.getConfigAttributes().iterator(); + GrantedAuthority[] authorities = extractAuthorities(authentication); while (iter.hasNext()) { ConfigAttribute attribute = (ConfigAttribute) iter.next(); @@ -102,7 +103,6 @@ public class RoleVoter implements AccessDecisionVoter { if (this.supports(attribute)) { result = ACCESS_DENIED; - GrantedAuthority[] authorities = authentication.getAuthorities(); // Attempt to find a matching granted authority for (int i = 0; i < authorities.length; i++) { if (attribute.getAttribute().equals(authorities[i].getAuthority())) { @@ -114,4 +114,8 @@ public class RoleVoter implements AccessDecisionVoter { return result; } + + GrantedAuthority[] extractAuthorities(Authentication authentication) { + return authentication.getAuthorities(); + } } diff --git a/core/src/test/java/org/springframework/security/ldap/SpringSecurityAuthenticationSourceTests.java b/core/src/test/java/org/springframework/security/ldap/SpringSecurityAuthenticationSourceTests.java index 3f26ce4abd..b465498d77 100644 --- a/core/src/test/java/org/springframework/security/ldap/SpringSecurityAuthenticationSourceTests.java +++ b/core/src/test/java/org/springframework/security/ldap/SpringSecurityAuthenticationSourceTests.java @@ -44,8 +44,7 @@ public class SpringSecurityAuthenticationSourceTests { @Test(expected=IllegalArgumentException.class) public void getPrincipalRejectsNonLdapUserDetailsObject() { AuthenticationSource source = new SpringSecurityAuthenticationSource(); - SecurityContextHolder.getContext().setAuthentication( - new TestingAuthenticationToken(new Object(), "password", null)); + SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken(new Object(), "password")); source.getPrincipal(); } @@ -53,8 +52,7 @@ public class SpringSecurityAuthenticationSourceTests { @Test public void expectedCredentialsAreReturned() { AuthenticationSource source = new SpringSecurityAuthenticationSource(); - SecurityContextHolder.getContext().setAuthentication( - new TestingAuthenticationToken(new Object(), "password", null)); + SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken(new Object(), "password")); assertEquals("password", source.getCredentials()); } @@ -66,7 +64,7 @@ public class SpringSecurityAuthenticationSourceTests { user.setDn(new DistinguishedName("uid=joe,ou=users")); AuthenticationSource source = new SpringSecurityAuthenticationSource(); SecurityContextHolder.getContext().setAuthentication( - new TestingAuthenticationToken(user.createUserDetails(), null, null)); + new TestingAuthenticationToken(user.createUserDetails(), null)); assertEquals("uid=joe, ou=users", source.getPrincipal()); } diff --git a/core/src/main/java/org/springframework/security/providers/TestingAuthenticationProvider.java b/core/src/test/java/org/springframework/security/providers/TestingAuthenticationProvider.java similarity index 100% rename from core/src/main/java/org/springframework/security/providers/TestingAuthenticationProvider.java rename to core/src/test/java/org/springframework/security/providers/TestingAuthenticationProvider.java diff --git a/core/src/main/java/org/springframework/security/providers/TestingAuthenticationToken.java b/core/src/test/java/org/springframework/security/providers/TestingAuthenticationToken.java similarity index 80% rename from core/src/main/java/org/springframework/security/providers/TestingAuthenticationToken.java rename to core/src/test/java/org/springframework/security/providers/TestingAuthenticationToken.java index 4708e2ac5f..4d4ae97206 100644 --- a/core/src/main/java/org/springframework/security/providers/TestingAuthenticationToken.java +++ b/core/src/test/java/org/springframework/security/providers/TestingAuthenticationToken.java @@ -16,6 +16,7 @@ package org.springframework.security.providers; import org.springframework.security.GrantedAuthority; +import org.springframework.security.util.AuthorityUtils; /** @@ -34,6 +35,17 @@ public class TestingAuthenticationToken extends AbstractAuthenticationToken { //~ Constructors =================================================================================================== + public TestingAuthenticationToken(Object principal, Object credentials) { + super(null); + this.principal = principal; + this.credentials = credentials; + } + + + public TestingAuthenticationToken(Object principal, Object credentials, String... authorities) { + this(principal, credentials, AuthorityUtils.stringArrayToAuthorityArray(authorities)); + } + public TestingAuthenticationToken(Object principal, Object credentials, GrantedAuthority[] authorities) { super(authorities); this.principal = principal; diff --git a/core/src/test/java/org/springframework/security/providers/TestingAuthenticationTokenTests.java b/core/src/test/java/org/springframework/security/providers/TestingAuthenticationTokenTests.java deleted file mode 100644 index 638121edab..0000000000 --- a/core/src/test/java/org/springframework/security/providers/TestingAuthenticationTokenTests.java +++ /dev/null @@ -1,77 +0,0 @@ -/* Copyright 2004, 2005, 2006 Acegi Technology Pty Limited - * - * 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.security.providers; - -import junit.framework.TestCase; - -import org.springframework.security.GrantedAuthority; -import org.springframework.security.GrantedAuthorityImpl; - - -/** - * Tests {@link TestingAuthenticationToken}. - * - * @author Ben Alex - * @version $Id$ - */ -public class TestingAuthenticationTokenTests extends TestCase { - //~ Constructors =================================================================================================== - - public TestingAuthenticationTokenTests() { - super(); - } - - public TestingAuthenticationTokenTests(String arg0) { - super(arg0); - } - - //~ Methods ======================================================================================================== - - public static void main(String[] args) { - junit.textui.TestRunner.run(TestingAuthenticationTokenTests.class); - } - - public final void setUp() throws Exception { - super.setUp(); - } - - public void testAuthenticated() { - TestingAuthenticationToken token = new TestingAuthenticationToken("Test", "Password", null); - assertTrue(!token.isAuthenticated()); - token.setAuthenticated(true); - assertTrue(token.isAuthenticated()); - } - - public void testGetters() { - TestingAuthenticationToken token = new TestingAuthenticationToken("Test", "Password", - new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl("ROLE_TWO")}); - assertEquals("Test", token.getPrincipal()); - assertEquals("Password", token.getCredentials()); - assertEquals("ROLE_ONE", token.getAuthorities()[0].getAuthority()); - assertEquals("ROLE_TWO", token.getAuthorities()[1].getAuthority()); - } - - public void testNoArgConstructorDoesntExist() { - Class clazz = TestingAuthenticationToken.class; - - try { - clazz.getDeclaredConstructor((Class[]) null); - fail("Should have thrown NoSuchMethodException"); - } catch (NoSuchMethodException expected) { - assertTrue(true); - } - } -} diff --git a/core/src/test/java/org/springframework/security/ui/SessionFixationProtectionFilterTests.java b/core/src/test/java/org/springframework/security/ui/SessionFixationProtectionFilterTests.java index 3adab7834b..0db018f924 100644 --- a/core/src/test/java/org/springframework/security/ui/SessionFixationProtectionFilterTests.java +++ b/core/src/test/java/org/springframework/security/ui/SessionFixationProtectionFilterTests.java @@ -74,6 +74,6 @@ public class SessionFixationProtectionFilterTests { } private void authenticateUser() { - SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken("user", "pass", null)); + SecurityContextHolder.getContext().setAuthentication(new TestingAuthenticationToken("user", "pass")); } } diff --git a/core/src/test/java/org/springframework/security/ui/rememberme/RememberMeProcessingFilterTests.java b/core/src/test/java/org/springframework/security/ui/rememberme/RememberMeProcessingFilterTests.java index ccba0c3a69..14ae6da118 100644 --- a/core/src/test/java/org/springframework/security/ui/rememberme/RememberMeProcessingFilterTests.java +++ b/core/src/test/java/org/springframework/security/ui/rememberme/RememberMeProcessingFilterTests.java @@ -160,7 +160,7 @@ public class RememberMeProcessingFilterTests extends TestCase { public void testOnunsuccessfulLoginIsCalledWhenProviderRejectsAuth() throws Exception { Authentication remembered = new TestingAuthenticationToken("remembered", "password", new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_REMEMBERED")}); - final Authentication failedAuth = new TestingAuthenticationToken("failed", "", null); + final Authentication failedAuth = new TestingAuthenticationToken("failed", ""); RememberMeProcessingFilter filter = new RememberMeProcessingFilter() { protected void onUnsuccessfulAuthentication(HttpServletRequest request, HttpServletResponse response, AuthenticationException failed) { diff --git a/core/src/test/java/org/springframework/security/vote/RoleHierarchyVoterTests.java b/core/src/test/java/org/springframework/security/vote/RoleHierarchyVoterTests.java new file mode 100644 index 0000000000..6c02d20046 --- /dev/null +++ b/core/src/test/java/org/springframework/security/vote/RoleHierarchyVoterTests.java @@ -0,0 +1,24 @@ +package org.springframework.security.vote; + +import static org.junit.Assert.*; + +import org.junit.Test; +import org.springframework.security.ConfigAttributeDefinition; +import org.springframework.security.providers.TestingAuthenticationToken; +import org.springframework.security.userdetails.hierarchicalroles.RoleHierarchyImpl; + +public class RoleHierarchyVoterTests { + + @Test + public void hierarchicalRoleIsIncludedInDecision() { + RoleHierarchyImpl roleHierarchyImpl = new RoleHierarchyImpl(); + roleHierarchyImpl.setHierarchy("ROLE_A > ROLE_B"); + + // User has role A, role B is required + TestingAuthenticationToken auth = new TestingAuthenticationToken("user", "password", "ROLE_A"); + RoleHierarchyVoter voter = new RoleHierarchyVoter(roleHierarchyImpl); + ConfigAttributeDefinition config = new ConfigAttributeDefinition("ROLE_B"); + + assertEquals(RoleHierarchyVoter.ACCESS_GRANTED, voter.vote(auth, new Object(), config)); + } +}