nikrozman/element-web
1
0
Fork 0
Code Issues Actions 3 Packages Projects Releases Wiki Activity

In force-verify mode, prevent bypassing by cancelling device verification (#29487)

Browse Source 
* In force-verify mode, prevent bypassing by cancelling device verification

* Don't show the after-login screen if we are racing with forced verification

* Unit test for not bypassing verification by cancelling device verify
This commit is contained in:
Andy Balaam
2025-03-20 15:10:08 +00:00
committed by GitHub
parent 435d0f96b8
commit 170dcd1c0e
3 changed files with 129 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
playwright/e2e/login/login-consent.spec.ts
View File

@@ -13,6 +13,7 @@ import { selectHomeserver } from "../utils";
import { type Credentials, type HomeserverInstance } from "../../plugins/homeserver";
import { consentHomeserver } from "../../plugins/homeserver/synapse/consentHomeserver.ts";
import { isDendrite } from "../../plugins/homeserver/dendrite";
import { createBot } from "../crypto/utils.ts";
// This test requires fixed credentials for the device signing keys below to work
const username = "user1234";
@@ -258,6 +259,34 @@ test.describe("Login", () => {
await expect(h1.locator(".mx_CompleteSecurity_skip")).toHaveCount(0);
});
test("Continues to show verification prompt after cancelling device verification", async ({
page,
homeserver,
credentials,
}) => {
// Create a different device which is cross-signed, meaning we need to verify this device
await createBot(page, homeserver, credentials, true);
// Wait to avoid homeserver rate limit on logins
await page.waitForTimeout(100);
// Load the page and see that we are asked to verify
await page.goto("/#/welcome");
await login(page, homeserver, credentials);
let h1 = page.getByRole("heading", { name: "Verify this device", level: 1 });
await expect(h1).toBeVisible();
// Click "Verify with another device"
await page.getByRole("button", { name: "Verify with another device" }).click();
// Cancel the new dialog
await page.getByRole("button", { name: "Close dialog" }).click();
// Check that we are still being asked to verify
h1 = page.getByRole("heading", { name: "Verify this device", level: 1 });
await expect(h1).toBeVisible();
});
});
});
});

16
src/components/structures/MatrixChat.tsx
View File

@@ -1388,7 +1388,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
// so show the homepage.
dis.dispatch<ViewHomePagePayload>({ action: Action.ViewHomePage, justRegistered: true });
}
} else {
} else if (!(await this.shouldForceVerification())) {
this.showScreenAfterLogin();
}
@@ -2003,9 +2003,17 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
};
// complete security / e2e setup has finished
private onCompleteSecurityE2eSetupFinished = (): void => {
// This is async but we making this function async to wait for it isn't useful
this.onShowPostLoginScreen().catch((e) => {
private onCompleteSecurityE2eSetupFinished = async (): Promise<void> => {
const forceVerify = await this.shouldForceVerification();
if (forceVerify) {
const isVerified = await MatrixClientPeg.safeGet().getCrypto()?.isCrossSigningReady();
if (!isVerified) {
// We must verify but we haven't yet verified - don't continue logging in
return;
}
}
await this.onShowPostLoginScreen().catch((e) => {
logger.error("Exception showing post-login screen", e);
});
};

91
test/unit-tests/components/structures/MatrixChat-test.tsx
View File

@@ -22,7 +22,12 @@ import { logger } from "matrix-js-sdk/src/logger";
import { OidcError } from "matrix-js-sdk/src/oidc/error";
import { type BearerTokenResponse } from "matrix-js-sdk/src/oidc/validate";
import { defer, type IDeferred, sleep } from "matrix-js-sdk/src/utils";
import { CryptoEvent, UserVerificationStatus } from "matrix-js-sdk/src/crypto-api";
import {
CryptoEvent,
type DeviceVerificationStatus,
UserVerificationStatus,
type CryptoApi,
} from "matrix-js-sdk/src/crypto-api";
import MatrixChat from "../../../../src/components/structures/MatrixChat";
import * as StorageAccess from "../../../../src/utils/StorageAccess";
@@ -62,6 +67,7 @@ import { UIFeature } from "../../../../src/settings/UIFeature";
import AutoDiscoveryUtils from "../../../../src/utils/AutoDiscoveryUtils";
import { type ValidatedServerConfig } from "../../../../src/utils/ValidatedServerConfig";
import Modal from "../../../../src/Modal.tsx";
import { SetupEncryptionStore } from "../../../../src/stores/SetupEncryptionStore.ts";
jest.mock("matrix-js-sdk/src/oidc/authorize", () => ({
completeAuthorizationCodeGrant: jest.fn(),
@@ -894,13 +900,92 @@ describe("<MatrixChat />", () => {
});
describe("unskippable verification", () => {
it("should show the complete security screen if unskippable verification is enabled", async () => {
beforeEach(() => {
// Force verification is turned on in settings
defaultProps.config.force_verification = true;
// And this device is being force-verified (because it logged in after
// enforcement was turned on).
localStorage.setItem("must_verify_device", "true");
// lostKeys returns false, meaning there are other devices to verify against
const realStore = SetupEncryptionStore.sharedInstance();
jest.spyOn(realStore, "lostKeys").mockReturnValue(false);
});
afterEach(() => {
jest.restoreAllMocks();
// Reset things back to how they were before we started
defaultProps.config.force_verification = false;
localStorage.removeItem("must_verify_device");
});
it("should show the complete security screen if unskippable verification is enabled", async () => {
// Given we have force verification on, and an existing logged-in session
// that is not verified (see beforeEach())
// When we render MatrixChat
getComponent();
await screen.findByRole("heading", { name: "Unable to verify this device", level: 1 });
// Then we are asked to verify our device
await screen.findByRole("heading", { name: "Verify this device", level: 1 });
// Sanity: we are not racing with another screen update, so this heading stays visible
await screen.findByRole("heading", { name: "Verify this device", level: 1 });
});
it("should not open app after cancelling device verify if unskippable verification is on", async () => {
// See https://github.com/element-hq/element-web/issues/29230
// We used to allow bypassing force verification by choosing "Verify with
// another device" and not completing the verification.
// Given we have force verification on, and an existing logged-in session
// that is not verified (see beforeEach())
// And our crypto is set up
mockClient.getCrypto.mockReturnValue(createMockCrypto());
// And MatrixChat is rendered
getComponent();
// When we click "Verify with another device"
await screen.findByRole("heading", { name: "Verify this device", level: 1 });
const verify = screen.getByRole("button", { name: "Verify with another device" });
act(() => verify.click());
// And close the device verification dialog
const closeButton = await screen.findByRole("button", { name: "Close dialog" });
act(() => closeButton.click());
// Then we are not allowed in - we are still being asked to verify
await screen.findByRole("heading", { name: "Verify this device", level: 1 });
});
function createMockCrypto(): CryptoApi {
return {
getVersion: jest.fn().mockReturnValue("Version 0"),
getVerificationRequestsToDeviceInProgress: jest.fn().mockReturnValue([]),
getUserDeviceInfo: jest.fn().mockReturnValue({
get: jest
.fn()
.mockReturnValue(
new Map([
["devid", { dehydrated: false, getIdentityKey: jest.fn().mockReturnValue("k") }],
]),
),
}),
getUserVerificationStatus: jest
.fn()
.mockResolvedValue(new UserVerificationStatus(true, true, false)),
setDeviceIsolationMode: jest.fn(),
isDehydrationSupported: jest.fn().mockReturnValue(false),
getDeviceVerificationStatus: jest
.fn()
.mockResolvedValue({ signedByOwner: true } as DeviceVerificationStatus),
isCrossSigningReady: jest.fn().mockReturnValue(false),
requestOwnUserVerification: jest.fn().mockResolvedValue({ cancel: jest.fn() }),
} as any;
}
});
});