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

Rework security auto-configuration #7958

Closed
philwebb opened this issue Jan 11, 2017 · 15 comments
Closed

Rework security auto-configuration #7958

philwebb opened this issue Jan 11, 2017 · 15 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@philwebb
Copy link
Member

As discussed with @rwinch we would like to significantly simplify security configuration in 2.0.

@kakawait
Copy link
Contributor

kakawait commented May 16, 2017

Any details about rework? Or links to discussion if publicly accessible.

@wilkinsona
Copy link
Member

@kakawait Not yet, no. The work's scheduled for 2.0.0.M5 so it's a little way off yet.

@kakawait
Copy link
Contributor

kakawait commented May 16, 2017

@wilkinsona Ok thank you for respond I will continue to track the issue.

Just to be sure I was not asking about definite implementations but Which part do you want to simplicy and globally how (if you know)? If you understand previous comment like that please do not repeat yourself 🔁 I will wait hihi

@wilkinsona
Copy link
Member

Our main concern is with the various WebSecurityConfigurerAdapter classes that are auto-configured and use "magic" numbers to order themselves. This had proven to be a source of confusion and bugs. IIRC, the outcome of the discussion with @rwinch is that we should do less automatically, and instead provide helper methods and classes to allow users to easily piece Boot's opinionated view of Spring Security together.

@rwinch
Copy link
Member

rwinch commented May 16, 2017

@wilkinsona is correct. Part of the problem now is that Boot sometimes works by using multiple WebSecurityConfigurerAdapters and cannot apply the logic to user provided WebSecurityConfigurerAdapters.

@kakawait
Copy link
Contributor

Ok thank for clarification.

I completely understand since I was facing such issues and it takes many time to understand well how order and overriding works when using multiple WebConfigurerAdapters. I think, now, I'm mastering this subject but can be a good enhancement for futur users.

@wilkinsona wilkinsona modified the milestones: 2.0.0.M4, 2.0.0.M5 Jul 28, 2017
@mbhave mbhave added the type: enhancement A general enhancement label Jul 28, 2017
@mbhave mbhave self-assigned this Aug 7, 2017
mbhave added a commit to mbhave/spring-boot that referenced this issue Aug 28, 2017
This commit combines security autoconfigurations for
management endpoints and the rest of the application. By default,
if Spring Security is on the classpath, it turns on @EnableWebSecurity.
In the presence of another WebSecurityConfigurerAdapter this backs off
completely. A default AuthenticationManager is also provided with a user
and generated password. This can be turned off by specifying a bean of
type AuthenticationManager, AuthenticationProvider or UserDetailsService.

Closes spring-projectsgh-7958
mbhave added a commit to mbhave/spring-boot that referenced this issue Aug 28, 2017
Since the handler interceptors have been removed, web endpoints
are all disabled by default to prevent accidental exposure of
sensitive information.

Closes spring-projectsgh-7958
@mbhave mbhave closed this as completed in e08ddbf Aug 28, 2017
mbhave added a commit that referenced this issue Aug 28, 2017
Since the handler interceptors have been removed, web endpoints
are all disabled by default to prevent accidental exposure of
sensitive information.

Closes gh-7958
@mbhave
Copy link
Contributor

mbhave commented Aug 28, 2017

Reopening for documentation update.

@mbhave mbhave reopened this Aug 28, 2017
mbhave added a commit that referenced this issue Aug 28, 2017
Since the autoconfig totally backs off in the presence
of a WebSecurityConfigurerAdapter, there is no need to
order them ahead of/after the one provided by Spring Boot.

See gh-7958
philwebb added a commit to philwebb/spring-boot that referenced this issue Sep 12, 2017
Update security configuration formatting to follow conventions
recommended in the Spring Security documentation.

See spring-projectsgh-7958
philwebb pushed a commit that referenced this issue Sep 28, 2017
Simplify `AuthenticationManagerConfiguration` following the recent
Spring Security auto-configuration updates.

See gh-7958
@lnhrdt
Copy link

lnhrdt commented Oct 26, 2017

I had a question about the work done in this issue. I posted on StackOverflow because I thought that was the right forum but it's not getting any response organically. The contributors in this issue are probably the right people to answer my question, can you take a look?

https://stackoverflow.com/questions/46870411/how-to-require-ssl-in-some-environments-using-spring-boot-2-0-0-m4

@dsyer
Copy link
Member

dsyer commented Nov 9, 2017

I appreciate the drive to simplify security configuration, but I think it has gone too far. This guide is a useful reference: https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-Security-2.0. (See also https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-2.0-Migration-Guide#actuator-security.) It shows what to do if you want to replace the features in Boot 1.x. In particular:

To replace security.user.password=password you have to add a bean

	@Bean
	public InMemoryUserDetailsManager inMemoryUserDetailsManager() {
		return new InMemoryUserDetailsManager(User.withDefaultPasswordEncoder()
				.username("user").password("password").roles("USER").build());
	}

That's a huge amount of boilerplate and it's not discoverable through autocomplete in the IDE. Please can we have security.user.* back?

Another example. To secure only the actuator endpoints used to be a one liner in a config file: security.basic.enabled=false. Now I need this (in addition to the UserDetails bean):

@Configuration
public class ActuatorSecurityConfigurer extends WebSecurityConfigurerAdapter {

  @Override
  public void configure(HttpSecurity http) throws Exception {
    http
    .authorizeRequests()
        .requestMatchers(EndpointRequest.to("status")).permitAll()
        .requestMatchers(EndpointRequest.to("info")).permitAll()
        .requestMatchers(EndpointRequest.toAnyEndpoint()).hasRole("ACTUATOR")
        .antMatchers("/**").permitAll()
    .and()
        .httpBasic();
  }
}

It's too much boilerplate IMO.

@wilkinsona
Copy link
Member

wilkinsona commented Nov 9, 2017

I agree that's a lot of boilerplate, but I'm not sure that bringing back security.user.* is the right way to address it. As things stand, security.user.* would be of limited use as it will have no effect once the user has taken control of the security configuration, and we expect the vast majority of users who care about security to define their own security configuration.

1.x's solution to this problem was for Boot to auto-configure multiple, ordered WebSecurityConfigurerAdapter beans. This required users to slot in their own configuration with the right order so that they overrode the right parts of the auto-configured security and left the rest intact. These required orders were not discoverable in the IDE either, unless you happened to know exactly where to go looking. Experience has taught us that this was too complex, both in terms of users understanding it and in terms of us shipping bugs.

If we need to make a trade-off between being understandable and being concise, then I would much prefer the former over the latter. I think 2.0 gets that trade-off right. Perhaps there's something that can be done to reduce the boilerplate that's needed and we should spend some time thinking about it. Right now, I'm not sure how it can be done without repeating the mistakes of 1.x.

@wilkinsona
Copy link
Member

AuthenticationManagerConfiguration could consume some properties for configuring the default user's username and password. I'm still not sure how useful it would be as it'll quickly back off when the user sets up their own security. Perhaps it's still of sufficient value for demos and the out-of-the-box experience as users are finding their feet?

I've opened #10963.

@dsyer
Copy link
Member

dsyer commented Nov 9, 2017

That would be a good start (and it's fine to back off once user adds a UserDetailsService).

On the WebSecurityConfigurer, how about an @EnableActuatorSecurity annotation to replace the boilerplate, capturing one or two common use cases? Make it optional (i.e. user has to know it exists, but easy to discover if you know that @Enable* is a naming convention for Spring).

@dsyer
Copy link
Member

dsyer commented Nov 9, 2017

Also, I've just realized that the example above only works for Servlet apps. I don't even know how to start with a WebFlux app, but it's probably something similar. An annotation could hide the details.

@rwinch
Copy link
Member

rwinch commented Nov 9, 2017

how about an @EnableActuatorSecurity annotation

If we do provide deeper integration with Actuator and Security, I would not do this via an annotation as this provides little value as soon as the user needs to provide any sort of customization. Instead, I'd lean towards providing something that hooks into the Spring Security DSL. In its simplest form it might look something like this:

http
    .apply(actuatorSecurity()).and()
    ... any customization ...;

Likely under the covers actuatorSecurity() would use the same RequestMatcher implementations that already exist. There would be properties on actuatorSecurity() to customize the defaults. Since users also have access to HttpSecurity they would have full control over how security was configured.

@philwebb
Copy link
Member Author

philwebb commented Nov 9, 2017

I've raised #10968 for the actuator security suggestion.

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

No branches or pull requests

7 participants