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(internaloption): add better support for self-signed JWT #738

Merged
merged 5 commits into from Dec 2, 2020

Conversation

@codyoss
Copy link
Member

@codyoss codyoss commented Nov 3, 2020

Added a couple of new internaloptions, WithDefaultAudience and
WithDefaultScopes, which will be used to start using self-signed
JWTs by default when certain criteria are met:

  1. Authenticating with a service account.
  2. User does not explicitly provide their own scopes.
  3. An audience is provided -- either by default from a client or
    by the user.

In the future gapic clients will begin to pass these new
internaloptions which will enable them to authenticate with with
a JWT signed by a service account. This is a non-standard oAuth2
flow and is an optimization to save an extra network request.

Added a couple of new internaloptions, WithDefaultAudience and
WithDefaultScopes, which will be used to start using self-signed
JWTs by default when certain criteria are met:

1. Authenticating with a service account.
2. User does not pass explicitly provide their own scopes.
3. An audience is provided.

In the future gapic client will begin to pass these new
internaloptions which will enable them to authenticate with with
a JWT signed by a service account. This is a non-standard oAuth2
flow, and is an optimization to save an extra network request.
@codyoss codyoss requested a review from googleapis/yoshi-go-admins as a code owner Nov 3, 2020
@google-cla google-cla bot added the cla: yes label Nov 3, 2020
@codyoss codyoss requested a review from bshaffer Nov 3, 2020
codyoss added a commit to codyoss/gapic-generator-go that referenced this pull request Nov 3, 2020
This option will be used to generate self-signed JWTs when the
client is authenticated with a service account and the user does
not provide additional scopes.

Also, now marking the generated scopes as default so we can
distinguish between generated and user provided scopes.

Related: googleapis/google-api-go-client#738
internal/creds.go Show resolved Hide resolved
@tbpg
tbpg approved these changes Nov 10, 2020
@tbpg tbpg added the automerge label Nov 10, 2020
@gcf-merge-on-green
Copy link
Contributor

@gcf-merge-on-green gcf-merge-on-green bot commented Nov 11, 2020

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, or one of your required reviews was not approved. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

1 similar comment
@gcf-merge-on-green
Copy link
Contributor

@gcf-merge-on-green gcf-merge-on-green bot commented Nov 11, 2020

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, or one of your required reviews was not approved. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@codyoss codyoss merged commit 1a7550f into googleapis:master Dec 2, 2020
3 checks passed
3 checks passed
@google-cla
cla/google All necessary CLAs are signed
@conventional-commit-lint-gcf
conventionalcommits.org
Details
@kokoro-team
kokoro Kokoro CI build successful
Details
@codyoss codyoss deleted the codyoss:new-opts branch Dec 2, 2020
codyoss added a commit to googleapis/gapic-generator-go that referenced this pull request Dec 3, 2020
This option will be used to generate self-signed JWTs when the
client is authenticated with a service account and the user does
not provide additional scopes.

Also, now marking the generated scopes as default so we can
distinguish between generated and user provided scopes.

Related: googleapis/google-api-go-client#738
myusuf3 added a commit to agilebits/google-api-go-client that referenced this pull request Dec 4, 2020
…pis#738)

Added a couple of new internaloptions, WithDefaultAudience and
WithDefaultScopes, which will be used to start using self-signed
JWTs by default when certain criteria are met:

1. Authenticating with a service account.
2. User does not pass explicitly provide their own scopes.
3. An audience is provided.

In the future gapic client will begin to pass these new
internaloptions which will enable them to authenticate with with
a JWT signed by a service account. This is a non-standard oAuth2
flow, and is an optimization to save an extra network request.
@bendiknesbo
Copy link

@bendiknesbo bendiknesbo commented Dec 10, 2020

@bshaffer @tbpg @codyoss
This PR broke our builds!

(ds.DefaultAudience != "" || len(ds.Audiences) > 0)

The above change appears to be the faulting change.

Using cloud.google.com/go v0.72.0 + google.golang.org/api v0.35.0 works. Upgrading either with 1 minor version causes runtime errors.

Reproducing code:

opt := option.WithCredentialsFile("path/to/svc-acc.json")
validator, err := idtoken.NewValidator(ctx, opt)
validator.Validate(ctx, "some-token", "some-audience")

The fault happens during idtoken.NewValidator, where the self-signing JWT token source is not set, due to the changes in this PR. There is no error returned from running NewValidator. When running validator.Validate, an error occurs during the roundtrip when retrieving the certificates from google.

@codyoss
Copy link
Member Author

@codyoss codyoss commented Dec 10, 2020

@bendiknesbo Thanks for the report, I will look into this and get a patch out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants