spring-projects / spring-projects/spring-security
Use only one instance of `successHandler` in `AbstractAuthenticationFilterConfigurer`
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 9.6k
- Forks
- 6.3k
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 52
Description
(Flipped current and expected behavior because it's easier to read)
Current Behavior
AbstractAuthenticationFilterConfigurer currently has 2 instances of AuthenticationSuccessHandler:
https://github.com/spring-projects/spring-security/blob/b932571913d21c1f145578764135203607344245/config/src/main/java/org/springframework/security/config/annotation/web/configurers/AbstractAuthenticationFilterConfigurer.java#L69-L71
This is a problem when supplying your own AuthenticationSuccessHandler because only the defaultSuccessHandler will receive the requestCache:
https://github.com/spring-projects/spring-security/blob/b932571913d21c1f145578764135203607344245/config/src/main/java/org/springframework/security/config/annotation/web/configurers/AbstractAuthenticationFilterConfigurer.java#L277-L279
So you have to manually set the RequestCache on your own AuthenticationSuccessHandler which requires some extra work.
Expected Behavior
AbstractAuthenticationFilterConfigurer should contain one instance of AuthenticationSuccessHandler. If this instance implements a interface that allows setting the RequestCache it will be set during configure.
Additional it would probably be a good idea to also have a flag in the configurer which allows enabling or disabling this behavior.
Context
I'm currently trying to add a custom AuthenticationSuccessHandler and this handler requires the correct RequestCache being set.
There seems to be no way to do this by default so for now I required to add a custom Configurer.
Example pseudocode:
http.oauth2Login(c -> {
final var handler = new MySavedRequestAwareAuthSuccessHandler();
handler.setDefaultTargetUrl("/");
c.successHandler(handler);
// We need to set RequestCache later during build phase once it exists!
http.setSharedObject(MySavedRequestAwareAuthSuccessHandler.class, handler);
})
.with(new CustomConfigurer(), Customizer.withDefaults())
// ...
static class CustomConfigurer extends AbstractHttpConfigurer<CustomConfigurer, HttpSecurity>
{
@Override
public void configure(final HttpSecurity http)
{
super.configure(http);
http.getSharedObject(MySavedRequestAwareAuthSuccessHandler.class)
.setsetRequestCache(http.getSharedObject(RequestCache.class));
}
// ...
}
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with config/src/main/java/org/springframework/security/config/annotation/web/configurers/AbstractAuthenticationFilterConfigurer.java, especially the success-handler fields and configure method around the linked lines. Trace how successHandler and RequestCache are supplied, then verify that a custom AuthenticationSuccessHandler receives the configured RequestCache without a separate custom configurer; the payload does not name a test file.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java, spring
- Domain
- authentication, security
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100