Skip to content

fix compute engine IDTokenCredentials.with_target_audience method #361

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

Closed

Conversation

lmiglio
Copy link
Contributor

@lmiglio lmiglio commented Jul 30, 2019

Hi everybody,

Here is the PR for this issue: #359

It should be backward compatible and it should not break anything.

I added an integration test of IDTokenCredentials and Signer where I used responses for mocking REST responses, I do not know if that is fine.

I followed these guidelines.

Hope everything is ok, if not let me know :)

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 30, 2019
@busunkim96 busunkim96 self-requested a review August 7, 2019 21:31
setup.py Outdated
@@ -32,7 +32,7 @@

setup(
name='google-auth',
version='1.6.3',
version='1.6.4',
Copy link
Contributor

Choose a reason for hiding this comment

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

We cut separate release PRs to just bump the version and add changelog notes. See #325 for an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, sorry, did not know that. I will revert the version to the one present before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I squashed the commits in order to have a single commit with the fix (thus avoiding a commit just for reverting the version back, I do not know if this is compliant with your branch strategy). Let me know if there is other I can do. Thanks!

@salrashid123
Copy link
Contributor

cross referencing with PR
#362

that'll remove using the iam signing just to get ID tokens on compute...
you can easily an id_token through the metadata server

@lmiglio lmiglio force-pushed the bugfix/with_target_audience branch from 9e72686 to a84e427 Compare August 19, 2019 09:24
Copy link
Contributor

@busunkim96 busunkim96 left a comment

Choose a reason for hiding this comment

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

Thank you for submitting this PR!

@bshaffer
Copy link
Contributor

bshaffer commented Feb 7, 2020

Hi @lmiglio !
Could you fix the merge conflicts so we can merge this PR? I've resolved them here for example (see #438)

@bshaffer
Copy link
Contributor

Superceded by #438

@bshaffer bshaffer closed this Feb 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants