diff --git a/playwright/url-transport.spec.ts b/playwright/url-transport.spec.ts index f07730fae..aa194f8a6 100644 --- a/playwright/url-transport.spec.ts +++ b/playwright/url-transport.spec.ts @@ -79,6 +79,12 @@ 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) { @@ -130,47 +136,6 @@ 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["url+legacy"]); - 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(["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, diff --git a/playwright/widget/url-transport.test.ts b/playwright/widget/url-transport.test.ts index a840b43d4..22ce6db4d 100644 --- a/playwright/widget/url-transport.test.ts +++ b/playwright/widget/url-transport.test.ts @@ -11,13 +11,12 @@ 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 + livekit_service_url transport and a legacy one", + "Call between a url transport and a legacy one", async ({ addUser, browserName }) => { test.skip( browserName === "firefox", @@ -30,12 +29,10 @@ widgetTest( addUser("Pevara", HOST1), ]); - // The host's homeserver has started advertising a `url` next to its JWT - // service; the guest's still advertises the JWT service alone. + // Host advertises a url transport, guest still advertises a legacy one 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 @@ -67,7 +64,7 @@ widgetTest( } // Element Web used for Playwright does support MSC4195 for widgets. - expect(hostFlows).toEqual(new Set(["service-legacy", "service-default"])); + expect(hostFlows).toEqual(new Set(["cs-api", "service-default"])); expect(guestFlows).toEqual(new Set(["service-legacy", "cs-api"])); }, ); diff --git a/sdk/src/media-backend/livekit/LivekitBackend.ts b/sdk/src/media-backend/livekit/LivekitBackend.ts index f2a04720d..e17f50730 100644 --- a/sdk/src/media-backend/livekit/LivekitBackend.ts +++ b/sdk/src/media-backend/livekit/LivekitBackend.ts @@ -166,9 +166,8 @@ export function createLivekitBackend( ); canDelegate = Promise.resolve(false); } else { - // Delegation goes through whichever service authenticates us as a - // publisher, so only that service's support counts - const flow = chooseAuthFlow(transport, "publisher"); + // Check for support using the same auth flow that we intend to use later + const flow = chooseAuthFlow(transport); canDelegate = flow.kind === "openID" ? supportsDelegation( diff --git a/sdk/src/media-backend/livekit/auth/index.test.ts b/sdk/src/media-backend/livekit/auth/index.test.ts index bce375312..733d5e10d 100644 --- a/sdk/src/media-backend/livekit/auth/index.test.ts +++ b/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 { ConnectionError, MatrixError } from "matrix-js-sdk"; +import { 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,26 +144,8 @@ describe("getSFUConfig", () => { expect(jwtServiceCalls()).toEqual(["/get_token"]); }); - it("uses CS API when subscribing with url + livekit_service_url transport", async () => { + it("uses OpenID 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, @@ -175,25 +157,7 @@ describe("getSFUConfig", () => { livekitAlias: "via default endpoint", }), ); - 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(client._unstable_getLivekitToken).not.toHaveBeenCalled(); expect(jwtServiceCalls()).toEqual(["/get_token"]); }); @@ -222,7 +186,7 @@ describe("getSFUConfig", () => { expect(jwtServiceCalls()).toEqual([]); }); - it("does not fall back to OpenID when publishing", async () => { + it("does not fall back to OpenID when publishing with url transport", async () => { mockJWTService(); client._unstable_getLivekitToken.mockRejectedValueOnce(unsupportedError); await expect( @@ -255,33 +219,6 @@ 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", () => { @@ -362,46 +299,21 @@ describe("delegateDelayedLeave", () => { describe("chooseAuthFlow", () => { it.each([ - [ - "url", - "publisher", - urlTransport, - { kind: "csApi", url: urlTransport.url }, - ], - [ - "url", - "subscriber", - urlTransport, - { kind: "csApi", url: urlTransport.url }, - ], + ["url", 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, role, transport, flow) => { - expect(chooseAuthFlow(transport, role)).toEqual(flow); + (_shape, transport, flow) => { + expect(chooseAuthFlow(transport)).toEqual(flow); }, ); }); diff --git a/sdk/src/media-backend/livekit/auth/index.ts b/sdk/src/media-backend/livekit/auth/index.ts index 409917fae..50c180127 100644 --- a/sdk/src/media-backend/livekit/auth/index.ts +++ b/sdk/src/media-backend/livekit/auth/index.ts @@ -65,7 +65,10 @@ export interface GetSFUConfigParams { logger: Logger; } -/** Authenticates with a transport, through whichever flow suits it and our role. */ +/** + * Gets an {@link SFUConfig} appropriate for connecting to a given transport, + * using whichever auth flow is appropriate. + */ export async function getSFUConfig({ client, membership, @@ -76,48 +79,36 @@ export async function getSFUConfig({ role, logger, }: GetSFUConfigParams): Promise { - const viaOpenID = async (serviceUrl: string): Promise => - await openID.getSFUConfig({ + const flow = chooseAuthFlow(transport); + if (flow.kind === "openID") { + return await openID.getSFUConfig({ client, membership, - serviceUrl, + serviceUrl: flow.serviceUrl, roomId, slotId, role, logger, }); - - 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); + } 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; } - 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; } } @@ -149,7 +140,7 @@ export async function delegateDelayedLeave({ delayTimeoutMs, logger, }: DelegateDelayedLeaveParams): Promise { - const flow = chooseAuthFlow(transport, "publisher"); + const flow = chooseAuthFlow(transport); if (flow.kind === "csApi") { try { await csApi.delegateDelayedLeave({ @@ -190,23 +181,18 @@ export type AuthFlow = | { kind: "openID"; serviceUrl: string }; /** - * 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. + * 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. */ -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 }; - } +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. return { kind: "openID", serviceUrl: transport.livekit_service_url }; }