AbstractAuthenticationFilterConfigurer: add defaultSuccessUrl() function which doesn't override the successHandler field, or the variant with successHandler parameter
Expected Behavior
For FormLoginConfigurer, builder chain defaultSuccessUrl().<...>.successHandler() applies results from both methods
Current Behavior Because defaultSuccessUrl() creates its own successHandler inside, and calls successHandler() as well, we get 2 possible errors when we call them both in the chain, depending on the order:
- defaultSuccessUrl().successHandler(): We lose the result of defaultSuccessUrl
- .successHandler().defaultSuccessUrl(): We lose the result of successHandler
Context I tried to build a chain for FormLoginConfigurer with both custom successHandler and providing the defaultSuccessURL. The issue is not blocking and it was possibly to work around, by removing defaultSuccessUrl() builder call from the chain and recreating the whole function for the handler which I've used as parameter for .successHandler()
Suggested solutions
- create a variant for defaultSuccessUrl which does not override the successHandler
- create a variant for defaultSuccessUrl with successHandler as input parameter
What you've described is the intended design. Since both resolve to a successHandler (defaultSuccessUrl is a shortcut for a pre-defined success handler), then whichever one is called last is the one that takes precedence.
I tried to build a chain for FormLoginConfigurer with both custom successHandler and providing the defaultSuccessURL.
What application behavior was your aim when trying to do this?
@jzheaux defaultSuccessUrl was initially added for redirecting on success, and another successHandler was added later for custom functionality, that had to trigger on successful login. I understood the intentionality behind defaultSuccessURL being a shortcut, hence I opened the issue as enhancement, if it makes sense to use already applied successHandler in defaultSuccessUrl (or provide such an option with backward compatibility in mind)
if it makes sense to use already applied successHandler in defaultSuccessUrl
It does not as this would controvert the existing design. I believe you are asking for something like this:
defaultSuccesUrl(String url) {
if (this.successHandler != null) {
this.successHandler = new SavedRequestAwareAuthenticationSuccessHandler();
// ...
}
return this;
}
Ultimately, these two settings, regardless of behavior, resolve to the same configuration point. There is no way to provide both a defaultSuccessUrl and a successHandler and know what to do with both of them. Because of that, one will have to take precedence over the other. And it sounds like you were surprised to learn that Spring Security uses "last-one-wins" instead of "first-one-wins".
If you are indeed asking for first-one-wins, then I'll need to decline the request as this would need to be decided across the DSL as a whole. Last-one-wins was chosen because it tends to simplify the logic, especially when there are more than two ways to generate the underlying component. It's also nice because then an application can publish an HttpSecurity as a prototype with reasonable defaults that someone dependent on that bean can override.
Or if that's not what you are saying, I'll need to better understand the code you were trying to write and what your expectation was for that code. It may be that you need to create a custom success handler that can differentiate between when to use your success url and when to do something else, but without more detail, I'm just speculating.
Can you please provide a code snippet and explain what you would have expected Spring Security to do with the two values?
If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.
Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.