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.

Sign up for GitHub

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

Disable device authorization flow by default #1498

Closed
wants to merge 3 commits into from
Closed

Disable device authorization flow by default #1498

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0848c7d
gh-1454:
Jan 8, 2024
d33e4d2
gh-1454:
Jan 8, 2024
e324369
gh-1454:
Jan 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions docs/src/test/java/sample/jpa/JpaTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
* Tests for the guide How-to: Implement core services with JPA.
*
* @author Steve Riesenberg
* @author Greg Li
*/
@ExtendWith(SpringTestContextExtension.class)
public class JpaTests {
Expand All @@ -88,7 +89,7 @@ public class JpaTests {

@Test
public void oidcLoginWhenJpaCoreServicesAutowiredThenUsed() throws Exception {
this.spring.register(AuthorizationServerConfig.class).autowire();
this.spring.register(AuthorizationServerConfigDeviceAuthorize.class).autowire();
assertThat(this.registeredClientRepository).isInstanceOf(JpaRegisteredClientRepository.class);
assertThat(this.authorizationService).isInstanceOf(JpaOAuth2AuthorizationService.class);
assertThat(this.authorizationConsentService).isInstanceOf(JpaOAuth2AuthorizationConsentService.class);
Expand Down Expand Up @@ -135,7 +136,7 @@ public void oidcLoginWhenJpaCoreServicesAutowiredThenUsed() throws Exception {

@Test
public void deviceAuthorizationWhenJpaCoreServicesAutowiredThenSuccess() throws Exception {
this.spring.register(AuthorizationServerConfig.class).autowire();
this.spring.register(AuthorizationServerConfigDeviceAuthorize.class).autowire();
assertThat(this.registeredClientRepository).isInstanceOf(JpaRegisteredClientRepository.class);
assertThat(this.authorizationService).isInstanceOf(JpaOAuth2AuthorizationService.class);
assertThat(this.authorizationConsentService).isInstanceOf(JpaOAuth2AuthorizationConsentService.class);
Expand Down Expand Up @@ -191,13 +192,15 @@ private OAuth2Authorization findAuthorization(String token, String tokenType) {
@EnableWebSecurity
@EnableAutoConfiguration
@ComponentScan
static class AuthorizationServerConfig {
static class AuthorizationServerConfigDeviceAuthorize {

@Bean
@Order(Ordered.HIGHEST_PRECEDENCE)
public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity http) throws Exception {
OAuth2AuthorizationServerConfiguration.applyDefaultSecurity(http);
http.getConfigurer(OAuth2AuthorizationServerConfigurer.class)
.deviceAuthorizationEndpoint(Customizer.withDefaults())
.deviceVerificationEndpoint(Customizer.withDefaults())
.oidc(Customizer.withDefaults()); // Enable OpenID Connect 1.0

// @formatter:off
Expand Down
64 changes: 53 additions & 11 deletions .../authorization/config/annotation/web/configurers/OAuth2AuthorizationServerConfigurer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationCodeRequestAuthenticationException;
import org.springframework.security.oauth2.server.authorization.authentication.OAuth2AuthorizationCodeRequestAuthenticationToken;
import org.springframework.security.oauth2.server.authorization.client.RegisteredClientRepository;
import org.springframework.security.oauth2.server.authorization.context.AuthorizationServerContext;
import org.springframework.security.oauth2.server.authorization.context.AuthorizationServerContextHolder;
import org.springframework.security.oauth2.server.authorization.settings.AuthorizationServerSettings;
import org.springframework.security.oauth2.server.authorization.token.OAuth2TokenGenerator;
import org.springframework.security.oauth2.server.authorization.web.NimbusJwkSetEndpointFilter;
Expand All @@ -55,6 +57,7 @@
import org.springframework.security.web.util.matcher.OrRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher;
import org.springframework.util.Assert;
import org.springframework.web.util.UriComponentsBuilder;

/**
* An {@link AbstractHttpConfigurer} for OAuth 2.0 Authorization Server support.
Expand All @@ -64,6 +67,7 @@
* @author Gerardo Roza
* @author Ovidiu Popa
* @author Gaurav Tiwari
* @author Greg Li
* @since 0.0.1
* @see AbstractHttpConfigurer
* @see OAuth2ClientAuthenticationConfigurer
Expand Down Expand Up @@ -218,26 +222,40 @@ public OAuth2AuthorizationServerConfigurer tokenRevocationEndpoint(Customizer<OA
}

/**
* Configures the OAuth 2.0 Device Authorization Endpoint.
* Configures the OAuth 2.0 Device Authorization Endpoint (disabled by default).
*
* @param deviceAuthorizationEndpointCustomizer the {@link Customizer} providing access to the {@link OAuth2DeviceAuthorizationEndpointConfigurer}
* @return the {@link OAuth2AuthorizationServerConfigurer} for further configuration
* @since 1.1
*/
public OAuth2AuthorizationServerConfigurer deviceAuthorizationEndpoint(Customizer<OAuth2DeviceAuthorizationEndpointConfigurer> deviceAuthorizationEndpointCustomizer) {
deviceAuthorizationEndpointCustomizer.customize(getConfigurer(OAuth2DeviceAuthorizationEndpointConfigurer.class));
OAuth2DeviceAuthorizationEndpointConfigurer deviceAuthorizationEndpointConfigurer =
getConfigurer(OAuth2DeviceAuthorizationEndpointConfigurer.class);
if (deviceAuthorizationEndpointConfigurer == null) {
addConfigurer(OAuth2DeviceAuthorizationEndpointConfigurer.class,
new OAuth2DeviceAuthorizationEndpointConfigurer(this::postProcess));
deviceAuthorizationEndpointConfigurer = getConfigurer(OAuth2DeviceAuthorizationEndpointConfigurer.class);
}
deviceAuthorizationEndpointCustomizer.customize(deviceAuthorizationEndpointConfigurer);
return this;
}

/**
* Configures the OAuth 2.0 Device Verification Endpoint.
* Configures the OAuth 2.0 Device Verification Endpoint (disabled by default).
*
* @param deviceVerificationEndpointCustomizer the {@link Customizer} providing access to the {@link OAuth2DeviceVerificationEndpointConfigurer}
* @return the {@link OAuth2AuthorizationServerConfigurer} for further configuration
* @since 1.1
*/
public OAuth2AuthorizationServerConfigurer deviceVerificationEndpoint(Customizer<OAuth2DeviceVerificationEndpointConfigurer> deviceVerificationEndpointCustomizer) {
deviceVerificationEndpointCustomizer.customize(getConfigurer(OAuth2DeviceVerificationEndpointConfigurer.class));
OAuth2DeviceVerificationEndpointConfigurer deviceVerificationEndpointConfigurer =
getConfigurer(OAuth2DeviceVerificationEndpointConfigurer.class);
if (deviceVerificationEndpointConfigurer == null) {
addConfigurer(OAuth2DeviceVerificationEndpointConfigurer.class,
new OAuth2DeviceVerificationEndpointConfigurer(this::postProcess));
deviceVerificationEndpointConfigurer = getConfigurer(OAuth2DeviceVerificationEndpointConfigurer.class);
}
deviceVerificationEndpointCustomizer.customize(deviceVerificationEndpointConfigurer);
return this;
}

Expand Down Expand Up @@ -319,19 +337,45 @@ public void init(HttpSecurity httpSecurity) {

ExceptionHandlingConfigurer<HttpSecurity> exceptionHandling = httpSecurity.getConfigurer(ExceptionHandlingConfigurer.class);
if (exceptionHandling != null) {
OrRequestMatcher preferredRequestMatcher = null;
if (getRequestMatcher(OAuth2DeviceAuthorizationEndpointConfigurer.class) != null) {
preferredRequestMatcher = new OrRequestMatcher(
getRequestMatcher(OAuth2TokenEndpointConfigurer.class),
getRequestMatcher(OAuth2TokenIntrospectionEndpointConfigurer.class),
getRequestMatcher(OAuth2TokenRevocationEndpointConfigurer.class),
getRequestMatcher(OAuth2DeviceAuthorizationEndpointConfigurer.class));
} else {
preferredRequestMatcher = new OrRequestMatcher(
getRequestMatcher(OAuth2TokenEndpointConfigurer.class),
getRequestMatcher(OAuth2TokenIntrospectionEndpointConfigurer.class),
getRequestMatcher(OAuth2TokenRevocationEndpointConfigurer.class));
}
exceptionHandling.defaultAuthenticationEntryPointFor(
new HttpStatusEntryPoint(HttpStatus.UNAUTHORIZED),
new OrRequestMatcher(
getRequestMatcher(OAuth2TokenEndpointConfigurer.class),
getRequestMatcher(OAuth2TokenIntrospectionEndpointConfigurer.class),
getRequestMatcher(OAuth2TokenRevocationEndpointConfigurer.class),
getRequestMatcher(OAuth2DeviceAuthorizationEndpointConfigurer.class))
preferredRequestMatcher
);
}
}

@Override
public void configure(HttpSecurity httpSecurity) {
OAuth2DeviceAuthorizationEndpointConfigurer deviceAuthorizationEndpointConfigurer =
getConfigurer(OAuth2DeviceAuthorizationEndpointConfigurer.class);
if (deviceAuthorizationEndpointConfigurer != null) {
OAuth2AuthorizationServerMetadataEndpointConfigurer auth2AuthorizationServerMetadataEndpointConfigurer =
getConfigurer(OAuth2AuthorizationServerMetadataEndpointConfigurer.class);

auth2AuthorizationServerMetadataEndpointConfigurer
.addDefaultAuthorizationServerMetadataCustomizer((builder) -> {
AuthorizationServerContext authorizationServerContext = AuthorizationServerContextHolder.getContext();
String issuer = authorizationServerContext.getIssuer();
AuthorizationServerSettings authorizationServerSettings = authorizationServerContext.getAuthorizationServerSettings();
String deviceAuthorizationEndpoint = UriComponentsBuilder.fromUriString(issuer)
.path(authorizationServerSettings.getDeviceAuthorizationEndpoint()).build().toUriString();

builder.deviceAuthorizationEndpoint(deviceAuthorizationEndpoint);
});
}
this.configurers.values().forEach(configurer -> configurer.configure(httpSecurity));

AuthorizationServerSettings authorizationServerSettings = OAuth2ConfigurerUtils.getAuthorizationServerSettings(httpSecurity);
Expand Down Expand Up @@ -359,8 +403,6 @@ private Map<Class<? extends AbstractOAuth2Configurer>, AbstractOAuth2Configurer>
configurers.put(OAuth2TokenEndpointConfigurer.class, new OAuth2TokenEndpointConfigurer(this::postProcess));
configurers.put(OAuth2TokenIntrospectionEndpointConfigurer.class, new OAuth2TokenIntrospectionEndpointConfigurer(this::postProcess));
configurers.put(OAuth2TokenRevocationEndpointConfigurer.class, new OAuth2TokenRevocationEndpointConfigurer(this::postProcess));
configurers.put(OAuth2DeviceAuthorizationEndpointConfigurer.class, new OAuth2DeviceAuthorizationEndpointConfigurer(this::postProcess));
Copy link
Collaborator

@jgrandja jgrandja Jan 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change breaks (backward) compatibility since the feature is enabled by default. Instead of disabling by default, there needs to be a configurable way of disabling the feature.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to provider a configuration(e.g. disableDeviceAuthorizationFlow) and default value is true? Any setting we can refer to?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jgrandja ，
Adding a configuration deviceAuthorizationEndpoint.enableDeviceAuthorizationEndpoint(true); for enable/disable device authorization flow. The default value of deviceAuthorizationEndpoint.enableDeviceAuthorizationEndpoint is true which enable by default for backward compatibility.

configurers.put(OAuth2DeviceVerificationEndpointConfigurer.class, new OAuth2DeviceVerificationEndpointConfigurer(this::postProcess));
return configurers;
}

Expand Down
2 changes: 1 addition & 1 deletion ...rity/oauth2/server/authorization/web/OAuth2AuthorizationServerMetadataEndpointFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
*
* @author Daniel Garnier-Moiroux
* @author Joe Grandja
* @author Greg Li
* @since 0.1.1
* @see OAuth2AuthorizationServerMetadata
* @see AuthorizationServerSettings
Expand Down Expand Up @@ -92,7 +93,6 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
OAuth2AuthorizationServerMetadata.Builder authorizationServerMetadata = OAuth2AuthorizationServerMetadata.builder()
.issuer(issuer)
.authorizationEndpoint(asUrl(issuer, authorizationServerSettings.getAuthorizationEndpoint()))
.deviceAuthorizationEndpoint(asUrl(issuer, authorizationServerSettings.getDeviceAuthorizationEndpoint()))
.tokenEndpoint(asUrl(issuer, authorizationServerSettings.getTokenEndpoint()))
.tokenEndpointAuthenticationMethods(clientAuthenticationMethods())
.jwkSetUrl(asUrl(issuer, authorizationServerSettings.getJwkSetEndpoint()))
Expand Down
59 changes: 49 additions & 10 deletions ...h2/server/authorization/config/annotation/web/configurers/OAuth2DeviceCodeGrantTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Import;
import org.springframework.core.Ordered;
import org.springframework.core.annotation.Order;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
Expand All @@ -45,6 +47,8 @@
import org.springframework.mock.http.client.MockClientHttpResponse;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.config.Customizer;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.crypto.password.NoOpPasswordEncoder;
import org.springframework.security.crypto.password.PasswordEncoder;
Expand Down Expand Up @@ -72,6 +76,7 @@
import org.springframework.security.oauth2.server.authorization.config.annotation.web.configuration.OAuth2AuthorizationServerConfiguration;
import org.springframework.security.oauth2.server.authorization.test.SpringTestContext;
import org.springframework.security.oauth2.server.authorization.test.SpringTestContextExtension;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.MvcResult;
import org.springframework.util.LinkedMultiValueMap;
Expand All @@ -90,6 +95,7 @@
* Integration tests for OAuth 2.0 Device Grant.
*
* @author Steve Riesenberg
* @author Greg Li
*/
@ExtendWith(SpringTestContextExtension.class)
public class OAuth2DeviceCodeGrantTests {
Expand Down Expand Up @@ -158,7 +164,7 @@ public static void destroy() {

@Test
public void requestWhenDeviceAuthorizationRequestNotAuthenticatedThenUnauthorized() throws Exception {
this.spring.register(AuthorizationServerConfiguration.class).autowire();
this.spring.register(AuthorizationServerConfigurationDeviceAuthorize.class).autowire();

// @formatter:off
RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
Expand All @@ -179,9 +185,32 @@ public void requestWhenDeviceAuthorizationRequestNotAuthenticatedThenUnauthorize
// @formatter:on
}

@Test
public void requestWhenDeviceAuthorizationRequestDisabledThenUnauthorized() throws Exception {
this.spring.register(AuthorizationServerConfigurationDeviceAuthorize.class).autowire();

// @formatter:off
RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
.authorizationGrantType(AuthorizationGrantType.DEVICE_CODE)
.build();
// @formatter:on
this.registeredClientRepository.save(registeredClient);

MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
parameters.set(OAuth2ParameterNames.CLIENT_ID, registeredClient.getClientId());
parameters.set(OAuth2ParameterNames.SCOPE,
StringUtils.collectionToDelimitedString(registeredClient.getScopes(), " "));

// @formatter:off
this.mvc.perform(post(DEFAULT_DEVICE_AUTHORIZATION_ENDPOINT_URI)
.params(parameters))
.andExpect(status().isUnauthorized());
// @formatter:on
}

@Test
public void requestWhenRegisteredClientMissingThenUnauthorized() throws Exception {
this.spring.register(AuthorizationServerConfiguration.class).autowire();
this.spring.register(AuthorizationServerConfigurationDeviceAuthorize.class).autowire();

// @formatter:off
RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
Expand All @@ -204,7 +233,7 @@ public void requestWhenRegisteredClientMissingThenUnauthorized() throws Exceptio

@Test
public void requestWhenDeviceAuthorizationRequestValidThenReturnDeviceAuthorizationResponse() throws Exception {
this.spring.register(AuthorizationServerConfiguration.class).autowire();
this.spring.register(AuthorizationServerConfigurationDeviceAuthorize.class).autowire();

// @formatter:off
RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
Expand Down Expand Up @@ -252,7 +281,7 @@ public void requestWhenDeviceAuthorizationRequestValidThenReturnDeviceAuthorizat

@Test
public void requestWhenDeviceVerificationRequestUnauthenticatedThenUnauthorized() throws Exception {
this.spring.register(AuthorizationServerConfiguration.class).autowire();
this.spring.register(AuthorizationServerConfigurationDeviceAuthorize.class).autowire();

// @formatter:off
RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
Expand Down Expand Up @@ -286,7 +315,7 @@ public void requestWhenDeviceVerificationRequestUnauthenticatedThenUnauthorized(

@Test
public void requestWhenDeviceVerificationRequestValidThenDisplaysConsentPage() throws Exception {
this.spring.register(AuthorizationServerConfiguration.class).autowire();
this.spring.register(AuthorizationServerConfigurationDeviceAuthorize.class).autowire();

// @formatter:off
RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
Expand Down Expand Up @@ -335,7 +364,7 @@ public void requestWhenDeviceVerificationRequestValidThenDisplaysConsentPage() t

@Test
public void requestWhenDeviceAuthorizationConsentRequestUnauthenticatedThenBadRequest() throws Exception {
this.spring.register(AuthorizationServerConfiguration.class).autowire();
this.spring.register(AuthorizationServerConfigurationDeviceAuthorize.class).autowire();

// @formatter:off
RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
Expand Down Expand Up @@ -373,7 +402,7 @@ public void requestWhenDeviceAuthorizationConsentRequestUnauthenticatedThenBadRe

@Test
public void requestWhenDeviceAuthorizationConsentRequestValidThenRedirectsToSuccessPage() throws Exception {
this.spring.register(AuthorizationServerConfiguration.class).autowire();
this.spring.register(AuthorizationServerConfigurationDeviceAuthorize.class).autowire();

// @formatter:off
RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
Expand Down Expand Up @@ -423,7 +452,7 @@ public void requestWhenDeviceAuthorizationConsentRequestValidThenRedirectsToSucc

@Test
public void requestWhenAccessTokenRequestUnauthenticatedThenUnauthorized() throws Exception {
this.spring.register(AuthorizationServerConfiguration.class).autowire();
this.spring.register(AuthorizationServerConfigurationDeviceAuthorize.class).autowire();

// @formatter:off
RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
Expand Down Expand Up @@ -459,7 +488,7 @@ public void requestWhenAccessTokenRequestUnauthenticatedThenUnauthorized() throw

@Test
public void requestWhenAccessTokenRequestValidThenReturnAccessTokenResponse() throws Exception {
this.spring.register(AuthorizationServerConfiguration.class).autowire();
this.spring.register(AuthorizationServerConfigurationDeviceAuthorize.class).autowire();

// @formatter:off
RegisteredClient registeredClient = TestRegisteredClients.registeredClient()
Expand Down Expand Up @@ -545,7 +574,17 @@ private static Function<OAuth2Authorization.Token<? extends OAuth2Token>, Boolea

@EnableWebSecurity
@Import(OAuth2AuthorizationServerConfiguration.class)
static class AuthorizationServerConfiguration {
static class AuthorizationServerConfigurationDeviceAuthorize {

@Bean
@Order(Ordered.HIGHEST_PRECEDENCE)
public SecurityFilterChain authorizationServerSecurityFilterChain(HttpSecurity http) throws Exception {
OAuth2AuthorizationServerConfiguration.applyDefaultSecurity(http);
http.getConfigurer(OAuth2AuthorizationServerConfigurer.class)
.deviceAuthorizationEndpoint(Customizer.withDefaults()) // Enable deviceAuthorizationEndpoint
.deviceVerificationEndpoint(Customizer.withDefaults()); // Enable deviceVerificationEndpoint
return http.build();
}

@Bean
RegisteredClientRepository registeredClientRepository(JdbcOperations jdbcOperations) {
Expand Down