-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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.
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
Add Reactive Support for UserDetailsChecker #6229
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! I have replied inline
...ingframework/security/authentication/UserDetailsRepositoryReactiveAuthenticationManager.java
Outdated
Show resolved
Hide resolved
...ingframework/security/authentication/UserDetailsRepositoryReactiveAuthenticationManager.java
Outdated
Show resolved
Hide resolved
...ity/oauth2/server/resource/authentication/JwtUserDetailsCheckingAuthenticationConverter.java
Outdated
Show resolved
Hide resolved
...h2/server/resource/authentication/ReactiveJwtUserDetailsCheckingAuthenticationConverter.java
Outdated
Show resolved
Hide resolved
...auth2/server/resource/authentication/JwtUserDetailsCheckingAuthenticationConverterTests.java
Outdated
Show resolved
Hide resolved
...rver/resource/authentication/ReactiveJwtUserDetailsCheckingAuthenticationConverterTests.java
Outdated
Show resolved
Hide resolved
I made the requested changes & rebased the commits into a single commit. I'll open a separate ticket for the |
I opened #6237 for the other part of what I removed from this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the the fast turnaround. I have provided additional feedback inline
...amework/security/authentication/UserDetailsRepositoryReactiveAuthenticationManagerTests.java
Outdated
Show resolved
Hide resolved
...amework/security/authentication/UserDetailsRepositoryReactiveAuthenticationManagerTests.java
Show resolved
Hide resolved
Added a new commit with requested changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @edeandrea! Can you please squash your commits and change the commit subject to be a little more descriptive? Something like Add Reactive Support for UserDetailsChecker
Integrate UserDetailsChecker into ReactiveAuthenticationManager and OAuth2 resource server authentication converters. Fixes gh-6219
Done! |
Integrate UserDetailsChecker into ReactiveAuthenticationManager and
OAuth2 resource server authentication converters.
Fixes gh-6219