fix(enterprise): fix start interval when using access tokens #2211
Conversation
10ko
force-pushed
the
fix-access-token-refresh
branch
from
162b46e to
0cafefe
Compare
| if (!tokenIsValid) { | ||
| await this.refreshToken() | ||
| // If the token is an Access Token and it's invalid we return. | ||
| if (gardenEnv.GARDEN_AUTH_TOKEN) { |
There was a problem hiding this comment.
Shouldn't this throw? If you're explicitly setting an access token that doesn't work you probably want an error.
There was a problem hiding this comment.
Possibly. I wanted to try a more conservative approach since it fails later, but I agree we can throw here.
I'll add it.
core/src/enterprise/api.ts
Outdated
| if (gardenEnv.GARDEN_AUTH_TOKEN) { | ||
| this.log.error({ | ||
| msg: | ||
| "The provided access token is expired or revoked, please create a new one from the Garden Enterprise UI.", |
There was a problem hiding this comment.
is expired or revoked, > is expired or has been revoked,
| @@ -55,13 +57,22 @@ export async function login(enterpriseApi: EnterpriseApi, log: LogEntry): Promis | |||
| } | |||
|
|
|||
| export async function logout(enterpriseApi: EnterpriseApi, log: LogEntry): Promise<void> { | |||
There was a problem hiding this comment.
This function isn't used anywhere.
There was a problem hiding this comment.
I updated the PR, I left out the LogOutCommand
| @@ -207,26 +229,6 @@ export class EnterpriseApi { | |||
| return token | |||
| } | |||
|
|
|||
| async logout() { | |||
There was a problem hiding this comment.
Why are we removing this? Note that the "other" logout function isn't used.
There was a problem hiding this comment.
I moved the logic to be out of the EnterpriseApi class to maintain consistency with the login function
10ko
force-pushed
the
fix-access-token-refresh
branch
from
0cafefe to
352159b
Compare
|
Thanks @eysi09, I addressed all your comments. |
What this PR does / why we need it:
This PR fixes a problem we introduced recently where we would start the auth token refresh interval when using an Access Token.
Additionally, I refactored the
logoutfunction, replacing old unused code with the actual implementation that was inEnterpriseApiWhich issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: