Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OidcBackChannelLogoutTokenValidator should not construct when missing OIDC Provider Issuer #15824

Merged
merged 2 commits into from
Sep 30, 2024
Merged

OidcBackChannelLogoutTokenValidator should not construct when missing OIDC Provider Issuer #15824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9289f22
Improve OidcBackChannelLogoutTokenValidator error when provider issue…
Sep 18, 2024
93fd363
Fix Formatting
jzheaux Sep 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions .../config/annotation/web/configurers/oauth2/client/OidcBackChannelLogoutTokenValidator.java
View file
Open in desktop
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
Open in desktop
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
79 changes: 77 additions & 2 deletions ...k/security/config/annotation/web/configurers/oauth2/client/OidcLogoutConfigurerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,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;
Expand Down Expand Up @@ -259,6 +260,22 @@ void logoutWhenCustomComponentsThenUses() throws Exception {
verify(sessionRegistry).removeSessionInformation(any(OidcLogoutToken.class));
}

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

private MockHttpSession login() throws Exception {
MockMvcDispatcher dispatcher = (MockMvcDispatcher) this.web.getDispatcher();
this.mvc.perform(get("/token/logout")).andExpect(status().isUnauthorized());
Expand Down Expand Up @@ -410,6 +427,54 @@ LogoutHandler logoutHandler() {

}

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

}

@Configuration
@EnableWebSecurity
@EnableWebMvc
Expand Down Expand Up @@ -448,6 +513,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 +552,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 +569,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
86 changes: 84 additions & 2 deletions config/src/test/java/org/springframework/security/config/web/server/OidcLogoutSpecTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,30 @@ void logoutWhenCustomComponentsThenUses() {
verify(sessionRegistry, atLeastOnce()).removeSessionInformation(any(OidcLogoutToken.class));
}

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

private String login() {
this.test.get().uri("/token/logout").exchange().expectStatus().isUnauthorized();
String registrationId = this.clientRegistration.getRegistrationId();
Expand Down Expand Up @@ -499,6 +523,54 @@ ServerLogoutHandler logoutHandler() {

}

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

}

@Configuration
@EnableWebFluxSecurity
@EnableWebFlux
Expand Down Expand Up @@ -537,6 +609,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 +656,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 +673,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