Skip to content

Commit

Permalink
Improve OidcBackChannelLogoutTokenValidator error when provider issue…
Browse files Browse the repository at this point in the history 
…r is missing

Closes gh-15771
  • Loading branch information
@jzheaux
chao.wang authored and jzheaux committed Sep 23, 2024
1 parent 95cee40 commit 9289f22
Show file tree
Hide file tree
Showing 4 changed files with 173 additions and 8 deletions.
7 changes: 5 additions & 2 deletions .../config/annotation/web/configurers/oauth2/client/OidcBackChannelLogoutTokenValidator.java
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -30,6 +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.Assert;

/**
* A {@link OAuth2TokenValidator} that validates OIDC Logout Token claims in conformance
Expand Down Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions ...a/org/springframework/security/config/web/server/OidcBackChannelLogoutTokenValidator.java
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -30,6 +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.Assert;

/**
* A {@link OAuth2TokenValidator} that validates OIDC Logout Token claims in conformance
Expand Down Expand Up @@ -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
Expand Down
81 changes: 79 additions & 2 deletions ...k/security/config/annotation/web/configurers/oauth2/client/OidcLogoutConfigurerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,14 @@
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;
Expand Down Expand Up @@ -448,6 +450,9 @@ private static JWKSource<SecurityContext> jwks(RSAKey key) {
@Autowired
ClientRegistration registration;

@Autowired(required = false)
MockWebServer web;

@Bean
@Order(0)
SecurityFilterChain authorizationServer(HttpSecurity http, ClientRegistration registration) throws Exception {
Expand Down Expand Up @@ -484,15 +489,15 @@ Map<String, Object> accessToken(HttpServletRequest request) {
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)
.toJSONObject();
}

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();
Expand All @@ -501,6 +506,13 @@ String idToken(String sessionId) {
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);
Expand Down Expand Up @@ -638,4 +650,69 @@ private String getContentAsString(MockHttpServletResponse response) {

}

@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();
}

}

}
86 changes: 84 additions & 2 deletions config/src/test/java/org/springframework/security/config/web/server/OidcLogoutSpecTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
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;
Expand Down Expand Up @@ -537,6 +538,9 @@ private static JWKSource<SecurityContext> jwks(RSAKey key) {
@Autowired
ClientRegistration registration;

@Autowired(required = false)
MockWebServer web;

static ServerWebExchangeMatcher or(String... patterns) {
List<ServerWebExchangeMatcher> matchers = new ArrayList<>();
for (String pattern : patterns) {
Expand Down Expand Up @@ -581,15 +585,15 @@ String nonce(@RequestParam("nonce") String nonce, @RequestParam("state") String
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)
.toJSONObject();
}

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();
Expand All @@ -598,6 +602,13 @@ String idToken(String sessionId) {
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);
Expand Down Expand Up @@ -730,4 +741,75 @@ private MockResponse toMockResponse(FluxExchangeResult<String> result) {

}

@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();
}

}

}

0 comments on commit 9289f22

Please sign in to comment.