Skip to content

Conversation

@loreto
Copy link
Contributor

@loreto loreto commented Sep 25, 2023

Summary

Adds token refresh logic

How was it tested?

I'm actually unsure how best to test this other than going through the auth flow, waiting for my token to expire, and running again. Any ideas on how best to shortcut that process? Is there a way to purposefully give myself an expired token on the first try?

@loreto loreto requested a review from mikeland73 September 25, 2023 15:58
@mikeland73
Copy link
Contributor

I'm actually unsure how best to test this other than going through the auth flow, waiting for my token to expire, and running again. Any ideas on how best to shortcut that process? Is there a way to purposefully give myself an expired token on the first try?

  • You can change the expiration to be super short.
  • You can also just call refresh and inspect that the tokens changed and you got new expirations.
  • Add ValidAlwaysExpiredForTesting function and use it to test

if !tok.Valid() {
return nil, false
}
return tok, true
Copy link
Contributor

Choose a reason for hiding this comment

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

no needed

func (c *Client) RefreshSession() *session.Token {
panic("refresh session not implemented")
func (c *Client) refresh(tok *session.Token) *session.Token {
ctx := context.Background()
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, pass it in.

return tok
}

if newToken.AccessToken != tok.AccessToken {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this check needed?

return nil
}

// TODO: figure out how to share oidc provider and outh2 client
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: outh2

@mikeland73
Copy link
Contributor

I tested by:

  • Had an expired id token
  • Ran envsec auth whoami which triggered refresh
  • Printed correct info and extended life of token

@mikeland73 mikeland73 merged commit de89e93 into main Sep 28, 2023
@mikeland73 mikeland73 deleted the daniel/auth-refresh branch September 28, 2023 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants