Skip to content

Commit 27a893f

Browse files
committed
Validate authorized principal instead of sub during logout
Closes gh-1235
1 parent a731811 commit 27a893f

File tree

2 files changed

+49
-5
lines changed

2 files changed

+49
-5
lines changed

oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcLogoutAuthenticationProvider.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.nio.charset.StandardCharsets;
1919
import java.security.MessageDigest;
2020
import java.security.NoSuchAlgorithmException;
21+
import java.security.Principal;
2122
import java.util.Base64;
2223
import java.util.List;
2324

@@ -130,16 +131,17 @@ public Authentication authenticate(Authentication authentication) throws Authent
130131

131132
// Validate user identity
132133
if (oidcLogoutAuthentication.isPrincipalAuthenticated()) {
133-
Authentication userPrincipal = (Authentication) oidcLogoutAuthentication.getPrincipal();
134+
Authentication currentUserPrincipal = (Authentication) oidcLogoutAuthentication.getPrincipal();
135+
Authentication authorizedUserPrincipal = authorization.getAttribute(Principal.class.getName());
134136
if (!StringUtils.hasText(idToken.getSubject()) ||
135-
!idToken.getSubject().equals(userPrincipal.getName())) {
137+
!currentUserPrincipal.getName().equals(authorizedUserPrincipal.getName())) {
136138
throwError(OAuth2ErrorCodes.INVALID_TOKEN, IdTokenClaimNames.SUB);
137139
}
138140

139141
// Check for active session
140142
if (StringUtils.hasText(oidcLogoutAuthentication.getSessionId())) {
141143
SessionInformation sessionInformation = findSessionInformation(
142-
userPrincipal, oidcLogoutAuthentication.getSessionId());
144+
currentUserPrincipal, oidcLogoutAuthentication.getSessionId());
143145
if (sessionInformation != null) {
144146
String sessionIdHash;
145147
try {

oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcLogoutAuthenticationProviderTests.java

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -317,12 +317,11 @@ public void authenticateWhenInvalidPostLogoutRedirectUriThenThrowOAuth2Authentic
317317
}
318318

319319
@Test
320-
public void authenticateWhenInvalidSubThenThrowOAuth2AuthenticationException() {
320+
public void authenticateWhenMissingSubThenThrowOAuth2AuthenticationException() {
321321
TestingAuthenticationToken principal = new TestingAuthenticationToken("principal", "credentials");
322322
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
323323
OidcIdToken idToken = OidcIdToken.withTokenValue("id-token")
324324
.issuer("https://provider.com")
325-
.subject("other-sub")
326325
.audience(Collections.singleton(registeredClient.getClientId()))
327326
.issuedAt(Instant.now().minusSeconds(60).truncatedTo(ChronoUnit.MILLIS))
328327
.expiresAt(Instant.now().plusSeconds(60).truncatedTo(ChronoUnit.MILLIS))
@@ -355,6 +354,49 @@ public void authenticateWhenInvalidSubThenThrowOAuth2AuthenticationException() {
355354
eq(authorization.getRegisteredClientId()));
356355
}
357356

357+
// gh-1235
358+
@Test
359+
public void authenticateWhenInvalidPrincipalThenThrowOAuth2AuthenticationException() {
360+
TestingAuthenticationToken principal = new TestingAuthenticationToken("principal", "credentials");
361+
RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build();
362+
OidcIdToken idToken = OidcIdToken.withTokenValue("id-token")
363+
.issuer("https://provider.com")
364+
.subject(principal.getName())
365+
.audience(Collections.singleton(registeredClient.getClientId()))
366+
.issuedAt(Instant.now().minusSeconds(60).truncatedTo(ChronoUnit.MILLIS))
367+
.expiresAt(Instant.now().plusSeconds(60).truncatedTo(ChronoUnit.MILLIS))
368+
.build();
369+
OAuth2Authorization authorization = TestOAuth2Authorizations.authorization(registeredClient)
370+
.principalName(principal.getName())
371+
.token(idToken,
372+
(metadata) -> metadata.put(OAuth2Authorization.Token.CLAIMS_METADATA_NAME, idToken.getClaims()))
373+
.build();
374+
when(this.authorizationService.findByToken(eq(idToken.getTokenValue()), eq(ID_TOKEN_TOKEN_TYPE)))
375+
.thenReturn(authorization);
376+
when(this.registeredClientRepository.findById(eq(authorization.getRegisteredClientId())))
377+
.thenReturn(registeredClient);
378+
379+
principal.setAuthenticated(true);
380+
381+
TestingAuthenticationToken otherPrincipal = new TestingAuthenticationToken("other-principal", "credentials");
382+
otherPrincipal.setAuthenticated(true);
383+
384+
OidcLogoutAuthenticationToken authentication = new OidcLogoutAuthenticationToken(
385+
idToken.getTokenValue(), otherPrincipal, "session-1", null, null, null);
386+
387+
assertThatThrownBy(() -> this.authenticationProvider.authenticate(authentication))
388+
.isInstanceOf(OAuth2AuthenticationException.class)
389+
.extracting(ex -> ((OAuth2AuthenticationException) ex).getError())
390+
.satisfies(error -> {
391+
assertThat(error.getErrorCode()).isEqualTo(OAuth2ErrorCodes.INVALID_TOKEN);
392+
assertThat(error.getDescription()).contains("sub");
393+
});
394+
verify(this.authorizationService).findByToken(
395+
eq(authentication.getIdTokenHint()), eq(ID_TOKEN_TOKEN_TYPE));
396+
verify(this.registeredClientRepository).findById(
397+
eq(authorization.getRegisteredClientId()));
398+
}
399+
358400
@Test
359401
public void authenticateWhenMissingSidThenThrowOAuth2AuthenticationException() {
360402
TestingAuthenticationToken principal = new TestingAuthenticationToken("principal", "credentials");

0 commit comments

Comments
 (0)