-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Add option to disable anonymous authentication in RSocketSecurity
#17159
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
Conversation
RSocketSecurity
RSocketSecurity
Hmmm, is the build going down by my mistake or is it not about me? |
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, @therepanic! I've left feedback inline.
Also, are you able to add a test that confirms that when anonymous is disabled then it isn't active in the filter chain?
...ig/src/main/java/org/springframework/security/config/annotation/rsocket/RSocketSecurity.java
Outdated
Show resolved
Hide resolved
Thanks for review, @jzheaux! I'd be happy to add the test. I just have one question about it. I think we could make a test that checks the functionality you described by creating a new test class, let's call it |
@therepanic, that sounds like a good approach. Also, you might consider a test like
AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl();
ReactiveAuthorizationManager<PayloadExchange> anonymous = (authentication, exchange) ->
authentication.map(trustResolver::isAnonymous).map(AuthorizationDecision::new);
// ...
.anyExchange().access(anonymous) |
7defe58
to
4496a48
Compare
...java/org/springframework/security/config/annotation/rsocket/RSocketMessageHandlerITests.java
Outdated
Show resolved
Hide resolved
Thanks for review @jzheaux again. Yes, you are right, I did make a mistake. In a separate commit, for your convenience, I added the creation of an anonymousAuthSpec instance by default, and also added a disable method to |
Closes: spring-projectsgh-17132 Signed-off-by: Andrey Litvitski <[email protected]> 1 Signed-off-by: Andrey Litvitski <[email protected]> 1 Signed-off-by: Andrey Litvitski <[email protected]>
Changed the DSL method name to anonymous to align with jwt. Since basicAuthenication is deprecated, we don't need to align with its naming convention. Also added a since attribute to the method. Issue spring-projectsgh-17132
Thanks for your help with this, @therepanic! This will merge to |
Adding anonymous interceptor hardcoded to
RSocketSecurity#payloadInterceptors
and is added anyway. I think you should add an option to disable adding anonymous interceptor.I think the most logical way is to add a flag, as in our case I did. I also added a method
disableAnonymous
which sets the field to true and we no longer add the anonymous interceptor.Fixes: #17132