-
-
Notifications
You must be signed in to change notification settings - Fork 38
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
Return the latest token instead of panicking, even if it's expired #1
Conversation
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.
Thank you @d0x2f for the contribution. I wasn't even sure if anyone except me is using this 🙂
The change with timeout makes perfect sense.
@@ -22,7 +22,8 @@ impl AuthenticationManager { | |||
pub async fn get_token(&self, scopes: &[&str]) -> Result<Token, GCPAuthError> { | |||
let mut sa = self.service_account.lock().await; | |||
let mut token = sa.get_token(scopes); | |||
if token.is_none() { | |||
|
|||
if token.is_none() || token.clone().unwrap().has_expired() { |
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.
The current code is just refactor as it keeps the same functionality (returning library error after refresh returned invalid token)
I'll merge you current changes as they don't change handling of the expired token, except the timeout. To the second point the current behaviour (even after your PR) is that after token refresh failure the library returns |
Thank you for merging! let mut token = sa.get_token(scopes);
if token.is_none() || token.clone().unwrap().has_expired() {
sa.refresh_token(&self.client, scopes).await?;
token = sa.get_token(scopes);
}
Ok(token.expect("Token obtained with refresh or failed before")) Regarding point 2, previously if the token was considered expired the |
Oh sorry, I totally missed that. I've got confused with |
When running in cloudrun, the
metadata.google.internal/computeMetadata/v1/instance/service-accounts/default/token
endpoint will cache an access token until it's 60s from expiry. This project also considers a token to be expired 60 seconds before the real expiry.This causes an issue in that there is a short period of time between the lib considering the token to be expired and the cloud run endpoint returning a new one. During this time the library panics with
Token obtained with refresh or failed before
. This happens consistently every 30 minutes in my project.This pull request changes two things:
PS: Thanks for the super handy lib 👍