diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 8f26fd0623..e83b8df20d 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -203,7 +203,7 @@ export async function loadSession(opts: ILoadSessionOpts = {}): Promise false, ).then(() => true); } - const success = await restoreFromLocalStorage({ + const success = await restoreSessionFromStorage({ ignoreGuest: Boolean(opts.ignoreGuest), }); if (success) { @@ -548,17 +548,19 @@ async function abortLogin(): Promise { } } -// returns a promise which resolves to true if a session is found in -// localstorage -// -// N.B. Lifecycle.js should not maintain any further localStorage state, we -// are moving towards using SessionStore to keep track of state related -// to the current session (which is typically backed by localStorage). -// -// The plan is to gradually move the localStorage access done here into -// SessionStore to avoid bugs where the view becomes out-of-sync with -// localStorage (e.g. isGuest etc.) -export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): Promise { +/** Attempt to restore the session from localStorage or indexeddb. + * + * @returns true if a session was found; false if no existing session was found. + * + * N.B. Lifecycle.js should not maintain any further localStorage state, we + * are moving towards using SessionStore to keep track of state related + * to the current session (which is typically backed by localStorage). + * + * The plan is to gradually move the localStorage access done here into + * SessionStore to avoid bugs where the view becomes out-of-sync with + * localStorage (e.g. isGuest etc.) + */ +export async function restoreSessionFromStorage(opts?: { ignoreGuest?: boolean }): Promise { const ignoreGuest = opts?.ignoreGuest; if (!localStorage) { @@ -582,10 +584,11 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): if (pickleKey) { logger.log(`Got pickle key for ${userId}|${deviceId}`); } else { - logger.log("No pickle key available"); + logger.log(`No pickle key available for ${userId}|${deviceId}`); } const decryptedAccessToken = await tryDecryptToken(pickleKey, accessToken, ACCESS_TOKEN_IV); - const decryptedRefreshToken = await tryDecryptToken(pickleKey, refreshToken, REFRESH_TOKEN_IV); + const decryptedRefreshToken = + refreshToken && (await tryDecryptToken(pickleKey, refreshToken, REFRESH_TOKEN_IV)); const freshLogin = sessionStorage.getItem("mx_fresh_login") === "true"; sessionStorage.removeItem("mx_fresh_login"); @@ -595,7 +598,7 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): { userId: userId, deviceId: deviceId, - accessToken: decryptedAccessToken!, + accessToken: decryptedAccessToken, refreshToken: decryptedRefreshToken, homeserverUrl: hsUrl, identityServerUrl: isUrl, diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 2f3e590239..cae0a549b7 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -367,7 +367,7 @@ export default class MatrixChat extends React.PureComponent { // Create and start the client // accesses the new credentials just set in storage during attemptDelegatedAuthLogin // and sets logged in state - await Lifecycle.restoreFromLocalStorage({ ignoreGuest: true }); + await Lifecycle.restoreSessionFromStorage({ ignoreGuest: true }); await this.postLoginSetup(); return; } diff --git a/src/utils/tokens/tokens.ts b/src/utils/tokens/tokens.ts index 4655d4e613..febd7f23ff 100644 --- a/src/utils/tokens/tokens.ts +++ b/src/utils/tokens/tokens.ts @@ -16,13 +16,13 @@ import * as StorageAccess from "../StorageAccess"; */ /* - * Keys used when storing the tokens in indexeddb or localstorage + * Names used when storing the tokens in indexeddb or localstorage */ export const ACCESS_TOKEN_STORAGE_KEY = "mx_access_token"; export const REFRESH_TOKEN_STORAGE_KEY = "mx_refresh_token"; /* - * Used as initialization vector during encryption in persistTokenInStorage - * And decryption in restoreFromLocalStorage + * Names of the tokens. Used as part of the calculation to derive AES keys during encryption in persistTokenInStorage, + * and decryption in restoreSessionFromStorage. */ export const ACCESS_TOKEN_IV = "access_token"; export const REFRESH_TOKEN_IV = "refresh_token"; @@ -60,50 +60,63 @@ async function pickleKeyToAesKey(pickleKey: string): Promise { ); } -const isEncryptedPayload = (token?: IEncryptedPayload | string | undefined): token is IEncryptedPayload => { - return !!token && typeof token !== "string"; -}; /** * Try to decrypt a token retrieved from storage - * Where token is not encrypted (plain text) returns the plain text token - * Where token is encrypted, attempts decryption. Returns successfully decrypted token, else undefined. - * @param pickleKey pickle key used during encryption of token, or undefined - * @param token - * @param tokenIv initialization vector used during encryption of token eg ACCESS_TOKEN_IV - * @returns the decrypted token, or the plain text token. Returns undefined when token cannot be decrypted + * + * Where token is not encrypted (plain text) returns the plain text token. + * + * Where token is encrypted, attempts decryption. Returns successfully decrypted token, or throws if + * decryption failed. + * + * @param pickleKey Pickle key: used to derive the encryption key, or undefined if the token is not encrypted. + * Must be the same as provided to {@link persistTokenInStorage}. + * @param token token to be decrypted. + * @param tokenName Name of the token. Used in logging, but also used as an input when generating the actual AES key, + * so the same value must be provided to {@link persistTokenInStorage}. + * + * @returns the decrypted token, or the plain text token. */ export async function tryDecryptToken( pickleKey: string | undefined, - token: IEncryptedPayload | string | undefined, - tokenIv: string, -): Promise { - if (pickleKey && isEncryptedPayload(token)) { - const encrKey = await pickleKeyToAesKey(pickleKey); - const decryptedToken = await decryptAES(token, encrKey, tokenIv); - encrKey.fill(0); - return decryptedToken; - } - // if the token wasn't encrypted (plain string) just return it back + token: IEncryptedPayload | string, + tokenName: string, +): Promise { if (typeof token === "string") { + // Looks like an unencrypted token return token; } - // otherwise return undefined + + // Otherwise, it must be an encrypted token. + if (!pickleKey) { + throw new Error(`Error decrypting secret ${tokenName}: no pickle key found.`); + } + + const encrKey = await pickleKeyToAesKey(pickleKey); + const decryptedToken = await decryptAES(token, encrKey, tokenName); + encrKey.fill(0); + return decryptedToken; } /** * Persist a token in storage - * When pickle key is present, will attempt to encrypt the token - * Stores in idb, falling back to localStorage * - * @param storageKey key used to store the token - * @param initializationVector Initialization vector for encrypting the token. Only used when `pickleKey` is present - * @param token the token to store, when undefined any existing token at the storageKey is removed from storage - * @param pickleKey optional pickle key used to encrypt token - * @param hasTokenStorageKey Localstorage key for an item which stores whether we expect to have a token in indexeddb, eg "mx_has_access_token". + * When pickle key is present, will attempt to encrypt the token. If encryption fails (typically because + * WebCrypto is unavailable), the key will be stored unencrypted. + * + * Stores in IndexedDB, falling back to localStorage. + * + * @param storageKey key used to store the token. Note: not an encryption key; rather a localstorage or indexeddb key. + * @param tokenName Name of the token. Used in logging, but also used as an input when generating the actual AES key, + * so the same value must be provided to {@link tryDecryptToken} when decrypting. + * @param token the token to store. When undefined, any existing token at the `storageKey` is removed from storage. + * @param pickleKey Pickle key: used to derive the key used to encrypt token. If `undefined`, the token will be stored + * unencrypted. + * @param hasTokenStorageKey Localstorage key for an item which stores whether we expect to have a token in indexeddb, + * eg "mx_has_access_token". */ export async function persistTokenInStorage( storageKey: string, - initializationVector: string, + tokenName: string, token: string | undefined, pickleKey: string | undefined, hasTokenStorageKey: string, @@ -122,12 +135,12 @@ export async function persistTokenInStorage( try { // try to encrypt the access token using the pickle key const encrKey = await pickleKeyToAesKey(pickleKey); - encryptedToken = await encryptAES(token, encrKey, initializationVector); + encryptedToken = await encryptAES(token, encrKey, tokenName); encrKey.fill(0); } catch (e) { // This is likely due to the browser not having WebCrypto or somesuch. // Warn about it, but fall back to storing the unencrypted token. - logger.warn(`Could not encrypt token for ${storageKey}`, e); + logger.warn(`Could not encrypt token for ${tokenName}`, e); } } try { @@ -159,9 +172,11 @@ export async function persistTokenInStorage( } /** - * Wraps persistTokenInStorage with accessToken storage keys - * @param token the token to store, when undefined any existing accessToken is removed from storage - * @param pickleKey optional pickle key used to encrypt token + * Wraps {@link persistTokenInStorage} with accessToken storage keys + * + * @param token - The token to store. When undefined, any existing accessToken is removed from storage. + * @param pickleKey - Pickle key: used to derive the key used to encrypt token. If `undefined`, the token will be stored + * unencrypted. */ export async function persistAccessTokenInStorage( token: string | undefined, @@ -177,9 +192,11 @@ export async function persistAccessTokenInStorage( } /** - * Wraps persistTokenInStorage with refreshToken storage keys - * @param token the token to store, when undefined any existing refreshToken is removed from storage - * @param pickleKey optional pickle key used to encrypt token + * Wraps {@link persistTokenInStorage} with refreshToken storage keys. + * + * @param token - The token to store. When undefined, any existing refreshToken is removed from storage. + * @param pickleKey - Pickle key: used to derive the key used to encrypt token. If `undefined`, the token will be stored + * unencrypted. */ export async function persistRefreshTokenInStorage( token: string | undefined, diff --git a/test/Lifecycle-test.ts b/test/Lifecycle-test.ts index 61af46e9de..04c7408b68 100644 --- a/test/Lifecycle-test.ts +++ b/test/Lifecycle-test.ts @@ -15,7 +15,7 @@ import { mocked, MockedObject } from "jest-mock"; import fetchMock from "fetch-mock-jest"; import StorageEvictedDialog from "../src/components/views/dialogs/StorageEvictedDialog"; -import { logout, restoreFromLocalStorage, setLoggedIn } from "../src/Lifecycle"; +import { logout, restoreSessionFromStorage, setLoggedIn } from "../src/Lifecycle"; import { MatrixClientPeg } from "../src/MatrixClientPeg"; import Modal from "../src/Modal"; import * as StorageAccess from "../src/utils/StorageAccess"; @@ -137,7 +137,12 @@ describe("Lifecycle", () => { mockStore[tableKey] = table; }, ); - jest.spyOn(StorageAccess, "idbDelete").mockClear().mockResolvedValue(undefined); + jest.spyOn(StorageAccess, "idbDelete") + .mockClear() + .mockImplementation(async (tableKey: string, key: string | string[]) => { + const table = mockStore[tableKey]; + delete table?.[key as string]; + }); }; const homeserverUrl = "https://server.org"; @@ -172,7 +177,7 @@ describe("Lifecycle", () => { mac: expect.any(String), }; - describe("restoreFromLocalStorage()", () => { + describe("restoreSessionFromStorage()", () => { beforeEach(() => { initLocalStorageMock(); initSessionStorageMock(); @@ -196,18 +201,18 @@ describe("Lifecycle", () => { // @ts-ignore dirty mocking global.localStorage = undefined; - expect(await restoreFromLocalStorage()).toEqual(false); + expect(await restoreSessionFromStorage()).toEqual(false); }); it("should return false when no session data is found in local storage", async () => { - expect(await restoreFromLocalStorage()).toEqual(false); + expect(await restoreSessionFromStorage()).toEqual(false); expect(logger.log).toHaveBeenCalledWith("No previous session found."); }); it("should abort login when we expect to find an access token but don't", async () => { initLocalStorageMock({ mx_has_access_token: "true" }); - await expect(() => restoreFromLocalStorage()).rejects.toThrow(); + await expect(() => restoreSessionFromStorage()).rejects.toThrow(); expect(Modal.createDialog).toHaveBeenCalledWith(StorageEvictedDialog); expect(mockClient.clearStores).toHaveBeenCalled(); }); @@ -220,12 +225,12 @@ describe("Lifecycle", () => { }); it("should ignore guest accounts when ignoreGuest is true", async () => { - expect(await restoreFromLocalStorage({ ignoreGuest: true })).toEqual(false); + expect(await restoreSessionFromStorage({ ignoreGuest: true })).toEqual(false); expect(logger.log).toHaveBeenCalledWith(`Ignoring stored guest account: ${userId}`); }); it("should restore guest accounts when ignoreGuest is false", async () => { - expect(await restoreFromLocalStorage({ ignoreGuest: false })).toEqual(true); + expect(await restoreSessionFromStorage({ ignoreGuest: false })).toEqual(true); expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( expect.objectContaining({ @@ -245,7 +250,7 @@ describe("Lifecycle", () => { }); it("should persist credentials", async () => { - expect(await restoreFromLocalStorage()).toEqual(true); + expect(await restoreSessionFromStorage()).toEqual(true); expect(localStorage.setItem).toHaveBeenCalledWith("mx_user_id", userId); expect(localStorage.setItem).toHaveBeenCalledWith("mx_has_access_token", "true"); @@ -259,7 +264,7 @@ describe("Lifecycle", () => { it("should persist access token when idb is not available", async () => { jest.spyOn(StorageAccess, "idbSave").mockRejectedValue("oups"); - expect(await restoreFromLocalStorage()).toEqual(true); + expect(await restoreSessionFromStorage()).toEqual(true); expect(StorageAccess.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken); // put accessToken in localstorage as fallback @@ -267,7 +272,7 @@ describe("Lifecycle", () => { }); it("should create and start new matrix client with credentials", async () => { - expect(await restoreFromLocalStorage()).toEqual(true); + expect(await restoreSessionFromStorage()).toEqual(true); expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( { @@ -287,13 +292,13 @@ describe("Lifecycle", () => { }); it("should remove fresh login flag from session storage", async () => { - expect(await restoreFromLocalStorage()).toEqual(true); + expect(await restoreSessionFromStorage()).toEqual(true); expect(sessionStorage.removeItem).toHaveBeenCalledWith("mx_fresh_login"); }); it("should start matrix client", async () => { - expect(await restoreFromLocalStorage()).toEqual(true); + expect(await restoreSessionFromStorage()).toEqual(true); expect(MatrixClientPeg.start).toHaveBeenCalled(); }); @@ -308,7 +313,7 @@ describe("Lifecycle", () => { }); it("should persist credentials", async () => { - expect(await restoreFromLocalStorage()).toEqual(true); + expect(await restoreSessionFromStorage()).toEqual(true); // refresh token from storage is re-persisted expect(localStorage.setItem).toHaveBeenCalledWith("mx_has_refresh_token", "true"); @@ -316,7 +321,7 @@ describe("Lifecycle", () => { }); it("should create new matrix client with credentials", async () => { - expect(await restoreFromLocalStorage()).toEqual(true); + expect(await restoreSessionFromStorage()).toEqual(true); expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( { @@ -354,7 +359,7 @@ describe("Lifecycle", () => { }); it("should persist credentials", async () => { - expect(await restoreFromLocalStorage()).toEqual(true); + expect(await restoreSessionFromStorage()).toEqual(true); expect(localStorage.setItem).toHaveBeenCalledWith("mx_has_access_token", "true"); @@ -376,7 +381,7 @@ describe("Lifecycle", () => { }, ); - expect(await restoreFromLocalStorage()).toEqual(true); + expect(await restoreSessionFromStorage()).toEqual(true); expect(StorageAccess.idbSave).toHaveBeenCalledWith( "account", @@ -395,7 +400,7 @@ describe("Lifecycle", () => { }); // Perform the restore - expect(await restoreFromLocalStorage()).toEqual(true); + expect(await restoreSessionFromStorage()).toEqual(true); // Ensure that the expected calls were made expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( @@ -422,7 +427,7 @@ describe("Lifecycle", () => { }); it("should persist credentials", async () => { - expect(await restoreFromLocalStorage()).toEqual(true); + expect(await restoreSessionFromStorage()).toEqual(true); // refresh token from storage is re-persisted expect(localStorage.setItem).toHaveBeenCalledWith("mx_has_refresh_token", "true"); @@ -434,7 +439,7 @@ describe("Lifecycle", () => { }); it("should create new matrix client with credentials", async () => { - expect(await restoreFromLocalStorage()).toEqual(true); + expect(await restoreSessionFromStorage()).toEqual(true); expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( { @@ -484,7 +489,7 @@ describe("Lifecycle", () => { it("should create and start new matrix client with credentials", async () => { // Perform the restore - expect(await restoreFromLocalStorage()).toEqual(true); + expect(await restoreSessionFromStorage()).toEqual(true); // Ensure that the expected calls were made expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( @@ -511,7 +516,24 @@ describe("Lifecycle", () => { initIdbMock(idbStorageSession); mockClient.isVersionSupported.mockRejectedValue(new Error("Oh, noes, the server is down!")); - expect(await restoreFromLocalStorage()).toEqual(true); + expect(await restoreSessionFromStorage()).toEqual(true); + }); + + it("should throw if the token was persisted with a pickle key but there is no pickle key available now", async () => { + initLocalStorageMock(localStorageSession); + initIdbMock({}); + + // Create a pickle key, and store it, encrypted, in IDB. + const pickleKey = (await PlatformPeg.get()!.createPickleKey(credentials.userId, credentials.deviceId))!; + localStorage.setItem("mx_has_pickle_key", "true"); + await persistAccessTokenInStorage(credentials.accessToken, pickleKey); + + // Now destroy the pickle key + await PlatformPeg.get()!.destroyPickleKey(credentials.userId, credentials.deviceId); + + await expect(restoreSessionFromStorage()).rejects.toThrow( + "Error decrypting secret access_token: no pickle key found.", + ); }); }); });