(core) Fix problem with localStorage in some cross-origin embed situations

Summary:
- Handle the possibility that any access to localStorage causes error.
- Move getStorage() and getSessionStorage() safe functions to a separate file.
- Use these safe functions in more places.

Test Plan:
Added a test case, using a webdriver instance that blocks third-party cookies,
to enforce third-party restrictions. Added to gristUtil a way to override the
webdriver instance.

Reviewers: jarek

Reviewed By: jarek

Differential Revision: https://phab.getgrist.com/D3719

app/client/components/CursorMonitor.ts

@@ -1,6 +1,6 @@
 import {CursorPos} from 'app/client/components/Cursor';
 import {GristDoc} from 'app/client/components/GristDoc';
-import {getStorage} from 'app/client/lib/localStorageObs';
+import {getStorage} from 'app/client/lib/storage';
 import {IDocPage, isViewDocPage, ViewDocPage} from 'app/common/gristUrls';
 import {Disposable, Listener, Observable} from 'grainjs';
 import {reportError} from 'app/client/models/errors';

app/client/components/EditorMonitor.ts

@@ -1,7 +1,7 @@
 import {CellPosition, toCursor} from 'app/client/components/CellPosition';
 import {oneTimeListener} from 'app/client/components/CursorMonitor';
 import {GristDoc} from 'app/client/components/GristDoc';
-import {getStorage} from 'app/client/lib/localStorageObs';
+import {getStorage} from 'app/client/lib/storage';
 import {UserError} from 'app/client/models/errors';
 import {FieldEditor, FieldEditorStateEvent} from 'app/client/widgets/FieldEditor';
 import {isViewDocPage} from 'app/common/gristUrls';

app/client/components/GristWSConnection.ts

@@ -1,5 +1,6 @@
 import {get as getBrowserGlobals} from 'app/client/lib/browserGlobals';
 import {guessTimezone} from 'app/client/lib/guessTimezone';
+import {getSessionStorage} from 'app/client/lib/storage';
 import {getWorker} from 'app/client/models/gristConfigCache';
 import {CommResponseBase} from 'app/common/CommTypes';
 import * as gutil from 'app/common/gutil';
@@ -70,6 +71,8 @@ export interface GristWSSettings {
  * An implementation of Grist websocket connection settings for the browser.
  */
 export class GristWSSettingsBrowser implements GristWSSettings {
+  private _sessionStorage = getSessionStorage();
+
   public makeWebSocket(url: string) { return new WebSocket(url); }
   public getTimezone()              { return guessTimezone(); }
   public getPageUrl()               { return G.window.location.href; }
@@ -77,18 +80,18 @@ export class GristWSSettingsBrowser implements GristWSSettings {
     return getDocWorkerUrl(assignmentId);
   }
   public getClientId(assignmentId: string|null) {
-    return window.sessionStorage.getItem(`clientId_${assignmentId}`) || null;
+    return this._sessionStorage.getItem(`clientId_${assignmentId}`) || null;
   }
   public getUserSelector(): string {
     // TODO: find/create a more official way to get the user.
     return (window as any).gristDocPageModel?.appModel.currentUser?.email || '';
   }
   public updateClientId(assignmentId: string|null, id: string) {
-    window.sessionStorage.setItem(`clientId_${assignmentId}`, id);
+    this._sessionStorage.setItem(`clientId_${assignmentId}`, id);
   }
   public advanceCounter(): string {
-    const value = parseInt(window.sessionStorage.getItem('clientCounter')!, 10) || 0;
+    const value = parseInt(this._sessionStorage.getItem('clientCounter')!, 10) || 0;
-    window.sessionStorage.setItem('clientCounter', String(value + 1));
+    this._sessionStorage.setItem('clientCounter', String(value + 1));
     return String(value);
   }
   public log(...args: any[]): void {

app/client/lib/localStorageObs.ts

@@ -1,68 +1,6 @@
 import {safeJsonParse} from 'app/common/gutil';
 import {Observable} from 'grainjs';
-
+import {getSessionStorage, getStorage} from 'app/client/lib/storage';
-/**
- * Returns true if storage is functional. In some cass (e.g. when embedded), localStorage may
- * throw errors. If so, we return false. This implementation is the approach taken by store.js.
- */
-function testStorage(storage: Storage) {
-  try {
-    const testStr = '__localStorage_test';
-    storage.setItem(testStr, testStr);
-    const ok = (storage.getItem(testStr) === testStr);
-    storage.removeItem(testStr);
-    return ok;
-  } catch (e) {
-    return false;
-  }
-}
-
-/**
- * Returns localStorage if functional, or sessionStorage, or an in-memory storage. The fallbacks
- * help with tests, and may help when Grist is embedded.
- */
-export function getStorage(): Storage {
-  return _storage || (_storage = createStorage());
-}
-
-/**
- * Similar to `getStorage`, but always returns sessionStorage (or an in-memory equivalent).
- */
-export function getSessionStorage(): Storage {
-  return _sessionStorage || (_sessionStorage = createSessionStorage());
-}
-
-let _storage: Storage|undefined;
-let _sessionStorage: Storage|undefined;
-
-function createStorage(): Storage {
-  if (typeof localStorage !== 'undefined' && testStorage(localStorage)) {
-    return localStorage;
-  } else {
-    return createSessionStorage();
-  }
-}
-
-function createSessionStorage(): Storage {
-  if (typeof sessionStorage !== 'undefined' && testStorage(sessionStorage)) {
-    return sessionStorage;
-  } else {
-    // Fall back to a Map-based implementation of (non-persistent) sessionStorage.
-    return createInMemoryStorage();
-  }
-}
-
-function createInMemoryStorage(): Storage {
-  const values = new Map<string, string>();
-  return {
-    setItem(key: string, val: string) { values.set(key, val); },
-    getItem(key: string) { return values.get(key) ?? null; },
-    removeItem(key: string) { values.delete(key); },
-    clear() { values.clear(); },
-    get length() { return values.size; },
-    key(index: number): string|null { throw new Error('Not implemented'); },
-  };
-}
 
 function getStorageBoolObs(store: Storage, key: string, defValue: boolean) {
   const storedNegation = defValue ? 'false' : 'true';

app/client/lib/sessionObs.ts

@@ -4,6 +4,7 @@
  */
 import {safeJsonParse} from 'app/common/gutil';
 import {IDisposableOwner, Observable} from 'grainjs';
+import {getSessionStorage} from 'app/client/lib/storage';
 
 export interface SessionObs<T> extends Observable<T> {
   pauseSaving(yesNo: boolean): void;
@@ -45,14 +46,15 @@ export function createSessionObs<T>(
     return value === _default || !isValid(value) ? null : JSON.stringify(value);
   }
   let _pauseSaving = false;
-  const obs = Observable.create<T>(owner, fromString(window.sessionStorage.getItem(key)));
+  const storage = getSessionStorage();
+  const obs = Observable.create<T>(owner, fromString(storage.getItem(key)));
   obs.addListener((value: T) => {
     if (_pauseSaving) { return; }
     const stored = toString(value);
     if (stored == null) {
-      window.sessionStorage.removeItem(key);
+      storage.removeItem(key);
     } else {
-      window.sessionStorage.setItem(key, stored);
+      storage.setItem(key, stored);
     }
   });
   return Object.assign(obs, {pauseSaving(yesNo: boolean) { _pauseSaving = yesNo; }});

app/client/lib/storage.ts

@@ -0,0 +1,65 @@
+/**
+ * Expose localStorage and sessionStorage with fallbacks for cases when they don't work (e.g.
+ * cross-domain embeds in Firefox and Safari).
+ *
+ * Usage:
+ *    import {getStorage, getSessionStorage} from 'app/client/lib/storage';
+ *    ... use getStorage() in place of localStorage...
+ *    ... use getSessionStorage() in place of sessionStorage...
+ */
+
+/**
+ * Returns localStorage if functional, or sessionStorage, or an in-memory storage. The fallbacks
+ * help with tests, and when Grist is embedded.
+ */
+export function getStorage(): Storage {
+  _storage ??= testStorage('localStorage') || getSessionStorage();
+  return _storage;
+}
+
+/**
+ * Return window.sessionStorage, or when not available, an in-memory storage.
+ */
+export function getSessionStorage(): Storage {
+  // If can't use sessionStorage, fall back to a Map-based non-persistent implementation.
+  _sessionStorage ??= testStorage('sessionStorage') || createInMemoryStorage();
+  return _sessionStorage;
+}
+
+
+let _storage: Storage|undefined;
+let _sessionStorage: Storage|undefined;
+
+/**
+ * Returns the result of window[storageName] if storage is functional, or null otherwise. In some
+ * cases (e.g. when embedded), using localStorage may throw errors, in which case we return null.
+ * This is similar to the approach taken by store.js.
+ */
+function testStorage(storageName: 'localStorage'|'sessionStorage'): Storage|null {
+  try {
+    const testStr = '__localStorage_test';
+    const storage = window[storageName];
+    storage.setItem(testStr, testStr);
+    const ok = (storage.getItem(testStr) === testStr);
+    storage.removeItem(testStr);
+    if (ok) {
+      return storage;
+    }
+  } catch (e) {
+    // Fall through
+  }
+  console.warn(`${storageName} is not available; will use fallback`);
+  return null;
+}
+
+function createInMemoryStorage(): Storage {
+  const values = new Map<string, string>();
+  return {
+    setItem(key: string, val: string) { values.set(key, val); },
+    getItem(key: string) { return values.get(key) ?? null; },
+    removeItem(key: string) { values.delete(key); },
+    clear() { values.clear(); },
+    get length() { return values.size; },
+    key(index: number): string|null { throw new Error('Not implemented'); },
+  };
+}

app/client/ui2018/cssVars.ts

@@ -6,6 +6,7 @@
  *  https://css-tricks.com/snippets/css/system-font-stack/
  *
  */
+import {getStorage} from 'app/client/lib/storage';
 import {urlState} from 'app/client/models/gristUrlState';
 import {getTheme, ProductFlavor} from 'app/client/ui/CustomThemes';
 import {Theme, ThemeAppearance} from 'app/common/ThemePrefs';
@@ -838,7 +839,7 @@ ${properties.join('\n')}
 
   // Cache the appearance in local storage; this is currently used to apply a suitable
   // background image that's shown while the application is loading.
-  localStorage.setItem('appearance', appearance);
+  getStorage().setItem('appearance', appearance);
 }
 
 /**

test/nbrowser/gristUtils.ts

@@ -7,8 +7,8 @@ import * as fse from 'fs-extra';
 import escapeRegExp = require('lodash/escapeRegExp');
 import noop = require('lodash/noop');
 import startCase = require('lodash/startCase');
-import { assert, driver, error, Key, WebElement, WebElementPromise } from 'mocha-webdriver';
+import { assert, driver as driverOrig, error, Key, WebElement, WebElementPromise } from 'mocha-webdriver';
-import { stackWrapFunc, stackWrapOwnMethods } from 'mocha-webdriver';
+import { stackWrapFunc, stackWrapOwnMethods, WebDriver } from 'mocha-webdriver';
 import * as path from 'path';
 
 import { decodeUrl } from 'app/common/gristUrls';
@@ -32,6 +32,15 @@ import type { AssertionError } from 'assert';
 // Wrap in a namespace so that we can apply stackWrapOwnMethods to all the exports together.
 namespace gristUtils {
 
+// Allow overriding the global 'driver' to use in gristUtil.
+let driver: WebDriver;
+
+// Substitute a custom driver to use with gristUtils functions. Omit argument to restore to default.
+export function setDriver(customDriver: WebDriver = driverOrig) { driver = customDriver; }
+
+// Set the 'driver' to use here in before() callback, because driverOrig isn't set until then.
+before(() => setDriver());
+
 const homeUtil = new HomeUtil(testUtils.fixturesRoot, server);
 
 export const createNewDoc = homeUtil.createNewDoc.bind(homeUtil);