Skip to content

Commit 2b4b856

Browse files
deniswjgrandja
authored andcommitted
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 a325216 commit 2b4b856

File tree

4 files changed

+80
-6
lines changed

4 files changed

+80
-6
lines changed

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

Lines changed: 8 additions & 3 deletions
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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
import org.springframework.security.oauth2.client.web.server.ServerOAuth2AuthorizedClientRepository;
8181
import org.springframework.security.oauth2.client.web.server.WebSessionOAuth2ServerAuthorizationRequestRepository;
8282
import org.springframework.security.oauth2.client.web.server.authentication.OAuth2LoginAuthenticationWebFilter;
83+
import org.springframework.security.oauth2.core.AuthorizationGrantType;
8384
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
8485
import org.springframework.security.oauth2.core.OAuth2AuthorizationException;
8586
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
@@ -3356,8 +3357,11 @@ private Map<String, String> getLinks() {
33563357
return Collections.emptyMap();
33573358
}
33583359
Map<String, String> result = new HashMap<>();
3359-
registrations.iterator().forEachRemaining(
3360-
(r) -> result.put("/oauth2/authorization/" + r.getRegistrationId(), r.getClientName()));
3360+
registrations.iterator().forEachRemaining((r) -> {
3361+
if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(r.getAuthorizationGrantType())) {
3362+
result.put("/oauth2/authorization/" + r.getRegistrationId(), r.getClientName());
3363+
}
3364+
});
33613365
return result;
33623366
}
33633367

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

Lines changed: 35 additions & 1 deletion
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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,11 @@ public class OAuth2LoginTests {
134134
private static ClientRegistration google = CommonOAuth2Provider.GOOGLE.getBuilder("google").clientId("client")
135135
.clientSecret("secret").build();
136136

137+
// @formatter:off
138+
private static ClientRegistration clientCredentials = TestClientRegistrations.clientCredentials()
139+
.build();
140+
// @formatter:on
141+
137142
@Autowired
138143
public void setApplicationContext(ApplicationContext context) {
139144
if (context.getBeanNamesForType(WebHandler.class).length > 0) {
@@ -179,6 +184,22 @@ public void defaultLoginPageWithSingleClientRegistrationThenRedirect() {
179184
assertThat(driver.getCurrentUrl()).startsWith("https://github.com/login/oauth/authorize");
180185
}
181186

187+
// gh-9457
188+
@Test
189+
public void defaultLoginPageWithAuthorizationCodeAndClientCredentialsClientRegistrationThenRedirect() {
190+
this.spring.register(OAuth2LoginWithAuthorizationCodeAndClientCredentialsClientRegistration.class).autowire();
191+
// @formatter:off
192+
WebTestClient webTestClient = WebTestClientBuilder
193+
.bindToWebFilters(new GitHubWebFilter(), this.springSecurity)
194+
.build();
195+
WebDriver driver = WebTestClientHtmlUnitDriverBuilder
196+
.webTestClientSetup(webTestClient)
197+
.build();
198+
// @formatter:on
199+
driver.get("http://localhost/");
200+
assertThat(driver.getCurrentUrl()).startsWith("https://github.com/login/oauth/authorize");
201+
}
202+
182203
@Test
183204
public void defaultLoginPageWithSingleClientRegistrationAndFormLoginThenLinks() {
184205
this.spring.register(OAuth2LoginWithSingleClientRegistrations.class, OAuth2LoginWithFormLogin.class).autowire();
@@ -564,6 +585,16 @@ InMemoryReactiveClientRegistrationRepository clientRegistrationRepository() {
564585

565586
}
566587

588+
@EnableWebFluxSecurity
589+
static class OAuth2LoginWithAuthorizationCodeAndClientCredentialsClientRegistration {
590+
591+
@Bean
592+
InMemoryReactiveClientRegistrationRepository clientRegistrationRepository() {
593+
return new InMemoryReactiveClientRegistrationRepository(github, clientCredentials);
594+
}
595+
596+
}
597+
567598
@EnableWebFlux
568599
static class OAuth2AuthorizeWithMockObjectsConfig {
569600

0 commit comments

Comments
 (0)