Skip to content
This repository was archived by the owner on Nov 20, 2025. It is now read-only.

feat: Backwards-Compatible Auth#1624

Closed
danielbankhead wants to merge 21 commits intomainfrom
backwards-compatible-auth
Closed

feat: Backwards-Compatible Auth#1624
danielbankhead wants to merge 21 commits intomainfrom
backwards-compatible-auth

Conversation

@danielbankhead
Copy link
Contributor

@danielbankhead danielbankhead commented Jul 28, 2023 •
edited
Loading

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 a review from a team as a code owner July 28, 2023 21:03
@danielbankhead danielbankhead requested a review from a team 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 mentioned this pull request Jul 28, 2023
Closed
@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

danielbankhead commented Jul 31, 2023

do not merge added until internal proposal has been approved.

@ruyadorno ruyadorno mentioned this pull request Oct 10, 2023
Closed
@danielbankhead danielbankhead mentioned this pull request Oct 13, 2023
Closed
@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
ruyadorno
ruyadorno reviewed Oct 20, 2023
View reviewed changes
test/fixtures/key.p12
Copy link

@ruyadorno ruyadorno 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

@danielbankhead danielbankhead Oct 20, 2023

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

test/test.jwt.ts
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';
Copy link

@ruyadorno ruyadorno Oct 20, 2023

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

@danielbankhead danielbankhead Oct 20, 2023

Choose a reason for hiding this comment

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

Yep, an unused value

ruyadorno
ruyadorno approved these changes Oct 20, 2023
View reviewed changes
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

danielbankhead commented Mar 13, 2024

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 subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@ruyadorno ruyadorno ruyadorno approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size: m Pull request size is medium.

Projects

None yet

Milestone

No milestone

4 participants

@danielbankhead @ruyadorno @yoshi-kokoro @d-goog