Skip to content

authorizeHttpRequests should pick up AuthorizationManager bean #11067

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

Closed
Tracked by #11066
jzheaux opened this issue Apr 6, 2022 · 1 comment
Closed
Tracked by #11066

authorizeHttpRequests should pick up AuthorizationManager bean #11067

jzheaux opened this issue Apr 6, 2022 · 1 comment
Assignees
Labels
in: web An issue in web modules (web, webmvc) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@jzheaux
Copy link
Contributor

jzheaux commented Apr 6, 2022

authorizeHttpRequests replaces authorizeRequests. Specifically, it presents applications with the option to use a simplified API for programmatic authorization through AuthorizationManager.

It would be nice to pick up authorization manager @Beans and apply them by default. This would simplify constructs like:

@Bean 
SecurityFilterChain web(HttpSecurity http, AuthorizationManager<RequestAuthorizationContext> manager) throws Exception {
    http
        .authorizeRequests((authorize) -> authorize
            .anyRequest().access(manager)
        )
        // ...
}

@Bean 
AuthorizationManager<RequestAuthorizationContext> manager() {
    return AuthorityAuthorizationManager.hasRole("USER");
}

to become:

@Bean 
SecurityFilterChain web(HttpSecurity http) throws Exception {
    http
        .authorizeRequests(Customizer.withDefaults())
        // ...
}

@Bean 
AuthorizationManager<HttpServletRequest> manager() {
    return AuthorityAuthorizationManager.hasRole("USER");
}

Then, applications can specify the authorization subsystem simply by publishing a bean.

@jzheaux jzheaux self-assigned this Apr 6, 2022
@jzheaux jzheaux added in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement labels Apr 6, 2022
@jzheaux jzheaux added this to the 5.7.0-RC1 milestone Apr 6, 2022
jzheaux added a commit that referenced this issue Apr 8, 2022
@jzheaux jzheaux closed this as completed in 4ca5346 Apr 8, 2022
@jzheaux
Copy link
Contributor Author

jzheaux commented Apr 12, 2022

After having added this, I've had some second thoughts. While it is nice to reduce DSL boilerplate, that's not historically the primary motivation for accepting a @Bean. Generally when we add @Bean support to the DSL, it's because we anticipate reuse of that bean in other parts of the application or it's because we need that component to participate in the Framework's bean lifecycle.

The nice thing about the DSL is that you have all your security configurations in one discoverable location, having all the benefits of auto-complete and the associated documentation. So, I think what we'd want to do first is consider adding an authorizationManager method to authorizeHttpRequests.

In the context of a Boot application where several HttpSecurity defaults are already specified, a more effective way to reduce DSL boilerplate is likely to introduce some kind of HttpSecurityCustomizer so the entire HttpSecurity can be post-processed instead of just authorization.

That said, I'm open to the possibility that there are compelling cases out there that having picking up an AuthorizationManager @Bean really simplifies. This ticket can be revisited if such a use case surfaces.

@jzheaux jzheaux added the status: declined A suggestion or change that we don't feel we should currently apply label Apr 12, 2022
@jzheaux jzheaux removed this from the 5.7.0-RC1 milestone Apr 12, 2022
jzheaux added a commit that referenced this issue Apr 12, 2022
jzheaux added a commit that referenced this issue Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

1 participant