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

Author: t3chguy

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");
+        });
+    });
 });

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);
         });

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.
+*/
+
+/// <reference types="cypress" />
+
+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<string, any>): Chainable<AUTWindow>;
+        }
+    }
+}
+
+Cypress.Commands.add("tweakConfig", (tweaks: Record<string, any>): Chainable<AUTWindow> => {
+    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;
+        }
+    });
+});

cypress/support/index.ts

@@ -27,3 +27,4 @@ import "./settings";
 import "./bot";
 import "./clipboard";
 import "./util";
+import "./app";

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<string> = null;
     // The set of device IDs we're currently displaying toasts for
     private displayingToastsForDeviceIds = new Set<string>();
+    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<string>) {
+    public async dismissUnverifiedSessions(deviceIds: Iterable<string>) {
         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;

src/Lifecycle.ts

@@ -168,7 +168,7 @@ export async function loadSession(opts: ILoadSessionOpts = {}): Promise<boolean>
  * 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<boolean> {
  * 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<Matr
  * If the credentials belong to a different user from the session already stored,
  * the old session will be cleared automatically.
  *
- * @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
  */
@@ -731,27 +731,25 @@ export function logout(): void {
     if (MatrixClientPeg.get().isGuest()) {
         // logout doesn't work for guest sessions
         // Also we sometimes want to re-log in a guest session if we abort the login.
-        // defer until next tick because it calls a synchronous dispatch and we are likely here from a dispatch.
+        // defer until next tick because it calls a synchronous dispatch, and we are likely here from a dispatch.
         setImmediate(() => onLoggedOut());
         return;
     }
 
     _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<void> {
  * storage. Used after a session has been logged out.
  */
 export async function onLoggedOut(): Promise<void> {
-    _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<void> {
     // 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<void
         }
     }
 
-    if (window.sessionStorage) {
-        window.sessionStorage.clear();
-    }
+    window.sessionStorage?.clear();
 
     // create a temporary client to clear out the persistent stores.
     const cli = createMatrixClient({
@@ -937,7 +937,7 @@ export function stopMatrixClient(unsetClient = true): void {
     IntegrationManagers.sharedInstance().stopWatching();
     Mjolnir.sharedInstance().stop();
     DeviceListener.sharedInstance().stop();
-    if (DMRoomMap.shared()) DMRoomMap.shared().stop();
+    DMRoomMap.shared()?.stop();
     EventIndexPeg.stop();
     const cli = MatrixClientPeg.get();
     if (cli) {

src/SecurityManager.ts

@@ -257,7 +257,7 @@ async function onSecretRequested(
     if (userId !== client.getUserId()) {
         return;
     }
-    if (!deviceTrust || !deviceTrust.isVerified()) {
+    if (!deviceTrust?.isVerified()) {
         logger.log(`Ignoring secret request from untrusted device ${deviceId}`);
         return;
     }
@@ -296,7 +296,7 @@ export const crossSigningCallbacks: ICryptoCallbacks = {
 };
 
 export async function promptForBackupPassphrase(): Promise<Uint8Array> {
-    let key;
+    let key: Uint8Array;
 
     const { finished } = Modal.createTrackedDialog('Restore Backup', '', RestoreKeyBackupDialog, {
         showSummary: false, keyCallback: k => key = k,

src/stores/SetupEncryptionStore.ts

@@ -89,16 +89,15 @@ 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);
         }
     }
 
     public async fetchKeyInfo(): Promise<void> {
+        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<void> {
+        if (!this.started) return; // bail if we were stopped
         if (request.otherUserId !== MatrixClientPeg.get().getUserId()) return;
 
         if (this.verificationRequest) {

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<ElementSession>,
     restCreator: RestSessionCreator): Promise<void> {
     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<ElementSess
     const stickerSession = await createSession("sally");
     await stickerScenarios("sally", "ilikestickers", stickerSession, restCreator);
 
-    // we spawn yet another session for SSO stuff because it involves authentication and
-    // logout, which can/does affect other tests dramatically. See notes above regarding
-    // stickers for the performance loss of doing this.
-    const ssoSession = await createUser("enterprise_erin");
-    await ssoCustomisationScenarios(ssoSession);
-
     // Create a new window to test app auto-updating
     const updateSession = await createSession("update");
     await updateScenarios(updateSession);