Browse Source

Auto-redirect to provider login when one client configured

Fixes gh-5347
Joe Grandja 7 years ago
parent
commit
6c7d49759e

+ 33 - 11
config/src/main/java/org/springframework/security/config/annotation/web/configurers/AbstractAuthenticationFilterConfigurer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -234,20 +234,27 @@ public abstract class AbstractAuthenticationFilterConfigurer<B extends HttpSecur
 	@Override
 	public void init(B http) throws Exception {
 		updateAuthenticationDefaults();
-		if (permitAll) {
-			PermitAllSupport.permitAll(http, loginPage, loginProcessingUrl, failureUrl);
-		}
-
+		updateAccessDefaults(http);
 		registerDefaultAuthenticationEntryPoint(http);
 	}
 
 	@SuppressWarnings("unchecked")
-	private void registerDefaultAuthenticationEntryPoint(B http) {
+	protected final void registerDefaultAuthenticationEntryPoint(B http) {
+		registerAuthenticationEntryPoint(http, this.authenticationEntryPoint);
+	}
+
+	@SuppressWarnings("unchecked")
+	protected final void registerAuthenticationEntryPoint(B http, AuthenticationEntryPoint authenticationEntryPoint) {
 		ExceptionHandlingConfigurer<B> exceptionHandling = http
 				.getConfigurer(ExceptionHandlingConfigurer.class);
 		if (exceptionHandling == null) {
 			return;
 		}
+		exceptionHandling.defaultAuthenticationEntryPointFor(
+				postProcess(authenticationEntryPoint), getAuthenticationEntryPointMatcher(http));
+	}
+
+	protected final RequestMatcher getAuthenticationEntryPointMatcher(B http) {
 		ContentNegotiationStrategy contentNegotiationStrategy = http
 				.getSharedObject(ContentNegotiationStrategy.class);
 		if (contentNegotiationStrategy == null) {
@@ -262,10 +269,7 @@ public abstract class AbstractAuthenticationFilterConfigurer<B extends HttpSecur
 		RequestMatcher notXRequestedWith = new NegatedRequestMatcher(
 				new RequestHeaderRequestMatcher("X-Requested-With", "XMLHttpRequest"));
 
-		RequestMatcher preferredMatcher = new AndRequestMatcher(Arrays.asList(notXRequestedWith, mediaMatcher));
-
-		exceptionHandling.defaultAuthenticationEntryPointFor(
-				postProcess(authenticationEntryPoint), preferredMatcher);
+		return new AndRequestMatcher(Arrays.asList(notXRequestedWith, mediaMatcher));
 	}
 
 	@Override
@@ -351,6 +355,15 @@ public abstract class AbstractAuthenticationFilterConfigurer<B extends HttpSecur
 		return loginPage;
 	}
 
+	/**
+	 * Gets the Authentication Entry Point
+	 *
+	 * @return the Authentication Entry Point
+	 */
+	protected final AuthenticationEntryPoint getAuthenticationEntryPoint() {
+		return authenticationEntryPoint;
+	}
+
 	/**
 	 * Gets the URL to submit an authentication request to (i.e. where username/password
 	 * must be submitted)
@@ -375,7 +388,7 @@ public abstract class AbstractAuthenticationFilterConfigurer<B extends HttpSecur
 	 *
 	 * @throws Exception
 	 */
-	private void updateAuthenticationDefaults() {
+	protected final void updateAuthenticationDefaults() {
 		if (loginProcessingUrl == null) {
 			loginProcessingUrl(loginPage);
 		}
@@ -390,6 +403,15 @@ public abstract class AbstractAuthenticationFilterConfigurer<B extends HttpSecur
 		}
 	}
 
+	/**
+	 * Updates the default values for access.
+	 */
+	protected final void updateAccessDefaults(B http) {
+		if (permitAll) {
+			PermitAllSupport.permitAll(http, loginPage, loginProcessingUrl, failureUrl);
+		}
+	}
+
 	/**
 	 * Sets the loginPage and updates the {@link AuthenticationEntryPoint}.
 	 * @param loginPage

+ 60 - 16
config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java

@@ -54,15 +54,23 @@ import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequ
 import org.springframework.security.oauth2.core.oidc.OidcScopes;
 import org.springframework.security.oauth2.core.oidc.user.OidcUser;
 import org.springframework.security.oauth2.core.user.OAuth2User;
+import org.springframework.security.web.AuthenticationEntryPoint;
+import org.springframework.security.web.authentication.DelegatingAuthenticationEntryPoint;
+import org.springframework.security.web.authentication.LoginUrlAuthenticationEntryPoint;
 import org.springframework.security.web.authentication.ui.DefaultLoginPageGeneratingFilter;
 import org.springframework.security.web.savedrequest.RequestCache;
+import org.springframework.security.web.util.matcher.AndRequestMatcher;
 import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
+import org.springframework.security.web.util.matcher.NegatedRequestMatcher;
+import org.springframework.security.web.util.matcher.OrRequestMatcher;
 import org.springframework.security.web.util.matcher.RequestMatcher;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -420,10 +428,24 @@ public final class OAuth2LoginConfigurer<B extends HttpSecurityBuilder<B>> exten
 				this.loginProcessingUrl);
 		this.setAuthenticationFilter(authenticationFilter);
 		super.loginProcessingUrl(this.loginProcessingUrl);
+
 		if (this.loginPage != null) {
+			// Set custom login page
 			super.loginPage(this.loginPage);
+			super.init(http);
+		} else {
+			Map<String, String> loginUrlToClientName = this.getLoginLinks();
+			if (loginUrlToClientName.size() == 1) {
+				// Setup auto-redirect to provider login page
+				// when only 1 client is configured
+				this.updateAuthenticationDefaults();
+				this.updateAccessDefaults(http);
+				String providerLoginPage = loginUrlToClientName.keySet().iterator().next();
+				this.registerAuthenticationEntryPoint(http, this.getLoginEntryPoint(http, providerLoginPage));
+			} else {
+				super.init(http);
+			}
 		}
-		super.init(http);
 
 		OAuth2AccessTokenResponseClient<OAuth2AuthorizationCodeGrantRequest> accessTokenResponseClient =
 			this.tokenEndpointConfig.accessTokenResponseClient;
@@ -529,9 +551,9 @@ public final class OAuth2LoginConfigurer<B extends HttpSecurityBuilder<B>> exten
 
 	private GrantedAuthoritiesMapper getGrantedAuthoritiesMapperBean() {
 		Map<String, GrantedAuthoritiesMapper> grantedAuthoritiesMapperMap =
-			BeanFactoryUtils.beansOfTypeIncludingAncestors(
-				this.getBuilder().getSharedObject(ApplicationContext.class),
-				GrantedAuthoritiesMapper.class);
+				BeanFactoryUtils.beansOfTypeIncludingAncestors(
+						this.getBuilder().getSharedObject(ApplicationContext.class),
+						GrantedAuthoritiesMapper.class);
 		return (!grantedAuthoritiesMapperMap.isEmpty() ? grantedAuthoritiesMapperMap.values().iterator().next() : null);
 	}
 
@@ -541,29 +563,51 @@ public final class OAuth2LoginConfigurer<B extends HttpSecurityBuilder<B>> exten
 			return;
 		}
 
+		loginPageGeneratingFilter.setOauth2LoginEnabled(true);
+		loginPageGeneratingFilter.setOauth2AuthenticationUrlToClientName(this.getLoginLinks());
+		loginPageGeneratingFilter.setLoginPageUrl(this.getLoginPage());
+		loginPageGeneratingFilter.setFailureUrl(this.getFailureUrl());
+	}
+
+	@SuppressWarnings("unchecked")
+	private Map<String, String> getLoginLinks() {
 		Iterable<ClientRegistration> clientRegistrations = null;
 		ClientRegistrationRepository clientRegistrationRepository =
-			OAuth2ClientConfigurerUtils.getClientRegistrationRepository(this.getBuilder());
+				OAuth2ClientConfigurerUtils.getClientRegistrationRepository(this.getBuilder());
 		ResolvableType type = ResolvableType.forInstance(clientRegistrationRepository).as(Iterable.class);
 		if (type != ResolvableType.NONE && ClientRegistration.class.isAssignableFrom(type.resolveGenerics()[0])) {
 			clientRegistrations = (Iterable<ClientRegistration>) clientRegistrationRepository;
 		}
 		if (clientRegistrations == null) {
-			return;
+			return Collections.emptyMap();
 		}
 
 		String authorizationRequestBaseUri = this.authorizationEndpointConfig.authorizationRequestBaseUri != null ?
-			this.authorizationEndpointConfig.authorizationRequestBaseUri :
-			OAuth2AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI;
-		Map<String, String> authenticationUrlToClientName = new HashMap<>();
+				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()));
+
+		return loginUrlToClientName;
+	}
 
-		clientRegistrations.forEach(registration -> authenticationUrlToClientName.put(
-			authorizationRequestBaseUri + "/" + registration.getRegistrationId(),
-			registration.getClientName()));
-		loginPageGeneratingFilter.setOauth2LoginEnabled(true);
-		loginPageGeneratingFilter.setOauth2AuthenticationUrlToClientName(authenticationUrlToClientName);
-		loginPageGeneratingFilter.setLoginPageUrl(this.getLoginPage());
-		loginPageGeneratingFilter.setFailureUrl(this.getFailureUrl());
+	private AuthenticationEntryPoint getLoginEntryPoint(B http, String providerLoginPage) {
+		RequestMatcher loginPageMatcher = new AntPathRequestMatcher(this.getLoginPage());
+		RequestMatcher faviconMatcher = new AntPathRequestMatcher("/favicon.ico");
+		RequestMatcher defaultEntryPointMatcher = this.getAuthenticationEntryPointMatcher(http);
+		RequestMatcher defaultLoginPageMatcher = new AndRequestMatcher(
+				new OrRequestMatcher(loginPageMatcher, faviconMatcher), defaultEntryPointMatcher);
+
+		LinkedHashMap<RequestMatcher, AuthenticationEntryPoint> entryPoints = new LinkedHashMap<>();
+		entryPoints.put(new NegatedRequestMatcher(defaultLoginPageMatcher),
+				new LoginUrlAuthenticationEntryPoint(providerLoginPage));
+
+		DelegatingAuthenticationEntryPoint loginEntryPoint = new DelegatingAuthenticationEntryPoint(entryPoints);
+		loginEntryPoint.setDefaultEntryPoint(this.getAuthenticationEntryPoint());
+
+		return loginEntryPoint;
 	}
 
 	private static class OidcAuthenticationRequestChecker implements AuthenticationProvider {

+ 107 - 11
config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurerTests.java

@@ -15,6 +15,7 @@
  */
 package org.springframework.security.config.annotation.web.configurers.oauth2.client;
 
+import org.apache.http.HttpHeaders;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -22,6 +23,7 @@ import org.springframework.beans.PropertyAccessorFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.annotation.Bean;
+import org.springframework.http.MediaType;
 import org.springframework.mock.web.MockFilterChain;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
@@ -81,14 +83,19 @@ import static org.assertj.core.api.Assertions.assertThat;
  * Tests for {@link OAuth2LoginConfigurer}.
  *
  * @author Kazuki Shimizu
+ * @author Joe Grandja
  * @since 5.0.1
  */
 public class OAuth2LoginConfigurerTests {
 
-	private static final ClientRegistration CLIENT_REGISTRATION = CommonOAuth2Provider.GOOGLE
+	private static final ClientRegistration GOOGLE_CLIENT_REGISTRATION = CommonOAuth2Provider.GOOGLE
 			.getBuilder("google").clientId("clientId").clientSecret("clientSecret")
 			.build();
 
+	private static final ClientRegistration GITHUB_CLIENT_REGISTRATION = CommonOAuth2Provider.GITHUB
+			.getBuilder("github").clientId("clientId").clientSecret("clientSecret")
+			.build();
+
 	private ConfigurableApplicationContext context;
 
 	@Autowired
@@ -239,6 +246,62 @@ public class OAuth2LoginConfigurerTests {
 		assertThat(this.response.getRedirectedUrl()).matches("https://accounts.google.com/o/oauth2/v2/auth\\?response_type=code&client_id=clientId&scope=openid\\+profile\\+email&state=.{15,}&redirect_uri=http%3A%2F%2Flocalhost%2Flogin%2Foauth2%2Fcode%2Fgoogle&custom-param1=custom-value1");
 	}
 
+	// gh-5347
+	@Test
+	public void oauth2LoginWithOneClientConfiguredThenRedirectForAuthorization() throws Exception {
+		loadConfig(OAuth2LoginConfig.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");
+	}
+
+	// gh-5347
+	@Test
+	public void oauth2LoginWithOneClientConfiguredAndRequestFaviconNotAuthenticatedThenRedirectDefaultLoginPage() throws Exception {
+		loadConfig(OAuth2LoginConfig.class);
+
+		String requestUri = "/favicon.ico";
+		this.request = new MockHttpServletRequest("GET", requestUri);
+		this.request.setServletPath(requestUri);
+		this.request.addHeader(HttpHeaders.ACCEPT, new MediaType("image", "*").toString());
+
+		this.springSecurityFilterChain.doFilter(this.request, this.response, this.filterChain);
+
+		assertThat(this.response.getRedirectedUrl()).matches("http://localhost/login");
+	}
+
+	// gh-5347
+	@Test
+	public void oauth2LoginWithMultipleClientsConfiguredThenRedirectDefaultLoginPage() throws Exception {
+		loadConfig(OAuth2LoginConfigMultipleClients.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/login");
+	}
+
+	@Test
+	public void oauth2LoginWithCustomLoginPageThenRedirectCustomLoginPage() throws Exception {
+		loadConfig(OAuth2LoginConfigCustomLoginPage.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/custom-login");
+	}
+
 	@Test
 	public void oidcLogin() throws Exception {
 		// setup application context
@@ -348,15 +411,19 @@ public class OAuth2LoginConfigurerTests {
 	}
 
 	private OAuth2AuthorizationRequest createOAuth2AuthorizationRequest(String... scopes) {
+		return this.createOAuth2AuthorizationRequest(GOOGLE_CLIENT_REGISTRATION, scopes);
+	}
+
+	private OAuth2AuthorizationRequest createOAuth2AuthorizationRequest(ClientRegistration registration, String... scopes) {
 		return OAuth2AuthorizationRequest.authorizationCode()
-				.authorizationUri(CLIENT_REGISTRATION.getProviderDetails().getAuthorizationUri())
-				.clientId(CLIENT_REGISTRATION.getClientId())
+				.authorizationUri(registration.getProviderDetails().getAuthorizationUri())
+				.clientId(registration.getClientId())
 				.state("state123")
 				.redirectUri("http://localhost")
 				.additionalParameters(
-					Collections.singletonMap(
-						OAuth2ParameterNames.REGISTRATION_ID,
-						CLIENT_REGISTRATION.getRegistrationId()))
+						Collections.singletonMap(
+								OAuth2ParameterNames.REGISTRATION_ID,
+								registration.getRegistrationId()))
 				.scope(scopes)
 				.build();
 	}
@@ -368,7 +435,7 @@ public class OAuth2LoginConfigurerTests {
 			http
 				.oauth2Login()
 					.clientRegistrationRepository(
-						new InMemoryClientRegistrationRepository(CLIENT_REGISTRATION));
+						new InMemoryClientRegistrationRepository(GOOGLE_CLIENT_REGISTRATION));
 			super.configure(http);
 		}
 	}
@@ -380,7 +447,7 @@ public class OAuth2LoginConfigurerTests {
 			http
 				.oauth2Login()
 					.clientRegistrationRepository(
-							new InMemoryClientRegistrationRepository(CLIENT_REGISTRATION))
+							new InMemoryClientRegistrationRepository(GOOGLE_CLIENT_REGISTRATION))
 					.userInfoEndpoint()
 						.userAuthoritiesMapper(createGrantedAuthoritiesMapper());
 			super.configure(http);
@@ -398,7 +465,7 @@ public class OAuth2LoginConfigurerTests {
 
 		@Bean
 		ClientRegistrationRepository clientRegistrationRepository() {
-			return new InMemoryClientRegistrationRepository(CLIENT_REGISTRATION);
+			return new InMemoryClientRegistrationRepository(GOOGLE_CLIENT_REGISTRATION);
 		}
 
 		@Bean
@@ -414,7 +481,7 @@ public class OAuth2LoginConfigurerTests {
 			http
 				.oauth2Login()
 					.clientRegistrationRepository(
-						new InMemoryClientRegistrationRepository(CLIENT_REGISTRATION))
+						new InMemoryClientRegistrationRepository(GOOGLE_CLIENT_REGISTRATION))
 					.loginProcessingUrl("/login/oauth2/*");
 			super.configure(http);
 		}
@@ -423,7 +490,7 @@ public class OAuth2LoginConfigurerTests {
 	@EnableWebSecurity
 	static class OAuth2LoginConfigCustomAuthorizationRequestResolver extends CommonWebSecurityConfigurerAdapter {
 		private ClientRegistrationRepository clientRegistrationRepository =
-				new InMemoryClientRegistrationRepository(CLIENT_REGISTRATION);
+				new InMemoryClientRegistrationRepository(GOOGLE_CLIENT_REGISTRATION);
 
 		@Override
 		protected void configure(HttpSecurity http) throws Exception {
@@ -449,10 +516,39 @@ public class OAuth2LoginConfigurerTests {
 		}
 	}
 
+	@EnableWebSecurity
+	static class OAuth2LoginConfigMultipleClients extends CommonWebSecurityConfigurerAdapter {
+		@Override
+		protected void configure(HttpSecurity http) throws Exception {
+			http
+					.oauth2Login()
+					.clientRegistrationRepository(
+							new InMemoryClientRegistrationRepository(
+									GOOGLE_CLIENT_REGISTRATION, GITHUB_CLIENT_REGISTRATION));
+			super.configure(http);
+		}
+	}
+
+	@EnableWebSecurity
+	static class OAuth2LoginConfigCustomLoginPage extends CommonWebSecurityConfigurerAdapter {
+		@Override
+		protected void configure(HttpSecurity http) throws Exception {
+			http
+					.oauth2Login()
+					.clientRegistrationRepository(
+							new InMemoryClientRegistrationRepository(GOOGLE_CLIENT_REGISTRATION))
+					.loginPage("/custom-login");
+			super.configure(http);
+		}
+	}
+
 	private static abstract class CommonWebSecurityConfigurerAdapter extends WebSecurityConfigurerAdapter {
 		@Override
 		protected void configure(HttpSecurity http) throws Exception {
 			http
+				.authorizeRequests()
+					.anyRequest().authenticated()
+					.and()
 				.securityContext()
 					.securityContextRepository(securityContextRepository())
 					.and()