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

Jump to bottom

feat(internal): [AIP-4111] support scopes for self-signed JWT auth flow #1075

Merged
merged 8 commits into from Jun 22, 2021
Merged

feat(internal): [AIP-4111] support scopes for self-signed JWT auth flow #1075

merged 8 commits into from Jun 22, 2021

Conversation

shinfan
Copy link
Contributor

@shinfan shinfan commented Jun 16, 2021 •
edited

  • A self-signed JWT flow will be executed if the following conditions are met:
    • One of the following is true:
      (a) The scope for self-signed JWT flow is enabled (EnableJwtWithScope == true)
      (b) Custom audiences are explicitly provided by users
      (c) No scopes are provided at all
    • No service account impersonation
  • Otherwise, executes standard OAuth 2.0 flow as fallback
  • Following cases will result in error:
    • Neither scopes nor audiences are available with the service account JSON
    • Malformed or empty JSON input

More information for self-signed JWT at: https://google.aip.dev/auth/4111

@shinfan shinfan requested review from yoshi-approver and a team as code owners June 16, 2021 22:24
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jun 16, 2021
andyrzhao
andyrzhao suggested changes Jun 16, 2021
View reviewed changes
internal/settings.go Outdated Show resolved Hide resolved
internal/creds.go Outdated Show resolved Hide resolved
codyoss
codyoss requested changes Jun 17, 2021
View reviewed changes
Copy link
Member

@codyoss codyoss left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, a few suggestions and comments.

option/internaloption/internaloption.go Outdated Show resolved Hide resolved
internal/creds.go Show resolved Hide resolved
internal/creds.go Outdated Show resolved Hide resolved
internal/creds.go Show resolved Hide resolved
internal/creds.go Outdated Show resolved Hide resolved
andyrzhao
andyrzhao reviewed Jun 17, 2021
View reviewed changes
internal/creds.go Outdated
if len(audiences) > 1 {
return nil, fmt.Errorf("multiple audiences support is not implemented")
func isSelfSignedJWTFlow(data []byte, ds *DialSettings) (bool, error) {
if (ds.EnableJwtWithScope || ds.HasCustomAudience()) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, what if EnableJwtWithScope is false and scopes is empty and no custom audience is specified by user? Shouldn't we still try SelfSignedJWTFlow with the default audience?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I don't think in practice the scope will ever be empty since we always provide default scopes in our clients, but it does not harm to do the extra check.

internal/creds.go Show resolved Hide resolved
@shinfan
Copy link
Contributor Author

shinfan commented Jun 17, 2021

Thanks for review. @codyoss @andyrzhao PTAL

andyrzhao
andyrzhao approved these changes Jun 18, 2021
View reviewed changes
Copy link
Contributor

@andyrzhao andyrzhao left a comment

Choose a reason for hiding this comment

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

Looks good overall. Some minor comments.

internal/creds.go Outdated Show resolved Hide resolved
internal/creds.go Show resolved Hide resolved
codyoss
codyoss reviewed Jun 22, 2021
View reviewed changes
internal/creds.go Show resolved Hide resolved
option/internaloption/internaloption.go Outdated Show resolved Hide resolved
codyoss
codyoss approved these changes Jun 22, 2021
View reviewed changes
Copy link
Member

@codyoss codyoss left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for your work on this feature!

@codyoss codyoss changed the title internal: [AIP-4111] support scopes for self-signed JWT auth flow feat(internal): [AIP-4111] support scopes for self-signed JWT auth flow Jun 22, 2021
@codyoss codyoss added the automerge Merge the pull request once unit tests and other checks pass. label Jun 22, 2021
@gcf-merge-on-green gcf-merge-on-green bot merged commit 29cab68 into googleapis:master Jun 22, 2021
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jun 22, 2021
@shinfan shinfan mentioned this pull request Jun 22, 2021
Merged
gcf-merge-on-green bot pushed a commit that referenced this pull request Jun 23, 2021
@release-please
chore: release 0.49.0 (#1061)
6c8f9c5 
🤖 I have created a release \*beep\* \*boop\*
---
## [0.49.0](https://www.github.com/googleapis/google-api-go-client/compare/v0.48.0...v0.49.0) (2021-06-23)


### Features

* **all:** auto-regenerate discovery clients , refs [#1085](https://www.github.com/googleapis/google-api-go-client/issues/1085) [#1084](https://www.github.com/googleapis/google-api-go-client/issues/1084) [#1082](https://www.github.com/googleapis/google-api-go-client/issues/1082) [#1080](https://www.github.com/googleapis/google-api-go-client/issues/1080) [#1079](https://www.github.com/googleapis/google-api-go-client/issues/1079) [#1078](https://www.github.com/googleapis/google-api-go-client/issues/1078) [#1076](https://www.github.com/googleapis/google-api-go-client/issues/1076) [#1073](https://www.github.com/googleapis/google-api-go-client/issues/1073) [#1071](https://www.github.com/googleapis/google-api-go-client/issues/1071) [#1070](https://www.github.com/googleapis/google-api-go-client/issues/1070) [#1068](https://www.github.com/googleapis/google-api-go-client/issues/1068) [#1067](https://www.github.com/googleapis/google-api-go-client/issues/1067) [#1066](https://www.github.com/googleapis/google-api-go-client/issues/1066) [#1064](https://www.github.com/googleapis/google-api-go-client/issues/1064) [#1063](https://www.github.com/googleapis/google-api-go-client/issues/1063) [#1060](https://www.github.com/googleapis/google-api-go-client/issues/1060)
* **internal:** [AIP-4111] support scopes for self-signed JWT auth flow ([#1075](https://www.github.com/googleapis/google-api-go-client/issues/1075)) ([29cab68](https://www.github.com/googleapis/google-api-go-client/commit/29cab68ebd5b9ca017503baf46d91a29f8e84716))

This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
gcf-merge-on-green bot pushed a commit to googleapis/gapic-generator-go that referenced this pull request Jun 23, 2021
@shinfan
gapic: support EnableJwtWithScope for gRPC GAPIC clients (#674)
f6bebb1 
- Update the GAPIC clients to allow scope for self-signed JWT
- The self-signed JWT is now the default authentication method for service account instead of OAuth 2.0
- Self-signed JWT AIP: https://google.aip.dev/auth/4111
- Related PRs:
  -  googleapis/google-api-go-client#1075
  -  golang/oauth2#504
  - aip-dev/google.aip.dev#764
@apstndb apstndb mentioned this pull request Jun 28, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codyoss codyoss codyoss approved these changes

@andyrzhao andyrzhao andyrzhao approved these changes

@yoshi-approver yoshi-approver Awaiting requested review from yoshi-approver

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@shinfan @codyoss @andyrzhao