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

feat: Add PKCE to 3LO exchange. #1146

merged 5 commits into from Feb 6, 2023

Conversation

clundin25
Copy link
Contributor

@clundin25 clundin25 commented Feb 3, 2023

go/client-auth-java-oauth-pkce

@clundin25 clundin25 requested a review from a team as a code owner February 3, 2023 00:50
@product-auto-label product-auto-label bot added the size: l Pull request size is large. label Feb 3, 2023
Copy link

@wangyutongg wangyutongg left a comment

Choose a reason for hiding this comment

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

LGTM

@clundin25 clundin25 merged commit 5bf606b into googleapis:main Feb 6, 2023
Copy link
Member

@TimurSadykov TimurSadykov left a 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 {
Copy link
Member

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
Member

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)
Copy link
Member

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 {
Copy link
Member

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

gcf-merge-on-green bot pushed a commit that referenced this pull request Feb 15, 2023
🤖 I have created a release *beep* *boop*
---


## [1.16.0](https://togithub.com/googleapis/google-auth-library-java/compare/v1.15.0...v1.16.0) (2023-02-15)


### Features

* Add PKCE to 3LO exchange. ([#1146](https://togithub.com/googleapis/google-auth-library-java/issues/1146)) ([5bf606b](https://togithub.com/googleapis/google-auth-library-java/commit/5bf606bb8f6d863b44e87587eebf51eaeea4a0ae))


### Bug Fixes

* Create and reuse self signed jwt creds for better performance ([#1154](https://togithub.com/googleapis/google-auth-library-java/issues/1154)) ([eaaa8e8](https://togithub.com/googleapis/google-auth-library-java/commit/eaaa8e89cf69d1e0d581443121f315854d52c75f))
* Java doc for DefaultPKCEProvider.java ([#1148](https://togithub.com/googleapis/google-auth-library-java/issues/1148)) ([154c127](https://togithub.com/googleapis/google-auth-library-java/commit/154c1279b3ec96cc34a3225e5e78800ccdda927c))
* Removed url pattern validation for google urls in external account credential configurations ([#1150](https://togithub.com/googleapis/google-auth-library-java/issues/1150)) ([35495b1](https://togithub.com/googleapis/google-auth-library-java/commit/35495b1207ffe11712ee996d3e305449752fb87c))


### Documentation

* Clarified Maven artifact for HTTP-based clients ([#1136](https://togithub.com/googleapis/google-auth-library-java/issues/1136)) ([b49fc13](https://togithub.com/googleapis/google-auth-library-java/commit/b49fc13b10d0e326c7296e2aad7a50ea03e774f5))

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants