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

Support Configuring Authorization Server in HttpSecurity.with(authorizationServer(), ...) #1707

Open
rwinch opened this issue Aug 29, 2024 · 12 comments
Assignees
Labels
type: enhancement A general enhancement

Comments

@rwinch
Copy link
Member

rwinch commented Aug 29, 2024

Expected Behavior

It would be nice if all of the Authorization Server configuration options could be made/discoverable from a single entry point & on an existing SecurityFilterChain.

For example, it would be very nice if adding a default Authorization Server to an existing application could be done with a single invocation of the HttpSecurity DSL using something like this:

http
    .with(authorizationServer(), Customizer.withDefaults())
    // ...

If a user wanted to make changes to the defaults, then they would do so using the Customizer. For example, if OIDC was not enabled by default, then the user would be able to use:

http
    .with(authorizationServer(), authz -> authz
        .oidc(Customizer.withDefaults())
   )
   // ...

NOTE: I'm not saying that OIDC should or should not be a default, but I'm just using this as an example of how a user would be able to change the defaults.

Using this approach:

  • Should feel familiar to Spring Security users
  • It will make all of the features available via autocomplete after a single static import
  • Should allow users to enable a feature (authorization server) rather than needing to know about the implementation details such as exception handling.

Current Behavior

Currently the recommendation is:

@Bean 
@Order(1)
public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity http)
		throws Exception {
	OAuth2AuthorizationServerConfiguration.applyDefaultSecurity(http);
	http.getConfigurer(OAuth2AuthorizationServerConfigurer.class)
		.oidc(Customizer.withDefaults());	// Enable OpenID Connect 1.0
	http
		// Redirect to the login page when not authenticated from the
		// authorization endpoint
		.exceptionHandling((exceptions) -> exceptions
			.defaultAuthenticationEntryPointFor(
				new LoginUrlAuthenticationEntryPoint("/login"),
				new MediaTypeRequestMatcher(MediaType.TEXT_HTML)
			)
		)
		// Accept access tokens for User Info and/or Client Registration
		.oauth2ResourceServer((resourceServer) -> resourceServer
			.jwt(Customizer.withDefaults()));
		return http.build();
}

The disadvantages of this approach are:

  • It is less concise
  • Having multiple SecurityFilterChain is less familiar and adds additional complexity to understanding/debugging
  • Using OAuth2AuthorizationServerConfiguration.applyDefaultSecurity is unfamiliar to users
  • Using http.getConfigurer(OAuth2AuthorizationServerConfigurer.class) is also unfamiliar to users
  • It is more difficult to discover. In the proposal, a user only needs to remember a single static import and every other option can be auto completed. With the current configuration a user must:
    • Remember to use multiple SecurityFilterChain
    • Remember to use @Order (and know where to import it from)
    • Remember OAuth2AuthorizationServerConfiguration.applyDefaultSecurity so it can be imported
    • Remember OAuth2AuthorizationServerConfigurer.class and how to get it from HttpSecurity and know that this is how to configure oidc (vs another static method import like applyDefaultSecurity or something returned from applyDefaultSecurity, etc).
  • The user must remember to configure exceptionHandling and how to configure it.
  • The user must enable the resource server support

Context

I'd like to only need to remember a single thing (ideally a concept) in order to add Spring Authorization Server to an existing SecurityFilterChain.

This came up a few times (in various contexts) around trying to simplify (remove) the additional SecurityFilterChain and align more with webauthn in @joshlong and my Goodbye Passwords Hello Security talk.

@rwinch rwinch added the type: enhancement A general enhancement label Aug 29, 2024
@jgrandja jgrandja self-assigned this Aug 29, 2024
@jgrandja
Copy link
Collaborator

jgrandja commented Sep 11, 2024

@rwinch Thanks for the extra details.

Regarding:

Using OAuth2AuthorizationServerConfiguration.applyDefaultSecurity is unfamiliar to users

Using http.getConfigurer(OAuth2AuthorizationServerConfigurer.class) is also unfamiliar to users

These have been available since Day 1 and is well documented in the reference doc and is also used throughout all the samples and tests. So it's quite familiar and natural to users.

With the current configuration a user must: Remember to use multiple SecurityFilterChain

A dedicated SecurityFilterChain for the Authorization Server endpoints is recommended, however, it is not required. An application can choose to configure the Authorization Server and their custom application features into a single SecurityFilterChain.

For example, looking at the sample configuration in the Getting Started section of the reference, one could combine the authorizationServerSecurityFilterChain @Bean and the defaultSecurityFilterChain @Bean into a single SecurityFilterChain, as follows:

@Bean
public SecurityFilterChain securityFilterChain(HttpSecurity http)
		throws Exception {

	OAuth2AuthorizationServerConfigurer authorizationServerConfigurer =
			new OAuth2AuthorizationServerConfigurer();

	http.with(authorizationServerConfigurer, authorizationServer ->
			authorizationServer
					.oidc(Customizer.withDefaults()))	// Enable OpenID Connect 1.0
			.authorizeHttpRequests((authorize) -> authorize
					.anyRequest().authenticated()
			)
			.csrf((csrf) -> csrf
					.ignoringRequestMatchers(authorizationServerConfigurer.getEndpointsMatcher())
			)
			// Accept access tokens for User Info and/or Client Registration
			.oauth2ResourceServer((resourceServer) -> resourceServer
					.jwt(Customizer.withDefaults())
			)
			.formLogin(Customizer.withDefaults());

	return http.build();
}

Given the above configuration and your comment for expected behaviour:

If a user wanted to make changes to the defaults, then they would do so using the Customizer. For example, if OIDC was not enabled by default, then the user would be able to use:

This is already possible as demonstrated in the sample configuration.

At this point, I don't see anything that needs to be improved upon. Thoughts?

@jgrandja jgrandja added the status: waiting-for-feedback We need additional information before we can continue label Sep 11, 2024
@rwinch rwinch self-assigned this Sep 12, 2024
@rwinch rwinch changed the title Support Configuring Authorization Server in HttpSecurity.with(authorizationServer(), ...) Reply to https://github.com/spring-projects/spring-authorization-server/issues/1707 Sep 12, 2024
@rwinch rwinch changed the title Reply to https://github.com/spring-projects/spring-authorization-server/issues/1707 Support Configuring Authorization Server in HttpSecurity.with(authorizationServer(), ...) Sep 12, 2024
@rwinch
Copy link
Member Author

rwinch commented Sep 12, 2024

Thanks @jgrandja! This appears to be a great start.

These have been available since Day 1 and is well documented in the reference doc and is also used throughout all the samples and tests. So it's quite familiar and natural to users.

The following lines are not used anywhere else within Spring Security:

OAuth2AuthorizationServerConfiguration.applyDefaultSecurity(http);
http.getConfigurer(OAuth2AuthorizationServerConfigurer.class)
			.oidc(Customizer.withDefaults());	// Enable OpenID Connect 1.0

The feedback we have received is that it is difficult to remember to do this because it is not following the standard conventions of Spring Security for configuring extensions.

I agree. Looking at this configuration, it does not feel like Spring Security's configuration model. The standard way to configure a Spring Security extension can be found in the reference and aligns with what is suggested in this issue.

What's more is using the way that is suggested in this ticket only requires remembering a single static method. The remaining configuration can be autocompleted from the IDE.

The documented way of configuring Authorization Server requires remembering:

  • a static method
  • to use getConfigurer
  • To pass in OAuth2AuthorizationServerConfigurer to the getConfigurer method
  • to configure exceptionHandling (and how)
  • to configure resourceServer
  • to configure csrf.

A dedicated SecurityFilterChain for the Authorization Server endpoints is recommended, however, it is not required.

Why is it recommended to use a dedicated SecurityFilterChain? Is this documented somewhere?

For example, looking at the sample configuration in the Getting Started section of the reference, one could combine the authorizationServerSecurityFilterChain @bean and the defaultSecurityFilterChain @bean into a single SecurityFilterChain, as follows:

How does someone know how to combine them? For example, the documentation uses

OAuth2AuthorizationServerConfiguration.applyDefaultSecurity(http);

However, when using a single @Bean this is no longer used.

Another example is that the documentation uses http.getConfigurer(OAuth2AuthorizationServerConfigurer.class) but this does not work without applyDefaultSecurity. How does a user know to use new OAuth2AuthorizationServerConfigurer()?

Why is it necessary to configure csrf and oauth2Resource server? This seems like unnecessary explicit configuration.

@jgrandja jgrandja removed the status: waiting-for-feedback We need additional information before we can continue label Sep 17, 2024
jgrandja added a commit to jgrandja/spring-authorization-server that referenced this issue Sep 19, 2024
@jgrandja
Copy link
Collaborator

jgrandja commented Sep 19, 2024

@rwinch

Why is it recommended to use a dedicated SecurityFilterChain? Is this documented somewhere?

IMO it is a recommended best practice to isolate all the authorization server capabilities in it's own SecurityFilterChain and not mix it up with the default (Spring Security) SecurityFilterChain.

In a production environment, the Identity Provider is a separately managed system and the same goes for an OAuth2/OIDC Authorization Server as it is also a completely separate system. Obviously, the 2 systems need to be integrated to enable the OAuth2 flows, but they should most definitely be completely isolated, especially the Identity Provider as it should never "sit with" another system, for security and privacy reasons. From a general architecture standpoint, a SecurityFilterChain allows you to establish security boundaries between different parts of the overall system, hence the use of a dedicated SecurityFilterChain for authorization server and it's specific configuration.

This is the main reason why we have documented this configuration pattern across the reference docs, samples and tests. However, it does appear that we have not formally documented (in words) this as a best practice in our docs.

Another reason worth mentioning, although not as important, is by combining the default (Spring Security) SecurityFilterChain and the authorization server SecurityFilterChain into one SecurityFilterChain would make the chain quite big at around 26x Filter's, because the OAuth2 Endpoints add an additional 13x Filter's, and therefore may introduce a lot of noise when troubleshooting/debugging issues.

@jgrandja
Copy link
Collaborator

@rwinch I've opened a Draft PR (gh-1725) to iterate on this.

Take a look at the 1st commit showing a simplified configuration with the existing 2x SecurityFilterChain setup.

Take a look at the 2nd commit showing a simplified configuration with a single SecurityFilterChain setup.

@rwinch
Copy link
Member Author

rwinch commented Sep 19, 2024

Thanks @jgrandja! Let me know when you are ready for feedback.

@jgrandja
Copy link
Collaborator

@rwinch It's ready for feedback.

@akuma8
Copy link

akuma8 commented Sep 21, 2024

@jgrandja if this enhancement is added, please do not depreciate the current available configuration. I agree with you to not mix authorization server filter chain with business filter chain. I think that this enhancement will add more confusion than clarity.

@jgrandja
Copy link
Collaborator

Thanks for the feedback @akuma8.

Can you provide more details in regards to:

I think that this enhancement will add more confusion than clarity.

@johnsonr
Copy link

I agree with @rwinch that "it would be very nice if adding a default Authorization Server to an existing application could be done with a single invocation of the HttpSecurity DSL". I find the present approach somewhat inconsistent, and more complex than I'd expect from Spring.

@jgrandja
Copy link
Collaborator

Thanks for the feedback @johnsonr.

I find the present approach somewhat inconsistent.

I'm assuming you are referring to the following configuration as documented in the Getting Started page:

OAuth2AuthorizationServerConfiguration.applyDefaultSecurity(http);
http.getConfigurer(OAuth2AuthorizationServerConfigurer.class)
	.oidc(Customizer.withDefaults());	// Enable OpenID Connect 1.0

To give you more context on why the configuration is like this is because before HttpSecurity.with() was introduced in 6.2 (2023), the only way to add a custom SecurityConfigurer (OAuth2AuthorizationServerConfigurer) to HttpSecurity was via HttpSecurity.apply(). If you look at HttpSecurity.apply(), you will notice it returns the SecurityConfigurer NOT HttpSecurity, which does not allow for a fluent way of configuring.

Looking at OAuth2AuthorizationServerConfiguration.applyDefaultSecurity(http) (introduced in 2020):

you will notice it applies some defaults and then calls apply(authorizationServerConfigurer).

If the application needs to further customize the authorization server configuration, then you need to obtain the reference of the authorizationServerConfigurer via HttpSecurity.getConfigurer(OAuth2AuthorizationServerConfigurer.class).

To sum up, before HttpSecurity.with() was introduced in 6.2, this was the optimal way to configure a custom SecurityConfigurer (OAuth2AuthorizationServerConfigurer). One important thing to point out is that the use of OAuth2AuthorizationServerConfiguration.applyDefaultSecurity(http) is not required, and is only provided as a convenience method as documented in the reference:

OAuth2AuthorizationServerConfiguration.applyDefaultSecurity(HttpSecurity) is a convenience (static) utility method that applies the default OAuth2 security configuration to HttpSecurity.

An application can freely choose to configure the defaults themselves if they prefer.

One other important thing to mention is that Spring Authorization Server is built on top of Spring Security, so it inherits Spring Security's configuration model and does not have it's own proprietary ways of configuring.

it would be very nice if adding a default Authorization Server to an existing application could be done with a single
invocation of the HttpSecurity DSL

A single invocation of the HttpSecurity DSL is already possible today. Taking a look at the authorizationServerSecurityFilterChain @Bean in the Getting Started page, you can update the config as follows:

@Bean
@Order(1)
public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity http)
		throws Exception {

	OAuth2AuthorizationServerConfigurer authorizationServerConfigurer =
			new OAuth2AuthorizationServerConfigurer();

	// @formatter:off
	http
		.securityMatcher(authorizationServerConfigurer.getEndpointsMatcher())
		.with(authorizationServerConfigurer, (authorizationServer) ->
			authorizationServer
				.oidc(Customizer.withDefaults())	// Enable OpenID Connect 1.0
		)
		.authorizeHttpRequests((authorize) ->
			authorize.anyRequest().authenticated()
		)
		.csrf((csrf) ->
			csrf.ignoringRequestMatchers(authorizationServerConfigurer.getEndpointsMatcher()))
		// Redirect to the login page when not authenticated from the
		// authorization endpoint
		.exceptionHandling((exceptions) -> exceptions
			.defaultAuthenticationEntryPointFor(
				new LoginUrlAuthenticationEntryPoint("/login"),
				new MediaTypeRequestMatcher(MediaType.TEXT_HTML)
			)
		)
		// Accept access tokens for User Info and/or Client Registration
		.oauth2ResourceServer((resourceServer) -> resourceServer
			.jwt(Customizer.withDefaults()));
	// @formatter:on

	return http.build();
}

To sum up once again, OAuth2AuthorizationServerConfiguration.applyDefaultSecurity(HttpSecurity) was introduced in 2020 and HttpSecurity.with() was introduced recently in 2023. We obviously did not get around to updating the reference doc and samples to make use of HttpSecurity.with(), which ultimately enables a fluent way of configuring.

I am curious, if the reference doc was updated and made use of HttpSecurity.with(), demonstrating a single invocation of HttpSecurity for configuration, would this be a non-issue?

I hope any of this helps?

I'm looking forward to your response and helping move this forward.

@joshlong
Copy link
Member

I love this PR and would love to see it merged so that SAS feels more in line with the rest of Spring Security. We could deprecate the old way but not remove it until the next major release, while promoting the vastly cleaner and more consistent new way.

@jgrandja
Copy link
Collaborator

@joshlong

We could deprecate the old way but not remove it until the next major release, while promoting the vastly cleaner and more consistent new way.

We will be merging the 1st commit, which shows a simplified configuration with the existing 2x SecurityFilterChain setup (see Multiple HttpSecurity Instances).

However, we will not be merging the 2nd commit, which shows a single SecurityFilterChain setup, as this is not the recommended best practice as explained in this comment. The 2nd commit was provided only to show that it is possible to setup a single SecurityFilterChain using standard Spring Security configuration.

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
Status: In Progress
Development

No branches or pull requests

5 participants