diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 38852c68fba..47e44c43dc1 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -211,7 +211,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) { @@ -556,17 +556,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) { @@ -590,10 +592,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"); @@ -603,7 +606,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 53514ab817b..53d4c96acaf 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -375,7 +375,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 f526775e63e..a82259242aa 100644 --- a/src/utils/tokens/tokens.ts +++ b/src/utils/tokens/tokens.ts @@ -24,13 +24,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"; @@ -68,50 +68,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, @@ -130,12 +143,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 { @@ -167,9 +180,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, @@ -185,9 +200,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 f9146630f9b..6ee6cb11540 100644 --- a/test/Lifecycle-test.ts +++ b/test/Lifecycle-test.ts @@ -23,7 +23,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"; @@ -145,7 +145,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"; @@ -180,7 +185,7 @@ describe("Lifecycle", () => { mac: expect.any(String), }; - describe("restoreFromLocalStorage()", () => { + describe("restoreSessionFromStorage()", () => { beforeEach(() => { initLocalStorageMock(); initSessionStorageMock(); @@ -204,18 +209,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(); }); @@ -228,12 +233,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({ @@ -253,7 +258,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"); @@ -267,7 +272,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 @@ -275,7 +280,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( { @@ -295,13 +300,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(); }); @@ -316,7 +321,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"); @@ -324,7 +329,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( { @@ -362,7 +367,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"); @@ -384,7 +389,7 @@ describe("Lifecycle", () => { }, ); - expect(await restoreFromLocalStorage()).toEqual(true); + expect(await restoreSessionFromStorage()).toEqual(true); expect(StorageAccess.idbSave).toHaveBeenCalledWith( "account", @@ -403,7 +408,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( @@ -430,7 +435,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"); @@ -442,7 +447,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( { @@ -492,7 +497,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( @@ -519,7 +524,23 @@ 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); + + console.log("10"); + expect(await restoreSessionFromStorage()).toEqual(true); }); }); });