Skip to content

Commit 7c66892

Browse files
committed
fix(media): recover authenticated video sessions
1 parent 6d1dd90 commit 7c66892

15 files changed

Lines changed: 421 additions & 74 deletions
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
default: patch
3+
---
4+
5+
Improve authenticated media loading across web and Tauri, especially for videos on slow or unstable connections.

src/app/pages/client/ClientRoot.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ export function ClientRoot({ children }: ClientRootProps) {
272272
log.log('initClient for', activeSession.userId);
273273
const newMx = await initClient(activeSession);
274274
loadedUserIdRef.current = activeSession.userId;
275-
pushSessionToSW(activeSession.baseUrl, activeSession.accessToken);
275+
await pushSessionToSW(activeSession.baseUrl, activeSession.accessToken);
276276
return newMx;
277277
}, [activeSession, activeSessionId, setActiveSessionId])
278278
);
@@ -311,7 +311,7 @@ export function ClientRoot({ children }: ClientRootProps) {
311311
activeSession.userId,
312312
'— reloading client'
313313
);
314-
pushSessionToSW(activeSession.baseUrl, activeSession.accessToken);
314+
void pushSessionToSW(activeSession.baseUrl, activeSession.accessToken);
315315
if (mx?.clientRunning) {
316316
stopClient(mx);
317317
}

src/app/utils/matrix.test.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
import type { MatrixClient } from '$types/matrix-sdk';
23

34
const tauriApi = vi.hoisted(() => ({
45
isTauri: vi.fn<() => boolean>(),
@@ -15,7 +16,7 @@ const mediaTransport = vi.hoisted(() => ({
1516
vi.mock('@tauri-apps/api/core', () => tauriApi);
1617
vi.mock('./mediaTransport', () => mediaTransport);
1718

18-
const { rewriteAuthenticatedMediaUrl } = await import('./matrix');
19+
const { mxcUrlToHttp, rewriteAuthenticatedMediaUrl } = await import('./matrix');
1920

2021
describe('rewriteAuthenticatedMediaUrl', () => {
2122
beforeEach(() => {
@@ -59,6 +60,36 @@ describe('rewriteAuthenticatedMediaUrl', () => {
5960
);
6061
});
6162

63+
it.each([
64+
'/_matrix/media/v3/download/example.org/abc123',
65+
'/_matrix/media/v3/thumbnail/example.org/abc123?width=96&height=96&method=crop',
66+
'/_matrix/media/r0/download/example.org/abc123',
67+
'/_matrix/media/r0/thumbnail/example.org/abc123?width=96&height=96&method=crop',
68+
])('rewrites legacy media paths under Tauri: %s', (path) => {
69+
tauriApi.isTauri.mockReturnValue(true);
70+
const url = `https://matrix.example.org${path}`;
71+
const separator = url.includes('?') ? '&' : '?';
72+
expect(rewriteAuthenticatedMediaUrl(url)).toBe(
73+
`sable-media://${url}${separator}__sable_media_cache=2&__sable_media_session=%40user%3Aexample.com`
74+
);
75+
});
76+
77+
it('does not rewrite unrelated Matrix media URLs', () => {
78+
tauriApi.isTauri.mockReturnValue(true);
79+
const url = 'https://matrix.example.org/_matrix/media/v3/config';
80+
expect(rewriteAuthenticatedMediaUrl(url)).toBe(url);
81+
expect(tauriApi.convertFileSrc).not.toHaveBeenCalled();
82+
});
83+
84+
it.each([
85+
'https://example.org/avatar.png?next=/_matrix/media/v3/download/example.org/abc123',
86+
'https://example.org/avatar.png#/_matrix/media/r0/thumbnail/example.org/abc123',
87+
])('does not rewrite a media path only present in query or hash: %s', (url) => {
88+
tauriApi.isTauri.mockReturnValue(true);
89+
expect(rewriteAuthenticatedMediaUrl(url)).toBe(url);
90+
expect(tauriApi.convertFileSrc).not.toHaveBeenCalled();
91+
});
92+
6293
it('passes through already-rewritten sable-media:// URLs', () => {
6394
tauriApi.isTauri.mockReturnValue(true);
6495
const url =
@@ -69,3 +100,17 @@ describe('rewriteAuthenticatedMediaUrl', () => {
69100
expect(tauriApi.convertFileSrc).not.toHaveBeenCalled();
70101
});
71102
});
103+
104+
describe('mxcUrlToHttp', () => {
105+
it('rewrites SDK legacy media URLs under Tauri without useAuthentication', () => {
106+
tauriApi.isTauri.mockReturnValue(true);
107+
const legacyUrl = 'https://matrix.example.org/_matrix/media/v3/download/example.org/video';
108+
const mx = {
109+
mxcUrlToHttp: vi.fn<() => string>(() => legacyUrl),
110+
} as unknown as MatrixClient;
111+
112+
expect(mxcUrlToHttp(mx, 'mxc://example.org/video', false)).toBe(
113+
`sable-media://${legacyUrl}?__sable_media_cache=2&__sable_media_session=%40user%3Aexample.com`
114+
);
115+
});
116+
});

src/app/utils/matrix.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ import {
3636

3737
const DOMAIN_REGEX = /\b(?:[a-zA-Z0-9-]+\.)+[a-zA-Z]{2,}\b/;
3838
const TAURI_MEDIA_CACHE_VERSION = '__sable_media_cache=2';
39+
const TAURI_MEDIA_PATH_PREFIXES = [
40+
'/_matrix/client/v1/media/',
41+
'/_matrix/media/v3/download/',
42+
'/_matrix/media/v3/thumbnail/',
43+
'/_matrix/media/r0/download/',
44+
'/_matrix/media/r0/thumbnail/',
45+
];
3946

4047
export const isServerName = (serverName: string): boolean => DOMAIN_REGEX.test(serverName);
4148

@@ -478,7 +485,22 @@ export const removeRoomIdFromMDirect = async (mx: MatrixClient, roomId: string):
478485
export const rewriteAuthenticatedMediaUrl = (httpUrl: string | null): string | null => {
479486
if (!httpUrl) return null;
480487
if (!isTauri()) return httpUrl;
481-
if (!httpUrl.includes('/_matrix/client/v1/media/')) return httpUrl;
488+
const sourceUrl = httpUrl.startsWith('sable-media://')
489+
? httpUrl.slice('sable-media://'.length)
490+
: httpUrl;
491+
let parsedUrl: URL;
492+
try {
493+
parsedUrl = new URL(sourceUrl);
494+
} catch {
495+
return httpUrl;
496+
}
497+
if (
498+
(parsedUrl.protocol !== 'http:' && parsedUrl.protocol !== 'https:') ||
499+
parsedUrl.origin === 'null' ||
500+
!TAURI_MEDIA_PATH_PREFIXES.some((path) => parsedUrl.pathname.startsWith(path))
501+
) {
502+
return httpUrl;
503+
}
482504
if (httpUrl.includes(TAURI_MEDIA_CACHE_VERSION)) return httpUrl;
483505
const mediaUrl = httpUrl.startsWith('sable-media://')
484506
? httpUrl
@@ -508,9 +530,7 @@ export const mxcUrlToHttp = (
508530
useAuthentication
509531
);
510532

511-
// Authenticated media has no service worker under Tauri to attach the token, so route it
512-
// through the native sable-media:// protocol which injects the token in Rust.
513-
if (httpUrl && useAuthentication) {
533+
if (httpUrl && isTauri()) {
514534
return rewriteAuthenticatedMediaUrl(httpUrl);
515535
}
516536
return httpUrl;

src/app/utils/mediaTransport.test.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ describe('fetchMediaBlob', () => {
356356
expect(headersSeen).toEqual([null]);
357357
});
358358

359-
it('retries once on the service worker path without direct auth headers', async () => {
359+
it('fetches once on the service worker path when it returns an auth error', async () => {
360360
platform.hasControllingServiceWorker.mockReturnValue(true);
361361
const { fetchMediaBlob } = await import('./mediaTransport');
362362
const url = 'https://example.org/auth-media.png';
@@ -365,17 +365,13 @@ describe('fetchMediaBlob', () => {
365365
vi.mocked(fetch).mockImplementation(async (_input, init) => {
366366
const headers = new Headers(init?.headers);
367367
headersSeen.push(headers.get('authorization'));
368-
if (headersSeen.length === 1) {
369-
return new Response('denied', { status: 403 });
370-
}
371-
return new Response('ok', { status: 200 });
368+
return new Response('denied', { status: 403 });
372369
});
373370

374-
const blob = await fetchMediaBlob(url);
371+
await expect(fetchMediaBlob(url)).rejects.toThrow('Failed to fetch media: 403');
375372

376-
expect(await blob.text()).toBe('ok');
377-
expect(headersSeen).toEqual([null, null]);
378-
expect(fetch).toHaveBeenCalledTimes(2);
373+
expect(headersSeen).toEqual([null]);
374+
expect(fetch).toHaveBeenCalledTimes(1);
379375
});
380376

381377
it('bypasses the service worker path when explicit auth overrides are provided', async () => {

src/app/utils/mediaTransport.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -296,10 +296,6 @@ async function fetchMediaBlobInternal(url: string, options?: MediaTransportOptio
296296
};
297297

298298
if (useServiceWorker) {
299-
const response = await fetchMediaResponse(url, undefined, cacheMode);
300-
if (response.ok || !isRetryableAuthError(response)) {
301-
return fetchAndCache(response);
302-
}
303299
return fetchAndCache(await fetchMediaResponse(url, undefined, cacheMode));
304300
}
305301

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
3+
const tauriApi = vi.hoisted(() => ({
4+
isTauri: vi.fn<() => boolean>(),
5+
}));
6+
const commands = vi.hoisted(() => ({
7+
clearMediaSession: vi.fn<() => Promise<void>>(),
8+
setMediaSession:
9+
vi.fn<({ baseUrl, token }: { baseUrl: string; token: string }) => Promise<void>>(),
10+
}));
11+
const mediaTransport = vi.hoisted(() => ({
12+
getActiveMediaSession: vi.fn<() => { baseUrl: string; accessToken: string } | undefined>(),
13+
}));
14+
15+
vi.mock('@tauri-apps/api/core', () => tauriApi);
16+
vi.mock('$generated/tauri/commands', () => commands);
17+
vi.mock('./mediaTransport', () => mediaTransport);
18+
19+
const { initTauriMediaSession, updateTauriMediaSession } = await import('./tauriMediaAuth');
20+
21+
describe('Tauri media session coordinator', () => {
22+
beforeEach(() => {
23+
vi.clearAllMocks();
24+
tauriApi.isTauri.mockReturnValue(true);
25+
commands.clearMediaSession.mockResolvedValue();
26+
commands.setMediaSession.mockResolvedValue();
27+
});
28+
29+
it('serializes writes and applies the last requested state', async () => {
30+
let resolveFirst: (() => void) | undefined;
31+
commands.setMediaSession
32+
.mockImplementationOnce(
33+
() =>
34+
new Promise<void>((resolve) => {
35+
resolveFirst = resolve;
36+
})
37+
)
38+
.mockResolvedValueOnce();
39+
40+
const first = updateTauriMediaSession('https://one.example', 'one');
41+
const second = updateTauriMediaSession('https://two.example', 'two');
42+
const clear = updateTauriMediaSession();
43+
44+
await Promise.resolve();
45+
expect(commands.setMediaSession).toHaveBeenCalledTimes(1);
46+
resolveFirst?.();
47+
await Promise.all([first, second, clear]);
48+
49+
expect(commands.setMediaSession).toHaveBeenNthCalledWith(1, {
50+
baseUrl: 'https://one.example',
51+
token: 'one',
52+
});
53+
expect(commands.setMediaSession).toHaveBeenNthCalledWith(2, {
54+
baseUrl: 'https://two.example',
55+
token: 'two',
56+
});
57+
expect(commands.clearMediaSession).toHaveBeenCalledTimes(1);
58+
});
59+
60+
it('waits for the initial active session write', async () => {
61+
let resolveWrite: (() => void) | undefined;
62+
mediaTransport.getActiveMediaSession.mockReturnValue({
63+
baseUrl: 'https://matrix.example',
64+
accessToken: 'token',
65+
});
66+
commands.setMediaSession.mockImplementation(
67+
() =>
68+
new Promise<void>((resolve) => {
69+
resolveWrite = resolve;
70+
})
71+
);
72+
73+
const ready = initTauriMediaSession();
74+
let complete = false;
75+
void ready.then(() => {
76+
complete = true;
77+
});
78+
await Promise.resolve();
79+
expect(complete).toBe(false);
80+
81+
resolveWrite?.();
82+
await ready;
83+
expect(complete).toBe(true);
84+
});
85+
});

src/app/utils/tauriMediaAuth.ts

500 Bytes
Binary file not shown.

src/client/initMatrix.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { beforeEach, describe, expect, it } from 'vitest';
2+
import type { Session } from '$state/sessions';
3+
import { ACTIVE_SESSION_KEY, MATRIX_SESSIONS_KEY } from '$state/sessions';
4+
import { ownsActiveMediaSession } from './initMatrix';
5+
6+
const alice = { userId: '@alice:example.org' } as Session;
7+
const bob = { userId: '@bob:example.org' } as Session;
8+
9+
describe('ownsActiveMediaSession', () => {
10+
beforeEach(() => {
11+
localStorage.clear();
12+
localStorage.setItem(MATRIX_SESSIONS_KEY, JSON.stringify([alice, bob]));
13+
});
14+
15+
it('keeps Alice media session while logging out secondary Bob', () => {
16+
localStorage.setItem(ACTIVE_SESSION_KEY, JSON.stringify(alice.userId));
17+
18+
expect(ownsActiveMediaSession(bob)).toBe(false);
19+
});
20+
21+
it('clears the active account media session', () => {
22+
localStorage.setItem(ACTIVE_SESSION_KEY, JSON.stringify(alice.userId));
23+
24+
expect(ownsActiveMediaSession(alice)).toBe(true);
25+
});
26+
});

src/client/initMatrix.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,14 @@ import { fetch } from '$utils/fetch';
1616
import { clearMediaCache } from '$utils/mediaCache';
1717

1818
import { clearNavToActivePathStore } from '$state/navToActivePath';
19-
import type { Session, SessionStoreName } from '$state/sessions';
20-
import { getSessionStoreName, getStoredSessionRefreshToken } from '$state/sessions';
19+
import type { Session, Sessions, SessionStoreName } from '$state/sessions';
20+
import {
21+
ACTIVE_SESSION_KEY,
22+
getSessionStoreName,
23+
getStoredSessionRefreshToken,
24+
MATRIX_SESSIONS_KEY,
25+
} from '$state/sessions';
26+
import { getLocalStorageItem } from '$state/utils/atomWithLocalStorage';
2127
import { createLogger } from '$utils/debug';
2228
import { createDebugLogger } from '$utils/debugLogger';
2329
import * as Sentry from '@sentry/react';
@@ -43,6 +49,14 @@ const debugLog = createDebugLogger('initMatrix');
4349
const slidingSyncByClient = new WeakMap<MatrixClient, SlidingSyncManager>();
4450
const membershipActionCleanupByClient = new WeakMap<MatrixClient, () => void>();
4551
const presenceSyncByClient = new WeakMap<MatrixClient, PresenceSyncManager>();
52+
53+
export const ownsActiveMediaSession = (session?: Session): boolean => {
54+
if (!session) return true;
55+
const sessions = getLocalStorageItem<Sessions>(MATRIX_SESSIONS_KEY, []);
56+
const activeSessionId = getLocalStorageItem<string | undefined>(ACTIVE_SESSION_KEY, undefined);
57+
const activeSession = sessions.find((item) => item.userId === activeSessionId) ?? sessions[0];
58+
return activeSession?.userId === session.userId;
59+
};
4660
const presenceStartCleanupByClient = new WeakMap<MatrixClient, () => void>();
4761
const SLIDING_SYNC_POLL_TIMEOUT_MS = 45000;
4862

@@ -584,7 +598,6 @@ export const logoutClient = async (mx: MatrixClient, session?: Session) => {
584598
sessionUserId: session?.userId,
585599
});
586600
debugLog.info('general', 'Logging out client', { userId: mx.getUserId() });
587-
pushSessionToSW();
588601
stopClient(mx);
589602
try {
590603
if (session?.oidc) {
@@ -611,7 +624,14 @@ export const logoutClient = async (mx: MatrixClient, session?: Session) => {
611624
window.localStorage.clear();
612625
}
613626

614-
await clearMediaCache();
627+
try {
628+
await clearMediaCache();
629+
} finally {
630+
if (ownsActiveMediaSession(session)) {
631+
// Queue the final clear after any in-flight refresh.
632+
await pushSessionToSW();
633+
}
634+
}
615635
};
616636

617637
export const clearLoginData = async () => {

0 commit comments

Comments
 (0)