Skip to content
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

HttpSecurity bean has no option to disable defaults #11633

Open
filiphr opened this issue Jul 27, 2022 · 11 comments
Open

HttpSecurity bean has no option to disable defaults #11633

filiphr opened this issue Jul 27, 2022 · 11 comments
Labels
in: config An issue in spring-security-config type: enhancement A general enhancement

Comments

@filiphr
Copy link
Contributor

filiphr commented Jul 27, 2022

Starting from 5.7 the WebSecurityConfigurerAdapter has been deprecated in favour of using a custom bean for creating a SecurityFilterChain that will inject an HttpSecurity and build it.

The WebSecurityConfigurerAdapter has a constructor field disableDefaults which was used to determine whether defaults should be applied to the configuration or not.

When that flag was set to true then the default configuration and default configurers would not be applied to it. When using the new recommended approach there is no way to disable these defaults.

What would be the recommended approach for disabling the defaults.

Does it perhaps make sense to have an interface like:

public interface HttpSecurityProvider {

    default HttpSecurity create() {
        return create(false);
    }

    HttpSecurity create(boolean disableDefaults);
}

Then instead of doing:

@Configuration
public class SecurityConfiguration {

    @Bean
    public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
        http
            .authorizeHttpRequests((authz) -> authz
                .anyRequest().authenticated()
            )
            .httpBasic(withDefaults());
        return http.build();
    }

}

we can do:

@Configuration
public class SecurityConfiguration {

    @Bean
    public SecurityFilterChain filterChain(HttpSecurityProvider httpProvider) throws Exception {
        HttpSecurity http = httpProvider.create(true);
        http
            .authorizeHttpRequests((authz) -> authz
                .anyRequest().authenticated()
            )
            .httpBasic(withDefaults());
        return http.build();
    }

}

The approach is only a potential idea. It doesn't have to look like that. However, I do think that it might make sense to expose a functionality that was possible to be used like before.

@filiphr filiphr added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jul 27, 2022
@marcusdacoregio marcusdacoregio added in: config An issue in spring-security-config and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 27, 2022
@marcusdacoregio
Copy link
Contributor

Hi, @filiphr.

Can you share more about the scenario where you need the defaults to be disabled?

The HttpSecurity bean is declared in HttpSecurityConfiguration#httpSecurity. You could create your own bean and use it in your filter chain, something like this:

@Bean("emptyHttpSecurity")
@Scope("prototype")
HttpSecurity httpSecurity() throws Exception {
	WebSecurityConfigurerAdapter.LazyPasswordEncoder passwordEncoder = new WebSecurityConfigurerAdapter.LazyPasswordEncoder(
			this.context);
	AuthenticationManagerBuilder authenticationBuilder = new WebSecurityConfigurerAdapter.DefaultPasswordEncoderAuthenticationManagerBuilder(
			this.objectPostProcessor, passwordEncoder);
	authenticationBuilder.parentAuthenticationManager(authenticationManager());
	HttpSecurity http = new HttpSecurity(this.objectPostProcessor, authenticationBuilder, createSharedObjects());
        return http;
}

@Bean
public SecurityFilterChain filterChain(@Qualifier("emptyHttpSecurity") HttpSecurity http) throws Exception {
   ... 
}

@marcusdacoregio marcusdacoregio changed the title Cannot entirely replace usage of WebSecurityConfigurerAdapter with new SecurityFIlterChain bean approach HttpSecurity bean has no option to disable defaults Jul 27, 2022
@filiphr
Copy link
Contributor Author

filiphr commented Jul 27, 2022

Can you share more about the scenario where you need the defaults to be disabled?

On our side we are providing a custom extension of the WebSecurityConfigurerAdapter that is exposing the same constructor. We are not using the disableDefaults, but we are not sure whether our customers are doing that or not.

What you have shared is not possible to be done because WebSecurityConfigurerAdapter.LazyPasswordEncoder and WebSecurityConfigurerAdapter.DefaultPasswordEncoderAuthenticationManagerBuilder are package protected static classes. It cannot be accessed from our own packages.

In addition to that, the snippet for creating an empty http security is quite verbose and relies on copying things from the HttpSecurityConfiguration from Spring Security.

Finally, if we expose a custom HttpSecurity as a bean, then the default HttpSecurity provided by HttpSecurityConfiguration can only be used with a qualifier. Considering that the HttpSecurityConfiguration is package protected someone will need to write:

@Bean
public SecurityFilterChain anotherSecurityFilterChain(@Qualifier(""org.springframework.security.config.annotation.web.configuration.HttpSecurityConfiguration.httpSecurity") HttpSecurity http) {
    ...
}

@marcusdacoregio
Copy link
Contributor

I think what you suggested is valid. If we move the HttpSecurity creation to inside, let's say, a factory, we could have something like HttpSecurity http = HttpSecurityFactory.withDefaultsDisabled(true). We also would have the opportunity to remove the usage of WebSecurityConfigurerAdapter inner classes.

I'll page @rwinch in order to know his input about this, I'm not so sure tho if it is a bug.

@marcusdacoregio marcusdacoregio self-assigned this Jul 27, 2022
@filiphr
Copy link
Contributor Author

filiphr commented Jul 27, 2022

Thanks @marcusdacoregio.

I'm not so sure tho if it is a bug.

I was not sure which category to use when creating the issue and picked the bug template. Right now, It isn't necessarily a bug since we can still use the WebSecurityConfigurerAdapter. However, I am not sure what happens once 6 is out and people cannot use the same logic anymore. There is a slight difference in the HttpSecurity with and without defaults. I can imagine that it can be a tricky problem to figure out for many people.

@marcusdacoregio marcusdacoregio added type: enhancement A general enhancement and removed type: bug A general bug labels Aug 1, 2022
@marcusdacoregio marcusdacoregio added this to the 5.8.x milestone Aug 1, 2022
@mdrg-gh
Copy link

mdrg-gh commented Aug 2, 2022

I have similar needs. I'm creating an internal lib that changes the defaults of HttpSecurity, and for that I need to remove a few default customizers before they are executed. The only way I found for that was overriding AbstractHttpConfigurer#setBuilder(B) and using the received HttpSecurity object to remove the desired customizers. It works but feels like I'm getting into undocumented behavior territory. I'd like something more "stable" for it.

@filiphr suggestion looks good as long as HttpSecurity#create makes use of AbstractHttpConfigurer or similar because I want to change what the "default" means, without the lib client ever doing the tweak themselves. The bottom line is to have as little as possible from the defaults in order to automatically receive any enhancements that the default HttpSecurity receives over time (like a new safety filter to protect against a new class of threats - like CSRF, CORS).

Example of potential changes forced through setBuilder: removing LogoutConfigurer, tweaking frameOptions(), setting exceptionHandling() with internally standardized handlers, etc.

My original post on SO: https://stackoverflow.com/questions/73192974/how-to-remove-abstracthttpconfigurer-from-default-httpsecurity?noredirect=1#comment129274078_73192974

@filiphr
Copy link
Contributor Author

filiphr commented Aug 2, 2022

How are you doing this right now @mdrg-gh? The disableDefaults flag from the WebSecurityConfigurerAdapter is not going to apply the default configurations and it isn't going to apply the default AbstractHttpConfigurer(s).

I think that what you are looking for is something else.

@marcusdacoregio
Copy link
Contributor

Thank you folks for the detailed explanation of your use cases. It is indeed something that we should have. I talked to @rwinch and we are considering this for 5.8.0. I have some tasks that are a higher priority for now, so it will take a few weeks to get into this.

You can keep track of the tasks of the team on the projects page.

@mdrg-gh
Copy link

mdrg-gh commented Aug 4, 2022

@marcusdacoregio Thanks for considering this feature, it really is of great benefit the community.

Just want to make a small amend to my post. The use case I described maybe would still have a place as an enhancement, but following @filiphr suggestion in my original SO question, I replaced the setBuilder() hack with BeanPostProcessor classes targeting HttpSecurity. The result is much cleaner and robust, sure might benefit from a more specific and flexible API for HttpSecurity adjustments but I the current solution suits my needs for now.

@marcusdacoregio
Copy link
Contributor

Related to #7449

@marcusdacoregio
Copy link
Contributor

An explicit option to disable defaults is an idea that I'm not convinced of yet. #7449 can achieve this by creating an HttpSecurity not tied to Spring Security's defaults. Something like:

@Bean
@Primary
HttpSecurity customHttpSecurity(ApplicationContext context) throws Exception {
	return new HttpSecurity(context);
}

or

@Bean
SecurityFilterChain filterChain(ApplicationContext context) throws Exception {
	return new HttpSecurity(context).build();
}

The defaults can always be disabled by disabling the Configurers themselves, like csrf(CsrfConfigurer::disable).

In order to use Spring Security's HttpSecurity bean while still having another primary HttpSecurity bean, folks would have to use the @Qualifier(Qualifier(HttpSecurityConfiguration.HTTPSECURITY_BEAN_NAME)). That HTTPSECURITY_BEAN_NAME field is private but I think it is okay to make it public.

@Bean
SecurityFilterChain filterChain(@Qualifier(HttpSecurityConfiguration.HTTPSECURITY_BEAN_NAME) HttpSecurity http) throws Exception {
	return http.build();
}

@filiphr
Copy link
Contributor Author

filiphr commented Sep 20, 2022

@marcusdacoregio if we compare the suggested approach vs what it was possible to do previously you can see that it is way more complicated to disable the defaults. Yes it is possible to disable the current defaults, but explicitly doing that. However, what if there is a new feature from Spring Security and a new default is applied, now people will need to know this and disable it again explicitly.

I do believe that having the option to disable the defaults that Spring Security applies, like it was done before it is good to do in order to provide the same functionality that already exists / existed prior the deprecation of the WebSecurityConfigurerAdapter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: config An issue in spring-security-config type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants