-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
internal: Refactor cert logic to support OAuth2 token exchange over mTLS #1886
Conversation
@andyrzhao Is this PR related to #1874 in any way? |
Hi Chris, this PR is not related to #1874 and I'm not familiar with the details of s2a PR - I will have to take a look at that one more closely later to see how it fits into the existing google api transport layer and enterprise certificate proxy stack. In the meantime, I've shared you an internal doc for the motivation of this PR 1886 - TLDR: we need this patch to unblock a CAA policy rollout which will effectively disable the oauth2 non-mtls token exchange endpoint for users under the policy restriction. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR moves files that contain publicly exposed functions. This is a breaking change.
That's correct, it moves transport/cert to internal. Do you have a recommendation on how to proceed? Should we add wrapper files to re-expose the affected functions? Technically, the package comment says "This package is not intended for use by end developers. Use the |
@codyoss Can we have some exception for "breaking changes" here? Technically these files are very specific to mTLS use cases and not being used by anyone at this point so the user impact should be negligible. |
I will take a closer look at this package today and get back to you |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After more consideration and looking at our docs I agree, I think this is fine.
With Context Aware Access enabled, users must use the endpoint "https://oauth2.mtls.googleapis.com/token" for token exchange. This PR refactors the cert logic currently used by the transport layer to be reused by the internal credentials layer in order to inject an mTLS-enabled HTTPClient (via the "context" mechanism) for use by the OAuth2 transport (along with the mTLS OAuth2 endpoint if so).