From 67d55201946bc934e56a599e79881bfbc77debcf 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 | 11 ++++-- .../config/web/server/ServerHttpSecurity.java | 8 +++-- .../client/OAuth2LoginConfigurerTests.java | 36 ++++++++++++++++++- .../config/web/server/OAuth2LoginTests.java | 33 ++++++++++++++++- 4 files changed, 81 insertions(+), 7 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 a7e0cc46cb..9fdbb2b8c8 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-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. @@ -58,6 +58,7 @@ import org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequest 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; @@ -485,8 +486,12 @@ public final class OAuth2LoginConfigurer> ? 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 2cbbaeb231..2e578002de 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 @@ -81,6 +81,7 @@ import org.springframework.security.oauth2.client.web.server.ServerOAuth2Authori 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.OAuth2AuthenticationException; import org.springframework.security.oauth2.core.OAuth2AuthorizationException; import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest; @@ -3303,8 +3304,11 @@ public class ServerHttpSecurity { 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 5207bfd772..15f0dd2996 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. @@ -126,6 +126,11 @@ public class OAuth2LoginConfigurerTests { .build(); // @formatter:on + // @formatter:off + private static final ClientRegistration CLIENT_CREDENTIALS_REGISTRATION = TestClientRegistrations.clientCredentials() + .build(); + // @formatter:on + private ConfigurableApplicationContext context; @Autowired @@ -396,6 +401,18 @@ public class OAuth2LoginConfigurerTests { 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); @@ -799,6 +816,23 @@ public class OAuth2LoginConfigurerTests { } + @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 { 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 211d353203..259b49bd55 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. @@ -131,6 +131,11 @@ public class OAuth2LoginTests { private static ClientRegistration google = CommonOAuth2Provider.GOOGLE.getBuilder("google").clientId("client") .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) { @@ -176,6 +181,22 @@ public class OAuth2LoginTests { assertThat(driver.getCurrentUrl()).startsWith("https://github.com/login/oauth/authorize"); } + // 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"); + } + // gh-8118 @Test public void defaultLoginPageWithSingleClientRegistrationAndXhrRequestThenDoesNotRedirectForAuthorization() { @@ -543,6 +564,16 @@ public class OAuth2LoginTests { } + @EnableWebFluxSecurity + static class OAuth2LoginWithAuthorizationCodeAndClientCredentialsClientRegistration { + + @Bean + InMemoryReactiveClientRegistrationRepository clientRegistrationRepository() { + return new InMemoryReactiveClientRegistrationRepository(github, clientCredentials); + } + + } + @EnableWebFlux static class OAuth2AuthorizeWithMockObjectsConfig {