feat: service account is able to use a private token endpoint

[liuchaoren]

In Private Service Connect, users can use an endpoint which is private to their VPC network. The request is eventually routed to the oauth2.googleapis.com/token so the "aud" in the assertion still should be oauth2.googleapis.com/token.

After this change, service account can send requests to the private endpoint (if configured) and still use the oauth2.googleapis.com/token in the assertion.

[busunkim96]

@silvolu @arithmetic1728 PTAL. The corresponding bug is 190841374.

[silvolu]

Please wait for me on this.

[tseaver]

Tagged as do not merge per request of @silvolu.

[silvolu]

The fix should be simpler here: when exchanging a JWT for an access token, the value of the 'aud' field must be https://oauth2.googleapis.com/token, so in _make_authorization_grant_assertion the value should be read from a constant or hardcoded string instead of set to the token uri.

[arithmetic1728]

The fix should be simpler here: when exchanging a JWT for an access token, the value of the 'aud' field must be https://oauth2.googleapis.com/token, so in _make_authorization_grant_assertion the value should be read from a constant or hardcoded string instead of set to the token uri.

@silvolu The token uri is not hardcoded because

(1) google-auth allows users to provide their own token uri (even though it should be always https://oauth2.googleapis.com/token for googleapis)

google-auth-library-python/google/oauth2/service_account.py

Lines 123 to 127 in 22affe1

	def __init__(
	self,
	signer,
	service_account_email,
	token_uri,

(2) if the creds is created from service_account.json file, then google-auth reads token uri from the json file

google-auth-library-python/google/oauth2/service_account.py

Lines 237 to 240 in 22affe1

	info, signer = _service_account_info.from_filename(
	filename, require=["client_email", "token_uri"]
	)
	return cls._from_signer_and_info(signer, info, **kwargs)

[silvolu]

I'm not saying that the token_uri should be hardcoded, I'm saying that the audience field should be hardcoded. This class is not a generic implementation, the description clearly says "This module implements the JWT Profile for OAuth 2.0 Authorization Grants as defined by RFC 7523_ with particular support for how this RFC is implemented in Google's infrastructure."

In Google's infrastructure, when exchanging a JWT for a token, the audience is always https://oauth2.googleapis.com/token. Once that is done, you can actually use different token uris supported by the google infrastructure, e.g. a Private Secure Connect endpoint, because with the audience set to the token_uri, the audience check at the token exchange endpoint fails.

[arithmetic1728]

I'm not saying that the token_uri should be hardcoded, I'm saying that the audience field should be hardcoded. This class is not a generic implementation, the description clearly says "This module implements the JWT Profile for OAuth 2.0 Authorization Grants as defined by RFC 7523_ with particular support for how this RFC is implemented in Google's infrastructure."

In Google's infrastructure, when exchanging a JWT for a token, the audience is always https://oauth2.googleapis.com/token. Once that is done, you can actually use different token uris supported by the google infrastructure, e.g. a Private Secure Connect endpoint, because with the audience set to the token_uri, the audience check at the token exchange endpoint fails.

Sorry I misunderstood your comment. Yea, this way is much simpler.

[liuchaoren]

I updated the PR to hard-code the aud which is much simpler. Please take another look.

[arithmetic1728]

looks good to me

[liuchaoren]

Hi @tseaver, is it okay to remove the "do not merge" tag given that @silvolu approved it? I confirmed with @silvolu yesterday.

[busunkim96]

@liuchaoren Yes I believe Tres added that to prevent an early merge. I've removed the label.

[busunkim96]

@liuchaoren It looks like some unit tests are failing, could you take a look?

To run the unit tests locally, pip install nox and run one of the unit test sessions: nox -s unit-3.6

[gcf-merge-on-green]

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, one of your required reviews was not approved, or there is a do not merge label. 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.