From 290567183a714f0340fbc4a300a3afda5825883b Mon Sep 17 00:00:00 2001 From: Misagh Moayyed Date: Tue, 10 Oct 2023 18:39:39 +0400 Subject: [PATCH 1/4] allow AuthenticationConverter to be settable in BasicAuthenticationFilter --- .../authentication/www/BasicAuthenticationFilter.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java b/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java index c91099e4bf..7a3cfbd40c 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java @@ -95,9 +95,9 @@ public class BasicAuthenticationFilter extends OncePerRequestFilter { private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder .getContextHolderStrategy(); - private AuthenticationEntryPoint authenticationEntryPoint; + protected AuthenticationEntryPoint authenticationEntryPoint; - private AuthenticationManager authenticationManager; + protected final AuthenticationManager authenticationManager; private RememberMeServices rememberMeServices = new NullRememberMeServices(); @@ -266,6 +266,11 @@ public void setCredentialsCharset(String credentialsCharset) { this.authenticationConverter.setCredentialsCharset(Charset.forName(credentialsCharset)); } + public void setAuthenticationConverter(final BasicAuthenticationConverter authenticationConverter) { + Assert.notNull(rememberMeServices, "authenticationConverter cannot be null"); + this.authenticationConverter = authenticationConverter; + } + protected String getCredentialsCharset(HttpServletRequest httpRequest) { return this.credentialsCharset; } From dd60d3667ca262336e45c4f2c6616a03c13d4c8c Mon Sep 17 00:00:00 2001 From: Misagh Moayyed Date: Tue, 10 Oct 2023 19:04:45 +0400 Subject: [PATCH 2/4] fix null check issue --- .../web/authentication/www/BasicAuthenticationFilter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java b/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java index 7a3cfbd40c..72430fa239 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java @@ -267,7 +267,7 @@ public void setCredentialsCharset(String credentialsCharset) { } public void setAuthenticationConverter(final BasicAuthenticationConverter authenticationConverter) { - Assert.notNull(rememberMeServices, "authenticationConverter cannot be null"); + Assert.notNull(authenticationConverter, "authenticationConverter cannot be null"); this.authenticationConverter = authenticationConverter; } From d30d253f7f8bb69f509d0cf289fcc9ef0c15694a Mon Sep 17 00:00:00 2001 From: Misagh Moayyed Date: Tue, 10 Oct 2023 19:14:55 +0400 Subject: [PATCH 3/4] add test cases to prove settable authentication converter works correctly --- .../www/BasicAuthenticationFilterTests.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/web/src/test/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilterTests.java b/web/src/test/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilterTests.java index 134a5d46f3..d7a3c23f3c 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilterTests.java @@ -21,6 +21,7 @@ import jakarta.servlet.FilterChain; import jakarta.servlet.ServletRequest; import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -81,6 +82,15 @@ public void setUp() { given(this.manager.authenticate(rodRequest)).willReturn(rod); given(this.manager.authenticate(not(eq(rodRequest)))).willThrow(new BadCredentialsException("")); this.filter = new BasicAuthenticationFilter(this.manager, new BasicAuthenticationEntryPoint()); + this.filter.setAuthenticationConverter(new BasicAuthenticationConverter() { + @Override + public UsernamePasswordAuthenticationToken convert(final HttpServletRequest request) { + if (request.getServletPath().equalsIgnoreCase("/ignored-request")) { + return null; + } + return super.convert(request); + } + }); } @AfterEach @@ -106,6 +116,20 @@ public void testGettersSetters() { assertThat(this.filter.getAuthenticationEntryPoint()).isNotNull(); } + @Test + public void testAuthenticationConverterIgnoresRequest() throws Exception { + String token = "NOT_A_VALID_TOKEN_AS_MISSING_COLON"; + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addHeader("Authorization", "Basic " + CodecTestUtils.encodeBase64(token)); + request.setServletPath("/ignored-request"); + request.setSession(new MockHttpSession()); + final MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain chain = mock(FilterChain.class); + this.filter.doFilter(request, response, chain); + assertThat(SecurityContextHolder.getContext().getAuthentication()).isNull(); + assertThat(response.getStatus()).isEqualTo(200); + } + @Test public void testInvalidBasicAuthorizationTokenIsIgnored() throws Exception { String token = "NOT_A_VALID_TOKEN_AS_MISSING_COLON"; From 22777e51f0c95e33ba45fe1e2e25a7f4084f81a2 Mon Sep 17 00:00:00 2001 From: Misagh Moayyed Date: Wed, 11 Oct 2023 09:00:36 +0400 Subject: [PATCH 4/4] address feedback in review - partial --- .../www/BasicAuthenticationFilter.java | 4 ++-- .../www/BasicAuthenticationFilterTests.java | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java b/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java index 72430fa239..e666d62ff2 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilter.java @@ -95,9 +95,9 @@ public class BasicAuthenticationFilter extends OncePerRequestFilter { private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder .getContextHolderStrategy(); - protected AuthenticationEntryPoint authenticationEntryPoint; + private AuthenticationEntryPoint authenticationEntryPoint; - protected final AuthenticationManager authenticationManager; + private AuthenticationManager authenticationManager; private RememberMeServices rememberMeServices = new NullRememberMeServices(); diff --git a/web/src/test/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilterTests.java b/web/src/test/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilterTests.java index d7a3c23f3c..a55ff88953 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/www/BasicAuthenticationFilterTests.java @@ -82,15 +82,6 @@ public void setUp() { given(this.manager.authenticate(rodRequest)).willReturn(rod); given(this.manager.authenticate(not(eq(rodRequest)))).willThrow(new BadCredentialsException("")); this.filter = new BasicAuthenticationFilter(this.manager, new BasicAuthenticationEntryPoint()); - this.filter.setAuthenticationConverter(new BasicAuthenticationConverter() { - @Override - public UsernamePasswordAuthenticationToken convert(final HttpServletRequest request) { - if (request.getServletPath().equalsIgnoreCase("/ignored-request")) { - return null; - } - return super.convert(request); - } - }); } @AfterEach @@ -125,6 +116,15 @@ public void testAuthenticationConverterIgnoresRequest() throws Exception { request.setSession(new MockHttpSession()); final MockHttpServletResponse response = new MockHttpServletResponse(); FilterChain chain = mock(FilterChain.class); + this.filter.setAuthenticationConverter(new BasicAuthenticationConverter() { + @Override + public UsernamePasswordAuthenticationToken convert(final HttpServletRequest request) { + if (request.getServletPath().equalsIgnoreCase("/ignored-request")) { + return null; + } + return super.convert(request); + } + }); this.filter.doFilter(request, response, chain); assertThat(SecurityContextHolder.getContext().getAuthentication()).isNull(); assertThat(response.getStatus()).isEqualTo(200);