diff --git a/src/app/pages/auth/login/oidcLoginUtil.test.ts b/src/app/pages/auth/login/oidcLoginUtil.test.ts index aa5d0001e..4ca4a2aa1 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,81 @@ describe('expiresInMsFromToken', () => { ).toBeUndefined(); }); }); + +const mocks = vi.hoisted(() => ({ + oauth2Context: vi.fn<(context: unknown) => void>(), + completeAuthorizationCodeGrant: vi.fn<(code: string) => Promise>(), + getAuthMetadata: vi.fn<() => Promise>(), + whoami: vi.fn<() => Promise<{ user_id: string; device_id?: string }>>(), + setAccessToken: vi.fn<(token: string) => void>(), +})); + +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<(opts: { baseUrl: string; fetchFn: typeof fetch }) => Record>() + .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' }) + ); + }); +}); diff --git a/src/app/pages/client/BackgroundNotifications.tsx b/src/app/pages/client/BackgroundNotifications.tsx index be8cdc92c..293d4e78d 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 141ee990e..d72b19277 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('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('/login?code=c&state=s'); + }); + + 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 f3f9aef21..18981c572 100644 --- a/src/app/pages/pathUtils.ts +++ b/src/app/pages/pathUtils.ts @@ -64,16 +64,23 @@ 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); + const appPath = trimmedHref.startsWith(trimmedBaseUrl) + ? trimmedHref.slice(trimmedBaseUrl.length) + : ''; return `/${trimLeadingSlash(appPath)}`; } - return href.slice(trimTrailingSlash(baseUrl).length); + const trimmedBaseUrl = trimTrailingSlash(baseUrl); + 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 f1edf6f3f..b463b3139 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);