From eb6278df1dc49f0fa15c9f6304f7b943ba01149e Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Thu, 23 Feb 2023 08:46:49 +0100 Subject: [PATCH] =?UTF-8?q?Do=20not=20prompt=20for=20a=20password=20when?= =?UTF-8?q?=20doing=20a=20=E2=80=9Ereset=20all=E2=80=9C=20after=20login=20?= =?UTF-8?q?(#10208)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/components/structures/MatrixChat.tsx | 17 ++---- src/contexts/SDKContext.ts | 9 ++++ src/stores/AccountPasswordStore.ts | 43 +++++++++++++++ src/stores/SetupEncryptionStore.ts | 16 ++++++ test/stores/AccountPasswordStore-test.ts | 61 +++++++++++++++++++++ test/stores/SetupEncryptionStore-test.ts | 68 ++++++++++++++++++++++++ test/test-utils/test-utils.ts | 2 + 7 files changed, 202 insertions(+), 14 deletions(-) create mode 100644 src/stores/AccountPasswordStore.ts create mode 100644 test/stores/AccountPasswordStore-test.ts create mode 100644 test/stores/SetupEncryptionStore-test.ts diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 66f681476b..43c82b61ff 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 53e5d8a72b..3254e69aab 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 0000000000..7b7b5a85e9 --- /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; + private passwordTimeoutId?: ReturnType; + + public setPassword(password: string): void { + this.password = password; + clearTimeout(this.passwordTimeoutId); + this.passwordTimeoutId = setTimeout(this.clearPassword, PASSWORD_TIMEOUT); + } + + public getPassword(): string | undefined { + return this.password; + } + + public clearPassword = (): void => { + clearTimeout(this.passwordTimeoutId); + this.passwordTimeoutId = undefined; + this.password = undefined; + }; +} diff --git a/src/stores/SetupEncryptionStore.ts b/src/stores/SetupEncryptionStore.ts index baedc7de17..d2d2ee0a4d 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/AccountPasswordStore-test.ts b/test/stores/AccountPasswordStore-test.ts new file mode 100644 index 0000000000..f54886fd1b --- /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()).toBeUndefined(); + }); + + 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()).toBeUndefined(); + }); + }); + + describe("and setting another password", () => { + beforeEach(() => { + accountPasswordStore.setPassword("pass2"); + }); + + it("should return the other password", () => { + expect(accountPasswordStore.getPassword()).toBe("pass2"); + }); + }); + }); +}); diff --git a/test/stores/SetupEncryptionStore-test.ts b/test/stores/SetupEncryptionStore-test.ts new file mode 100644 index 0000000000..f2e27948f2 --- /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: () => Promise) => { + 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 fb2910145c..8ac6f74a38 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([]),