Skip to content

Commit 690e012

Browse files
chao.wangjzheaux
chao.wang
authored and
jzheaux
committed
Improve OidcBackChannelLogoutTokenValidator error when provider issuer is missing
Closes gh-15771
1 parent f7b85ed commit 690e012

File tree

4 files changed

+173
-8
lines changed

4 files changed

+173
-8
lines changed

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -30,6 +30,7 @@
3030
import org.springframework.security.oauth2.core.OAuth2TokenValidator;
3131
import org.springframework.security.oauth2.core.OAuth2TokenValidatorResult;
3232
import org.springframework.security.oauth2.jwt.Jwt;
33+
import org.springframework.util.Assert;
3334

3435
/**
3536
* A {@link OAuth2TokenValidator} that validates OIDC Logout Token claims in conformance
@@ -57,7 +58,9 @@ final class OidcBackChannelLogoutTokenValidator implements OAuth2TokenValidator<
5758

5859
OidcBackChannelLogoutTokenValidator(ClientRegistration clientRegistration) {
5960
this.audience = clientRegistration.getClientId();
60-
this.issuer = clientRegistration.getProviderDetails().getIssuerUri();
61+
String issuer = clientRegistration.getProviderDetails().getIssuerUri();
62+
Assert.hasText(issuer, "Provider issuer cannot be null");
63+
this.issuer = issuer;
6164
}
6265

6366
@Override

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -30,6 +30,7 @@
3030
import org.springframework.security.oauth2.core.OAuth2TokenValidator;
3131
import org.springframework.security.oauth2.core.OAuth2TokenValidatorResult;
3232
import org.springframework.security.oauth2.jwt.Jwt;
33+
import org.springframework.util.Assert;
3334

3435
/**
3536
* A {@link OAuth2TokenValidator} that validates OIDC Logout Token claims in conformance
@@ -57,7 +58,9 @@ final class OidcBackChannelLogoutTokenValidator implements OAuth2TokenValidator<
5758

5859
OidcBackChannelLogoutTokenValidator(ClientRegistration clientRegistration) {
5960
this.audience = clientRegistration.getClientId();
60-
this.issuer = clientRegistration.getProviderDetails().getIssuerUri();
61+
String issuer = clientRegistration.getProviderDetails().getIssuerUri();
62+
Assert.hasText(issuer, "Provider issuer cannot be null");
63+
this.issuer = issuer;
6164
}
6265

6366
@Override

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

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,14 @@
8585
import org.springframework.test.web.servlet.MockMvc;
8686
import org.springframework.test.web.servlet.MvcResult;
8787
import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
88+
import org.springframework.util.StringUtils;
8889
import org.springframework.web.bind.annotation.GetMapping;
8990
import org.springframework.web.bind.annotation.PostMapping;
9091
import org.springframework.web.bind.annotation.RequestParam;
9192
import org.springframework.web.bind.annotation.RestController;
9293
import org.springframework.web.servlet.config.annotation.EnableWebMvc;
9394

95+
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
9496
import static org.hamcrest.Matchers.containsString;
9597
import static org.mockito.ArgumentMatchers.any;
9698
import static org.mockito.BDDMockito.willThrow;
@@ -448,6 +450,9 @@ private static JWKSource<SecurityContext> jwks(RSAKey key) {
448450
@Autowired
449451
ClientRegistration registration;
450452

453+
@Autowired(required = false)
454+
MockWebServer web;
455+
451456
@Bean
452457
@Order(0)
453458
SecurityFilterChain authorizationServer(HttpSecurity http, ClientRegistration registration) throws Exception {
@@ -484,15 +489,15 @@ Map<String, Object> accessToken(HttpServletRequest request) {
484489
HttpSession session = request.getSession();
485490
JwtEncoderParameters parameters = JwtEncoderParameters
486491
.from(JwtClaimsSet.builder().id("id").subject(this.username)
487-
.issuer(this.registration.getProviderDetails().getIssuerUri()).issuedAt(Instant.now())
492+
.issuer(getIssuerUri()).issuedAt(Instant.now())
488493
.expiresAt(Instant.now().plusSeconds(86400)).claim("scope", "openid").build());
489494
String token = this.encoder.encode(parameters).getTokenValue();
490495
return new OIDCTokens(idToken(session.getId()), new BearerAccessToken(token, 86400, new Scope("openid")), null)
491496
.toJSONObject();
492497
}
493498

494499
String idToken(String sessionId) {
495-
OidcIdToken token = TestOidcIdTokens.idToken().issuer(this.registration.getProviderDetails().getIssuerUri())
500+
OidcIdToken token = TestOidcIdTokens.idToken().issuer(getIssuerUri())
496501
.subject(this.username).expiresAt(Instant.now().plusSeconds(86400))
497502
.audience(List.of(this.registration.getClientId())).nonce(this.nonce)
498503
.claim(LogoutTokenClaimNames.SID, sessionId).build();
@@ -501,6 +506,13 @@ String idToken(String sessionId) {
501506
return this.encoder.encode(parameters).getTokenValue();
502507
}
503508

509+
private String getIssuerUri() {
510+
if (this.web == null) {
511+
return TestClientRegistrations.clientRegistration().build().getProviderDetails().getIssuerUri();
512+
}
513+
return this.web.url("/").toString();
514+
}
515+
504516
@GetMapping("/user")
505517
Map<String, Object> userinfo() {
506518
return Map.of("sub", this.username, "id", this.username);
@@ -638,4 +650,69 @@ private String getContentAsString(MockHttpServletResponse response) {
638650

639651
}
640652

653+
@Test
654+
void logoutWhenProviderIssuerMissingThenThrowIllegalArgumentException() throws Exception {
655+
this.spring.register(WebServerConfig.class, OidcProviderConfig.class, ProviderIssuerMissingConfig.class).autowire();
656+
String registrationId = this.clientRegistration.getRegistrationId();
657+
MockHttpSession session = login();
658+
String logoutToken = this.mvc.perform(get("/token/logout").session(session))
659+
.andExpect(status().isOk())
660+
.andReturn()
661+
.getResponse()
662+
.getContentAsString();
663+
assertThatIllegalArgumentException().isThrownBy(() -> {
664+
this.mvc
665+
.perform(post(this.web.url("/logout/connect/back-channel/" + registrationId).toString())
666+
.param("logout_token", logoutToken));
667+
});
668+
}
669+
670+
@Configuration
671+
static class ProviderIssuerMissingRegistrationConfig {
672+
673+
@Autowired(required = false)
674+
MockWebServer web;
675+
676+
@Bean
677+
ClientRegistration clientRegistration() {
678+
if (this.web == null) {
679+
return TestClientRegistrations.clientRegistration().issuerUri(null).build();
680+
}
681+
String issuer = this.web.url("/").toString();
682+
return TestClientRegistrations.clientRegistration()
683+
.issuerUri(null)
684+
.jwkSetUri(issuer + "jwks")
685+
.tokenUri(issuer + "token")
686+
.userInfoUri(issuer + "user")
687+
.scope("openid")
688+
.build();
689+
}
690+
691+
@Bean
692+
ClientRegistrationRepository clientRegistrationRepository(ClientRegistration clientRegistration) {
693+
return new InMemoryClientRegistrationRepository(clientRegistration);
694+
}
695+
696+
}
697+
698+
@Configuration
699+
@EnableWebSecurity
700+
@Import(ProviderIssuerMissingRegistrationConfig.class)
701+
static class ProviderIssuerMissingConfig {
702+
703+
@Bean
704+
@Order(1)
705+
SecurityFilterChain filters(HttpSecurity http) throws Exception {
706+
// @formatter:off
707+
http
708+
.authorizeHttpRequests((authorize) -> authorize.anyRequest().authenticated())
709+
.oauth2Login(Customizer.withDefaults())
710+
.oidcLogout((oidc) -> oidc.backChannel(Customizer.withDefaults()));
711+
// @formatter:on
712+
713+
return http.build();
714+
}
715+
716+
}
717+
641718
}

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

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@
8686
import org.springframework.test.web.reactive.server.FluxExchangeResult;
8787
import org.springframework.test.web.reactive.server.WebTestClient;
8888
import org.springframework.test.web.reactive.server.WebTestClientConfigurer;
89+
import org.springframework.util.StringUtils;
8990
import org.springframework.web.bind.annotation.GetMapping;
9091
import org.springframework.web.bind.annotation.PostMapping;
9192
import org.springframework.web.bind.annotation.RequestParam;
@@ -537,6 +538,9 @@ private static JWKSource<SecurityContext> jwks(RSAKey key) {
537538
@Autowired
538539
ClientRegistration registration;
539540

541+
@Autowired(required = false)
542+
MockWebServer web;
543+
540544
static ServerWebExchangeMatcher or(String... patterns) {
541545
List<ServerWebExchangeMatcher> matchers = new ArrayList<>();
542546
for (String pattern : patterns) {
@@ -581,15 +585,15 @@ String nonce(@RequestParam("nonce") String nonce, @RequestParam("state") String
581585
Map<String, Object> accessToken(WebSession session) {
582586
JwtEncoderParameters parameters = JwtEncoderParameters
583587
.from(JwtClaimsSet.builder().id("id").subject(this.username)
584-
.issuer(this.registration.getProviderDetails().getIssuerUri()).issuedAt(Instant.now())
588+
.issuer(getIssuerUri()).issuedAt(Instant.now())
585589
.expiresAt(Instant.now().plusSeconds(86400)).claim("scope", "openid").build());
586590
String token = this.encoder.encode(parameters).getTokenValue();
587591
return new OIDCTokens(idToken(session.getId()), new BearerAccessToken(token, 86400, new Scope("openid")), null)
588592
.toJSONObject();
589593
}
590594

591595
String idToken(String sessionId) {
592-
OidcIdToken token = TestOidcIdTokens.idToken().issuer(this.registration.getProviderDetails().getIssuerUri())
596+
OidcIdToken token = TestOidcIdTokens.idToken().issuer(getIssuerUri())
593597
.subject(this.username).expiresAt(Instant.now().plusSeconds(86400))
594598
.audience(List.of(this.registration.getClientId())).nonce(this.nonce)
595599
.claim(LogoutTokenClaimNames.SID, sessionId).build();
@@ -598,6 +602,13 @@ String idToken(String sessionId) {
598602
return this.encoder.encode(parameters).getTokenValue();
599603
}
600604

605+
private String getIssuerUri() {
606+
if (this.web == null) {
607+
return TestClientRegistrations.clientRegistration().build().getProviderDetails().getIssuerUri();
608+
}
609+
return this.web.url("/").toString();
610+
}
611+
601612
@GetMapping("/user")
602613
Map<String, Object> userinfo() {
603614
return Map.of("sub", this.username, "id", this.username);
@@ -730,4 +741,75 @@ private MockResponse toMockResponse(FluxExchangeResult<String> result) {
730741

731742
}
732743

744+
@Test
745+
void logoutWhenProviderIssuerMissingThen5xxServerError() {
746+
this.spring.register(WebServerConfig.class, OidcProviderConfig.class, ProviderIssuerMissingConfig.class).autowire();
747+
String registrationId = this.clientRegistration.getRegistrationId();
748+
String session = login();
749+
String logoutToken = this.test.mutateWith(session(session))
750+
.get()
751+
.uri("/token/logout")
752+
.exchange()
753+
.expectStatus()
754+
.isOk()
755+
.returnResult(String.class)
756+
.getResponseBody()
757+
.blockFirst();
758+
this.test.post()
759+
.uri(this.web.url("/logout/connect/back-channel/" + registrationId).toString())
760+
.body(BodyInserters.fromFormData("logout_token", logoutToken))
761+
.exchange()
762+
.expectStatus()
763+
.is5xxServerError();
764+
this.test.mutateWith(session(session)).get().uri("/token/logout").exchange().expectStatus().isOk();
765+
}
766+
767+
@Configuration
768+
static class ProviderIssuerMissingRegistrationConfig {
769+
770+
@Autowired(required = false)
771+
MockWebServer web;
772+
773+
@Bean
774+
ClientRegistration clientRegistration() {
775+
if (this.web == null) {
776+
return TestClientRegistrations.clientRegistration().issuerUri(null).build();
777+
}
778+
String issuer = this.web.url("/").toString();
779+
return TestClientRegistrations.clientRegistration()
780+
.issuerUri(null)
781+
.jwkSetUri(issuer + "jwks")
782+
.tokenUri(issuer + "token")
783+
.userInfoUri(issuer + "user")
784+
.scope("openid")
785+
.build();
786+
}
787+
788+
@Bean
789+
ReactiveClientRegistrationRepository clientRegistrationRepository(ClientRegistration clientRegistration) {
790+
return new InMemoryReactiveClientRegistrationRepository(clientRegistration);
791+
}
792+
793+
}
794+
795+
@Configuration
796+
@EnableWebFluxSecurity
797+
@Import(ProviderIssuerMissingRegistrationConfig.class)
798+
static class ProviderIssuerMissingConfig {
799+
800+
@Bean
801+
@Order(1)
802+
SecurityWebFilterChain filters(ServerHttpSecurity http) throws Exception {
803+
// @formatter:off
804+
http
805+
.authorizeExchange((authorize) -> authorize.anyExchange().authenticated())
806+
.oauth2Login(Customizer.withDefaults())
807+
.oidcLogout((oidc) -> oidc.backChannel(Customizer.withDefaults()));
808+
// @formatter:on
809+
810+
return http.build();
811+
}
812+
813+
}
814+
733815
}

0 commit comments

Comments
 (0)