mirror of
https://github.com/element-hq/element-web.git
synced 2025-12-05 01:10:40 +00:00
Fix bug which caused app not to load correctly when force_verification is enabled (#31265)
* MatrixChat: add a load of logging for view transitions This stuff was essentially impossible to follow and debug. I think a load of logging will help. * Add more comments on `state.view` * Add a new state between LOADING/SOFT_LOGOUT and LOGGED_IN ... so that we can transition into COMPLETE_SECURITY without going via LOGGED_IN. * Remove redundant check for `force_verification` This check was previously necessary to keep the tests working, because: * onLoggedIn would call `onShowPostLoginScreen`, * which (without the check) would call `showScreenAfterLogin` * which would queue up an action `Action.ViewHomePage` * Then we would receive an already-queued `ClientStarted` action, which would transition us (correctly) to the `COMPLETE_SECURITY` view * Then we would receive the `ViewHomePage` action, taking us back to `LOGGED_IN`. I don't think the check was necessary in practice, because in practice there would be enough delay between the OnLoggedIn and ClientStarted actions that the race didn't happen. The *problem* with the check was that it meant that, whenever `force_verification` was enabled, we would get stuck in the LOADING state. The check is now unnecessary, because `onLoggedIn` no longer calls `onShowPostLoginScreen`. * `onShowPostLoginScreen` need no longer be `async` * Regression test for https://github.com/element-hq/element-web/issues/31203
This commit is contained in:
Richard van der Hoff
committed by
GitHub
GitHub
parent
57fd3c46bb
commit
04cf53e7aa
30
playwright/e2e/login/login.spec.ts
Normal file
30
playwright/e2e/login/login.spec.ts
Normal file
@@ -0,0 +1,30 @@
|
||||
/*
|
||||
Copyright 2025 Element Creations Ltd.
|
||||
|
||||
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
|
||||
Please see LICENSE files in the repository root for full details.
|
||||
*/
|
||||
|
||||
import { expect, test } from "../../element-web-test";
|
||||
import { logIntoElement } from "../crypto/utils.ts";
|
||||
|
||||
test.describe(`With force_verification: true`, () => {
|
||||
test.use({
|
||||
config: {
|
||||
force_verification: true,
|
||||
},
|
||||
});
|
||||
|
||||
test("Can reload after login", async ({ page, credentials }) => {
|
||||
// The page should reload fine when going to the base client URL
|
||||
// Regression test for https://github.com/element-hq/element-web/issues/31203
|
||||
await logIntoElement(page, credentials);
|
||||
|
||||
// We should auto-upload the E2EE keys, and show a welcome page
|
||||
await expect(page.getByRole("heading", { name: `Welcome ${credentials.displayName}` })).toBeVisible();
|
||||
|
||||
await page.goto("/");
|
||||
|
||||
await expect(page.getByRole("heading", { name: `Welcome ${credentials.displayName}` })).toBeVisible();
|
||||
});
|
||||
});
|
||||
@@ -27,6 +27,7 @@ const notLoggedInMap: Record<Exclude<Views, Views.LOGGED_IN>, ScreenName> = {
|
||||
[Views.FORGOT_PASSWORD]: "ForgotPassword",
|
||||
[Views.COMPLETE_SECURITY]: "CompleteSecurity",
|
||||
[Views.E2E_SETUP]: "E2ESetup",
|
||||
[Views.PENDING_CLIENT_START]: "Loading",
|
||||
[Views.SOFT_LOGOUT]: "SoftLogout",
|
||||
[Views.LOCK_STOLEN]: "SessionLockStolen",
|
||||
};
|
||||
|
||||
108
src/Views.ts
108
src/Views.ts
@@ -20,52 +20,62 @@ Please see LICENSE files in the repository root for full details.
|
||||
* │ LOADING │─────────────────────────────►│ CONFIRM_LOCK_ │
|
||||
* │ │◄─────────────────────────────│ THEFT │
|
||||
* └─────────────────┘ Lock theft confirmed └─────────────────┘
|
||||
* Session recovered │ │ │
|
||||
* ┌──────────────┘ │ └────────────────┐
|
||||
* │ ┌─────────────┘ │ No previous session
|
||||
* │ │ Token/OIDC login succeeded │
|
||||
* │ │ ▼
|
||||
* │ │ ┌─────────────────┐
|
||||
* │ │ │ WELCOME │ (from all other states
|
||||
* │ │ │ │ except LOCK_STOLEN)
|
||||
* │ │ └─────────────────┘ │
|
||||
* │ │ "Create Account" │ │ "Sign in" │ Client logged out
|
||||
* │ │ ┌────────────────────────┘ │ │
|
||||
* │ │ │ │ ┌────────────────────┘
|
||||
* │ │ │ │ │
|
||||
* │ │ ▼ "Create an ▼ ▼ "Forgot
|
||||
* │ │ ┌─────────────────┐ account" ┌─────────────────┐ password" ┌─────────────────┐
|
||||
* │ │ │ REGISTER │◄───────────────│ LOGIN │───────────────►│ FORGOT_PASSWORD │
|
||||
* │ │ │ │───────────────►│ │◄───────────────│ │
|
||||
* │ │ └─────────────────┘ "Sign in here" └─────────────────┘ Complete / └─────────────────┘
|
||||
* │ │ │ │ "Sign in instead" ▲
|
||||
* │ │ └────────────────────────────────┐ │ │
|
||||
* │ └────────────────────────────────────────┐ │ │ │
|
||||
* │ ▼ ▼ ▼ │
|
||||
* │ ┌──────────────────┐ │
|
||||
* │ │ (postLoginSetup) │ │
|
||||
* │ └──────────────────┘ │
|
||||
* │ ┌────────────────────────────────────┘ │ │ │
|
||||
* │ │ E2EE not enabled ┌─────────────┘ └──────┐ │
|
||||
* │ │ │ Account has │ Account lacks │
|
||||
* │ │ │ cross-signing │ cross-signing │
|
||||
* │ │ │ keys │ keys │
|
||||
* │ │ Client started and ▼ ▼ │
|
||||
* │ │ force_verification ┌─────────────────┐ ┌─────────────────┐ │
|
||||
* │ │ pending │ COMPLETE_ │ │ E2E_SETUP │ │
|
||||
* │ │ ┌─────────────────►│ SECURITY │ │ │ │
|
||||
* │ │ │ └─────────────────┘ └─────────────────┘ │ "Forgotten
|
||||
* │ │ │ ┌───────────────────────┘ │ │ your
|
||||
* │ │ │ │ ┌───────────────────────────────────────────────┘ │ password?"
|
||||
* │ │ │ │ │ │
|
||||
* │ │ │ │ │ (from all other states │
|
||||
* │ │ │ │ │ except LOCK_STOLEN) │
|
||||
* │ │ │ │ │ └──────────────┐ │
|
||||
* ▼ ▼ │ ▼ ▼ Soft logout error ▼ │
|
||||
* ┌─────────────────┐ ┌─────────────────┐
|
||||
* │ LOGGED_IN │ Re-authentication succeeded │ SOFT_LOGOUT │
|
||||
* │ │◄────────────────────────────────────────────────────────│ │
|
||||
* └─────────────────┘ └─────────────────┘
|
||||
* Session recovered │ │ │ Token/OIDC login succeeded
|
||||
* ┌──────────────┘ │ └──────────────────────────────────────────────────────────────────┐
|
||||
* │ └───────────────────────────────────────────┐ │
|
||||
* │ │ No previous session │
|
||||
* │ ▼ │
|
||||
* │ (from all other states ┌─────────────────┐ │
|
||||
* │ except LOCK_STOLEN) │ WELCOME │ │
|
||||
* │ │ │ │ │
|
||||
* │ │ Client logged out └─────────────────┘ │
|
||||
* │ │ │ │ │
|
||||
* │ └──────────────────────────┐ "Sign in" │ │ "Create account" │
|
||||
* │ │ ┌────────────┘ └──────────────┐ │
|
||||
* │ │ │ │ │
|
||||
* │ "Forgot ▼ ▼ "Create an ▼ │
|
||||
* │ ┌─────────────────┐ password" ┌─────────────────┐ account" ┌─────────────────┐ │
|
||||
* │ │ FORGOT_PASSWORD │◄───────────────│ LOGIN │───────────────►│ REGISTER │ │
|
||||
* │ │ │───────────────►│ │◄───────────────│ │ │
|
||||
* │ └─────────────────┘ Complete / └─────────────────┘ "Sign in here" └─────────────────┘ │
|
||||
* │ ▲ "Sign in instead" │ │ │
|
||||
* │ │ └──────────────────────┐ ┌─────────┘ │
|
||||
* │ │"Forgotten your │ │ ┌──────────────────┘
|
||||
* │ │ password?" │ │ │
|
||||
* │ │ │ │ │
|
||||
* │ ┌─────────────────┐ Soft-logout error │ │ │
|
||||
* │ │ SOFT_LOGOUT │◄───────────── (from all other states │ │ │
|
||||
* │ │ │ except LOCK_STOLEN) │ │ │
|
||||
* │ └─────────────────┘ │ │ │
|
||||
* │ │ Re-authentication succeeded ▼ ▼ ▼
|
||||
* │ │ ┌──────────────────┐
|
||||
* ▼ ▼ │ (postLoginSetup) │
|
||||
* ┌─────────────────┐ └──────────────────┘
|
||||
* │ PENDING_CLIENT_ │ Account has │ │ │ Account lacks
|
||||
* │ START │ cross-signing │ │ │ cross-signing
|
||||
* └─────────────────┘ keys │ │ │ keys
|
||||
* │ │ │ │ │
|
||||
* │ └───────────────────────────────┐ │ │ │
|
||||
* │ Client started, │ │ │ └──────┐
|
||||
* │ force_verification pending │ │ │ │
|
||||
* │ ▼ │ │ │
|
||||
* │ Client started, ┌─────────────────┐ │ │ │
|
||||
* │ force_verification │ COMPLETE_ │◄────────────┘ │ ▼
|
||||
* │ not needed │ SECURITY │ │ ┌─────────────────┐
|
||||
* │ └─────────────────┘ │ │ E2E_SETUP │
|
||||
* │ │ │ │ │
|
||||
* │ ┌─────────────────────────────────────┘ E2EE not enabled │ └─────────────────┘
|
||||
* │ │ ┌─────────────────────────────────────────────────────────────┘ │
|
||||
* │ │ │ ┌──────────────────────────────────────────────────────────────────────┘
|
||||
* │ │ │ │
|
||||
* │ │ │ │
|
||||
* │ │ │ │
|
||||
* │ │ │ │
|
||||
* ▼ ▼ ▼ ▼
|
||||
* ┌─────────────────┐
|
||||
* │ LOGGED_IN │
|
||||
* │ │
|
||||
* └─────────────────┘
|
||||
*
|
||||
* (from all other states)
|
||||
* │
|
||||
@@ -102,6 +112,12 @@ enum Views {
|
||||
// flow to setup SSSS / cross-signing on this account
|
||||
E2E_SETUP,
|
||||
|
||||
/**
|
||||
* We have successfully recovered a session from localstorage, but the client
|
||||
* has not yet been started.
|
||||
*/
|
||||
PENDING_CLIENT_START,
|
||||
|
||||
// we are logged in with an active matrix client. The logged_in state also
|
||||
// includes guests users as they too are logged in at the client level.
|
||||
LOGGED_IN,
|
||||
|
||||
@@ -173,8 +173,18 @@ interface IProps {
|
||||
}
|
||||
|
||||
interface IState {
|
||||
// the master view we are showing.
|
||||
/**
|
||||
* The master view we are showing.
|
||||
*
|
||||
* This represents the state of a state machine: see the documentation on {@link Views} for a transition diagram.
|
||||
*
|
||||
* TODO: this doesn't work well, because updates to React state are not instantaneous, meaning that if several
|
||||
* events or {@link Action}s happen in quick succession, we may end up following the wrong transition.
|
||||
* We should probably move the view into a separate object (like a ViewModel) and have the React state subscribe
|
||||
* to updates.
|
||||
*/
|
||||
view: Views;
|
||||
|
||||
// What the LoggedInView would be showing if visible.
|
||||
// A member of the enum for standard pages or a string for those provided by
|
||||
// a module.
|
||||
@@ -473,6 +483,9 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
|
||||
| (Pick<IState, K> | IState | null),
|
||||
callback?: () => void,
|
||||
): void {
|
||||
if (state && "view" in state) {
|
||||
logger.debug(`MatrixChat: Queuing change of view from ${Views[this.state.view]} to ${Views[state.view]}`);
|
||||
}
|
||||
if (this.shouldTrackPageChange(this.state, { ...this.state, ...state })) {
|
||||
this.startPageChangeTimer();
|
||||
}
|
||||
@@ -648,9 +661,15 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
|
||||
private onAction = (payload: ActionPayload): void => {
|
||||
// once the session lock has been stolen, don't try to do anything.
|
||||
if (this.state.view === Views.LOCK_STOLEN) {
|
||||
logger.warn(`MatrixChat: ignoring action ${payload.action} as session lock has been stolen`);
|
||||
return;
|
||||
}
|
||||
|
||||
// Exclude some rather spammy actions from being logged.
|
||||
if (payload.action != Action.UserActivity) {
|
||||
logger.debug(`MatrixChat: handling action ${payload.action}`);
|
||||
}
|
||||
|
||||
// Start the onboarding process for certain actions
|
||||
if (
|
||||
MatrixClientPeg.get()?.isGuest() &&
|
||||
@@ -1390,11 +1409,14 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
|
||||
*
|
||||
* In other words, whenever we think we have completed the login and E2E setup tasks.
|
||||
*/
|
||||
private async onShowPostLoginScreen(): Promise<void> {
|
||||
private onShowPostLoginScreen(): void {
|
||||
logger.debug("onShowPostLoginScreen: Transitioning to logged in view.");
|
||||
|
||||
this.setStateForNewView({ view: Views.LOGGED_IN });
|
||||
// If a specific screen is set to be shown after login, show that above
|
||||
// all else, as it probably means the user clicked on something already.
|
||||
if (this.screenAfterLogin?.screen) {
|
||||
logger.debug(`onShowPostLoginScreen: showing screen ${this.screenAfterLogin.screen}`);
|
||||
this.showScreen(this.screenAfterLogin.screen, this.screenAfterLogin.params);
|
||||
this.screenAfterLogin = undefined;
|
||||
} else if (MatrixClientPeg.currentUserIsJustRegistered()) {
|
||||
@@ -1403,6 +1425,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
|
||||
if (ThreepidInviteStore.instance.pickBestInvite()) {
|
||||
// The user has a 3pid invite pending - show them that
|
||||
const threepidInvite = ThreepidInviteStore.instance.pickBestInvite();
|
||||
logger.debug(`onShowPostLoginScreen: showing room ${threepidInvite.roomId} after registration`);
|
||||
|
||||
// HACK: This is a pretty brutal way of threading the invite back through
|
||||
// our systems, but it's the safest we have for now.
|
||||
@@ -1411,9 +1434,11 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
|
||||
} else {
|
||||
// The user has just logged in after registering,
|
||||
// so show the homepage.
|
||||
logger.debug("onShowPostLoginScreen: Showing home page after registration");
|
||||
dis.dispatch<ViewHomePagePayload>({ action: Action.ViewHomePage, justRegistered: true });
|
||||
}
|
||||
} else if (!(await this.shouldForceVerification())) {
|
||||
} else {
|
||||
logger.debug("onShowPostLoginScreen: showScreenAfterLogin");
|
||||
this.showScreenAfterLogin();
|
||||
}
|
||||
|
||||
@@ -1477,15 +1502,19 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
|
||||
// If screenAfterLogin is set, use that, then null it so that a second login will
|
||||
// result in view_home_page, _user_settings or _room_directory
|
||||
if (this.screenAfterLogin && this.screenAfterLogin.screen) {
|
||||
logger.debug(`showScreenAfterLogin: showing screen ${this.screenAfterLogin.screen}`);
|
||||
this.showScreen(this.screenAfterLogin.screen, this.screenAfterLogin.params);
|
||||
this.screenAfterLogin = undefined;
|
||||
} else if (localStorage && localStorage.getItem("mx_last_room_id")) {
|
||||
// Before defaulting to directory, show the last viewed room
|
||||
logger.debug("showScreenAfterLogin: showing last room");
|
||||
this.viewLastRoom();
|
||||
} else {
|
||||
if (MatrixClientPeg.safeGet().isGuest()) {
|
||||
logger.debug("showScreenAfterLogin: showing guest welcome page");
|
||||
dis.dispatch({ action: "view_welcome_page" });
|
||||
} else {
|
||||
logger.debug("showScreenAfterLogin: showing home page");
|
||||
dis.dispatch({ action: Action.ViewHomePage });
|
||||
}
|
||||
}
|
||||
@@ -1506,18 +1535,27 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
|
||||
this.stores.client = MatrixClientPeg.safeGet();
|
||||
StorageManager.tryPersistStorage();
|
||||
|
||||
// If we're in the middle of a login/registration, we wait for it to complete before transitioning to the logged
|
||||
// in view the login flow will call `postLoginSetup` when it's done, which will arrange for `onShowPostLoginScreen`
|
||||
// to be called.
|
||||
// If we're loading the app for the first time, we can now transition to a splash screen while we wait for the
|
||||
// client to start. The exceptions are:
|
||||
//
|
||||
// - If there is a token login in flight: in that case we wait for the login to complete (which hits
|
||||
// `postLoginSetup`).
|
||||
//
|
||||
// - Lifecycle emits an `Action.OnLoggedIn` event during startup even if the localstorage flag indicating a
|
||||
// previous soft logout is set. In that situation we actually want to wait for the `Action.ClientNotViable`
|
||||
// event, which will transition us into Views.SOFT_LOGOUT. We therefore have to check for !isSoftLogout().
|
||||
// There will be a subsequent `Action.OnLoggedIn` event once the reauthentication completes.
|
||||
//
|
||||
// XXX: fix this properly by having Lifecycle not emit OnLoggedIn when it knows it is about to emit a
|
||||
// ClientNotViable.
|
||||
//
|
||||
// If we're already in the SOFT_LOGOUT view, that means that reauthentication has succeeded, and we can
|
||||
// transition to the splash screen.
|
||||
if (
|
||||
!this.tokenLogin &&
|
||||
!Lifecycle.isSoftLogout() &&
|
||||
this.state.view !== Views.LOGIN &&
|
||||
this.state.view !== Views.REGISTER &&
|
||||
this.state.view !== Views.COMPLETE_SECURITY &&
|
||||
this.state.view !== Views.E2E_SETUP
|
||||
(this.state.view === Views.LOADING && !Lifecycle.isSoftLogout() && !this.tokenLogin) ||
|
||||
this.state.view === Views.SOFT_LOGOUT
|
||||
) {
|
||||
this.onShowPostLoginScreen();
|
||||
this.setStateForNewView({ view: Views.PENDING_CLIENT_START });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1750,15 +1788,6 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
|
||||
const cli = MatrixClientPeg.safeGet();
|
||||
|
||||
const shouldForceVerification = await this.shouldForceVerification();
|
||||
// XXX: Don't replace the screen if it's already one of these: postLoginSetup
|
||||
// changes to these screens in certain circumstances so we shouldn't clobber it.
|
||||
// We should probably have one place where we decide what the next screen is after
|
||||
// login.
|
||||
if (![Views.COMPLETE_SECURITY, Views.E2E_SETUP].includes(this.state.view)) {
|
||||
if (shouldForceVerification) {
|
||||
this.setStateForNewView({ view: Views.COMPLETE_SECURITY });
|
||||
}
|
||||
}
|
||||
|
||||
const crypto = cli.getCrypto();
|
||||
if (crypto) {
|
||||
@@ -1775,13 +1804,31 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
|
||||
this.setState({
|
||||
ready: true,
|
||||
});
|
||||
|
||||
// If the view is PENDING_CLIENT_START, that means we recovered the session from localstorage, or from
|
||||
// soft-logout: we can now transition to the logged-in view.
|
||||
//
|
||||
// If the view is something else, that probably means it's a login or registration view; we handle that in
|
||||
// `postLoginSetup`.
|
||||
if (this.state.view === Views.PENDING_CLIENT_START) {
|
||||
if (shouldForceVerification) {
|
||||
this.setStateForNewView({ view: Views.COMPLETE_SECURITY });
|
||||
} else {
|
||||
this.onShowPostLoginScreen();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public showScreen(screen: string, params?: { [key: string]: any }): void {
|
||||
logger.debug(`showScreen ${screen}`);
|
||||
|
||||
const cli = MatrixClientPeg.get();
|
||||
const isLoggedOutOrGuest = !cli || cli.isGuest();
|
||||
if (!isLoggedOutOrGuest && AUTH_SCREENS.includes(screen)) {
|
||||
// user is logged in and landing on an auth page which will uproot their session, redirect them home instead
|
||||
logger.info(
|
||||
`showScreen: suppressing change to AuthScreen ${screen} for logged-in user, and going to home screen instead`,
|
||||
);
|
||||
dis.dispatch({ action: Action.ViewHomePage });
|
||||
return;
|
||||
}
|
||||
@@ -2087,9 +2134,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
|
||||
}
|
||||
}
|
||||
|
||||
await this.onShowPostLoginScreen().catch((e) => {
|
||||
logger.error("Exception showing post-login screen", e);
|
||||
});
|
||||
this.onShowPostLoginScreen();
|
||||
};
|
||||
|
||||
private getFragmentAfterLogin(): string {
|
||||
@@ -2128,6 +2173,15 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
|
||||
view = <CompleteSecurity onFinished={this.onCompleteSecurityE2eSetupFinished} />;
|
||||
} else if (this.state.view === Views.E2E_SETUP) {
|
||||
view = <E2eSetup onCancelled={this.onCompleteSecurityE2eSetupFinished} />;
|
||||
} else if (this.state.view === Views.PENDING_CLIENT_START) {
|
||||
// we think we are logged in, but are still waiting for the /sync to complete
|
||||
view = (
|
||||
<LoginSplashView
|
||||
matrixClient={MatrixClientPeg.safeGet()}
|
||||
onLogoutClick={this.onLogoutClick}
|
||||
syncError={this.state.syncError}
|
||||
/>
|
||||
);
|
||||
} else if (this.state.view === Views.LOGGED_IN) {
|
||||
// `ready` and `view==LOGGED_IN` may be set before `page_type` (because the
|
||||
// latter is set via the dispatcher). If we don't yet have a `page_type`,
|
||||
|
||||
Reference in New Issue
Block a user