Skip to content

Commit

Permalink
Limit oauth2Login() links to redirect-based flows
Browse files Browse the repository at this point in the history
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
  • Loading branch information
denisw authored and jgrandja committed Apr 14, 2021
1 parent a511750 commit d3af4f7
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 9 deletions.
@@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -686,10 +687,12 @@ private Map<String, String> getLoginLinks() {
this.authorizationEndpointConfig.authorizationRequestBaseUri :
OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI;
Map<String, String> 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;
}

Expand Down
@@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1283,7 +1284,11 @@ private Map<String, String> getLinks() {
return Collections.emptyMap();
}
Map<String, String> 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;
}

Expand Down
@@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
@@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit d3af4f7

Please sign in to comment.