Skip to content

Commit 62c7de0

Browse files
ttddyyeleftherias
authored andcommitted
Add RequestMatcher to AbstractPreAuthenticatedProcessingFilter
Moved the existing auth check logic to the matcher. Issue: gh-5928
1 parent 63607ee commit 62c7de0

File tree

2 files changed

+93
-34
lines changed

2 files changed

+93
-34
lines changed

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

+54-34
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.springframework.security.core.context.SecurityContextHolder;
3535
import org.springframework.security.web.WebAttributes;
3636
import org.springframework.security.web.authentication.*;
37+
import org.springframework.security.web.util.matcher.RequestMatcher;
3738
import org.springframework.util.Assert;
3839
import org.springframework.web.filter.GenericFilterBean;
3940

@@ -73,6 +74,7 @@
7374
* @author Luke Taylor
7475
* @author Ruud Senden
7576
* @author Rob Winch
77+
* @author Tadaya Tsuyukubo
7678
* @since 2.0
7779
*/
7880
public abstract class AbstractPreAuthenticatedProcessingFilter extends GenericFilterBean
@@ -86,6 +88,7 @@ public abstract class AbstractPreAuthenticatedProcessingFilter extends GenericFi
8688
private boolean invalidateSessionOnPrincipalChange = true;
8789
private AuthenticationSuccessHandler authenticationSuccessHandler = null;
8890
private AuthenticationFailureHandler authenticationFailureHandler = null;
91+
private RequestMatcher requiresAuthenticationRequestMatcher = new PreAuthenticatedProcessingRequestMatcher();
8992

9093
/**
9194
* Check whether all required properties have been set.
@@ -114,7 +117,7 @@ public void doFilter(ServletRequest request, ServletResponse response,
114117
+ SecurityContextHolder.getContext().getAuthentication());
115118
}
116119

117-
if (requiresAuthentication((HttpServletRequest) request)) {
120+
if (requiresAuthenticationRequestMatcher.matches((HttpServletRequest) request)) {
118121
doAuthenticate((HttpServletRequest) request, (HttpServletResponse) response);
119122
}
120123

@@ -193,39 +196,6 @@ private void doAuthenticate(HttpServletRequest request, HttpServletResponse resp
193196
}
194197
}
195198

196-
private boolean requiresAuthentication(HttpServletRequest request) {
197-
Authentication currentUser = SecurityContextHolder.getContext()
198-
.getAuthentication();
199-
200-
if (currentUser == null) {
201-
return true;
202-
}
203-
204-
if (!checkForPrincipalChanges) {
205-
return false;
206-
}
207-
208-
if (!principalChanged(request, currentUser)) {
209-
return false;
210-
}
211-
212-
logger.debug("Pre-authenticated principal has changed and will be reauthenticated");
213-
214-
if (invalidateSessionOnPrincipalChange) {
215-
SecurityContextHolder.clearContext();
216-
217-
HttpSession session = request.getSession(false);
218-
219-
if (session != null) {
220-
logger.debug("Invalidating existing session");
221-
session.invalidate();
222-
request.getSession();
223-
}
224-
}
225-
226-
return true;
227-
}
228-
229199
/**
230200
* Puts the <code>Authentication</code> instance returned by the authentication
231201
* manager into the secure context.
@@ -348,6 +318,14 @@ public void setAuthenticationFailureHandler(AuthenticationFailureHandler authent
348318
this.authenticationFailureHandler = authenticationFailureHandler;
349319
}
350320

321+
/**
322+
* Sets the request matcher to check whether to proceed the request further.
323+
*/
324+
public void setRequiresAuthenticationRequestMatcher(RequestMatcher requiresAuthenticationRequestMatcher) {
325+
Assert.notNull(requiresAuthenticationRequestMatcher, "requestMatcher cannot be null");
326+
this.requiresAuthenticationRequestMatcher = requiresAuthenticationRequestMatcher;
327+
}
328+
351329
/**
352330
* Override to extract the principal information from the current request
353331
*/
@@ -359,4 +337,46 @@ public void setAuthenticationFailureHandler(AuthenticationFailureHandler authent
359337
* return a dummy value.
360338
*/
361339
protected abstract Object getPreAuthenticatedCredentials(HttpServletRequest request);
340+
341+
/**
342+
* Request matcher for default auth check logic
343+
*/
344+
private class PreAuthenticatedProcessingRequestMatcher implements RequestMatcher {
345+
346+
@Override
347+
public boolean matches(HttpServletRequest request) {
348+
349+
Authentication currentUser = SecurityContextHolder.getContext().getAuthentication();
350+
351+
if (currentUser == null) {
352+
return true;
353+
}
354+
355+
if (!checkForPrincipalChanges) {
356+
return false;
357+
}
358+
359+
if (!principalChanged(request, currentUser)) {
360+
return false;
361+
}
362+
363+
logger.debug("Pre-authenticated principal has changed and will be reauthenticated");
364+
365+
if (invalidateSessionOnPrincipalChange) {
366+
SecurityContextHolder.clearContext();
367+
368+
HttpSession session = request.getSession(false);
369+
370+
if (session != null) {
371+
logger.debug("Invalidating existing session");
372+
session.invalidate();
373+
request.getSession();
374+
}
375+
}
376+
377+
return true;
378+
}
379+
380+
}
381+
362382
}

web/src/test/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilterTests.java

+39
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,12 @@
4545
import org.springframework.security.web.WebAttributes;
4646
import org.springframework.security.web.authentication.ForwardAuthenticationFailureHandler;
4747
import org.springframework.security.web.authentication.ForwardAuthenticationSuccessHandler;
48+
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
4849

4950
/**
5051
*
5152
* @author Rob Winch
53+
* @author Tadaya Tsuyukubo
5254
*
5355
*/
5456
public class AbstractPreAuthenticatedProcessingFilterTests {
@@ -376,6 +378,43 @@ protected boolean principalChanged(HttpServletRequest request,
376378
verifyZeroInteractions(am);
377379
}
378380

381+
@Test
382+
public void requestNotMatchRequestMatcher() throws Exception {
383+
MockHttpServletRequest request = new MockHttpServletRequest();
384+
MockHttpServletResponse response = new MockHttpServletResponse();
385+
MockFilterChain chain = new MockFilterChain();
386+
387+
ConcretePreAuthenticatedProcessingFilter filter = new ConcretePreAuthenticatedProcessingFilter();
388+
filter.setRequiresAuthenticationRequestMatcher(new AntPathRequestMatcher("/no-matching"));
389+
390+
AuthenticationManager am = mock(AuthenticationManager.class);
391+
filter.setAuthenticationManager(am);
392+
filter.afterPropertiesSet();
393+
394+
filter.doFilter(request, response, chain);
395+
396+
verifyZeroInteractions(am);
397+
}
398+
399+
@Test
400+
public void requestMatchesRequestMatcher() throws Exception {
401+
MockHttpServletRequest request = new MockHttpServletRequest();
402+
MockHttpServletResponse response = new MockHttpServletResponse();
403+
MockFilterChain chain = new MockFilterChain();
404+
405+
ConcretePreAuthenticatedProcessingFilter filter = new ConcretePreAuthenticatedProcessingFilter();
406+
filter.setRequiresAuthenticationRequestMatcher(new AntPathRequestMatcher("/**"));
407+
408+
AuthenticationManager am = mock(AuthenticationManager.class);
409+
filter.setAuthenticationManager(am);
410+
filter.afterPropertiesSet();
411+
412+
filter.doFilter(request, response, chain);
413+
414+
verify(am).authenticate(any(PreAuthenticatedAuthenticationToken.class));
415+
416+
}
417+
379418
private void testDoFilter(boolean grantAccess) throws Exception {
380419
MockHttpServletRequest req = new MockHttpServletRequest();
381420
MockHttpServletResponse res = new MockHttpServletResponse();

0 commit comments

Comments
 (0)