Skip to content

Commit b1a50a2

Browse files
jzheauxjzheaux
committed
Check If toBuilder Is Implemented
Since RC1 is right around the corner, let's change the API footprint as little as possible by using reflection to check if a class has declared toBuilder themselves. If they have, we can assume that that class's builder will produce that class. Issue gh-18052
1 parent 4281f6b commit b1a50a2

13 files changed

Lines changed: 214 additions & 6 deletions

File tree

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/*
2+
* Copyright 2004-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.security.authentication;
18+
19+
public class NonBuildableAuthenticationToken extends TestingAuthenticationToken {
20+
21+
public NonBuildableAuthenticationToken(String user, String password, String... authorities) {
22+
super(user, password, authorities);
23+
}
24+
25+
}

‎oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/web/authentication/BearerTokenAuthenticationFilter.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.security.oauth2.server.resource.web.authentication;
1818

1919
import java.io.IOException;
20+
import java.lang.reflect.Method;
2021
import java.util.Map;
2122
import java.util.Set;
2223
import java.util.stream.Collectors;
@@ -56,7 +57,6 @@
5657
import org.springframework.security.web.context.SecurityContextRepository;
5758
import org.springframework.util.Assert;
5859
import org.springframework.util.CollectionUtils;
59-
import org.springframework.util.ReflectionUtils;
6060
import org.springframework.util.StringUtils;
6161
import org.springframework.web.filter.OncePerRequestFilter;
6262

@@ -214,7 +214,12 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
214214
}
215215

216216
private static boolean declaresToBuilder(Authentication authentication) {
217-
return ReflectionUtils.findMethod(authentication.getClass(), "toBuilder") != null;
217+
for (Method method : authentication.getClass().getDeclaredMethods()) {
218+
if (method.getName().equals("toBuilder") && method.getParameterTypes().length == 0) {
219+
return true;
220+
}
221+
}
222+
return false;
218223
}
219224

220225
/**

‎oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/web/authentication/BearerTokenAuthenticationFilterTests.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.springframework.security.authentication.AuthenticationManager;
3939
import org.springframework.security.authentication.AuthenticationManagerResolver;
4040
import org.springframework.security.authentication.AuthenticationServiceException;
41+
import org.springframework.security.authentication.NonBuildableAuthenticationToken;
4142
import org.springframework.security.authentication.SecurityAssertions;
4243
import org.springframework.security.authentication.TestingAuthenticationToken;
4344
import org.springframework.security.core.Authentication;
@@ -316,6 +317,24 @@ void doFilterWhenDefaultEqualsGrantedAuthorityThenNoDuplicates() throws Exceptio
316317
// @formatter:on
317318
}
318319

320+
@Test
321+
void doFilterWhenNonBuildableAuthenticationSubclassThenSkipsToBuilder() throws Exception {
322+
TestingAuthenticationToken existingAuthn = new TestingAuthenticationToken("username", "password", "FACTORONE");
323+
SecurityContextHolder.setContext(new SecurityContextImpl(existingAuthn));
324+
given(this.authenticationManager.authenticate(any()))
325+
.willReturn(new NonBuildableAuthenticationToken("username", "password", "FACTORTWO"));
326+
given(this.bearerTokenResolver.resolve(any())).willReturn("token");
327+
BearerTokenAuthenticationFilter filter = addMocks(
328+
new BearerTokenAuthenticationFilter(this.authenticationManager));
329+
filter.doFilter(this.request, this.response, this.filterChain);
330+
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
331+
// @formatter:off
332+
SecurityAssertions.assertThat(authentication).authorities()
333+
.extracting(GrantedAuthority::getAuthority)
334+
.containsExactly("FACTORTWO");
335+
// @formatter:on
336+
}
337+
319338
@Test
320339
public void setAuthenticationEntryPointWhenNullThenThrowsException() {
321340
BearerTokenAuthenticationFilter filter = new BearerTokenAuthenticationFilter(this.authenticationManager);

‎web/src/main/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilter.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.security.web.authentication;
1818

1919
import java.io.IOException;
20+
import java.lang.reflect.Method;
2021
import java.util.Set;
2122
import java.util.stream.Collectors;
2223

@@ -252,7 +253,7 @@ private void doFilter(HttpServletRequest request, HttpServletResponse response,
252253
return;
253254
}
254255
Authentication current = this.securityContextHolderStrategy.getContext().getAuthentication();
255-
if (current != null && current.isAuthenticated()) {
256+
if (current != null && current.isAuthenticated() && declaresToBuilder(authenticationResult)) {
256257
authenticationResult = authenticationResult.toBuilder()
257258
// @formatter:off
258259
.authorities((a) -> {
@@ -285,6 +286,15 @@ private void doFilter(HttpServletRequest request, HttpServletResponse response,
285286
}
286287
}
287288

289+
private static boolean declaresToBuilder(Authentication authentication) {
290+
for (Method method : authentication.getClass().getDeclaredMethods()) {
291+
if (method.getName().equals("toBuilder") && method.getParameterTypes().length == 0) {
292+
return true;
293+
}
294+
}
295+
return false;
296+
}
297+
288298
/**
289299
* Indicates whether this filter should attempt to process a login request for the
290300
* current invocation.

‎web/src/main/java/org/springframework/security/web/authentication/AuthenticationFilter.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.security.web.authentication;
1818

1919
import java.io.IOException;
20+
import java.lang.reflect.Method;
2021
import java.util.Set;
2122
import java.util.stream.Collectors;
2223

@@ -188,7 +189,7 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
188189
return;
189190
}
190191
Authentication current = this.securityContextHolderStrategy.getContext().getAuthentication();
191-
if (current != null && current.isAuthenticated()) {
192+
if (current != null && current.isAuthenticated() && declaresToBuilder(authenticationResult)) {
192193
authenticationResult = authenticationResult.toBuilder()
193194
// @formatter:off
194195
.authorities((a) -> {
@@ -215,6 +216,15 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
215216
}
216217
}
217218

219+
private static boolean declaresToBuilder(Authentication authentication) {
220+
for (Method method : authentication.getClass().getDeclaredMethods()) {
221+
if (method.getName().equals("toBuilder") && method.getParameterTypes().length == 0) {
222+
return true;
223+
}
224+
}
225+
return false;
226+
}
227+
218228
@Override
219229
protected String getAlreadyFilteredAttributeName() {
220230
String name = getFilterName();

‎web/src/main/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilter.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.security.web.authentication.preauth;
1818

1919
import java.io.IOException;
20+
import java.lang.reflect.Method;
2021
import java.util.Set;
2122
import java.util.stream.Collectors;
2223

@@ -208,7 +209,7 @@ private void doAuthenticate(HttpServletRequest request, HttpServletResponse resp
208209
authenticationRequest.setDetails(this.authenticationDetailsSource.buildDetails(request));
209210
Authentication authenticationResult = this.authenticationManager.authenticate(authenticationRequest);
210211
Authentication current = this.securityContextHolderStrategy.getContext().getAuthentication();
211-
if (current != null && current.isAuthenticated()) {
212+
if (current != null && current.isAuthenticated() && declaresToBuilder(authenticationResult)) {
212213
authenticationResult = authenticationResult.toBuilder()
213214
// @formatter:off
214215
.authorities((a) -> {
@@ -234,6 +235,15 @@ private void doAuthenticate(HttpServletRequest request, HttpServletResponse resp
234235
}
235236
}
236237

238+
private static boolean declaresToBuilder(Authentication authentication) {
239+
for (Method method : authentication.getClass().getDeclaredMethods()) {
240+
if (method.getName().equals("toBuilder") && method.getParameterTypes().length == 0) {
241+
return true;
242+
}
243+
}
244+
return false;
245+
}
246+
237247
/**
238248
* Puts the <code>Authentication</code> instance returned by the authentication
239249
* manager into the secure context.

‎web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.security.web.authentication.www;
1818

1919
import java.io.IOException;
20+
import java.lang.reflect.Method;
2021
import java.nio.charset.Charset;
2122
import java.util.Set;
2223
import java.util.stream.Collectors;
@@ -190,7 +191,7 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
190191
if (authenticationIsRequired(username)) {
191192
Authentication authResult = this.authenticationManager.authenticate(authRequest);
192193
Authentication current = this.securityContextHolderStrategy.getContext().getAuthentication();
193-
if (current != null && current.isAuthenticated()) {
194+
if (current != null && current.isAuthenticated() && declaresToBuilder(authResult)) {
194195
authResult = authResult.toBuilder()
195196
// @formatter:off
196197
.authorities((a) -> {
@@ -234,6 +235,15 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
234235
chain.doFilter(request, response);
235236
}
236237

238+
private static boolean declaresToBuilder(Authentication authentication) {
239+
for (Method method : authentication.getClass().getDeclaredMethods()) {
240+
if (method.getName().equals("toBuilder") && method.getParameterTypes().length == 0) {
241+
return true;
242+
}
243+
}
244+
return false;
245+
}
246+
237247
protected boolean authenticationIsRequired(String username) {
238248
// Only reauthenticate if username doesn't match SecurityContextHolder and user
239249
// isn't authenticated (see SEC-53)

‎web/src/main/java/org/springframework/security/web/server/authentication/AuthenticationWebFilter.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.security.web.server.authentication;
1818

19+
import java.lang.reflect.Method;
1920
import java.util.Set;
2021
import java.util.function.Function;
2122
import java.util.stream.Collectors;
@@ -141,6 +142,9 @@ private Mono<Authentication> applyCurrentAuthenication(Authentication result) {
141142
if (!current.isAuthenticated()) {
142143
return result;
143144
}
145+
if (!declaresToBuilder(result)) {
146+
return result;
147+
}
144148
return result.toBuilder()
145149
// @formatter:off
146150
.authorities((a) -> {
@@ -158,6 +162,15 @@ private Mono<Authentication> applyCurrentAuthenication(Authentication result) {
158162
}).switchIfEmpty(Mono.just(result));
159163
}
160164

165+
private static boolean declaresToBuilder(Authentication authentication) {
166+
for (Method method : authentication.getClass().getDeclaredMethods()) {
167+
if (method.getName().equals("toBuilder") && method.getParameterTypes().length == 0) {
168+
return true;
169+
}
170+
}
171+
return false;
172+
}
173+
161174
protected Mono<Void> onAuthenticationSuccess(Authentication authentication, WebFilterExchange webFilterExchange) {
162175
ServerWebExchange exchange = webFilterExchange.getExchange();
163176
SecurityContextImpl securityContext = new SecurityContextImpl();

‎web/src/test/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilterTests.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
import org.springframework.security.authentication.AuthenticationManager;
3838
import org.springframework.security.authentication.BadCredentialsException;
3939
import org.springframework.security.authentication.InternalAuthenticationServiceException;
40+
import org.springframework.security.authentication.NonBuildableAuthenticationToken;
41+
import org.springframework.security.authentication.SecurityAssertions;
4042
import org.springframework.security.authentication.TestAuthentication;
4143
import org.springframework.security.authentication.TestingAuthenticationToken;
4244
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
@@ -480,6 +482,22 @@ void doFilterWhenDefaultEqualsAuthorityThenNoDuplicates() throws Exception {
480482
.containsExactly(DefaultEqualsGrantedAuthority.AUTHORITY);
481483
}
482484

485+
@Test
486+
void doFilterWhenNotOverridingToBuilderThenDoesNotMergeAuthorities() throws Exception {
487+
TestingAuthenticationToken existingAuthn = new TestingAuthenticationToken("username", "password", "FACTORONE");
488+
SecurityContextHolder.setContext(new SecurityContextImpl(existingAuthn));
489+
MockHttpServletRequest request = createMockAuthenticationRequest();
490+
MockHttpServletResponse response = new MockHttpServletResponse();
491+
MockAuthenticationFilter filter = new MockAuthenticationFilter(
492+
new NonBuildableAuthenticationToken("username", "password", "FACTORTWO"));
493+
filter.doFilter(request, response, new MockFilterChain(false));
494+
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
495+
SecurityAssertions.assertThat(authentication)
496+
.authorities()
497+
.extracting(GrantedAuthority::getAuthority)
498+
.containsExactly("FACTORTWO");
499+
}
500+
483501
/**
484502
* https://github.com/spring-projects/spring-security/pull/3905
485503
*/

‎web/src/test/java/org/springframework/security/web/authentication/AuthenticationFilterTests.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
import org.springframework.security.authentication.AuthenticationManager;
3838
import org.springframework.security.authentication.AuthenticationManagerResolver;
3939
import org.springframework.security.authentication.BadCredentialsException;
40+
import org.springframework.security.authentication.NonBuildableAuthenticationToken;
41+
import org.springframework.security.authentication.SecurityAssertions;
4042
import org.springframework.security.authentication.TestingAuthenticationToken;
4143
import org.springframework.security.core.Authentication;
4244
import org.springframework.security.core.GrantedAuthority;
@@ -348,6 +350,26 @@ public void doFilterWhenDefaultEqualsGrantedAuthorityThenNoDuplicates() throws E
348350
.containsExactlyInAnyOrder(DefaultEqualsGrantedAuthority.AUTHORITY);
349351
}
350352

353+
@Test
354+
void doFilterWhenNotOverridingToBuilderThenDoesNotMergeAuthorities() throws Exception {
355+
TestingAuthenticationToken existingAuthn = new TestingAuthenticationToken("username", "password", "FACTORONE");
356+
SecurityContextHolder.setContext(new SecurityContextImpl(existingAuthn));
357+
given(this.authenticationConverter.convert(any())).willReturn(existingAuthn);
358+
given(this.authenticationManager.authenticate(any()))
359+
.willReturn(new NonBuildableAuthenticationToken("user", "password", "FACTORTWO"));
360+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/");
361+
MockHttpServletResponse response = new MockHttpServletResponse();
362+
FilterChain chain = new MockFilterChain();
363+
AuthenticationFilter filter = new AuthenticationFilter(this.authenticationManager,
364+
this.authenticationConverter);
365+
filter.doFilter(request, response, chain);
366+
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
367+
SecurityAssertions.assertThat(authentication)
368+
.authorities()
369+
.extracting(GrantedAuthority::getAuthority)
370+
.containsExactly("FACTORTWO");
371+
}
372+
351373
@Test
352374
public void filterWhenCustomSecurityContextRepositoryAndSuccessfulAuthenticationRepositoryUsed() throws Exception {
353375
SecurityContextRepository securityContextRepository = mock(SecurityContextRepository.class);

0 commit comments

Comments
 (0)