[msal-node][#2] Merge cache logic in `msal-common` for `node` and `browser` platforms #1762

sameerag · 2020-06-08T05:36:12Z

This PR merges the cache operations for node and browser platforms. It:

Rewrites storing cache with redefined API`s for browser with the new Cache format
Updates SPAClient to consume the new APIs
Remove old cache references for browser and SPA Client

…on this)

jasonnutter · 2020-06-11T23:10:14Z

lib/msal-browser/src/cache/BrowserStorage.ts

-        const itemCookie = this.getItemCookie(msalKey);
-        if (this.cacheConfig.storeAuthStateInCookie && itemCookie) {
-            return itemCookie;
+    getItem(key: string, type: string): string | object {


Per discussion with @pkanher617, instead of baking this business logic into these functions, you should instead create an abstract class with functions for each of the case statements, and then have BrowserStorage implement ICacheStorage and extend the abstract class.

That way consumers can use a use-case specific function instead of passing a magic string, and that way if applications provide their own storage implementations, they don't have to know about this logic.

We will revisit this when we add the feature for consumer driven storage.

We already have consumer driven storage for Node that we want to support (the extensions library).

@sameerag @pkanher617 Let's talk about this next design meeting.

We also have other storage scenarios for the web such as VS code extensions that we will want to support, which would fall under "user-provided storage"

The abstract class is now implemented in the follow up PR #1771. Regarding abstracting getItem implementation, I agree we can improve it for custom storage, however I would like to track it separately and as an enhancement.

We can prioritize if we can arrive at a design soon but considering the timelines to get a working branch earlier, I would like to close this PR.

lib/msal-common/src/client/SilentFlowClient.ts

samples/VanillaJSTestApp2.0/app/default/auth.js

lib/msal-browser/src/cache/BrowserStorage.ts

lib/msal-browser/src/app/PublicClientApplication.ts

lib/msal-browser/src/config/Configuration.ts

coveralls · 2020-06-17T18:23:04Z

Coverage decreased (-0.4%) to 77.575% when pulling e80a2ed on msal-node-cache-merge into 189cbe7 on dev.

…built on this)

Prithvi Kanherkar added 2 commits June 5, 2020 15:16

Moving from SPA cache to unified cache

39def0f

Update SPAClient.ts

03cf370

github-actions bot added msal-browser Related to msal-browser package msal-common Related to msal-common package labels Jun 8, 2020

pkanher617 and others added 8 commits June 8, 2020 10:04

Update auth.js

51137c5

Changed to IAccount, getting ready to fix getAccount APIs

eaa7b99

Clean up files

c1a2263

Updating exports

2e3682d

Update folder name

2456cd4

Clean up usage

8bfcd4a

Pushing changes to cache

7796c05

Generic cache lookup for UnifiedCacheManager

4071482

sameerag changed the base branch from msal-node-cache-silentFlow to dev June 8, 2020 22:55

sameerag changed the base branch from dev to msal-node-cache-silentFlow June 8, 2020 22:56

pkanher617 added 9 commits June 8, 2020 16:58

Merge branch 'msal-node-cache-silentFlow' into msal-node-cache-merge

1ea54f1

Getting cache working

c123f80

Fixed getAllAccounts

bdf807b

Cleanup code

2c6b3f8

Fixing AcquireTokenSilent

0d7baa9

Silent flow is now working

837ad79

Logout working correctly

5b6d912

Update AuthenticationResult.ts

5526b81

Update Constants.ts

d8f91fc

sameerag added a commit that referenced this pull request Jun 9, 2020

Silent works / logout works (tested in browser with a PR #1762 built …

6fc0fc0

…on this)

sameerag changed the base branch from msal-node-cache-silentFlow to dev June 9, 2020 22:39

sameerag changed the base branch from dev to msal-node-cache-silentFlow June 9, 2020 22:40

pkanher617 and others added 4 commits June 9, 2020 15:45

Merge branch 'msal-node-cache-silentFlow' into msal-node-cache-merge

f46d70e

Getting builds to pass

c3e7dfa

Adding expiry fixes

5207b2b

Marking todo for scopes for a follow up PR

fae05ce

Merge branch 'msal-node-cache-silentFlow' into msal-node-cache-merge

6de8c74

pkanher617 marked this pull request as ready for review June 11, 2020 00:55

pkanher617 requested review from DarylThayil, jasonnutter, jmckennon, jo-arroyo, pkanher617, hectormmg and tnorling as code owners June 11, 2020 00:55

pkanher617 changed the title ~~[msal-mode] Merge cache logic in msal-common for node and browser platforms~~ [msal-node][#2] Merge cache logic in msal-common for node and browser platforms Jun 11, 2020

pkanher617 approved these changes Jun 11, 2020

View reviewed changes

jasonnutter suggested changes Jun 11, 2020

View reviewed changes

sangonzal approved these changes Jun 12, 2020

View reviewed changes

lib/msal-browser/src/config/Configuration.ts Show resolved Hide resolved

pkanher617 added 8 commits June 12, 2020 18:05

Merge branch 'msal-node-cache-silentFlow' into msal-node-cache-merge

d143986

Fixing tests in common

e19fcc3

all tests passing

7aaee41

Merge branch 'msal-node-cache-silentFlow' into msal-node-cache-merge

efe14f0

Update SPAClient.ts

dd41baa

Merge branch 'msal-node-cache-silentFlow' into msal-node-cache-merge

f35e8c6

updating with feedback from PR

92ede8b

Merge branch 'msal-node-cache-silentFlow' into msal-node-cache-merge

ecd2986

pkanher617 changed the base branch from msal-node-cache-silentFlow to dev June 17, 2020 16:55

Merge branch 'dev' into msal-node-cache-merge

e80a2ed

jasonnutter approved these changes Jun 17, 2020

View reviewed changes

pkanher617 merged commit c968ef7 into dev Jun 17, 2020

pkanher617 deleted the msal-node-cache-merge branch June 17, 2020 23:15

sameerag mentioned this pull request Jun 24, 2020

msal-node PR tracker #1258

Closed

1 task

sameerag mentioned this pull request Jul 2, 2020

[msal-common] End-to-end common code works for msal-browser and msal-node #1492

Closed

3 tasks

azure-pipelines bot pushed a commit to pkanher617/microsoft-authentication-library-for-js that referenced this pull request Jan 4, 2022

Silent works / logout works (tested in browser with a PR AzureAD#1762 …

335d0a5

…built on this)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[msal-node][#2] Merge cache logic in `msal-common` for `node` and `browser` platforms #1762

[msal-node][#2] Merge cache logic in `msal-common` for `node` and `browser` platforms #1762

sameerag commented Jun 8, 2020 •

edited

jasonnutter Jun 11, 2020

jasonnutter Jun 11, 2020

pkanher617 Jun 17, 2020

jasonnutter Jun 17, 2020 •

edited

jasonnutter Jun 17, 2020

jasonnutter Jun 17, 2020

sameerag Jun 17, 2020 •

edited

sameerag Jun 17, 2020

coveralls commented Jun 17, 2020

[msal-node][#2] Merge cache logic in msal-common for node and browser platforms #1762

[msal-node][#2] Merge cache logic in msal-common for node and browser platforms #1762

Conversation

sameerag commented Jun 8, 2020 • edited

jasonnutter Jun 11, 2020

Choose a reason for hiding this comment

jasonnutter Jun 11, 2020

Choose a reason for hiding this comment

pkanher617 Jun 17, 2020

Choose a reason for hiding this comment

jasonnutter Jun 17, 2020 • edited

Choose a reason for hiding this comment

jasonnutter Jun 17, 2020

Choose a reason for hiding this comment

jasonnutter Jun 17, 2020

Choose a reason for hiding this comment

sameerag Jun 17, 2020 • edited

Choose a reason for hiding this comment

sameerag Jun 17, 2020

Choose a reason for hiding this comment

coveralls commented Jun 17, 2020

[msal-node][#2] Merge cache logic in `msal-common` for `node` and `browser` platforms #1762

[msal-node][#2] Merge cache logic in `msal-common` for `node` and `browser` platforms #1762

sameerag commented Jun 8, 2020 •

edited

jasonnutter Jun 17, 2020 •

edited

sameerag Jun 17, 2020 •

edited