Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add PKCE to 3LO exchange. #1146

Merged
merged 5 commits into from
Feb 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 103 additions & 0 deletions oauth2_http/java/com/google/auth/oauth2/DefaultPKCEProvider.java
Original file line number Diff line number Diff line change
@@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing javadoc: what is a default PKCE provider ) with a link to a related google docs

private String codeVerifier;
private CodeChallenge codeChallenge;
private static final int MAX_CODE_VERIFIER_LENGTH = 127;

private class CodeChallenge {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Helpers should go below

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and missing javadoc

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();
}
}
56 changes: 56 additions & 0 deletions oauth2_http/java/com/google/auth/oauth2/PKCEProvider.java
Original file line number Diff line number Diff line change
@@ -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.
*
* <p>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();
}
44 changes: 42 additions & 2 deletions oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand All @@ -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;
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -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) {
clundin25 marked this conversation as resolved.
Show resolved Hide resolved
url.put("code_challenge", pkce.getCodeChallenge());
url.put("code_challenge_method", pkce.getCodeChallengeMethod());
}
return url.toURL();
}

Expand Down Expand Up @@ -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) {
clundin25 marked this conversation as resolved.
Show resolved Hide resolved
tokenData.put("code_verifier", pkce.getCodeVerifier());
}

UrlEncodedContent tokenContent = new UrlEncodedContent(tokenData);
HttpRequestFactory requestFactory = transportFactory.create().createRequestFactory();
HttpRequest tokenRequest =
Expand Down Expand Up @@ -430,6 +443,7 @@ public static class Builder {
private URI userAuthUri;
private Collection<String> scopes;
private HttpTransportFactory transportFactory;
private PKCEProvider pkce;

protected Builder() {}

Expand All @@ -441,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) {
Expand Down Expand Up @@ -478,6 +493,20 @@ public Builder setHttpTransportFactory(HttpTransportFactory transportFactory) {
return this;
}

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;
}

public ClientId getClientId() {
return clientId;
}
Expand Down Expand Up @@ -506,9 +535,20 @@ public HttpTransportFactory getHttpTransportFactory() {
return transportFactory;
}

public PKCEProvider getPKCEProvider() {
return pkce;
}

public UserAuthorizer build() {
return new UserAuthorizer(
clientId, scopes, tokenStore, callbackUri, transportFactory, tokenServerUri, userAuthUri);
clientId,
scopes,
tokenStore,
callbackUri,
transportFactory,
tokenServerUri,
userAuthUri,
pkce);
}
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -148,6 +149,7 @@ public void getAuthorizationUrl() throws IOException {
.setScopes(DUMMY_SCOPES)
.setCallbackUri(CALLBACK_URI)
.setUserAuthUri(AUTH_URI)
.setPKCEProvider(pkce)
clundin25 marked this conversation as resolved.
Show resolved Hide resolved
.build();

URL authorizationUrl = authorizer.getAuthorizationUrl(USER_ID, CUSTOM_STATE, BASE_URI);
Expand All @@ -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
Expand Down Expand Up @@ -471,4 +475,33 @@ public void revokeAuthorization_revokesAndClears() throws IOException {
UserCredentials credentials2 = authorizer.getCredentials(USER_ID);
assertNull(credentials2);
}

@Test(expected = IllegalArgumentException.class)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not test all the properties, only one.

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();
}
}