Make authenticationSuccessHandler Configurable in Resource Server DSL#18895
Conversation
jzheaux
left a comment
There was a problem hiding this comment.
Thanks, @iain-henderson! I've left some feedback inline. Will you also please add a unit test to OAuth2ResourceServerSpecTests to confirm the authenticationSuccessHandler is used when specified?
|
In addition to the review comments, will you please make sure to sign the DCO? |
1b5f59b to
c342b89
Compare
|
DCO Signoff fixed and unit tests added. Please review to see if changes cover what is needed. |
Signed-off-by: Iain Henderson <Iain.henderson@mac.com>
4a329ed to
3329884
Compare
Signed-off-by: Josh Cummings <3627351+jzheaux@users.noreply.github.com>
Signed-off-by: Josh Cummings <3627351+jzheaux@users.noreply.github.com>
Signed-off-by: Josh Cummings <3627351+jzheaux@users.noreply.github.com>
|
@iain-henderson can you clarify for me what you are needing this for? Is the primary motivation to address what feels like an oversight? |
|
I've added some polish as well as the symmetric Kotlin support. However, to be clear, I'd like to wait until we have a clear use case to add this feature. |
|
@jzheaux Just to clarify, my primary motivation is to address an oversight. Our implementation supports multiple authentication methods (including OAUTH) and we were using successHandlers on all of these, but we had to re-implement our success handlers as a WebFilter to work around this oversight. It actually might be more maintainable the way it is (it matches our non-reactive implementation), but I'm sure we aren't the only ones impacted by this. |
…spec-success-handler
…spec-success-handler
The lack of a configurable ServerAuthenticationSuccessHandler in OAuth2ResourceServerSpec seemed like an oversight.
This PR corrects that.