Merge pull request #212 from element-hq/t3chguy/wat/382

modules/restricted-guests/element-web/e2e/restricted-guests.spec.ts

@@ -6,24 +6,23 @@ Please see LICENSE files in the repository root for full details.
 */
 
 import {
-    MatrixAuthenticationServiceContainer,
     type MasConfig,
     type StartedMatrixAuthenticationServiceContainer,
     type StartedSynapseContainer,
-    type SynapseConfig,
+    type SynapseContainer,
 } from "@element-hq/element-web-playwright-common/lib/testcontainers/index.js";
-import type { Credentials } from "@element-hq/element-web-playwright-common/lib/utils/api";
-import type { Fixtures } from "@playwright/test";
+import { type Credentials } from "@element-hq/element-web-playwright-common/lib/utils/api";
+import { makePostgres } from "@element-hq/element-web-playwright-common/lib/testcontainers/postgres.js";
+import { makeMas } from "@element-hq/element-web-playwright-common/lib/testcontainers/mas.js";
 
-import { test as base, expect } from "../../../../playwright/element-web-test";
 import { RestrictedGuestsSynapseContainer, RestrictedGuestsSynapseWithMasContainer } from "./services";
+import { test as subBase, expect } from "../../../../playwright/element-web-test";
 
 const MAS_CLIENT_ID = "01ARZ3NDEKTSV4RRFFQ69G5FAV";
 const MAS_CLIENT_SECRET = "restricted-guests-secret";
 const MAS_SHARED_SECRET = "restricted-guests-shared-secret";
-const MAS_INTERNAL_URL = "http://mas:8080";
+const MAS_INTERNAL_URL = "http://guest-mas:8080";
 const GUEST_HOMESERVER_NAME = "guest-homeserver";
-const GUEST_HOMESERVER_INTERNAL_URL = "http://guest-homeserver:8008";
 
 const MAS_HTTP_LISTENERS: NonNullable<MasConfig["http"]>["listeners"] = [
     {
@@ -60,17 +59,11 @@ const MAS_HTTP_LISTENERS: NonNullable<MasConfig["http"]>["listeners"] = [
     },
 ];
 
-const MAS_CONFIG: Partial<MasConfig> = {
+const BASE_MAS_CONFIG: Partial<MasConfig> = {
     http: {
         listeners: MAS_HTTP_LISTENERS,
         public_base: "",
     },
-    matrix: {
-        kind: "synapse",
-        homeserver: GUEST_HOMESERVER_NAME,
-        endpoint: GUEST_HOMESERVER_INTERNAL_URL,
-        secret: MAS_SHARED_SECRET,
-    },
     policy: {
         data: {
             admin_clients: [MAS_CLIENT_ID],
@@ -88,17 +81,27 @@ const MAS_CONFIG: Partial<MasConfig> = {
     ],
 };
 
-const applySharedTestConfig = (testInstance: typeof base) => {
-    testInstance.use({
-        displayName: "Tommy",
-        synapseConfig: {
-            allow_guest_access: true,
-        },
-        labsFlags: ["feature_ask_to_join"],
-    });
-};
+declare module "@element-hq/element-web-module-api" {
+    export interface Config {
+        embedded_pages?: {
+            login_for_welcome?: boolean;
+        };
+    }
+}
 
-const sharedFixtures: Fixtures<{ testRoomId: string }, { bot: Credentials }, any, any> = {
+// We do some wacky things here in order to run the test suite against multiple homeserver configurations
+const base = subBase.extend<
+    {
+        testRoomId: string;
+    },
+    {
+        auth: "mas" | "legacy";
+
+        bot: Credentials;
+        guestMas?: StartedMatrixAuthenticationServiceContainer;
+        guestHomeserver: StartedSynapseContainer;
+    }
+>({
     testRoomId: [
         async ({ homeserver, bot }, use) => {
             const { room_id: roomId } = (await homeserver.csApi.request("POST", "/v3/createRoom", bot.accessToken, {
@@ -125,158 +128,178 @@ const sharedFixtures: Fixtures<{ testRoomId: string }, { bot: Credentials }, any
         },
         { scope: "worker" },
     ],
-};
-
-const test = base.extend<
-    {
-        testRoomId: string;
-    },
-    {
-        guestHomeserver: StartedSynapseContainer;
-        bot: Credentials;
-    }
->({
-    ...sharedFixtures,
-    guestHomeserver: [
-        async ({ logger, synapseConfig, network }, use) => {
-            const container = await new RestrictedGuestsSynapseContainer()
-                .withConfig(synapseConfig)
-                .withConfig({ server_name: GUEST_HOMESERVER_NAME })
-                .withNetwork(network)
-                .withNetworkAliases(GUEST_HOMESERVER_NAME)
-                .withLogConsumer(logger.getConsumer("guest_homeserver"))
-                .start();
 
+    auth: ["mas", { scope: "worker" }],
+    // Optional MAS on the default homeserver, enabled only when we are testing the non-guest login UX
+    mas: [
+        async ({ logger, network, postgres, auth, synapseConfig }, use) => {
+            if (auth !== "mas" || synapseConfig.allow_guest_access !== false) {
+                return use(undefined);
+            }
+            const container = await makeMas(
+                postgres,
+                network,
+                logger,
+                {
+                    ...BASE_MAS_CONFIG,
+                    matrix: {
+                        kind: "synapse",
+                        homeserver: "homeserver",
+                        endpoint: "http://homeserver:8008",
+                        secret: MAS_SHARED_SECRET,
+                    },
+                },
+                "mas",
+            );
             await use(container);
             await container.stop();
         },
         { scope: "worker" },
     ],
-});
-
-const masTest = base.extend<
-    {
-        testRoomId: string;
-    },
-    {
-        guestHomeserver: StartedSynapseContainer;
-        guestMas: StartedMatrixAuthenticationServiceContainer;
-        bot: Credentials;
-    }
->({
-    ...sharedFixtures,
+    // Optional MAS on the module homeserver
     guestMas: [
-        async ({ logger, network, postgres }, use) => {
-            const container = await new MatrixAuthenticationServiceContainer(postgres)
-                .withNetwork(network)
-                .withNetworkAliases("mas")
-                .withLogConsumer(logger.getConsumer("guest_mas"))
-                .withConfig(MAS_CONFIG)
-                .start();
+        async ({ logger, network, auth }, use) => {
+            if (auth !== "mas") {
+                return use(undefined);
+            }
 
+            // We need a separate postgres so it doesn't fight with the default MAS
+            const postgres = await makePostgres(network, logger, "guest-mas-postgres");
+
+            const container = await makeMas(
+                postgres,
+                network,
+                logger,
+                {
+                    ...BASE_MAS_CONFIG,
+                    matrix: {
+                        kind: "synapse",
+                        homeserver: GUEST_HOMESERVER_NAME,
+                        endpoint: "http://guest-homeserver:8008",
+                        secret: MAS_SHARED_SECRET,
+                    },
+                },
+                "guest-mas",
+            );
             await use(container);
             await container.stop();
+            await postgres.stop();
         },
         { scope: "worker" },
     ],
+    // Module homeserver
     guestHomeserver: [
         async ({ logger, synapseConfig, network, guestMas }, use) => {
-            const container = await new RestrictedGuestsSynapseWithMasContainer({
-                adminApiBaseUrl: MAS_INTERNAL_URL,
-                oauthBaseUrl: MAS_INTERNAL_URL,
-                clientId: MAS_CLIENT_ID,
-                clientSecret: MAS_CLIENT_SECRET,
-            })
+            let container: SynapseContainer;
+            if (guestMas) {
+                container = new RestrictedGuestsSynapseWithMasContainer({
+                    adminApiBaseUrl: MAS_INTERNAL_URL,
+                    oauthBaseUrl: MAS_INTERNAL_URL,
+                    clientId: MAS_CLIENT_ID,
+                    clientSecret: MAS_CLIENT_SECRET,
+                }).withMatrixAuthenticationService(guestMas);
+            } else {
+                container = new RestrictedGuestsSynapseContainer();
+            }
+
+            const startedContainer = await container
                 .withConfig(synapseConfig)
                 .withConfig({
                     server_name: GUEST_HOMESERVER_NAME,
-                    matrix_authentication_service: {
-                        enabled: true,
-                        endpoint: `${MAS_INTERNAL_URL}/`,
-                        secret: MAS_SHARED_SECRET,
-                    },
-                    // Must be disabled when using MAS.
-                    password_config: {
-                        enabled: false,
-                    },
-                    // Must be disabled when using MAS.
-                    enable_registration: false,
-                } as Partial<SynapseConfig>)
-                .withMatrixAuthenticationService(guestMas)
+                })
                 .withNetwork(network)
                 .withNetworkAliases(GUEST_HOMESERVER_NAME)
                 .withLogConsumer(logger.getConsumer("guest_homeserver"))
                 .start();
 
-            await use(container);
-            await container.stop();
+            await use(startedContainer);
+            await startedContainer.stop();
         },
         { scope: "worker" },
     ],
+    displayName: "Tommy",
+    labsFlags: ["feature_ask_to_join"],
+    config: {
+        embedded_pages: {
+            login_for_welcome: true,
+        },
+    },
 });
 
-type RestrictedGuestsTestInstance = typeof test;
-
-const defineRestrictedGuestsTests = (testInstance: RestrictedGuestsTestInstance, suiteName: string) => {
-    applySharedTestConfig(testInstance);
-
-    testInstance.describe(suiteName, () => {
-        testInstance.use({
-            page: async ({ page }, use) => {
-                await page.goto("/");
-                await use(page);
+base.slow();
+for (const auth of ["mas", "legacy"] as const) {
+    for (const guestsEnabled of [true, false]) {
+        const test = base.extend({
+            auth,
+            synapseConfig: {
+                allow_guest_access: guestsEnabled,
             },
         });
 
-        testInstance("should error if config is missing", async ({ page }) => {
-            await expect(page.getByText("Your Element is misconfigured")).toBeVisible();
-            await expect(page.getByText("Errors in module configuration")).toBeVisible();
-        });
-
-        testInstance.describe("with config", () => {
-            testInstance.beforeEach(({ config, guestHomeserver }) => {
-                config["io.element.element-web-modules.restricted-guests"] = {
-                    guest_user_homeserver_url: guestHomeserver.baseUrl,
-                };
+        test.describe(`Restricted guests auth=${auth} guests=${guestsEnabled}`, () => {
+            test("should error if config is missing", async ({ page }) => {
+                await page.goto("/");
+                await expect(page.getByText("Your Element is misconfigured")).toBeVisible();
+                await expect(page.getByText("Errors in module configuration")).toBeVisible();
             });
 
-            testInstance(
-                "should show the default room preview bar for logged in users",
-                { tag: ["@screenshot"] },
-                async ({ page, user, testRoomId }) => {
+            test.describe("with config", () => {
+                test.beforeEach(async ({ config, guestHomeserver, page, testRoomId }) => {
+                    config["io.element.element-web-modules.restricted-guests"] = {
+                        guest_user_homeserver_url: guestHomeserver.baseUrl,
+                    };
                     // Go to a room we are not a member of
                     await page.goto(`/#/room/${testRoomId}`);
+                });
 
-                    const button = page.getByRole("button", { name: "Join the discussion" });
-                    await expect(button).toBeVisible();
-                },
-            );
+                if (guestsEnabled) {
+                    // The screenshots between the two auth type tests for guests should be identical.
+                    test(
+                        "should show the default room preview bar for logged in users",
+                        { tag: ["@screenshot"] },
+                        async ({ page, user, testRoomId }) => {
+                            // Go to a room we are not a member of
+                            await page.goto(`/#/room/${testRoomId}`);
+                            const button = page.getByRole("button", { name: "Join the discussion" });
+                            await expect(button).toBeVisible();
+                        },
+                    );
 
-            testInstance(
-                "should show the module's room preview bar for guests",
-                { tag: ["@screenshot"] },
-                async ({ page, testRoomId }) => {
-                    // Go to a room we are not a member of
-                    await page.goto(`/#/room/${testRoomId}`);
+                    test(
+                        "should show the module's room preview bar for guests",
+                        { tag: ["@screenshot"] },
+                        async ({ page }) => {
+                            const button = page.getByRole("button", { name: "Join as guest", exact: true });
+                            await expect(button).toBeVisible();
+                            await expect(page.locator(".mx_RoomPreviewBar")).toMatchScreenshot(`preview-bar.png`);
 
-                    const button = page.getByRole("button", { name: "Join", exact: true });
-                    await expect(button).toBeVisible();
-                    await expect(page.locator(".mx_RoomPreviewBar")).toMatchScreenshot(`preview-bar.png`);
+                            await button.click();
+                            const dialog = page.getByRole("dialog");
+                            await expect(dialog).toMatchScreenshot(`dialog.png`);
 
-                    await button.click();
-                    const dialog = page.getByRole("dialog");
-                    await expect(dialog).toMatchScreenshot(`dialog.png`);
+                            await dialog.getByPlaceholder("Name").fill("Jim");
+                            await dialog.getByRole("button", { name: "Continue as guest" }).click();
 
-                    await dialog.getByPlaceholder("Name").fill("Jim");
-                    await dialog.getByRole("button", { name: "Continue as guest" }).click();
+                            await expect(page.getByText("Ask to join?")).toBeVisible();
+                        },
+                    );
+                } else {
+                    test("should show the module login ux", { tag: ["@screenshot"] }, async ({ page }) => {
+                        const button = page.getByRole("button", { name: "Join as guest", exact: true });
+                        await expect(button).toBeVisible();
+                        await expect(page.getByRole("main")).toMatchScreenshot(`login-${auth}.png`);
 
-                    await expect(page.getByText("Ask to join?")).toBeVisible();
-                },
-            );
+                        await button.click();
+                        const dialog = page.getByRole("dialog");
+                        await expect(dialog).toMatchScreenshot(`dialog-login.png`);
+
+                        await dialog.getByPlaceholder("Name").fill("Jim");
+                        await dialog.getByRole("button", { name: "Continue as guest" }).click();
+
+                        await expect(page.getByText("Join the discussion")).toBeVisible();
+                    });
+                }
+            });
         });
-    });
-};
-
-// The screenshots between the two tests should be identical.
-defineRestrictedGuestsTests(test, "Restricted Guests");
-defineRestrictedGuestsTests(masTest as RestrictedGuestsTestInstance, "Restricted Guests (MAS)");
+    }
+}

modules/restricted-guests/element-web/src/AuthFooter.tsx

@@ -0,0 +1,59 @@
+/*
+Copyright 2026 Element Creations Ltd.
+
+SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
+Please see LICENSE files in the repository root for full details.
+*/
+
+import { type FC } from "react";
+import { Button } from "@vector-im/compound-web";
+import { type AccountAuthInfo, type Api } from "@element-hq/element-web-module-api";
+import styled from "styled-components";
+
+import { type ModuleConfig } from "./config.ts";
+import RegisterDialog from "./RegisterDialog.tsx";
+
+interface Props {
+    api: Api;
+    config: ModuleConfig;
+    onLoggedIn(data: AccountAuthInfo): void;
+}
+
+const Container = styled.aside`
+    margin: var(--cpd-space-3x) 0;
+
+    button {
+        width: 100%;
+    }
+`;
+
+const AuthFooter: FC<Props> = ({ api, config, onLoggedIn }) => {
+    const onTryJoin = async (): Promise<void> => {
+        const { finished } = api.openDialog(
+            {
+                title: api.i18n.translate("register_dialog_title"),
+            },
+            RegisterDialog,
+            {
+                api,
+                config,
+            },
+        );
+
+        const { model: accountAuthInfo, ok } = await finished;
+
+        if (ok && accountAuthInfo) {
+            onLoggedIn(accountAuthInfo);
+        }
+    };
+
+    return (
+        <Container>
+            <Button onClick={onTryJoin} size="sm" kind="secondary">
+                {api.i18n.translate("join_cta")}
+            </Button>
+        </Container>
+    );
+};
+
+export default AuthFooter;

modules/restricted-guests/element-web/src/RegisterDialog.tsx

@@ -15,6 +15,7 @@ import { type ModuleConfig } from "./config.ts";
 interface RegisterDialogProps extends DialogProps<AccountAuthInfo> {
     api: Api;
     config: ModuleConfig;
+    showLoginLink?: boolean;
 }
 
 const enum State {
@@ -29,7 +30,7 @@ const StyledFormRoot = styled(Form.Root)`
     font-feature-settings: normal;
 `;
 
-const RegisterDialog: FC<RegisterDialogProps> = ({ api, config, onCancel, onSubmit }) => {
+const RegisterDialog: FC<RegisterDialogProps> = ({ api, config, onCancel, onSubmit, showLoginLink }) => {
     const [username, setUsername] = useState("");
     const [state, setState] = useState<State>(State.Idle);
 
@@ -69,7 +70,7 @@ const RegisterDialog: FC<RegisterDialogProps> = ({ api, config, onCancel, onSubm
 
     return (
         <StyledFormRoot onSubmit={trySubmit}>
-            <Form.Field name="mxid">
+            <Form.Field name="name">
                 <Form.Label>{api.i18n.translate("register_dialog_register_username_label")}</Form.Label>
                 <Form.TextControl
                     disabled={disabled}
@@ -82,9 +83,11 @@ const RegisterDialog: FC<RegisterDialogProps> = ({ api, config, onCancel, onSubm
                 {message}
             </Form.Field>
 
-            <a href={config.skip_single_sign_on ? "/#/login" : "/#/start_sso"} onClick={onCancel}>
-                {api.i18n.translate("register_dialog_existing_account")}
-            </a>
+            {showLoginLink && (
+                <a href={config.skip_single_sign_on ? "/#/login" : "/#/start_sso"} onClick={onCancel}>
+                    {api.i18n.translate("register_dialog_existing_account")}
+                </a>
+            )}
 
             <Form.Submit disabled={disabled || !username}>
                 {api.i18n.translate("register_dialog_continue_label")}

modules/restricted-guests/element-web/src/RoomPreviewBar.tsx

@@ -45,6 +45,7 @@ const RoomPreviewBar: FC<RoomPreviewBarProps> = ({ api, config, roomId, roomAlia
             {
                 api,
                 config,
+                showLoginLink: true,
             },
         );
 

modules/restricted-guests/element-web/src/config.ts

@@ -40,5 +40,8 @@ export const CONFIG_KEY = "io.element.element-web-modules.restricted-guests";
 declare module "@element-hq/element-web-module-api" {
     export interface Config {
         [CONFIG_KEY]: input<ConfigSchema>;
+        sso_redirect_options?: {
+            immediate?: boolean; // incompatible option
+        };
     }
 }

modules/restricted-guests/element-web/src/index.tsx

@@ -12,6 +12,7 @@ import Translations from "./translations.json";
 import { ModuleConfig, CONFIG_KEY } from "./config";
 import { name as ModuleName } from "../package.json";
 import RoomPreviewBar from "./RoomPreviewBar.tsx";
+import AuthFooter from "./AuthFooter.tsx";
 
 const GUEST_INVISIBLE_COMPONENTS = [
     "UIComponent.sendInvites",
@@ -41,14 +42,26 @@ class RestrictedGuestsModule implements Module {
             throw new Error(`Errors in module configuration for "${ModuleName}"`);
         }
 
+        const appConfig = this.api.config.get();
+        if (appConfig.sso_redirect_options?.immediate) {
+            console.warn(`${ModuleName} found incompatible option 'sso_redirect_options.immediate', turning it off.`);
+            appConfig.sso_redirect_options.immediate = false;
+        }
+
+        // Room preview bar customisations (for Matrix guest support)
         this.api.customComponents.registerRoomPreviewBar((props, OriginalComponent) => (
             <RoomPreviewBar {...props} api={this.api} config={this.config!}>
                 <OriginalComponent {...props} />
             </RoomPreviewBar>
         ));
+        this.api.customisations.registerShouldShowComponent(this.shouldShowComponent);
 
-        // TODO replace this with a more generic API
-        this.api._registerLegacyComponentVisibilityCustomisations(this);
+        // Login component customisations (for no guest support)
+        this.api.customComponents.registerLoginComponent((props, OriginalComponent) => (
+            <OriginalComponent {...props}>
+                <AuthFooter onLoggedIn={props.onLoggedIn} api={this.api} config={this.config!} />
+            </OriginalComponent>
+        ));
     }
 
     /**
@@ -58,11 +71,12 @@ class RestrictedGuestsModule implements Module {
      * @returns true, if the user should see the component
      */
     public readonly shouldShowComponent = (component: string): boolean => {
-        if (!this.config || !this.api.profile.value.userId?.startsWith(this.config.guest_user_prefix)) {
-            return true;
+        const profile = this.api.profile.value;
+        if (this.config && (profile.isGuest || profile.userId?.startsWith(this.config.guest_user_prefix))) {
+            return GUEST_INVISIBLE_COMPONENTS.includes(component);
         }
 
-        return GUEST_INVISIBLE_COMPONENTS.includes(component);
+        return true;
     };
 }
 

modules/restricted-guests/element-web/src/translations.json

@@ -4,8 +4,8 @@
         "de": "Benutzername"
     },
     "register_dialog_title": {
-        "en": "Request room access",
-        "de": "Raumbeitritt anfragen"
+        "en": "Request access",
+        "de": "Zugriff anfordern"
     },
     "register_dialog_busy": {
         "en": "Creating your account...",
@@ -32,7 +32,7 @@
         "de": "Treten Sie dem Raum bei, um teilzunehmen"
     },
     "join_cta": {
-        "en": "Join",
-        "de": "Verbinden"
+        "en": "Join as guest",
+        "de": "Als Gast beitreten"
     }
 }