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: Backwards-Compatible Auth #1624

Closed
wants to merge 21 commits into from
Closed

Conversation

danielbankhead
Copy link
Contributor

@danielbankhead danielbankhead commented Jul 28, 2023

This will greatly improve the experience for customers transitioning between different versions of google-auth-library.

Consumers can use the new GoogleAuth.normalize and AuthClient.normalize APIs to ensure the provided GoogleAuth and AuthClient instances are compatible with the current google-auth-library version.

Example:

// say this depends on an older version of `google-auth-library` (e.g. `v8.9.0`)
import { google } from 'googleapis';

// say this is the latest version of `google-auth-library` (e.g. `v9.0.0`)
import { GoogleAuth } from 'google-auth-library';

const auth = new GoogleAuth();

// this used to fail, but should work after internally using `instanceof GoogleAuth`.
google.options({auth});

Fixes #1402 🦕

@danielbankhead danielbankhead requested review from a team as code owners July 28, 2023 21:03
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Jul 28, 2023
@danielbankhead danielbankhead changed the title featBackwards compatible auth feat: Backwards compatible auth Jul 28, 2023
@danielbankhead danielbankhead changed the title feat: Backwards compatible auth feat: Backwards-compatible Auth Jul 28, 2023
@danielbankhead danielbankhead changed the title feat: Backwards-compatible Auth feat: Backwards-Compatible Auth Jul 28, 2023
@danielbankhead danielbankhead added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 31, 2023
@danielbankhead
Copy link
Contributor Author

do not merge added until internal proposal has been approved.

@danielbankhead danielbankhead added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Oct 20, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 20, 2023

Choose a reason for hiding this comment

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

is this related to the changes here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not really; noticed it was an unused file that we can remove

@@ -28,7 +28,6 @@ describe('jwt', () => {
const keypair = require('keypair');
const PEM_PATH = './test/fixtures/private.pem';
const PEM_CONTENTS = fs.readFileSync(PEM_PATH, 'utf8');
const P12_PATH = './test/fixtures/key.p12';

Choose a reason for hiding this comment

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

same here, was it just an unused value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, an unused value

Copy link

@ruyadorno ruyadorno left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@danielbankhead danielbankhead added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 20, 2023
@danielbankhead danielbankhead removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 20, 2023
@danielbankhead danielbankhead added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Nov 15, 2023
@danielbankhead
Copy link
Contributor Author

This would introduce more complexity than is required to solve this problem; instead, we should encourage dependencies to expose their version of google-auth-library so that customers can have a guaranteed, consistently working version.

@danielbankhead danielbankhead removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium.
Projects
None yet
4 participants