From 153c404c3e3df70d85568da0cd389695da331d44 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 7 Oct 2019 16:28:50 +1100 Subject: [PATCH] Issue #4128 - test the decoding of OpenId Credentials Signed-off-by: Lachlan Roberts --- .../security/openid/CredentialsDecoder.java | 73 +++++++++++++++++++ .../security/openid/OpenIdCredentials.java | 56 +------------- .../openid/CredentialsDecoderTest.java | 73 +++++++++++++++++++ .../jetty/security/openid/JwtEncoder.java | 34 +++++++++ .../openid/OpenIdAuthenticationTest.java | 4 +- .../jetty/security/openid/OpenIdProvider.java | 45 +++--------- 6 files changed, 194 insertions(+), 91 deletions(-) create mode 100644 jetty-openid/src/main/java/org/eclipse/jetty/security/openid/CredentialsDecoder.java create mode 100644 jetty-openid/src/test/java/org/eclipse/jetty/security/openid/CredentialsDecoderTest.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/CredentialsDecoder.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/CredentialsDecoder.java new file mode 100644 index 000000000000..e9e164bed316 --- /dev/null +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/CredentialsDecoder.java @@ -0,0 +1,73 @@ +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 CredentialsDecoder +{ + private static final Logger LOG = Log.getLogger(CredentialsDecoder.class); + private static final Base64.Decoder decoder = Base64.getUrlDecoder(); + + /** + * 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"); + + 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]; + + Map jwtHeader = (Map)JSON.parse(jwtHeaderString); + 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); + + return (Map)JSON.parse(jwtClaimString); + } + + static byte[] padJWTSection(String unpaddedEncodedJwtSection) + { + int length = unpaddedEncodedJwtSection.length(); + int remainder = length % 4; + + 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 230396b85995..8e1a193aed9b 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 @@ -25,8 +25,6 @@ import java.net.HttpURLConnection; import java.net.URL; import java.nio.charset.StandardCharsets; -import java.util.Arrays; -import java.util.Base64; import java.util.Map; import org.eclipse.jetty.util.IO; @@ -103,7 +101,7 @@ public void redeemAuthCode() throws IOException if (!"Bearer".equalsIgnoreCase(tokenType)) throw new IllegalArgumentException("invalid token_type"); - claims = decodeJWT(idToken); + claims = CredentialsDecoder.decode(idToken); if (LOG.isDebugEnabled()) LOG.debug("claims {}", claims); validateClaims(); @@ -150,58 +148,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]; - - Map jwtHeader = (Map)JSON.parse(jwtHeaderString); - 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); - - return (Map)JSON.parse(jwtClaimString); - } - - 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(String authCode) throws IOException { if (LOG.isDebugEnabled()) diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/CredentialsDecoderTest.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/CredentialsDecoderTest.java new file mode 100644 index 000000000000..69fd94ecf229 --- /dev/null +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/CredentialsDecoderTest.java @@ -0,0 +1,73 @@ +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 CredentialsDecoderTest +{ + public static Stream paddingExamples() + { + return Stream.of( + Arguments.of("XXXX", "XXXX"), + Arguments.of("XXX", "XXX="), + Arguments.of("XX", "XX==") + ); + } + + 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 = CredentialsDecoder.padJWTSection(input); + assertThat(actual, is(expected.getBytes())); + } + + @ParameterizedTest + @MethodSource("badPaddingExamples") + public void testPaddingInvalidBase64(String input) + { + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, + () -> CredentialsDecoder.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 = CredentialsDecoder.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)); + } +} \ 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..d032fd0e4068 --- /dev/null +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/JwtEncoder.java @@ -0,0 +1,34 @@ +package org.eclipse.jetty.security.openid; + +import java.util.Base64; + +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]; + } + + 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 + "@fake-email.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..da5dc0c43228 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@fake-email.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); } } }