From aa8c0f5cc7e06d8a76a719cdb6de40436e3fdd1d Mon Sep 17 00:00:00 2001
From: Andy Balaam <andy.balaam@matrix.org>
Date: Mon, 24 Apr 2023 11:02:17 +0100
Subject: [PATCH] Honour feature toggles in guest mode (#10651)

* Honour feature toggles in guest mode

* Suppress TS warning about returning null MatrixClient

* Revert "Suppress TS warning about returning null MatrixClient"

Don't ts-ignore this - we will eventually fix it via the strict work.

This reverts commit 0c657e6afd4c69d07c39bc2783184f36feaf940f.
---
 .../handlers/DeviceSettingsHandler.ts         | 11 +--
 .../handlers/DeviceSettingsHandler-test.ts    | 81 +++++++++++++++++++
 2 files changed, 87 insertions(+), 5 deletions(-)
 create mode 100644 test/settings/handlers/DeviceSettingsHandler-test.ts

diff --git a/src/settings/handlers/DeviceSettingsHandler.ts b/src/settings/handlers/DeviceSettingsHandler.ts
index e50015f847..978c83c1a6 100644
--- a/src/settings/handlers/DeviceSettingsHandler.ts
+++ b/src/settings/handlers/DeviceSettingsHandler.ts
@@ -16,7 +16,6 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { MatrixClientPeg } from "../../MatrixClientPeg";
 import { SettingLevel } from "../SettingLevel";
 import { CallbackFn, WatchManager } from "../WatchManager";
 import AbstractLocalStorageSettingsHandler from "./AbstractLocalStorageSettingsHandler";
@@ -117,10 +116,12 @@ export default class DeviceSettingsHandler extends AbstractLocalStorageSettingsH
 
     // 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;
-        }
+        // Previously, we disabled all features for guests, but since different
+        // installations can have site-specific config files which might set up
+        // different behaviour that is relevant to guests, we removed that
+        // special behaviour. See
+        // https://github.com/vector-im/element-web/issues/24513 for the
+        // discussion.
 
         // 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.
diff --git a/test/settings/handlers/DeviceSettingsHandler-test.ts b/test/settings/handlers/DeviceSettingsHandler-test.ts
new file mode 100644
index 0000000000..19d19d4c81
--- /dev/null
+++ b/test/settings/handlers/DeviceSettingsHandler-test.ts
@@ -0,0 +1,81 @@
+/*
+Copyright 2023 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 { mocked } from "jest-mock";
+import { MatrixClient } from "matrix-js-sdk/src/client";
+
+import { MatrixClientPeg } from "../../../src/MatrixClientPeg";
+import DeviceSettingsHandler from "../../../src/settings/handlers/DeviceSettingsHandler";
+import { CallbackFn, WatchManager } from "../../../src/settings/WatchManager";
+import { stubClient } from "../../test-utils/test-utils";
+
+describe("DeviceSettingsHandler", () => {
+    const ROOM_ID_IS_UNUSED = "";
+
+    const unknownSettingKey = "unknown_setting";
+    const featureKey = "my_feature";
+
+    let watchers: WatchManager;
+    let handler: DeviceSettingsHandler;
+    let settingListener: CallbackFn;
+
+    beforeEach(() => {
+        watchers = new WatchManager();
+        handler = new DeviceSettingsHandler([featureKey], watchers);
+        settingListener = jest.fn();
+    });
+
+    afterEach(() => {
+        watchers.unwatchSetting(settingListener);
+    });
+
+    it("Returns undefined for an unknown setting", () => {
+        expect(handler.getValue(unknownSettingKey, ROOM_ID_IS_UNUSED)).toBeUndefined();
+    });
+
+    it("Returns the value for a disabled feature", () => {
+        handler.setValue(featureKey, ROOM_ID_IS_UNUSED, false);
+        expect(handler.getValue(featureKey, ROOM_ID_IS_UNUSED)).toBe(false);
+    });
+
+    it("Returns the value for an enabled feature", () => {
+        handler.setValue(featureKey, ROOM_ID_IS_UNUSED, true);
+        expect(handler.getValue(featureKey, ROOM_ID_IS_UNUSED)).toBe(true);
+    });
+
+    describe("If I am a guest", () => {
+        let client: MatrixClient;
+
+        beforeEach(() => {
+            client = stubClient();
+            mocked(client.isGuest).mockReturnValue(true);
+        });
+
+        afterEach(() => {
+            MatrixClientPeg.get = () => null;
+        });
+
+        it("Returns the value for a disabled feature", () => {
+            handler.setValue(featureKey, ROOM_ID_IS_UNUSED, false);
+            expect(handler.getValue(featureKey, ROOM_ID_IS_UNUSED)).toBe(false);
+        });
+
+        it("Returns the value for an enabled feature", () => {
+            handler.setValue(featureKey, ROOM_ID_IS_UNUSED, true);
+            expect(handler.getValue(featureKey, ROOM_ID_IS_UNUSED)).toBe(true);
+        });
+    });
+});