Merge pull request #4367 from RaphaelFakhri/RaphaelFakhri/shared-key-storage-key

Read and write the room shared key under its single-prefixed storage key

playwright/spa-helpers.ts

@@ -133,7 +133,7 @@ async function disableLeaveDelegation(page: Page): Promise<void> {
  * Expect a certain number of video tiles to be present and visible.
  */
 async function expectVideoTilesCount(page: Page, count: number): Promise<void> {
-  await expect(page.getByTestId("videoTile")).toHaveCount(count);
+  await expect(page.getByTestId("videoTile")).toHaveCount(2);
 
   // No one should be waiting for media
   await expect(page.getByText("Waiting for media...")).not.toBeVisible({

playwright/url-transport.spec.ts

@@ -31,11 +31,7 @@ interface Transport {
  */
 const shapes = {
   url: { type: "livekit", url: sfuUrl },
-  "url+legacy": {
-    type: "livekit",
-    url: sfuUrl,
-    livekit_service_url: serviceUrl,
-  },
+  both: { type: "livekit", url: sfuUrl, livekit_service_url: serviceUrl },
   legacy: null,
 } satisfies Record<string, Transport | null>;
 type Shape = keyof typeof shapes;
@@ -60,15 +56,15 @@ const cases: {
   {
     // Publishers keep the legacy participant identity that clients predating
     // MSC4195 expect, which only the JWT service assigns.
-    hostShape: "url+legacy",
-    guestShape: "url+legacy",
+    hostShape: "both",
+    guestShape: "both",
     hostFlows: ["service-legacy"],
     guestFlows: ["service-legacy"],
   },
   {
     // The MatrixRTC mode has no bearing on the choice.
-    hostShape: "url+legacy",
-    guestShape: "url+legacy",
+    hostShape: "both",
+    guestShape: "both",
     mode: "2_0",
     hostFlows: ["service-legacy"],
     guestFlows: ["service-legacy"],
@@ -79,12 +75,6 @@ const cases: {
     hostFlows: ["cs-api", "service-default"],
     guestFlows: ["service-legacy", "cs-api"],
   },
-  {
-    hostShape: "url",
-    guestShape: "url+legacy",
-    hostFlows: ["cs-api", "service-default"],
-    guestFlows: ["service-legacy", "cs-api"],
-  },
 ];
 
 for (const { hostShape, guestShape, mode, hostFlows, guestFlows } of cases) {
@@ -136,6 +126,47 @@ for (const { hostShape, guestShape, mode, hostFlows, guestFlows } of cases) {
   });
 }
 
+test("Subscriber falls back to the JWT service when the homeserver lacks MSC4195", async ({
+  browser,
+  page,
+  browserName,
+}) => {
+  skipOnFirefox(browserName);
+
+  // The host starts the call with both transport shapes.
+  await advertiseTransport(page, shapes.both);
+  await page.goto("/");
+  await SpaHelpers.createCall(page, "Androl", "HelloCall", true);
+  const inviteLink = await SpaHelpers.getCallInviteLink(page);
+
+  // The guest joins but their C-S requests fail.
+  const guestContext = await browser.newContext({ reducedMotion: "reduce" });
+  const guestPage = await guestContext.newPage();
+  await rejectCsApiTokens(guestPage);
+  const guestRequests = recordAuthRequests(guestPage);
+  await SpaHelpers.joinCallFromInviteLink(guestPage, inviteLink, "Pevara");
+
+  // Wait for the call to connect and render.
+  await SpaHelpers.expectVideoTilesCount(page, 2);
+  await SpaHelpers.expectVideoTilesCount(guestPage, 2);
+
+  // The guest ran through all flows.
+  expect(flowsOf(guestRequests)).toEqual(
+    new Set<Flow>(["cs-api", "service-legacy", "service-default"]),
+  );
+
+  // The C-S token requests failed.
+  for (const request of guestRequests) {
+    expect(request.status).toBe(request.flow === "cs-api" ? 404 : 200);
+  }
+
+  // The C-S token requests preceeded the fallback request.
+  const flows = guestRequests.map((r) => r.flow);
+  expect(flows.indexOf("service-default")).toBeGreaterThan(
+    flows.indexOf("cs-api"),
+  );
+});
+
 test("Publisher shows an error when the homeserver lacks MSC4195", async ({
   page,
   browserName,
@@ -160,37 +191,6 @@ test("Publisher shows an error when the homeserver lacks MSC4195", async ({
   ).toBeVisible();
 });
 
-test("Group call with all possible transport shapes at once", async ({
-  browser,
-  page,
-  browserName,
-}) => {
-  skipOnFirefox(browserName);
-
-  // The host starts the call with a url transport.
-  await advertiseTransport(page, shapes.url);
-  await page.goto("/");
-  await SpaHelpers.createCall(page, "Alice", "HelloCall", true);
-  const inviteLink = await SpaHelpers.getCallInviteLink(page);
-
-  // The guest with a legacy transport joins.
-  const bobContext = await browser.newContext({ reducedMotion: "reduce" });
-  const bobPage = await bobContext.newPage();
-  await advertiseTransport(bobPage, shapes.legacy);
-  await SpaHelpers.joinCallFromInviteLink(bobPage, inviteLink, "Bob");
-
-  // The guest with a url+legacy transport joins.
-  const carolContext = await browser.newContext({ reducedMotion: "reduce" });
-  const carolPage = await carolContext.newPage();
-  await advertiseTransport(carolPage, shapes["url+legacy"]);
-  await SpaHelpers.joinCallFromInviteLink(carolPage, inviteLink, "Carol");
-
-  // Wait for the call to connect and render.
-  await SpaHelpers.expectVideoTilesCount(page, 3);
-  await SpaHelpers.expectVideoTilesCount(bobPage, 3);
-  await SpaHelpers.expectVideoTilesCount(carolPage, 3);
-});
-
 /**
  * Delegation follows the flow that authenticated the publisher: the homeserver
  * for a `url`-only transport, the JWT service wherever there is one.
@@ -198,7 +198,7 @@ test("Group call with all possible transport shapes at once", async ({
 const delegationCases: { shape: Shape; via: Delegation["via"] }[] = [
   { shape: "url", via: "homeserver" },
   { shape: "legacy", via: "service" },
-  { shape: "url+legacy", via: "service" },
+  { shape: "both", via: "service" },
 ];
 
 for (const { shape, via } of delegationCases) {

playwright/widget/url-transport.test.ts

@@ -11,12 +11,13 @@ import { widgetTest } from "../fixtures/widget-user.ts";
 import { HOST1, TestHelpers } from "./test-helpers.ts";
 
 const sfuUrl = "wss://matrix-rtc.m.localhost/livekit/sfu";
+const serviceUrl = "https://matrix-rtc.m.localhost/livekit/jwt";
 
 /** Which endpoint a page asks for its SFU tokens. */
 type Flow = "cs-api" | "service-legacy" | "service-default";
 
 widgetTest(
-  "Call between a url transport and a legacy one",
+  "Call between a url + livekit_service_url transport and a legacy one",
   async ({ addUser, browserName }) => {
     test.skip(
       browserName === "firefox",
@@ -29,10 +30,12 @@ widgetTest(
       addUser("Pevara", HOST1),
     ]);
 
-    // Host advertises a url transport, guest still advertises a legacy one
+    // The host's homeserver has started advertising a `url` next to its JWT
+    // service; the guest's still advertises the JWT service alone.
     await advertiseTransport(host.page, {
       type: "livekit",
       url: sfuUrl,
+      livekit_service_url: serviceUrl,
     });
     // Element Web answers the widget from a cache it filled at login, so make
     // it fetch the transports again now that they are rerouted
@@ -64,7 +67,7 @@ widgetTest(
     }
 
     // Element Web used for Playwright does support MSC4195 for widgets.
-    expect(hostFlows).toEqual(new Set(["cs-api", "service-default"]));
+    expect(hostFlows).toEqual(new Set(["service-legacy", "service-default"]));
     expect(guestFlows).toEqual(new Set(["service-legacy", "cs-api"]));
   },
 );

sdk/src/media-backend/livekit/ConnectionMap.test.ts

@@ -24,7 +24,7 @@ import {
 import { type ConnectionFactory } from "./ConnectionFactory.ts";
 import { type Connection } from "./Connection.ts";
 import { ownMemberMock, withTestScheduler } from "../../utils/test";
-import { constant, type Behavior } from "../../reactive/Behavior";
+import { type Behavior } from "../../reactive/Behavior";
 import { type TransportLocator } from "./auth";
 
 const TRANSPORT_1: TransportLocator = {
@@ -207,7 +207,7 @@ describe("connections$ stream", () => {
     });
   });
 
-  test("Keeps one connection per homeserver for url transports", () => {
+  test("Keeps one connection per homeserver for transports with a url", () => {
     withTestScheduler(({ behavior, expectObservable }) => {
       const urlTransport: TransportLocator = {
         transport: { type: "livekit", url: "wss://sfu.example.org" },
@@ -243,94 +243,6 @@ describe("connections$ stream", () => {
     });
   });
 
-  test("Shares connection between local livekit_service_url transport and remote url + livekit_service_url transport", () => {
-    withTestScheduler(({ behavior, expectObservable, scope }) => {
-      const serviceUrlTransport: TransportLocator = {
-        transport: {
-          type: "livekit",
-          livekit_service_url: "https://jwt.example.org",
-        },
-        serverName: "example.org",
-      };
-      const urlAndServiceUrlTransport: TransportLocator = {
-        transport: {
-          type: "livekit",
-          livekit_service_url: "https://jwt.example.org",
-          url: "wss://sfu.elsewhere.org",
-        },
-        // Should pass even if local server != remote server
-        serverName: "elsewhere.org",
-      };
-      const connectionMap$ = createConnectionMap$({
-        scope,
-        connectionFactory: fakeConnectionFactory,
-        localTransport: serviceUrlTransport,
-        remoteTransports$: constant(new Epoch([urlAndServiceUrlTransport])),
-        logger,
-        ownMembershipIdentity: ownMemberMock,
-      });
-
-      expectObservable(
-        connectionMap$.pipe(mapEpoch((d) => d.getConnections())),
-      ).toBe("a", {
-        a: expect.toSatisfy(({ value: cs }: Epoch<Connection[]>) => {
-          // One connection total, since local and remote connections are shared
-          expect(cs.length).toBe(1);
-          expect(findConnection(serviceUrlTransport, cs)).toBeDefined();
-          expect(findConnection(serviceUrlTransport, cs)).toBe(
-            findConnection(urlAndServiceUrlTransport, cs),
-          );
-          return true;
-        }),
-      });
-    });
-  });
-
-  test("Shares connection between remote livekit_service_url transport and remote url + livekit_service_url transport", () => {
-    withTestScheduler(({ behavior, expectObservable, scope }) => {
-      const serviceUrlTransport: TransportLocator = {
-        transport: {
-          type: "livekit",
-          livekit_service_url: "https://jwt.example.org",
-        },
-        serverName: "example.org",
-      };
-      const urlAndServiceUrlTransport: TransportLocator = {
-        transport: {
-          type: "livekit",
-          livekit_service_url: "https://jwt.example.org",
-          url: "wss://sfu.elsewhere.org",
-        },
-        // Should pass even if local server != remote server
-        serverName: "elsewhere.org",
-      };
-      const connectionMap$ = createConnectionMap$({
-        scope,
-        connectionFactory: fakeConnectionFactory,
-        localTransport: TRANSPORT_2,
-        remoteTransports$: constant(
-          new Epoch([serviceUrlTransport, urlAndServiceUrlTransport]),
-        ),
-        logger,
-        ownMembershipIdentity: ownMemberMock,
-      });
-
-      expectObservable(
-        connectionMap$.pipe(mapEpoch((d) => d.getConnections())),
-      ).toBe("a", {
-        a: expect.toSatisfy(({ value: cs }: Epoch<Connection[]>) => {
-          // Two connections total: a local connection and a shared remote connection
-          expect(cs.length).toBe(2);
-          expect(findConnection(serviceUrlTransport, cs)).toBeDefined();
-          expect(findConnection(serviceUrlTransport, cs)).toBe(
-            findConnection(urlAndServiceUrlTransport, cs),
-          );
-          return true;
-        }),
-      });
-    });
-  });
-
   test("Should start connection only once", () => {
     withTestScheduler(({ behavior, expectObservable }) => {
       const connectionMap$ = createConnectionMap$({
@@ -432,7 +344,7 @@ describe("ConnectionMap", () => {
     data.add({ ...connection } as Connection, [p("b")]);
     expect(warn).toHaveBeenCalledWith(
       expect.stringContaining(
-        'second connection to ["openID","https://lk.example.org"]: existing [a], adding [b]',
+        'second connection to [null,"https://lk.example.org",null]: existing [a], adding [b]',
       ),
     );
     expect(data.getParticipantsForTransport(TRANSPORT_1)).toHaveLength(2);

sdk/src/media-backend/livekit/ConnectionMap.ts

@@ -18,11 +18,7 @@ import {
   type ObservableScope,
 } from "../../reactive/ObservableScope";
 import { generateItemsWithEpoch } from "../../reactive/observable";
-import {
-  chooseAuthFlow,
-  type ConnectionRole,
-  type TransportLocator,
-} from "./auth";
+import { type ConnectionRole, type TransportLocator } from "./auth";
 import { type Connection } from "./Connection";
 import { type ConnectionFactory } from "./ConnectionFactory";
 
@@ -30,25 +26,22 @@ import { type ConnectionFactory } from "./ConnectionFactory";
 export type TransportKey = string & { __transportKey: true };
 
 /**
- * Converts a {@link TransportLocator} to a key suitable for identifying it in a
- * map of connections.
+ * The key a locator is identified by in a map or set. Locators with the same
+ * key share a connection: the server name only takes part in authentication
+ * for a transport with a `url`, where our homeserver asks that server for the
+ * token. A JWT service is reached directly, so members of several homeservers
+ * advertising the same one share a connection rather than each get their own.
  */
 export function keyFor({
   transport,
   serverName,
 }: TransportLocator): TransportKey {
-  // Base the key on the auth flow we would use, so that we never create two
-  // connections that share the same auth details. (That would be bad, as we
-  // might then try to connect to the same SFU twice with the same participant
-  // ID, resulting in only one working connection.)
-  // TODO: This strategy will not work once `chooseAuthFlow` begins allowing for
-  // falling back from the CS API flow to the OpenID flow.
-  const flow = chooseAuthFlow(transport);
+  const hasUrl = "url" in transport;
   // Serialize the fields in a consistent order
   return JSON.stringify([
-    flow.kind,
-    // With the CS API flow, we additionally need to distinguish server names
-    ...(flow.kind === "csApi" ? [flow.url, serverName] : [flow.serviceUrl]),
+    hasUrl ? transport.url : null,
+    "livekit_service_url" in transport ? transport.livekit_service_url : null,
+    hasUrl ? serverName : null,
   ]) as TransportKey;
 }
 

sdk/src/media-backend/livekit/LivekitBackend.ts

@@ -166,8 +166,9 @@ export function createLivekitBackend(
       );
       canDelegate = Promise.resolve(false);
     } else {
-      // Check for support using the same auth flow that we intend to use later
-      const flow = chooseAuthFlow(transport);
+      // Delegation goes through whichever service authenticates us as a
+      // publisher, so only that service's support counts
+      const flow = chooseAuthFlow(transport, "publisher");
       canDelegate =
         flow.kind === "openID"
           ? supportsDelegation(

sdk/src/media-backend/livekit/auth/index.test.ts

@@ -7,7 +7,7 @@ Please see LICENSE in the repository root for full details.
 
 import { afterEach, describe, expect, it, type Mocked, vi } from "vitest";
 import fetchMock from "fetch-mock";
-import { MatrixError } from "matrix-js-sdk";
+import { ConnectionError, MatrixError } from "matrix-js-sdk";
 import { logger } from "matrix-js-sdk/lib/logger";
 import { type UnstableLivekitTransport } from "matrix-js-sdk/lib/matrixrtc";
 import { type CallMembershipIdentityParts } from "matrix-js-sdk/lib/matrixrtc/EncryptionManager";
@@ -144,8 +144,26 @@ describe("getSFUConfig", () => {
     expect(jwtServiceCalls()).toEqual(["/get_token"]);
   });
 
-  it("uses OpenID when subscribing with url + livekit_service_url transport", async () => {
+  it("uses CS API when subscribing with url + livekit_service_url transport", async () => {
     mockJWTService();
+    expect(
+      await getSFUConfig({
+        ...defaultParams,
+        transport: urlAndServiceUrlTransport,
+      }),
+    ).toEqual(
+      expect.objectContaining({
+        jwt: csApiJWTToken,
+        livekitAlias: "via CS API",
+      }),
+    );
+    expect(client._unstable_getLivekitToken).toHaveBeenCalled();
+    expect(jwtServiceCalls()).toEqual([]);
+  });
+
+  it("falls back to OpenID with url + livekit_service_url transport", async () => {
+    mockJWTService();
+    client._unstable_getLivekitToken.mockRejectedValueOnce(unsupportedError);
     expect(
       await getSFUConfig({
         ...defaultParams,
@@ -157,7 +175,25 @@ describe("getSFUConfig", () => {
         livekitAlias: "via default endpoint",
       }),
     );
-    expect(client._unstable_getLivekitToken).not.toHaveBeenCalled();
+    expect(client._unstable_getLivekitToken).toHaveBeenCalled();
+    expect(jwtServiceCalls()).toEqual(["/get_token"]);
+  });
+
+  it("falls back to OpenID when the widget host refuses the CS API", async () => {
+    mockJWTService();
+    // In widget mode a host without MSC4533 rejects the request before it
+    // reaches the homeserver, with a plain error rather than a Matrix error
+    client._unstable_getLivekitToken.mockRejectedValueOnce(
+      new Error(
+        "The rtc_livekit_get_token action is not supported by the client.",
+      ),
+    );
+    expect(
+      await getSFUConfig({
+        ...defaultParams,
+        transport: urlAndServiceUrlTransport,
+      }),
+    ).toEqual(expect.objectContaining({ jwt: defaultJWTToken }));
     expect(jwtServiceCalls()).toEqual(["/get_token"]);
   });
 
@@ -186,7 +222,7 @@ describe("getSFUConfig", () => {
     expect(jwtServiceCalls()).toEqual([]);
   });
 
-  it("does not fall back to OpenID when publishing with url transport", async () => {
+  it("does not fall back to OpenID when publishing", async () => {
     mockJWTService();
     client._unstable_getLivekitToken.mockRejectedValueOnce(unsupportedError);
     await expect(
@@ -219,6 +255,33 @@ describe("getSFUConfig", () => {
     expect(client._unstable_getLivekitToken).not.toHaveBeenCalled();
     expect(jwtServiceCalls()).toEqual(["/sfu/get"]);
   });
+
+  it("does not fall back to OpenID on connection error", async () => {
+    vi.useFakeTimers();
+    try {
+      mockJWTService();
+      const client_ = {
+        ...client,
+        _unstable_getLivekitToken: vi
+          .fn()
+          .mockRejectedValue(new ConnectionError("No connection")),
+      };
+      await Promise.all([
+        expect(
+          getSFUConfig({
+            ...defaultParams,
+            client: client_,
+            transport: urlAndServiceUrlTransport,
+          }),
+        ).rejects.toThrow(expect.any(ConnectionError)),
+        vi.runAllTimersAsync(), // Allow request to be retried
+      ]);
+      expect(client_._unstable_getLivekitToken).toHaveBeenCalled();
+      expect(jwtServiceCalls()).toEqual([]);
+    } finally {
+      vi.useRealTimers();
+    }
+  });
 });
 
 describe("delegateDelayedLeave", () => {
@@ -299,21 +362,46 @@ describe("delegateDelayedLeave", () => {
 
 describe("chooseAuthFlow", () => {
   it.each([
-    ["url", urlTransport, { kind: "csApi", url: urlTransport.url }],
+    [
+      "url",
+      "publisher",
+      urlTransport,
+      { kind: "csApi", url: urlTransport.url },
+    ],
+    [
+      "url",
+      "subscriber",
+      urlTransport,
+      { kind: "csApi", url: urlTransport.url },
+    ],
     [
       "livekit_service_url",
+      "publisher",
+      serviceUrlTransport,
+      { kind: "openID", serviceUrl },
+    ],
+    [
+      "livekit_service_url",
+      "subscriber",
       serviceUrlTransport,
       { kind: "openID", serviceUrl },
     ],
     [
       "url + livekit_service_url",
+      "publisher",
       urlAndServiceUrlTransport,
       { kind: "openID", serviceUrl },
     ],
+    [
+      "url + livekit_service_url",
+      "subscriber",
+      urlAndServiceUrlTransport,
+      { kind: "csApi", url: urlTransport.url },
+    ],
   ] as const)(
     "chooses the flow for a %s transport as %s",
-    (_shape, transport, flow) => {
-      expect(chooseAuthFlow(transport)).toEqual(flow);
+    (_shape, role, transport, flow) => {
+      expect(chooseAuthFlow(transport, role)).toEqual(flow);
     },
   );
 });

sdk/src/media-backend/livekit/auth/index.ts

@@ -65,10 +65,7 @@ export interface GetSFUConfigParams {
   logger: Logger;
 }
 
-/**
- * Gets an {@link SFUConfig} appropriate for connecting to a given transport,
- * using whichever auth flow is appropriate.
- */
+/** Authenticates with a transport, through whichever flow suits it and our role. */
 export async function getSFUConfig({
   client,
   membership,
@@ -79,36 +76,48 @@ export async function getSFUConfig({
   role,
   logger,
 }: GetSFUConfigParams): Promise<SFUConfig> {
-  const flow = chooseAuthFlow(transport);
-  if (flow.kind === "openID") {
-    return await openID.getSFUConfig({
+  const viaOpenID = async (serviceUrl: string): Promise<SFUConfig> =>
+    await openID.getSFUConfig({
       client,
       membership,
-      serviceUrl: flow.serviceUrl,
+      serviceUrl,
       roomId,
       slotId,
       role,
       logger,
     });
-  } else {
-    try {
-      return await csApi.getSFUConfig({
-        client,
-        membership,
-        url: flow.url,
-        serverName,
-        roomId,
-        slotId,
-      });
-    } catch (e) {
-      if (isEndpointUnsupported(e)) {
-        logger.error(
-          `Homeserver does not support the MSC4195 get_token endpoint, cannot get token for transport ${flow.url}`,
-        );
-        throw new NoMatrix2AuthorizationService(e as Error);
-      }
-      throw e;
+
+  const flow = chooseAuthFlow(transport, role);
+  if (flow.kind === "openID") return await viaOpenID(flow.serviceUrl);
+
+  try {
+    return await csApi.getSFUConfig({
+      client,
+      membership,
+      url: flow.url,
+      serverName,
+      roomId,
+      slotId,
+    });
+  } catch (e) {
+    // A publisher never falls back: a homeserver that advertises a `url` but
+    // does not implement MSC4195 is misconfigured, and the admin should hear
+    // about it before `livekit_service_url` is deprecated
+    const mayFallBack =
+      role === "subscriber" && "livekit_service_url" in transport;
+    if (mayFallBack && isCsApiUnavailable(e)) {
+      logger.warn(
+        `Cannot use the MSC4195 get_token endpoint (${e}), falling back to the OpenID flow with service ${transport.livekit_service_url}`,
+      );
+      return await viaOpenID(transport.livekit_service_url);
     }
+    if (isEndpointUnsupported(e)) {
+      logger.error(
+        `Homeserver does not support the MSC4195 get_token endpoint, cannot get token for transport ${flow.url}`,
+      );
+      throw new NoMatrix2AuthorizationService(e as Error);
+    }
+    throw e;
   }
 }
 
@@ -140,7 +149,7 @@ export async function delegateDelayedLeave({
   delayTimeoutMs,
   logger,
 }: DelegateDelayedLeaveParams): Promise<void> {
-  const flow = chooseAuthFlow(transport);
+  const flow = chooseAuthFlow(transport, "publisher");
   if (flow.kind === "csApi") {
     try {
       await csApi.delegateDelayedLeave({
@@ -181,18 +190,23 @@ export type AuthFlow =
   | { kind: "openID"; serviceUrl: string };
 
 /**
- * Chooses which auth flow to use for a given transport. For now this returns a
- * single auth flow, with no provision for a fallback behavior.
+ * Chooses between the MSC4195 client-server API and the OpenID flow against
+ * the transport's JWT service.
+ *
+ * Only the JWT service's `/sfu/get` endpoint assigns the legacy participant
+ * identity that clients predating MSC4195 expect from anyone advertising a
+ * membership, so a publisher keeps using the JWT service wherever the
+ * transport still offers one, even next to a `url`. A subscriber may use the
+ * client-server API regardless, as nobody matches its identity.
  */
-export function chooseAuthFlow(transport: UnstableLivekitTransport): AuthFlow {
-  if ("url" in transport && !("livekit_service_url" in transport))
-    return { kind: "csApi", url: transport.url };
-  // All other transports use the OpenID flow, even if they contain a `url`.
-  // This is necessary since the auth flow determines the participant identity
-  // format, and we still have older clients out in the wild which would not yet
-  // know to look for a hashed participant identity in case `url` is present in
-  // someone else's transport.
-  // TODO: Once users have updated their clients, choose the CS API flow if
-  // available, keeping OpenID as a fallback.
+export function chooseAuthFlow(
+  transport: UnstableLivekitTransport,
+  role: ConnectionRole,
+): AuthFlow {
+  if ("url" in transport) {
+    const useCsApi =
+      !("livekit_service_url" in transport) || role === "subscriber";
+    if (useCsApi) return { kind: "csApi", url: transport.url };
+  }
   return { kind: "openID", serviceUrl: transport.livekit_service_url };
 }

src/e2ee/sharedKeyManagement.test.ts

@@ -5,11 +5,24 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
 Please see LICENSE in the repository root for full details.
 */
 
-import { afterEach, describe, expect, test } from "vitest";
+import { afterEach, describe, expect, test, vi } from "vitest";
+import { act, renderHook } from "@testing-library/react";
+import { E2eeType } from "@element-hq/matrixrtc-sdk";
 
-import { getKeyForRoom, saveKeyForRoom } from "./sharedKeyManagement";
+import {
+  getKeyForRoom,
+  saveKeyForRoom,
+  useRoomEncryptionSystem,
+} from "./sharedKeyManagement";
+
+vi.mock("../ClientContext", () => {
+  const room = { hasEncryptionStateEvent: (): boolean => false };
+  const client = { getRoom: (): typeof room => room };
+  return { useClient: (): { client: typeof client } => ({ client }) };
+});
 
 const roomId = "!room:example.org";
+const otherRoomId = "!other:example.org";
 
 describe("getKeyForRoom", () => {
   afterEach(() => {
@@ -33,4 +46,58 @@ describe("getKeyForRoom", () => {
   test("has no key to offer for a room it has never seen", () => {
     expect(getKeyForRoom(roomId)).toBeNull();
   });
+
+  test("prefers the stored key when the parameters are for another room", () => {
+    saveKeyForRoom(roomId, "stored");
+    window.location.hash = `#?roomId=${encodeURIComponent(otherRoomId)}&password=other-room`;
+
+    expect(getKeyForRoom(roomId)).toBe("stored");
+  });
+
+  test("ignores the password in the parameters for another room", () => {
+    window.location.hash = `#?roomId=${encodeURIComponent(otherRoomId)}&password=other-room`;
+
+    expect(getKeyForRoom(roomId)).toBeNull();
+  });
+});
+
+describe("useRoomEncryptionSystem", () => {
+  afterEach(() => {
+    window.location.hash = "#";
+    localStorage.clear();
+  });
+
+  test("picks up a key saved for the room after it rendered", () => {
+    const { result } = renderHook(() => useRoomEncryptionSystem(roomId));
+    expect(result.current).toEqual({ kind: E2eeType.NONE });
+
+    act(() => saveKeyForRoom(roomId, "stored"));
+
+    expect(result.current).toEqual({
+      kind: E2eeType.SHARED_KEY,
+      secret: "stored",
+    });
+  });
+
+  test("stores a key from the parameters under the room's key only", () => {
+    window.location.hash = `#?roomId=${encodeURIComponent(roomId)}&password=from-the-link`;
+
+    renderHook(() => useRoomEncryptionSystem(roomId));
+
+    expect(Object.keys(localStorage)).toEqual([`room-shared-key-${roomId}`]);
+    expect(getKeyForRoom(roomId)).toBe("from-the-link");
+  });
+
+  test("keeps the stored key when the parameters are for another room", () => {
+    saveKeyForRoom(roomId, "stored");
+    window.location.hash = `#?roomId=${encodeURIComponent(otherRoomId)}&password=other-room`;
+
+    const { result } = renderHook(() => useRoomEncryptionSystem(roomId));
+
+    expect(result.current).toEqual({
+      kind: E2eeType.SHARED_KEY,
+      secret: "stored",
+    });
+    expect(localStorage.getItem(`room-shared-key-${roomId}`)).toBe("stored");
+  });
 });

src/e2ee/sharedKeyManagement.ts

@@ -59,22 +59,27 @@ const useRoomSharedKey = (
 
 /**
  * The shared key for a room, preferring one supplied in the parameters Element
- * Call was started with over whatever is in local storage.
+ * Call was started with over whatever is in local storage. If the parameters
+ * are for another room, only the stored key is used, because the password in
+ * the parameters belongs to that other room.
  */
 function keyForRoom(
   roomId: string,
   paramsRoomId: string | null,
   password: string | null,
 ): string | null {
-  if (roomId !== paramsRoomId)
+  const storedKey = localStorage.getItem(
+    getRoomSharedKeyLocalStorageKey(roomId),
+  );
+  if (roomId !== paramsRoomId) {
     logger.warn(
       "requested key for a roomId which is not the current call room id (from the URL)",
       roomId,
       paramsRoomId,
     );
-  return (
-    password ?? localStorage.getItem(getRoomSharedKeyLocalStorageKey(roomId))
-  );
+    return storedKey;
+  }
+  return password ?? storedKey;
 }
 
 export function getKeyForRoom(roomId: string): string | null {
@@ -89,11 +94,7 @@ export function useRoomEncryptionSystem(roomId: string): EncryptionSystem {
   const { roomId: paramsRoomId, password } = useUrlParams();
 
   const [storedPassword] = useRoomSharedKey(
-    // TODO: this passes an already-prefixed key where a room ID is expected, so
-    // the local storage key ends up prefixed twice and never matches what
-    // saveKeyForRoom writes. Preserved as-is here to keep this commit a pure
-    // refactor; the reactive read is effectively dead until it is fixed.
-    getRoomSharedKeyLocalStorageKey(roomId),
+    roomId,
     keyForRoom(roomId, paramsRoomId, password) ?? undefined,
   );