diff --git a/cypress/e2e/settings/hidden-rr-migration.spec.ts b/cypress/e2e/settings/hidden-rr-migration.spec.ts new file mode 100644 index 0000000000..1d5419af33 --- /dev/null +++ b/cypress/e2e/settings/hidden-rr-migration.spec.ts @@ -0,0 +1,90 @@ +/* +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 { SynapseInstance } from "../../plugins/synapsedocker"; + +function seedLabs(synapse: SynapseInstance, labsVal: boolean | null): void { + cy.initTestUser(synapse, "Sally", () => { + // seed labs flag + cy.window({ log: false }).then(win => { + if (typeof labsVal === "boolean") { + // stringify boolean + win.localStorage.setItem("mx_labs_feature_feature_hidden_read_receipts", `${labsVal}`); + } + }); + }); +} + +function testForVal(settingVal: boolean | null): void { + const testRoomName = "READ RECEIPTS"; + cy.createRoom({ name: testRoomName }).as("roomId"); + cy.all([cy.get("@roomId")]).then(() => { + cy.viewRoomByName(testRoomName).then(() => { + // if we can see the room, then sync is working for us. It's time to see if the + // migration even ran. + + cy.getSettingValue("sendReadReceipts", null, true).should("satisfy", (val) => { + if (typeof settingVal === "boolean") { + return val === settingVal; + } else { + return !val; // falsy - we don't actually care if it's undefined, null, or a literal false + } + }); + }); + }); +} + +describe("Hidden Read Receipts Setting Migration", () => { + // We run this as a full-blown end-to-end test to ensure it works in an integration + // sense. If we unit tested it, we'd be testing that the code works but not that the + // migration actually runs. + // + // Here, we get to test that not only the code works but also that it gets run. Most + // of our interactions are with the JS console as we're honestly just checking that + // things got set correctly. + // + // For a security-sensitive feature like hidden read receipts, it's absolutely vital + // that we migrate the setting appropriately. + + let synapse: SynapseInstance; + + beforeEach(() => { + cy.startSynapse("default").then(data => { + synapse = data; + }); + }); + + afterEach(() => { + cy.stopSynapse(synapse); + }); + + it('should not migrate the lack of a labs flag', () => { + seedLabs(synapse, null); + testForVal(null); + }); + + it('should migrate labsHiddenRR=false as sendRR=true', () => { + seedLabs(synapse, false); + testForVal(true); + }); + + it('should migrate labsHiddenRR=true as sendRR=false', () => { + seedLabs(synapse, true); + testForVal(false); + }); +}); diff --git a/cypress/support/login.ts b/cypress/support/login.ts index da72ec69a4..4cdfd6a84d 100644 --- a/cypress/support/login.ts +++ b/cypress/support/login.ts @@ -35,13 +35,19 @@ declare global { * Generates a test user and instantiates an Element session with that user. * @param synapse the synapse returned by startSynapse * @param displayName the displayName to give the test user + * @param prelaunchFn optional function to run before the app is visited */ - initTestUser(synapse: SynapseInstance, displayName: string): Chainable; + initTestUser( + synapse: SynapseInstance, + displayName: string, + prelaunchFn?: () => void, + ): Chainable; } } } -Cypress.Commands.add("initTestUser", (synapse: SynapseInstance, displayName: string): Chainable => { +// eslint-disable-next-line max-len +Cypress.Commands.add("initTestUser", (synapse: SynapseInstance, displayName: string, prelaunchFn?: () => void): Chainable => { // XXX: work around Cypress not clearing IDB between tests cy.window({ log: false }).then(win => { win.indexedDB.databases().then(databases => { @@ -87,6 +93,8 @@ Cypress.Commands.add("initTestUser", (synapse: SynapseInstance, displayName: str win.localStorage.setItem("mx_local_settings", '{"language":"en"}'); }); + prelaunchFn?.(); + return cy.visit("/").then(() => { // wait for the app to load return cy.get(".mx_MatrixChat", { timeout: 15000 }); diff --git a/cypress/support/settings.ts b/cypress/support/settings.ts index a44f3f06d2..06ec815364 100644 --- a/cypress/support/settings.ts +++ b/cypress/support/settings.ts @@ -96,7 +96,7 @@ declare global { * value. * @return {*} The value, or null if not found */ - getSettingValue(name: string, roomId?: string): Chainable; + getSettingValue(name: string, roomId?: string, excludeDefault?: boolean): Chainable; } } } @@ -116,9 +116,10 @@ Cypress.Commands.add("setSettingValue", ( }); }); -Cypress.Commands.add("getSettingValue", (name: string, roomId?: string): Chainable => { +// eslint-disable-next-line max-len +Cypress.Commands.add("getSettingValue", (name: string, roomId?: string, excludeDefault?: boolean): Chainable => { return cy.getSettingsStore().then((store: typeof SettingsStore) => { - return store.getValue(name, roomId); + return store.getValue(name, roomId, excludeDefault); }); }); diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index df9dec9f5e..94b2e25706 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -826,6 +826,9 @@ async function startMatrixClient(startSyncing = true): Promise { await MatrixClientPeg.assign(); } + // Run the migrations after the MatrixClientPeg has been assigned + SettingsStore.runMigrations(); + // This needs to be started after crypto is set up DeviceListener.sharedInstance().start(); // Similarly, don't start sending presence updates until we've started diff --git a/src/settings/SettingsStore.ts b/src/settings/SettingsStore.ts index b6f63d4e9c..76b12b62bb 100644 --- a/src/settings/SettingsStore.ts +++ b/src/settings/SettingsStore.ts @@ -35,6 +35,9 @@ import SettingsHandler from "./handlers/SettingsHandler"; import { SettingUpdatedPayload } from "../dispatcher/payloads/SettingUpdatedPayload"; import { Action } from "../dispatcher/actions"; import PlatformSettingsHandler from "./handlers/PlatformSettingsHandler"; +import dispatcher from "../dispatcher/dispatcher"; +import { ActionPayload } from "../dispatcher/payloads"; +import { MatrixClientPeg } from "../MatrixClientPeg"; const defaultWatchManager = new WatchManager(); @@ -565,6 +568,44 @@ export default class SettingsStore { return null; } + /** + * Runs or queues any setting migrations needed. + */ + public static runMigrations(): void { + // Dev notes: to add your migration, just add a new `migrateMyFeature` function, call it, and + // add a comment to note when it can be removed. + + SettingsStore.migrateHiddenReadReceipts(); // Can be removed after October 2022. + } + + private static migrateHiddenReadReceipts(): void { + if (MatrixClientPeg.get().isGuest()) return; // not worth it + + // We wait for the first sync to ensure that the user's existing account data has loaded, as otherwise + // getValue() for an account-level setting like sendReadReceipts will return `null`. + const disRef = dispatcher.register((payload: ActionPayload) => { + if (payload.action === "MatrixActions.sync") { + dispatcher.unregister(disRef); + + const rrVal = SettingsStore.getValue("sendReadReceipts", null, true); + if (typeof rrVal !== "boolean") { + // new setting isn't set - see if the labs flag was. We have to manually reach into the + // handler for this because it isn't a setting anymore (`getValue` will yell at us). + const handler = LEVEL_HANDLERS[SettingLevel.DEVICE] as DeviceSettingsHandler; + const labsVal = handler.readFeature("feature_hidden_read_receipts"); + if (typeof labsVal === "boolean") { + // Inverse of labs flag because negative->positive language switch in setting name + const newVal = !labsVal; + console.log(`Setting sendReadReceipts to ${newVal} because of previously-set labs flag`); + + // noinspection JSIgnoredPromiseFromCall + SettingsStore.setValue("sendReadReceipts", null, SettingLevel.ACCOUNT, newVal); + } + } + } + }); + } + /** * Debugging function for reading explicit setting values without going through the * complicated/biased functions in the SettingsStore. This will print information to diff --git a/src/settings/handlers/DeviceSettingsHandler.ts b/src/settings/handlers/DeviceSettingsHandler.ts index 138457d5a9..4be80b764d 100644 --- a/src/settings/handlers/DeviceSettingsHandler.ts +++ b/src/settings/handlers/DeviceSettingsHandler.ts @@ -114,12 +114,16 @@ export default class DeviceSettingsHandler extends AbstractLocalStorageSettingsH // Note: features intentionally don't use the same key as settings to avoid conflicts // and to be backwards compatible. - private readFeature(featureName: string): boolean | null { + // public for access to migrations - not exposed from the SettingsHandler interface + public readFeature(featureName: string): boolean | null { if (MatrixClientPeg.get() && MatrixClientPeg.get().isGuest()) { // Guests should not have any labs features enabled. return false; } + // XXX: This turns they key names into `mx_labs_feature_feature_x` (double feature). + // This is because all feature names start with `feature_` as a matter of policy. + // Oh well. return this.getBoolean("mx_labs_feature_" + featureName); }