Skip to content

Commit 67d5520

Browse files
deniswjgrandja
denisw
authored and
jgrandja
committed
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
1 parent c381503 commit 67d5520

File tree

4 files changed

+81
-7
lines changed

4 files changed

+81
-7
lines changed

‎config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java

+8-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 the original author or authors.
2+
* Copyright 2002-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -58,6 +58,7 @@
5858
import org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequestResolver;
5959
import org.springframework.security.oauth2.client.web.OAuth2AuthorizedClientRepository;
6060
import org.springframework.security.oauth2.client.web.OAuth2LoginAuthenticationFilter;
61+
import org.springframework.security.oauth2.core.AuthorizationGrantType;
6162
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
6263
import org.springframework.security.oauth2.core.OAuth2Error;
6364
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
@@ -485,8 +486,12 @@ private Map<String, String> getLoginLinks() {
485486
? this.authorizationEndpointConfig.authorizationRequestBaseUri
486487
: OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI;
487488
Map<String, String> loginUrlToClientName = new HashMap<>();
488-
clientRegistrations.forEach((registration) -> loginUrlToClientName.put(
489-
authorizationRequestBaseUri + "/" + registration.getRegistrationId(), registration.getClientName()));
489+
clientRegistrations.forEach((registration) -> {
490+
if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(registration.getAuthorizationGrantType())) {
491+
String authorizationRequestUri = authorizationRequestBaseUri + "/" + registration.getRegistrationId();
492+
loginUrlToClientName.put(authorizationRequestUri, registration.getClientName());
493+
}
494+
});
490495
return loginUrlToClientName;
491496
}
492497

‎config/src/main/java/org/springframework/security/config/web/server/ServerHttpSecurity.java

+6-2
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
import org.springframework.security.oauth2.client.web.server.ServerOAuth2AuthorizedClientRepository;
8282
import org.springframework.security.oauth2.client.web.server.WebSessionOAuth2ServerAuthorizationRequestRepository;
8383
import org.springframework.security.oauth2.client.web.server.authentication.OAuth2LoginAuthenticationWebFilter;
84+
import org.springframework.security.oauth2.core.AuthorizationGrantType;
8485
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
8586
import org.springframework.security.oauth2.core.OAuth2AuthorizationException;
8687
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
@@ -3303,8 +3304,11 @@ private Map<String, String> getLinks() {
33033304
return Collections.emptyMap();
33043305
}
33053306
Map<String, String> result = new HashMap<>();
3306-
registrations.iterator().forEachRemaining(
3307-
(r) -> result.put("/oauth2/authorization/" + r.getRegistrationId(), r.getClientName()));
3307+
registrations.iterator().forEachRemaining((r) -> {
3308+
if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(r.getAuthorizationGrantType())) {
3309+
result.put("/oauth2/authorization/" + r.getRegistrationId(), r.getClientName());
3310+
}
3311+
});
33083312
return result;
33093313
}
33103314

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurerTests.java

+35-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -126,6 +126,11 @@ public class OAuth2LoginConfigurerTests {
126126
.build();
127127
// @formatter:on
128128

129+
// @formatter:off
130+
private static final ClientRegistration CLIENT_CREDENTIALS_REGISTRATION = TestClientRegistrations.clientCredentials()
131+
.build();
132+
// @formatter:on
133+
129134
private ConfigurableApplicationContext context;
130135

131136
@Autowired
@@ -396,6 +401,18 @@ public void oauth2LoginWithOneClientConfiguredAndRequestXHRNotAuthenticatedThenD
396401
assertThat(this.response.getRedirectedUrl()).doesNotMatch("http://localhost/oauth2/authorization/google");
397402
}
398403

404+
// gh-9457
405+
@Test
406+
public void oauth2LoginWithOneAuthorizationCodeClientAndOtherClientsConfiguredThenRedirectForAuthorization()
407+
throws Exception {
408+
loadConfig(OAuth2LoginConfigAuthorizationCodeClientAndOtherClients.class);
409+
String requestUri = "/";
410+
this.request = new MockHttpServletRequest("GET", requestUri);
411+
this.request.setServletPath(requestUri);
412+
this.springSecurityFilterChain.doFilter(this.request, this.response, this.filterChain);
413+
assertThat(this.response.getRedirectedUrl()).matches("http://localhost/oauth2/authorization/google");
414+
}
415+
399416
@Test
400417
public void oauth2LoginWithCustomLoginPageThenRedirectCustomLoginPage() throws Exception {
401418
loadConfig(OAuth2LoginConfigCustomLoginPage.class);
@@ -799,6 +816,23 @@ protected void configure(HttpSecurity http) throws Exception {
799816

800817
}
801818

819+
@EnableWebSecurity
820+
static class OAuth2LoginConfigAuthorizationCodeClientAndOtherClients extends CommonWebSecurityConfigurerAdapter {
821+
822+
@Override
823+
protected void configure(HttpSecurity http) throws Exception {
824+
// @formatter:off
825+
http
826+
.oauth2Login()
827+
.clientRegistrationRepository(
828+
new InMemoryClientRegistrationRepository(
829+
GOOGLE_CLIENT_REGISTRATION, CLIENT_CREDENTIALS_REGISTRATION));
830+
// @formatter:on
831+
super.configure(http);
832+
}
833+
834+
}
835+
802836
@EnableWebSecurity
803837
static class OAuth2LoginConfigCustomLoginPage extends CommonWebSecurityConfigurerAdapter {
804838

‎config/src/test/java/org/springframework/security/config/web/server/OAuth2LoginTests.java

+32-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 the original author or authors.
2+
* Copyright 2002-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -131,6 +131,11 @@ public class OAuth2LoginTests {
131131
private static ClientRegistration google = CommonOAuth2Provider.GOOGLE.getBuilder("google").clientId("client")
132132
.clientSecret("secret").build();
133133

134+
// @formatter:off
135+
private static ClientRegistration clientCredentials = TestClientRegistrations.clientCredentials()
136+
.build();
137+
// @formatter:on
138+
134139
@Autowired
135140
public void setApplicationContext(ApplicationContext context) {
136141
if (context.getBeanNamesForType(WebHandler.class).length > 0) {
@@ -176,6 +181,22 @@ public void defaultLoginPageWithSingleClientRegistrationThenRedirect() {
176181
assertThat(driver.getCurrentUrl()).startsWith("https://github.com/login/oauth/authorize");
177182
}
178183

184+
// gh-9457
185+
@Test
186+
public void defaultLoginPageWithAuthorizationCodeAndClientCredentialsClientRegistrationThenRedirect() {
187+
this.spring.register(OAuth2LoginWithAuthorizationCodeAndClientCredentialsClientRegistration.class).autowire();
188+
// @formatter:off
189+
WebTestClient webTestClient = WebTestClientBuilder
190+
.bindToWebFilters(new GitHubWebFilter(), this.springSecurity)
191+
.build();
192+
WebDriver driver = WebTestClientHtmlUnitDriverBuilder
193+
.webTestClientSetup(webTestClient)
194+
.build();
195+
// @formatter:on
196+
driver.get("http://localhost/");
197+
assertThat(driver.getCurrentUrl()).startsWith("https://github.com/login/oauth/authorize");
198+
}
199+
179200
// gh-8118
180201
@Test
181202
public void defaultLoginPageWithSingleClientRegistrationAndXhrRequestThenDoesNotRedirectForAuthorization() {
@@ -543,6 +564,16 @@ InMemoryReactiveClientRegistrationRepository clientRegistrationRepository() {
543564

544565
}
545566

567+
@EnableWebFluxSecurity
568+
static class OAuth2LoginWithAuthorizationCodeAndClientCredentialsClientRegistration {
569+
570+
@Bean
571+
InMemoryReactiveClientRegistrationRepository clientRegistrationRepository() {
572+
return new InMemoryReactiveClientRegistrationRepository(github, clientCredentials);
573+
}
574+
575+
}
576+
546577
@EnableWebFlux
547578
static class OAuth2AuthorizeWithMockObjectsConfig {
548579

0 commit comments

Comments
 (0)