From d0bb94b8874367d9d1c25fb150fb7be5152b259d Mon Sep 17 00:00:00 2001
From: Fang Xia <felord@qq.com>
Date: Sat, 19 Mar 2022 22:53:12 +0800
Subject: [PATCH] Enhance validation for configured issuer

Closes gh-649
---
 .../OAuth2AuthorizationServerConfigurer.java  |  8 +-
 .../server/authorization/OidcTests.java       | 85 +++++++++++++++++++
 2 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationServerConfigurer.java b/oauth2-authorization-server/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationServerConfigurer.java
index bcfccb63..8186c4d3 100644
--- a/oauth2-authorization-server/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationServerConfigurer.java
+++ b/oauth2-authorization-server/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OAuth2AuthorizationServerConfigurer.java
@@ -427,11 +427,17 @@ public final class OAuth2AuthorizationServerConfigurer<B extends HttpSecurityBui
 
 	private static void validateProviderSettings(ProviderSettings providerSettings) {
 		if (providerSettings.getIssuer() != null) {
+			URI issuerUri;
 			try {
-				new URI(providerSettings.getIssuer()).toURL();
+				issuerUri = new URI(providerSettings.getIssuer());
+				issuerUri.toURL();
 			} catch (Exception ex) {
 				throw new IllegalArgumentException("issuer must be a valid URL", ex);
 			}
+			// rfc8414 https://datatracker.ietf.org/doc/html/rfc8414#section-2
+			if (issuerUri.getQuery() != null || issuerUri.getFragment() != null) {
+				throw new IllegalArgumentException("issuer cannot contain query or fragment component");
+			}
 		}
 	}
 
diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OidcTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OidcTests.java
index 791d0692..2f4b0b12 100644
--- a/oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OidcTests.java
+++ b/oauth2-authorization-server/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/server/authorization/OidcTests.java
@@ -210,6 +210,41 @@ public class OidcTests {
 		);
 	}
 
+	@Test
+	public void loadContextWhenIssuerWithQueryThenThrowException() {
+		assertThatThrownBy(
+				() -> this.spring.register(AuthorizationServerConfigurationWithInvalidQueryIssuerUrl.class).autowire()
+		);
+	}
+
+	@Test
+	public void loadContextWhenIssuerWithFragmentThenThrowException() {
+		assertThatThrownBy(
+				() -> this.spring.register(AuthorizationServerConfigurationWithInvalidFragmentIssuerUrl.class).autowire()
+		);
+	}
+
+	@Test
+	public void loadContextWhenIssuerWithQueryAndFragmentThenThrowException() {
+		assertThatThrownBy(
+				() -> this.spring.register(AuthorizationServerConfigurationWithInvalidQueryAndFragmentIssuerUrl.class).autowire()
+		);
+	}
+
+	@Test
+	public void loadContextWhenIssuerEndWithQuestionMarkCharacterThenThrowException() {
+		assertThatThrownBy(
+				() -> this.spring.register(AuthorizationServerConfigurationWithInvalidIssuerUrlEndWithQuestionMarkCharacter.class).autowire()
+		);
+	}
+
+	@Test
+	public void loadContextWhenIssuerEndWithNumberSignCharacterThenThrowException() {
+		assertThatThrownBy(
+				() -> this.spring.register(AuthorizationServerConfigurationWithInvalidIssuerUrlEndWithNumberSignCharacter.class).autowire()
+		);
+	}
+
 	@Test
 	public void requestWhenAuthenticationRequestThenTokenResponseIncludesIdToken() throws Exception {
 		this.spring.register(AuthorizationServerConfiguration.class).autowire();
@@ -459,4 +494,54 @@ public class OidcTests {
 		}
 	}
 
+	@EnableWebSecurity
+	@Import(OAuth2AuthorizationServerConfiguration.class)
+	static class AuthorizationServerConfigurationWithInvalidQueryIssuerUrl extends AuthorizationServerConfiguration {
+
+		@Bean
+		ProviderSettings providerSettings() {
+			return ProviderSettings.builder().issuer("https://localhost:9000?something=any").build();
+		}
+	}
+
+	@EnableWebSecurity
+	@Import(OAuth2AuthorizationServerConfiguration.class)
+	static class AuthorizationServerConfigurationWithInvalidFragmentIssuerUrl extends AuthorizationServerConfiguration {
+
+		@Bean
+		ProviderSettings providerSettings() {
+			return ProviderSettings.builder().issuer("https://localhost:9000#fragment").build();
+		}
+	}
+
+	@EnableWebSecurity
+	@Import(OAuth2AuthorizationServerConfiguration.class)
+	static class AuthorizationServerConfigurationWithInvalidQueryAndFragmentIssuerUrl extends AuthorizationServerConfiguration {
+
+		@Bean
+		ProviderSettings providerSettings() {
+			return ProviderSettings.builder().issuer("https://localhost:9000?something=any#fragment").build();
+		}
+	}
+
+	@EnableWebSecurity
+	@Import(OAuth2AuthorizationServerConfiguration.class)
+	static class AuthorizationServerConfigurationWithInvalidIssuerUrlEndWithQuestionMarkCharacter extends AuthorizationServerConfiguration {
+
+		@Bean
+		ProviderSettings providerSettings() {
+			return ProviderSettings.builder().issuer("https://localhost:9000?").build();
+		}
+	}
+
+	@EnableWebSecurity
+	@Import(OAuth2AuthorizationServerConfiguration.class)
+	static class AuthorizationServerConfigurationWithInvalidIssuerUrlEndWithNumberSignCharacter extends AuthorizationServerConfiguration {
+
+		@Bean
+		ProviderSettings providerSettings() {
+			return ProviderSettings.builder().issuer("https://localhost:9000/#").build();
+		}
+	}
+
 }