From 22c0ba474d2bffe0242bea67cc5b7367feb73f04 Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Thu, 23 Jul 2026 08:32:41 +0200 Subject: [PATCH 1/3] fix: guard getAppPathFromHref against Tauri synthetic origin mismatch --- src/app/pages/pathUtils.test.ts | 32 +++++++++++++++++++++++++++++++- src/app/pages/pathUtils.ts | 13 +++++++++++-- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/app/pages/pathUtils.test.ts b/src/app/pages/pathUtils.test.ts index 141ee990e5..3928d7750f 100644 --- a/src/app/pages/pathUtils.test.ts +++ b/src/app/pages/pathUtils.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { getSettingsPath } from './pathUtils'; +import { getAppPathFromHref, getSettingsPath } from './pathUtils'; describe('getSettingsPath', () => { it('returns the settings root path', () => { @@ -13,3 +13,33 @@ describe('getSettingsPath', () => { ); }); }); + +describe('getAppPathFromHref', () => { + it('extracts the app path for a matching browser-router origin', () => { + expect(getAppPathFromHref('https://app.sable.moe/', 'https://app.sable.moe/')).toBe('/'); + expect(getAppPathFromHref('https://app.sable.moe/', 'https://app.sable.moe/login')).toBe( + '/login' + ); + expect( + getAppPathFromHref('https://app.sable.moe/', 'https://app.sable.moe/home/room/%21abc') + ).toBe('/home/room/%21abc'); + }); + + it('extracts the app path for a matching hash-router origin', () => { + expect(getAppPathFromHref('https://app.sable.moe/#/', 'https://app.sable.moe/#/')).toBe('/'); + expect( + getAppPathFromHref('https://app.sable.moe/#/', 'https://app.sable.moe/#/login?code=c&state=s') + ).toBe('/login?code=c&state=s'); + }); + + it('returns empty when the href origin does not match the base', () => { + expect(getAppPathFromHref('https://app.sable.moe/', 'https://tauri.localhost/')).toBe(''); + expect( + getAppPathFromHref('https://app.sable.moe/', 'https://tauri.localhost/login?code=c&state=s') + ).toBe(''); + }); + + it('returns empty when a hash-router base is paired with a hashless href', () => { + expect(getAppPathFromHref('https://app.sable.moe/#/', 'https://tauri.localhost/')).toBe(''); + }); +}); diff --git a/src/app/pages/pathUtils.ts b/src/app/pages/pathUtils.ts index f3f9aef212..37cfc0606c 100644 --- a/src/app/pages/pathUtils.ts +++ b/src/app/pages/pathUtils.ts @@ -64,16 +64,25 @@ export const getAppPathFromHref = (baseUrl: string, href: string): string => { const baseHashIndex = baseUrl.indexOf('#'); if (baseHashIndex > -1) { const hrefHashIndex = href.indexOf('#'); + if (hrefHashIndex === -1) return ''; + // href may/not have "/" around "#" // we need to take care of this when extracting app path const trimmedBaseUrl = trimLeadingSlash(baseUrl.slice(baseHashIndex + 1)); const trimmedHref = trimLeadingSlash(href.slice(hrefHashIndex + 1)); - const appPath = trimmedHref.slice(trimmedBaseUrl.length); + // On Tauri the base origin is synthetic and never matches the href origin; guard + // against slicing a basename the href does not start with. + const appPath = trimmedHref.startsWith(trimmedBaseUrl) + ? trimmedHref.slice(trimmedBaseUrl.length) + : ''; return `/${trimLeadingSlash(appPath)}`; } - return href.slice(trimTrailingSlash(baseUrl).length); + // On Tauri the base origin is synthetic and never matches the href origin; return + // empty instead of slicing a garbage path that would be stored as a redirect. + const trimmedBaseUrl = trimTrailingSlash(baseUrl); + return href.startsWith(trimmedBaseUrl) ? href.slice(trimmedBaseUrl.length) : ''; }; export const getRootPath = (): string => ROOT_PATH; From bc1db07f425b285bdabe66f07898e6859fdee54b Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Thu, 23 Jul 2026 09:02:38 +0200 Subject: [PATCH 2/3] test: assert OIDC token exchange uses the registered redirect URI --- .../pages/auth/login/oidcLoginUtil.test.ts | 87 ++++++++++++++++++- 1 file changed, 84 insertions(+), 3 deletions(-) diff --git a/src/app/pages/auth/login/oidcLoginUtil.test.ts b/src/app/pages/auth/login/oidcLoginUtil.test.ts index aa5d0001ee..0170c213e6 100644 --- a/src/app/pages/auth/login/oidcLoginUtil.test.ts +++ b/src/app/pages/auth/login/oidcLoginUtil.test.ts @@ -1,6 +1,11 @@ -import { describe, it, expect } from 'vitest'; -import type { BearerTokenResponse } from '$types/matrix-sdk'; -import { deviceIdFromScope, expiresInMsFromToken } from './oidcLoginUtil'; +import { beforeEach, describe, it, expect, vi } from 'vitest'; +import type { BearerTokenResponse, ValidatedAuthMetadata } from '$types/matrix-sdk'; +import { + deviceIdFromScope, + expiresInMsFromToken, + completeOidcLogin, + persistOauthContext, +} from './oidcLoginUtil'; describe('deviceIdFromScope', () => { it('extracts the device id from the stable device scope', () => { @@ -35,3 +40,79 @@ describe('expiresInMsFromToken', () => { ).toBeUndefined(); }); }); + +const mocks = vi.hoisted(() => ({ + oauth2Context: vi.fn(), + completeAuthorizationCodeGrant: vi.fn<(code: string) => Promise>(), + getAuthMetadata: vi.fn<() => Promise>(), + whoami: vi.fn(), + setAccessToken: vi.fn(), +})); + +vi.mock('$types/matrix-sdk', async (importOriginal) => { + const actual = await importOriginal>(); + return { + ...actual, + OAuth2: class { + public constructor(_metadata: unknown, context: unknown) { + mocks.oauth2Context(context); + } + public completeAuthorizationCodeGrant = mocks.completeAuthorizationCodeGrant; + public context = { deviceId: 'dev', codeVerifier: 'verifier' }; + }, + createClient: vi.fn().mockImplementation(() => ({ + getAuthMetadata: mocks.getAuthMetadata, + setAccessToken: mocks.setAccessToken, + whoami: mocks.whoami, + })), + }; +}); + +const metadata = { + issuer: 'https://issuer.example', + authorization_endpoint: 'https://issuer.example/authorize', + token_endpoint: 'https://issuer.example/token', + revocation_endpoint: 'https://issuer.example/revoke', + registration_endpoint: 'https://issuer.example/register', + response_modes_supported: ['query', 'fragment'], + response_types_supported: ['code'], + grant_types_supported: ['authorization_code', 'refresh_token'], + code_challenge_methods_supported: ['S256'], +} as ValidatedAuthMetadata; + +const tokenResponse = { + access_token: 'access-tok', + token_type: 'Bearer', + refresh_token: 'refresh-tok', + expires_in: 300, +} as BearerTokenResponse; + +describe('completeOidcLogin', () => { + beforeEach(() => { + sessionStorage.clear(); + mocks.oauth2Context.mockReset(); + mocks.completeAuthorizationCodeGrant.mockReset().mockResolvedValue(tokenResponse); + mocks.getAuthMetadata.mockReset().mockResolvedValue(metadata); + mocks.whoami + .mockReset() + .mockResolvedValue({ user_id: '@alice:hs.example', device_id: 'DEV123' }); + mocks.setAccessToken.mockReset(); + }); + + it('uses the registered redirect URI from the OAuth context, not a re-parsed callback URL', async () => { + persistOauthContext('s1', { + issuer: 'https://issuer.example', + clientId: 'client', + redirectUri: 'moe.sable.app:/login', + deviceId: 'dev', + codeVerifier: 'verifier', + homeserverUrl: 'https://hs.example', + }); + + await completeOidcLogin('code-1', 's1'); + + expect(mocks.oauth2Context).toHaveBeenCalledWith( + expect.objectContaining({ redirectUri: 'moe.sable.app:/login' }) + ); + }); +}); From a9fb56e7d0ca0676ecba1dc71aa9c665d4373c7f Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Thu, 23 Jul 2026 09:11:45 +0200 Subject: [PATCH 3/3] fix: strip credentials from token-refresher tempClient to prevent logout cascade --- src/app/pages/auth/login/oidcLoginUtil.test.ts | 18 ++++++++++-------- .../pages/client/BackgroundNotifications.tsx | 4 ---- src/app/pages/pathUtils.test.ts | 6 +++--- src/app/pages/pathUtils.ts | 8 +++----- src/client/initMatrix.ts | 4 ---- 5 files changed, 16 insertions(+), 24 deletions(-) diff --git a/src/app/pages/auth/login/oidcLoginUtil.test.ts b/src/app/pages/auth/login/oidcLoginUtil.test.ts index 0170c213e6..4ca4a2aa1d 100644 --- a/src/app/pages/auth/login/oidcLoginUtil.test.ts +++ b/src/app/pages/auth/login/oidcLoginUtil.test.ts @@ -42,11 +42,11 @@ describe('expiresInMsFromToken', () => { }); const mocks = vi.hoisted(() => ({ - oauth2Context: vi.fn(), + oauth2Context: vi.fn<(context: unknown) => void>(), completeAuthorizationCodeGrant: vi.fn<(code: string) => Promise>(), getAuthMetadata: vi.fn<() => Promise>(), - whoami: vi.fn(), - setAccessToken: vi.fn(), + whoami: vi.fn<() => Promise<{ user_id: string; device_id?: string }>>(), + setAccessToken: vi.fn<(token: string) => void>(), })); vi.mock('$types/matrix-sdk', async (importOriginal) => { @@ -60,11 +60,13 @@ vi.mock('$types/matrix-sdk', async (importOriginal) => { public completeAuthorizationCodeGrant = mocks.completeAuthorizationCodeGrant; public context = { deviceId: 'dev', codeVerifier: 'verifier' }; }, - createClient: vi.fn().mockImplementation(() => ({ - getAuthMetadata: mocks.getAuthMetadata, - setAccessToken: mocks.setAccessToken, - whoami: mocks.whoami, - })), + createClient: vi + .fn<(opts: { baseUrl: string; fetchFn: typeof fetch }) => Record>() + .mockImplementation(() => ({ + getAuthMetadata: mocks.getAuthMetadata, + setAccessToken: mocks.setAccessToken, + whoami: mocks.whoami, + })), }; }); diff --git a/src/app/pages/client/BackgroundNotifications.tsx b/src/app/pages/client/BackgroundNotifications.tsx index be8cdc92c4..293d4e78de 100644 --- a/src/app/pages/client/BackgroundNotifications.tsx +++ b/src/app/pages/client/BackgroundNotifications.tsx @@ -86,10 +86,6 @@ const startBackgroundClient = async (session: Session): Promise => const tempClient = createClient({ baseUrl: session.baseUrl, fetchFn: fetch, - accessToken: session.accessToken, - refreshToken: session.refreshToken, - userId: session.userId, - deviceId: session.deviceId, }); const tokenRefresher = createSessionTokenRefresher(session, tempClient); diff --git a/src/app/pages/pathUtils.test.ts b/src/app/pages/pathUtils.test.ts index 3928d7750f..d72b192770 100644 --- a/src/app/pages/pathUtils.test.ts +++ b/src/app/pages/pathUtils.test.ts @@ -32,11 +32,11 @@ describe('getAppPathFromHref', () => { ).toBe('/login?code=c&state=s'); }); - it('returns empty when the href origin does not match the base', () => { - expect(getAppPathFromHref('https://app.sable.moe/', 'https://tauri.localhost/')).toBe(''); + it('extracts the path from the href when the origin does not match the base (Tauri)', () => { + expect(getAppPathFromHref('https://app.sable.moe/', 'https://tauri.localhost/')).toBe('/'); expect( getAppPathFromHref('https://app.sable.moe/', 'https://tauri.localhost/login?code=c&state=s') - ).toBe(''); + ).toBe('/login?code=c&state=s'); }); it('returns empty when a hash-router base is paired with a hashless href', () => { diff --git a/src/app/pages/pathUtils.ts b/src/app/pages/pathUtils.ts index 37cfc0606c..18981c5722 100644 --- a/src/app/pages/pathUtils.ts +++ b/src/app/pages/pathUtils.ts @@ -71,18 +71,16 @@ export const getAppPathFromHref = (baseUrl: string, href: string): string => { const trimmedBaseUrl = trimLeadingSlash(baseUrl.slice(baseHashIndex + 1)); const trimmedHref = trimLeadingSlash(href.slice(hrefHashIndex + 1)); - // On Tauri the base origin is synthetic and never matches the href origin; guard - // against slicing a basename the href does not start with. const appPath = trimmedHref.startsWith(trimmedBaseUrl) ? trimmedHref.slice(trimmedBaseUrl.length) : ''; return `/${trimLeadingSlash(appPath)}`; } - // On Tauri the base origin is synthetic and never matches the href origin; return - // empty instead of slicing a garbage path that would be stored as a redirect. const trimmedBaseUrl = trimTrailingSlash(baseUrl); - return href.startsWith(trimmedBaseUrl) ? href.slice(trimmedBaseUrl.length) : ''; + if (href.startsWith(trimmedBaseUrl)) return href.slice(trimmedBaseUrl.length); + const { pathname, search } = new URL(href); + return pathname + search; }; export const getRootPath = (): string => ROOT_PATH; diff --git a/src/client/initMatrix.ts b/src/client/initMatrix.ts index f1edf6f3f6..b463b31394 100644 --- a/src/client/initMatrix.ts +++ b/src/client/initMatrix.ts @@ -260,10 +260,6 @@ const buildClient = async (session: Session): Promise => { const tempClient = createClient({ baseUrl: session.baseUrl, fetchFn: fetch, - accessToken: session.accessToken, - refreshToken: session.refreshToken, - userId: session.userId, - deviceId: session.deviceId, }); const tokenRefresher = createSessionTokenRefresher(session, tempClient);