Improve OidcBackChannelLogoutTokenValidator error when provider issuer is missing

Closes gh-15771

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -30,6 +30,7 @@ import org.springframework.security.oauth2.core.OAuth2ErrorCodes;
 import org.springframework.security.oauth2.core.OAuth2TokenValidator;
 import org.springframework.security.oauth2.core.OAuth2TokenValidatorResult;
 import org.springframework.security.oauth2.jwt.Jwt;
+import org.springframework.util.Assert;
 
 /**
  * A {@link OAuth2TokenValidator} that validates OIDC Logout Token claims in conformance
@@ -57,7 +58,9 @@ final class OidcBackChannelLogoutTokenValidator implements OAuth2TokenValidator<
 
 	OidcBackChannelLogoutTokenValidator(ClientRegistration clientRegistration) {
 		this.audience = clientRegistration.getClientId();
-		this.issuer = clientRegistration.getProviderDetails().getIssuerUri();
+		String issuer = clientRegistration.getProviderDetails().getIssuerUri();
+		Assert.hasText(issuer, "Provider issuer cannot be null");
+		this.issuer = issuer;
 	}
 
 	@Override

config/src/main/java/org/springframework/security/config/web/server/OidcBackChannelLogoutTokenValidator.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -30,6 +30,7 @@ import org.springframework.security.oauth2.core.OAuth2ErrorCodes;
 import org.springframework.security.oauth2.core.OAuth2TokenValidator;
 import org.springframework.security.oauth2.core.OAuth2TokenValidatorResult;
 import org.springframework.security.oauth2.jwt.Jwt;
+import org.springframework.util.Assert;
 
 /**
  * A {@link OAuth2TokenValidator} that validates OIDC Logout Token claims in conformance
@@ -57,7 +58,9 @@ final class OidcBackChannelLogoutTokenValidator implements OAuth2TokenValidator<
 
 	OidcBackChannelLogoutTokenValidator(ClientRegistration clientRegistration) {
 		this.audience = clientRegistration.getClientId();
-		this.issuer = clientRegistration.getProviderDetails().getIssuerUri();
+		String issuer = clientRegistration.getProviderDetails().getIssuerUri();
+		Assert.hasText(issuer, "Provider issuer cannot be null");
+		this.issuer = issuer;
 	}
 
 	@Override

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

@@ -85,12 +85,14 @@ import org.springframework.security.web.authentication.logout.LogoutHandler;
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.MvcResult;
 import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
+import org.springframework.util.StringUtils;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.servlet.config.annotation.EnableWebMvc;
 
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.hamcrest.Matchers.containsString;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.BDDMockito.willThrow;
@@ -448,6 +450,9 @@ public class OidcLogoutConfigurerTests {
 		@Autowired
 		ClientRegistration registration;
 
+		@Autowired(required = false)
+		MockWebServer web;
+
 		@Bean
 		@Order(0)
 		SecurityFilterChain authorizationServer(HttpSecurity http, ClientRegistration registration) throws Exception {
@@ -484,7 +489,7 @@ public class OidcLogoutConfigurerTests {
 			HttpSession session = request.getSession();
 			JwtEncoderParameters parameters = JwtEncoderParameters
 					.from(JwtClaimsSet.builder().id("id").subject(this.username)
-							.issuer(this.registration.getProviderDetails().getIssuerUri()).issuedAt(Instant.now())
+							.issuer(getIssuerUri()).issuedAt(Instant.now())
 							.expiresAt(Instant.now().plusSeconds(86400)).claim("scope", "openid").build());
 			String token = this.encoder.encode(parameters).getTokenValue();
 			return new OIDCTokens(idToken(session.getId()), new BearerAccessToken(token, 86400, new Scope("openid")), null)
@@ -492,7 +497,7 @@ public class OidcLogoutConfigurerTests {
 		}
 
 		String idToken(String sessionId) {
-			OidcIdToken token = TestOidcIdTokens.idToken().issuer(this.registration.getProviderDetails().getIssuerUri())
+			OidcIdToken token = TestOidcIdTokens.idToken().issuer(getIssuerUri())
 					.subject(this.username).expiresAt(Instant.now().plusSeconds(86400))
 					.audience(List.of(this.registration.getClientId())).nonce(this.nonce)
 					.claim(LogoutTokenClaimNames.SID, sessionId).build();
@@ -501,6 +506,13 @@ public class OidcLogoutConfigurerTests {
 			return this.encoder.encode(parameters).getTokenValue();
 		}
 
+		private String getIssuerUri() {
+			if (this.web == null) {
+				return TestClientRegistrations.clientRegistration().build().getProviderDetails().getIssuerUri();
+			}
+			return this.web.url("/").toString();
+		}
+
 		@GetMapping("/user")
 		Map<String, Object> userinfo() {
 			return Map.of("sub", this.username, "id", this.username);
@@ -638,4 +650,69 @@ public class OidcLogoutConfigurerTests {
 
 	}
 
+	@Test
+	void logoutWhenProviderIssuerMissingThenThrowIllegalArgumentException() throws Exception {
+		this.spring.register(WebServerConfig.class, OidcProviderConfig.class, ProviderIssuerMissingConfig.class).autowire();
+		String registrationId = this.clientRegistration.getRegistrationId();
+		MockHttpSession session = login();
+		String logoutToken = this.mvc.perform(get("/token/logout").session(session))
+				.andExpect(status().isOk())
+				.andReturn()
+				.getResponse()
+				.getContentAsString();
+		assertThatIllegalArgumentException().isThrownBy(() -> {
+			this.mvc
+					.perform(post(this.web.url("/logout/connect/back-channel/" + registrationId).toString())
+							.param("logout_token", logoutToken));
+		});
+	}
+
+	@Configuration
+	static class ProviderIssuerMissingRegistrationConfig {
+
+		@Autowired(required = false)
+		MockWebServer web;
+
+		@Bean
+		ClientRegistration clientRegistration() {
+			if (this.web == null) {
+				return TestClientRegistrations.clientRegistration().issuerUri(null).build();
+			}
+			String issuer = this.web.url("/").toString();
+			return TestClientRegistrations.clientRegistration()
+					.issuerUri(null)
+					.jwkSetUri(issuer + "jwks")
+					.tokenUri(issuer + "token")
+					.userInfoUri(issuer + "user")
+					.scope("openid")
+					.build();
+		}
+
+		@Bean
+		ClientRegistrationRepository clientRegistrationRepository(ClientRegistration clientRegistration) {
+			return new InMemoryClientRegistrationRepository(clientRegistration);
+		}
+
+	}
+
+	@Configuration
+	@EnableWebSecurity
+	@Import(ProviderIssuerMissingRegistrationConfig.class)
+	static class ProviderIssuerMissingConfig {
+
+		@Bean
+		@Order(1)
+		SecurityFilterChain filters(HttpSecurity http) throws Exception {
+			// @formatter:off
+			http
+					.authorizeHttpRequests((authorize) -> authorize.anyRequest().authenticated())
+					.oauth2Login(Customizer.withDefaults())
+					.oidcLogout((oidc) -> oidc.backChannel(Customizer.withDefaults()));
+			// @formatter:on
+
+			return http.build();
+		}
+
+	}
+
 }

config/src/test/java/org/springframework/security/config/web/server/OidcLogoutSpecTests.java

@@ -86,6 +86,7 @@ import org.springframework.security.web.server.util.matcher.ServerWebExchangeMat
 import org.springframework.test.web.reactive.server.FluxExchangeResult;
 import org.springframework.test.web.reactive.server.WebTestClient;
 import org.springframework.test.web.reactive.server.WebTestClientConfigurer;
+import org.springframework.util.StringUtils;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.RequestParam;
@@ -537,6 +538,9 @@ public class OidcLogoutSpecTests {
 		@Autowired
 		ClientRegistration registration;
 
+		@Autowired(required = false)
+		MockWebServer web;
+
 		static ServerWebExchangeMatcher or(String... patterns) {
 			List<ServerWebExchangeMatcher> matchers = new ArrayList<>();
 			for (String pattern : patterns) {
@@ -581,7 +585,7 @@ public class OidcLogoutSpecTests {
 		Map<String, Object> accessToken(WebSession session) {
 			JwtEncoderParameters parameters = JwtEncoderParameters
 					.from(JwtClaimsSet.builder().id("id").subject(this.username)
-							.issuer(this.registration.getProviderDetails().getIssuerUri()).issuedAt(Instant.now())
+							.issuer(getIssuerUri()).issuedAt(Instant.now())
 							.expiresAt(Instant.now().plusSeconds(86400)).claim("scope", "openid").build());
 			String token = this.encoder.encode(parameters).getTokenValue();
 			return new OIDCTokens(idToken(session.getId()), new BearerAccessToken(token, 86400, new Scope("openid")), null)
@@ -589,7 +593,7 @@ public class OidcLogoutSpecTests {
 		}
 
 		String idToken(String sessionId) {
-			OidcIdToken token = TestOidcIdTokens.idToken().issuer(this.registration.getProviderDetails().getIssuerUri())
+			OidcIdToken token = TestOidcIdTokens.idToken().issuer(getIssuerUri())
 					.subject(this.username).expiresAt(Instant.now().plusSeconds(86400))
 					.audience(List.of(this.registration.getClientId())).nonce(this.nonce)
 					.claim(LogoutTokenClaimNames.SID, sessionId).build();
@@ -598,6 +602,13 @@ public class OidcLogoutSpecTests {
 			return this.encoder.encode(parameters).getTokenValue();
 		}
 
+		private String getIssuerUri() {
+			if (this.web == null) {
+				return TestClientRegistrations.clientRegistration().build().getProviderDetails().getIssuerUri();
+			}
+			return this.web.url("/").toString();
+		}
+
 		@GetMapping("/user")
 		Map<String, Object> userinfo() {
 			return Map.of("sub", this.username, "id", this.username);
@@ -730,4 +741,75 @@ public class OidcLogoutSpecTests {
 
 	}
 
+	@Test
+	void logoutWhenProviderIssuerMissingThen5xxServerError() {
+		this.spring.register(WebServerConfig.class, OidcProviderConfig.class, ProviderIssuerMissingConfig.class).autowire();
+		String registrationId = this.clientRegistration.getRegistrationId();
+		String session = login();
+		String logoutToken = this.test.mutateWith(session(session))
+				.get()
+				.uri("/token/logout")
+				.exchange()
+				.expectStatus()
+				.isOk()
+				.returnResult(String.class)
+				.getResponseBody()
+				.blockFirst();
+		this.test.post()
+				.uri(this.web.url("/logout/connect/back-channel/" + registrationId).toString())
+				.body(BodyInserters.fromFormData("logout_token", logoutToken))
+				.exchange()
+				.expectStatus()
+				.is5xxServerError();
+		this.test.mutateWith(session(session)).get().uri("/token/logout").exchange().expectStatus().isOk();
+	}
+
+	@Configuration
+	static class ProviderIssuerMissingRegistrationConfig {
+
+		@Autowired(required = false)
+		MockWebServer web;
+
+		@Bean
+		ClientRegistration clientRegistration() {
+			if (this.web == null) {
+				return TestClientRegistrations.clientRegistration().issuerUri(null).build();
+			}
+			String issuer = this.web.url("/").toString();
+			return TestClientRegistrations.clientRegistration()
+					.issuerUri(null)
+					.jwkSetUri(issuer + "jwks")
+					.tokenUri(issuer + "token")
+					.userInfoUri(issuer + "user")
+					.scope("openid")
+					.build();
+		}
+
+		@Bean
+		ReactiveClientRegistrationRepository clientRegistrationRepository(ClientRegistration clientRegistration) {
+			return new InMemoryReactiveClientRegistrationRepository(clientRegistration);
+		}
+
+	}
+
+	@Configuration
+	@EnableWebFluxSecurity
+	@Import(ProviderIssuerMissingRegistrationConfig.class)
+	static class ProviderIssuerMissingConfig {
+
+		@Bean
+		@Order(1)
+		SecurityWebFilterChain filters(ServerHttpSecurity http) throws Exception {
+			// @formatter:off
+			http
+					.authorizeExchange((authorize) -> authorize.anyExchange().authenticated())
+					.oauth2Login(Customizer.withDefaults())
+					.oidcLogout((oidc) -> oidc.backChannel(Customizer.withDefaults()));
+			// @formatter:on
+
+			return http.build();
+		}
+
+	}
+
 }