Skip to content

Commit 6e7a181

Browse files
jzheauxjzheaux
committed
Polish Authentication Factors
Issue spring-projectsgh-17933
1 parent 758b35d commit 6e7a181

File tree

10 files changed

+63
-12
lines changed

10 files changed

+63
-12
lines changed

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616

1717
package org.springframework.security.cas.authentication;
1818

19+
import java.util.ArrayList;
20+
import java.util.Collection;
21+
1922
import org.apache.commons.logging.Log;
2023
import org.apache.commons.logging.LogFactory;
2124
import org.apereo.cas.client.validation.Assertion;
@@ -35,7 +38,9 @@
3538
import org.springframework.security.cas.ServiceProperties;
3639
import org.springframework.security.core.Authentication;
3740
import org.springframework.security.core.AuthenticationException;
41+
import org.springframework.security.core.GrantedAuthority;
3842
import org.springframework.security.core.SpringSecurityMessageSource;
43+
import org.springframework.security.core.authority.SimpleGrantedAuthority;
3944
import org.springframework.security.core.authority.mapping.GrantedAuthoritiesMapper;
4045
import org.springframework.security.core.authority.mapping.NullAuthoritiesMapper;
4146
import org.springframework.security.core.userdetails.AuthenticationUserDetailsService;
@@ -64,6 +69,8 @@ public class CasAuthenticationProvider implements AuthenticationProvider, Initia
6469

6570
private static final Log logger = LogFactory.getLog(CasAuthenticationProvider.class);
6671

72+
private static final String AUTHORITY = "FACTOR_CAS";
73+
6774
@SuppressWarnings("NullAway.Init")
6875
private AuthenticationUserDetailsService<CasAssertionAuthenticationToken> authenticationUserDetailsService;
6976

@@ -141,8 +148,10 @@ private CasAuthenticationToken authenticateNow(final Authentication authenticati
141148
Assertion assertion = this.ticketValidator.validate(credentials.toString(), getServiceUrl(authentication));
142149
UserDetails userDetails = loadUserByAssertion(assertion);
143150
this.userDetailsChecker.check(userDetails);
144-
return new CasAuthenticationToken(this.key, userDetails, credentials,
145-
this.authoritiesMapper.mapAuthorities(userDetails.getAuthorities()), userDetails, assertion);
151+
Collection<GrantedAuthority> authorities = new ArrayList<>(
152+
this.authoritiesMapper.mapAuthorities(userDetails.getAuthorities()));
153+
authorities.add(new SimpleGrantedAuthority(AUTHORITY));
154+
return new CasAuthenticationToken(this.key, userDetails, credentials, authorities, userDetails, assertion);
146155
}
147156
catch (TicketValidationException ex) {
148157
throw new BadCredentialsException(ex.getMessage(), ex);

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
import org.springframework.mock.web.MockHttpServletRequest;
2929
import org.springframework.security.authentication.BadCredentialsException;
30+
import org.springframework.security.authentication.SecurityAssertions;
3031
import org.springframework.security.authentication.TestingAuthenticationToken;
3132
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
3233
import org.springframework.security.cas.ServiceProperties;
@@ -346,6 +347,22 @@ public void check(UserDetails user) {
346347
assertThat(checkCount.get()).isEqualTo(1);
347348
}
348349

350+
@Test
351+
public void authenticateWhenSuccessfulThenIssuesFactor() throws Exception {
352+
CasAuthenticationProvider cap = new CasAuthenticationProvider();
353+
cap.setAuthenticationUserDetailsService(new MockAuthoritiesPopulator());
354+
cap.setKey("qwerty");
355+
StatelessTicketCache cache = new MockStatelessTicketCache();
356+
cap.setStatelessTicketCache(cache);
357+
cap.setServiceProperties(makeServiceProperties());
358+
cap.setTicketValidator(new MockTicketValidator(true));
359+
cap.afterPropertiesSet();
360+
CasServiceTicketAuthenticationToken token = CasServiceTicketAuthenticationToken.stateful("ST-123");
361+
token.setDetails("details");
362+
Authentication result = cap.authenticate(token);
363+
SecurityAssertions.assertThat(result).hasAuthority("FACTOR_CAS");
364+
}
365+
349366
private class MockAuthoritiesPopulator implements AuthenticationUserDetailsService {
350367

351368
@Override

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.springframework.mock.web.MockHttpServletRequest;
3131
import org.springframework.mock.web.MockHttpServletResponse;
3232
import org.springframework.mock.web.MockHttpSession;
33+
import org.springframework.security.authentication.SecurityAssertions;
3334
import org.springframework.security.authentication.event.AuthenticationSuccessEvent;
3435
import org.springframework.security.config.test.SpringTestContext;
3536
import org.springframework.security.config.test.SpringTestContextExtension;
@@ -322,8 +323,10 @@ public void requestWhenCustomGrantedAuthoritiesMapperThenCalled() throws Excepti
322323
verify(this.authenticationSuccessHandler).onAuthenticationSuccess(any(), any(), authenticationCaptor.capture());
323324
Authentication authentication = authenticationCaptor.getValue();
324325
assertThat(authentication.getPrincipal()).isInstanceOf(OAuth2User.class);
325-
assertThat(authentication.getAuthorities()).hasSize(1);
326-
assertThat(authentication.getAuthorities()).first()
326+
SecurityAssertions.assertThat(authentication)
327+
.roles()
328+
.hasSize(1)
329+
.first()
327330
.isInstanceOf(SimpleGrantedAuthority.class)
328331
.hasToString("ROLE_OAUTH2_USER");
329332
// re-setup for OIDC test

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616

1717
package org.springframework.security.authentication.dao;
1818

19-
import java.util.ArrayList;
2019
import java.util.Collection;
20+
import java.util.LinkedHashSet;
2121

2222
import org.apache.commons.logging.Log;
2323
import org.apache.commons.logging.LogFactory;
@@ -204,7 +204,7 @@ protected Authentication createSuccessAuthentication(Object principal, Authentic
204204
// so subsequent attempts are successful even with encoded passwords.
205205
// Also ensure we return the original getDetails(), so that future
206206
// authentication events after cache expiry contain the details
207-
Collection<GrantedAuthority> authorities = new ArrayList<>(
207+
Collection<GrantedAuthority> authorities = new LinkedHashSet<>(
208208
this.authoritiesMapper.mapAuthorities(user.getAuthorities()));
209209
authorities.add(new SimpleGrantedAuthority(AUTHORITY));
210210
UsernamePasswordAuthenticationToken result = UsernamePasswordAuthenticationToken.authenticated(principal,

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import org.springframework.security.core.Authentication;
4646
import org.springframework.security.core.AuthenticationException;
4747
import org.springframework.security.core.GrantedAuthority;
48+
import org.springframework.security.core.authority.SimpleGrantedAuthority;
4849
import org.springframework.security.core.context.SecurityContext;
4950
import org.springframework.security.core.session.SessionDestroyedEvent;
5051
import org.springframework.util.Assert;
@@ -120,6 +121,8 @@
120121
public abstract class AbstractJaasAuthenticationProvider implements AuthenticationProvider,
121122
ApplicationEventPublisherAware, InitializingBean, ApplicationListener<SessionDestroyedEvent> {
122123

124+
private static final String AUTHORITY = "FACTOR_PASSWORD";
125+
123126
private ApplicationEventPublisher applicationEventPublisher = (event) -> {
124127
};
125128

@@ -210,6 +213,7 @@ private Set<GrantedAuthority> getAuthorities(Set<Principal> principals) {
210213
}
211214
}
212215
}
216+
authorities.add(new SimpleGrantedAuthority(AUTHORITY));
213217
return authorities;
214218
}
215219

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@ public CollectionAssert<GrantedAuthority> hasAuthorities(String... authorities)
7575
return authorities().has(new Condition<>(test, "contains %s", Arrays.toString(authorities)));
7676
}
7777

78+
public CollectionAssert<GrantedAuthority> roles() {
79+
return authorities().filteredOn((authority) -> authority.getAuthority().startsWith("ROLE_"));
80+
}
81+
7882
public CollectionAssert<GrantedAuthority> authorities() {
7983
return new CollectionAssert<>(this.authentication.getAuthorities());
8084
}

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.springframework.context.support.ClassPathXmlApplicationContext;
3636
import org.springframework.core.io.FileSystemResource;
3737
import org.springframework.security.authentication.LockedException;
38+
import org.springframework.security.authentication.SecurityAssertions;
3839
import org.springframework.security.authentication.TestingAuthenticationToken;
3940
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
4041
import org.springframework.security.core.Authentication;
@@ -224,7 +225,9 @@ public void testNullDefaultAuthorities() {
224225
"password");
225226
assertThat(this.jaasProvider.supports(UsernamePasswordAuthenticationToken.class)).isTrue();
226227
Authentication auth = this.jaasProvider.authenticate(token);
227-
assertThat(auth.getAuthorities()).withFailMessage("Only ROLE_TEST1 and ROLE_TEST2 should have been returned")
228+
SecurityAssertions.assertThat(auth)
229+
.roles()
230+
.withFailMessage("Only ROLE_TEST1 and ROLE_TEST2 should have been returned")
228231
.hasSize(2);
229232
}
230233

@@ -234,6 +237,13 @@ public void testUnsupportedAuthenticationObjectReturnsNull() {
234237
.authenticate(new TestingAuthenticationToken("foo", "bar", AuthorityUtils.NO_AUTHORITIES))).isNull();
235238
}
236239

240+
@Test
241+
public void authenticateWhenSuccessThenIssuesFactor() {
242+
UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("user", "password");
243+
Authentication result = this.jaasProvider.authenticate(token);
244+
SecurityAssertions.assertThat(result).hasAuthority("FACTOR_PASSWORD");
245+
}
246+
237247
private static class MockLoginContext extends LoginContext {
238248

239249
boolean loggedOut = false;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616

1717
package org.springframework.security.ldap.authentication;
1818

19-
import java.util.ArrayList;
2019
import java.util.Collection;
20+
import java.util.LinkedHashSet;
2121

2222
import org.apache.commons.logging.Log;
2323
import org.apache.commons.logging.LogFactory;
@@ -104,7 +104,7 @@ protected Authentication createSuccessfulAuthentication(UsernamePasswordAuthenti
104104
UserDetails user) {
105105
Object password = this.useAuthenticationRequestCredentials ? authentication.getCredentials()
106106
: user.getPassword();
107-
Collection<GrantedAuthority> authorities = new ArrayList<>(
107+
Collection<GrantedAuthority> authorities = new LinkedHashSet<>(
108108
this.authoritiesMapper.mapAuthorities(user.getAuthorities()));
109109
authorities.add(new SimpleGrantedAuthority(AUTHORITY));
110110
UsernamePasswordAuthenticationToken result = UsernamePasswordAuthenticationToken.authenticated(user, password,

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import java.util.Collection;
2020
import java.util.HashSet;
21+
import java.util.LinkedHashSet;
2122
import java.util.Map;
2223

2324
import org.springframework.security.authentication.AuthenticationProvider;
@@ -123,8 +124,9 @@ public Authentication authenticate(Authentication authentication) throws Authent
123124
OAuth2User oauth2User = this.userService.loadUser(new OAuth2UserRequest(
124125
loginAuthenticationToken.getClientRegistration(), accessToken, additionalParameters));
125126
Collection<GrantedAuthority> authorities = new HashSet<>(oauth2User.getAuthorities());
126-
authorities.add(new SimpleGrantedAuthority(AUTHORITY));
127-
Collection<? extends GrantedAuthority> mappedAuthorities = this.authoritiesMapper.mapAuthorities(authorities);
127+
Collection<GrantedAuthority> mappedAuthorities = new LinkedHashSet<>(
128+
this.authoritiesMapper.mapAuthorities(authorities));
129+
mappedAuthorities.add(new SimpleGrantedAuthority(AUTHORITY));
128130
OAuth2LoginAuthenticationToken authenticationResult = new OAuth2LoginAuthenticationToken(
129131
loginAuthenticationToken.getClientRegistration(), loginAuthenticationToken.getAuthorizationExchange(),
130132
oauth2User, mappedAuthorities, accessToken, authorizationCodeAuthenticationToken.getRefreshToken());

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import static org.mockito.ArgumentMatchers.anyCollection;
6060
import static org.mockito.BDDMockito.given;
6161
import static org.mockito.Mockito.mock;
62+
import static org.mockito.Mockito.verify;
6263

6364
/**
6465
* Tests for {@link OAuth2LoginAuthenticationProvider}.
@@ -190,7 +191,8 @@ public void authenticateWhenAuthoritiesMapperSetThenReturnMappedAuthorities() {
190191
this.authenticationProvider.setAuthoritiesMapper(authoritiesMapper);
191192
OAuth2LoginAuthenticationToken authentication = (OAuth2LoginAuthenticationToken) this.authenticationProvider
192193
.authenticate(new OAuth2LoginAuthenticationToken(this.clientRegistration, this.authorizationExchange));
193-
assertThat(authentication.getAuthorities()).isEqualTo(mappedAuthorities);
194+
verify(authoritiesMapper).mapAuthorities(any());
195+
SecurityAssertions.assertThat(authentication).authorities().containsAll(mappedAuthorities);
194196
}
195197

196198
// gh-5368

0 commit comments

Comments
 (0)