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 all 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
47 changes: 42 additions & 5 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 @@ -307,6 +311,10 @@ public void init(HttpSecurity httpSecurity) {
}
});
}
if (!isDeviceAuthorizationEnabled()) {
this.configurers.remove(OAuth2DeviceAuthorizationEndpointConfigurer.class);
this.configurers.remove(OAuth2DeviceVerificationEndpointConfigurer.class);
}

List<RequestMatcher> requestMatchers = new ArrayList<>();
this.configurers.values().forEach(configurer -> {
Expand All @@ -319,19 +327,43 @@ public void init(HttpSecurity httpSecurity) {

ExceptionHandlingConfigurer<HttpSecurity> exceptionHandling = httpSecurity.getConfigurer(ExceptionHandlingConfigurer.class);
if (exceptionHandling != null) {
OrRequestMatcher preferredRequestMatcher = null;
if (isDeviceAuthorizationEnabled()) {
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) {
if (isDeviceAuthorizationEnabled()) {
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 All @@ -351,6 +383,11 @@ private boolean isOidcEnabled() {
return getConfigurer(OidcConfigurer.class) != null;
}

private boolean isDeviceAuthorizationEnabled() {
OAuth2DeviceAuthorizationEndpointConfigurer deviceAuthorizationEndpointConfigurer = getConfigurer(OAuth2DeviceAuthorizationEndpointConfigurer.class);
return deviceAuthorizationEndpointConfigurer != null && deviceAuthorizationEndpointConfigurer.isEnableDeviceAuthorizationEndpoint();
}

private Map<Class<? extends AbstractOAuth2Configurer>, AbstractOAuth2Configurer> createConfigurers() {
Map<Class<? extends AbstractOAuth2Configurer>, AbstractOAuth2Configurer> configurers = new LinkedHashMap<>();
configurers.put(OAuth2ClientAuthenticationConfigurer.class, new OAuth2ClientAuthenticationConfigurer(this::postProcess));
Expand Down
11 changes: 11 additions & 0 deletions ...zation/config/annotation/web/configurers/OAuth2DeviceAuthorizationEndpointConfigurer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,13 @@ public final class OAuth2DeviceAuthorizationEndpointConfigurer extends AbstractO
private Consumer<List<AuthenticationProvider>> authenticationProvidersConsumer = (authenticationProviders) -> {};
private AuthenticationSuccessHandler deviceAuthorizationResponseHandler;
private AuthenticationFailureHandler errorResponseHandler;

public boolean isEnableDeviceAuthorizationEndpoint() {
return enableDeviceAuthorizationEndpoint;
}

private String verificationUri;
private boolean enableDeviceAuthorizationEndpoint = true;

/**
* Restrict for internal use only.
Expand Down Expand Up @@ -161,6 +167,11 @@ public OAuth2DeviceAuthorizationEndpointConfigurer verificationUri(String verifi
return this;
}

public OAuth2DeviceAuthorizationEndpointConfigurer enableDeviceAuthorizationEndpoint(boolean enableDeviceAuthorizationEndpoint) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not keen on exposing it this way. As well, we would need to provide the same to OAuth2DeviceVerificationEndpointConfigurer to be consistent.

I also think we should come up with a solution that allows other endpoints to be disabled if needed. I don't think a "flag" setting on all the configurers makes sense.

At this point, I don't have any suggestions but I'll give it some thought. Please give it some further thought s well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gregecho Just circling back to this to see if you have come up with an alternative solution?

Copy link
Author

Choose a reason for hiding this comment

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

@jgrandja , sorry about that. I didn't have a chance to discovery the alternative solution. Any ideas?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gregecho I haven't had time either to come up with a solution. I'm going to close this PR for now. When you have time and come up with an alternative solution, please propose it here first and then I can either re-open this PR or you can submit a new one. Thanks.

this.enableDeviceAuthorizationEndpoint = enableDeviceAuthorizationEndpoint;
return this;
}

@Override
public void init(HttpSecurity builder) {
AuthorizationServerSettings authorizationServerSettings =
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
6 changes: 4 additions & 2 deletions samples/demo-authorizationserver/src/main/java/sample/config/AuthorizationServerConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,10 @@ public SecurityFilterChain authorizationServerSecurityFilterChain(

// @formatter:off
http.getConfigurer(OAuth2AuthorizationServerConfigurer.class)
.deviceAuthorizationEndpoint(deviceAuthorizationEndpoint ->
deviceAuthorizationEndpoint.verificationUri("/activate")
.deviceAuthorizationEndpoint(deviceAuthorizationEndpoint -> {
deviceAuthorizationEndpoint.verificationUri("/activate");
deviceAuthorizationEndpoint.enableDeviceAuthorizationEndpoint(true);
}
)
.deviceVerificationEndpoint(deviceVerificationEndpoint ->
deviceVerificationEndpoint.consentPage(CUSTOM_CONSENT_PAGE_URI)
Expand Down