From 8aa443deb5cc0e608b6206deb3bc8c6b95e6c3ad Mon Sep 17 00:00:00 2001 From: Carl Lundin Date: Fri, 3 Feb 2023 00:50:16 +0000 Subject: [PATCH 1/5] feat: Add PKCE to 3LO exchange. --- .../auth/oauth2/DefaultPKCEProvider.java | 103 ++++++++++++++++++ .../com/google/auth/oauth2/PKCEProvider.java | 56 ++++++++++ .../google/auth/oauth2/UserAuthorizer.java | 38 ++++++- .../auth/oauth2/DefaultPKCEProviderTest.java | 61 +++++++++++ .../auth/oauth2/UserAuthorizerTest.java | 4 + 5 files changed, 260 insertions(+), 2 deletions(-) create mode 100644 oauth2_http/java/com/google/auth/oauth2/DefaultPKCEProvider.java create mode 100644 oauth2_http/java/com/google/auth/oauth2/PKCEProvider.java create mode 100644 oauth2_http/javatests/com/google/auth/oauth2/DefaultPKCEProviderTest.java diff --git a/oauth2_http/java/com/google/auth/oauth2/DefaultPKCEProvider.java b/oauth2_http/java/com/google/auth/oauth2/DefaultPKCEProvider.java new file mode 100644 index 000000000..ae684f162 --- /dev/null +++ b/oauth2_http/java/com/google/auth/oauth2/DefaultPKCEProvider.java @@ -0,0 +1,103 @@ +/* + * Copyright 2023, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.google.auth.oauth2; + +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; +import java.util.Base64; + +public class DefaultPKCEProvider implements PKCEProvider { + private String codeVerifier; + private CodeChallenge codeChallenge; + final int MAX_CODE_VERIFIER_LENGTH = 128; + + private class CodeChallenge { + private String codeChallenge; + private String codeChallengeMethod; + + CodeChallenge(String codeVerifier) { + try { + byte[] bytes = codeVerifier.getBytes(); + MessageDigest md = MessageDigest.getInstance("SHA-256"); + md.update(bytes); + + byte[] digest = md.digest(); + + this.codeChallenge = Base64.getUrlEncoder().encodeToString(digest); + this.codeChallengeMethod = "S256"; + } catch (NoSuchAlgorithmException e) { + this.codeChallenge = codeVerifier; + this.codeChallengeMethod = "plain"; + } + } + + public String getCodeChallenge() { + return codeChallenge; + } + + public String getCodeChallengeMethod() { + return codeChallengeMethod; + } + } + + private String createCodeVerifier() { + SecureRandom sr = new SecureRandom(); + byte[] code = new byte[MAX_CODE_VERIFIER_LENGTH]; + sr.nextBytes(code); + return Base64.getUrlEncoder().encodeToString(code); + } + + private CodeChallenge createCodeChallenge(String codeVerifier) { + return new DefaultPKCEProvider.CodeChallenge(codeVerifier); + } + + public DefaultPKCEProvider() { + this.codeVerifier = createCodeVerifier(); + this.codeChallenge = createCodeChallenge(this.codeVerifier); + } + + @Override + public String getCodeVerifier() { + return codeVerifier; + } + + @Override + public String getCodeChallenge() { + return codeChallenge.getCodeChallenge(); + } + + @Override + public String getCodeChallengeMethod() { + return codeChallenge.getCodeChallengeMethod(); + } +} diff --git a/oauth2_http/java/com/google/auth/oauth2/PKCEProvider.java b/oauth2_http/java/com/google/auth/oauth2/PKCEProvider.java new file mode 100644 index 000000000..4800dd1cd --- /dev/null +++ b/oauth2_http/java/com/google/auth/oauth2/PKCEProvider.java @@ -0,0 +1,56 @@ +/* + * Copyright 2023, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.google.auth.oauth2; + +public interface PKCEProvider { + /** + * Get the code_challenge parameter used in PKCE. + * + * @return The code_challenge String. + */ + String getCodeChallenge(); + + /** + * Get the code_challenge_method parameter used in PKCE. + * + *

Currently possible values are: S256,plain + * + * @return The code_challenge_method String. + */ + String getCodeChallengeMethod(); + /** + * Get the code_verifier parameter used in PKCE. + * + * @return The code_verifier String. + */ + String getCodeVerifier(); +} diff --git a/oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java b/oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java index c152b8b44..8980e0260 100644 --- a/oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java +++ b/oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java @@ -67,6 +67,7 @@ public class UserAuthorizer { private final HttpTransportFactory transportFactory; private final URI tokenServerUri; private final URI userAuthUri; + private final PKCEProvider pkce; /** * Constructor with all parameters. @@ -79,6 +80,7 @@ public class UserAuthorizer { * tokens. * @param tokenServerUri URI of the end point that provides tokens * @param userAuthUri URI of the Web UI for user consent + * @param pkce PKCE implementation */ private UserAuthorizer( ClientId clientId, @@ -87,7 +89,8 @@ private UserAuthorizer( URI callbackUri, HttpTransportFactory transportFactory, URI tokenServerUri, - URI userAuthUri) { + URI userAuthUri, + PKCEProvider pkce) { this.clientId = Preconditions.checkNotNull(clientId); this.scopes = ImmutableList.copyOf(Preconditions.checkNotNull(scopes)); this.callbackUri = (callbackUri == null) ? DEFAULT_CALLBACK_URI : callbackUri; @@ -96,6 +99,7 @@ private UserAuthorizer( this.tokenServerUri = (tokenServerUri == null) ? OAuth2Utils.TOKEN_SERVER_URI : tokenServerUri; this.userAuthUri = (userAuthUri == null) ? OAuth2Utils.USER_AUTH_URI : userAuthUri; this.tokenStore = (tokenStore == null) ? new MemoryTokensStorage() : tokenStore; + this.pkce = pkce; } /** @@ -181,6 +185,10 @@ public URL getAuthorizationUrl(String userId, String state, URI baseUri) { url.put("login_hint", userId); } url.put("include_granted_scopes", true); + if (pkce != null) { + url.put("code_challenge", pkce.getCodeChallenge()); + url.put("code_challenge_method", pkce.getCodeChallengeMethod()); + } return url.toURL(); } @@ -248,6 +256,11 @@ public UserCredentials getCredentialsFromCode(String code, URI baseUri) throws I tokenData.put("client_secret", clientId.getClientSecret()); tokenData.put("redirect_uri", resolvedCallbackUri); tokenData.put("grant_type", "authorization_code"); + + if (pkce != null) { + tokenData.put("code_verifier", pkce.getCodeVerifier()); + } + UrlEncodedContent tokenContent = new UrlEncodedContent(tokenData); HttpRequestFactory requestFactory = transportFactory.create().createRequestFactory(); HttpRequest tokenRequest = @@ -430,6 +443,7 @@ public static class Builder { private URI userAuthUri; private Collection scopes; private HttpTransportFactory transportFactory; + private PKCEProvider pkce; protected Builder() {} @@ -478,6 +492,11 @@ public Builder setHttpTransportFactory(HttpTransportFactory transportFactory) { return this; } + public Builder setPKCEProvider(PKCEProvider pkce) { + this.pkce = pkce; + return this; + } + public ClientId getClientId() { return clientId; } @@ -506,9 +525,24 @@ public HttpTransportFactory getHttpTransportFactory() { return transportFactory; } + public PKCEProvider getPKCEProvider() { + return pkce; + } + public UserAuthorizer build() { + if (pkce == null) { + pkce = new DefaultPKCEProvider(); + } + return new UserAuthorizer( - clientId, scopes, tokenStore, callbackUri, transportFactory, tokenServerUri, userAuthUri); + clientId, + scopes, + tokenStore, + callbackUri, + transportFactory, + tokenServerUri, + userAuthUri, + pkce); } } } diff --git a/oauth2_http/javatests/com/google/auth/oauth2/DefaultPKCEProviderTest.java b/oauth2_http/javatests/com/google/auth/oauth2/DefaultPKCEProviderTest.java new file mode 100644 index 000000000..e56739aad --- /dev/null +++ b/oauth2_http/javatests/com/google/auth/oauth2/DefaultPKCEProviderTest.java @@ -0,0 +1,61 @@ +/* + * Copyright 2023, Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.google.auth.oauth2; + +import static org.junit.Assert.assertEquals; + +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.Base64; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public final class DefaultPKCEProviderTest { + @Test + public void testPkceExpected() throws NoSuchAlgorithmException { + PKCEProvider pkce = new DefaultPKCEProvider(); + + byte[] bytes = pkce.getCodeVerifier().getBytes(); + MessageDigest md = MessageDigest.getInstance("SHA-256"); + md.update(bytes); + + byte[] digest = md.digest(); + + String expectedCodeChallenge = Base64.getUrlEncoder().encodeToString(digest); + String expectedCodeChallengeMethod = "S256"; + + assertEquals(pkce.getCodeChallenge(), expectedCodeChallenge); + assertEquals(pkce.getCodeChallengeMethod(), expectedCodeChallengeMethod); + } +} diff --git a/oauth2_http/javatests/com/google/auth/oauth2/UserAuthorizerTest.java b/oauth2_http/javatests/com/google/auth/oauth2/UserAuthorizerTest.java index 822fcbe12..48bd2f925 100644 --- a/oauth2_http/javatests/com/google/auth/oauth2/UserAuthorizerTest.java +++ b/oauth2_http/javatests/com/google/auth/oauth2/UserAuthorizerTest.java @@ -72,6 +72,7 @@ public class UserAuthorizerTest { private static final URI CALLBACK_URI = URI.create("/testcallback"); private static final String CODE = "thisistheend"; private static final URI BASE_URI = URI.create("http://example.com/foo"); + private static final PKCEProvider pkce = new DefaultPKCEProvider(); @Test public void constructorMinimum() { @@ -148,6 +149,7 @@ public void getAuthorizationUrl() throws IOException { .setScopes(DUMMY_SCOPES) .setCallbackUri(CALLBACK_URI) .setUserAuthUri(AUTH_URI) + .setPKCEProvider(pkce) .build(); URL authorizationUrl = authorizer.getAuthorizationUrl(USER_ID, CUSTOM_STATE, BASE_URI); @@ -164,6 +166,8 @@ public void getAuthorizationUrl() throws IOException { assertEquals(CLIENT_ID_VALUE, parameters.get("client_id")); assertEquals(DUMMY_SCOPE, parameters.get("scope")); assertEquals("code", parameters.get("response_type")); + assertEquals(pkce.getCodeChallenge(), parameters.get("code_challenge")); + assertEquals(pkce.getCodeChallengeMethod(), parameters.get("code_challenge_method")); } @Test From 4ff7f565e7d115e859e8dea2aced14883fd07b83 Mon Sep 17 00:00:00 2001 From: Carl Lundin Date: Fri, 3 Feb 2023 01:03:25 +0000 Subject: [PATCH 2/5] Reduce buffer size by one. --- .../java/com/google/auth/oauth2/DefaultPKCEProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oauth2_http/java/com/google/auth/oauth2/DefaultPKCEProvider.java b/oauth2_http/java/com/google/auth/oauth2/DefaultPKCEProvider.java index ae684f162..ce4bc04e9 100644 --- a/oauth2_http/java/com/google/auth/oauth2/DefaultPKCEProvider.java +++ b/oauth2_http/java/com/google/auth/oauth2/DefaultPKCEProvider.java @@ -39,7 +39,7 @@ public class DefaultPKCEProvider implements PKCEProvider { private String codeVerifier; private CodeChallenge codeChallenge; - final int MAX_CODE_VERIFIER_LENGTH = 128; + final int MAX_CODE_VERIFIER_LENGTH = 127; private class CodeChallenge { private String codeChallenge; From 960adf570596050044258621cf57d7e313407760 Mon Sep 17 00:00:00 2001 From: Carl Lundin Date: Fri, 3 Feb 2023 19:31:28 +0000 Subject: [PATCH 3/5] Pr feedback. --- .../java/com/google/auth/oauth2/DefaultPKCEProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oauth2_http/java/com/google/auth/oauth2/DefaultPKCEProvider.java b/oauth2_http/java/com/google/auth/oauth2/DefaultPKCEProvider.java index ce4bc04e9..c114383ff 100644 --- a/oauth2_http/java/com/google/auth/oauth2/DefaultPKCEProvider.java +++ b/oauth2_http/java/com/google/auth/oauth2/DefaultPKCEProvider.java @@ -39,7 +39,7 @@ public class DefaultPKCEProvider implements PKCEProvider { private String codeVerifier; private CodeChallenge codeChallenge; - final int MAX_CODE_VERIFIER_LENGTH = 127; + private static final int MAX_CODE_VERIFIER_LENGTH = 127; private class CodeChallenge { private String codeChallenge; From b7112b43e49430fdb08d953fc6e1c7aad58aeb14 Mon Sep 17 00:00:00 2001 From: Carl Lundin Date: Mon, 6 Feb 2023 16:28:58 +0000 Subject: [PATCH 4/5] Added null function check. --- .../google/auth/oauth2/UserAuthorizer.java | 9 ++++++ .../auth/oauth2/UserAuthorizerTest.java | 29 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java b/oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java index 8980e0260..cf390684c 100644 --- a/oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java +++ b/oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java @@ -493,6 +493,15 @@ public Builder setHttpTransportFactory(HttpTransportFactory transportFactory) { } public Builder setPKCEProvider(PKCEProvider pkce) { + if (pkce != null) { + if (pkce.getCodeChallenge() == null + || pkce.getCodeVerifier() == null + || pkce.getCodeChallengeMethod() == null) { + + throw new IllegalArgumentException( + "PKCE provider contained null implementations. PKCE object must implement all PKCEProvider methods."); + } + } this.pkce = pkce; return this; } diff --git a/oauth2_http/javatests/com/google/auth/oauth2/UserAuthorizerTest.java b/oauth2_http/javatests/com/google/auth/oauth2/UserAuthorizerTest.java index 48bd2f925..60dd8f464 100644 --- a/oauth2_http/javatests/com/google/auth/oauth2/UserAuthorizerTest.java +++ b/oauth2_http/javatests/com/google/auth/oauth2/UserAuthorizerTest.java @@ -475,4 +475,33 @@ public void revokeAuthorization_revokesAndClears() throws IOException { UserCredentials credentials2 = authorizer.getCredentials(USER_ID); assertNull(credentials2); } + + @Test(expected = IllegalArgumentException.class) + public void illegalPKCEProvider() { + PKCEProvider pkce = + new PKCEProvider() { + @Override + public String getCodeVerifier() { + return null; + } + + @Override + public String getCodeChallengeMethod() { + return null; + } + + @Override + public String getCodeChallenge() { + return null; + } + }; + + UserAuthorizer authorizer = + UserAuthorizer.newBuilder() + .setClientId(CLIENT_ID) + .setScopes(DUMMY_SCOPES) + .setTokenStore(new MemoryTokensStorage()) + .setPKCEProvider(pkce) + .build(); + } } From b9c2e6c5a1d3ca493c1fcccbff73188e071040e2 Mon Sep 17 00:00:00 2001 From: Carl Lundin Date: Mon, 6 Feb 2023 23:29:29 +0000 Subject: [PATCH 5/5] Move PKCE NULL check so PKCE can be disabled by setting a NULL PKCE provider. --- oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java b/oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java index cf390684c..29a8284d5 100644 --- a/oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java +++ b/oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java @@ -455,6 +455,7 @@ protected Builder(UserAuthorizer authorizer) { this.tokenStore = authorizer.tokenStore; this.callbackUri = authorizer.callbackUri; this.userAuthUri = authorizer.userAuthUri; + this.pkce = new DefaultPKCEProvider(); } public Builder setClientId(ClientId clientId) { @@ -539,10 +540,6 @@ public PKCEProvider getPKCEProvider() { } public UserAuthorizer build() { - if (pkce == null) { - pkce = new DefaultPKCEProvider(); - } - return new UserAuthorizer( clientId, scopes,