feat: support self-signed JWT flow for service accounts #774

busunkim96 · 2021-02-10T17:20:42Z

See RFC (internal only) and https://aip.dev/auth/4111.

Support the self-signed JWT flow for service accounts by passing default_scopes and default_host in calls to the auth library and create_channel. This depends on features exposed in the following PRs: googleapis/python-api-core#134, googleapis/google-auth-library-python#665.

It may be easier to look at https://github.com/googleapis/python-translate/pull/107/files for a diff on a real library.

This change is written so that the library is (temporarily) compatible with older google-api-core and google-auth versions. Because of this it not possible to reach 100% coverage on a single unit test run. pytest runs twice in two of the nox sessions.

Miscellaneous changes:

sprinkled in __init__.py files in subdirs of the test/ directory, as otherwise pytest-cov seems to fail to collect coverage properly in some instances.
new dependency on packaging for Version comparison https://pypi.org/project/packaging/

codecov · 2021-02-10T17:31:05Z

Codecov Report

Merging #774 (93ffdd2) into master (7c185e8) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #774   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           26        27    +1     
  Lines         1608      1653   +45     
  Branches       328       337    +9     
=========================================
+ Hits          1608      1653   +45

Impacted Files	Coverage Δ
gapic/schema/metadata.py	`100.00% <ø> (ø)`
gapic/utils/case.py	`100.00% <ø> (ø)`
gapic/utils/reserved_names.py	`100.00% <ø> (ø)`
gapic/generator/generator.py	`100.00% <100.00%> (ø)`
gapic/schema/api.py	`100.00% <100.00%> (ø)`
gapic/schema/wrappers.py	`100.00% <100.00%> (ø)`
gapic/utils/__init__.py	`100.00% <100.00%> (ø)`
gapic/utils/checks.py	`100.00% <100.00%> (ø)`
gapic/utils/options.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7ca9222...93ffdd2. Read the comment docs.

busunkim96 · 2021-02-24T21:38:50Z

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/base.py.j2

+        if host != self.DEFAULT_HOST and not scopes:
+            scopes = self.AUTH_SCOPES


If port is provided (say pubsub.googleapis.com:443 instead of pubsub.googleapis.com) the library will think the OAuth2 flow is necessary.

I initially tried splitting on : but that broke the showcase tests where default host is localhost. Is there a better way to parse the provided host reliably?

Silly questions: which is the OAuth2 flow, and why do we not want it all the time? Is it the "old style"? What are we trying to parse out? We can always fall back to regexes, something like

host_re = re.compile("(?P<host>[^:]+)(:(?P<port>\d+))?")

Or is that not the part of parsing out the host that's problematic?

Ah sorry for being vague.

The OAuth2 flow (currently always used for service accounts) requires a call out to oauth2.googleapis.com. With a service account, it's sometimes possible to "self-sign" the token, which avoids a network round trip.

The self-signed JWT option can be used when:

A service account is being used

A scope has not been set by the user

A target_audience has not been set by the user (irrelevant as we don't have an client option for this)

An api_endpoint has not been set by the user

Number 4 is what's relevant here. If the user is using the non-default endpoint we want to make sure the auth library uses OAuth2. The suggested way to do this in the doc is to pass user scopes to the auth library.

I think that regex will work! I'll experiment and push a new commit.

software-dov

Wow, this is quite the endeavor. Thanks for doing it!

software-dov · 2021-02-24T23:04:33Z

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/base.py.j2

+        if host != self.DEFAULT_HOST and not scopes:
+            scopes = self.AUTH_SCOPES


Silly questions: which is the OAuth2 flow, and why do we not want it all the time? Is it the "old style"? What are we trying to parse out? We can always fall back to regexes, something like

host_re = re.compile("(?P<host>[^:]+)(:(?P<port>\d+))?")

Or is that not the part of parsing out the host that's problematic?

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2

arithmetic1728 · 2021-02-25T23:02:24Z

FYI, cl/357773081 is submitted, so I guess custom endpoint will no longer be an issue for self signed JWT going forward. @bshaffer

busunkim96 · 2021-03-02T01:16:28Z

@bshaffer Would you be able to take a look?

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/base.py.j2

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc_asyncio.py.j2

bshaffer · 2021-03-08T20:25:57Z

FYI, cl/357773081 is submitted, so I guess custom endpoint will no longer be an issue for self signed JWT going forward. @bshaffer

This is true but it still will be some time before it rolls out. It's up to the owners of this repo if that's acceptable to wait for the rollout or go ahead with the fix we have here. I'd prefer the latter.

arithmetic1728 · 2021-03-08T20:32:01Z

FYI, cl/357773081 is submitted, so I guess custom endpoint will no longer be an issue for self signed JWT going forward. @bshaffer

This is true but it still will be some time before it rolls out. It's up to the owners of this repo if that's acceptable to wait for the rollout or go ahead with the fix we have here. I'd prefer the latter.

Yep, agreed, it is just a FYI. It still takes one or two weeks for the roll out.

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/base.py.j2

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc.py.j2

busunkim96 · 2021-03-11T22:07:33Z

@bshaffer PTAL. :)

software-dov · 2021-03-19T22:48:29Z

Does @bshaffer need to take a look, or is this good to merge?

busunkim96 · 2021-03-19T23:21:00Z

@bshaffer or @arithmetic1728 would you mind taking another look?

Following guidance in https://docs.google.com/document/d/1FoJPYQu8ZHdGBFY9TBRZCDrWJJnxw3fAz5CG5mdZR9E/edit

busunkim96

@bshaffer I removed the scope override with custom endpoints to match guidance in https://docs.google.com/document/d/1FoJPYQu8ZHdGBFY9TBRZCDrWJJnxw3fAz5CG5mdZR9E/edit. Could you take a look?

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/base.py.j2

busunkim96 · 2021-04-15T18:22:51Z

@software-dov Could you take one more look? Thank you for your help getting this over the line. :)

bshaffer

One question I still have is, unfortunately, for the case of the regional endpoint us-documentai.googleapis.com, we need a check to make sure the DEFAULT_HOST doesn't contain a regional endpoint.

If this is something we should do here, or somewhere else, I am not sure, I just want to verify it's being considered!

bshaffer · 2021-04-16T16:43:53Z

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/base.py.j2

@@ -99,13 +99,13 @@ class {{ service.name }}Transport(abc.ABC):
        scopes_kwargs = self._get_scopes_kwargs(self._host, scopes)

        # Save the scopes.
-        self._scopes = scopes_kwargs["scopes"]
+        self._scopes = scopes or self.AUTH_SCOPES


If I'm understanding this correctly, scopes_kwargs["scopes"] was essentially scopes but with a check for if the endpoint was supplied. And I am interpreting scopes or self.AUTH_SCOPES to mean "user supplied scopes or the default scopes"

Is that accurate?

Yep! Since the endpoint restriction has been lifted I went back to "user supplied scopes or the default scopes"

bshaffer · 2021-04-16T16:44:42Z

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/base.py.j2

@@ -120,19 +120,7 @@ class {{ service.name }}Transport(abc.ABC):
        self._credentials = credentials


-    @classmethod
-    def _get_user_scopes(cls, host: str, scopes: Optional[Sequence[str]]) -> Optional[Sequence[str]]:


busunkim96 · 2021-04-16T23:04:50Z

One question I still have is, unfortunately, for the case of the regional endpoint us-documentai.googleapis.com, we need a check to make sure the DEFAULT_HOST doesn't contain a regional endpoint.

@bshaffer From reading the doc it seems like documentai is the exception, so I am planning on handling that with a synthtool/owlbot replace. If we anticipate more APIs using the regional endpoints as the default host I can build logic into the generator.

bshaffer · 2021-04-16T23:07:55Z

That sounds great!

🤖 I have created a release \*beep\* \*boop\* --- ## [0.44.0](https://www.github.com/googleapis/gapic-generator-python/compare/v0.43.3...v0.44.0) (2021-04-23) ### Features * support self-signed JWT flow for service accounts ([#774](https://www.github.com/googleapis/gapic-generator-python/issues/774)) ([89d6f35](https://www.github.com/googleapis/gapic-generator-python/commit/89d6f35c54b0a9b81c9b5f580d2e9eb87352ed93)) ### Bug Fixes * enable GAPIC metadata generation ([#843](https://www.github.com/googleapis/gapic-generator-python/issues/843)) ([697816c](https://www.github.com/googleapis/gapic-generator-python/commit/697816ce7d5b201d6ced85fadd89f9140da67b37)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 10, 2021

busunkim96 force-pushed the self-signed-jwt branch from d696ce3 to f154176 Compare February 24, 2021 00:52

busunkim96 commented Feb 24, 2021

View reviewed changes

busunkim96 marked this pull request as ready for review February 24, 2021 21:39

busunkim96 requested review from lukesneeringer and a team as code owners February 24, 2021 21:39

busunkim96 requested review from software-dov, bshaffer and arithmetic1728 February 24, 2021 21:40

software-dov reviewed Feb 24, 2021

View reviewed changes

arithmetic1728 approved these changes Feb 25, 2021

View reviewed changes

software-dov approved these changes Mar 2, 2021

View reviewed changes

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/base.py.j2 Outdated Show resolved Hide resolved

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/grpc_asyncio.py.j2 Outdated Show resolved Hide resolved

bshaffer reviewed Mar 8, 2021

View reviewed changes

busunkim96 added 10 commits March 11, 2021 00:34

feat: support self-signed JWT flow for servie accounts

1002828

fix: fix mypy and iam tests

96074bd

fix: remove py version req for lower bound checks

a5e50d5

fix: set fail-under to 100

0fa39d7

fix: standardize scope / default scopes

f227371

fix: use self.host in rest transport

04e0485

fix: use host regex for comparison

a414ae3

fix: resolve mypy issues

2b5a7ab

fix: fix test

0c95d0d

refactor: simplify code after rebase

38c3b18

busunkim96 force-pushed the self-signed-jwt branch from 75472e0 to 38c3b18 Compare March 11, 2021 21:36

fix: fix rest operations client

49ecf11

busunkim96 requested review from arithmetic1728 and software-dov March 11, 2021 21:46

Merge branch 'master' into self-signed-jwt

b3409cf

software-dov approved these changes Mar 11, 2021

View reviewed changes

busunkim96 mentioned this pull request Apr 14, 2021

chore: use trim_blocks and lstrip_blocks for jinja templates #838

Merged

fix: use default_host even with custom endpoints

c85fdd4

Following guidance in https://docs.google.com/document/d/1FoJPYQu8ZHdGBFY9TBRZCDrWJJnxw3fAz5CG5mdZR9E/edit

busunkim96 commented Apr 14, 2021

View reviewed changes

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/base.py.j2 Show resolved Hide resolved

Merge branch 'master' into self-signed-jwt

b284a46

busunkim96 requested a review from software-dov April 15, 2021 18:22

bshaffer reviewed Apr 16, 2021

View reviewed changes

Merge branch 'master' into self-signed-jwt

93ffdd2

bshaffer approved these changes Apr 16, 2021

View reviewed changes

software-dov approved these changes Apr 21, 2021

View reviewed changes

busunkim96 merged commit 89d6f35 into master Apr 21, 2021

busunkim96 deleted the self-signed-jwt branch April 21, 2021 18:55

busunkim96 mentioned this pull request May 19, 2021

fix: remove support for google-api-core<1.26.0 #893

Merged

tseaver mentioned this pull request Jun 24, 2021

feat: add always_use_jwt_access googleapis/python-bigquery#714

Merged

This was referenced Jan 18, 2022

chore(master): release reverted #1133

Merged

chore(master): release 0.40.0 #1138

Closed

chore(master): release 0.39.1 #1146

Closed

This was referenced Jan 26, 2022

chore(main): release 0.39.1 #1160

Closed

chore(main): release 0.39.1 #1162

Closed

chore(main): release 0.61.0 #1163

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support self-signed JWT flow for service accounts #774

feat: support self-signed JWT flow for service accounts #774

busunkim96 commented Feb 10, 2021 •

edited

Loading

codecov bot commented Feb 10, 2021 •

edited

Loading

busunkim96 Feb 24, 2021

software-dov Feb 24, 2021 •

edited

Loading

busunkim96 Feb 25, 2021

software-dov left a comment

software-dov Feb 24, 2021 •

edited

Loading

arithmetic1728 commented Feb 25, 2021 •

edited

Loading

busunkim96 commented Mar 2, 2021

bshaffer commented Mar 8, 2021

arithmetic1728 commented Mar 8, 2021

busunkim96 commented Mar 11, 2021

software-dov commented Mar 19, 2021

busunkim96 commented Mar 19, 2021

busunkim96 left a comment

busunkim96 commented Apr 15, 2021

bshaffer left a comment

bshaffer Apr 16, 2021

busunkim96 Apr 16, 2021

bshaffer Apr 16, 2021

busunkim96 commented Apr 16, 2021

bshaffer commented Apr 16, 2021 •

edited

Loading

		if host != self.DEFAULT_HOST and not scopes:
		scopes = self.AUTH_SCOPES

feat: support self-signed JWT flow for service accounts #774

feat: support self-signed JWT flow for service accounts #774

Conversation

busunkim96 commented Feb 10, 2021 • edited Loading

codecov bot commented Feb 10, 2021 • edited Loading

Codecov Report

busunkim96 Feb 24, 2021

Choose a reason for hiding this comment

software-dov Feb 24, 2021 • edited Loading

Choose a reason for hiding this comment

busunkim96 Feb 25, 2021

Choose a reason for hiding this comment

software-dov left a comment

Choose a reason for hiding this comment

software-dov Feb 24, 2021 • edited Loading

Choose a reason for hiding this comment

arithmetic1728 commented Feb 25, 2021 • edited Loading

busunkim96 commented Mar 2, 2021

bshaffer commented Mar 8, 2021

arithmetic1728 commented Mar 8, 2021

busunkim96 commented Mar 11, 2021

software-dov commented Mar 19, 2021

busunkim96 commented Mar 19, 2021

busunkim96 left a comment

Choose a reason for hiding this comment

busunkim96 commented Apr 15, 2021

bshaffer left a comment

Choose a reason for hiding this comment

bshaffer Apr 16, 2021

Choose a reason for hiding this comment

busunkim96 Apr 16, 2021

Choose a reason for hiding this comment

bshaffer Apr 16, 2021

Choose a reason for hiding this comment

busunkim96 commented Apr 16, 2021

bshaffer commented Apr 16, 2021 • edited Loading

busunkim96 commented Feb 10, 2021 •

edited

Loading

codecov bot commented Feb 10, 2021 •

edited

Loading

software-dov Feb 24, 2021 •

edited

Loading

software-dov Feb 24, 2021 •

edited

Loading

arithmetic1728 commented Feb 25, 2021 •

edited

Loading

bshaffer commented Apr 16, 2021 •

edited

Loading