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
Separate Cache Lookup and Token Refresh in SilentFlowClient #2189
Conversation
…tion-library-for-js into separate-acquireCachedToken-refreshToken
…tion-library-for-js into separate-acquireCachedToken-refreshToken
…thub.com/AzureAD/microsoft-authentication-library-for-js into separate-acquireCachedToken-refreshToken
credentialType: CredentialType.ACCESS_TOKEN, | ||
clientId, | ||
realm: account.tenantId, | ||
target: scopes.printScopesLowerCase() |
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.
We are storing scopes
in lower case and look-up
in lower case. We need to address this, as I observed in cache-compat test cases, other libraries store the case as is. We may need to talk about this.
My take: Look up should be case insensitive but storing should be as is.
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.
100% agree. As this PR is scoped to a refactor, let's take functional changes in a follow up PR
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.
Please test silent use case for node before merging this!
This PR mainly refactors the SilentFlowClient and provides 2 additional APIs
Changes include:
acquireCachedToken
andrefreshToken
function inSilentFlowClient
acquireToken
function refactored to call the 2 new APIsCacheManager
getCacheRecord
function toCacheManager
to do all the token cache lookups in one place