Skip to content

Commit 29331a0

Browse files
jzheauxjzheaux
committed
Merge branch '6.3.x'
2 parents 8b97fdd + 746464e commit 29331a0

File tree

4 files changed

+171
-8
lines changed

4 files changed

+171
-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: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@
9393
import org.springframework.web.servlet.config.annotation.EnableWebMvc;
9494

9595
import static org.assertj.core.api.Assertions.assertThat;
96+
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
9697
import static org.hamcrest.Matchers.containsString;
9798
import static org.mockito.ArgumentMatchers.any;
9899
import static org.mockito.BDDMockito.willThrow;
@@ -295,6 +296,22 @@ void logoutWhenCustomComponentsThenUses() throws Exception {
295296
verify(sessionRegistry).removeSessionInformation(any(OidcLogoutToken.class));
296297
}
297298

299+
@Test
300+
void logoutWhenProviderIssuerMissingThenThrowIllegalArgumentException() throws Exception {
301+
this.spring.register(WebServerConfig.class, OidcProviderConfig.class, ProviderIssuerMissingConfig.class)
302+
.autowire();
303+
String registrationId = this.clientRegistration.getRegistrationId();
304+
MockHttpSession session = login();
305+
String logoutToken = this.mvc.perform(get("/token/logout").session(session))
306+
.andExpect(status().isOk())
307+
.andReturn()
308+
.getResponse()
309+
.getContentAsString();
310+
assertThatIllegalArgumentException().isThrownBy(
311+
() -> this.mvc.perform(post(this.web.url("/logout/connect/back-channel/" + registrationId).toString())
312+
.param("logout_token", logoutToken)));
313+
}
314+
298315
private MockHttpSession login() throws Exception {
299316
MockMvcDispatcher dispatcher = (MockMvcDispatcher) this.web.getDispatcher();
300317
this.mvc.perform(get("/token/logout")).andExpect(status().isUnauthorized());
@@ -523,6 +540,54 @@ LogoutHandler logoutHandler() {
523540

524541
}
525542

543+
@Configuration
544+
static class ProviderIssuerMissingRegistrationConfig {
545+
546+
@Autowired(required = false)
547+
MockWebServer web;
548+
549+
@Bean
550+
ClientRegistration clientRegistration() {
551+
if (this.web == null) {
552+
return TestClientRegistrations.clientRegistration().issuerUri(null).build();
553+
}
554+
String issuer = this.web.url("/").toString();
555+
return TestClientRegistrations.clientRegistration()
556+
.issuerUri(null)
557+
.jwkSetUri(issuer + "jwks")
558+
.tokenUri(issuer + "token")
559+
.userInfoUri(issuer + "user")
560+
.scope("openid")
561+
.build();
562+
}
563+
564+
@Bean
565+
ClientRegistrationRepository clientRegistrationRepository(ClientRegistration clientRegistration) {
566+
return new InMemoryClientRegistrationRepository(clientRegistration);
567+
}
568+
569+
}
570+
571+
@Configuration
572+
@EnableWebSecurity
573+
@Import(ProviderIssuerMissingRegistrationConfig.class)
574+
static class ProviderIssuerMissingConfig {
575+
576+
@Bean
577+
@Order(1)
578+
SecurityFilterChain filters(HttpSecurity http) throws Exception {
579+
// @formatter:off
580+
http
581+
.authorizeHttpRequests((authorize) -> authorize.anyRequest().authenticated())
582+
.oauth2Login(Customizer.withDefaults())
583+
.oidcLogout((oidc) -> oidc.backChannel(Customizer.withDefaults()));
584+
// @formatter:on
585+
586+
return http.build();
587+
}
588+
589+
}
590+
526591
@Configuration
527592
@EnableWebSecurity
528593
@EnableWebMvc
@@ -561,6 +626,9 @@ private static JWKSource<SecurityContext> jwks(RSAKey key) {
561626
@Autowired
562627
ClientRegistration registration;
563628

629+
@Autowired(required = false)
630+
MockWebServer web;
631+
564632
@Bean
565633
@Order(0)
566634
SecurityFilterChain authorizationServer(HttpSecurity http, ClientRegistration registration) throws Exception {
@@ -597,15 +665,15 @@ Map<String, Object> accessToken(HttpServletRequest request) {
597665
HttpSession session = request.getSession();
598666
JwtEncoderParameters parameters = JwtEncoderParameters
599667
.from(JwtClaimsSet.builder().id("id").subject(this.username)
600-
.issuer(this.registration.getProviderDetails().getIssuerUri()).issuedAt(Instant.now())
668+
.issuer(getIssuerUri()).issuedAt(Instant.now())
601669
.expiresAt(Instant.now().plusSeconds(86400)).claim("scope", "openid").build());
602670
String token = this.encoder.encode(parameters).getTokenValue();
603671
return new OIDCTokens(idToken(session.getId()), new BearerAccessToken(token, 86400, new Scope("openid")), null)
604672
.toJSONObject();
605673
}
606674

607675
String idToken(String sessionId) {
608-
OidcIdToken token = TestOidcIdTokens.idToken().issuer(this.registration.getProviderDetails().getIssuerUri())
676+
OidcIdToken token = TestOidcIdTokens.idToken().issuer(getIssuerUri())
609677
.subject(this.username).expiresAt(Instant.now().plusSeconds(86400))
610678
.audience(List.of(this.registration.getClientId())).nonce(this.nonce)
611679
.claim(LogoutTokenClaimNames.SID, sessionId).build();
@@ -614,6 +682,13 @@ String idToken(String sessionId) {
614682
return this.encoder.encode(parameters).getTokenValue();
615683
}
616684

685+
private String getIssuerUri() {
686+
if (this.web == null) {
687+
return TestClientRegistrations.clientRegistration().build().getProviderDetails().getIssuerUri();
688+
}
689+
return this.web.url("/").toString();
690+
}
691+
617692
@GetMapping("/user")
618693
Map<String, Object> userinfo() {
619694
return Map.of("sub", this.username, "id", this.username);

‎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
@@ -371,6 +371,30 @@ void logoutWhenCustomComponentsThenUses() {
371371
verify(sessionRegistry, atLeastOnce()).removeSessionInformation(any(OidcLogoutToken.class));
372372
}
373373

374+
@Test
375+
void logoutWhenProviderIssuerMissingThen5xxServerError() {
376+
this.spring.register(WebServerConfig.class, OidcProviderConfig.class, ProviderIssuerMissingConfig.class)
377+
.autowire();
378+
String registrationId = this.clientRegistration.getRegistrationId();
379+
String session = login();
380+
String logoutToken = this.test.mutateWith(session(session))
381+
.get()
382+
.uri("/token/logout")
383+
.exchange()
384+
.expectStatus()
385+
.isOk()
386+
.returnResult(String.class)
387+
.getResponseBody()
388+
.blockFirst();
389+
this.test.post()
390+
.uri(this.web.url("/logout/connect/back-channel/" + registrationId).toString())
391+
.body(BodyInserters.fromFormData("logout_token", logoutToken))
392+
.exchange()
393+
.expectStatus()
394+
.is5xxServerError();
395+
this.test.mutateWith(session(session)).get().uri("/token/logout").exchange().expectStatus().isOk();
396+
}
397+
374398
private String login() {
375399
this.test.get().uri("/token/logout").exchange().expectStatus().isUnauthorized();
376400
String registrationId = this.clientRegistration.getRegistrationId();
@@ -624,6 +648,54 @@ ServerLogoutHandler logoutHandler() {
624648

625649
}
626650

651+
@Configuration
652+
static class ProviderIssuerMissingRegistrationConfig {
653+
654+
@Autowired(required = false)
655+
MockWebServer web;
656+
657+
@Bean
658+
ClientRegistration clientRegistration() {
659+
if (this.web == null) {
660+
return TestClientRegistrations.clientRegistration().issuerUri(null).build();
661+
}
662+
String issuer = this.web.url("/").toString();
663+
return TestClientRegistrations.clientRegistration()
664+
.issuerUri(null)
665+
.jwkSetUri(issuer + "jwks")
666+
.tokenUri(issuer + "token")
667+
.userInfoUri(issuer + "user")
668+
.scope("openid")
669+
.build();
670+
}
671+
672+
@Bean
673+
ReactiveClientRegistrationRepository clientRegistrationRepository(ClientRegistration clientRegistration) {
674+
return new InMemoryReactiveClientRegistrationRepository(clientRegistration);
675+
}
676+
677+
}
678+
679+
@Configuration
680+
@EnableWebFluxSecurity
681+
@Import(ProviderIssuerMissingRegistrationConfig.class)
682+
static class ProviderIssuerMissingConfig {
683+
684+
@Bean
685+
@Order(1)
686+
SecurityWebFilterChain filters(ServerHttpSecurity http) throws Exception {
687+
// @formatter:off
688+
http
689+
.authorizeExchange((authorize) -> authorize.anyExchange().authenticated())
690+
.oauth2Login(Customizer.withDefaults())
691+
.oidcLogout((oidc) -> oidc.backChannel(Customizer.withDefaults()));
692+
// @formatter:on
693+
694+
return http.build();
695+
}
696+
697+
}
698+
627699
@Configuration
628700
@EnableWebFluxSecurity
629701
@EnableWebFlux
@@ -662,6 +734,9 @@ private static JWKSource<SecurityContext> jwks(RSAKey key) {
662734
@Autowired
663735
ClientRegistration registration;
664736

737+
@Autowired(required = false)
738+
MockWebServer web;
739+
665740
static ServerWebExchangeMatcher or(String... patterns) {
666741
List<ServerWebExchangeMatcher> matchers = new ArrayList<>();
667742
for (String pattern : patterns) {
@@ -706,15 +781,15 @@ String nonce(@RequestParam("nonce") String nonce, @RequestParam("state") String
706781
Map<String, Object> accessToken(WebSession session) {
707782
JwtEncoderParameters parameters = JwtEncoderParameters
708783
.from(JwtClaimsSet.builder().id("id").subject(this.username)
709-
.issuer(this.registration.getProviderDetails().getIssuerUri()).issuedAt(Instant.now())
784+
.issuer(getIssuerUri()).issuedAt(Instant.now())
710785
.expiresAt(Instant.now().plusSeconds(86400)).claim("scope", "openid").build());
711786
String token = this.encoder.encode(parameters).getTokenValue();
712787
return new OIDCTokens(idToken(session.getId()), new BearerAccessToken(token, 86400, new Scope("openid")), null)
713788
.toJSONObject();
714789
}
715790

716791
String idToken(String sessionId) {
717-
OidcIdToken token = TestOidcIdTokens.idToken().issuer(this.registration.getProviderDetails().getIssuerUri())
792+
OidcIdToken token = TestOidcIdTokens.idToken().issuer(getIssuerUri())
718793
.subject(this.username).expiresAt(Instant.now().plusSeconds(86400))
719794
.audience(List.of(this.registration.getClientId())).nonce(this.nonce)
720795
.claim(LogoutTokenClaimNames.SID, sessionId).build();
@@ -723,6 +798,13 @@ String idToken(String sessionId) {
723798
return this.encoder.encode(parameters).getTokenValue();
724799
}
725800

801+
private String getIssuerUri() {
802+
if (this.web == null) {
803+
return TestClientRegistrations.clientRegistration().build().getProviderDetails().getIssuerUri();
804+
}
805+
return this.web.url("/").toString();
806+
}
807+
726808
@GetMapping("/user")
727809
Map<String, Object> userinfo() {
728810
return Map.of("sub", this.username, "id", this.username);

0 commit comments

Comments
 (0)