SEC-552: Replaced authorites populators in CAS and OpenID with a plain UserDetailsService

core/src/main/java/org/springframework/security/providers/DaoAuthoritiesPopulator.java

@@ -1,54 +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 org.springframework.beans.factory.InitializingBean;
-import org.springframework.security.AuthenticationException;
-import org.springframework.security.userdetails.UserDetails;
-import org.springframework.security.userdetails.UserDetailsService;
-import org.springframework.util.Assert;
-
-/**
- * Populates the CAS authorities via an {@link org.springframework.security.userdetails.UserDetailsService}.<P>The additional information (username,
- * password, enabled status etc)  an <code>AuthenticationDao</code> implementation provides about  a <code>User</code>
- * is ignored. Only the <code>GrantedAuthority</code>s are relevant to this class.</p>
- *
- * @author Ben Alex
- * @version $Id$
- */
-public class DaoAuthoritiesPopulator implements AuthoritiesPopulator, InitializingBean {
-    //~ Instance fields ================================================================================================
-
-    private UserDetailsService userDetailsService;
-
-    //~ Methods ========================================================================================================
-
-    public void afterPropertiesSet() throws Exception {
-        Assert.notNull(this.userDetailsService, "A UserDetailsService must be set");
-    }
-
-    public UserDetails getUserDetails(String casUserId)
-        throws AuthenticationException {
-        return this.userDetailsService.loadUserByUsername(casUserId);
-    }
-
-    public UserDetailsService getUserDetailsService() {
-        return userDetailsService;
-    }
-
-    public void setUserDetailsService(UserDetailsService userDetailsService) {
-        this.userDetailsService = userDetailsService;
-    }
-}

core/src/main/java/org/springframework/security/providers/AuthoritiesPopulator.java → core/src/main/java/org/springframework/security/providers/UserDetailsService.java

@@ -50,7 +50,7 @@ import org.springframework.security.userdetails.UserDetails;
  * @author Ray Krueger
  * @version $Id$
  */
-public interface AuthoritiesPopulator {
+public interface UserDetailsService {
     /**
      * Obtains the granted authorities for the specified user.<P>May throw any
      * <code>AuthenticationException</code> or return <code>null</code> if the authorities are unavailable.</p>

core/src/main/java/org/springframework/security/providers/cas/CasAuthenticationProvider.java

@@ -22,12 +22,12 @@ import org.springframework.security.BadCredentialsException;
 
 import org.springframework.security.providers.AuthenticationProvider;
 import org.springframework.security.providers.UsernamePasswordAuthenticationToken;
-import org.springframework.security.providers.AuthoritiesPopulator;
 import org.springframework.security.providers.cas.cache.NullStatelessTicketCache;
 
 import org.springframework.security.ui.cas.CasProcessingFilter;
 
 import org.springframework.security.userdetails.UserDetails;
+import org.springframework.security.userdetails.UserDetailsService;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -60,7 +60,7 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
 
     //~ Instance fields ================================================================================================
 
-    private AuthoritiesPopulator casAuthoritiesPopulator;
+    private UserDetailsService userDetailsService;
     private CasProxyDecider casProxyDecider;
     protected MessageSourceAccessor messages = SpringSecurityMessageSource.getAccessor();
     private StatelessTicketCache statelessTicketCache = new NullStatelessTicketCache();
@@ -70,7 +70,7 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
     //~ Methods ========================================================================================================
 
 	public void afterPropertiesSet() throws Exception {
-        Assert.notNull(this.casAuthoritiesPopulator, "A casAuthoritiesPopulator must be set");
+        Assert.notNull(this.userDetailsService, "A userDetailsService must be set");
         Assert.notNull(this.ticketValidator, "A ticketValidator must be set");
         Assert.notNull(this.casProxyDecider, "A casProxyDecider must be set");
         Assert.notNull(this.statelessTicketCache, "A statelessTicketCache must be set");
@@ -141,19 +141,19 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
         this.casProxyDecider.confirmProxyListTrusted(response.getProxyList());
 
         // Lookup user details
-        UserDetails userDetails = this.casAuthoritiesPopulator.getUserDetails(response.getUser());
+        UserDetails userDetails = userDetailsService.loadUserByUsername(response.getUser());
 
         // Construct CasAuthenticationToken
         return new CasAuthenticationToken(this.key, userDetails, authentication.getCredentials(),
             userDetails.getAuthorities(), userDetails, response.getProxyList(), response.getProxyGrantingTicketIou());
     }
 
-    protected AuthoritiesPopulator getCasAuthoritiesPopulator() {
-        return casAuthoritiesPopulator;
+    protected UserDetailsService getUserDetailsService() {
+        return userDetailsService;
     }
 
-    public void setCasAuthoritiesPopulator(AuthoritiesPopulator casAuthoritiesPopulator) {
-        this.casAuthoritiesPopulator = casAuthoritiesPopulator;
+    public void setUserDetailsService(UserDetailsService userDetailsService) {
+        this.userDetailsService = userDetailsService;
     }
 
     public CasProxyDecider getCasProxyDecider() {

core/src/main/java/org/springframework/security/providers/cas/CasAuthenticationToken.java

@@ -53,10 +53,10 @@ public class CasAuthenticationToken extends AbstractAuthenticationToken implemen
      * @param principal typically the UserDetails object (cannot  be <code>null</code>)
      * @param credentials the service/proxy ticket ID from CAS (cannot be
      *        <code>null</code>)
-     * @param authorities the authorities granted to the user (from {@link
-     *        CasAuthoritiesPopulator}) (cannot be <code>null</code>)
-     * @param userDetails the user details (from {@link
-     *        CasAuthoritiesPopulator}) (cannot be <code>null</code>)
+     * @param authorities the authorities granted to the user (from the {@link
+     *        org.springframework.security.userdetails.UserDetailsService}) (cannot be <code>null</code>)
+     * @param userDetails the user details (from the {@link
+     *        org.springframework.security.userdetails.UserDetailsService}) (cannot be <code>null</code>)
      * @param proxyList the list of proxies from CAS (cannot be
      *        <code>null</code>)
      * @param proxyGrantingTicketIou the PGT-IOU ID from CAS (cannot be

core/src/main/java/org/springframework/security/providers/cas/CasAuthoritiesPopulator.java

@@ -1,50 +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.cas;
-
-import org.springframework.security.providers.AuthoritiesPopulator;
-
-
-/**
- * <p>
- * <i>Backwards compatible extension to the {@link AuthoritiesPopulator} interface.
- * This interface has usefulness outside of the CAS usecase. Thus, the {@link AuthoritiesPopulator}
- * interface was refactored in.</i>
- * </p>
- * <p>
- * Populates the <code>UserDetails</code> associated with a CAS authenticated
- * user.
- * </p>
- *
- * <p>
- * CAS does not provide the authorities (roles) granted to a user. It merely
- * authenticates their identity. As Spring Security needs
- * to know the authorities granted to a user in order to construct a valid
- * <code>Authentication</code> object, implementations of this interface will
- * provide this information.
- * </p>
- *
- * <p>
- * Implementations should not perform any caching. They will only be called
- * when a refresh is required.
- * </p>
- *
- * @author Ben Alex
- * @version $Id$
- */
-public interface CasAuthoritiesPopulator extends AuthoritiesPopulator {
-
-}

core/src/main/java/org/springframework/security/providers/cas/populator/DaoCasAuthoritiesPopulator.java

@@ -1,35 +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.cas.populator;
-
-import org.springframework.beans.factory.InitializingBean;
-import org.springframework.security.providers.DaoAuthoritiesPopulator;
-
-
-/**
- * Backwards compatible placeholder.
- * This class will be removed, use {@link DaoAuthoritiesPopulator} instead.
- * 
- * @deprecated Use {@link org.springframework.security.providers.DaoAuthoritiesPopulator}
- * @author Ben Alex
- * @version $Id$
- */
-public class DaoCasAuthoritiesPopulator extends DaoAuthoritiesPopulator implements InitializingBean {
-
-    public void afterPropertiesSet() throws Exception {
-        super.afterPropertiesSet();
-    }
-}

core/src/main/java/org/springframework/security/providers/cas/populator/package.html

@@ -1,5 +0,0 @@
-<html>
-<body>
-Implementations that populate GrantedAuthority[]s of CAS authentications.
-</body>
-</html>

core/src/test/java/org/springframework/security/providers/cas/CasAuthenticationProviderTests.java

@@ -15,8 +15,6 @@
 
 package org.springframework.security.providers.cas;
 
-import junit.framework.TestCase;
-
 import org.springframework.security.Authentication;
 import org.springframework.security.AuthenticationException;
 import org.springframework.security.BadCredentialsException;
@@ -31,12 +29,16 @@ import org.springframework.security.ui.cas.CasProcessingFilter;
 
 import org.springframework.security.userdetails.User;
 import org.springframework.security.userdetails.UserDetails;
+import org.springframework.security.userdetails.UserDetailsService;
 
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Vector;
 
+import org.junit.Test;
+import static org.junit.Assert.*;
+
 
 /**
  * Tests {@link CasAuthenticationProvider}.
@@ -44,16 +46,7 @@ import java.util.Vector;
  * @author Ben Alex
  * @version $Id$
  */
-public class CasAuthenticationProviderTests extends TestCase {
-    //~ Constructors ===================================================================================================
-
-    public CasAuthenticationProviderTests() {
-    }
-
-    public CasAuthenticationProviderTests(String arg0) {
-        super(arg0);
-    }
-
+public class CasAuthenticationProviderTests {
     //~ Methods ========================================================================================================
 
     private UserDetails makeUserDetails() {
@@ -66,13 +59,10 @@ public class CasAuthenticationProviderTests extends TestCase {
             new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_A"), new GrantedAuthorityImpl("ROLE_B")});
     }
 
-    public final void setUp() throws Exception {
-        super.setUp();
-    }
-
-    public void testAuthenticateStateful() throws Exception {
+    @Test
+    public void statefulAuthenticationIsSuccessful() throws Exception {
         CasAuthenticationProvider cap = new CasAuthenticationProvider();
-        cap.setCasAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        cap.setUserDetailsService(new MockAuthoritiesPopulator());
         cap.setCasProxyDecider(new MockProxyDecider(true));
         cap.setKey("qwerty");
 
@@ -111,9 +101,10 @@ public class CasAuthenticationProviderTests extends TestCase {
         assertEquals(result, laterResult);
     }
 
-    public void testAuthenticateStateless() throws Exception {
+    @Test
+    public void statelessAuthenticationIsSuccessful() throws Exception {
         CasAuthenticationProvider cap = new CasAuthenticationProvider();
-        cap.setCasAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        cap.setUserDetailsService(new MockAuthoritiesPopulator());
         cap.setCasProxyDecider(new MockProxyDecider(true));
         cap.setKey("qwerty");
 
@@ -147,9 +138,10 @@ public class CasAuthenticationProviderTests extends TestCase {
         assertEquals("ST-456", newResult.getCredentials());
     }
 
-    public void testDetectsAMissingTicketId() throws Exception {
+    @Test(expected = BadCredentialsException.class)
+    public void missingTicketIdIsDetected() throws Exception {
         CasAuthenticationProvider cap = new CasAuthenticationProvider();
-        cap.setCasAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        cap.setUserDetailsService(new MockAuthoritiesPopulator());
         cap.setCasProxyDecider(new MockProxyDecider(true));
         cap.setKey("qwerty");
 
@@ -158,19 +150,16 @@ public class CasAuthenticationProviderTests extends TestCase {
         cap.setTicketValidator(new MockTicketValidator(true));
         cap.afterPropertiesSet();
 
-        UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken(CasProcessingFilter.CAS_STATEFUL_IDENTIFIER,
-                "");
+        UsernamePasswordAuthenticationToken token =
+                new UsernamePasswordAuthenticationToken(CasProcessingFilter.CAS_STATEFUL_IDENTIFIER, "");
 
-        try {
-            Authentication result = cap.authenticate(token);
-            fail("Should have thrown BadCredentialsException");
-        } catch (BadCredentialsException expected) {
-        }
+        Authentication result = cap.authenticate(token);
     }
 
-    public void testDetectsAnInvalidKey() throws Exception {
+    @Test(expected = BadCredentialsException.class)
+    public void invalidKeyIsDetected() throws Exception {
         CasAuthenticationProvider cap = new CasAuthenticationProvider();
-        cap.setCasAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        cap.setUserDetailsService(new MockAuthoritiesPopulator());
         cap.setCasProxyDecider(new MockProxyDecider(true));
         cap.setKey("qwerty");
 
@@ -182,112 +171,82 @@ public class CasAuthenticationProviderTests extends TestCase {
         CasAuthenticationToken token = new CasAuthenticationToken("WRONG_KEY", makeUserDetails(), "credentials",
                 new GrantedAuthority[] {new GrantedAuthorityImpl("XX")}, makeUserDetails(), new Vector(), "IOU-xxx");
 
-        try {
-            Authentication result = cap.authenticate(token);
-            fail("Should have thrown BadCredentialsException");
-        } catch (BadCredentialsException expected) {
-        }
+        cap.authenticate(token);
     }
 
-    public void testDetectsMissingAuthoritiesPopulator()
-        throws Exception {
+    @Test(expected = IllegalArgumentException.class)
+    public void detectsMissingAuthoritiesPopulator() throws Exception {
         CasAuthenticationProvider cap = new CasAuthenticationProvider();
         cap.setCasProxyDecider(new MockProxyDecider());
         cap.setKey("qwerty");
         cap.setStatelessTicketCache(new MockStatelessTicketCache());
         cap.setTicketValidator(new MockTicketValidator(true));
-
-        try {
-            cap.afterPropertiesSet();
-            fail("Should have thrown IllegalArgumentException");
-        } catch (IllegalArgumentException expected) {
-            assertEquals("A casAuthoritiesPopulator must be set", expected.getMessage());
-        }
+        cap.afterPropertiesSet();
     }
 
-    public void testDetectsMissingKey() throws Exception {
+    @Test(expected = IllegalArgumentException.class)
+    public void detectsMissingKey() throws Exception {
         CasAuthenticationProvider cap = new CasAuthenticationProvider();
-        cap.setCasAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        cap.setUserDetailsService(new MockAuthoritiesPopulator());
         cap.setCasProxyDecider(new MockProxyDecider());
         cap.setStatelessTicketCache(new MockStatelessTicketCache());
         cap.setTicketValidator(new MockTicketValidator(true));
-
-        try {
-            cap.afterPropertiesSet();
-            fail("Should have thrown IllegalArgumentException");
-        } catch (IllegalArgumentException expected) {
-            assertEquals("A Key is required so CasAuthenticationProvider can identify tokens it previously authenticated",
-                expected.getMessage());
-        }
+        cap.afterPropertiesSet();
     }
 
-    public void testDetectsMissingProxyDecider() throws Exception {
+    @Test(expected = IllegalArgumentException.class)
+    public void detectsMissingProxyDecider() throws Exception {
         CasAuthenticationProvider cap = new CasAuthenticationProvider();
-        cap.setCasAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        cap.setUserDetailsService(new MockAuthoritiesPopulator());
         cap.setKey("qwerty");
         cap.setStatelessTicketCache(new MockStatelessTicketCache());
         cap.setTicketValidator(new MockTicketValidator(true));
-
-        try {
-            cap.afterPropertiesSet();
-            fail("Should have thrown IllegalArgumentException");
-        } catch (IllegalArgumentException expected) {
-            assertEquals("A casProxyDecider must be set", expected.getMessage());
-        }
+        cap.afterPropertiesSet();
     }
 
-    public void testDetectsMissingStatelessTicketCache()
-        throws Exception {
+    @Test(expected = IllegalArgumentException.class)
+    public void detectsMissingStatelessTicketCache() throws Exception {
         CasAuthenticationProvider cap = new CasAuthenticationProvider();
         // set this explicitly to null to test failure
         cap.setStatelessTicketCache(null);
-        cap.setCasAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        cap.setUserDetailsService(new MockAuthoritiesPopulator());
         cap.setCasProxyDecider(new MockProxyDecider());
         cap.setKey("qwerty");
         cap.setTicketValidator(new MockTicketValidator(true));
-
-        try {
-            cap.afterPropertiesSet();
-            fail("Should have thrown IllegalArgumentException");
-        } catch (IllegalArgumentException expected) {
-            assertEquals("A statelessTicketCache must be set", expected.getMessage());
-        }
+        cap.afterPropertiesSet();
     }
 
-    public void testDetectsMissingTicketValidator() throws Exception {
+    @Test(expected = IllegalArgumentException.class)
+    public void detectsMissingTicketValidator() throws Exception {
         CasAuthenticationProvider cap = new CasAuthenticationProvider();
-        cap.setCasAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        cap.setUserDetailsService(new MockAuthoritiesPopulator());
         cap.setCasProxyDecider(new MockProxyDecider(true));
         cap.setKey("qwerty");
         cap.setStatelessTicketCache(new MockStatelessTicketCache());
-
-        try {
-            cap.afterPropertiesSet();
-            fail("Should have thrown IllegalArgumentException");
-        } catch (IllegalArgumentException expected) {
-            assertEquals("A ticketValidator must be set", expected.getMessage());
-        }
+        cap.afterPropertiesSet();
     }
 
-    public void testGettersSetters() throws Exception {
+    @Test
+    public void gettersAndSettersMatch() throws Exception {
         CasAuthenticationProvider cap = new CasAuthenticationProvider();
-        cap.setCasAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        cap.setUserDetailsService(new MockAuthoritiesPopulator());
         cap.setCasProxyDecider(new MockProxyDecider());
         cap.setKey("qwerty");
         cap.setStatelessTicketCache(new MockStatelessTicketCache());
         cap.setTicketValidator(new MockTicketValidator(true));
         cap.afterPropertiesSet();
 
-        assertTrue(cap.getCasAuthoritiesPopulator() != null);
+        assertTrue(cap.getUserDetailsService() != null);
         assertTrue(cap.getCasProxyDecider() != null);
         assertEquals("qwerty", cap.getKey());
         assertTrue(cap.getStatelessTicketCache() != null);
         assertTrue(cap.getTicketValidator() != null);
     }
 
-    public void testIgnoresClassesItDoesNotSupport() throws Exception {
+    @Test
+    public void ignoresClassesItDoesNotSupport() throws Exception {
         CasAuthenticationProvider cap = new CasAuthenticationProvider();
-        cap.setCasAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        cap.setUserDetailsService(new MockAuthoritiesPopulator());
         cap.setCasProxyDecider(new MockProxyDecider());
         cap.setKey("qwerty");
         cap.setStatelessTicketCache(new MockStatelessTicketCache());
@@ -302,10 +261,10 @@ public class CasAuthenticationProviderTests extends TestCase {
         assertEquals(null, cap.authenticate(token));
     }
 
-    public void testIgnoresUsernamePasswordAuthenticationTokensWithoutCasIdentifiersAsPrincipal()
-        throws Exception {
+    @Test
+    public void ignoresUsernamePasswordAuthenticationTokensWithoutCasIdentifiersAsPrincipal() throws Exception {
         CasAuthenticationProvider cap = new CasAuthenticationProvider();
-        cap.setCasAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        cap.setUserDetailsService(new MockAuthoritiesPopulator());
         cap.setCasProxyDecider(new MockProxyDecider());
         cap.setKey("qwerty");
         cap.setStatelessTicketCache(new MockStatelessTicketCache());
@@ -317,7 +276,8 @@ public class CasAuthenticationProviderTests extends TestCase {
         assertEquals(null, cap.authenticate(token));
     }
 
-    public void testSupports() {
+    @Test
+    public void supportsRequiredTokens() {
         CasAuthenticationProvider cap = new CasAuthenticationProvider();
         assertTrue(cap.supports(UsernamePasswordAuthenticationToken.class));
         assertTrue(cap.supports(CasAuthenticationToken.class));
@@ -325,9 +285,8 @@ public class CasAuthenticationProviderTests extends TestCase {
 
     //~ Inner Classes ==================================================================================================
 
-    private class MockAuthoritiesPopulator implements CasAuthoritiesPopulator {
-        public UserDetails getUserDetails(String casUserId)
-            throws AuthenticationException {
+    private class MockAuthoritiesPopulator implements UserDetailsService {
+        public UserDetails loadUserByUsername(String casUserId) throws AuthenticationException {
             return makeUserDetailsFromAuthoritiesPopulator();
         }
     }
@@ -380,10 +339,6 @@ public class CasAuthenticationProviderTests extends TestCase {
             this.returnTicket = returnTicket;
         }
 
-        private MockTicketValidator() {
-            super();
-        }
-
         public TicketResponse confirmTicketValid(String serviceTicket)
             throws AuthenticationException {
             if (returnTicket) {

core/src/test/java/org/springframework/security/providers/cas/populator/DaoCasAuthoritiesPopulatorTests.java

@@ -1,145 +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.cas.populator;
-
-import junit.framework.TestCase;
-
-import org.springframework.security.GrantedAuthority;
-import org.springframework.security.GrantedAuthorityImpl;
-
-import org.springframework.security.userdetails.User;
-import org.springframework.security.userdetails.UserDetails;
-import org.springframework.security.userdetails.UserDetailsService;
-import org.springframework.security.userdetails.UsernameNotFoundException;
-
-import org.springframework.dao.DataAccessException;
-import org.springframework.dao.DataRetrievalFailureException;
-
-
-/**
- * Tests {@link DaoCasAuthoritiesPopulator}.
- *
- * @author Ben Alex
- * @version $Id$
- */
-public class DaoCasAuthoritiesPopulatorTests extends TestCase {
-    //~ Constructors ===================================================================================================
-
-    public DaoCasAuthoritiesPopulatorTests() {
-        super();
-    }
-
-    public DaoCasAuthoritiesPopulatorTests(String arg0) {
-        super(arg0);
-    }
-
-    //~ Methods ========================================================================================================
-
-    public static void main(String[] args) {
-        junit.textui.TestRunner.run(DaoCasAuthoritiesPopulatorTests.class);
-    }
-
-    public final void setUp() throws Exception {
-        super.setUp();
-    }
-
-    public void testDetectsMissingAuthenticationDao() throws Exception {
-        DaoCasAuthoritiesPopulator populator = new DaoCasAuthoritiesPopulator();
-
-        try {
-            populator.afterPropertiesSet();
-            fail("Should have thrown IllegalArgumentException");
-        } catch (IllegalArgumentException expected) {
-            assertEquals("A UserDetailsService must be set", expected.getMessage());
-        }
-    }
-
-    public void testGetGrantedAuthoritiesForInvalidUsername()
-        throws Exception {
-        DaoCasAuthoritiesPopulator populator = new DaoCasAuthoritiesPopulator();
-        populator.setUserDetailsService(new MockAuthenticationDaoUserrod());
-        populator.afterPropertiesSet();
-
-        try {
-            populator.getUserDetails("scott");
-            fail("Should have thrown UsernameNotFoundException");
-        } catch (UsernameNotFoundException expected) {
-            assertTrue(true);
-        }
-    }
-
-    public void testGetGrantedAuthoritiesForValidUsername()
-        throws Exception {
-        DaoCasAuthoritiesPopulator populator = new DaoCasAuthoritiesPopulator();
-        populator.setUserDetailsService(new MockAuthenticationDaoUserrod());
-        populator.afterPropertiesSet();
-
-        UserDetails results = populator.getUserDetails("rod");
-        assertEquals(2, results.getAuthorities().length);
-        assertEquals(new GrantedAuthorityImpl("ROLE_ONE"), results.getAuthorities()[0]);
-        assertEquals(new GrantedAuthorityImpl("ROLE_TWO"), results.getAuthorities()[1]);
-    }
-
-    public void testGetGrantedAuthoritiesWhenDaoThrowsException()
-        throws Exception {
-        DaoCasAuthoritiesPopulator populator = new DaoCasAuthoritiesPopulator();
-        populator.setUserDetailsService(new MockAuthenticationDaoSimulateBackendError());
-        populator.afterPropertiesSet();
-
-        try {
-            populator.getUserDetails("THE_DAO_WILL_FAIL");
-            fail("Should have thrown DataRetrievalFailureException");
-        } catch (DataRetrievalFailureException expected) {
-            assertTrue(true);
-        }
-    }
-
-    public void testGettersSetters() {
-        DaoCasAuthoritiesPopulator populator = new DaoCasAuthoritiesPopulator();
-        UserDetailsService dao = new MockAuthenticationDaoUserrod();
-        populator.setUserDetailsService(dao);
-        assertEquals(dao, populator.getUserDetailsService());
-    }
-
-    //~ Inner Classes ==================================================================================================
-
-    private class MockAuthenticationDaoSimulateBackendError implements UserDetailsService {
-        public long getRefreshDuration() {
-            return 0;
-        }
-
-        public UserDetails loadUserByUsername(String username)
-            throws UsernameNotFoundException, DataAccessException {
-            throw new DataRetrievalFailureException("This mock simulator is designed to fail");
-        }
-    }
-
-    private class MockAuthenticationDaoUserrod implements UserDetailsService {
-        public long getRefreshDuration() {
-            return 0;
-        }
-
-        public UserDetails loadUserByUsername(String username)
-            throws UsernameNotFoundException, DataAccessException {
-            if ("rod".equals(username)) {
-                return new User("rod", "koala", true, true, true, true,
-                    new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl("ROLE_TWO")});
-            } else {
-                throw new UsernameNotFoundException("Could not find: " + username);
-            }
-        }
-    }
-}

openid/src/main/java/org/springframework/security/providers/openid/OpenIDAuthenticationProvider.java

@@ -20,32 +20,43 @@ import org.springframework.security.AuthenticationException;
 import org.springframework.security.AuthenticationServiceException;
 import org.springframework.security.BadCredentialsException;
 import org.springframework.security.providers.AuthenticationProvider;
-import org.springframework.security.providers.AuthoritiesPopulator;
 import org.springframework.security.userdetails.UserDetails;
+import org.springframework.security.userdetails.UserDetailsService;
 import org.springframework.util.Assert;
 
 
 /**
- * Finalises the OpenID authentication by obtaining local roles
+ * Finalises the OpenID authentication by obtaining local authorities for the authenticated user.
+ * <p>
+ * The authorities are obtained by calling the configured <tt>UserDetailsService</tt>.
+ * The <code>UserDetails</code> it returns must, at minimum, contain the username and <code>GrantedAuthority[]</code>
+ * objects applicable to the authenticated user. Note that by default, Spring Security ignores the password and
+ * enabled/disabled status of the <code>UserDetails</code> because this is
+ * authentication-related and should have been enforced by another provider server.
+ * <p>
+ * You can optionally have these checked by configuring wrapping the <tt>UserDetailsService</tt> in a
+ * {@link org.springframework.security.userdetails.decorator.StatusCheckingUserDetailsService} decorator.
+ * <p>
+ * The <code>UserDetails</code> returned by implementations is stored in the generated <code>AuthenticationToken</code>,
+ * so additional properties such as email addresses, telephone numbers etc can easily be stored.
  *
  * @author Robin Bramley, Opsera Ltd.
  */
 public class OpenIDAuthenticationProvider implements AuthenticationProvider, InitializingBean {
     //~ Instance fields ================================================================================================
 
-    private AuthoritiesPopulator authoritiesPopulator;
+    private UserDetailsService userDetailsService;
 
     //~ Methods ========================================================================================================
 
     public void afterPropertiesSet() throws Exception {
-        Assert.notNull(this.authoritiesPopulator, "The authoritiesPopulator must be set");
+        Assert.notNull(this.userDetailsService, "The userDetailsService must be set");
     }
 
     /* (non-Javadoc)
      * @see org.springframework.security.providers.AuthenticationProvider#authenticate(org.springframework.security.Authentication)
      */
-    public Authentication authenticate(final Authentication authentication)
-            throws AuthenticationException {
+    public Authentication authenticate(final Authentication authentication) throws AuthenticationException {
 
         if (!supports(authentication.getClass())) {
             return null;
@@ -59,7 +70,7 @@ public class OpenIDAuthenticationProvider implements AuthenticationProvider, Ini
             if (status == OpenIDAuthenticationStatus.SUCCESS) {
 
                 // Lookup user details
-                UserDetails userDetails = this.authoritiesPopulator.getUserDetails(response.getIdentityUrl());
+                UserDetails userDetails = userDetailsService.loadUserByUsername(response.getIdentityUrl());
 
                 return new OpenIDAuthenticationToken(userDetails.getAuthorities(), response.getStatus(),
                         response.getIdentityUrl());
@@ -81,8 +92,11 @@ public class OpenIDAuthenticationProvider implements AuthenticationProvider, Ini
         return null;
     }
 
-    public void setAuthoritiesPopulator(AuthoritiesPopulator authoritiesPopulator) {
-        this.authoritiesPopulator = authoritiesPopulator;
+    /**
+     * Used to load the authorities for the authenticated OpenID user.
+     */
+    public void setUserDetailsService(UserDetailsService userDetailsService) {
+        this.userDetailsService = userDetailsService;
     }
 
     /* (non-Javadoc)

openid/src/main/java/org/springframework/security/providers/openid/OpenIDAuthenticationToken.java

@@ -41,13 +41,10 @@ public class OpenIDAuthenticationToken extends AbstractAuthenticationToken {
         setAuthenticated(false);
     }
 
-/**
+    /**
      * Created by the OpenIDAuthenticationProvider on successful authentication.
      * <b>Do not use directly</b>
      *
-     * @param authorities
-     * @param status
-     * @param identityUrl
      */
     public OpenIDAuthenticationToken(GrantedAuthority[] authorities, OpenIDAuthenticationStatus status, String identityUrl) {
         super(authorities);

openid/src/test/java/org/springframework/security/providers/openid/MockAuthoritiesPopulator.java

@@ -1,38 +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.openid;
-
-import org.springframework.security.AuthenticationException;
-import org.springframework.security.GrantedAuthority;
-import org.springframework.security.GrantedAuthorityImpl;
-import org.springframework.security.providers.AuthoritiesPopulator;
-import org.springframework.security.userdetails.User;
-import org.springframework.security.userdetails.UserDetails;
-
-
-/**
- * DOCUMENT ME!
- *
- * @author Robin Bramley, Opsera Ltd
- */
-public class MockAuthoritiesPopulator implements AuthoritiesPopulator {
-    //~ Methods ========================================================================================================
-
-    public UserDetails getUserDetails(String ssoUserId)
-        throws AuthenticationException {
-        return new User(ssoUserId, "password", true, true, true, true,
-            new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_A"), new GrantedAuthorityImpl("ROLE_B")});
-    }
-}

openid/src/test/java/org/springframework/security/providers/openid/OpenIDAuthenticationProviderTests.java

@@ -18,7 +18,13 @@ import junit.framework.TestCase;
 import org.springframework.security.Authentication;
 import org.springframework.security.AuthenticationServiceException;
 import org.springframework.security.BadCredentialsException;
+import org.springframework.security.AuthenticationException;
+import org.springframework.security.GrantedAuthority;
+import org.springframework.security.GrantedAuthorityImpl;
+import org.springframework.security.userdetails.UserDetails;
+import org.springframework.security.userdetails.User;
 import org.springframework.security.providers.UsernamePasswordAuthenticationToken;
+import org.springframework.security.userdetails.UserDetailsService;
 
 
 /**
@@ -38,7 +44,7 @@ public class OpenIDAuthenticationProviderTests extends TestCase {
      */
     public void testAuthenticateCancel() {
         OpenIDAuthenticationProvider provider = new OpenIDAuthenticationProvider();
-        provider.setAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        provider.setUserDetailsService(new MockUserDetailsService());
 
         Authentication preAuth = new OpenIDAuthenticationToken(OpenIDAuthenticationStatus.CANCELLED, USERNAME, "");
 
@@ -57,7 +63,7 @@ public class OpenIDAuthenticationProviderTests extends TestCase {
      */
     public void testAuthenticateError() {
         OpenIDAuthenticationProvider provider = new OpenIDAuthenticationProvider();
-        provider.setAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        provider.setUserDetailsService(new MockUserDetailsService());
 
         Authentication preAuth = new OpenIDAuthenticationToken(OpenIDAuthenticationStatus.ERROR, USERNAME, "");
 
@@ -76,7 +82,7 @@ public class OpenIDAuthenticationProviderTests extends TestCase {
      */
     public void testAuthenticateFailure() {
         OpenIDAuthenticationProvider provider = new OpenIDAuthenticationProvider();
-        provider.setAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        provider.setUserDetailsService(new MockUserDetailsService());
 
         Authentication preAuth = new OpenIDAuthenticationToken(OpenIDAuthenticationStatus.FAILURE, USERNAME, "");
 
@@ -95,7 +101,7 @@ public class OpenIDAuthenticationProviderTests extends TestCase {
      */
     public void testAuthenticateSetupNeeded() {
         OpenIDAuthenticationProvider provider = new OpenIDAuthenticationProvider();
-        provider.setAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        provider.setUserDetailsService(new MockUserDetailsService());
 
         Authentication preAuth = new OpenIDAuthenticationToken(OpenIDAuthenticationStatus.SETUP_NEEDED, USERNAME, "");
 
@@ -114,7 +120,7 @@ public class OpenIDAuthenticationProviderTests extends TestCase {
      */
     public void testAuthenticateSuccess() {
         OpenIDAuthenticationProvider provider = new OpenIDAuthenticationProvider();
-        provider.setAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        provider.setUserDetailsService(new MockUserDetailsService());
 
         Authentication preAuth = new OpenIDAuthenticationToken(OpenIDAuthenticationStatus.SUCCESS, USERNAME, "");
 
@@ -149,7 +155,7 @@ public class OpenIDAuthenticationProviderTests extends TestCase {
      */
     public void testDoesntSupport() {
         OpenIDAuthenticationProvider provider = new OpenIDAuthenticationProvider();
-        provider.setAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        provider.setUserDetailsService(new MockUserDetailsService());
 
         assertFalse(provider.supports(UsernamePasswordAuthenticationToken.class));
     }
@@ -159,7 +165,7 @@ public class OpenIDAuthenticationProviderTests extends TestCase {
      */
     public void testIgnoresUserPassAuthToken() {
         OpenIDAuthenticationProvider provider = new OpenIDAuthenticationProvider();
-        provider.setAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        provider.setUserDetailsService(new MockUserDetailsService());
 
         UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken(USERNAME, "password");
         assertEquals(null, provider.authenticate(token));
@@ -170,17 +176,17 @@ public class OpenIDAuthenticationProviderTests extends TestCase {
      */
     public void testSupports() {
         OpenIDAuthenticationProvider provider = new OpenIDAuthenticationProvider();
-        provider.setAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        provider.setUserDetailsService(new MockUserDetailsService());
 
         assertTrue(provider.supports(OpenIDAuthenticationToken.class));
     }
 
     public void testValidation() throws Exception {
         OpenIDAuthenticationProvider provider = new OpenIDAuthenticationProvider();
-        provider.setAuthoritiesPopulator(new MockAuthoritiesPopulator());
+        provider.setUserDetailsService(new MockUserDetailsService());
         provider.afterPropertiesSet();
 
-        provider.setAuthoritiesPopulator(null);
+        provider.setUserDetailsService(null);
 
         try {
             provider.afterPropertiesSet();
@@ -189,4 +195,12 @@ public class OpenIDAuthenticationProviderTests extends TestCase {
             //expected
         }
     }
+
+    static class MockUserDetailsService implements UserDetailsService {
+        public UserDetails loadUserByUsername(String ssoUserId)
+            throws AuthenticationException {
+            return new User(ssoUserId, "password", true, true, true, true,
+                new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_A"), new GrantedAuthorityImpl("ROLE_B")});
+        }
+    }
 }

portlet/src/main/java/org/springframework/security/providers/portlet/PortletAuthenticationProvider.java

@@ -65,7 +65,7 @@ public class PortletAuthenticationProvider
 	//~ Methods ========================================================================================================
 
 	public void afterPropertiesSet() throws Exception {
-		Assert.notNull(this.portletAuthoritiesPopulator, "An authorities populator must be set");
+	    Assert.notNull(this.portletAuthoritiesPopulator, "An authorities populator must be set");
 		Assert.notNull(this.userCache, "A user cache must be set");
 	}
 

samples/openid/src/main/webapp/WEB-INF/applicationContext-security.xml

@@ -29,11 +29,7 @@
 
     <b:bean id="openIdAuthenticationProvider" class="org.springframework.security.providers.openid.OpenIDAuthenticationProvider">
         <custom-authentication-provider />
-        <b:property name="authoritiesPopulator">
-            <b:bean class="org.springframework.security.providers.DaoAuthoritiesPopulator">
-                <b:property name="userDetailsService" ref="userService"/>
-            </b:bean>
-        </b:property>
+        <b:property name="userDetailsService" ref="userDetailsService"/>
     </b:bean>
 
     <b:bean id="entryPoint" class="org.springframework.security.ui.webapp.AuthenticationProcessingFilterEntryPoint">