feat: add PKCE support to AuthorizationCodeFlow #470

StFS · 2020-05-29T17:10:27Z

feat: Add PKCE support to the AuthorizationCodeFlow

According to the OAuth 2.0 for Native Apps RFC, public native clients MUST implement PKCE (https://tools.ietf.org/html/rfc8252#section-6). It is therefor important to add this mechanism to the google-oauth-java-client library.

[✓] Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
[✓] Ensure the tests and linter pass
[✓] Code coverage does not decrease (if any source code was changed)
[✓] Appropriate docs were updated (if necessary)

Fixes #469 ☕️

…nt into pkce-support

…class. Add documentation.

…zation url class.

…or code sample.

…nt into pkce-support

chingor13

Thanks for submitting this. The overall approach seems ok to me.

...auth-client/src/main/java/com/google/api/client/auth/oauth2/AuthorizationCodeRequestUrl.java

google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/AuthorizationCodeFlow.java

...mdline-sample/src/main/java/com/google/api/services/samples/keycloak/cmdline/PKCESample.java

google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/AuthorizationCodeFlow.java

StFS · 2020-06-08T19:57:38Z

@chingor13 I think I've responded to all your comments and resolved them.

Thank you again so much for taking the time to review this and thanks for your excellent comments. Please let me know if anything else is needed. Should I update the versions.txt file to bump the version or is that just handled by you when you release a new version?

google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/AuthorizationCodeFlow.java

samples/keycloak-pkce-cmdline-sample/README.md

elharo · 2020-06-08T20:21:37Z

samples/keycloak-pkce-cmdline-sample/README.md

+**Prerequisites:** install [Java 6 or higher][install-java], [git][install-git], and
+[Maven][install-maven]. You may need to set your `JAVA_HOME`.
+
+1. Check out the sample code:


Not sure what keycloak is, or if we can include this part in a google project

ahh... that had not occurred to me. Keycloak is a JBoss (Red Hat) project, it's an "open source identity and access management solution" (https://www.keycloak.org/) and it's released under the Apache 2.0 license.
So, as such there shouldn't be a problem using it, especially since it's only being used for the sample demonstration and not as an actual component. But maybe there is some other more political/legal reason for it not being possible. I could remove the sample but I would like to keep it as it's pretty good and clear for the purpose of testing and demonstration.

Not a license issue, but we would like the docs for this product to be self-contained.

gotcha. However, the daily motion example isn't exactly self-contained is is? It actually depends on the user registering an account with dailymotion.com.

But if using keycloak is not acceptable, can you suggest an alternative or should I remove the sample completely?

@elharo any thoughts on this?

samples/keycloak-pkce-cmdline-sample/pom.xml

StFS · 2020-06-11T11:51:40Z

@chingor13 what are the next steps here? I believe I've addressed everything you and @elharo have pointed out. The only still outstanding question is whether we can include the PKCE sampe project because it requires the user to run Keycloak in a docker container. I can remove it, but as I said, compared to the other example which requires a user to register with dailymotion.com I would argue that the PKCE sample is more self contained that that.

But mainly I'm eager to get this merged and into a new release so I don't have to create my own custom patch version of the library for us to use ;-)

chingor13

Sorry for the delay in reviewing again. Keycloak looks fine to me. The only change we need to make is to ensure the source files have license headers. You can duplicate the other source files for examples.

...mdline-sample/src/main/java/com/google/api/services/samples/keycloak/cmdline/PKCESample.java

samples/keycloak-pkce-cmdline-sample/scripts/initialize-keycloak.sh

StFS · 2020-06-28T14:27:45Z

@chingor13 just another friendly nudge to get this merged :)

StFS · 2020-06-29T20:29:44Z

Thanks @chingor13

Just two questions:

When would you expect a new version to be released?
You commented on the PR that this should be a bump on the minor version since new API is being introduced. Do you keep track of that somewhere (that the next release should be a minor release, not a patch release)?

🤖 I have created a release \*beep\* \*boop\* --- ## [1.31.0](https://www.github.com/googleapis/google-oauth-java-client/compare/v1.30.6...v1.31.0) (2020-06-29) ### Features * add PKCE support to AuthorizationCodeFlow ([#470](https://www.github.com/googleapis/google-oauth-java-client/issues/470)) ([13433cd](https://www.github.com/googleapis/google-oauth-java-client/commit/13433cd7dd06267fc261f0b1d4764f8e3432c824)) ### Dependencies * update google-http-client to v1.35.0 ([#466](https://www.github.com/googleapis/google-oauth-java-client/issues/466)) ([6447917](https://www.github.com/googleapis/google-oauth-java-client/commit/6447917c657a5ae4267afbab74dfdb890bbfbf28)) * update to guava 29.0-android ([#456](https://www.github.com/googleapis/google-oauth-java-client/issues/456)) ([fc75233](https://www.github.com/googleapis/google-oauth-java-client/commit/fc752336af9cbdb9b2ed816a63d7bd3d8d1e2778)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).

StFS and others added 15 commits May 26, 2020 21:53

Initial test code for a PKCE enabled Authorization Code Flow

61a616e

WIP: work on README.md

4bf2925

Script to initialize keycloak by adding client via REST API.

778797b

Improve keycloak init script and some code cleanup. Still WIP.

73fa760

Merge branch 'pkce-support' of github.com:StFS/google-oauth-java-clie…

b0f0a9f

…nt into pkce-support

WIP: work on README.md

8b5e316

Working PKCE AuthorizationCodeFlow. Some cleanup of test classes.

3bb51aa

Add scopes back to the AuthorizationCodeRequestUrl creation.

b582b87

Simplify code by moving PKCE entirely into the AuthorizationCodeFlow …

7238b21

…class. Add documentation.

Remove wildcard imports as that seems to be the way to do things here.

c5def75

Add @SInCE annotation in JavaDoc to the PKCE parameters of the autori…

b089e33

…zation url class.

Add PKCE unit test, documentation and minor cleanup of dependencies f…

066cadc

…or code sample.

Add PKCE unit test, documentation and minor cleanup of dependencies f…

7272beb

…or code sample.

Merge branch 'pkce-support' of github.com:StFS/google-oauth-java-clie…

bdb43ac

…nt into pkce-support

Annotate PKCE with Beta annotation.

f2efde6

StFS requested a review from a team as a code owner May 29, 2020 17:10

googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 29, 2020

chingor13 changed the title ~~Add PKCE support to AuthorizationCodeFlow~~ feat: add PKCE support to AuthorizationCodeFlow Jun 8, 2020

chingor13 requested changes Jun 8, 2020

View reviewed changes

chingor13 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 8, 2020

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 8, 2020

Responding to code review comments

1f0e9b8

elharo reviewed Jun 8, 2020

View reviewed changes

StFS added 2 commits June 8, 2020 22:11

Responding to more PR comments

d3e7b47

Improve Keycloak PKCE sample documentation

1ee31a0

chingor13 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 19, 2020

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 19, 2020

chingor13 requested changes Jun 19, 2020

View reviewed changes

...mdline-sample/src/main/java/com/google/api/services/samples/keycloak/cmdline/PKCESample.java Show resolved Hide resolved

samples/keycloak-pkce-cmdline-sample/scripts/initialize-keycloak.sh Show resolved Hide resolved

chingor13 approved these changes Jun 29, 2020

View reviewed changes

chingor13 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 29, 2020

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 29, 2020

chingor13 merged commit 13433cd into googleapis:master Jun 29, 2020

StFS deleted the pkce-support branch June 30, 2020 09:50

mend-bolt-for-github bot mentioned this pull request Jul 14, 2020

CVE-2020-7692 (Critical) detected in google-oauth-client-1.30.5.jar mgh3326/google-calendar-slackbot#23

Open

mend-for-github-com bot mentioned this pull request Oct 6, 2020

CVE-2020-7692 (High) detected in google-oauth-client-1.20.0.jar LevyForchh/java-asana#3

Open

1 task

mend-bolt-for-github bot mentioned this pull request Oct 14, 2020

CVE-2020-7692 (High) detected in google-oauth-client-1.30.1.jar - autoclosed jainisking/MyTest#24

Closed

chingor13 mentioned this pull request Oct 14, 2020

when will be the release of the new version 1.31.1? #537

Closed

mend-bolt-for-github bot mentioned this pull request Oct 19, 2020

CVE-2020-7692 (High) detected in google-oauth-client-1.28.0.jar nakasho-dev/conference-app-2020#2

Open

mend-for-github-com bot mentioned this pull request Dec 15, 2020

CVE-2020-7692 (High) detected in google-oauth-client-1.25.0.jar - autoclosed bitbar/testdroid-api#400

Closed

This was referenced Dec 22, 2020

CVE-2020-7692 (High) detected in google-oauth-client-1.21.0.jar SmartBear/readyapi4j#129

Open

CVE-2020-7692 (High) detected in google-oauth-client-1.15.0-rc.jar bitbar/test-samples#213

Closed

mend-for-github-com bot mentioned this pull request Dec 31, 2020

CVE-2020-7692 (High) detected in google-oauth-client-1.21.0.jar - autoclosed SmartBear/ready-mqtt-plugin#25

Closed

This was referenced Jan 6, 2021

CVE-2020-7692 (Critical) detected in google-oauth-client-1.20.0.jar rdharmedran/codeguru#4

Open

CVE-2020-7692 (High) detected in google-oauth-client-1.20.0.jar guardian/amigo#409

Closed

mend-bolt-for-github bot mentioned this pull request Mar 18, 2021

CVE-2020-7692 (Critical) detected in google-oauth-client-1.23.0.jar gsylvie/t#100

Open

mend-for-github-com bot mentioned this pull request Mar 25, 2021

CVE-2020-7692 (High) detected in multiple libraries jmacwhitesource/cloud-pipeline#153

Open

This was referenced May 4, 2021

[Dependencies] Update dependencies with CVEs. opensearch-project/OpenSearch#646

Closed

Upgrade dependencies to mitigate multiple CVEs opensearch-project/OpenSearch#657

Merged

mend-bolt-for-github bot mentioned this pull request Jun 16, 2021

CVE-2020-7692 (High) detected in google-oauth-client-1.21.0.jar tt9133github/jetty.project#32

Open

release-please bot mentioned this pull request Jan 4, 2022

chore(main): release google-oauth-java-client 1.33.0 #791

Closed

SamHeadrickCx mentioned this pull request Mar 14, 2023

CVE-2020-7692 @ Maven-com.google.oauth-client:google-oauth-client-1.30.3 SamHeadrickCx/easybuggy4sb#81

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add PKCE support to AuthorizationCodeFlow #470

feat: add PKCE support to AuthorizationCodeFlow #470

StFS commented May 29, 2020 •

edited

Loading

chingor13 left a comment

StFS commented Jun 8, 2020

elharo Jun 8, 2020

StFS Jun 8, 2020

elharo Jun 8, 2020

StFS Jun 8, 2020

StFS Jun 10, 2020

StFS commented Jun 11, 2020

chingor13 left a comment

StFS commented Jun 28, 2020

StFS commented Jun 29, 2020

feat: add PKCE support to AuthorizationCodeFlow #470

feat: add PKCE support to AuthorizationCodeFlow #470

Conversation

StFS commented May 29, 2020 • edited Loading

chingor13 left a comment

Choose a reason for hiding this comment

StFS commented Jun 8, 2020

elharo Jun 8, 2020

Choose a reason for hiding this comment

StFS Jun 8, 2020

Choose a reason for hiding this comment

elharo Jun 8, 2020

Choose a reason for hiding this comment

StFS Jun 8, 2020

Choose a reason for hiding this comment

StFS Jun 10, 2020

Choose a reason for hiding this comment

StFS commented Jun 11, 2020

chingor13 left a comment

Choose a reason for hiding this comment

StFS commented Jun 28, 2020

StFS commented Jun 29, 2020

StFS commented May 29, 2020 •

edited

Loading