-
Notifications
You must be signed in to change notification settings - Fork 383
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(impersonated): add impersonated credentials auth #1207
Conversation
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
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.
@salrashid123 sorry for screwing up your branch, I left some refactoring and recommendations in this PR, to act as a baseline.
One additional thought, we should probably add a blurb in the README about how to use the impersonated client?
src/auth/impersonated.ts
Outdated
} catch (err) { | ||
let helpfulMessage = null; | ||
if (err.status === 403) { | ||
helpfulMessage = |
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.
Likewise, perhaps we can get away with checking for just the 403
and 404
case?
thanks, i retested the code from this PR and it worked fine with the snippet below 1,2 wokred fine but ...how do you supply the impersonated client to a GCP library like i do know in the original PR i showed 3 working below but ...i must've just had that as a place holder or somthing of a wishlist is it possible to supply i looked at but those credentials and what not are something else like a keyfile const {Logging} = require('@google-cloud/logging');
const { GoogleAuth, JWT, Impersonated } = require('google-auth-library');
const { Storage } = require('@google-cloud/storage');
var svcAccountFile = '/home/srashid/gcp_misc/certs/mineral-minutia-820-e9a7c8665867.json';
var project_id = 'fabled-ray-104117';
const keys = require(svcAccountFile);
// create a sourceCredential
let saclient = new JWT(
keys.client_email,
null,
keys.private_key,
['https://www.googleapis.com/auth/cloud-platform', 'https://www.googleapis.com/auth/iam'],
);
// Use that to impersonate the targetPrincipal
let targetClient = new Impersonated({
sourceClient: saclient,
targetPrincipal: "impersonated-account@fabled-ray-104117.iam.gserviceaccount.com",
lifetime: 30,
delegates: [],
targetScopes: ["https://www.googleapis.com/auth/cloud-platform"]
});
// At this point you can use the client as any other:
// 1. Acquire Headers
const authHeaders = await targetClient.getRequestHeaders();
console.log(authHeaders);
// 2. Use client in authorized session
targetClient.getAccessToken().then(res => {
let url = 'https://www.googleapis.com/storage/v1/b?project=' + project_id
targetClient.requestAsync({ url }).then(resp => {
console.log(resp.data.items[0]);
}).catch(function (error) {
console.error('Unable to list buckets: ' + error);
});
});
// 3. Use client with GCP Service
const logging = new Logging({projectId: project_id});
const log = logging.log('mylog');
const text = 'Hello, world!';
const metadata = {
resource: {type: 'global'},
};
const entry = log.entry(metadata, text);
await log.write(entry);
console.log(`Logged: ${text}`); |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
thanks. verified the last push works. here's a suggestion for the README.md https://gist.github.com/salrashid123/6e3c2eaa0575d6c18632931cbf8cb496 |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
Warning: This pull request is touching the following templated files:
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
Hey @bcoe thanks for addressing the comments. I noticed some comments may have been accidentally missed. Perhaps you forgot to commit those changes? This may be relevant but when I try to "show changes since my last review", I get an error message: We went looking everywhere, but couldn’t find those commits. |
@bojeil-google the push had just failed. |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
1 similar comment
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
src/auth/impersonated.ts
Outdated
* the "Service Account Token Creator" IAM role. | ||
* | ||
* @param {object} options - The configuration object. | ||
* @param {object} [options.credentials] the service account email address. |
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.
I don't think this exists in ImpersonatedOptions
test/test.impersonated.ts
Outdated
@@ -0,0 +1,351 @@ | |||
/** | |||
* Copyright 2019 Google LLC |
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.
Missed this one: 2021
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
@bojeil-google thank you for the thorough review, this is much better than it would have been if landed earlier (also it identified a bug with refreshing token logic). |
🤖 I have created a release \*beep\* \*boop\* --- ## [7.4.0](https://www.github.com/googleapis/google-auth-library-nodejs/compare/v7.3.0...v7.4.0) (2021-07-29) ### Features * **impersonated:** add impersonated credentials auth ([#1207](https://www.github.com/googleapis/google-auth-library-nodejs/issues/1207)) ([ab1cd31](https://www.github.com/googleapis/google-auth-library-nodejs/commit/ab1cd31e07d45424f614e0401d1068df2fbd914c)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release \*beep\* \*boop\* --- ## [7.4.0](https://www.github.com/googleapis/google-auth-library-nodejs/compare/v7.3.0...v7.4.0) (2021-07-29) ### Features * **impersonated:** add impersonated credentials auth ([googleapis#1207](https://www.github.com/googleapis/google-auth-library-nodejs/issues/1207)) ([ab1cd31](https://www.github.com/googleapis/google-auth-library-nodejs/commit/ab1cd31e07d45424f614e0401d1068df2fbd914c)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Adds ImpersonatedCredentials. This credential type basically takes one source credential and exchanges it for another, different service account.
Fixes #535
Refs #779
@salrashid123 I've gone through the original #779 and made some edits to make the work more idiomatic for Node.js.
I'm leaving some review comments on this PR that would get it ready to land.