diff --git a/extensions/general/cloud-backup/lib/google-oauth.ts b/extensions/general/cloud-backup/lib/google-oauth.ts index 9eeefd85..004c137c 100644 --- a/extensions/general/cloud-backup/lib/google-oauth.ts +++ b/extensions/general/cloud-backup/lib/google-oauth.ts @@ -7,6 +7,12 @@ * re-issued even if the user has previously authorised the app. */ +import { + fetchWithTimeout, + OAUTH_TIMEOUT_MS, + OAUTH_REVOKE_TIMEOUT_MS, +} from '@/lib/http/fetch-with-timeout' + const DRIVE_SCOPE = 'https://www.googleapis.com/auth/drive.file' const AUTH_ENDPOINT = 'https://accounts.google.com/o/oauth2/v2/auth' const TOKEN_ENDPOINT = 'https://oauth2.googleapis.com/token' @@ -65,11 +71,15 @@ export async function exchangeCodeForTokens( redirect_uri: env.redirectUri, grant_type: 'authorization_code', }) - const res = await fetch(TOKEN_ENDPOINT, { - method: 'POST', - headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, - body: body.toString(), - }) + const res = await fetchWithTimeout( + TOKEN_ENDPOINT, + { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: body.toString(), + }, + { timeoutMs: OAUTH_TIMEOUT_MS, description: 'Google token exchange' }, + ) if (!res.ok) { const errText = await res.text() throw new Error(`Google token exchange failed: ${res.status} ${errText}`) @@ -99,11 +109,15 @@ export async function refreshAccessToken( refresh_token: refreshToken, grant_type: 'refresh_token', }) - const res = await fetch(TOKEN_ENDPOINT, { - method: 'POST', - headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, - body: body.toString(), - }) + const res = await fetchWithTimeout( + TOKEN_ENDPOINT, + { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: body.toString(), + }, + { timeoutMs: OAUTH_TIMEOUT_MS, description: 'Google token refresh' }, + ) if (!res.ok) { const errText = await res.text() throw new Error(`Google token refresh failed: ${res.status} ${errText}`) @@ -112,16 +126,28 @@ export async function refreshAccessToken( } export async function revokeToken(token: string): Promise { - await fetch(`https://oauth2.googleapis.com/revoke?token=${encodeURIComponent(token)}`, { - method: 'POST', - headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, - }) + try { + await fetchWithTimeout( + `https://oauth2.googleapis.com/revoke?token=${encodeURIComponent(token)}`, + { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + }, + { timeoutMs: OAUTH_REVOKE_TIMEOUT_MS, description: 'Google token revoke' }, + ) + } catch { + // Best-effort revoke: swallow timeouts and network errors so disconnect flows still complete locally. + } } export async function fetchUserEmail(accessToken: string): Promise { - const res = await fetch(USERINFO_ENDPOINT, { - headers: { Authorization: `Bearer ${accessToken}` }, - }) + const res = await fetchWithTimeout( + USERINFO_ENDPOINT, + { + headers: { Authorization: `Bearer ${accessToken}` }, + }, + { timeoutMs: OAUTH_TIMEOUT_MS, description: 'Google userinfo fetch' }, + ) if (!res.ok) { throw new Error(`Failed to fetch Google user info: ${res.status}`) } diff --git a/extensions/general/skatteverket/index.ts b/extensions/general/skatteverket/index.ts index d43e3049..51da6311 100644 --- a/extensions/general/skatteverket/index.ts +++ b/extensions/general/skatteverket/index.ts @@ -1,6 +1,7 @@ import crypto from 'crypto' import type { Extension, ExtensionContext } from '@/lib/extensions/types' import { NextResponse } from 'next/server' +import { TimeoutError } from '@/lib/http/fetch-with-timeout' import { buildAuthorizeUrl, exchangeCodeForTokens } from './lib/oauth' import { storeTokens, getTokens, deleteTokens } from './lib/token-store' import { skvRequest, SkatteverketAuthError } from './lib/api-client' @@ -162,10 +163,15 @@ export const skatteverketExtension: Extension = { ) } catch (err) { console.error('[skatteverket] Token exchange failed:', err) + // BankID auth codes expire after 5 minutes. Surface timeouts distinctly + // so the user retries quickly instead of exhausting the code window. + const message = err instanceof TimeoutError + ? 'Tidsgränsen mot Skatteverket överskreds — försök igen med BankID' + : err instanceof Error + ? err.message + : 'Token exchange misslyckades' return NextResponse.redirect( - `${appUrl}/reports?tab=vat-declaration&skv_error=${encodeURIComponent( - err instanceof Error ? err.message : 'Token exchange misslyckades' - )}` + `${appUrl}/reports?tab=vat-declaration&skv_error=${encodeURIComponent(message)}` ) } }, diff --git a/extensions/general/skatteverket/lib/oauth.ts b/extensions/general/skatteverket/lib/oauth.ts index 1ed0b838..6b3fa394 100644 --- a/extensions/general/skatteverket/lib/oauth.ts +++ b/extensions/general/skatteverket/lib/oauth.ts @@ -1,4 +1,9 @@ import type { SkatteverketTokens } from '../types' +import { + fetchWithTimeout, + OAUTH_TIMEOUT_MS, + SKATTEVERKET_EXCHANGE_TIMEOUT_MS, +} from '@/lib/http/fetch-with-timeout' /** * Skatteverket OAuth2 helpers for the `per` (BankID) flow. @@ -68,11 +73,18 @@ export async function exchangeCodeForTokens( code, }) - const response = await fetch(`${base}/token`, { - method: 'POST', - headers: { 'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8' }, - body: body.toString(), - }) + const response = await fetchWithTimeout( + `${base}/token`, + { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8' }, + body: body.toString(), + }, + { + timeoutMs: SKATTEVERKET_EXCHANGE_TIMEOUT_MS, + description: 'Skatteverket token exchange', + }, + ) if (!response.ok) { const text = await response.text() @@ -110,11 +122,18 @@ export async function refreshAccessToken( refresh_token: refreshToken, }) - const response = await fetch(`${base}/token`, { - method: 'POST', - headers: { 'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8' }, - body: body.toString(), - }) + const response = await fetchWithTimeout( + `${base}/token`, + { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8' }, + body: body.toString(), + }, + { + timeoutMs: OAUTH_TIMEOUT_MS, + description: 'Skatteverket token refresh', + }, + ) if (!response.ok) { const text = await response.text() diff --git a/lib/errors/get-error-message.ts b/lib/errors/get-error-message.ts index 81d4948f..3d335160 100644 --- a/lib/errors/get-error-message.ts +++ b/lib/errors/get-error-message.ts @@ -86,6 +86,10 @@ const ERROR_PATTERN_MAP: [RegExp, string | null][] = [ /Entry date .+ is outside fiscal period/i, 'Datumet ligger utanför det valda räkenskapsåret.', ], + [ + /timed out after \d+m?s/i, + 'Anslutningen mot tjänsten tog för lång tid. Försök igen.', + ], ] /** diff --git a/lib/http/__tests__/fetch-with-timeout.test.ts b/lib/http/__tests__/fetch-with-timeout.test.ts new file mode 100644 index 00000000..971197b1 --- /dev/null +++ b/lib/http/__tests__/fetch-with-timeout.test.ts @@ -0,0 +1,124 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { fetchWithTimeout, TimeoutError } from '../fetch-with-timeout' + +describe('fetchWithTimeout', () => { + const originalFetch = globalThis.fetch + + beforeEach(() => { + vi.resetAllMocks() + }) + + afterEach(() => { + globalThis.fetch = originalFetch + }) + + it('returns the response when fetch resolves before the deadline', async () => { + const mockResponse = new Response('ok', { status: 200 }) + globalThis.fetch = vi.fn().mockResolvedValue(mockResponse) + + const result = await fetchWithTimeout( + 'https://example.test/', + { method: 'POST' }, + { timeoutMs: 1000, description: 'test fetch' }, + ) + + expect(result).toBe(mockResponse) + expect(globalThis.fetch).toHaveBeenCalledOnce() + }) + + it('throws TimeoutError when fetch hangs past the timeout', async () => { + globalThis.fetch = vi.fn().mockImplementation((_url, init?: RequestInit) => { + return new Promise((_resolve, reject) => { + const signal = init?.signal + if (!signal) return + signal.addEventListener('abort', () => { + const reason = (signal as AbortSignal & { reason?: unknown }).reason + const err = new Error('aborted') + err.name = + reason instanceof DOMException ? reason.name : 'AbortError' + reject(err) + }) + }) + }) + + const start = Date.now() + await expect( + fetchWithTimeout( + 'https://example.test/', + { method: 'POST' }, + { timeoutMs: 50, description: 'slow fetch' }, + ), + ).rejects.toBeInstanceOf(TimeoutError) + const elapsed = Date.now() - start + expect(elapsed).toBeGreaterThanOrEqual(40) + expect(elapsed).toBeLessThan(1000) + }) + + it('includes the description and timeout ms in the error message', async () => { + globalThis.fetch = vi.fn().mockImplementation((_url, init?: RequestInit) => { + return new Promise((_resolve, reject) => { + const signal = init?.signal + signal?.addEventListener('abort', () => { + const err = new Error('aborted') + err.name = 'TimeoutError' + reject(err) + }) + }) + }) + + try { + await fetchWithTimeout( + 'https://example.test/', + { method: 'POST' }, + { timeoutMs: 50, description: 'Fortnox token exchange' }, + ) + expect.fail('expected TimeoutError to be thrown') + } catch (err) { + expect(err).toBeInstanceOf(TimeoutError) + expect((err as Error).name).toBe('TimeoutError') + expect((err as Error).message).toContain('Fortnox token exchange') + expect((err as Error).message).toContain('50') + } + }) + + it('propagates non-timeout errors unchanged', async () => { + const networkError = new TypeError('fetch failed') + globalThis.fetch = vi.fn().mockRejectedValue(networkError) + + await expect( + fetchWithTimeout( + 'https://example.test/', + { method: 'POST' }, + { timeoutMs: 1000, description: 'test fetch' }, + ), + ).rejects.toBe(networkError) + }) + + it('honours an external AbortSignal without labelling it a timeout', async () => { + const externalController = new AbortController() + globalThis.fetch = vi.fn().mockImplementation((_url, init?: RequestInit) => { + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + const err = new Error('aborted by caller') + err.name = 'AbortError' + reject(err) + }) + }) + }) + + const promise = fetchWithTimeout( + 'https://example.test/', + { method: 'POST', signal: externalController.signal }, + { timeoutMs: 10_000, description: 'caller-cancelable fetch' }, + ) + + externalController.abort() + + await expect(promise).rejects.toSatisfy( + (err: unknown) => + err instanceof Error && + !(err instanceof TimeoutError) && + err.name === 'AbortError', + ) + }) +}) diff --git a/lib/http/fetch-with-timeout.ts b/lib/http/fetch-with-timeout.ts new file mode 100644 index 00000000..861a8e5a --- /dev/null +++ b/lib/http/fetch-with-timeout.ts @@ -0,0 +1,54 @@ +/** + * Thin `fetch` wrapper that aborts after a deadline. + * + * Used by OAuth token endpoints where a hung provider would otherwise hold + * the request thread indefinitely. The Skatteverket callback handler also + * relies on `TimeoutError` to distinguish a hung token exchange (where the + * 5-minute BankID auth code may expire mid-call) from other failures. + */ + +export class TimeoutError extends Error { + readonly name = 'TimeoutError' +} + +export function isTimeoutError(error: unknown): boolean { + return ( + error instanceof Error && + (error.name === 'TimeoutError' || error.name === 'AbortError') + ) +} + +export const OAUTH_TIMEOUT_MS = 10_000 +export const OAUTH_REVOKE_TIMEOUT_MS = 5_000 +export const SKATTEVERKET_EXCHANGE_TIMEOUT_MS = 8_000 + +interface FetchWithTimeoutOptions { + timeoutMs: number + description: string +} + +export async function fetchWithTimeout( + input: RequestInfo | URL, + init: RequestInit, + options: FetchWithTimeoutOptions, +): Promise { + const { timeoutMs, description } = options + + const timeoutSignal = AbortSignal.timeout(timeoutMs) + const signal = init.signal + ? AbortSignal.any([init.signal, timeoutSignal]) + : timeoutSignal + + try { + return await fetch(input, { ...init, signal }) + } catch (err) { + if ( + timeoutSignal.aborted && + err instanceof Error && + (err.name === 'TimeoutError' || err.name === 'AbortError') + ) { + throw new TimeoutError(`${description} timed out after ${timeoutMs}ms`) + } + throw err + } +} diff --git a/lib/providers/bjornlunden/client.ts b/lib/providers/bjornlunden/client.ts index fc901ade..c62c2f8f 100644 --- a/lib/providers/bjornlunden/client.ts +++ b/lib/providers/bjornlunden/client.ts @@ -1,6 +1,9 @@ import { TokenBucketRateLimiter } from '../rate-limiter'; import { withRetry } from '../retry'; import { BL_BASE_URL, BL_RATE_LIMIT } from './config'; +import { isTimeoutError } from '@/lib/http/fetch-with-timeout'; + +const FETCH_TIMEOUT_MS = 15_000; export class BjornLundenApiError extends Error { constructor( @@ -14,6 +17,7 @@ export class BjornLundenApiError extends Error { } function isRetryableError(error: unknown): boolean { + if (isTimeoutError(error)) return true; if (error instanceof BjornLundenApiError) { if (error.statusCode === 401 || error.statusCode === 403 || error.statusCode === 404) { return false; @@ -50,6 +54,7 @@ export class BjornLundenClient { 'User-Key': userKey, Accept: 'application/json', }, + signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), }); if (!response.ok) { diff --git a/lib/providers/bjornlunden/oauth.ts b/lib/providers/bjornlunden/oauth.ts index 4f615a53..4974f936 100644 --- a/lib/providers/bjornlunden/oauth.ts +++ b/lib/providers/bjornlunden/oauth.ts @@ -1,4 +1,8 @@ import type { TokenResponse } from '../types'; +import { + fetchWithTimeout, + OAUTH_TIMEOUT_MS, +} from '@/lib/http/fetch-with-timeout'; const BL_AUTH_URL = 'https://apigateway.blinfo.se/auth/oauth/v2/token'; @@ -6,17 +10,21 @@ export async function fetchBjornLundenToken( clientId: string, clientSecret: string, ): Promise { - const response = await fetch(BL_AUTH_URL, { - method: 'POST', - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', + const response = await fetchWithTimeout( + BL_AUTH_URL, + { + method: 'POST', + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + }, + body: new URLSearchParams({ + grant_type: 'client_credentials', + client_id: clientId, + client_secret: clientSecret, + }).toString(), }, - body: new URLSearchParams({ - grant_type: 'client_credentials', - client_id: clientId, - client_secret: clientSecret, - }).toString(), - }); + { timeoutMs: OAUTH_TIMEOUT_MS, description: 'Björn Lundén token request' }, + ); if (!response.ok) { const body = await response.text().catch(() => ''); diff --git a/lib/providers/bokio/client.ts b/lib/providers/bokio/client.ts index 5dd53a32..609dd603 100644 --- a/lib/providers/bokio/client.ts +++ b/lib/providers/bokio/client.ts @@ -2,9 +2,12 @@ import { TokenBucketRateLimiter } from '../rate-limiter'; import { withRetry } from '../retry'; import { BOKIO_BASE_URL, BOKIO_RATE_LIMIT } from './config'; import { createLogger } from '@/lib/logger'; +import { isTimeoutError } from '@/lib/http/fetch-with-timeout'; const log = createLogger('bokio-client'); +const FETCH_TIMEOUT_MS = 15_000; + export class BokioApiError extends Error { constructor( message: string, @@ -17,6 +20,7 @@ export class BokioApiError extends Error { } function isRetryableError(error: unknown): boolean { + if (isTimeoutError(error)) return true; if (error instanceof BokioApiError) { if (error.statusCode === 401 || error.statusCode === 403 || error.statusCode === 404) { return false; @@ -53,6 +57,7 @@ export class BokioClient { Authorization: `Bearer ${accessToken}`, Accept: 'application/json', }, + signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), }); if (!response.ok) { diff --git a/lib/providers/briox/client.ts b/lib/providers/briox/client.ts index e6ce890f..0af04432 100644 --- a/lib/providers/briox/client.ts +++ b/lib/providers/briox/client.ts @@ -1,6 +1,9 @@ import { TokenBucketRateLimiter } from '../rate-limiter'; import { withRetry } from '../retry'; import { BRIOX_BASE_URL, BRIOX_RATE_LIMIT } from './config'; +import { isTimeoutError } from '@/lib/http/fetch-with-timeout'; + +const FETCH_TIMEOUT_MS = 15_000; export class BrioxApiError extends Error { constructor( @@ -14,6 +17,7 @@ export class BrioxApiError extends Error { } function isRetryableError(error: unknown): boolean { + if (isTimeoutError(error)) return true; if (error instanceof BrioxApiError) { if (error.statusCode === 401 || error.statusCode === 403 || error.statusCode === 404) { return false; @@ -53,6 +57,7 @@ export class BrioxClient { Accept: 'application/json', 'Content-Type': 'application/json', }, + signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), }); if (!response.ok) { diff --git a/lib/providers/briox/oauth.ts b/lib/providers/briox/oauth.ts index 2081abf5..dbd2ce65 100644 --- a/lib/providers/briox/oauth.ts +++ b/lib/providers/briox/oauth.ts @@ -1,5 +1,9 @@ import { BRIOX_TOKEN_URL, BRIOX_REFRESH_URL } from './config'; import type { TokenResponse } from '../types'; +import { + fetchWithTimeout, + OAUTH_TIMEOUT_MS, +} from '@/lib/http/fetch-with-timeout'; interface BrioxTokenData { access_token: string; @@ -29,12 +33,16 @@ export async function exchangeBrioxCode( ): Promise { const url = `${BRIOX_TOKEN_URL}?clientid=${encodeURIComponent(clientId)}&token=${encodeURIComponent(applicationToken)}`; - const response = await fetch(url, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', + const response = await fetchWithTimeout( + url, + { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, }, - }); + { timeoutMs: OAUTH_TIMEOUT_MS, description: 'Briox token exchange' }, + ); if (!response.ok) { const body = await response.text().catch(() => ''); @@ -51,12 +59,16 @@ export async function refreshBrioxToken( ): Promise { const url = `${BRIOX_REFRESH_URL}?refreshtoken=${encodeURIComponent(refreshToken)}&token=${encodeURIComponent(refreshToken)}`; - const response = await fetch(url, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', + const response = await fetchWithTimeout( + url, + { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, }, - }); + { timeoutMs: OAUTH_TIMEOUT_MS, description: 'Briox token refresh' }, + ); if (!response.ok) { const body = await response.text().catch(() => ''); diff --git a/lib/providers/fortnox/client.ts b/lib/providers/fortnox/client.ts index 9e2109b6..853a4aaf 100644 --- a/lib/providers/fortnox/client.ts +++ b/lib/providers/fortnox/client.ts @@ -1,6 +1,9 @@ import { TokenBucketRateLimiter } from '../rate-limiter'; import { withRetry } from '../retry'; import { FORTNOX_BASE_URL, FORTNOX_RATE_LIMIT } from './config'; +import { isTimeoutError } from '@/lib/http/fetch-with-timeout'; + +const FETCH_TIMEOUT_MS = 15_000; export class FortnoxApiError extends Error { constructor( @@ -15,6 +18,7 @@ export class FortnoxApiError extends Error { } function isRetryableError(error: unknown): boolean { + if (isTimeoutError(error)) return true; if (error instanceof FortnoxApiError) { if (error.statusCode === 401 || error.statusCode === 403 || error.statusCode === 404) { return false; @@ -44,6 +48,7 @@ export class FortnoxClient { Accept: 'application/json', 'Content-Type': 'application/json', }, + signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), }); if (!response.ok) { @@ -87,6 +92,7 @@ export class FortnoxClient { headers: { Authorization: `Bearer ${accessToken}`, }, + signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), }); if (!response.ok) { diff --git a/lib/providers/fortnox/oauth.ts b/lib/providers/fortnox/oauth.ts index af333ed1..5ed08a3f 100644 --- a/lib/providers/fortnox/oauth.ts +++ b/lib/providers/fortnox/oauth.ts @@ -1,5 +1,10 @@ import { FORTNOX_AUTH_URL, FORTNOX_TOKEN_URL } from './config'; import type { OAuthConfig, TokenResponse } from '../types'; +import { + fetchWithTimeout, + OAUTH_TIMEOUT_MS, + OAUTH_REVOKE_TIMEOUT_MS, +} from '@/lib/http/fetch-with-timeout'; const DEFAULT_SCOPES = [ 'companyinformation', @@ -40,18 +45,22 @@ export async function exchangeFortnoxCode( config: OAuthConfig, code: string, ): Promise { - const response = await fetch(FORTNOX_TOKEN_URL, { - method: 'POST', - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - Authorization: basicAuthHeader(config), + const response = await fetchWithTimeout( + FORTNOX_TOKEN_URL, + { + method: 'POST', + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + Authorization: basicAuthHeader(config), + }, + body: new URLSearchParams({ + grant_type: 'authorization_code', + code, + redirect_uri: config.redirectUri, + }).toString(), }, - body: new URLSearchParams({ - grant_type: 'authorization_code', - code, - redirect_uri: config.redirectUri, - }).toString(), - }); + { timeoutMs: OAUTH_TIMEOUT_MS, description: 'Fortnox token exchange' }, + ); if (!response.ok) { const body = await response.text().catch(() => ''); @@ -65,17 +74,21 @@ export async function refreshFortnoxToken( config: OAuthConfig, refreshToken: string, ): Promise { - const response = await fetch(FORTNOX_TOKEN_URL, { - method: 'POST', - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - Authorization: basicAuthHeader(config), + const response = await fetchWithTimeout( + FORTNOX_TOKEN_URL, + { + method: 'POST', + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + Authorization: basicAuthHeader(config), + }, + body: new URLSearchParams({ + grant_type: 'refresh_token', + refresh_token: refreshToken, + }).toString(), }, - body: new URLSearchParams({ - grant_type: 'refresh_token', - refresh_token: refreshToken, - }).toString(), - }); + { timeoutMs: OAUTH_TIMEOUT_MS, description: 'Fortnox token refresh' }, + ); if (!response.ok) { const body = await response.text().catch(() => ''); @@ -89,17 +102,21 @@ export async function revokeFortnoxToken( config: OAuthConfig, refreshToken: string, ): Promise { - const response = await fetch(FORTNOX_TOKEN_URL, { - method: 'POST', - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - Authorization: basicAuthHeader(config), + const response = await fetchWithTimeout( + FORTNOX_TOKEN_URL, + { + method: 'POST', + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + Authorization: basicAuthHeader(config), + }, + body: new URLSearchParams({ + token: refreshToken, + token_type_hint: 'refresh_token', + }).toString(), }, - body: new URLSearchParams({ - token: refreshToken, - token_type_hint: 'refresh_token', - }).toString(), - }); + { timeoutMs: OAUTH_REVOKE_TIMEOUT_MS, description: 'Fortnox token revoke' }, + ); return response.ok; } diff --git a/lib/providers/visma/client.ts b/lib/providers/visma/client.ts index 8a71a571..e82a455e 100644 --- a/lib/providers/visma/client.ts +++ b/lib/providers/visma/client.ts @@ -1,6 +1,9 @@ import { TokenBucketRateLimiter } from '../rate-limiter'; import { withRetry } from '../retry'; import { VISMA_BASE_URL, VISMA_RATE_LIMIT } from './config'; +import { isTimeoutError } from '@/lib/http/fetch-with-timeout'; + +const FETCH_TIMEOUT_MS = 15_000; export class VismaApiError extends Error { constructor( @@ -14,6 +17,7 @@ export class VismaApiError extends Error { } function isRetryableError(error: unknown): boolean { + if (isTimeoutError(error)) return true; if (error instanceof VismaApiError) { if (error.statusCode === 401 || error.statusCode === 403 || error.statusCode === 404) { return false; @@ -48,6 +52,7 @@ export class VismaClient { Accept: 'application/json', 'Content-Type': 'application/json', }, + signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), }); if (!response.ok) { diff --git a/lib/providers/visma/oauth.ts b/lib/providers/visma/oauth.ts index 7f7899f4..34b8d260 100644 --- a/lib/providers/visma/oauth.ts +++ b/lib/providers/visma/oauth.ts @@ -1,5 +1,10 @@ import { VISMA_AUTH_URL, VISMA_TOKEN_URL, VISMA_REVOKE_URL } from './config'; import type { OAuthConfig, TokenResponse } from '../types'; +import { + fetchWithTimeout, + OAUTH_TIMEOUT_MS, + OAUTH_REVOKE_TIMEOUT_MS, +} from '@/lib/http/fetch-with-timeout'; const DEFAULT_SCOPES = [ 'ea:api', @@ -41,18 +46,22 @@ export async function exchangeVismaCode( config: OAuthConfig, code: string, ): Promise { - const response = await fetch(VISMA_TOKEN_URL, { - method: 'POST', - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - Authorization: basicAuthHeader(config), + const response = await fetchWithTimeout( + VISMA_TOKEN_URL, + { + method: 'POST', + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + Authorization: basicAuthHeader(config), + }, + body: new URLSearchParams({ + grant_type: 'authorization_code', + code, + redirect_uri: config.redirectUri, + }).toString(), }, - body: new URLSearchParams({ - grant_type: 'authorization_code', - code, - redirect_uri: config.redirectUri, - }).toString(), - }); + { timeoutMs: OAUTH_TIMEOUT_MS, description: 'Visma token exchange' }, + ); if (!response.ok) { const body = await response.text().catch(() => ''); @@ -66,17 +75,21 @@ export async function refreshVismaToken( config: OAuthConfig, refreshToken: string, ): Promise { - const response = await fetch(VISMA_TOKEN_URL, { - method: 'POST', - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - Authorization: basicAuthHeader(config), + const response = await fetchWithTimeout( + VISMA_TOKEN_URL, + { + method: 'POST', + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + Authorization: basicAuthHeader(config), + }, + body: new URLSearchParams({ + grant_type: 'refresh_token', + refresh_token: refreshToken, + }).toString(), }, - body: new URLSearchParams({ - grant_type: 'refresh_token', - refresh_token: refreshToken, - }).toString(), - }); + { timeoutMs: OAUTH_TIMEOUT_MS, description: 'Visma token refresh' }, + ); if (!response.ok) { const body = await response.text().catch(() => ''); @@ -90,17 +103,21 @@ export async function revokeVismaToken( config: OAuthConfig, refreshToken: string, ): Promise { - const response = await fetch(VISMA_REVOKE_URL, { - method: 'POST', - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - Authorization: basicAuthHeader(config), + const response = await fetchWithTimeout( + VISMA_REVOKE_URL, + { + method: 'POST', + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + Authorization: basicAuthHeader(config), + }, + body: new URLSearchParams({ + token: refreshToken, + token_type_hint: 'refresh_token', + }).toString(), }, - body: new URLSearchParams({ - token: refreshToken, - token_type_hint: 'refresh_token', - }).toString(), - }); + { timeoutMs: OAUTH_REVOKE_TIMEOUT_MS, description: 'Visma token revoke' }, + ); return response.ok; } diff --git a/supabase/migrations/20260421160000_booking_template_usage.sql b/supabase/migrations/20260421160000_booking_template_usage.sql index f14e1e5b..f10f839d 100644 --- a/supabase/migrations/20260421160000_booking_template_usage.sql +++ b/supabase/migrations/20260421160000_booking_template_usage.sql @@ -12,7 +12,7 @@ -- -- One row per (template_id, company_id). Upsert on use. -CREATE TABLE public.booking_template_usage ( +CREATE TABLE IF NOT EXISTS public.booking_template_usage ( id UUID PRIMARY KEY DEFAULT uuid_generate_v4(), template_id UUID NOT NULL REFERENCES public.booking_template_library(id) ON DELETE CASCADE, company_id UUID NOT NULL REFERENCES public.companies(id) ON DELETE CASCADE, @@ -25,28 +25,32 @@ CREATE TABLE public.booking_template_usage ( -- RLS ALTER TABLE public.booking_template_usage ENABLE ROW LEVEL SECURITY; +DROP POLICY IF EXISTS "btu_select" ON public.booking_template_usage; CREATE POLICY "btu_select" ON public.booking_template_usage FOR SELECT USING ( company_id IN (SELECT public.user_company_ids()) ); +DROP POLICY IF EXISTS "btu_insert" ON public.booking_template_usage; CREATE POLICY "btu_insert" ON public.booking_template_usage FOR INSERT WITH CHECK ( company_id IN (SELECT public.user_company_ids()) ); +DROP POLICY IF EXISTS "btu_update" ON public.booking_template_usage; CREATE POLICY "btu_update" ON public.booking_template_usage FOR UPDATE USING ( company_id IN (SELECT public.user_company_ids()) ); +DROP POLICY IF EXISTS "btu_delete" ON public.booking_template_usage; CREATE POLICY "btu_delete" ON public.booking_template_usage FOR DELETE USING ( company_id IN (SELECT public.user_company_ids()) ); -- Index for the sort query: fetch last_used_at for a given company. -CREATE INDEX idx_btu_company_last_used +CREATE INDEX IF NOT EXISTS idx_btu_company_last_used ON public.booking_template_usage (company_id, last_used_at DESC); -- Schema reload for PostgREST diff --git a/supabase/migrations/20260421160000_opening_balances_rpc.sql b/supabase/migrations/20260421160500_opening_balances_rpc.sql similarity index 100% rename from supabase/migrations/20260421160000_opening_balances_rpc.sql rename to supabase/migrations/20260421160500_opening_balances_rpc.sql diff --git a/supabase/migrations/20260421170000_commit_journal_entry_user_id_fallback.sql b/supabase/migrations/20260421170500_commit_journal_entry_user_id_fallback.sql similarity index 100% rename from supabase/migrations/20260421170000_commit_journal_entry_user_id_fallback.sql rename to supabase/migrations/20260421170500_commit_journal_entry_user_id_fallback.sql diff --git a/supabase/migrations/20260422120000_fix_rls_role_gates_on_membership_tables.sql b/supabase/migrations/20260422120000_fix_rls_role_gates_on_membership_tables.sql new file mode 100644 index 00000000..b276b9d9 --- /dev/null +++ b/supabase/migrations/20260422120000_fix_rls_role_gates_on_membership_tables.sql @@ -0,0 +1,265 @@ +-- ============================================================================= +-- Fix RLS escalation across multi-tenant authorization layer. +-- +-- The INSERT/UPDATE/DELETE policies defined in +-- 20260330130000_multi_tenant_company_refactor.sql +-- 20260330140000_company_invitations.sql +-- 20260331010000_teams_table_refactor.sql +-- gated writes only on `user_company_ids()` / `user_team_ids()` — i.e. any +-- membership regardless of role. This allowed a user with role='viewer' to +-- issue a direct PATCH against PostgREST and promote themselves to 'owner' +-- (confirmed in production), bypassing the app-layer requireWritePermission +-- guard entirely. +-- +-- Fix: require the caller to hold role IN ('owner','admin') in the target +-- company/team for every write on the authorization-sensitive tables. All +-- legitimate app flows write via service role or SECURITY DEFINER RPCs/ +-- triggers, which bypass RLS — so these tightened policies only block +-- direct PostgREST calls from user sessions, which was the exploit path. +-- +-- Role check is wrapped in SECURITY DEFINER helpers (user_is_company_admin, +-- user_is_team_admin) matching the existing user_company_ids() pattern. +-- This is necessary because inlining `EXISTS (SELECT ... FROM company_members)` +-- inside a policy ON company_members is detected by Postgres as recursive. +-- SECURITY DEFINER functions bypass RLS internally, breaking the cycle. +-- +-- Additionally: a BEFORE UPDATE trigger on company_members blocks any role +-- change unless the caller already holds role='owner'. This reserves +-- promotion to 'owner' (or demotion of one) to existing owners — admins +-- cannot mint further owners even though they can otherwise write. +-- ============================================================================= + +-- ============================================================================= +-- 1. SECURITY DEFINER helper functions +-- ============================================================================= + +CREATE OR REPLACE FUNCTION public.user_is_company_admin(p_company_id uuid) +RETURNS boolean +LANGUAGE sql +STABLE +SECURITY DEFINER +SET search_path = public +AS $$ + SELECT EXISTS ( + SELECT 1 FROM public.company_members cm + WHERE cm.company_id = p_company_id + AND cm.user_id = auth.uid() + AND cm.role IN ('owner', 'admin') + ); +$$; + +GRANT EXECUTE ON FUNCTION public.user_is_company_admin(uuid) TO authenticated; + +CREATE OR REPLACE FUNCTION public.user_is_team_admin(p_team_id uuid) +RETURNS boolean +LANGUAGE sql +STABLE +SECURITY DEFINER +SET search_path = public +AS $$ + SELECT EXISTS ( + SELECT 1 FROM public.team_members tm + WHERE tm.team_id = p_team_id + AND tm.user_id = auth.uid() + AND tm.role IN ('owner', 'admin') + ) OR EXISTS ( + SELECT 1 FROM public.teams t + WHERE t.id = p_team_id + AND t.created_by = auth.uid() + ); +$$; + +GRANT EXECUTE ON FUNCTION public.user_is_team_admin(uuid) TO authenticated; + +CREATE OR REPLACE FUNCTION public.user_role_in_company(p_company_id uuid) +RETURNS text +LANGUAGE sql +STABLE +SECURITY DEFINER +SET search_path = public +AS $$ + SELECT role FROM public.company_members + WHERE company_id = p_company_id AND user_id = auth.uid(); +$$; + +GRANT EXECUTE ON FUNCTION public.user_role_in_company(uuid) TO authenticated; + +-- ============================================================================= +-- 2. company_members — tighten write policies +-- ============================================================================= + +DROP POLICY IF EXISTS "company_members_insert" ON public.company_members; +DROP POLICY IF EXISTS "company_members_update" ON public.company_members; +DROP POLICY IF EXISTS "company_members_delete" ON public.company_members; + +CREATE POLICY "company_members_insert" ON public.company_members + FOR INSERT WITH CHECK (public.user_is_company_admin(company_id)); + +CREATE POLICY "company_members_update" ON public.company_members + FOR UPDATE + USING (public.user_is_company_admin(company_id)) + WITH CHECK (public.user_is_company_admin(company_id)); + +CREATE POLICY "company_members_delete" ON public.company_members + FOR DELETE USING (public.user_is_company_admin(company_id)); + +-- ============================================================================= +-- 3. team_members — tighten write policies +-- ============================================================================= + +DROP POLICY IF EXISTS "team_members_insert" ON public.team_members; +DROP POLICY IF EXISTS "team_members_update" ON public.team_members; +DROP POLICY IF EXISTS "team_members_delete" ON public.team_members; + +CREATE POLICY "team_members_insert" ON public.team_members + FOR INSERT WITH CHECK (public.user_is_team_admin(team_id)); + +CREATE POLICY "team_members_update" ON public.team_members + FOR UPDATE + USING (public.user_is_team_admin(team_id)) + WITH CHECK (public.user_is_team_admin(team_id)); + +CREATE POLICY "team_members_delete" ON public.team_members + FOR DELETE USING (public.user_is_team_admin(team_id)); + +-- ============================================================================= +-- 4. api_keys — tighten write policies +-- A viewer minting an API key with broad scopes is catastrophic. +-- ============================================================================= + +DROP POLICY IF EXISTS "api_keys_insert" ON public.api_keys; +DROP POLICY IF EXISTS "api_keys_update" ON public.api_keys; +DROP POLICY IF EXISTS "api_keys_delete" ON public.api_keys; + +CREATE POLICY "api_keys_insert" ON public.api_keys + FOR INSERT WITH CHECK (public.user_is_company_admin(company_id)); + +CREATE POLICY "api_keys_update" ON public.api_keys + FOR UPDATE + USING (public.user_is_company_admin(company_id)) + WITH CHECK (public.user_is_company_admin(company_id)); + +CREATE POLICY "api_keys_delete" ON public.api_keys + FOR DELETE USING (public.user_is_company_admin(company_id)); + +-- ============================================================================= +-- 5. company_invitations — tighten write policies +-- ============================================================================= + +DROP POLICY IF EXISTS "company_invitations_insert" ON public.company_invitations; +DROP POLICY IF EXISTS "company_invitations_update" ON public.company_invitations; +DROP POLICY IF EXISTS "company_invitations_delete" ON public.company_invitations; + +CREATE POLICY "company_invitations_insert" ON public.company_invitations + FOR INSERT WITH CHECK (public.user_is_company_admin(company_id)); + +CREATE POLICY "company_invitations_update" ON public.company_invitations + FOR UPDATE + USING (public.user_is_company_admin(company_id)) + WITH CHECK (public.user_is_company_admin(company_id)); + +CREATE POLICY "company_invitations_delete" ON public.company_invitations + FOR DELETE USING (public.user_is_company_admin(company_id)); + +-- ============================================================================= +-- 6. team_invitations — tighten write policies +-- ============================================================================= + +DROP POLICY IF EXISTS "team_invitations_insert" ON public.team_invitations; +DROP POLICY IF EXISTS "team_invitations_update" ON public.team_invitations; +DROP POLICY IF EXISTS "team_invitations_delete" ON public.team_invitations; + +CREATE POLICY "team_invitations_insert" ON public.team_invitations + FOR INSERT WITH CHECK (public.user_is_team_admin(team_id)); + +CREATE POLICY "team_invitations_update" ON public.team_invitations + FOR UPDATE + USING (public.user_is_team_admin(team_id)) + WITH CHECK (public.user_is_team_admin(team_id)); + +CREATE POLICY "team_invitations_delete" ON public.team_invitations + FOR DELETE USING (public.user_is_team_admin(team_id)); + +-- ============================================================================= +-- 7. companies — tighten UPDATE policy (INSERT keeps created_by check) +-- ============================================================================= + +DROP POLICY IF EXISTS "companies_update" ON public.companies; + +CREATE POLICY "companies_update" ON public.companies + FOR UPDATE + USING (public.user_is_company_admin(id)) + WITH CHECK (public.user_is_company_admin(id)); + +-- ============================================================================= +-- 8. teams — tighten UPDATE policy (INSERT keeps created_by check) +-- ============================================================================= + +DROP POLICY IF EXISTS "teams_update" ON public.teams; + +CREATE POLICY "teams_update" ON public.teams + FOR UPDATE + USING (public.user_is_team_admin(id) OR created_by = auth.uid()) + WITH CHECK (public.user_is_team_admin(id) OR created_by = auth.uid()); + +-- ============================================================================= +-- 9. company_settings — tighten write policies +-- ============================================================================= + +DROP POLICY IF EXISTS "company_settings_insert" ON public.company_settings; +DROP POLICY IF EXISTS "company_settings_update" ON public.company_settings; + +CREATE POLICY "company_settings_insert" ON public.company_settings + FOR INSERT WITH CHECK (public.user_is_company_admin(company_id)); + +CREATE POLICY "company_settings_update" ON public.company_settings + FOR UPDATE + USING (public.user_is_company_admin(company_id)) + WITH CHECK (public.user_is_company_admin(company_id)); + +-- ============================================================================= +-- 10. BEFORE UPDATE trigger on company_members: +-- block role transitions unless the caller already holds role='owner'. +-- Service role / SECURITY DEFINER / direct SQL (migration apply) pass through +-- because auth.uid() is NULL in those contexts. +-- ============================================================================= + +CREATE OR REPLACE FUNCTION public.enforce_company_member_role_transitions() +RETURNS trigger +LANGUAGE plpgsql +AS $$ +DECLARE + caller_role text; +BEGIN + -- Service role, SECURITY DEFINER cascades, and direct SQL have no auth context. + IF auth.uid() IS NULL THEN + RETURN NEW; + END IF; + + -- Nothing to enforce if the role field isn't changing. + IF NEW.role IS NOT DISTINCT FROM OLD.role THEN + RETURN NEW; + END IF; + + caller_role := public.user_role_in_company(OLD.company_id); + + IF caller_role IS DISTINCT FROM 'owner' THEN + RAISE EXCEPTION + 'Only owners can change member roles (your role: %)', + COALESCE(caller_role, 'none'); + END IF; + + RETURN NEW; +END; +$$; + +DROP TRIGGER IF EXISTS enforce_company_member_role_transitions + ON public.company_members; + +CREATE TRIGGER enforce_company_member_role_transitions + BEFORE UPDATE ON public.company_members + FOR EACH ROW + EXECUTE FUNCTION public.enforce_company_member_role_transitions(); + +-- Force PostgREST to pick up the new policies immediately. +NOTIFY pgrst, 'reload schema'; diff --git a/supabase/migrations/20260422130000_close_company_members_insert_role_bypass.sql b/supabase/migrations/20260422130000_close_company_members_insert_role_bypass.sql new file mode 100644 index 00000000..15355116 --- /dev/null +++ b/supabase/migrations/20260422130000_close_company_members_insert_role_bypass.sql @@ -0,0 +1,80 @@ +-- ============================================================================= +-- Close INSERT bypass in company_members owner-role guard. +-- +-- 20260422120000_fix_rls_role_gates_on_membership_tables.sql added a +-- BEFORE UPDATE trigger that blocks non-owners from promoting members to +-- role='owner'. The trigger fires only on UPDATE, and the INSERT policy +-- (company_members_insert) passes any caller with role IN ('owner','admin') +-- with no constraint on NEW.role. An admin could therefore bypass the guard +-- entirely via a direct PostgREST INSERT with role='owner', contradicting +-- the stated guarantee that only owners can mint further owners. +-- +-- Fix: add a BEFORE INSERT trigger that blocks role='owner' unless the +-- caller already holds role='owner' in the target company. +-- +-- Bootstrap case: create_company_with_owner() (SECURITY DEFINER RPC) +-- preserves auth.uid() while inserting the first owner membership for a +-- freshly created company. The trigger must allow this path. The helper +-- user_role_in_company() returns NULL at that moment (no prior membership), +-- so we permit the insert when (a) the caller is inserting themselves and +-- (b) the company has no existing owner. Both conditions together pin the +-- escape hatch to genuine first-time bootstrap; a subsequent attempt to +-- inject a second owner fails on condition (b). +-- ============================================================================= + +CREATE OR REPLACE FUNCTION public.enforce_company_member_role_on_insert() +RETURNS trigger +LANGUAGE plpgsql +AS $$ +DECLARE + caller_role text; +BEGIN + -- Service role and direct SQL (migration apply, admin console) have no + -- auth context; pass through unchanged. + IF auth.uid() IS NULL THEN + RETURN NEW; + END IF; + + -- Only the 'owner' role is gated; admin/member/viewer pass through so that + -- admins retain the ability to add non-owner members. + IF NEW.role IS DISTINCT FROM 'owner' THEN + RETURN NEW; + END IF; + + caller_role := public.user_role_in_company(NEW.company_id); + + -- Existing owners can always mint owners. + IF caller_role = 'owner' THEN + RETURN NEW; + END IF; + + -- Bootstrap: create_company_with_owner RPC inserts the first owner + -- membership. The caller has no prior membership (caller_role IS NULL) + -- and inserts themselves. Reject if an owner already exists to prevent + -- this path from being reused to mint a second owner. + IF caller_role IS NULL + AND NEW.user_id = auth.uid() + AND NOT EXISTS ( + SELECT 1 FROM public.company_members + WHERE company_id = NEW.company_id + AND role = 'owner' + ) + THEN + RETURN NEW; + END IF; + + RAISE EXCEPTION + 'Only owners can add members with role ''owner'' (your role: %)', + COALESCE(caller_role, 'none'); +END; +$$; + +DROP TRIGGER IF EXISTS enforce_company_member_role_on_insert + ON public.company_members; + +CREATE TRIGGER enforce_company_member_role_on_insert + BEFORE INSERT ON public.company_members + FOR EACH ROW + EXECUTE FUNCTION public.enforce_company_member_role_on_insert(); + +NOTIFY pgrst, 'reload schema';