search

stephenplusplus / google-auto-auth

Making it as easy as possible to authenticate a Google API request
MIT License
34 stars 9 forks source link

Concerns about PR #34 #35

Closed nolanmar511 closed 6 years ago

nolanmar511 commented 6 years ago

I'm very concerned about PR #34. Specifically I'm worried how the version of google-auth-library to 1.2.0 was updated.

After updating the version of google-auto-auth used in @google-cloud/common (locally) and then depending on @google-cloud/common locally in the module @google-cloud/profiler, several tests failed (due to timeouts) when they had previously passed.

My primary concern is not that I've seen failing tests after this update, though. It seems to me as if PR #34 switched to using version 1.2.0 of google-auth-library, but did not include other notable changes, even though the API of google-auth-library changed notably with the major version update.

Specifically, GoogleAuth.fromJson() no longer takes a callback function. (Current version here; And code from a version prior to 1.0.0 here). But it looks like GoogleAuth.fromJson() is still called with a callback here and here.

It is my impression that there were pretty notable changes to the API of google-auth-library with the major version change. And it looks like google-auth-library is mocked in the tests for google-auto-auth, so I'm not confident that the tests would capture problems that might arise due to the changes made in PR#34.

stephenplusplus commented 6 years ago

Thanks. Will roll back in the next few minutes and roll it out more carefully after.

stephenplusplus commented 6 years ago

0.9.3 published. PR to follow to re-upgrade.

nolanmar511 commented 6 years ago

Thanks for the quick response!

nolanmar511 commented 6 years ago

@stephenplusplus -- How long until this will be upgraded again?

stephenplusplus commented 6 years ago

Until google-auto-auth upgrades to google-auth-library's latest release? I'm stalled on some questions for @ofrobots: https://github.com/stephenplusplus/google-auto-auth/pull/38#discussion_r165398555

But it seems that google-auth-library is working to deprecate google-auto-auth, so I'm not sure which will happen first.

Is something included in the latest google-auth-library that we need?

nolanmar511 commented 6 years ago

The latest version of google-auth-library makes it possible to refresh a token before it expires (and, by default, refreshes the toke 5 minutes early). So upgrading this dependency will eliminated some "Unauthorized" errors we see in cloud-profiler-nodejs (https://github.com/GoogleCloudPlatform/cloud-profiler-nodejs/issues/94).

I'd ultimately be interested in making the time to refresh the token before it expires configurable in google-cloud/common (and I would need to make some changes to google-auto-auth after it's dependency on google-auth-library is upgraded) to do that.