From a8507983eccd00a641ff70b3327c1c6a37281930 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Tue, 21 Feb 2023 15:22:01 +0100 Subject: [PATCH 1/3] Factor out AccountPasswordStore --- src/components/structures/MatrixChat.tsx | 17 ++----- src/contexts/SDKContext.ts | 9 ++++ src/stores/AccountPasswordStore.ts | 43 +++++++++++++++++ test/stores/AccountPasswordStore-test.ts | 61 ++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 14 deletions(-) create mode 100644 src/stores/AccountPasswordStore.ts create mode 100644 test/stores/AccountPasswordStore-test.ts diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 66f681476ba..43c82b61ff1 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -226,8 +226,6 @@ export default class MatrixChat extends React.PureComponent { private screenAfterLogin?: IScreen; private tokenLogin?: boolean; - private accountPassword?: string; - private accountPasswordTimer?: number; private focusComposer: boolean; private subTitleStatus: string; private prevWindowWidth: number; @@ -296,9 +294,6 @@ export default class MatrixChat extends React.PureComponent { Lifecycle.loadSession(); } - this.accountPassword = null; - this.accountPasswordTimer = null; - this.dispatcherRef = dis.register(this.onAction); this.themeWatcher = new ThemeWatcher(); @@ -439,7 +434,7 @@ export default class MatrixChat extends React.PureComponent { this.state.resizeNotifier.removeListener("middlePanelResized", this.dispatchTimelineResize); window.removeEventListener("resize", this.onWindowResized); - if (this.accountPasswordTimer !== null) clearTimeout(this.accountPasswordTimer); + this.stores.accountPasswordStore.clearPassword(); if (this.voiceBroadcastResumer) this.voiceBroadcastResumer.destroy(); } @@ -1987,13 +1982,7 @@ export default class MatrixChat extends React.PureComponent { * this, as they instead jump straight into the app after `attemptTokenLogin`. */ private onUserCompletedLoginFlow = async (credentials: IMatrixClientCreds, password: string): Promise => { - this.accountPassword = password; - // self-destruct the password after 5mins - if (this.accountPasswordTimer !== null) clearTimeout(this.accountPasswordTimer); - this.accountPasswordTimer = window.setTimeout(() => { - this.accountPassword = null; - this.accountPasswordTimer = null; - }, 60 * 5 * 1000); + this.stores.accountPasswordStore.setPassword(password); // Create and start the client await Lifecycle.setLoggedIn(credentials); @@ -2037,7 +2026,7 @@ export default class MatrixChat extends React.PureComponent { view = ( ); diff --git a/src/contexts/SDKContext.ts b/src/contexts/SDKContext.ts index 53e5d8a72bb..3254e69aab8 100644 --- a/src/contexts/SDKContext.ts +++ b/src/contexts/SDKContext.ts @@ -21,6 +21,7 @@ import defaultDispatcher from "../dispatcher/dispatcher"; import LegacyCallHandler from "../LegacyCallHandler"; import { PosthogAnalytics } from "../PosthogAnalytics"; import { SlidingSyncManager } from "../SlidingSyncManager"; +import { AccountPasswordStore } from "../stores/AccountPasswordStore"; import { MemberListStore } from "../stores/MemberListStore"; import { RoomNotificationStateStore } from "../stores/notifications/RoomNotificationStateStore"; import RightPanelStore from "../stores/right-panel/RightPanelStore"; @@ -73,6 +74,7 @@ export class SdkContextClass { protected _VoiceBroadcastRecordingsStore?: VoiceBroadcastRecordingsStore; protected _VoiceBroadcastPreRecordingStore?: VoiceBroadcastPreRecordingStore; protected _VoiceBroadcastPlaybacksStore?: VoiceBroadcastPlaybacksStore; + protected _AccountPasswordStore?: AccountPasswordStore; /** * Automatically construct stores which need to be created eagerly so they can register with @@ -176,4 +178,11 @@ export class SdkContextClass { } return this._VoiceBroadcastPlaybacksStore; } + + public get accountPasswordStore(): AccountPasswordStore { + if (!this._AccountPasswordStore) { + this._AccountPasswordStore = new AccountPasswordStore(); + } + return this._AccountPasswordStore; + } } diff --git a/src/stores/AccountPasswordStore.ts b/src/stores/AccountPasswordStore.ts new file mode 100644 index 00000000000..051f58b7aa1 --- /dev/null +++ b/src/stores/AccountPasswordStore.ts @@ -0,0 +1,43 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +const PASSWORD_TIMEOUT = 5 * 60 * 1000; // five minutes + +/** + * Store for the account password. + * This password can be used for a short time after login + * to avoid requestin the password all the time for instance during e2ee setup. + */ +export class AccountPasswordStore { + private password: string = null; + private passwordTimeoutId: number = null; + + public setPassword(password: string): void { + this.password = password; + clearTimeout(this.passwordTimeoutId); + this.passwordTimeoutId = setTimeout(this.clearPassword, PASSWORD_TIMEOUT); + } + + public getPassword(): string | null { + return this.password; + } + + public clearPassword = (): void => { + clearTimeout(this.passwordTimeoutId); + this.passwordTimeoutId = null; + this.password = null; + }; +} diff --git a/test/stores/AccountPasswordStore-test.ts b/test/stores/AccountPasswordStore-test.ts new file mode 100644 index 00000000000..86b83693251 --- /dev/null +++ b/test/stores/AccountPasswordStore-test.ts @@ -0,0 +1,61 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { AccountPasswordStore } from "../../src/stores/AccountPasswordStore"; + +jest.useFakeTimers(); + +describe("AccountPasswordStore", () => { + let accountPasswordStore: AccountPasswordStore; + + beforeEach(() => { + accountPasswordStore = new AccountPasswordStore(); + }); + + it("should not have a password by default", () => { + expect(accountPasswordStore.getPassword()).toBeNull(); + }); + + describe("when setting a password", () => { + beforeEach(() => { + accountPasswordStore.setPassword("pass1"); + }); + + it("should return the password", () => { + expect(accountPasswordStore.getPassword()).toBe("pass1"); + }); + + describe("and the password timeout exceed", () => { + beforeEach(() => { + jest.advanceTimersToNextTimer(); + }); + + it("should clear the password", () => { + expect(accountPasswordStore.getPassword()).toBeNull(); + }); + }); + + describe("and setting another password", () => { + beforeEach(() => { + accountPasswordStore.setPassword("pass2"); + }); + + it("should return the other password", () => { + expect(accountPasswordStore.getPassword()).toBe("pass2"); + }); + }); + }); +}); From d5bd93005914e855fe6faee480e7ae4ef308da12 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Tue, 21 Feb 2023 17:31:26 +0100 Subject: [PATCH 2/3] Use cached password --- src/stores/SetupEncryptionStore.ts | 16 ++++++ test/stores/SetupEncryptionStore-test.ts | 68 ++++++++++++++++++++++++ test/test-utils/test-utils.ts | 2 + 3 files changed, 86 insertions(+) create mode 100644 test/stores/SetupEncryptionStore-test.ts diff --git a/src/stores/SetupEncryptionStore.ts b/src/stores/SetupEncryptionStore.ts index baedc7de171..d2d2ee0a4d8 100644 --- a/src/stores/SetupEncryptionStore.ts +++ b/src/stores/SetupEncryptionStore.ts @@ -30,6 +30,7 @@ import { AccessCancelledError, accessSecretStorage } from "../SecurityManager"; import Modal from "../Modal"; import InteractiveAuthDialog from "../components/views/dialogs/InteractiveAuthDialog"; import { _t } from "../languageHandler"; +import { SdkContextClass } from "../contexts/SDKContext"; export enum Phase { Loading = 0, @@ -224,6 +225,21 @@ export class SetupEncryptionStore extends EventEmitter { const cli = MatrixClientPeg.get(); await cli.bootstrapCrossSigning({ authUploadDeviceSigningKeys: async (makeRequest): Promise => { + const cachedPassword = SdkContextClass.instance.accountPasswordStore.getPassword(); + + if (cachedPassword) { + await makeRequest({ + type: "m.login.password", + identifier: { + type: "m.id.user", + user: cli.getUserId(), + }, + user: cli.getUserId(), + password: cachedPassword, + }); + return; + } + const { finished } = Modal.createDialog(InteractiveAuthDialog, { title: _t("Setting up keys"), matrixClient: cli, diff --git a/test/stores/SetupEncryptionStore-test.ts b/test/stores/SetupEncryptionStore-test.ts new file mode 100644 index 00000000000..4693176e589 --- /dev/null +++ b/test/stores/SetupEncryptionStore-test.ts @@ -0,0 +1,68 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { mocked, Mocked } from "jest-mock"; +import { IBootstrapCrossSigningOpts } from "matrix-js-sdk/src/crypto"; +import { MatrixClient } from "matrix-js-sdk/src/matrix"; + +import { SdkContextClass } from "../../src/contexts/SDKContext"; +import { accessSecretStorage } from "../../src/SecurityManager"; +import { SetupEncryptionStore } from "../../src/stores/SetupEncryptionStore"; +import { stubClient } from "../test-utils"; + +jest.mock("../../src/SecurityManager", () => ({ + accessSecretStorage: jest.fn(), +})); + +describe("SetupEncryptionStore", () => { + const cachedPassword = "p4assword"; + let client: Mocked; + let setupEncryptionStore: SetupEncryptionStore; + + beforeEach(() => { + client = mocked(stubClient()); + setupEncryptionStore = new SetupEncryptionStore(); + SdkContextClass.instance.accountPasswordStore.setPassword(cachedPassword); + }); + + afterEach(() => { + SdkContextClass.instance.accountPasswordStore.clearPassword(); + }); + + it("resetConfirm should work with a cached account password", async () => { + const makeRequest = jest.fn(); + client.hasSecretStorageKey.mockResolvedValue(true); + client.bootstrapCrossSigning.mockImplementation(async (opts: IBootstrapCrossSigningOpts) => { + await opts.authUploadDeviceSigningKeys(makeRequest); + }); + mocked(accessSecretStorage).mockImplementation(async (func) => { + await func(); + }); + + await setupEncryptionStore.resetConfirm(); + + expect(mocked(accessSecretStorage)).toHaveBeenCalledWith(expect.any(Function), true); + expect(makeRequest).toHaveBeenCalledWith({ + identifier: { + type: "m.id.user", + user: "@userId:matrix.org", + }, + password: cachedPassword, + type: "m.login.password", + user: "@userId:matrix.org", + }); + }); +}); diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index fb2910145c0..8ac6f74a387 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -102,6 +102,8 @@ export function createTestClient(): MatrixClient { getDevices: jest.fn().mockResolvedValue({ devices: [{ device_id: "ABCDEFGHI" }] }), getSessionId: jest.fn().mockReturnValue("iaszphgvfku"), credentials: { userId: "@userId:matrix.org" }, + bootstrapCrossSigning: jest.fn(), + hasSecretStorageKey: jest.fn(), store: { getPendingEvents: jest.fn().mockResolvedValue([]), From 6454a83402f86ee50615ee7044b9adf0758ef17e Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Wed, 22 Feb 2023 10:15:46 +0100 Subject: [PATCH 3/3] Fix types --- src/stores/AccountPasswordStore.ts | 10 +++++----- test/stores/AccountPasswordStore-test.ts | 4 ++-- test/stores/SetupEncryptionStore-test.ts | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/stores/AccountPasswordStore.ts b/src/stores/AccountPasswordStore.ts index 051f58b7aa1..7b7b5a85e98 100644 --- a/src/stores/AccountPasswordStore.ts +++ b/src/stores/AccountPasswordStore.ts @@ -22,8 +22,8 @@ const PASSWORD_TIMEOUT = 5 * 60 * 1000; // five minutes * to avoid requestin the password all the time for instance during e2ee setup. */ export class AccountPasswordStore { - private password: string = null; - private passwordTimeoutId: number = null; + private password?: string; + private passwordTimeoutId?: ReturnType; public setPassword(password: string): void { this.password = password; @@ -31,13 +31,13 @@ export class AccountPasswordStore { this.passwordTimeoutId = setTimeout(this.clearPassword, PASSWORD_TIMEOUT); } - public getPassword(): string | null { + public getPassword(): string | undefined { return this.password; } public clearPassword = (): void => { clearTimeout(this.passwordTimeoutId); - this.passwordTimeoutId = null; - this.password = null; + this.passwordTimeoutId = undefined; + this.password = undefined; }; } diff --git a/test/stores/AccountPasswordStore-test.ts b/test/stores/AccountPasswordStore-test.ts index 86b83693251..f54886fd1b9 100644 --- a/test/stores/AccountPasswordStore-test.ts +++ b/test/stores/AccountPasswordStore-test.ts @@ -26,7 +26,7 @@ describe("AccountPasswordStore", () => { }); it("should not have a password by default", () => { - expect(accountPasswordStore.getPassword()).toBeNull(); + expect(accountPasswordStore.getPassword()).toBeUndefined(); }); describe("when setting a password", () => { @@ -44,7 +44,7 @@ describe("AccountPasswordStore", () => { }); it("should clear the password", () => { - expect(accountPasswordStore.getPassword()).toBeNull(); + expect(accountPasswordStore.getPassword()).toBeUndefined(); }); }); diff --git a/test/stores/SetupEncryptionStore-test.ts b/test/stores/SetupEncryptionStore-test.ts index 4693176e589..f2e27948f24 100644 --- a/test/stores/SetupEncryptionStore-test.ts +++ b/test/stores/SetupEncryptionStore-test.ts @@ -46,9 +46,9 @@ describe("SetupEncryptionStore", () => { const makeRequest = jest.fn(); client.hasSecretStorageKey.mockResolvedValue(true); client.bootstrapCrossSigning.mockImplementation(async (opts: IBootstrapCrossSigningOpts) => { - await opts.authUploadDeviceSigningKeys(makeRequest); + await opts?.authUploadDeviceSigningKeys(makeRequest); }); - mocked(accessSecretStorage).mockImplementation(async (func) => { + mocked(accessSecretStorage).mockImplementation(async (func: () => Promise) => { await func(); });