From 27a893fbeea7ec1f8ebb85be9242f17b24223e72 Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Mon, 29 May 2023 11:04:59 -0400 Subject: [PATCH] Validate authorized principal instead of sub during logout Closes gh-1235 --- .../OidcLogoutAuthenticationProvider.java | 8 ++-- ...OidcLogoutAuthenticationProviderTests.java | 46 ++++++++++++++++++- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcLogoutAuthenticationProvider.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcLogoutAuthenticationProvider.java index 3fbd44d6..5323108e 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcLogoutAuthenticationProvider.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcLogoutAuthenticationProvider.java @@ -18,6 +18,7 @@ package org.springframework.security.oauth2.server.authorization.oidc.authentica import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; +import java.security.Principal; import java.util.Base64; import java.util.List; @@ -130,16 +131,17 @@ public final class OidcLogoutAuthenticationProvider implements AuthenticationPro // Validate user identity if (oidcLogoutAuthentication.isPrincipalAuthenticated()) { - Authentication userPrincipal = (Authentication) oidcLogoutAuthentication.getPrincipal(); + Authentication currentUserPrincipal = (Authentication) oidcLogoutAuthentication.getPrincipal(); + Authentication authorizedUserPrincipal = authorization.getAttribute(Principal.class.getName()); if (!StringUtils.hasText(idToken.getSubject()) || - !idToken.getSubject().equals(userPrincipal.getName())) { + !currentUserPrincipal.getName().equals(authorizedUserPrincipal.getName())) { throwError(OAuth2ErrorCodes.INVALID_TOKEN, IdTokenClaimNames.SUB); } // Check for active session if (StringUtils.hasText(oidcLogoutAuthentication.getSessionId())) { SessionInformation sessionInformation = findSessionInformation( - userPrincipal, oidcLogoutAuthentication.getSessionId()); + currentUserPrincipal, oidcLogoutAuthentication.getSessionId()); if (sessionInformation != null) { String sessionIdHash; try { diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcLogoutAuthenticationProviderTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcLogoutAuthenticationProviderTests.java index c1ab615e..63b29a24 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcLogoutAuthenticationProviderTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcLogoutAuthenticationProviderTests.java @@ -317,12 +317,11 @@ public class OidcLogoutAuthenticationProviderTests { } @Test - public void authenticateWhenInvalidSubThenThrowOAuth2AuthenticationException() { + public void authenticateWhenMissingSubThenThrowOAuth2AuthenticationException() { TestingAuthenticationToken principal = new TestingAuthenticationToken("principal", "credentials"); RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); OidcIdToken idToken = OidcIdToken.withTokenValue("id-token") .issuer("https://provider.com") - .subject("other-sub") .audience(Collections.singleton(registeredClient.getClientId())) .issuedAt(Instant.now().minusSeconds(60).truncatedTo(ChronoUnit.MILLIS)) .expiresAt(Instant.now().plusSeconds(60).truncatedTo(ChronoUnit.MILLIS)) @@ -355,6 +354,49 @@ public class OidcLogoutAuthenticationProviderTests { eq(authorization.getRegisteredClientId())); } + // gh-1235 + @Test + public void authenticateWhenInvalidPrincipalThenThrowOAuth2AuthenticationException() { + TestingAuthenticationToken principal = new TestingAuthenticationToken("principal", "credentials"); + RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); + OidcIdToken idToken = OidcIdToken.withTokenValue("id-token") + .issuer("https://provider.com") + .subject(principal.getName()) + .audience(Collections.singleton(registeredClient.getClientId())) + .issuedAt(Instant.now().minusSeconds(60).truncatedTo(ChronoUnit.MILLIS)) + .expiresAt(Instant.now().plusSeconds(60).truncatedTo(ChronoUnit.MILLIS)) + .build(); + OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient) + .principalName(principal.getName()) + .token(idToken, + (metadata) -> metadata.put(OAuth2Authorization.Token.CLAIMS_METADATA_NAME, idToken.getClaims())) + .build(); + when(this.authorizationService.findByToken(eq(idToken.getTokenValue()), eq(ID_TOKEN_TOKEN_TYPE))) + .thenReturn(authorization); + when(this.registeredClientRepository.findById(eq(authorization.getRegisteredClientId()))) + .thenReturn(registeredClient); + + principal.setAuthenticated(true); + + TestingAuthenticationToken otherPrincipal = new TestingAuthenticationToken("other-principal", "credentials"); + otherPrincipal.setAuthenticated(true); + + OidcLogoutAuthenticationToken authentication = new OidcLogoutAuthenticationToken( + idToken.getTokenValue(), otherPrincipal, "session-1", null, null, null); + + assertThatThrownBy(() -> this.authenticationProvider.authenticate(authentication)) + .isInstanceOf(OAuth2AuthenticationException.class) + .extracting(ex -> ((OAuth2AuthenticationException) ex).getError()) + .satisfies(error -> { + assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_TOKEN); + assertThat(error.getDescription()).contains("sub"); + }); + verify(this.authorizationService).findByToken( + eq(authentication.getIdTokenHint()), eq(ID_TOKEN_TOKEN_TYPE)); + verify(this.registeredClientRepository).findById( + eq(authorization.getRegisteredClientId())); + } + @Test public void authenticateWhenMissingSidThenThrowOAuth2AuthenticationException() { TestingAuthenticationToken principal = new TestingAuthenticationToken("principal", "credentials");