diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OidcBackChannelLogoutTokenValidator.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OidcBackChannelLogoutTokenValidator.java index e29bdaa6010..7a65826ab95 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OidcBackChannelLogoutTokenValidator.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OidcBackChannelLogoutTokenValidator.java @@ -30,7 +30,7 @@ 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.StringUtils; +import org.springframework.util.Assert; /** * A {@link OAuth2TokenValidator} that validates OIDC Logout Token claims in conformance @@ -58,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 @@ -78,9 +80,6 @@ else if (events.get(BACK_CHANNEL_LOGOUT_EVENT) == null) { if (issuer == null) { errors.add(invalidLogoutToken("iss claim must not be null")); } - else if (!StringUtils.hasText(this.issuer)) { - errors.add(invalidLogoutToken("Provider issuer must not be null")); - } else if (!this.issuer.equals(issuer)) { errors.add(invalidLogoutToken( "iss claim value must match `ClientRegistration#getProviderDetails#getIssuerUri`")); diff --git a/config/src/main/java/org/springframework/security/config/web/server/OidcBackChannelLogoutTokenValidator.java b/config/src/main/java/org/springframework/security/config/web/server/OidcBackChannelLogoutTokenValidator.java index 26ee970b40e..c1709e1ec77 100644 --- a/config/src/main/java/org/springframework/security/config/web/server/OidcBackChannelLogoutTokenValidator.java +++ b/config/src/main/java/org/springframework/security/config/web/server/OidcBackChannelLogoutTokenValidator.java @@ -30,7 +30,7 @@ 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.StringUtils; +import org.springframework.util.Assert; /** * A {@link OAuth2TokenValidator} that validates OIDC Logout Token claims in conformance @@ -58,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 @@ -78,9 +80,6 @@ else if (events.get(BACK_CHANNEL_LOGOUT_EVENT) == null) { if (issuer == null) { errors.add(invalidLogoutToken("iss claim must not be null")); } - else if (!StringUtils.hasText(this.issuer)) { - errors.add(invalidLogoutToken("Provider issuer must not be null")); - } else if (!this.issuer.equals(issuer)) { errors.add(invalidLogoutToken( "iss claim value must match `ClientRegistration#getProviderDetails#getIssuerUri`")); diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OidcLogoutConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OidcLogoutConfigurerTests.java index 70199a29476..541942212ff 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OidcLogoutConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OidcLogoutConfigurerTests.java @@ -92,6 +92,7 @@ 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; @@ -506,9 +507,8 @@ String idToken(String sessionId) { } private String getIssuerUri() { - String issuerUri = registration.getProviderDetails().getIssuerUri(); - if (StringUtils.hasText(issuerUri)) { - return issuerUri; + if (this.web == null) { + return TestClientRegistrations.clientRegistration().build().getProviderDetails().getIssuerUri(); } return this.web.url("/").toString(); } @@ -651,7 +651,7 @@ private String getContentAsString(MockHttpServletResponse response) { } @Test - void logoutWhenProviderIssuerMissingThenBadRequest() throws Exception { + void logoutWhenProviderIssuerMissingThenThrowIllegalArgumentException() throws Exception { this.spring.register(WebServerConfig.class, OidcProviderConfig.class, ProviderIssuerMissingConfig.class).autowire(); String registrationId = this.clientRegistration.getRegistrationId(); MockHttpSession session = login(); @@ -660,11 +660,11 @@ void logoutWhenProviderIssuerMissingThenBadRequest() throws Exception { .andReturn() .getResponse() .getContentAsString(); - this.mvc - .perform(post(this.web.url("/logout/connect/back-channel/" + registrationId).toString()) - .param("logout_token", logoutToken)) - .andExpect(status().isBadRequest()); - this.mvc.perform(get("/token/logout").session(session)).andExpect(status().isOk()); + assertThatIllegalArgumentException().isThrownBy(() -> { + this.mvc + .perform(post(this.web.url("/logout/connect/back-channel/" + registrationId).toString()) + .param("logout_token", logoutToken)); + }); } @Configuration @@ -676,7 +676,7 @@ static class ProviderIssuerMissingRegistrationConfig { @Bean ClientRegistration clientRegistration() { if (this.web == null) { - return TestClientRegistrations.clientRegistration().build(); + return TestClientRegistrations.clientRegistration().issuerUri(null).build(); } String issuer = this.web.url("/").toString(); return TestClientRegistrations.clientRegistration() diff --git a/config/src/test/java/org/springframework/security/config/web/server/OidcLogoutSpecTests.java b/config/src/test/java/org/springframework/security/config/web/server/OidcLogoutSpecTests.java index e4e4dffaa53..711481c2809 100644 --- a/config/src/test/java/org/springframework/security/config/web/server/OidcLogoutSpecTests.java +++ b/config/src/test/java/org/springframework/security/config/web/server/OidcLogoutSpecTests.java @@ -603,9 +603,8 @@ String idToken(String sessionId) { } private String getIssuerUri() { - String issuerUri = registration.getProviderDetails().getIssuerUri(); - if (StringUtils.hasText(issuerUri)) { - return issuerUri; + if (this.web == null) { + return TestClientRegistrations.clientRegistration().build().getProviderDetails().getIssuerUri(); } return this.web.url("/").toString(); } @@ -743,7 +742,7 @@ private MockResponse toMockResponse(FluxExchangeResult result) { } @Test - void logoutWhenProviderIssuerMissingThenBadRequest() { + void logoutWhenProviderIssuerMissingThen5xxServerError() { this.spring.register(WebServerConfig.class, OidcProviderConfig.class, ProviderIssuerMissingConfig.class).autowire(); String registrationId = this.clientRegistration.getRegistrationId(); String session = login(); @@ -761,7 +760,7 @@ void logoutWhenProviderIssuerMissingThenBadRequest() { .body(BodyInserters.fromFormData("logout_token", logoutToken)) .exchange() .expectStatus() - .isBadRequest(); + .is5xxServerError(); this.test.mutateWith(session(session)).get().uri("/token/logout").exchange().expectStatus().isOk(); } @@ -774,7 +773,7 @@ static class ProviderIssuerMissingRegistrationConfig { @Bean ClientRegistration clientRegistration() { if (this.web == null) { - return TestClientRegistrations.clientRegistration().build(); + return TestClientRegistrations.clientRegistration().issuerUri(null).build(); } String issuer = this.web.url("/").toString(); return TestClientRegistrations.clientRegistration()