Skip to content

Fix to allow multiple AuthenticationFilter instances to process each request #17186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
+28 −2
Closed

Fix to allow multiple AuthenticationFilter instances to process each request #17186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion web/src/main/java/org/springframework/security/web/authentication/AuthenticationFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -64,6 +64,7 @@
* </ul>
*
* @author Sergey Bespalov
* @author Andrey Litvitski
* @since 5.2.0
*/
public class AuthenticationFilter extends OncePerRequestFilter {
Expand Down Expand Up @@ -222,4 +223,13 @@ private Authentication attemptAuthentication(HttpServletRequest request, HttpSer
return authenticationResult;
}

@Override
protected String getAlreadyFilteredAttributeName() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized that this is a public API change and therefore cannot be applied in 6.5.1 release. We'll need to address this issue with AuthenticationFilter in a separate issue targeted for 7.0.

For now, we need to address gh-17173 and I think the easiest solution is to define the following in DPoPAuthenticationConfigurer:

private static final class DPoPAuthenticationFilter extends AuthenticationFilter {

	private DPoPAuthenticationFilter(AuthenticationManager authenticationManager, AuthenticationConverter authenticationConverter) {
		super(authenticationManager, authenticationConverter);
	}

}

I think this should solve the bug.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@therepanic Please hold off on the proposed change in my previous comment. The root issue is in AuthenticationFilter and we still need to address it as a bug. We might still be able to get your change into 6.5.1 but let me think about this for a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the advice. You want me to create a separate issue and link the PR to it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@therepanic See comment

String name = getFilterName();
if (name == null) {
name = getClass().getName().concat("-" + System.identityHashCode(this));
}
return name + ALREADY_FILTERED_SUFFIX;
}

}
18 changes: 17 additions & 1 deletion .../test/java/org/springframework/security/web/authentication/AuthenticationFilterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -17,6 +17,7 @@
package org.springframework.security.web.authentication;

import jakarta.servlet.FilterChain;
import jakarta.servlet.Servlet;
import jakarta.servlet.ServletException;
import jakarta.servlet.ServletRequest;
import jakarta.servlet.ServletResponse;
Expand Down Expand Up @@ -57,6 +58,7 @@

/**
* @author Sergey Bespalov
* @author Andrey Litvitski
* @since 5.2.0
*/
@ExtendWith(MockitoExtension.class)
Expand Down Expand Up @@ -318,4 +320,18 @@ public void filterWhenCustomSecurityContextRepositoryAndSuccessfulAuthentication
assertThat(securityContextArg.getValue().getAuthentication()).isEqualTo(authentication);
}

@Test
public void filterWhenInFilterChainThenBothAreExecuted() throws Exception {
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/");
MockHttpServletResponse response = new MockHttpServletResponse();
AuthenticationFilter filter1 = new AuthenticationFilter(this.authenticationManager,
this.authenticationConverter);
AuthenticationConverter converter2 = mock(AuthenticationConverter.class);
AuthenticationFilter filter2 = new AuthenticationFilter(this.authenticationManager, converter2);
FilterChain chain = new MockFilterChain(mock(Servlet.class), filter1, filter2);
chain.doFilter(request, response);
verify(this.authenticationConverter).convert(any());
verify(converter2).convert(any());
}

}