-
Notifications
You must be signed in to change notification settings - Fork 256
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
fix: Fixed parsing of expiration timestamp from ID tokens #492
Conversation
…uch as service accounts
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.
LGTM
scopes = ["https://www.googleapis.com/auth/drive", "https://www.googleapis.com/auth/bigtable.data"] | ||
stub = make_auth_stubs access_token: "1/abcdef1234567890", scope: scopes | ||
@client = GCECredentials.new(scope: scopes) | ||
@client.fetch_access_token! | ||
expect(stub).to have_been_requested | ||
end | ||
end | ||
|
||
describe "Fetch ID tokens" do |
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.
why is this a separate describe with the same name as below?
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.
Different context (metadata is available vs metadata is not available)
base64 = Base64.urlsafe_decode64 match[1] | ||
json = JSON.parse base64 rescue nil | ||
return unless json.respond_to?(:key?) && json.key?("exp") | ||
Time.at json["exp"].to_i |
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.
can this fail / does it need a rescue?
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 so, but good idea to wrap it anyway for safety.
Looks like this was never implemented, with the result that ID token credential objects stopped working when the token expired.