Merge pull request #3828 from element-hq/valere/fix/stop_calling_msc4143_in_widget

Stop calling rtc/transport in widget mode
This commit is contained in:
Valere Fedronic
2026-03-31 18:47:20 +02:00
committed by GitHub
3 changed files with 77 additions and 8 deletions

View File

@@ -137,6 +137,7 @@ export async function getSFUConfigWithOpenID(
); );
logger?.info(`Got JWT from call's active focus URL.`); logger?.info(`Got JWT from call's active focus URL.`);
} catch (e) { } catch (e) {
logger?.debug(`Failed fetching jwt with matrix 2.0 endpoint:`, e);
if (e instanceof NotSupportedError) { if (e instanceof NotSupportedError) {
logger?.warn( logger?.warn(
`Failed fetching jwt with matrix 2.0 endpoint (retry with legacy) Not supported`, `Failed fetching jwt with matrix 2.0 endpoint (retry with legacy) Not supported`,

View File

@@ -60,6 +60,7 @@ describe("LocalTransport", () => {
client: { client: {
// eslint-disable-next-line @typescript-eslint/naming-convention // eslint-disable-next-line @typescript-eslint/naming-convention
_unstable_getRTCTransports: async () => Promise.resolve([]), _unstable_getRTCTransports: async () => Promise.resolve([]),
getAccessToken: vi.fn().mockReturnValue("access_token"),
getDomain: () => "", getDomain: () => "",
baseUrl: "example.org", baseUrl: "example.org",
// These won't be called in this error path but satisfy the type // These won't be called in this error path but satisfy the type
@@ -102,6 +103,7 @@ describe("LocalTransport", () => {
baseUrl: "https://lk.example.org", baseUrl: "https://lk.example.org",
// Use empty domain to skip .well-known and use config directly // Use empty domain to skip .well-known and use config directly
getDomain: () => "", getDomain: () => "",
getAccessToken: vi.fn().mockReturnValue("access_token"),
// eslint-disable-next-line @typescript-eslint/naming-convention // eslint-disable-next-line @typescript-eslint/naming-convention
_unstable_getRTCTransports: async () => Promise.resolve([]), _unstable_getRTCTransports: async () => Promise.resolve([]),
getOpenIdToken: vi.fn(), getOpenIdToken: vi.fn(),
@@ -149,6 +151,7 @@ describe("LocalTransport", () => {
getOpenIdToken: vi.fn(), getOpenIdToken: vi.fn(),
getDeviceId: vi.fn(), getDeviceId: vi.fn(),
baseUrl: "https://lk.example.org", baseUrl: "https://lk.example.org",
getAccessToken: vi.fn().mockReturnValue("access_token"),
}, },
ownMembershipIdentity: ownMemberMock, ownMembershipIdentity: ownMemberMock,
forceJwtEndpoint: JwtEndpointVersion.Legacy, forceJwtEndpoint: JwtEndpointVersion.Legacy,
@@ -217,6 +220,7 @@ describe("LocalTransport", () => {
getDomain: () => "", getDomain: () => "",
// eslint-disable-next-line @typescript-eslint/naming-convention // eslint-disable-next-line @typescript-eslint/naming-convention
_unstable_getRTCTransports: async () => Promise.resolve([]), _unstable_getRTCTransports: async () => Promise.resolve([]),
getAccessToken: vi.fn().mockReturnValue("access_token"),
getOpenIdToken: vi.fn(), getOpenIdToken: vi.fn(),
getDeviceId: vi.fn(), getDeviceId: vi.fn(),
baseUrl: "https://lk.example.org", baseUrl: "https://lk.example.org",
@@ -273,6 +277,7 @@ describe("LocalTransport", () => {
// eslint-disable-next-line @typescript-eslint/naming-convention // eslint-disable-next-line @typescript-eslint/naming-convention
_unstable_getRTCTransports: async () => _unstable_getRTCTransports: async () =>
Promise.resolve([aliceTransport]), Promise.resolve([aliceTransport]),
getAccessToken: vi.fn().mockReturnValue("access_token"),
getOpenIdToken: vi.fn(), getOpenIdToken: vi.fn(),
getDeviceId: vi.fn(), getDeviceId: vi.fn(),
baseUrl: "https://lk.example.org", baseUrl: "https://lk.example.org",
@@ -323,6 +328,7 @@ describe("LocalTransport", () => {
getDomain: vi.fn().mockReturnValue(""), getDomain: vi.fn().mockReturnValue(""),
// eslint-disable-next-line @typescript-eslint/naming-convention // eslint-disable-next-line @typescript-eslint/naming-convention
_unstable_getRTCTransports: vi.fn().mockResolvedValue([]), _unstable_getRTCTransports: vi.fn().mockResolvedValue([]),
getAccessToken: vi.fn().mockReturnValue("access_token"),
getOpenIdToken: vi.fn(), getOpenIdToken: vi.fn(),
getDeviceId: vi.fn(), getDeviceId: vi.fn(),
}, },
@@ -410,6 +416,42 @@ describe("LocalTransport", () => {
}); });
}); });
it("Should not call _unstable_getRTCTransports in widget mode but use well-known", async () => {
mockConfig({
livekit: { livekit_service_url: "https://do-not-use.lk.example.org" },
});
localTransportOpts.client.getDomain.mockReturnValue("example.org");
fetchMock.getOnce("https://example.org/.well-known/matrix/client", {
"org.matrix.msc4143.rtc_foci": [
{
type: "livekit",
livekit_service_url: "https://use-me.jwt.call.example.org",
},
],
});
localTransportOpts.client.getAccessToken.mockReturnValue(null);
const { advertised$, active$ } =
createLocalTransport$(localTransportOpts);
openIdResolver.resolve?.(openIdResponse);
expect(advertised$.value).toBe(null);
expect(active$.value).toBe(null);
await flushPromises();
expect(
localTransportOpts.client._unstable_getRTCTransports,
).not.toHaveBeenCalled();
const expectedTransport = {
type: "livekit",
livekit_service_url: "https://use-me.jwt.call.example.org",
};
expect(advertised$.value).toStrictEqual(expectedTransport);
});
it("fails fast if the openID request fails for backend config", async () => { it("fails fast if the openID request fails for backend config", async () => {
localTransportOpts.client._unstable_getRTCTransports.mockResolvedValue([ localTransportOpts.client._unstable_getRTCTransports.mockResolvedValue([
{ type: "livekit", livekit_service_url: "https://lk.example.org" }, { type: "livekit", livekit_service_url: "https://lk.example.org" },
@@ -481,6 +523,7 @@ describe("LocalTransport", () => {
baseUrl: "https://example.org", baseUrl: "https://example.org",
// eslint-disable-next-line @typescript-eslint/naming-convention // eslint-disable-next-line @typescript-eslint/naming-convention
_unstable_getRTCTransports: async () => Promise.resolve([]), _unstable_getRTCTransports: async () => Promise.resolve([]),
getAccessToken: vi.fn().mockReturnValue("access_token"),
// These won't be called in this error path but satisfy the type // These won't be called in this error path but satisfy the type
getOpenIdToken: vi.fn(), getOpenIdToken: vi.fn(),
getDeviceId: vi.fn(), getDeviceId: vi.fn(),

View File

@@ -56,7 +56,7 @@ interface Props {
memberships$: Behavior<Epoch<CallMembership[]>>; memberships$: Behavior<Epoch<CallMembership[]>>;
client: Pick< client: Pick<
MatrixClient, MatrixClient,
"getDomain" | "baseUrl" | "_unstable_getRTCTransports" "getDomain" | "baseUrl" | "_unstable_getRTCTransports" | "getAccessToken"
> & > &
OpenIDClientParts; OpenIDClientParts;
// Used by the jwt service to create the livekit room and compute the livekit alias. // Used by the jwt service to create the livekit room and compute the livekit alias.
@@ -314,7 +314,7 @@ const FOCI_WK_KEY = "org.matrix.msc4143.rtc_foci";
async function makeTransport( async function makeTransport(
client: Pick< client: Pick<
MatrixClient, MatrixClient,
"getDomain" | "baseUrl" | "_unstable_getRTCTransports" "getDomain" | "baseUrl" | "_unstable_getRTCTransports" | "getAccessToken"
> & > &
OpenIDClientParts, OpenIDClientParts,
membership: CallMembershipIdentityParts, membership: CallMembershipIdentityParts,
@@ -371,11 +371,18 @@ async function makeTransport(
for (const potentialTransport of transports) { for (const potentialTransport of transports) {
if (isLivekitTransportConfig(potentialTransport)) { if (isLivekitTransportConfig(potentialTransport)) {
try { try {
logger.info(
`makeTransport: check transport authentication for "${potentialTransport.livekit_service_url}"`,
);
// This will call the jwt/sfu/get endpoint to pre create the livekit room. // This will call the jwt/sfu/get endpoint to pre create the livekit room.
return await doOpenIdAndJWTFromUrl( return await doOpenIdAndJWTFromUrl(
potentialTransport.livekit_service_url, potentialTransport.livekit_service_url,
); );
} catch (ex) { } catch (ex) {
logger.debug(
`makeTransport: Could not use SFU service "${potentialTransport.livekit_service_url}" as SFU`,
ex,
);
// Explictly throw these // Explictly throw these
if (ex instanceof FailToGetOpenIdToken) { if (ex instanceof FailToGetOpenIdToken) {
throw ex; throw ex;
@@ -383,24 +390,34 @@ async function makeTransport(
if (ex instanceof NoMatrix2AuthorizationService) { if (ex instanceof NoMatrix2AuthorizationService) {
throw ex; throw ex;
} }
logger.debug(
`Could not use SFU service "${potentialTransport.livekit_service_url}" as SFU`,
ex,
);
} }
} else {
logger.info(
`makeTransport: "${potentialTransport.livekit_service_url}" is not a valid livekit transport as SFU`,
);
} }
} }
return null; return null;
} }
// MSC4143: Attempt to fetch transports from backend. // MSC4143: Attempt to fetch transports from backend.
if ("_unstable_getRTCTransports" in client) { // TODO: Workaround for an issue in the js-sdk RoomWidgetClient that
// is not yet implementing _unstable_getRTCTransports properly (via widget API new action).
// For now we just skip this call if we are in a widget.
// In widget mode the client is a `RoomWidgetClient` which has no access token (it is using the widget API).
// Could be removed once the js-sdk is fixed (https://github.com/matrix-org/matrix-js-sdk/issues/5245)
const isSPA = !!client.getAccessToken();
if (isSPA && "_unstable_getRTCTransports" in client) {
logger.info(
"makeTransport: First try to use getRTCTransports end point ...",
);
try { try {
// TODO This should also check for server support?
const transportList = await client._unstable_getRTCTransports(); const transportList = await client._unstable_getRTCTransports();
const selectedTransport = await getFirstUsableTransport(transportList); const selectedTransport = await getFirstUsableTransport(transportList);
if (selectedTransport) { if (selectedTransport) {
logger.info( logger.info(
"Using backend-configured (client.getRTCTransports) SFU", "makeTransport: ...Using backend-configured (client.getRTCTransports) SFU",
selectedTransport, selectedTransport,
); );
return selectedTransport; return selectedTransport;
@@ -424,6 +441,10 @@ async function makeTransport(
} }
} }
logger.info(
`makeTransport: Trying to get transports from .well-known/matrix/client on domain ${client.getDomain()} ...`,
);
// Legacy MSC4143 (to be removed) WELL_KNOWN: Prioritize the .well-known/matrix/client, if available. // Legacy MSC4143 (to be removed) WELL_KNOWN: Prioritize the .well-known/matrix/client, if available.
const domain = client.getDomain(); const domain = client.getDomain();
if (domain) { if (domain) {
@@ -441,6 +462,10 @@ async function makeTransport(
} }
} }
logger.info(
`makeTransport: No valid transport found via backend or .well-known, falling back to config if available.`,
);
// CONFIG: Least prioritized; Load from config file // CONFIG: Least prioritized; Load from config file
const urlFromConf = Config.get().livekit?.livekit_service_url; const urlFromConf = Config.get().livekit?.livekit_service_url;
if (urlFromConf) { if (urlFromConf) {