From 655bca63e64e003b794721712ae768eee84381c5 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 26 May 2022 11:12:49 +0100 Subject: [PATCH] Move Enterprise Erin tests from Puppeteer to Cypress (#8569) * Move Enterprise Erin tests from Puppeteer to Cypress * delint * types * Fix double space * Better handle logout in Lifecycle * Fix test by awaiting the network request * Improve some logout handlings * Try try try again * Delint * Fix tests * Delint --- cypress/integration/2-login/login.spec.ts | 41 +++++++++++++++ .../integration/3-user-menu/user-menu.spec.ts | 2 +- cypress/support/app.ts | 43 ++++++++++++++++ cypress/support/index.ts | 1 + src/DeviceListener.ts | 21 +++++--- src/Lifecycle.ts | 48 +++++++++--------- src/SecurityManager.ts | 4 +- src/stores/SetupEncryptionStore.ts | 6 +-- test/end-to-end-tests/src/scenario.ts | 9 +--- .../src/scenarios/sso-customisations.ts | 50 ------------------- test/end-to-end-tests/src/usecases/logout.ts | 43 ---------------- test/end-to-end-tests/src/util.ts | 13 +---- 12 files changed, 131 insertions(+), 150 deletions(-) create mode 100644 cypress/support/app.ts delete mode 100644 test/end-to-end-tests/src/scenarios/sso-customisations.ts delete mode 100644 test/end-to-end-tests/src/usecases/logout.ts diff --git a/cypress/integration/2-login/login.spec.ts b/cypress/integration/2-login/login.spec.ts index 521eb66f25..85d2866e49 100644 --- a/cypress/integration/2-login/login.spec.ts +++ b/cypress/integration/2-login/login.spec.ts @@ -59,4 +59,45 @@ describe("Login", () => { cy.stopMeasuring("from-submit-to-home"); }); }); + + describe("logout", () => { + beforeEach(() => { + cy.initTestUser(synapse, "Erin"); + }); + + it("should go to login page on logout", () => { + cy.get('[aria-label="User menu"]').click(); + + // give a change for the outstanding requests queue to settle before logging out + cy.wait(500); + + cy.get(".mx_UserMenu_contextMenu").within(() => { + cy.get(".mx_UserMenu_iconSignOut").click(); + }); + + cy.url().should("contain", "/#/login"); + }); + + it("should respect logout_redirect_url", () => { + cy.tweakConfig({ + // We redirect to decoder-ring because it's a predictable page that isn't Element itself. + // We could use example.org, matrix.org, or something else, however this puts dependency of external + // infrastructure on our tests. In the same vein, we don't really want to figure out how to ship a + // `test-landing.html` page when running with an uncontrolled Element (via `yarn start`). + // Using the decoder-ring is just as fine, and we can search for strategic names. + logout_redirect_url: "/decoder-ring/", + }); + + cy.get('[aria-label="User menu"]').click(); + + // give a change for the outstanding requests queue to settle before logging out + cy.wait(500); + + cy.get(".mx_UserMenu_contextMenu").within(() => { + cy.get(".mx_UserMenu_iconSignOut").click(); + }); + + cy.url().should("contains", "decoder-ring"); + }); + }); }); diff --git a/cypress/integration/3-user-menu/user-menu.spec.ts b/cypress/integration/3-user-menu/user-menu.spec.ts index 671fd4eacf..40b4a0cd74 100644 --- a/cypress/integration/3-user-menu/user-menu.spec.ts +++ b/cypress/integration/3-user-menu/user-menu.spec.ts @@ -39,7 +39,7 @@ describe("User Menu", () => { it("should contain our name & userId", () => { cy.get('[aria-label="User menu"]').click(); - cy.get(".mx_ContextualMenu").within(() => { + cy.get(".mx_UserMenu_contextMenu").within(() => { cy.get(".mx_UserMenu_contextMenu_displayName").should("contain", "Jeff"); cy.get(".mx_UserMenu_contextMenu_userId").should("contain", user.userId); }); diff --git a/cypress/support/app.ts b/cypress/support/app.ts new file mode 100644 index 0000000000..21321e2b56 --- /dev/null +++ b/cypress/support/app.ts @@ -0,0 +1,43 @@ +/* +Copyright 2022 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 "./client"; // XXX: without an (any) import here, types break down +import Chainable = Cypress.Chainable; +import AUTWindow = Cypress.AUTWindow; + +declare global { + // eslint-disable-next-line @typescript-eslint/no-namespace + namespace Cypress { + interface Chainable { + /** + * Applies tweaks to the config read from config.json + */ + tweakConfig(tweaks: Record): Chainable; + } + } +} + +Cypress.Commands.add("tweakConfig", (tweaks: Record): Chainable => { + return cy.window().then(win => { + // note: we can't *set* the object because the window version is effectively a pointer. + for (const [k, v] of Object.entries(tweaks)) { + // @ts-ignore - for some reason it's not picking up on global.d.ts types. + win.mxReactSdkConfig[k] = v; + } + }); +}); diff --git a/cypress/support/index.ts b/cypress/support/index.ts index 3f40ca198c..06d6efc252 100644 --- a/cypress/support/index.ts +++ b/cypress/support/index.ts @@ -27,3 +27,4 @@ import "./settings"; import "./bot"; import "./clipboard"; import "./util"; +import "./app"; diff --git a/src/DeviceListener.ts b/src/DeviceListener.ts index c9a6d0386b..cf9af5befc 100644 --- a/src/DeviceListener.ts +++ b/src/DeviceListener.ts @@ -18,6 +18,7 @@ import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { logger } from "matrix-js-sdk/src/logger"; import { CryptoEvent } from "matrix-js-sdk/src/crypto"; import { ClientEvent, EventType, RoomStateEvent } from "matrix-js-sdk/src/matrix"; +import { SyncState } from "matrix-js-sdk/src/sync"; import { MatrixClientPeg } from './MatrixClientPeg'; import dis from "./dispatcher/dispatcher"; @@ -58,13 +59,15 @@ export default class DeviceListener { private ourDeviceIdsAtStart: Set = null; // The set of device IDs we're currently displaying toasts for private displayingToastsForDeviceIds = new Set(); + private running = false; - static sharedInstance() { + public static sharedInstance() { if (!window.mxDeviceListener) window.mxDeviceListener = new DeviceListener(); return window.mxDeviceListener; } - start() { + public start() { + this.running = true; MatrixClientPeg.get().on(CryptoEvent.WillUpdateDevices, this.onWillUpdateDevices); MatrixClientPeg.get().on(CryptoEvent.DevicesUpdated, this.onDevicesUpdated); MatrixClientPeg.get().on(CryptoEvent.DeviceVerificationChanged, this.onDeviceVerificationChanged); @@ -77,7 +80,8 @@ export default class DeviceListener { this.recheck(); } - stop() { + public stop() { + this.running = false; if (MatrixClientPeg.get()) { MatrixClientPeg.get().removeListener(CryptoEvent.WillUpdateDevices, this.onWillUpdateDevices); MatrixClientPeg.get().removeListener(CryptoEvent.DevicesUpdated, this.onDevicesUpdated); @@ -109,7 +113,7 @@ export default class DeviceListener { * * @param {String[]} deviceIds List of device IDs to dismiss notifications for */ - async dismissUnverifiedSessions(deviceIds: Iterable) { + public async dismissUnverifiedSessions(deviceIds: Iterable) { logger.log("Dismissing unverified sessions: " + Array.from(deviceIds).join(',')); for (const d of deviceIds) { this.dismissed.add(d); @@ -118,7 +122,7 @@ export default class DeviceListener { this.recheck(); } - dismissEncryptionSetup() { + public dismissEncryptionSetup() { this.dismissedThisDeviceToast = true; this.recheck(); } @@ -179,8 +183,10 @@ export default class DeviceListener { } }; - private onSync = (state, prevState) => { - if (state === 'PREPARED' && prevState === null) this.recheck(); + private onSync = (state: SyncState, prevState?: SyncState) => { + if (state === 'PREPARED' && prevState === null) { + this.recheck(); + } }; private onRoomStateEvents = (ev: MatrixEvent) => { @@ -217,6 +223,7 @@ export default class DeviceListener { } private async recheck() { + if (!this.running) return; // we have been stopped const cli = MatrixClientPeg.get(); if (!(await cli.doesServerSupportUnstableFeature("org.matrix.e2e_cross_signing"))) return; diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 2932779d93..e663a41046 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -168,7 +168,7 @@ export async function loadSession(opts: ILoadSessionOpts = {}): Promise * Gets the user ID of the persisted session, if one exists. This does not validate * that the user's credentials still work, just that they exist and that a user ID * is associated with them. The session is not loaded. - * @returns {[String, bool]} The persisted session's owner and whether the stored + * @returns {[string, boolean]} The persisted session's owner and whether the stored * session is for a guest user, if an owner exists. If there is no stored session, * return [null, null]. */ @@ -494,7 +494,7 @@ async function handleLoadSessionFailure(e: Error): Promise { * Also stops the old MatrixClient and clears old credentials/etc out of * storage before starting the new client. * - * @param {MatrixClientCreds} credentials The credentials to use + * @param {IMatrixClientCreds} credentials The credentials to use * * @returns {Promise} promise which resolves to the new MatrixClient once it has been started */ @@ -525,7 +525,7 @@ export async function setLoggedIn(credentials: IMatrixClientCreds): Promise onLoggedOut()); return; } @@ -739,19 +739,17 @@ export function logout(): void { _isLoggingOut = true; const client = MatrixClientPeg.get(); PlatformPeg.get().destroyPickleKey(client.getUserId(), client.getDeviceId()); - client.logout().then(onLoggedOut, - (err) => { - // Just throwing an error here is going to be very unhelpful - // if you're trying to log out because your server's down and - // you want to log into a different server, so just forget the - // access token. It's annoying that this will leave the access - // token still valid, but we should fix this by having access - // tokens expire (and if you really think you've been compromised, - // change your password). - logger.log("Failed to call logout API: token will not be invalidated"); - onLoggedOut(); - }, - ); + client.logout(undefined, true).then(onLoggedOut, (err) => { + // Just throwing an error here is going to be very unhelpful + // if you're trying to log out because your server's down and + // you want to log into a different server, so just forget the + // access token. It's annoying that this will leave the access + // token still valid, but we should fix this by having access + // tokens expire (and if you really think you've been compromised, + // change your password). + logger.warn("Failed to call logout API: token will not be invalidated", err); + onLoggedOut(); + }); } export function softLogout(): void { @@ -856,9 +854,8 @@ async function startMatrixClient(startSyncing = true): Promise { * storage. Used after a session has been logged out. */ export async function onLoggedOut(): Promise { - _isLoggingOut = false; // Ensure that we dispatch a view change **before** stopping the client, - // so that React components unmount first. This avoids React soft crashes + // that React components unmount first. This avoids React soft crashes // that can occur when components try to use a null client. dis.fire(Action.OnLoggedOut, true); stopMatrixClient(); @@ -869,8 +866,13 @@ export async function onLoggedOut(): Promise { // customisations got the memo. if (SdkConfig.get().logout_redirect_url) { logger.log("Redirecting to external provider to finish logout"); - window.location.href = SdkConfig.get().logout_redirect_url; + // XXX: Defer this so that it doesn't race with MatrixChat unmounting the world by going to /#/login + setTimeout(() => { + window.location.href = SdkConfig.get().logout_redirect_url; + }, 100); } + // Do this last to prevent racing `stopMatrixClient` and `on_logged_out` with MatrixChat handling Session.logged_out + _isLoggingOut = false; } /** @@ -908,9 +910,7 @@ async function clearStorage(opts?: { deleteEverything?: boolean }): Promise { - let key; + let key: Uint8Array; const { finished } = Modal.createTrackedDialog('Restore Backup', '', RestoreKeyBackupDialog, { showSummary: false, keyCallback: k => key = k, diff --git a/src/stores/SetupEncryptionStore.ts b/src/stores/SetupEncryptionStore.ts index b5df05b2ff..38ba0bd9b0 100644 --- a/src/stores/SetupEncryptionStore.ts +++ b/src/stores/SetupEncryptionStore.ts @@ -89,9 +89,7 @@ export class SetupEncryptionStore extends EventEmitter { return; } this.started = false; - if (this.verificationRequest) { - this.verificationRequest.off(VerificationRequestEvent.Change, this.onVerificationRequestChange); - } + this.verificationRequest?.off(VerificationRequestEvent.Change, this.onVerificationRequestChange); if (MatrixClientPeg.get()) { MatrixClientPeg.get().removeListener(CryptoEvent.VerificationRequest, this.onVerificationRequest); MatrixClientPeg.get().removeListener(CryptoEvent.UserTrustStatusChanged, this.onUserTrustStatusChanged); @@ -99,6 +97,7 @@ export class SetupEncryptionStore extends EventEmitter { } public async fetchKeyInfo(): Promise { + if (!this.started) return; // bail if we were stopped const cli = MatrixClientPeg.get(); const keys = await cli.isSecretStored('m.cross_signing.master'); if (keys === null || Object.keys(keys).length === 0) { @@ -270,6 +269,7 @@ export class SetupEncryptionStore extends EventEmitter { } private async setActiveVerificationRequest(request: VerificationRequest): Promise { + if (!this.started) return; // bail if we were stopped if (request.otherUserId !== MatrixClientPeg.get().getUserId()) return; if (this.verificationRequest) { diff --git a/test/end-to-end-tests/src/scenario.ts b/test/end-to-end-tests/src/scenario.ts index b0ead39167..42e47afac1 100644 --- a/test/end-to-end-tests/src/scenario.ts +++ b/test/end-to-end-tests/src/scenario.ts @@ -27,13 +27,12 @@ import { RestMultiSession } from "./rest/multi"; import { RestSession } from "./rest/session"; import { stickerScenarios } from './scenarios/sticker'; import { userViewScenarios } from "./scenarios/user-view"; -import { ssoCustomisationScenarios } from "./scenarios/sso-customisations"; import { updateScenarios } from "./scenarios/update"; export async function scenario(createSession: (s: string) => Promise, restCreator: RestSessionCreator): Promise { let firstUser = true; - async function createUser(username) { + async function createUser(username: string) { const session = await createSession(username); if (firstUser) { // only show browser version for first browser opened @@ -65,12 +64,6 @@ export async function scenario(createSession: (s: string) => Promise { - console.log(" injecting logout customisations for SSO scenarios:"); - - await session.delay(1000); // wait for dialogs to close - await applyConfigChange(session, { - // we redirect to config.json because it's a predictable page that isn't Element - // itself. We could use example.org, matrix.org, or something else, however this - // puts dependency of external infrastructure on our tests. In the same vein, we - // don't really want to figure out how to ship a `test-landing.html` page when - // running with an uncontrolled Element (via `./run.sh --app-url http://localhost:8080`). - // Using the config.json is just as fine, and we can search for strategic names. - 'logout_redirect_url': '/config.json', - }); - - await logoutCanCauseRedirect(session); -} - -async function logoutCanCauseRedirect(session: ElementSession): Promise { - await logout(session, false); // we'll check the login page ourselves, so don't assert - - session.log.step("waits for redirect to config.json (as external page)"); - const foundLoginUrl = await session.poll(async () => { - const url = session.page.url(); - return url === session.url('/config.json'); - }); - assert(foundLoginUrl); - session.log.done(); -} diff --git a/test/end-to-end-tests/src/usecases/logout.ts b/test/end-to-end-tests/src/usecases/logout.ts deleted file mode 100644 index b422ff3d74..0000000000 --- a/test/end-to-end-tests/src/usecases/logout.ts +++ /dev/null @@ -1,43 +0,0 @@ -/* -Copyright 2022 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 { strict as assert } from 'assert'; - -import { ElementSession } from "../session"; - -export async function logout(session: ElementSession, assertLoginPage = true): Promise { - session.log.startGroup("logs out"); - - session.log.step("navigates to user menu"); - const userButton = await session.query('.mx_UserMenu > div.mx_AccessibleButton'); - await userButton.click(); - session.log.done(); - - session.log.step("clicks the 'Sign Out' button"); - const signOutButton = await session.query('.mx_UserMenu_contextMenu .mx_UserMenu_iconSignOut'); - await signOutButton.click(); - session.log.done(); - - if (assertLoginPage) { - const foundLoginUrl = await session.poll(async () => { - const url = session.page.url(); - return url === session.url('/#/login'); - }); - assert(foundLoginUrl); - } - - session.log.endGroup(); -} diff --git a/test/end-to-end-tests/src/util.ts b/test/end-to-end-tests/src/util.ts index f45b23afcb..298e708007 100644 --- a/test/end-to-end-tests/src/util.ts +++ b/test/end-to-end-tests/src/util.ts @@ -28,7 +28,7 @@ export const range = function(start: number, amount: number, step = 1): Array { +export const delay = function(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); }; @@ -44,17 +44,6 @@ export const measureStop = function(session: ElementSession, name: string): Prom }, name); }; -// TODO: Proper types on `config` - for some reason won't accept an import of ConfigOptions. -export async function applyConfigChange(session: ElementSession, config: any): Promise { - await session.page.evaluate((_config) => { - // note: we can't *set* the object because the window version is effectively a pointer. - for (const [k, v] of Object.entries(_config)) { - // @ts-ignore - for some reason it's not picking up on global.d.ts types. - window.mxReactSdkConfig[k] = v; - } - }, config); -} - export async function serializeLog(msg: ConsoleMessage): Promise { // 9 characters padding is somewhat arbitrary ("warning".length + some) let s = `${new Date().toISOString()} | ${ padEnd(msg.type(), 9, ' ')}| ${msg.text()} `; // trailing space is intentional