fix: GCECredentials - Allow retrieval of ID token #425
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use -- conventional-commit-lint bot |
StupidCodeFactory
force-pushed
the
pass-options-to-gce-credentials
branch
4 times, most recently
from
1163c39
to
97255af
Compare
StupidCodeFactory
changed the title
StupidCodeFactory
changed the title
| end | ||
|
|
||
| it "honors passing options to OAuth 2 client" do | ||
| stub = stub_request(:get, "http://169.254.169.254") |
There was a problem hiding this comment.
Nit: since we're repeating this stub request. Can you put it in a variable?
StupidCodeFactory
force-pushed
the
pass-options-to-gce-credentials
branch
from
97255af
to
9c49861
Compare
| expect(creds).to_not be_nil | ||
| describe "when on compute engine" do | ||
| before do | ||
| @compute_metadata_server = stub_request(:get, "http://169.254.169.254") |
There was a problem hiding this comment.
@bajajneha27 thanks for your feedback.
While one would normally use a let!(:compute_metadata_server) as it's more idiomatic to RSpec test suite, this file seems to only be using @instance_variable so for the sake of consistency I've kept the pattern used in the file.
There seem to be a mixed use of let and before { @instance_varialbe = "foo" } throughout the test suite. It's also unclear what is the preferred way.
As a long term rubyist I'd be inclined to favour the let {}.
I'd be happy to convert this file from using @instance_variable to let blocks, in this pull request or another one.
There was a problem hiding this comment.
This looks fine. Thanks for implementing this.
StupidCodeFactory
force-pushed
the
pass-options-to-gce-credentials
branch
from
9c49861
to
0158267
Compare
|
@StupidCodeFactory , you'd have to update your branch with main. |
Passing of options down to the OAuth 2 client. optional options to GCEcredentials, enabling the creation of ID tokens.
StupidCodeFactory
force-pushed
the
pass-options-to-gce-credentials
branch
from
0158267
to
cc797e3
Compare
CONTEXT:
While deploying a couple of services on
Cloud RUN:roles/run.invokerfor service account B on service A.I expected to be able to retrieve an ID token and be able to call service A from service B. After some debugging I realised I was getting an
access_tokenrather then anid_token.I tracked it down to the GCECredentials instantiation only passing the scope to the subclass of the Signet::Oauth2::Client, hence not being able to pass the
target_audiencefor retrieve anid_tokenI believe this should fix: #299