Skip to content

Commit df0f6f8

Browse files
author
Steve Riesenberg
committed
Polish gh-9597
1 parent 925d531 commit df0f6f8

File tree

5 files changed

+27
-64
lines changed

5 files changed

+27
-64
lines changed

config/src/main/java/org/springframework/security/config/annotation/web/configurers/ServletApiConfigurer.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import javax.servlet.http.HttpServletRequest;
2222

2323
import org.springframework.context.ApplicationContext;
24-
import org.springframework.security.authentication.AuthenticationDetailsSource;
2524
import org.springframework.security.authentication.AuthenticationManager;
2625
import org.springframework.security.authentication.AuthenticationTrustResolver;
2726
import org.springframework.security.config.annotation.web.HttpSecurityBuilder;
@@ -91,11 +90,6 @@ public void configure(H http) {
9190
if (trustResolver != null) {
9291
this.securityContextRequestFilter.setTrustResolver(trustResolver);
9392
}
94-
AuthenticationDetailsSource<HttpServletRequest, ?> authenticationDetailsSource = http
95-
.getSharedObject(AuthenticationDetailsSource.class);
96-
if (authenticationDetailsSource != null) {
97-
this.securityContextRequestFilter.setAuthenticationDetailsSource(authenticationDetailsSource);
98-
}
9993
ApplicationContext context = http.getSharedObject(ApplicationContext.class);
10094
if (context != null) {
10195
String[] grantedAuthorityDefaultsBeanNames = context.getBeanNamesForType(GrantedAuthorityDefaults.class);

config/src/test/java/org/springframework/security/config/annotation/web/configurers/ServletApiConfigurerTests.java

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import org.springframework.context.annotation.Bean;
3131
import org.springframework.context.annotation.Configuration;
3232
import org.springframework.security.access.AccessDeniedException;
33-
import org.springframework.security.authentication.AuthenticationDetailsSource;
3433
import org.springframework.security.authentication.AuthenticationManager;
3534
import org.springframework.security.authentication.AuthenticationTrustResolver;
3635
import org.springframework.security.authentication.TestingAuthenticationToken;
@@ -150,15 +149,6 @@ public void configureWhenSharedObjectTrustResolverThenTrustResolverUsed() throws
150149
verify(SharedTrustResolverConfig.TR, atLeastOnce()).isAnonymous(any());
151150
}
152151

153-
@Test
154-
public void configureWhenSharedObjectAuthenticationDetailsSourceThenAuthenticationDetailsSourceUsed() {
155-
this.spring.register(SharedAuthenticationDetailsSourceConfig.class).autowire();
156-
SecurityContextHolderAwareRequestFilter scaFilter = getFilter(SecurityContextHolderAwareRequestFilter.class);
157-
AuthenticationDetailsSource<HttpServletRequest, ?> authenticationDetailsSource = getFieldValue(scaFilter,
158-
"authenticationDetailsSource");
159-
assertThat(authenticationDetailsSource).isEqualTo(SharedAuthenticationDetailsSourceConfig.ADS);
160-
}
161-
162152
@Test
163153
public void requestWhenServletApiWithDefaultsInLambdaThenUsesDefaultRolePrefix() throws Exception {
164154
this.spring.register(ServletApiWithDefaultsInLambdaConfig.class, AdminController.class).autowire();
@@ -331,22 +321,6 @@ protected void configure(HttpSecurity http) {
331321

332322
}
333323

334-
@EnableWebSecurity
335-
static class SharedAuthenticationDetailsSourceConfig extends WebSecurityConfigurerAdapter {
336-
337-
@SuppressWarnings("unchecked")
338-
static AuthenticationDetailsSource<HttpServletRequest, ?> ADS = spy(AuthenticationDetailsSource.class);
339-
340-
@Override
341-
protected void configure(HttpSecurity http) {
342-
// @formatter:off
343-
http
344-
.setSharedObject(AuthenticationDetailsSource.class, ADS);
345-
// @formatter:on
346-
}
347-
348-
}
349-
350324
@EnableWebSecurity
351325
static class ServletApiWithDefaultsInLambdaConfig extends WebSecurityConfigurerAdapter {
352326

web/src/main/java/org/springframework/security/web/servletapi/HttpServlet3RequestFactory.java

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ final class HttpServlet3RequestFactory implements HttpServletRequestFactory {
8181

8282
private AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl();
8383

84-
private AuthenticationDetailsSource<HttpServletRequest, ?> authenticationDetailsSource = new WebAuthenticationDetailsSource();
84+
private final AuthenticationDetailsSource<HttpServletRequest, ?> authenticationDetailsSource = new WebAuthenticationDetailsSource();
8585

8686
private AuthenticationEntryPoint authenticationEntryPoint;
8787

@@ -162,18 +162,6 @@ void setTrustResolver(AuthenticationTrustResolver trustResolver) {
162162
this.trustResolver = trustResolver;
163163
}
164164

165-
/**
166-
* Sets the {@link AuthenticationDetailsSource} to be used. The default is
167-
* {@link WebAuthenticationDetailsSource}.
168-
* @param authenticationDetailsSource the {@link AuthenticationDetailsSource} to use.
169-
* Cannot be null.
170-
*/
171-
void setAuthenticationDetailsSource(
172-
AuthenticationDetailsSource<HttpServletRequest, ?> authenticationDetailsSource) {
173-
Assert.notNull(authenticationDetailsSource, "authenticationDetailsSource cannot be null");
174-
this.authenticationDetailsSource = authenticationDetailsSource;
175-
}
176-
177165
@Override
178166
public HttpServletRequest create(HttpServletRequest request, HttpServletResponse response) {
179167
return new Servlet3SecurityContextHolderAwareRequestWrapper(request, this.rolePrefix, response);

web/src/main/java/org/springframework/security/web/servletapi/SecurityContextHolderAwareRequestFilter.java

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,12 @@
2727
import javax.servlet.http.HttpServletRequest;
2828
import javax.servlet.http.HttpServletResponse;
2929

30-
import org.springframework.security.authentication.AuthenticationDetailsSource;
3130
import org.springframework.security.authentication.AuthenticationManager;
3231
import org.springframework.security.authentication.AuthenticationTrustResolver;
3332
import org.springframework.security.authentication.AuthenticationTrustResolverImpl;
3433
import org.springframework.security.core.context.SecurityContext;
3534
import org.springframework.security.core.context.SecurityContextHolder;
3635
import org.springframework.security.web.AuthenticationEntryPoint;
37-
import org.springframework.security.web.authentication.WebAuthenticationDetailsSource;
3836
import org.springframework.security.web.authentication.logout.LogoutHandler;
3937
import org.springframework.util.Assert;
4038
import org.springframework.web.filter.GenericFilterBean;
@@ -82,8 +80,6 @@ public class SecurityContextHolderAwareRequestFilter extends GenericFilterBean {
8280

8381
private AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl();
8482

85-
private AuthenticationDetailsSource<HttpServletRequest, ?> authenticationDetailsSource = new WebAuthenticationDetailsSource();
86-
8783
public void setRolePrefix(String rolePrefix) {
8884
Assert.notNull(rolePrefix, "Role prefix must not be null");
8985
this.rolePrefix = rolePrefix;
@@ -176,23 +172,9 @@ public void setTrustResolver(AuthenticationTrustResolver trustResolver) {
176172
updateFactory();
177173
}
178174

179-
/**
180-
* Sets the {@link AuthenticationDetailsSource} to be used. The default is
181-
* {@link WebAuthenticationDetailsSource}.
182-
* @param authenticationDetailsSource the {@link AuthenticationDetailsSource} to use.
183-
* Cannot be null.
184-
*/
185-
public void setAuthenticationDetailsSource(
186-
AuthenticationDetailsSource<HttpServletRequest, ?> authenticationDetailsSource) {
187-
Assert.notNull(authenticationDetailsSource, "authenticationDetailsSource cannot be null");
188-
this.authenticationDetailsSource = authenticationDetailsSource;
189-
updateFactory();
190-
}
191-
192175
private HttpServletRequestFactory createServlet3Factory(String rolePrefix) {
193176
HttpServlet3RequestFactory factory = new HttpServlet3RequestFactory(rolePrefix);
194177
factory.setTrustResolver(this.trustResolver);
195-
factory.setAuthenticationDetailsSource(this.authenticationDetailsSource);
196178
factory.setAuthenticationEntryPoint(this.authenticationEntryPoint);
197179
factory.setAuthenticationManager(this.authenticationManager);
198180
factory.setLogoutHandlers(this.logoutHandlers);

web/src/test/java/org/springframework/security/web/servletapi/SecurityContextHolderAwareRequestFilterTests.java

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2004, 2005, 2006 Acegi Technology Pty Limited
2+
* Copyright 2004, 2005, 2006, 2021 Acegi Technology Pty Limited
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -36,6 +36,7 @@
3636

3737
import org.springframework.mock.web.MockHttpServletRequest;
3838
import org.springframework.mock.web.MockHttpServletResponse;
39+
import org.springframework.mock.web.MockHttpSession;
3940
import org.springframework.security.authentication.AuthenticationManager;
4041
import org.springframework.security.authentication.BadCredentialsException;
4142
import org.springframework.security.authentication.TestingAuthenticationToken;
@@ -45,12 +46,14 @@
4546
import org.springframework.security.core.context.SecurityContext;
4647
import org.springframework.security.core.context.SecurityContextHolder;
4748
import org.springframework.security.web.AuthenticationEntryPoint;
49+
import org.springframework.security.web.authentication.WebAuthenticationDetails;
4850
import org.springframework.security.web.authentication.logout.LogoutHandler;
4951
import org.springframework.test.util.ReflectionTestUtils;
5052

5153
import static org.assertj.core.api.Assertions.assertThat;
5254
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
5355
import static org.mockito.ArgumentMatchers.any;
56+
import static org.mockito.ArgumentMatchers.anyBoolean;
5457
import static org.mockito.ArgumentMatchers.anyString;
5558
import static org.mockito.ArgumentMatchers.eq;
5659
import static org.mockito.BDDMockito.given;
@@ -59,6 +62,7 @@
5962
import static org.mockito.Mockito.times;
6063
import static org.mockito.Mockito.verify;
6164
import static org.mockito.Mockito.verifyZeroInteractions;
65+
import static org.mockito.Mockito.when;
6266

6367
/**
6468
* Tests {@link SecurityContextHolderAwareRequestFilter}.
@@ -217,6 +221,27 @@ public void loginNullAuthenticationManagerFail() throws Exception {
217221
verifyZeroInteractions(this.authenticationEntryPoint, this.authenticationManager, this.logoutHandler);
218222
}
219223

224+
@Test
225+
public void loginWhenHttpServletRequestHasAuthenticationDetailsThenAuthenticationRequestHasDetails()
226+
throws Exception {
227+
String ipAddress = "10.0.0.100";
228+
String sessionId = "session-id";
229+
when(this.request.getRemoteAddr()).thenReturn(ipAddress);
230+
when(this.request.getSession(anyBoolean())).thenReturn(new MockHttpSession(null, sessionId));
231+
wrappedRequest().login("username", "password");
232+
233+
ArgumentCaptor<UsernamePasswordAuthenticationToken> authenticationCaptor = ArgumentCaptor
234+
.forClass(UsernamePasswordAuthenticationToken.class);
235+
verify(this.authenticationManager).authenticate(authenticationCaptor.capture());
236+
237+
UsernamePasswordAuthenticationToken authenticationRequest = authenticationCaptor.getValue();
238+
assertThat(authenticationRequest.getDetails()).isInstanceOf(WebAuthenticationDetails.class);
239+
240+
WebAuthenticationDetails details = (WebAuthenticationDetails) authenticationRequest.getDetails();
241+
assertThat(details.getRemoteAddress()).isEqualTo(ipAddress);
242+
assertThat(details.getSessionId()).isEqualTo(sessionId);
243+
}
244+
220245
@Test
221246
public void logout() throws Exception {
222247
TestingAuthenticationToken expectedAuth = new TestingAuthenticationToken("user", "password", "ROLE_USER");

0 commit comments

Comments
 (0)