From 61f7e57217edddb4e38543941a3c18c0f153b108 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 16 Aug 2021 10:45:26 +1000 Subject: [PATCH 1/3] Issue #6618 - azp claim should not be required for single value aud array Signed-off-by: Lachlan Roberts --- .../jetty/security/openid/JwtDecoder.java | 5 ++- .../security/openid/OpenIdCredentials.java | 14 ++++--- .../openid/OpenIdCredentialsTest.java | 40 +++++++++++++++++++ 3 files changed, 51 insertions(+), 8 deletions(-) create mode 100644 jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdCredentialsTest.java diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/JwtDecoder.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/JwtDecoder.java index 69478ee361dd..5e6c53f2d0cc 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/JwtDecoder.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/JwtDecoder.java @@ -39,6 +39,7 @@ public class JwtDecoder * @param jwt the JWT to decode. * @return the map of claims encoded in the JWT. */ + @SuppressWarnings("unchecked") public static Map decode(String jwt) { if (LOG.isDebugEnabled()) @@ -56,7 +57,7 @@ public static Map decode(String jwt) Object parsedJwtHeader = JSON.parse(jwtHeaderString); if (!(parsedJwtHeader instanceof Map)) throw new IllegalStateException("Invalid JWT header"); - Map jwtHeader = (Map)parsedJwtHeader; + Map jwtHeader = (Map)parsedJwtHeader; if (LOG.isDebugEnabled()) LOG.debug("JWT Header: {}", jwtHeader); @@ -69,7 +70,7 @@ and the Token Endpoint (which it is in this flow), the TLS server validation Object parsedClaims = JSON.parse(jwtClaimString); if (!(parsedClaims instanceof Map)) throw new IllegalStateException("Could not decode JSON for JWT claims."); - return (Map)parsedClaims; + return (Map)parsedClaims; } static byte[] padJWTSection(String unpaddedEncodedJwtSection) diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java index a22af818ac67..863e7f26ca80 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java @@ -20,6 +20,7 @@ import java.io.Serializable; import java.util.Arrays; +import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -100,7 +101,7 @@ public void redeemAuthCode(OpenIdConfiguration configuration) throws Exception claims = JwtDecoder.decode(idToken); if (LOG.isDebugEnabled()) LOG.debug("claims {}", claims); - validateClaims(configuration); + validateClaims(claims, configuration); } finally { @@ -110,14 +111,14 @@ public void redeemAuthCode(OpenIdConfiguration configuration) throws Exception } } - private void validateClaims(OpenIdConfiguration configuration) throws Exception + static void validateClaims(Map claims, OpenIdConfiguration configuration) throws Exception { // Issuer Identifier for the OpenID Provider MUST exactly match the value of the iss (issuer) Claim. if (!configuration.getIssuer().equals(claims.get("iss"))) throw new AuthenticationException("Issuer Identifier MUST exactly match the iss Claim"); // The aud (audience) Claim MUST contain the client_id value. - validateAudience(configuration); + validateAudience(claims, configuration); // If an azp (authorized party) Claim is present, verify that its client_id is the Claim Value. Object azp = claims.get("azp"); @@ -131,7 +132,7 @@ private void validateClaims(OpenIdConfiguration configuration) throws Exception throw new AuthenticationException("ID Token has expired"); } - private void validateAudience(OpenIdConfiguration configuration) throws AuthenticationException + private static void validateAudience(Map claims, OpenIdConfiguration configuration) throws AuthenticationException { Object aud = claims.get("aud"); String clientId = configuration.getClientId(); @@ -143,10 +144,11 @@ private void validateAudience(OpenIdConfiguration configuration) throws Authenti throw new AuthenticationException("Audience Claim MUST contain the client_id value"); else if (isList) { - if (!Arrays.asList((Object[])aud).contains(clientId)) + List list = Arrays.asList((Object[])aud); + if (!list.contains(clientId)) throw new AuthenticationException("Audience Claim MUST contain the client_id value"); - if (claims.get("azp") == null) + if (list.size() > 1 && claims.get("azp") == null) throw new AuthenticationException("A multi-audience ID token needs to contain an azp claim"); } else if (!isValidType) diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdCredentialsTest.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdCredentialsTest.java new file mode 100644 index 000000000000..b816d9752363 --- /dev/null +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdCredentialsTest.java @@ -0,0 +1,40 @@ +// +// ======================================================================== +// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.security.openid; + +import java.util.HashMap; +import java.util.Map; + +import org.eclipse.jetty.client.HttpClient; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; + +public class OpenIdCredentialsTest +{ + @Test + public void testSingleAudienceValueInArray() throws Exception + { + String issuer = "myIssuer123"; + String clientId = "myClientId456"; + OpenIdConfiguration configuration = new OpenIdConfiguration(issuer, "", "", clientId, "", new HttpClient()); + + Map claims = new HashMap<>(); + claims.put("iss", issuer); + claims.put("aud", new String[]{clientId}); + claims.put("exp", System.currentTimeMillis() + 5000); + + assertDoesNotThrow(() -> OpenIdCredentials.validateClaims(claims, configuration)); + } +} From 3e6446ef4aaba1e2e257d968301c658c5d2fccaf Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 17 Aug 2021 13:31:49 +1000 Subject: [PATCH 2/3] Issue #6618 - Use a new OpenIdCredentials constructor instead of static method. Signed-off-by: Lachlan Roberts --- .../security/openid/OpenIdCredentials.java | 21 +++++++++++++++---- .../openid/OpenIdCredentialsTest.java | 2 +- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java index 863e7f26ca80..622595ded24c 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java @@ -51,6 +51,14 @@ public class OpenIdCredentials implements Serializable private String authCode; private Map response; private Map claims; + private boolean verified = false; + + public OpenIdCredentials(Map claims) + { + this.redirectUri = null; + this.authCode = null; + this.claims = claims; + } public OpenIdCredentials(String authCode, String redirectUri) { @@ -101,7 +109,6 @@ public void redeemAuthCode(OpenIdConfiguration configuration) throws Exception claims = JwtDecoder.decode(idToken); if (LOG.isDebugEnabled()) LOG.debug("claims {}", claims); - validateClaims(claims, configuration); } finally { @@ -109,16 +116,22 @@ public void redeemAuthCode(OpenIdConfiguration configuration) throws Exception authCode = null; } } + + if (!verified) + { + validateClaims(configuration); + verified = true; + } } - static void validateClaims(Map claims, OpenIdConfiguration configuration) throws Exception + private void validateClaims(OpenIdConfiguration configuration) throws Exception { // Issuer Identifier for the OpenID Provider MUST exactly match the value of the iss (issuer) Claim. if (!configuration.getIssuer().equals(claims.get("iss"))) throw new AuthenticationException("Issuer Identifier MUST exactly match the iss Claim"); // The aud (audience) Claim MUST contain the client_id value. - validateAudience(claims, configuration); + validateAudience(configuration); // If an azp (authorized party) Claim is present, verify that its client_id is the Claim Value. Object azp = claims.get("azp"); @@ -132,7 +145,7 @@ static void validateClaims(Map claims, OpenIdConfiguration confi throw new AuthenticationException("ID Token has expired"); } - private static void validateAudience(Map claims, OpenIdConfiguration configuration) throws AuthenticationException + private void validateAudience(OpenIdConfiguration configuration) throws AuthenticationException { Object aud = claims.get("aud"); String clientId = configuration.getClientId(); diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdCredentialsTest.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdCredentialsTest.java index b816d9752363..18ac12841f46 100644 --- a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdCredentialsTest.java +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdCredentialsTest.java @@ -35,6 +35,6 @@ public void testSingleAudienceValueInArray() throws Exception claims.put("aud", new String[]{clientId}); claims.put("exp", System.currentTimeMillis() + 5000); - assertDoesNotThrow(() -> OpenIdCredentials.validateClaims(claims, configuration)); + assertDoesNotThrow(() -> new OpenIdCredentials(claims).redeemAuthCode(configuration)); } } From c20be7da4d8f1bd980ff0c061560df52d54c5649 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 18 Aug 2021 12:52:54 +1000 Subject: [PATCH 3/3] Fix licence header. Signed-off-by: Lachlan Roberts --- .../openid/OpenIdCredentialsTest.java | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdCredentialsTest.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdCredentialsTest.java index 18ac12841f46..48c62aff55f3 100644 --- a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdCredentialsTest.java +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdCredentialsTest.java @@ -1,14 +1,19 @@ // -// ======================================================================== -// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. +// ======================================================================== +// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. // -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html // -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== // package org.eclipse.jetty.security.openid;