Repository: auth0/auth0-spa-js
Pull request #758 (2021-07-07 11:44:43)

Changes to logout and cache synchronicity 
### Description

This PR makes the following changes around logout and caching:

* `logout` can now optionally return a Promise. This is currently the case when a custom cache implementation is specified using the `cache` option
* If no custom cache implementation is used, `logout` acts synchronously and does not return a promise
* `ICache` methods (`get`, `set`, `remove`, `allKeys`) can now optionally return a promise or a static value, and is handled seamlessly
* docs updated
* internal caches changed to return static values, instead of wrapping them in `Promise.resolve`

The motivation was for SDKs like `auth0-angular` and `auth0-react` who update their internal state reactively can now rely on logout being synchronous by default, and update their state accordingly.

Note that none of this should be a breaking change for consumers:

* If anyone already has a custom cache and are returning a Promise, that will continue to work
* Up until now, `logout` never returned a value so there was nothing to handle

### Testing

- [X] This change adds test coverage for new/changed/fixed functionality

### Checklist

- [X] I have added documentation for new/changed functionality in this PR or in auth0.com/docs
- [X] All active GitHub checks for tests, formatting, and security are passing
- [X] The correct base branch is being used, if not `master`


lgtm-com[bot]: This pull request **introduces 1 alert** when merging c31598aa059393b13a8f27747dac3b9d7470e106 into ecafb37fe3b9d5310c61a02f2974903101b7828b - [view on LGTM.com](https://lgtm.com/projects/g/auth0/auth0-spa-js/rev/pr-663fb4a86f33ff2331057e766d4bec78e666a8a4)

**new alerts:**

* 1 for Unused variable, import, function or class

Patch:
diff --git a/README.md b/README.md
index e21f6db..c839a64 100644
--- a/README.md
+++ b/README.md
@@ -216,36 +216,34 @@ The SDK can be configured to use a custom cache store that is implemented by you
 
 To do this, provide an object to the `cache` property of the SDK configuration.
 
-The object should implement the following functions:
+The object should implement the following functions. Note that all of these functions can optionally return a Promise or a static value.
 
-| Signature                                            | Description                                                                                                                                                                                                                                                                                       |
-| ---------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| `async get(key)`                                     | Returns the item from the cache with the specified key, or `undefined` if it was not found                                                                                                                                                                                                        |
-| `async set(key: string, object: any): Promise<void>` | Sets an item into the cache                                                                                                                                                                                                                                                                       |
-| `async remove(key)`                                  | Removes a single item from the cache at the specified key, or no-op if the item was not found                                                                                                                                                                                                     |
-| `async allKeys()`                                    | (optional) Implement this if your cache has the ability to return a list of all keys. Otherwise, the SDK internally records its own key manifest using your cache. **Note**: if you only want to ensure you only return keys used by this SDK, the keys we use are prefixed with `@@auth0spajs@@` |
+| Signature                        | Return type                    | Description                                                                                                                                                                                                                                                                                       |
+| -------------------------------- | ------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `get(key)`                       | Promise<object> or object      | Returns the item from the cache with the specified key, or `undefined` if it was not found                                                                                                                                                                                                        |
+| `set(key: string, object: any) ` | Promise<void> or void          | Sets an item into the cache                                                                                                                                                                                                                                                                       |
+| `remove(key)`                    | Promise<void> or void          | Removes a single item from the cache at the specified key, or no-op if the item was not found                                                                                                                                                                                                     |
+| `allKeys()`                      | Promise<string[]> or string [] | (optional) Implement this if your cache has the ability to return a list of all keys. Otherwise, the SDK internally records its own key manifest using your cache. **Note**: if you only want to ensure you only return keys used by this SDK, the keys we use are prefixed with `@@auth0spajs@@` |
 
 Here's an example of a custom cache implementation that uses `sessionStorage` to store tokens and apply it to the Auth0 SPA SDK:
 
 ```js
 const sessionStorageCache = {
   get: function (key) {
-    return Promise.resolve(JSON.parse(sessionStorage.getItem(key)));
+    return JSON.parse(sessionStorage.getItem(key));
   },
 
   set: function (key, value) {
     sessionStorage.setItem(key, JSON.stringify(value));
-    return Promise.resolve();
   },
 
   remove: function (key) {
     sessionStorage.removeItem(key);
-    return Promise.resolve();
   },
 
   // Optional
   allKeys: function () {
-    return Promise.resolve(Object.keys(sessionStorage));
+    return Object.keys(sessionStorage);
   }
 };
 
diff --git a/src/Auth0Client.ts b/src/Auth0Client.ts
index a3cac1d..857568b 100644
--- a/src/Auth0Client.ts
+++ b/src/Auth0Client.ts
@@ -833,6 +833,9 @@ export default class Auth0Client {
    *
    * Clears the application session and performs a redirect to `/v2/logout`, using
    * the parameters provided as arguments, to clear the Auth0 session.
+   *
+   * **Note:** If you are using a custom cache, and specifying `localOnly: true`, and you want to perform actions or read state from the SDK immediately after logout, you should `await` the result of calling `logout`.
+   *
    * If the `federated` option is specified it also clears the Identity Provider session.
    * If the `localOnly` option is specified, it only clears the application session.
    * It is invalid to set both the `federated` and `localOnly` options to `true`,
@@ -841,7 +844,7 @@ export default class Auth0Client {
    *
    * @param options
    */
-  public logout(options: LogoutOptions = {}) {
+  public logout(options: LogoutOptions = {}): Promise<void> | void {
     const { localOnly, ...logoutOptions } = options;
 
     if (localOnly && logoutOptions.federated) {
@@ -850,16 +853,24 @@ export default class Auth0Client {
       );
     }
 
-    this.cacheManager.clear();
-    this.cookieStorage.remove('auth0.is.authenticated');
+    const postCacheClear = () => {
+      this.cookieStorage.remove('auth0.is.authenticated');
 
-    if (localOnly) {
-      return;
-    }
+      if (localOnly) {
+        return;
+      }
 
-    const url = this.buildLogoutUrl(logoutOptions);
+      const url = this.buildLogoutUrl(logoutOptions);
 
-    window.location.assign(url);
+      window.location.assign(url);
+    };
+
+    if (this.options.cache) {
+      return this.cacheManager.clear().then(() => postCacheClear());
+    } else {
+      this.cacheManager.clearSync();
+      postCacheClear();
+    }
   }
 
   private async _getTokenFromIFrame(
diff --git a/src/cache/cache-localstorage.ts b/src/cache/cache-localstorage.ts
index f63f489..ab61091 100644
--- a/src/cache/cache-localstorage.ts
+++ b/src/cache/cache-localstorage.ts
@@ -1,35 +1,31 @@
 import { ICache, Cacheable, CACHE_KEY_PREFIX } from './shared';
 
 export class LocalStorageCache implements ICache {
-  public set<T = Cacheable>(key: string, entry: T): Promise<void> {
+  public set<T = Cacheable>(key: string, entry: T) {
     localStorage.setItem(key, JSON.stringify(entry));
-    return Promise.resolve();
   }
 
-  public get<T = Cacheable>(key: string): Promise<T> {
+  public get<T = Cacheable>(key: string) {
     const json = window.localStorage.getItem(key);
 
-    if (!json) return Promise.resolve(null);
+    if (!json) return;
 
     try {
-      const payload = JSON.parse(json);
-      return Promise.resolve(payload);
+      const payload = JSON.parse(json) as T;
+      return payload;
     } catch (e) {
       /* istanbul ignore next */
-      return Promise.resolve(null);
+      return;
     }
   }
 
-  public remove(key: string): Promise<void> {
+  public remove(key: string) {
     localStorage.removeItem(key);
-    return Promise.resolve();
   }
 
-  public allKeys(): Promise<string[]> {
-    return Promise.resolve(
-      Object.keys(window.localStorage).filter(key =>
-        key.startsWith(CACHE_KEY_PREFIX)
-      )
+  public allKeys() {
+    return Object.keys(window.localStorage).filter(key =>
+      key.startsWith(CACHE_KEY_PREFIX)
     );
   }
 }
diff --git a/src/cache/cache-manager.ts b/src/cache/cache-manager.ts
index 1c97ec8..9b7c6c8 100644
--- a/src/cache/cache-manager.ts
+++ b/src/cache/cache-manager.ts
@@ -90,6 +90,20 @@ export class CacheManager {
     await this.keyManifest?.clear();
   }
 
+  /**
+   * Note: only call this if you're sure one of our internal (synchronous) caches are being used.
+   */
+  clearSync(): void {
+    const keys = this.cache.allKeys() as string[];
+
+    /* istanbul ignore next */
+    if (!keys) return;
+
+    keys.forEach(key => {
+      this.cache.remove(key);
+    });
+  }
+
   private wrapCacheEntry(entry: CacheEntry): WrappedCacheEntry {
     const expiresInTime = Math.floor(Date.now() / 1000) + entry.expires_in;
 
diff --git a/src/cache/cache-memory.ts b/src/cache/cache-memory.ts
index 770004f..5044585 100644
--- a/src/cache/cache-memory.ts
+++ b/src/cache/cache-memory.ts
@@ -5,28 +5,26 @@ export class InMemoryCache {
     let cache: Record<string, unknown> = {};
 
     return {
-      set<T = Cacheable>(key: string, entry: T): Promise<void> {
+      set<T = Cacheable>(key: string, entry: T) {
         cache[key] = entry;
-        return Promise.resolve();
       },
 
-      get<T = Cacheable>(key: string): Promise<T> {
+      get<T = Cacheable>(key: string) {
         const cacheEntry = cache[key] as T;
 
         if (!cacheEntry) {
-          return Promise.resolve(null);
+          return;
         }
 
-        return Promise.resolve(cacheEntry);
+        return cacheEntry;
       },
 
-      remove(key: string): Promise<void> {
+      remove(key: string) {
         delete cache[key];
-        return Promise.resolve();
       },
 
-      allKeys(): Promise<string[]> {
-        return Promise.resolve(Object.keys(cache));
+      allKeys(): string[] {
+        return Object.keys(cache);
       }
     };
   })();
diff --git a/src/cache/key-manifest.ts b/src/cache/key-manifest.ts
index d537ec9..92ae553 100644
--- a/src/cache/key-manifest.ts
+++ b/src/cache/key-manifest.ts
@@ -1,4 +1,9 @@
-import { CACHE_KEY_PREFIX, ICache, KeyManifestEntry } from './shared';
+import {
+  CACHE_KEY_PREFIX,
+  ICache,
+  KeyManifestEntry,
+  MaybePromise
+} from './shared';
 
 export class CacheKeyManifest {
   private readonly manifestKey: string;
@@ -34,11 +39,11 @@ export class CacheKeyManifest {
     }
   }
 
-  get(): Promise<KeyManifestEntry> {
+  get(): MaybePromise<KeyManifestEntry> {
     return this.cache.get<KeyManifestEntry>(this.manifestKey);
   }
 
-  clear(): Promise<void> {
+  clear(): MaybePromise<void> {
     return this.cache.remove(this.manifestKey);
   }
 
diff --git a/src/cache/shared.ts b/src/cache/shared.ts
index 85aed5e..15edc35 100644
--- a/src/cache/shared.ts
+++ b/src/cache/shared.ts
@@ -81,9 +81,11 @@ export type KeyManifestEntry = {
 
 export type Cacheable = WrappedCacheEntry | KeyManifestEntry;
 
+export type MaybePromise<T> = Promise<T> | T;
+
 export interface ICache {
-  set<T = Cacheable>(key: string, entry: T): Promise<void>;
-  get<T = Cacheable>(key: string): Promise<T>;
-  remove(key: string): Promise<void>;
-  allKeys?(): Promise<string[]>;
+  set<T = Cacheable>(key: string, entry: T): MaybePromise<void>;
+  get<T = Cacheable>(key: string): MaybePromise<T>;
+  remove(key: string): MaybePromise<void>;
+  allKeys?(): MaybePromise<string[]>;
 }
diff --git a/static/index.html b/static/index.html
index d955d7b..e34a23f 100644
--- a/static/index.html
+++ b/static/index.html
@@ -340,27 +340,15 @@
       // An example of a custom cache that could be constructed by a developer.
       const sessionStorageCache = {
         get: function (key) {
-          return Promise.resolve(JSON.parse(sessionStorage.getItem(key)));
+          return JSON.parse(sessionStorage.getItem(key));
         },
 
         set: function (key, value) {
           sessionStorage.setItem(key, JSON.stringify(value));
-          return Promise.resolve();
-        },
-
-        clear: function () {
-          for (var i = sessionStorage.length - 1; i >= 0; i--) {
-            if (sessionStorage.key(i).startsWith('@@auth0spajs@@')) {
-              sessionStorage.removeItem(sessionStorage.key(i));
-            }
-          }
-
-          return Promise.resolve();
         },
 
         remove: function (key) {
           sessionStorage.removeItem(key);
-          return Promise.resolve();
         }
       };
 


Test patch:
diff --git a/__tests__/Auth0Client/logout.test.ts b/__tests__/Auth0Client/logout.test.ts
index c6b67fe..890402b 100644
--- a/__tests__/Auth0Client/logout.test.ts
+++ b/__tests__/Auth0Client/logout.test.ts
@@ -12,9 +12,10 @@ import { TEST_AUTH0_CLIENT_QUERY_STRING } from '../constants';
 
 // @ts-ignore
 
-import { setupFn } from './helpers';
+import { loginWithPopupFn, loginWithRedirectFn, setupFn } from './helpers';
 
 import { TEST_CLIENT_ID, TEST_CODE_CHALLENGE, TEST_DOMAIN } from '../constants';
+import { InMemoryAsyncCacheNoKeys } from '../cache/shared';
 
 jest.mock('unfetch');
 jest.mock('es-cookie');
@@ -24,6 +25,7 @@ jest.mock('../../src/worker/token.worker');
 const mockWindow = <any>global;
 const mockFetch = (mockWindow.fetch = <jest.Mock>unfetch);
 const mockVerify = <jest.Mock>verify;
+const loginWithRedirect = loginWithRedirectFn(mockWindow, mockFetch);
 
 jest
   .spyOn(utils, 'bufferToBase64UrlEncoded')
@@ -115,11 +117,13 @@ describe('Auth0Client', () => {
 
     it('clears the cache', async () => {
       const auth0 = setup();
-      jest.spyOn(auth0['cacheManager'], 'clear').mockReturnValueOnce(undefined);
+      jest
+        .spyOn(auth0['cacheManager'], 'clearSync')
+        .mockReturnValueOnce(undefined);
 
       auth0.logout();
 
-      expect(auth0['cacheManager']['clear']).toHaveBeenCalled();
+      expect(auth0['cacheManager']['clearSync']).toHaveBeenCalled();
     });
 
     it('removes `auth0.is.authenticated` key from storage when `options.localOnly` is true', async () => {
@@ -151,5 +155,27 @@ describe('Auth0Client', () => {
       const fn = () => auth0.logout({ localOnly: true, federated: true });
       expect(fn).toThrow();
     });
+
+    it('can access isAuthenticated immediately after local logout', async () => {
+      const auth0 = setup();
+
+      await loginWithRedirect(auth0);
+      expect(await auth0.isAuthenticated()).toBe(true);
+      auth0.logout({ localOnly: true });
+
+      expect(await auth0.isAuthenticated()).toBe(false);
+    });
+
+    it('can access isAuthenticated immediately after local logout when using a custom async cache', async () => {
+      const auth0 = setup({
+        cache: new InMemoryAsyncCacheNoKeys()
+      });
+
+      await loginWithRedirect(auth0);
+      expect(await auth0.isAuthenticated()).toBe(true);
+      await auth0.logout({ localOnly: true });
+
+      expect(await auth0.isAuthenticated()).toBe(false);
+    });
   });
 });
diff --git a/__tests__/cache/cache-manager.test.ts b/__tests__/cache/cache-manager.test.ts
index 51f4db7..39b87c3 100644
--- a/__tests__/cache/cache-manager.test.ts
+++ b/__tests__/cache/cache-manager.test.ts
@@ -5,7 +5,6 @@ import {
 } from '../../src/cache';
 
 import {
-  Cacheable,
   CacheEntry,
   CacheKey,
   CACHE_KEY_PREFIX,
@@ -22,30 +21,7 @@ import {
   nowSeconds,
   TEST_REFRESH_TOKEN
 } from '../constants';
-
-class InMemoryCacheNoKeys implements ICache {
-  private cache: Record<string, unknown> = {};
-
-  set<T = Cacheable>(key: string, entry: T): Promise<void> {
-    this.cache[key] = entry;
-    return Promise.resolve();
-  }
-
-  get<T = Cacheable>(key: string): Promise<T> {
-    const cacheEntry = this.cache[key] as T;
-
-    if (!cacheEntry) {
-      return Promise.resolve(null);
-    }
-
-    return Promise.resolve(cacheEntry);
-  }
-
-  remove(key: string): Promise<void> {
-    delete this.cache[key];
-    return Promise.resolve();
-  }
-}
+import { InMemoryAsyncCacheNoKeys } from './shared';
 
 const defaultKey = new CacheKey({
   client_id: TEST_CLIENT_ID,
@@ -77,8 +53,8 @@ const cacheFactories = [
     name: 'Cache with allKeys'
   },
   {
-    new: () => new InMemoryCacheNoKeys(),
-    name: 'Cache using key manifest'
+    new: () => new InMemoryAsyncCacheNoKeys(),
+    name: 'Async cache using key manifest'
   }
 ];
 
diff --git a/__tests__/cache/cache.test.ts b/__tests__/cache/cache.test.ts
index 16f0cb0..9427be7 100644
--- a/__tests__/cache/cache.test.ts
+++ b/__tests__/cache/cache.test.ts
@@ -15,10 +15,15 @@ import {
   nowSeconds,
   TEST_AUDIENCE
 } from '../constants';
+import { InMemoryAsyncCacheNoKeys } from './shared';
 
 const cacheFactories = [
   { new: () => new LocalStorageCache(), name: 'LocalStorage Cache' },
-  { new: () => new InMemoryCache().enclosedCache, name: 'In-memory Cache' }
+  { new: () => new InMemoryCache().enclosedCache, name: 'In-memory Cache' },
+  {
+    new: () => new InMemoryAsyncCacheNoKeys(),
+    name: 'In-memory async cache with no allKeys'
+  }
 ];
 
 const defaultEntry: CacheEntry = {
diff --git a/__tests__/cache/shared.ts b/__tests__/cache/shared.ts
new file mode 100644
index 0000000..7d69ea3
--- /dev/null
+++ b/__tests__/cache/shared.ts
@@ -0,0 +1,25 @@
+import { Cacheable, ICache } from '../../src/cache';
+
+export class InMemoryAsyncCacheNoKeys implements ICache {
+  private cache: Record<string, unknown> = {};
+
+  set<T = Cacheable>(key: string, entry: T) {
+    this.cache[key] = entry;
+    return Promise.resolve();
+  }
+
+  get<T = Cacheable>(key: string) {
+    const cacheEntry = this.cache[key] as T;
+
+    if (!cacheEntry) {
+      return Promise.resolve(null);
+    }
+
+    return Promise.resolve(cacheEntry);
+  }
+
+  remove(key: string) {
+    delete this.cache[key];
+    return Promise.resolve();
+  }
+}