-
Notifications
You must be signed in to change notification settings - Fork 229
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
Conversation
oauth2_http/java/com/google/auth/oauth2/DefaultPKCEProvider.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments
private CodeChallenge codeChallenge; | ||
private static final int MAX_CODE_VERIFIER_LENGTH = 127; | ||
|
||
private class CodeChallenge { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and missing javadoc
@@ -471,4 +475,33 @@ public void revokeAuthorization_revokesAndClears() throws IOException { | |||
UserCredentials credentials2 = authorizer.getCredentials(USER_ID); | |||
assertNull(credentials2); | |||
} | |||
|
|||
@Test(expected = IllegalArgumentException.class) |
There was a problem hiding this comment.
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.
import java.security.SecureRandom; | ||
import java.util.Base64; | ||
|
||
public class DefaultPKCEProvider implements PKCEProvider { |
There was a problem hiding this comment.
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
🤖 I have created a release *beep* *boop* --- ## [1.16.0](https://github.com/googleapis/google-auth-library-java/compare/v1.15.0...v1.16.0) (2023-02-15) ### Features * Add PKCE to 3LO exchange. ([#1146](https://github.com/googleapis/google-auth-library-java/issues/1146)) ([5bf606b](https://github.com/googleapis/google-auth-library-java/commit/5bf606bb8f6d863b44e87587eebf51eaeea4a0ae)) ### Bug Fixes * Create and reuse self signed jwt creds for better performance ([#1154](https://github.com/googleapis/google-auth-library-java/issues/1154)) ([eaaa8e8](https://github.com/googleapis/google-auth-library-java/commit/eaaa8e89cf69d1e0d581443121f315854d52c75f)) * Java doc for DefaultPKCEProvider.java ([#1148](https://github.com/googleapis/google-auth-library-java/issues/1148)) ([154c127](https://github.com/googleapis/google-auth-library-java/commit/154c1279b3ec96cc34a3225e5e78800ccdda927c)) * Removed url pattern validation for google urls in external account credential configurations ([#1150](https://github.com/googleapis/google-auth-library-java/issues/1150)) ([35495b1](https://github.com/googleapis/google-auth-library-java/commit/35495b1207ffe11712ee996d3e305449752fb87c)) ### Documentation * Clarified Maven artifact for HTTP-based clients ([#1136](https://github.com/googleapis/google-auth-library-java/issues/1136)) ([b49fc13](https://github.com/googleapis/google-auth-library-java/commit/b49fc13b10d0e326c7296e2aad7a50ea03e774f5)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
go/client-auth-java-oauth-pkce