Polish Authentication Factors

Issue gh-17933

cas/src/main/java/org/springframework/security/cas/authentication/CasAuthenticationProvider.java

@@ -16,6 +16,9 @@
 
 package org.springframework.security.cas.authentication;
 
+import java.util.ArrayList;
+import java.util.Collection;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apereo.cas.client.validation.Assertion;
@@ -35,7 +38,9 @@ import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.security.cas.ServiceProperties;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
+import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.SpringSecurityMessageSource;
+import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.security.core.authority.mapping.GrantedAuthoritiesMapper;
 import org.springframework.security.core.authority.mapping.NullAuthoritiesMapper;
 import org.springframework.security.core.userdetails.AuthenticationUserDetailsService;
@@ -64,6 +69,8 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
 
 	private static final Log logger = LogFactory.getLog(CasAuthenticationProvider.class);
 
+	private static final String AUTHORITY = "FACTOR_CAS";
+
 	@SuppressWarnings("NullAway.Init")
 	private AuthenticationUserDetailsService<CasAssertionAuthenticationToken> authenticationUserDetailsService;
 
@@ -141,8 +148,10 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
 			Assertion assertion = this.ticketValidator.validate(credentials.toString(), getServiceUrl(authentication));
 			UserDetails userDetails = loadUserByAssertion(assertion);
 			this.userDetailsChecker.check(userDetails);
-			return new CasAuthenticationToken(this.key, userDetails, credentials,
-					this.authoritiesMapper.mapAuthorities(userDetails.getAuthorities()), userDetails, assertion);
+			Collection<GrantedAuthority> authorities = new ArrayList<>(
+					this.authoritiesMapper.mapAuthorities(userDetails.getAuthorities()));
+			authorities.add(new SimpleGrantedAuthority(AUTHORITY));
+			return new CasAuthenticationToken(this.key, userDetails, credentials, authorities, userDetails, assertion);
 		}
 		catch (TicketValidationException ex) {
 			throw new BadCredentialsException(ex.getMessage(), ex);

cas/src/test/java/org/springframework/security/cas/authentication/CasAuthenticationProviderTests.java

@@ -27,6 +27,7 @@ import org.junit.jupiter.api.Test;
 
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.security.authentication.BadCredentialsException;
+import org.springframework.security.authentication.SecurityAssertions;
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.cas.ServiceProperties;
@@ -346,6 +347,22 @@ public class CasAuthenticationProviderTests {
 		assertThat(checkCount.get()).isEqualTo(1);
 	}
 
+	@Test
+	public void authenticateWhenSuccessfulThenIssuesFactor() throws Exception {
+		CasAuthenticationProvider cap = new CasAuthenticationProvider();
+		cap.setAuthenticationUserDetailsService(new MockAuthoritiesPopulator());
+		cap.setKey("qwerty");
+		StatelessTicketCache cache = new MockStatelessTicketCache();
+		cap.setStatelessTicketCache(cache);
+		cap.setServiceProperties(makeServiceProperties());
+		cap.setTicketValidator(new MockTicketValidator(true));
+		cap.afterPropertiesSet();
+		CasServiceTicketAuthenticationToken token = CasServiceTicketAuthenticationToken.stateful("ST-123");
+		token.setDetails("details");
+		Authentication result = cap.authenticate(token);
+		SecurityAssertions.assertThat(result).hasAuthority("FACTOR_CAS");
+	}
+
 	private class MockAuthoritiesPopulator implements AuthenticationUserDetailsService {
 
 		@Override

config/src/test/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests.java

@@ -30,6 +30,7 @@ import org.springframework.http.MediaType;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.mock.web.MockHttpSession;
+import org.springframework.security.authentication.SecurityAssertions;
 import org.springframework.security.authentication.event.AuthenticationSuccessEvent;
 import org.springframework.security.config.test.SpringTestContext;
 import org.springframework.security.config.test.SpringTestContextExtension;
@@ -322,8 +323,10 @@ public class OAuth2LoginBeanDefinitionParserTests {
 		verify(this.authenticationSuccessHandler).onAuthenticationSuccess(any(), any(), authenticationCaptor.capture());
 		Authentication authentication = authenticationCaptor.getValue();
 		assertThat(authentication.getPrincipal()).isInstanceOf(OAuth2User.class);
-		assertThat(authentication.getAuthorities()).hasSize(1);
-		assertThat(authentication.getAuthorities()).first()
+		SecurityAssertions.assertThat(authentication)
+			.roles()
+			.hasSize(1)
+			.first()
 			.isInstanceOf(SimpleGrantedAuthority.class)
 			.hasToString("ROLE_OAUTH2_USER");
 		// re-setup for OIDC test

core/src/main/java/org/springframework/security/authentication/dao/AbstractUserDetailsAuthenticationProvider.java

@@ -16,8 +16,8 @@
 
 package org.springframework.security.authentication.dao;
 
-import java.util.ArrayList;
 import java.util.Collection;
+import java.util.LinkedHashSet;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -204,7 +204,7 @@ public abstract class AbstractUserDetailsAuthenticationProvider
 		// so subsequent attempts are successful even with encoded passwords.
 		// Also ensure we return the original getDetails(), so that future
 		// authentication events after cache expiry contain the details
-		Collection<GrantedAuthority> authorities = new ArrayList<>(
+		Collection<GrantedAuthority> authorities = new LinkedHashSet<>(
 				this.authoritiesMapper.mapAuthorities(user.getAuthorities()));
 		authorities.add(new SimpleGrantedAuthority(AUTHORITY));
 		UsernamePasswordAuthenticationToken result = UsernamePasswordAuthenticationToken.authenticated(principal,

core/src/main/java/org/springframework/security/authentication/jaas/AbstractJaasAuthenticationProvider.java

@@ -45,6 +45,7 @@ import org.springframework.security.authentication.jaas.event.JaasAuthentication
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
 import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.security.core.context.SecurityContext;
 import org.springframework.security.core.session.SessionDestroyedEvent;
 import org.springframework.util.Assert;
@@ -120,6 +121,8 @@ import org.springframework.util.ObjectUtils;
 public abstract class AbstractJaasAuthenticationProvider implements AuthenticationProvider,
 		ApplicationEventPublisherAware, InitializingBean, ApplicationListener<SessionDestroyedEvent> {
 
+	private static final String AUTHORITY = "FACTOR_PASSWORD";
+
 	private ApplicationEventPublisher applicationEventPublisher = (event) -> {
 	};
 
@@ -210,6 +213,7 @@ public abstract class AbstractJaasAuthenticationProvider implements Authenticati
 				}
 			}
 		}
+		authorities.add(new SimpleGrantedAuthority(AUTHORITY));
 		return authorities;
 	}
 

core/src/test/java/org/springframework/security/authentication/SecurityAssertions.java

@@ -75,6 +75,10 @@ public final class SecurityAssertions {
 			return authorities().has(new Condition<>(test, "contains %s", Arrays.toString(authorities)));
 		}
 
+		public CollectionAssert<GrantedAuthority> roles() {
+			return authorities().filteredOn((authority) -> authority.getAuthority().startsWith("ROLE_"));
+		}
+
 		public CollectionAssert<GrantedAuthority> authorities() {
 			return new CollectionAssert<>(this.authentication.getAuthorities());
 		}

core/src/test/java/org/springframework/security/authentication/jaas/JaasAuthenticationProviderTests.java

@@ -35,6 +35,7 @@ import org.springframework.context.ApplicationContext;
 import org.springframework.context.support.ClassPathXmlApplicationContext;
 import org.springframework.core.io.FileSystemResource;
 import org.springframework.security.authentication.LockedException;
+import org.springframework.security.authentication.SecurityAssertions;
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.Authentication;
@@ -224,7 +225,9 @@ public class JaasAuthenticationProviderTests {
 				"password");
 		assertThat(this.jaasProvider.supports(UsernamePasswordAuthenticationToken.class)).isTrue();
 		Authentication auth = this.jaasProvider.authenticate(token);
-		assertThat(auth.getAuthorities()).withFailMessage("Only ROLE_TEST1 and ROLE_TEST2 should have been returned")
+		SecurityAssertions.assertThat(auth)
+			.roles()
+			.withFailMessage("Only ROLE_TEST1 and ROLE_TEST2 should have been returned")
 			.hasSize(2);
 	}
 
@@ -234,6 +237,13 @@ public class JaasAuthenticationProviderTests {
 			.authenticate(new TestingAuthenticationToken("foo", "bar", AuthorityUtils.NO_AUTHORITIES))).isNull();
 	}
 
+	@Test
+	public void authenticateWhenSuccessThenIssuesFactor() {
+		UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("user", "password");
+		Authentication result = this.jaasProvider.authenticate(token);
+		SecurityAssertions.assertThat(result).hasAuthority("FACTOR_PASSWORD");
+	}
+
 	private static class MockLoginContext extends LoginContext {
 
 		boolean loggedOut = false;

ldap/src/main/java/org/springframework/security/ldap/authentication/AbstractLdapAuthenticationProvider.java

@@ -16,8 +16,8 @@
 
 package org.springframework.security.ldap.authentication;
 
-import java.util.ArrayList;
 import java.util.Collection;
+import java.util.LinkedHashSet;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -104,7 +104,7 @@ public abstract class AbstractLdapAuthenticationProvider implements Authenticati
 			UserDetails user) {
 		Object password = this.useAuthenticationRequestCredentials ? authentication.getCredentials()
 				: user.getPassword();
-		Collection<GrantedAuthority> authorities = new ArrayList<>(
+		Collection<GrantedAuthority> authorities = new LinkedHashSet<>(
 				this.authoritiesMapper.mapAuthorities(user.getAuthorities()));
 		authorities.add(new SimpleGrantedAuthority(AUTHORITY));
 		UsernamePasswordAuthenticationToken result = UsernamePasswordAuthenticationToken.authenticated(user, password,

oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2LoginAuthenticationProvider.java

@@ -18,6 +18,7 @@ package org.springframework.security.oauth2.client.authentication;
 
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.Map;
 
 import org.springframework.security.authentication.AuthenticationProvider;
@@ -123,8 +124,9 @@ public class OAuth2LoginAuthenticationProvider implements AuthenticationProvider
 		OAuth2User oauth2User = this.userService.loadUser(new OAuth2UserRequest(
 				loginAuthenticationToken.getClientRegistration(), accessToken, additionalParameters));
 		Collection<GrantedAuthority> authorities = new HashSet<>(oauth2User.getAuthorities());
-		authorities.add(new SimpleGrantedAuthority(AUTHORITY));
-		Collection<? extends GrantedAuthority> mappedAuthorities = this.authoritiesMapper.mapAuthorities(authorities);
+		Collection<GrantedAuthority> mappedAuthorities = new LinkedHashSet<>(
+				this.authoritiesMapper.mapAuthorities(authorities));
+		mappedAuthorities.add(new SimpleGrantedAuthority(AUTHORITY));
 		OAuth2LoginAuthenticationToken authenticationResult = new OAuth2LoginAuthenticationToken(
 				loginAuthenticationToken.getClientRegistration(), loginAuthenticationToken.getAuthorizationExchange(),
 				oauth2User, mappedAuthorities, accessToken, authorizationCodeAuthenticationToken.getRefreshToken());

oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/authentication/OAuth2LoginAuthenticationProviderTests.java

@@ -59,6 +59,7 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyCollection;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 
 /**
  * Tests for {@link OAuth2LoginAuthenticationProvider}.
@@ -190,7 +191,8 @@ public class OAuth2LoginAuthenticationProviderTests {
 		this.authenticationProvider.setAuthoritiesMapper(authoritiesMapper);
 		OAuth2LoginAuthenticationToken authentication = (OAuth2LoginAuthenticationToken) this.authenticationProvider
 			.authenticate(new OAuth2LoginAuthenticationToken(this.clientRegistration, this.authorizationExchange));
-		assertThat(authentication.getAuthorities()).isEqualTo(mappedAuthorities);
+		verify(authoritiesMapper).mapAuthorities(any());
+		SecurityAssertions.assertThat(authentication).authorities().containsAll(mappedAuthorities);
 	}
 
 	// gh-5368