From d3af4f73547cd156b860b128cd6055fdc2e3b505 Mon Sep 17 00:00:00 2001 From: Denis Washington Date: Tue, 23 Feb 2021 22:57:43 +0100 Subject: [PATCH] Limit oauth2Login() links to redirect-based flows This prevents the generated login page from showing links for authorization grant types like "client_credentials" which are not redirect-based, and thus not meant for interactive use in the browser. Closes gh-9457 --- .../oauth2/client/OAuth2LoginConfigurer.java | 13 ++++--- .../config/web/server/ServerHttpSecurity.java | 9 +++-- .../client/OAuth2LoginConfigurerTests.java | 36 ++++++++++++++++++- .../config/web/server/OAuth2LoginTests.java | 33 ++++++++++++++++- 4 files changed, 82 insertions(+), 9 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java index 823de21892c..9c4e98d91c5 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -57,6 +57,7 @@ import org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequestResolver; import org.springframework.security.oauth2.client.web.OAuth2AuthorizedClientRepository; import org.springframework.security.oauth2.client.web.OAuth2LoginAuthenticationFilter; +import org.springframework.security.oauth2.core.AuthorizationGrantType; import org.springframework.security.oauth2.core.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.OAuth2Error; import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; @@ -686,10 +687,12 @@ private Map getLoginLinks() { this.authorizationEndpointConfig.authorizationRequestBaseUri : OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI; Map loginUrlToClientName = new HashMap<>(); - clientRegistrations.forEach(registration -> loginUrlToClientName.put( - authorizationRequestBaseUri + "/" + registration.getRegistrationId(), - registration.getClientName())); - + clientRegistrations.forEach((registration) -> { + if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(registration.getAuthorizationGrantType())) { + String authorizationRequestUri = authorizationRequestBaseUri + "/" + registration.getRegistrationId(); + loginUrlToClientName.put(authorizationRequestUri, registration.getClientName()); + } + }); return loginUrlToClientName; } diff --git a/config/src/main/java/org/springframework/security/config/web/server/ServerHttpSecurity.java b/config/src/main/java/org/springframework/security/config/web/server/ServerHttpSecurity.java index c6ebdbe2dd1..39392c1aaae 100644 --- a/config/src/main/java/org/springframework/security/config/web/server/ServerHttpSecurity.java +++ b/config/src/main/java/org/springframework/security/config/web/server/ServerHttpSecurity.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -82,6 +82,7 @@ import org.springframework.security.oauth2.client.web.server.ServerOAuth2AuthorizedClientRepository; import org.springframework.security.oauth2.client.web.server.WebSessionOAuth2ServerAuthorizationRequestRepository; import org.springframework.security.oauth2.client.web.server.authentication.OAuth2LoginAuthenticationWebFilter; +import org.springframework.security.oauth2.core.AuthorizationGrantType; import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; import org.springframework.security.oauth2.core.oidc.user.OidcUser; import org.springframework.security.oauth2.core.user.OAuth2User; @@ -1283,7 +1284,11 @@ private Map getLinks() { return Collections.emptyMap(); } Map result = new HashMap<>(); - registrations.iterator().forEachRemaining(r -> result.put("/oauth2/authorization/" + r.getRegistrationId(), r.getClientName())); + registrations.iterator().forEachRemaining((r) -> { + if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(r.getAuthorizationGrantType())) { + result.put("/oauth2/authorization/" + r.getRegistrationId(), r.getClientName()); + } + }); return result; } diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurerTests.java index 48274c5e08a..67b3fc36a21 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -116,6 +116,11 @@ public class OAuth2LoginConfigurerTests { .getBuilder("github").clientId("clientId").clientSecret("clientSecret") .build(); + // @formatter:off + private static final ClientRegistration CLIENT_CREDENTIALS_REGISTRATION = TestClientRegistrations.clientCredentials() + .build(); + // @formatter:on + private ConfigurableApplicationContext context; @Autowired @@ -431,6 +436,18 @@ public void oauth2LoginWithOneClientConfiguredAndRequestXHRNotAuthenticatedThenD assertThat(this.response.getRedirectedUrl()).doesNotMatch("http://localhost/oauth2/authorization/google"); } + // gh-9457 + @Test + public void oauth2LoginWithOneAuthorizationCodeClientAndOtherClientsConfiguredThenRedirectForAuthorization() + throws Exception { + loadConfig(OAuth2LoginConfigAuthorizationCodeClientAndOtherClients.class); + String requestUri = "/"; + this.request = new MockHttpServletRequest("GET", requestUri); + this.request.setServletPath(requestUri); + this.springSecurityFilterChain.doFilter(this.request, this.response, this.filterChain); + assertThat(this.response.getRedirectedUrl()).matches("http://localhost/oauth2/authorization/google"); + } + @Test public void oauth2LoginWithCustomLoginPageThenRedirectCustomLoginPage() throws Exception { loadConfig(OAuth2LoginConfigCustomLoginPage.class); @@ -801,6 +818,23 @@ protected void configure(HttpSecurity http) throws Exception { } } + @EnableWebSecurity + static class OAuth2LoginConfigAuthorizationCodeClientAndOtherClients extends CommonWebSecurityConfigurerAdapter { + + @Override + protected void configure(HttpSecurity http) throws Exception { + // @formatter:off + http + .oauth2Login() + .clientRegistrationRepository( + new InMemoryClientRegistrationRepository( + GOOGLE_CLIENT_REGISTRATION, CLIENT_CREDENTIALS_REGISTRATION)); + // @formatter:on + super.configure(http); + } + + } + @EnableWebSecurity static class OAuth2LoginConfigCustomLoginPage extends CommonWebSecurityConfigurerAdapter { @Override diff --git a/config/src/test/java/org/springframework/security/config/web/server/OAuth2LoginTests.java b/config/src/test/java/org/springframework/security/config/web/server/OAuth2LoginTests.java index b1e5662a3ef..5685de07b38 100644 --- a/config/src/test/java/org/springframework/security/config/web/server/OAuth2LoginTests.java +++ b/config/src/test/java/org/springframework/security/config/web/server/OAuth2LoginTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -136,6 +136,11 @@ public class OAuth2LoginTests { .clientSecret("secret") .build(); + // @formatter:off + private static ClientRegistration clientCredentials = TestClientRegistrations.clientCredentials() + .build(); + // @formatter:on + @Autowired public void setApplicationContext(ApplicationContext context) { if (context.getBeanNamesForType(WebHandler.class).length > 0) { @@ -214,6 +219,32 @@ InMemoryReactiveClientRegistrationRepository clientRegistrationRepository() { } } + // gh-9457 + @Test + public void defaultLoginPageWithAuthorizationCodeAndClientCredentialsClientRegistrationThenRedirect() { + this.spring.register(OAuth2LoginWithAuthorizationCodeAndClientCredentialsClientRegistration.class).autowire(); + // @formatter:off + WebTestClient webTestClient = WebTestClientBuilder + .bindToWebFilters(new GitHubWebFilter(), this.springSecurity) + .build(); + WebDriver driver = WebTestClientHtmlUnitDriverBuilder + .webTestClientSetup(webTestClient) + .build(); + // @formatter:on + driver.get("http://localhost/"); + assertThat(driver.getCurrentUrl()).startsWith("https://github.com/login/oauth/authorize"); + } + + @EnableWebFluxSecurity + static class OAuth2LoginWithAuthorizationCodeAndClientCredentialsClientRegistration { + + @Bean + InMemoryReactiveClientRegistrationRepository clientRegistrationRepository() { + return new InMemoryReactiveClientRegistrationRepository(github, clientCredentials); + } + + } + @Test public void oauth2AuthorizeWhenCustomObjectsThenUsed() { this.spring.register(OAuth2LoginWithSingleClientRegistrations.class,