From cff6bb444a2f7e985febffda3d5b85dea4909d7d Mon Sep 17 00:00:00 2001 From: Lachlan Date: Wed, 20 Nov 2019 14:23:19 +1100 Subject: [PATCH] Issue #4128 - test the decoding of OpenId Credentials (#4166) Signed-off-by: Lachlan Roberts --- .../jetty/security/openid/JwtDecoder.java | 102 +++++++++++++++ .../security/openid/OpenIdCredentials.java | 65 +--------- .../jetty/security/openid/JwtDecoderTest.java | 122 ++++++++++++++++++ .../jetty/security/openid/JwtEncoder.java | 58 +++++++++ .../openid/OpenIdAuthenticationTest.java | 4 +- .../jetty/security/openid/OpenIdProvider.java | 45 ++----- 6 files changed, 296 insertions(+), 100 deletions(-) create mode 100644 jetty-openid/src/main/java/org/eclipse/jetty/security/openid/JwtDecoder.java create mode 100644 jetty-openid/src/test/java/org/eclipse/jetty/security/openid/JwtDecoderTest.java create mode 100644 jetty-openid/src/test/java/org/eclipse/jetty/security/openid/JwtEncoder.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 new file mode 100644 index 000000000000..bbeed6bc6958 --- /dev/null +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/JwtDecoder.java @@ -0,0 +1,102 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// 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. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// 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; + +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Base64; +import java.util.Map; + +import org.eclipse.jetty.util.ajax.JSON; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; + +/** + * Used to decode the ID Token from the base64 encrypted JSON Web Token (JWT). + */ +public class JwtDecoder +{ + private static final Logger LOG = Log.getLogger(JwtDecoder.class); + + /** + * Decodes a JSON Web Token (JWT) into a Map of claims. + * @param jwt the JWT to decode. + * @return the map of claims encoded in the JWT. + */ + public static Map decode(String jwt) + { + if (LOG.isDebugEnabled()) + LOG.debug("decode {}", jwt); + + String[] sections = jwt.split("\\."); + if (sections.length != 3) + throw new IllegalArgumentException("JWT does not contain 3 sections"); + + Base64.Decoder decoder = Base64.getUrlDecoder(); + String jwtHeaderString = new String(decoder.decode(padJWTSection(sections[0])), StandardCharsets.UTF_8); + String jwtClaimString = new String(decoder.decode(padJWTSection(sections[1])), StandardCharsets.UTF_8); + String jwtSignature = sections[2]; + + Object parsedJwtHeader = JSON.parse(jwtHeaderString); + if (!(parsedJwtHeader instanceof Map)) + throw new IllegalStateException("Invalid JWT header"); + Map jwtHeader = (Map)parsedJwtHeader; + if (LOG.isDebugEnabled()) + LOG.debug("JWT Header: {}", jwtHeader); + + /* If the ID Token is received via direct communication between the Client + and the Token Endpoint (which it is in this flow), the TLS server validation + MAY be used to validate the issuer in place of checking the token signature. */ + if (LOG.isDebugEnabled()) + LOG.debug("JWT signature not validated {}", jwtSignature); + + Object parsedClaims = JSON.parse(jwtClaimString); + if (!(parsedClaims instanceof Map)) + throw new IllegalStateException("Could not decode JSON for JWT claims."); + return (Map)parsedClaims; + } + + static byte[] padJWTSection(String unpaddedEncodedJwtSection) + { + // If already padded just use what we are given. + if (unpaddedEncodedJwtSection.endsWith("=")) + return unpaddedEncodedJwtSection.getBytes(); + + int length = unpaddedEncodedJwtSection.length(); + int remainder = length % 4; + + // A valid base-64-encoded string will have a remainder of 0, 2 or 3. Never 1! + if (remainder == 1) + throw new IllegalArgumentException("Not a valid Base64-encoded string"); + + byte[] paddedEncodedJwtSection; + if (remainder > 0) + { + int paddingNeeded = (4 - remainder) % 4; + paddedEncodedJwtSection = Arrays.copyOf(unpaddedEncodedJwtSection.getBytes(), length + paddingNeeded); + Arrays.fill(paddedEncodedJwtSection, length, paddedEncodedJwtSection.length, (byte)'='); + } + else + { + paddedEncodedJwtSection = unpaddedEncodedJwtSection.getBytes(); + } + + return paddedEncodedJwtSection; + } +} 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 c3115b0bfa0f..cc23e5dd4733 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 @@ -18,11 +18,8 @@ package org.eclipse.jetty.security.openid; -import java.io.IOException; import java.io.Serializable; -import java.nio.charset.StandardCharsets; import java.util.Arrays; -import java.util.Base64; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -103,7 +100,7 @@ public void redeemAuthCode(HttpClient httpClient) throws Exception if (!"Bearer".equalsIgnoreCase(tokenType)) throw new IllegalArgumentException("invalid token_type"); - claims = decodeJWT(idToken); + claims = JwtDecoder.decode(idToken); if (LOG.isDebugEnabled()) LOG.debug("claims {}", claims); validateClaims(); @@ -171,65 +168,6 @@ public boolean isExpired() return false; } - protected Map decodeJWT(String jwt) throws IOException - { - if (LOG.isDebugEnabled()) - LOG.debug("decodeJWT {}", jwt); - - String[] sections = jwt.split("\\."); - if (sections.length != 3) - throw new IllegalArgumentException("JWT does not contain 3 sections"); - - Base64.Decoder decoder = Base64.getUrlDecoder(); - String jwtHeaderString = new String(decoder.decode(padJWTSection(sections[0])), StandardCharsets.UTF_8); - String jwtClaimString = new String(decoder.decode(padJWTSection(sections[1])), StandardCharsets.UTF_8); - String jwtSignature = sections[2]; - - Object parsedJwtHeader = JSON.parse(jwtHeaderString); - if (!(parsedJwtHeader instanceof Map)) - throw new IllegalStateException("Invalid JWT header"); - Map jwtHeader = (Map)parsedJwtHeader; - LOG.debug("JWT Header: {}", jwtHeader); - - /* If the ID Token is received via direct communication between the Client - and the Token Endpoint (which it is in this flow), the TLS server validation - MAY be used to validate the issuer in place of checking the token signature. */ - if (LOG.isDebugEnabled()) - LOG.debug("JWT signature not validated {}", jwtSignature); - - Object parsedClaims = JSON.parse(jwtClaimString); - if (!(parsedClaims instanceof Map)) - throw new IllegalStateException("Could not decode JSON for JWT claims."); - - return (Map)parsedClaims; - } - - private static byte[] padJWTSection(String unpaddedEncodedJwtSection) - { - int length = unpaddedEncodedJwtSection.length(); - int remainder = length % 4; - - if (remainder == 1) - // A valid base64-encoded string will never be have an odd number of characters. - throw new IllegalArgumentException("Not valid Base64-encoded string"); - - byte[] paddedEncodedJwtSection; - - if (remainder > 0) - { - int paddingNeeded = (4 - remainder) % 4; - - paddedEncodedJwtSection = Arrays.copyOf(unpaddedEncodedJwtSection.getBytes(), length + paddingNeeded); - Arrays.fill(paddedEncodedJwtSection, length, paddedEncodedJwtSection.length, (byte)'='); - } - else - { - paddedEncodedJwtSection = unpaddedEncodedJwtSection.getBytes(); - } - - return paddedEncodedJwtSection; - } - private Map claimAuthCode(HttpClient httpClient, String authCode) throws Exception { Fields fields = new Fields(); @@ -250,7 +188,6 @@ private Map claimAuthCode(HttpClient httpClient, String authCode Object parsedResponse = JSON.parse(responseBody); if (!(parsedResponse instanceof Map)) throw new IllegalStateException("Malformed response from OpenID Provider"); - return (Map)parsedResponse; } } diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/JwtDecoderTest.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/JwtDecoderTest.java new file mode 100644 index 000000000000..01ea028010ea --- /dev/null +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/JwtDecoderTest.java @@ -0,0 +1,122 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// 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. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// 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; + +import java.util.Map; +import java.util.stream.Stream; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class JwtDecoderTest +{ + public static Stream paddingExamples() + { + return Stream.of( + Arguments.of("XXXX", "XXXX"), + Arguments.of("XXX", "XXX="), + Arguments.of("XX", "XX=="), + Arguments.of("XXX=", "XXX="), + Arguments.of("X-X", "X-X="), + Arguments.of("@#", "@#=="), + Arguments.of("X=", "X="), + Arguments.of("XX=", "XX="), + Arguments.of("XX==", "XX=="), + Arguments.of("XXX=", "XXX="), + Arguments.of("", "") + ); + } + + public static Stream badPaddingExamples() + { + return Stream.of( + Arguments.of("X"), + Arguments.of("XXXXX") + ); + } + + @ParameterizedTest + @MethodSource("paddingExamples") + public void testPaddingBase64(String input, String expected) + { + byte[] actual = JwtDecoder.padJWTSection(input); + assertThat(actual, is(expected.getBytes())); + } + + @ParameterizedTest + @MethodSource("badPaddingExamples") + public void testPaddingInvalidBase64(String input) + { + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, + () -> JwtDecoder.padJWTSection(input)); + + assertThat(error.getMessage(), is("Not a valid Base64-encoded string")); + } + + @Test + public void testEncodeDecode() + { + String issuer = "example.com"; + String subject = "1234"; + String clientId = "1234.client.id"; + String name = "Bob"; + long expiry = 123; + + // Create a fake ID Token. + String claims = JwtEncoder.createIdToken(issuer, clientId, subject, name, expiry); + String idToken = JwtEncoder.encode(claims); + + // Decode the ID Token and verify the claims are the same. + Map decodedClaims = JwtDecoder.decode(idToken); + assertThat(decodedClaims.get("iss"), is(issuer)); + assertThat(decodedClaims.get("sub"), is(subject)); + assertThat(decodedClaims.get("aud"), is(clientId)); + assertThat(decodedClaims.get("name"), is(name)); + assertThat(decodedClaims.get("exp"), is(expiry)); + } + + @Test + public void testDecodeMissingPadding() + { + // Example given in Issue #4128 which requires the re-adding the B64 padding to decode. + String jwt = "eyJraWQiOiIxNTU1OTM0ODQ3IiwieDV0IjoiOWdCOW9zRldSRHRSMkhtNGNmVnJnWTBGcmZRIiwiYWxnIjoiUlMyNTYifQ" + + ".eyJhdF9oYXNoIjoiQTA0NUoxcE5YRk1nYzlXN2wxSk1fUSIsImRlbGVnYXRpb25faWQiOiJjZTBhNjRlNS0xYWY3LTQ2MzEtOGUz" + + "NC1mNDE5N2JkYzVjZTAiLCJhY3IiOiJ1cm46c2U6Y3VyaXR5OmF1dGhlbnRpY2F0aW9uOmh0bWwtZm9ybTpodG1sLXByaW1hcnkiL" + + "CJzX2hhc2giOiIwc1FtRG9YY3FwcnM4NWUzdy0wbHdBIiwiYXpwIjoiNzZiZTc5Y2ItM2E1Ni00ZTE3LTg3NzYtNDI1Nzc5MjRjYz" + + "c2IiwiYXV0aF90aW1lIjoxNTY5NjU4MDk1LCJleHAiOjE1Njk2NjE5OTUsIm5iZiI6MTU2OTY1ODM5NSwianRpIjoiZjJkNWI2YzE" + + "tNTIxYi00Y2Y5LThlNWEtOTg5NGJhNmE0MzkyIiwiaXNzIjoiaHR0cHM6Ly9ub3JkaWNhcGlzLmN1cml0eS5pby9-IiwiYXVkIjoi" + + "NzZiZTc5Y2ItM2E1Ni00ZTE3LTg3NzYtNDI1Nzc5MjRjYzc2Iiwic3ViIjoibmlrb3MiLCJpYXQiOjE1Njk2NTgzOTUsInB1cnBvc" + + "2UiOiJpZCJ9.Wd458zNmXggpkDN6vbS3-aiajh4-VbkmcStLYUqahYJUp9p-AUI_RZttWvwh3UDMG9rWww_ya8KFK_SkPfKooEaSN" + + "OjOhw0ox4d-9lgti3J49eRyO20RViXvRHyLVtcjv5IaqvMXgwW60Thubv19OION7DstyArffcxNNSpiqDq6wjd0T2DJ3gSXXlJHLT" + + "Wrry3svqu1j_GCbHc04XYGicxsusKgc3n22dh4I6p4trdo0Gu5Un0bZ8Yov7IzWItqTgm9X5r9gZlAOLcAuK1WTwkzAwZJ24HgvxK" + + "muYfV_4ZCg_VPN2Op8YPuRAQOgUERpeTv1RDFTOG9GKZIMBVR0A"; + + // Decode the ID Token and verify the claims are the correct. + Map decodedClaims = JwtDecoder.decode(jwt); + assertThat(decodedClaims.get("sub"), is("nikos")); + assertThat(decodedClaims.get("aud"), is("76be79cb-3a56-4e17-8776-42577924cc76")); + assertThat(decodedClaims.get("exp"), is(1569661995L)); + } +} \ No newline at end of file diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/JwtEncoder.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/JwtEncoder.java new file mode 100644 index 000000000000..3a575ede0986 --- /dev/null +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/JwtEncoder.java @@ -0,0 +1,58 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// 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. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// 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; + +import java.util.Base64; + +/** + * A basic JWT encoder for testing purposes. + */ +public class JwtEncoder +{ + private static final Base64.Encoder ENCODER = Base64.getUrlEncoder(); + private static final String DEFAULT_HEADER = "{\"INFO\": \"this is not used or checked in our implementation\"}"; + private static final String DEFAULT_SIGNATURE = "we do not validate signature as we use the authorization code flow"; + + public static String encode(String idToken) + { + return stripPadding(ENCODER.encodeToString(DEFAULT_HEADER.getBytes())) + "." + + stripPadding(ENCODER.encodeToString(idToken.getBytes())) + "." + + stripPadding(ENCODER.encodeToString(DEFAULT_SIGNATURE.getBytes())); + } + + private static String stripPadding(String paddedBase64) + { + return paddedBase64.split("=")[0]; + } + + /** + * Create a basic JWT for testing using argument supplied attributes. + */ + public static String createIdToken(String provider, String clientId, String subject, String name, long expiry) + { + return "{" + + "\"iss\": \"" + provider + "\"," + + "\"sub\": \"" + subject + "\"," + + "\"aud\": \"" + clientId + "\"," + + "\"exp\": " + expiry + "," + + "\"name\": \"" + name + "\"," + + "\"email\": \"" + name + "@example.com" + "\"" + + "}"; + } +} diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java index 54dd613c52d9..829b0f9aef0a 100644 --- a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java @@ -149,8 +149,8 @@ public void testLoginLogout() throws Exception content = response.getContentAsString().split("\n"); assertThat(content.length, is(3)); assertThat(content[0], is("userId: 123456789")); - assertThat(content[1], is("name: FirstName LastName")); - assertThat(content[2], is("email: FirstName@fake-email.com")); + assertThat(content[1], is("name: Alice")); + assertThat(content[2], is("email: Alice@example.com")); // Request to admin page gives 403 as we do not have admin role response = client.GET(appUriString + "/admin"); diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java index 83bfb3441b72..ce586dcac7f1 100644 --- a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java @@ -22,7 +22,6 @@ import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; -import java.util.Base64; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -137,7 +136,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se } String authCode = UUID.randomUUID().toString().replace("-", ""); - User user = new User(123456789, "FirstName", "LastName"); + User user = new User(123456789, "Alice"); issuedAuthCodes.put(authCode, user); final Request baseRequest = Request.getBaseRequest(req); @@ -173,20 +172,11 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S return; } - String jwtHeader = "{\"INFO\": \"this is not used or checked in our implementation\"}"; - String jwtBody = user.getIdToken(); - String jwtSignature = "we do not validate signature as we use the authorization code flow"; - - Base64.Encoder encoder = Base64.getEncoder(); - String jwt = encoder.encodeToString(jwtHeader.getBytes()) + "." + - encoder.encodeToString(jwtBody.getBytes()) + "." + - encoder.encodeToString(jwtSignature.getBytes()); - String accessToken = "ABCDEFG"; long expiry = System.currentTimeMillis() + Duration.ofMinutes(10).toMillis(); String response = "{" + "\"access_token\": \"" + accessToken + "\"," + - "\"id_token\": \"" + jwt + "\"," + + "\"id_token\": \"" + JwtEncoder.encode(user.getIdToken()) + "\"," + "\"expires_in\": " + expiry + "," + "\"token_type\": \"Bearer\"" + "}"; @@ -214,41 +204,28 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se public class User { private long subject; - private String firstName; - private String lastName; + private String name; - public User(String firstName, String lastName) + public User(String name) { - this(new Random().nextLong(), firstName, lastName); + this(new Random().nextLong(), name); } - public User(long subject, String firstName, String lastName) + public User(long subject, String name) { this.subject = subject; - this.firstName = firstName; - this.lastName = lastName; - } - - public String getFirstName() - { - return firstName; + this.name = name; } - public String getLastName() + public String getName() { - return lastName; + return name; } public String getIdToken() { - return "{" + - "\"iss\": \"" + provider + "\"," + - "\"sub\": \"" + subject + "\"," + - "\"aud\": \"" + clientId + "\"," + - "\"exp\": " + System.currentTimeMillis() + Duration.ofMinutes(1).toMillis() + "," + - "\"name\": \"" + firstName + " " + lastName + "\"," + - "\"email\": \"" + firstName + "@fake-email.com" + "\"" + - "}"; + long expiry = System.currentTimeMillis() + Duration.ofMinutes(1).toMillis(); + return JwtEncoder.createIdToken(provider, clientId, Long.toString(subject), name, expiry); } } }