diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java index 8d02992c2ec3a..4bf93fc10e96c 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticator.java @@ -477,7 +477,20 @@ static void handleUserinfoResponse( if (httpResponse.getStatusLine().getStatusCode() == 200) { if (ContentType.parse(contentHeader.getValue()).getMimeType().equals("application/json")) { final JWTClaimsSet userInfoClaims = JWTClaimsSet.parse(contentAsString); - validateUserInfoResponse(userInfoClaims, verifiedIdTokenClaims.getSubject(), claimsListener); + String expectedSub = verifiedIdTokenClaims.getSubject(); + if (userInfoClaims.getSubject() == null || userInfoClaims.getSubject().isEmpty()) { + claimsListener.onFailure(new ElasticsearchSecurityException("Userinfo Response did not contain a sub Claim")); + return; + } else if (userInfoClaims.getSubject().equals(expectedSub) == false) { + claimsListener.onFailure( + new ElasticsearchSecurityException( + "Userinfo Response is not valid as it is for " + "subject [{}] while the ID Token was for subject [{}]", + userInfoClaims.getSubject(), + expectedSub + ) + ); + return; + } if (LOGGER.isTraceEnabled()) { LOGGER.trace("Successfully retrieved user information: [{}]", userInfoClaims); } @@ -527,27 +540,6 @@ static void handleUserinfoResponse( } } - /** - * Validates that the userinfo response contains a sub Claim and that this claim value is the same as the one returned in the ID Token - */ - private static void validateUserInfoResponse( - JWTClaimsSet userInfoClaims, - String expectedSub, - ActionListener claimsListener - ) { - if (userInfoClaims.getSubject().isEmpty()) { - claimsListener.onFailure(new ElasticsearchSecurityException("Userinfo Response did not contain a sub Claim")); - } else if (userInfoClaims.getSubject().equals(expectedSub) == false) { - claimsListener.onFailure( - new ElasticsearchSecurityException( - "Userinfo Response is not valid as it is for " + "subject [{}] while the ID Token was for subject [{}]", - userInfoClaims.getSubject(), - expectedSub - ) - ); - } - } - /** * Attempts to make a request to the Token Endpoint of the OpenID Connect provider in order to exchange an * authorization code for an Id Token (and potentially an Access Token) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticatorTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticatorTests.java index 76069ce500ad9..ebf98b035f7d6 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticatorTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectAuthenticatorTests.java @@ -108,6 +108,7 @@ import java.util.Map; import java.util.UUID; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import javax.crypto.SecretKey; @@ -968,6 +969,53 @@ public void testHandleUserinfoResponseFailure() throws Exception { ); } + public void testHandleUserinfoValidationFailsOnNotMatchingSubject() throws Exception { + final ProtocolVersion httpVersion = randomFrom(HttpVersion.HTTP_0_9, HttpVersion.HTTP_1_0, HttpVersion.HTTP_1_1); + final HttpResponse response = new BasicHttpResponse(new BasicStatusLine(httpVersion, RestStatus.OK.getStatus(), "OK")); + + final String sub = randomAlphaOfLengthBetween(4, 36); + final String inf = randomAlphaOfLength(12); + final JWTClaimsSet infoClaims = new JWTClaimsSet.Builder().subject("it-is-a-different-subject").claim("inf", inf).build(); + final StringEntity entity = new StringEntity(infoClaims.toString(), ContentType.APPLICATION_JSON); + if (randomBoolean()) { + entity.setContentEncoding( + randomFrom(StandardCharsets.UTF_8.name(), StandardCharsets.UTF_16.name(), StandardCharsets.US_ASCII.name()) + ); + } + response.setEntity(entity); + + final String idx = randomAlphaOfLength(8); + final JWTClaimsSet idClaims = new JWTClaimsSet.Builder().subject(sub).claim("idx", idx).build(); + final AtomicBoolean listenerCalled = new AtomicBoolean(false); + final PlainActionFuture future = new PlainActionFuture<>() { + + @Override + public void onResponse(JWTClaimsSet result) { + assertTrue("listener called more than once", listenerCalled.compareAndSet(false, true)); + super.onResponse(result); + } + + @Override + public void onFailure(Exception e) { + assertTrue("listener called more than once", listenerCalled.compareAndSet(false, true)); + super.onFailure(e); + } + }; + + this.authenticator = buildAuthenticator(); + OpenIdConnectAuthenticator.handleUserinfoResponse(response, idClaims, future); + var e = expectThrows(ElasticsearchSecurityException.class, future::actionGet); + + assertThat( + e.getMessage(), + equalTo( + "Userinfo Response is not valid as it is for subject [it-is-a-different-subject] while the ID Token was for subject [" + + sub + + "]" + ) + ); + } + public void testHandleTokenResponseNullContentType() { final HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, RestStatus.OK.getStatus(), ""); final StringEntity entity = new StringEntity("", (ContentType) null);