-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Prevent using both authorizeRequests
and authorizeHttpRequests
#10574
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
Prevent using both authorizeRequests
and authorizeHttpRequests
#10574
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.
I think it may be valuable to let power users add the Filters manually, but rather prevent both authorizeRequests() and authorizeHttpRequests() from being invoked on the DSL. I also like the simplicity of adding a check on the dsl methods vs iterating over all the Filters. What are your thoughts?
Thanks for the feedback @rwinch. Totally agreed. Users who add filters manually know exactly what they are doing. I'm going to apply the changes. |
FilterSecurityInterceptor
and AuthorizationFilter
on filter chainauthorizeRequests
and authorizeHttpRequests
b1d2965
to
bcd6929
Compare
I've updated the PR to avoid having |
@Override | ||
public void configure(H http) { | ||
ExpressionUrlAuthorizationConfigurer<?> configurer = http |
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.
Can we move this to where we are building http? I'd prefer the configurers not need to know about each other.
bcd6929
to
5e73c5e
Compare
Thanks @rwinch. I've updated the 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.
The code looks great!
The only other change is the PR should be pointed to 5.7 branch rather than main since the ticket is (rightfully) assigned to 5.7
5e73c5e
to
c691fbd
Compare
Closes gh-10573