Skip to content

[msal-node] Cache-3: Add NodeStorage, token generation and separate SPAresponse #1519

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

Merged
merged 29 commits into from
May 8, 2020

Conversation

sameerag
Copy link
Member

@sameerag sameerag commented Apr 20, 2020

This PR is third in the series of cache PRs. Since I am breaking down a bigger PR into consumable chunks, this may not work end to end but does the following changes:

  • Defines NodeStorage with two new base APIs to read and write to external cache
  • A temporary NodeCacheManager created on request from AzureSDK which will soon morph into keytar hook and eventually the extension library
  • A UnifiedCacheManager, currently does getAccounts and adds credentials to cache. This file will expand in future for cache lookups.
  • TokenGenerator to generate the token entities from response data
  • Separated SPAResponseHandler for now ; this will fold to a single file in msal-common soon

Tests:
Tests are coming, they are 80% there, need a few more

@sameerag sameerag self-assigned this Apr 20, 2020
Copy link
Contributor

@jasonnutter jasonnutter left a comment

Choose a reason for hiding this comment

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

I think this could be split into two PRs. Also, are there tests you can add?

Copy link
Contributor

@sangonzal sangonzal left a comment

Choose a reason for hiding this comment

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

Overall looks good! Good progress, a couple of comments.

Copy link
Contributor

@pkanher617 pkanher617 left a comment

Choose a reason for hiding this comment

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

A couple questions, but overall should be ok.

[msal-node] Cache-4: ResponseHandler added to `msal-common`
@sameerag sameerag merged commit cf6df64 into msal-node-cache-utils May 8, 2020
@sameerag sameerag deleted the msal-node-response-cacheTokens branch May 20, 2020 08:06
This was referenced May 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants