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: support self-signed jwt in requests and urllib3 transports #679

Merged
merged 3 commits into from Feb 3, 2021

Conversation

busunkim96
Copy link
Contributor

Follow up to #665.

Allow self-signed JWT flow to be used in the requests and urllib3 transports if a default_host is provided. I don't think any of our clients wrap the urllib3 transport, but a good number of handwritten libraries (Storage for example) uses AuthorizedSession from requests.

@busunkim96 busunkim96 requested a review from a team as a code owner February 3, 2021 02:42
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 3, 2021
Comment on lines +21 to +40
def test_authorized_session_with_service_account_and_self_signed_jwt():
credentials, project_id = google.auth.default()

credentials = credentials.with_scopes(
scopes=[],
default_scopes=["https://www.googleapis.com/auth/pubsub"],
)

session = google.auth.transport.requests.AuthorizedSession(
credentials=credentials, default_host="pubsub.googleapis.com"
)

# List Pub/Sub Topics through the REST API
# https://cloud.google.com/pubsub/docs/reference/rest/v1/projects.topics/list
response = session.get("https://pubsub.googleapis.com/v1/projects/{}/topics".format(project_id))
response.raise_for_status()

# Check that self-signed JWT was created and is being used
assert credentials._jwt_credentials is not None
assert credentials._jwt_credentials.token == credentials.token
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I initially tried to write this test with the Storage API, but requests seemed to fail with invalid credentials (a test with the service account passed).

Are there any APIs where the self-signed JWT approach won't work? @bshaffer

@@ -313,6 +314,9 @@ def my_cert_callback():
refreshing credentials. If not passed,
an instance of :class:`~google.auth.transport.requests.Request`
is created.
default_host (Optional[str]): A host like "pubsub.googleapis.com".
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a little bit concern about setting the default_host in ctor. There are 2 default endpoints, for instance, pubsub.googleapis.com and pubsub.mtls.googleapis.com. mtls has auto switch logic to switch from the regular endpoint to mtls endpoint. It works as follows:

(1) client creates an AuthorizedSession
(2) client calls AuthorizedSession.is_mtls (with other conditions like the GOOGLE_API_USE_CLIENT_CERTIFICATE env var) to decide which endpoint to use

So this means in the AuthorizedSession ctor, we may not know exactly which default endpoint will be used. So if both endpoints use the regular endpoint as the audience, then this code works fine. Otherwise, we need to find a way.

@bshaffer

Copy link
Contributor

Choose a reason for hiding this comment

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

mtls only works with user credentials, so this should be fine since service account credentials will be rejected by the server anyway.

Copy link
Contributor

@arithmetic1728 arithmetic1728 left a comment

Choose a reason for hiding this comment

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

LGTM

@busunkim96 busunkim96 changed the title feat: support self-signed-jwt in requests and urllib3 transports feat: support self-signed jwt in requests and urllib3 transports Feb 3, 2021
@busunkim96 busunkim96 changed the title feat: support self-signed jwt in requests and urllib3 transports feat: support self-signed jwt in requests and urllib3 transports Feb 3, 2021
@busunkim96 busunkim96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 3, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 3, 2021
@busunkim96 busunkim96 merged commit 7a94acb into master Feb 3, 2021
@busunkim96 busunkim96 deleted the self-signed-jwt-requests branch February 3, 2021 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants