From bffda95251ea62e48f790534f1f866c6dde300e0 Mon Sep 17 00:00:00 2001 From: Jamie Brunton Date: Sat, 8 Aug 2026 14:30:40 +0100 Subject: [PATCH] Improve remote Matrix token refresh --- src/bootstrap.ts | 19 +++-- src/services/chat/index.ts | 5 ++ src/services/chat/matrix/token-extractor.ts | 36 ++++++++-- src/services/chat/matrix/token-refresher.ts | 69 ++++++++++++++++++ tests/matrix-token-extractor.test.ts | 33 +++++++++ tests/matrix-token-refresher.test.ts | 78 +++++++++++++++++++++ 6 files changed, 226 insertions(+), 14 deletions(-) create mode 100644 src/services/chat/matrix/token-refresher.ts create mode 100644 tests/matrix-token-extractor.test.ts create mode 100644 tests/matrix-token-refresher.test.ts diff --git a/src/bootstrap.ts b/src/bootstrap.ts index d318f5f..f17135a 100644 --- a/src/bootstrap.ts +++ b/src/bootstrap.ts @@ -22,9 +22,9 @@ import { ChatService, MatrixClient, MatrixMediaDownloader, + MatrixTokenRefresher, extractMatrixCredentials, loadCachedCredentials, - type TokenRefresher, type UsernameResolver, type MatrixCredentials, } from './services/chat/index.js'; @@ -187,16 +187,15 @@ export async function bootstrap(config: Config, logger: Logger): Promise chatService.setMatrixClient(new MatrixClient(fresh, logger)), + ); // 4. Username resolver — turns t2 IDs into real Reddit usernames so the // dashboard isn't full of `t2_abc123`. Uses Reddit's bulk endpoint. diff --git a/src/services/chat/index.ts b/src/services/chat/index.ts index 0e46cd7..1c6d97f 100644 --- a/src/services/chat/index.ts +++ b/src/services/chat/index.ts @@ -20,6 +20,11 @@ export type { MatrixCredentials } from './matrix/types.js'; export { extractMatrixCredentials, loadCachedCredentials, + MatrixBrowserLoginRequiredError, saveCredentials, } from './matrix/token-extractor.js'; +export { + MatrixRemoteLoginRequiredError, + MatrixTokenRefresher, +} from './matrix/token-refresher.js'; export { MatrixMediaDownloader } from './matrix/http-media-downloader.js'; diff --git a/src/services/chat/matrix/token-extractor.ts b/src/services/chat/matrix/token-extractor.ts index ac60ce4..e62e6f9 100644 --- a/src/services/chat/matrix/token-extractor.ts +++ b/src/services/chat/matrix/token-extractor.ts @@ -16,6 +16,16 @@ export interface TokenExtractorOptions { timeoutMs?: number; } +export class MatrixBrowserLoginRequiredError extends Error { + constructor(loginUrl: string) { + super( + `Reddit redirected Matrix token capture to the login page (${loginUrl}). ` + + 'Restore the browser session before retrying.', + ); + this.name = 'MatrixBrowserLoginRequiredError'; + } +} + export async function loadCachedCredentials( cachePath = DEFAULT_CACHE_PATH, ): Promise { @@ -101,6 +111,20 @@ export async function extractMatrixCredentials( }; page.on('request', onRequest); + page.on('framenavigated', (frame) => { + if (done || frame !== page.mainFrame()) return; + try { + const url = new URL(frame.url()); + const isRedditHost = url.hostname === 'reddit.com' || url.hostname === 'www.reddit.com'; + if (isRedditHost && url.pathname.startsWith('/login')) { + done = true; + logger.warn('[matrix] chat page redirected to Reddit login; token capture aborted'); + rejectCreds(new MatrixBrowserLoginRequiredError(url.toString())); + } + } catch { + // Ignore transient or malformed navigation URLs. + } + }); // Reddit now redirects `chat.reddit.com` → `www.reddit.com/chat/`. Going // direct to the canonical URL avoids one redirect hop and means the @@ -124,17 +148,19 @@ export async function extractMatrixCredentials( ); }); - const timeout = new Promise((_, reject) => - setTimeout( + let timeoutHandle: ReturnType | undefined; + const timeout = new Promise((_, reject) => { + timeoutHandle = setTimeout( () => reject(new Error(`Matrix token capture timed out after ${timeoutMs}ms`)), timeoutMs, - ), - ); + ); + }); let captured: MatrixCredentials; try { captured = await Promise.race([credsPromise, timeout]); } catch (err) { + if (err instanceof MatrixBrowserLoginRequiredError) throw err; // On timeout, surface enough info to tell the three failure modes apart: // (a) chat.reddit.com never loaded → page.url() still about:blank, almost no hosts // (b) page loaded, no Matrix host hit → reddit hosts present, no matrix.* @@ -156,6 +182,8 @@ export async function extractMatrixCredentials( logger.warn('[matrix] failed to collect timeout diagnostics:', diagErr); } throw err; + } finally { + if (timeoutHandle) clearTimeout(timeoutHandle); } // Resolve the user_id by calling /whoami so the caller knows who we are. diff --git a/src/services/chat/matrix/token-refresher.ts b/src/services/chat/matrix/token-refresher.ts new file mode 100644 index 0000000..e4c272e --- /dev/null +++ b/src/services/chat/matrix/token-refresher.ts @@ -0,0 +1,69 @@ +import type { BrowserContext } from 'playwright'; +import type { Logger } from '../../../utils/logger.js'; +import type { TokenRefresher } from '../service.js'; +import type { MatrixCredentials } from './types.js'; +import { extractMatrixCredentials } from './token-extractor.js'; + +export interface MatrixSessionBrowser { + getContext(): BrowserContext | null; + isSessionValid(): Promise; + login(): Promise; +} + +export type MatrixCredentialExtractor = ( + context: BrowserContext, + logger: Logger, +) => Promise; + +export class MatrixRemoteLoginRequiredError extends Error { + constructor() { + super( + 'Reddit browser session has expired and automatic login could not restore it. ' + + 'Open the dashboard and use the noVNC console to log in to Reddit, then retry chat sync.', + ); + this.name = 'MatrixRemoteLoginRequiredError'; + } +} + +/** + * Restores the browser login when needed, then captures and applies one fresh + * Matrix credential set. Concurrent callers share the same browser operation. + */ +export class MatrixTokenRefresher implements TokenRefresher { + private refreshInFlight: Promise | null = null; + + constructor( + private readonly browser: MatrixSessionBrowser, + private readonly logger: Logger, + private readonly applyCredentials: (credentials: MatrixCredentials) => void, + private readonly extract: MatrixCredentialExtractor = extractMatrixCredentials, + ) {} + + refresh(): Promise { + if (this.refreshInFlight) return this.refreshInFlight; + + this.refreshInFlight = this.performRefresh().finally(() => { + this.refreshInFlight = null; + }); + return this.refreshInFlight; + } + + private async performRefresh(): Promise { + const sessionValid = await this.browser.isSessionValid(); + if (!sessionValid) { + this.logger.warn('[matrix] Reddit browser session expired; attempting login recovery'); + if (!(await this.browser.login())) { + throw new MatrixRemoteLoginRequiredError(); + } + this.logger.info('[matrix] Reddit browser session restored'); + } + + const context = this.browser.getContext(); + if (!context) { + throw new Error('Matrix token refresh requires an initialized browser context'); + } + + const credentials = await this.extract(context, this.logger); + this.applyCredentials(credentials); + } +} diff --git a/tests/matrix-token-extractor.test.ts b/tests/matrix-token-extractor.test.ts new file mode 100644 index 0000000..01aaadb --- /dev/null +++ b/tests/matrix-token-extractor.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it, vi } from 'vitest'; +import type { BrowserContext, Frame } from 'playwright'; +import { createMockLogger } from './mockLogger.js'; +import { + extractMatrixCredentials, + MatrixBrowserLoginRequiredError, +} from '../src/services/chat/matrix/token-extractor.js'; + +describe('extractMatrixCredentials', () => { + it('fails as soon as Reddit redirects the chat page to login', async () => { + const handlers = new Map void>(); + const frame = { url: () => 'https://www.reddit.com/login/?dest=https://www.reddit.com/chat/' } as Frame; + const page = { + on: vi.fn((event: string, handler: (value: never) => void) => { + handlers.set(event, handler); + }), + mainFrame: vi.fn(() => frame), + goto: vi.fn(async () => { + handlers.get('framenavigated')?.(frame as never); + return null; + }), + url: vi.fn(() => frame.url()), + screenshot: vi.fn(async () => undefined), + close: vi.fn(async () => undefined), + }; + const context = { newPage: vi.fn(async () => page) } as unknown as BrowserContext; + + await expect( + extractMatrixCredentials(context, createMockLogger(), { timeoutMs: 5_000 }), + ).rejects.toBeInstanceOf(MatrixBrowserLoginRequiredError); + expect(page.screenshot).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/matrix-token-refresher.test.ts b/tests/matrix-token-refresher.test.ts new file mode 100644 index 0000000..91426f7 --- /dev/null +++ b/tests/matrix-token-refresher.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it, vi } from 'vitest'; +import type { BrowserContext } from 'playwright'; +import { createMockLogger } from './mockLogger.js'; +import { + MatrixRemoteLoginRequiredError, + MatrixTokenRefresher, +} from '../src/services/chat/matrix/token-refresher.js'; +import type { MatrixCredentials } from '../src/services/chat/matrix/types.js'; + +const credentials: MatrixCredentials = { + homeserver: 'https://matrix.redditspace.com', + accessToken: 'fresh-token', + userId: '@t2_bot:reddit.com', + capturedAt: 123, +}; + +function setup(options: { sessionValid?: boolean; loginSucceeds?: boolean } = {}) { + const context = {} as BrowserContext; + const browser = { + getContext: vi.fn(() => context), + isSessionValid: vi.fn(async () => options.sessionValid ?? true), + login: vi.fn(async () => options.loginSucceeds ?? true), + }; + const extract = vi.fn(async () => credentials); + const applyCredentials = vi.fn(); + const logger = createMockLogger(); + const refresher = new MatrixTokenRefresher( + browser, + logger, + applyCredentials, + extract, + ); + + return { refresher, browser, context, extract, applyCredentials, logger }; +} + +describe('MatrixTokenRefresher', () => { + it('repairs an expired Reddit browser session before capturing a Matrix token', async () => { + const { refresher, browser, context, extract, applyCredentials } = setup({ + sessionValid: false, + loginSucceeds: true, + }); + + await refresher.refresh(); + + expect(browser.login).toHaveBeenCalledOnce(); + expect(extract).toHaveBeenCalledWith(context, expect.anything()); + expect(applyCredentials).toHaveBeenCalledWith(credentials); + }); + + it('fails fast with remote-login guidance when the Reddit session cannot be restored', async () => { + const { refresher, browser, extract } = setup({ + sessionValid: false, + loginSucceeds: false, + }); + + await expect(refresher.refresh()).rejects.toBeInstanceOf(MatrixRemoteLoginRequiredError); + await expect(refresher.refresh()).rejects.toThrow(/dashboard.*noVNC/i); + expect(browser.login).toHaveBeenCalledTimes(2); + expect(extract).not.toHaveBeenCalled(); + }); + + it('coalesces concurrent refreshes so only one browser capture runs', async () => { + const { refresher, extract, applyCredentials } = setup(); + let release: ((value: MatrixCredentials) => void) | undefined; + extract.mockImplementation( + () => new Promise((resolve) => { release = resolve; }), + ); + + const first = refresher.refresh(); + const second = refresher.refresh(); + await vi.waitFor(() => expect(extract).toHaveBeenCalledOnce()); + release?.(credentials); + await Promise.all([first, second]); + + expect(applyCredentials).toHaveBeenCalledOnce(); + }); +});