fix: use iamcredentials API to sign blobs #908
Conversation
googlebot
added
the
cla: yes
test/test.googleauth.ts
Outdated
| const iamPath = `/v1/projects/${STUB_PROJECT}/serviceAccounts/${email}:signBlob`; | ||
| const signature = 'erutangis'; | ||
| const iamUri = `https://iamcredentials.googleapis.com`; | ||
| const iamPath = `/v1/%7Bname=projects/${STUB_PROJECT}/serviceAccounts/${email}%7D`; |
There was a problem hiding this comment.
In Java we used the following pattern, (without {name=})
static final String SIGN_BLOB_URL_FORMAT =
"https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/%s:signBlob";There was a problem hiding this comment.
Happy to go change it. Just so we're doing this with eyes wide open, a few things:
- I chose this pattern based on the docs (here)[https://cloud.google.com/iam/docs/reference/credentials/rest/v1/projects.serviceAccounts/signBlob#http-request]. Is the doc correct?
- I did write a sample, and made sure with an end to end test that the endpoint works. So what's there now is effective (again happy to change)
There was a problem hiding this comment.
I followed the same document in Java and I think I mistook {name=...} as being an expression pattern that's expected rather than the actual format of the URI. Additionally, I find it easier to parse when reading it.
For STUB_PROJECT, given the project id is in the service account, stating it explicitly seems unnecessary.
src/auth/googleauth.ts
Outdated
| @@ -812,13 +812,16 @@ export class GoogleAuth { | |||
| const id = `projects/${projectId}/serviceAccounts/${creds.client_email}`; | |||
| const res = await this.request<SignBlobResponse>({ | |||
| method: 'POST', | |||
| url: `https://iam.googleapis.com/v1/${id}:signBlob`, | |||
| data: {bytesToSign: crypto.encodeBase64StringUtf8(data)}, | |||
| url: `https://iamcredentials.googleapis.com/v1/{name=${id}}`, | |||
There was a problem hiding this comment.
If I understand correctly, we're moving from the iam API to sign, rather than the iamcredentials API.
Are we sure these API surfaces are identical, or should be consider making this a breaking change? CC: @frankyn
There was a problem hiding this comment.
iamcredentials is the v2 of the iam API. I synced with the IAM team on which API libraries should use. iamcredentials works in the same way for this specific scenario and isn't impacted by the requests having to be routed back to US region (which is the case with the existing iam API).
|
I agree with @bcoe about not understanding the implications of the change WRT backwards compatibility. Looks like |
|
@bshaffer I sent an email cc'd you on it and raised the question to the SWE PoC that I spoke with last on this specific question. I didn't check error code / messages in this case. Sorry about that. |
🤖 I have created a release \*beep\* \*boop\* --- ## [6.0.0](https://www.github.com/googleapis/google-auth-library-nodejs/compare/v5.10.1...v6.0.0) (2020-03-26) ### ⚠ BREAKING CHANGES * typescript@3.7.x introduced some breaking changes in generated code. * require node 10 in engines field (#926) * remove deprecated methods (#906) ### Features * require node 10 in engines field ([#926](https://www.github.com/googleapis/google-auth-library-nodejs/issues/926)) ([d89c59a](https://www.github.com/googleapis/google-auth-library-nodejs/commit/d89c59a316e9ca5b8c351128ee3e2d91e9729d5c)) ### Bug Fixes * do not warn for SDK creds ([#905](https://www.github.com/googleapis/google-auth-library-nodejs/issues/905)) ([9536840](https://www.github.com/googleapis/google-auth-library-nodejs/commit/9536840f88e77f747bbbc2c1b5b4289018fc23c9)) * use iamcredentials API to sign blobs ([#908](https://www.github.com/googleapis/google-auth-library-nodejs/issues/908)) ([7b8e4c5](https://www.github.com/googleapis/google-auth-library-nodejs/commit/7b8e4c52e31bb3d448c3ff8c05002188900eaa04)) * **deps:** update dependency gaxios to v3 ([#917](https://www.github.com/googleapis/google-auth-library-nodejs/issues/917)) ([1f4bf61](https://www.github.com/googleapis/google-auth-library-nodejs/commit/1f4bf6128a0dcf22cfe1ec492b2192f513836cb2)) * **deps:** update dependency gcp-metadata to v4 ([#918](https://www.github.com/googleapis/google-auth-library-nodejs/issues/918)) ([d337131](https://www.github.com/googleapis/google-auth-library-nodejs/commit/d337131d009cc1f8182f7a1f8a9034433ee3fbf7)) * **types:** add additional fields to TokenInfo ([#907](https://www.github.com/googleapis/google-auth-library-nodejs/issues/907)) ([5b48eb8](https://www.github.com/googleapis/google-auth-library-nodejs/commit/5b48eb86c108c47d317a0eb96b47c0cae86f98cb)) ### Build System * update to latest gts and TypeScript ([#927](https://www.github.com/googleapis/google-auth-library-nodejs/issues/927)) ([e11e18c](https://www.github.com/googleapis/google-auth-library-nodejs/commit/e11e18cb33eb60a666980d061c54bb8891cdd242)) ### Miscellaneous Chores * remove deprecated methods ([#906](https://www.github.com/googleapis/google-auth-library-nodejs/issues/906)) ([f453fb7](https://www.github.com/googleapis/google-auth-library-nodejs/commit/f453fb7d8355e6dc74800b18d6f43c4e91d4acc9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
Fixes #866