From 7bcc7de27def1c2bfd4d230a38da50e89cb25369 Mon Sep 17 00:00:00 2001 From: VitekHub Date: Sat, 30 May 2026 13:45:33 +0200 Subject: [PATCH 1/3] feat: add Supabase API adapter with ApiError, fields/recovery CRUD, and saveWrappedKey --- docs/implementation-plan/06-phase-6-data.md | 31 +++-- .../auth/model/auth-error-messages.test.ts | 19 ++- .../auth/model/auth-error-messages.ts | 12 +- src/shared/api/api-errors.test.ts | 66 +++++++++ src/shared/api/api-errors.ts | 31 +++++ src/shared/api/api.types.ts | 4 +- src/shared/api/supabase-fields.test.ts | 127 ++++++++++++++++++ src/shared/api/supabase-fields.ts | 46 +++++++ src/shared/api/supabase-keys.test.ts | 89 ++++++++++-- src/shared/api/supabase-keys.ts | 38 +++++- src/shared/api/supabase-recovery.test.ts | 126 +++++++++++++++++ src/shared/api/supabase-recovery.ts | 44 ++++++ src/shared/auth/auth-errors.ts | 1 - src/shared/ui/nav/PublicHeader.tsx | 2 +- src/test/supabase-mock.ts | 27 ++++ 15 files changed, 622 insertions(+), 41 deletions(-) create mode 100644 src/shared/api/api-errors.test.ts create mode 100644 src/shared/api/api-errors.ts create mode 100644 src/shared/api/supabase-fields.test.ts create mode 100644 src/shared/api/supabase-fields.ts create mode 100644 src/shared/api/supabase-recovery.test.ts create mode 100644 src/shared/api/supabase-recovery.ts create mode 100644 src/test/supabase-mock.ts diff --git a/docs/implementation-plan/06-phase-6-data.md b/docs/implementation-plan/06-phase-6-data.md index f85343e..2e377eb 100644 --- a/docs/implementation-plan/06-phase-6-data.md +++ b/docs/implementation-plan/06-phase-6-data.md @@ -5,27 +5,26 @@ **Goal:** Full CRUD implementation for all database operations, split into focused modules. **Code:** -- `src/shared/api/supabase-client.ts` — Supabase client initialization and export only (no business logic) +- `src/shared/api/api-errors.ts` — `ApiError` class + `ApiErrorCode` (`NETWORK_ERROR`, `NOT_FOUND`, `UNEXPECTED`), `isApiError()` type guard, `wrapApiError()` (reuses `isNetworkError` from auth-errors) - `src/shared/api/supabase-keys.ts` — Keys CRUD: - - `getMasterKeyEnvelope(userId: string): Promise` — fetch auth_salt, key_salt, wrapped_master_key, master_key_iv - - `getFieldKeys(userId: string): Promise` — fetch all wrapped field keys with versions - - `saveWrappedKey(userId: string, data: WrappedKeyData): Promise` — save/update wrapped key data + - `fetchMasterKeyEnvelope(userId)` — now throws `ApiError(NOT_FOUND)` instead of `AuthError(KEYS_NOT_FOUND)` + - `fetchFieldKeys(userId)` — same error refactor + - `saveWrappedKey(userId, data: SaveWrappedKeyData)` — upsert on `field_keys` with `onConflict: 'user_id,field_name,version'` + - `fetchLoginSalts` stays with `AuthError` (pre-auth, not an IApiAdapter method); local error helper renamed to `wrapAuthError` - `src/shared/api/supabase-fields.ts` — Fields CRUD: - - `getField(userId: string, fieldName: string): Promise` — fetch encrypted field data - - `saveField(userId: string, fieldName: string, blob: Uint8Array, iv: Uint8Array): Promise` — upsert encrypted field + - `fetchField(userId, fieldName)` — `.maybeSingle()` on `encrypted_fields`, returns `null` if missing, maps snake_case → camelCase + - `saveField(userId, fieldName, data: SaveFieldData)` — `.upsert()` with `onConflict: 'user_id,field_name'` - `src/shared/api/supabase-recovery.ts` — Recovery data CRUD: - - `saveRecoveryData(userId: string, data: RecoveryData): Promise` — save recovery data - - `getRecoveryData(userId: string): Promise` — fetch recovery data -- All queries use Supabase client with RLS (user can only access own data) -- `ServerKeys`, `ServerFieldKey`, `ServerEncryptedField`, `ServerRecoveryData` types in api.types.ts + - `fetchRecoveryData(userId)` — `.maybeSingle()` on `recovery`, returns `null` if missing + - `saveRecoveryData(userId, data: SaveRecoveryData)` — `.upsert()` with `onConflict: 'user_id'` +- All data flows as hex strings in the API layer — no Uint8Array conversion at this boundary +- Remove `KEYS_NOT_FOUND` from `AuthErrorCode`; update `auth-error-messages.ts` to handle `ApiError` codes +- Update `IApiAdapter` interface: rename `getField` → `fetchField`, `getRecoveryData` → `fetchRecoveryData` **Tests:** -- Integration tests against local Supabase: - - Create user → save keys → fetch keys → verify match - - Create user → save field → fetch field → verify match - - Update field → fetch → verify updated - - RLS: user A cannot read user B's data - - RLS: unauthenticated user cannot read any data +- Unit tests with mocked Supabase client for all CRUD functions +- `api-errors.test.ts` — construction, type guard, `wrapApiError` mapping +- Error assertions: `ApiError(NOT_FOUND)` for missing data, `ApiError(UNEXPECTED)` for query failures --- diff --git a/src/features/auth/model/auth-error-messages.test.ts b/src/features/auth/model/auth-error-messages.test.ts index 5f9839b..1015131 100644 --- a/src/features/auth/model/auth-error-messages.test.ts +++ b/src/features/auth/model/auth-error-messages.test.ts @@ -2,6 +2,7 @@ import type { TFunction } from 'i18next' import { describe, it, expect, vi } from 'vitest' import { getAuthErrorMessage } from '@/features/auth/model/auth-error-messages' import { AuthError, AuthErrorCode } from '@/shared/auth/auth-errors' +import { ApiError, ApiErrorCode } from '@/shared/api/api-errors' const t = vi.fn((key: string) => key) as unknown as TFunction<'auth'> @@ -26,13 +27,25 @@ describe('getAuthErrorMessage', () => { expect(t).toHaveBeenCalledWith('errors.networkError') }) - it('maps KEYS_NOT_FOUND to unexpectedError', () => { - getAuthErrorMessage(new AuthError(AuthErrorCode.KEYS_NOT_FOUND), t) + it('maps UNEXPECTED to unexpectedError', () => { + getAuthErrorMessage(new AuthError(AuthErrorCode.UNEXPECTED), t) + expect(t).toHaveBeenCalledWith('errors.unexpectedError') + }) + }) + + describe('ApiError code mapping', () => { + it('maps NETWORK_ERROR to networkError', () => { + getAuthErrorMessage(new ApiError(ApiErrorCode.NETWORK_ERROR), t) + expect(t).toHaveBeenCalledWith('errors.networkError') + }) + + it('maps NOT_FOUND to unexpectedError', () => { + getAuthErrorMessage(new ApiError(ApiErrorCode.NOT_FOUND), t) expect(t).toHaveBeenCalledWith('errors.unexpectedError') }) it('maps UNEXPECTED to unexpectedError', () => { - getAuthErrorMessage(new AuthError(AuthErrorCode.UNEXPECTED), t) + getAuthErrorMessage(new ApiError(ApiErrorCode.UNEXPECTED), t) expect(t).toHaveBeenCalledWith('errors.unexpectedError') }) }) diff --git a/src/features/auth/model/auth-error-messages.ts b/src/features/auth/model/auth-error-messages.ts index c6c5b14..06160fc 100644 --- a/src/features/auth/model/auth-error-messages.ts +++ b/src/features/auth/model/auth-error-messages.ts @@ -1,5 +1,6 @@ import type { TFunction } from 'i18next' import { AuthErrorCode, isAuthError, isNetworkError } from '@/shared/auth/auth-errors' +import { ApiErrorCode, isApiError } from '@/shared/api/api-errors' export function getAuthErrorMessage(error: unknown, t: TFunction<'auth'>): string { if (isAuthError(error)) { @@ -10,12 +11,21 @@ export function getAuthErrorMessage(error: unknown, t: TFunction<'auth'>): strin return t('errors.usernameTaken') case AuthErrorCode.NETWORK_ERROR: return t('errors.networkError') - case AuthErrorCode.KEYS_NOT_FOUND: case AuthErrorCode.UNEXPECTED: return t('errors.unexpectedError') } } + if (isApiError(error)) { + switch (error.code) { + case ApiErrorCode.NETWORK_ERROR: + return t('errors.networkError') + case ApiErrorCode.NOT_FOUND: + case ApiErrorCode.UNEXPECTED: + return t('errors.unexpectedError') + } + } + if (isNetworkError(error)) return t('errors.networkError') return t('errors.unexpectedError') diff --git a/src/shared/api/api-errors.test.ts b/src/shared/api/api-errors.test.ts new file mode 100644 index 0000000..cda875c --- /dev/null +++ b/src/shared/api/api-errors.test.ts @@ -0,0 +1,66 @@ +import { describe, it, expect } from 'vitest' +import { ApiError, ApiErrorCode, isApiError, wrapApiError } from '@/shared/api/api-errors' + +describe('ApiError', () => { + it('constructs with code and default message', () => { + const error = new ApiError(ApiErrorCode.NOT_FOUND) + expect(error).toBeInstanceOf(Error) + expect(error).toBeInstanceOf(ApiError) + expect(error.name).toBe('ApiError') + expect(error.message).toBe('ApiError: NOT_FOUND') + expect(error.code).toBe('NOT_FOUND') + }) + + it('preserves cause via ErrorOptions', () => { + const cause = new Error('db down') + const error = new ApiError(ApiErrorCode.UNEXPECTED, { cause }) + expect(error.cause).toBe(cause) + }) +}) + +describe('isApiError', () => { + it('returns true for ApiError instances', () => { + expect(isApiError(new ApiError(ApiErrorCode.NETWORK_ERROR))).toBe(true) + }) + + it('returns false for plain Error', () => { + expect(isApiError(new Error('oops'))).toBe(false) + }) + + it('returns false for null', () => { + expect(isApiError(null)).toBe(false) + }) + + it('returns false for string', () => { + expect(isApiError('ApiError: NOT_FOUND')).toBe(false) + }) +}) + +describe('wrapApiError', () => { + it('maps TypeError "Failed to fetch" to NETWORK_ERROR', () => { + const error = wrapApiError(new TypeError('Failed to fetch')) + expect(error).toBeInstanceOf(ApiError) + expect(error.code).toBe(ApiErrorCode.NETWORK_ERROR) + }) + + it('maps Error with "network" in message to NETWORK_ERROR', () => { + const error = wrapApiError(new Error('A Network failure occurred')) + expect(error).toBeInstanceOf(ApiError) + expect(error.code).toBe(ApiErrorCode.NETWORK_ERROR) + }) + + it('maps non-network errors to UNEXPECTED', () => { + const original = new Error('something else') + const error = wrapApiError(original) + expect(error).toBeInstanceOf(ApiError) + expect(error.code).toBe(ApiErrorCode.UNEXPECTED) + expect(error.cause).toBe(original) + }) + + it('maps non-Error values to UNEXPECTED', () => { + const error = wrapApiError('string error') + expect(error).toBeInstanceOf(ApiError) + expect(error.code).toBe(ApiErrorCode.UNEXPECTED) + expect(error.cause).toBeUndefined() + }) +}) diff --git a/src/shared/api/api-errors.ts b/src/shared/api/api-errors.ts new file mode 100644 index 0000000..2273fad --- /dev/null +++ b/src/shared/api/api-errors.ts @@ -0,0 +1,31 @@ +import { isNetworkError } from '@/shared/auth/auth-errors' + +export const ApiErrorCode = { + NETWORK_ERROR: 'NETWORK_ERROR', + NOT_FOUND: 'NOT_FOUND', + UNEXPECTED: 'UNEXPECTED', +} as const + +export type ApiErrorCode = (typeof ApiErrorCode)[keyof typeof ApiErrorCode] + +export class ApiError extends Error { + readonly code: ApiErrorCode + + constructor(code: ApiErrorCode, options?: ErrorOptions) { + super(`ApiError: ${code}`, options) + this.name = 'ApiError' + this.code = code + } +} + +export function isApiError(error: unknown): error is ApiError { + return error instanceof ApiError +} + +export function wrapApiError(error: unknown): ApiError { + const cause = error instanceof Error ? error : undefined + if (isNetworkError(error)) { + return new ApiError(ApiErrorCode.NETWORK_ERROR, { cause }) + } + return new ApiError(ApiErrorCode.UNEXPECTED, { cause }) +} diff --git a/src/shared/api/api.types.ts b/src/shared/api/api.types.ts index a1b850a..16c7c13 100644 --- a/src/shared/api/api.types.ts +++ b/src/shared/api/api.types.ts @@ -13,9 +13,9 @@ export interface IApiAdapter { fetchFieldKeys(userId: string): Promise saveWrappedKey(userId: string, data: SaveWrappedKeyData): Promise - getField(userId: string, fieldName: string): Promise + fetchField(userId: string, fieldName: string): Promise saveField(userId: string, fieldName: string, data: SaveFieldData): Promise saveRecoveryData(userId: string, data: SaveRecoveryData): Promise - getRecoveryData(userId: string): Promise + fetchRecoveryData(userId: string): Promise } diff --git a/src/shared/api/supabase-fields.test.ts b/src/shared/api/supabase-fields.test.ts new file mode 100644 index 0000000..f5ac610 --- /dev/null +++ b/src/shared/api/supabase-fields.test.ts @@ -0,0 +1,127 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { ApiError, ApiErrorCode } from '@/shared/api/api-errors' +import { createSupabaseQueryMocks, createQueryBuilder } from '@/test/supabase-mock' + +const { maybeSingle: mockMaybeSingle, upsert: mockUpsert } = createSupabaseQueryMocks() +const mockFrom = vi.fn() + +vi.mock('@/shared/api/supabase-client', () => ({ + getSupabase: () => ({ + from: mockFrom, + }), +})) + +import { fetchField, saveField } from '@/shared/api/supabase-fields' + +describe('fetchField', () => { + let qb: ReturnType + + beforeEach(() => { + vi.clearAllMocks() + qb = createQueryBuilder({ maybeSingle: mockMaybeSingle, upsert: mockUpsert }) + mockFrom.mockReturnValue(qb) + }) + + it('returns null when field does not exist', async () => { + mockMaybeSingle.mockResolvedValueOnce({ data: null, error: null }) + + const result = await fetchField('user-1', 'note') + + expect(result).toBeNull() + }) + + it('maps snake_case row to camelCase ServerEncryptedField', async () => { + mockMaybeSingle.mockResolvedValueOnce({ + data: { + field_name: 'note', + encrypted_blob: 'aa'.repeat(16), + iv: 'bb'.repeat(12), + updated_at: '2025-01-01T00:00:00Z', + }, + error: null, + }) + + const result = await fetchField('user-1', 'note') + + expect(result).toEqual({ + fieldName: 'note', + encryptedBlob: 'aa'.repeat(16), + iv: 'bb'.repeat(12), + updatedAt: '2025-01-01T00:00:00Z', + }) + }) + + it('queries with userId and fieldName filters', async () => { + mockMaybeSingle.mockResolvedValueOnce({ data: null, error: null }) + + await fetchField('user-1', 'website') + + expect(mockFrom).toHaveBeenCalledWith('encrypted_fields') + expect(qb.select).toHaveBeenCalledWith('field_name, encrypted_blob, iv, updated_at') + expect(qb.eq).toHaveBeenCalledWith('user_id', 'user-1') + expect(qb.eq).toHaveBeenCalledWith('field_name', 'website') + }) + + it('throws ApiError on Supabase query error', async () => { + mockMaybeSingle.mockResolvedValueOnce({ + data: null, + error: { message: 'Query error' }, + }) + + try { + await fetchField('user-1', 'note') + expect.unreachable('should have thrown') + } catch (e) { + expect(e).toBeInstanceOf(ApiError) + expect((e as ApiError).code).toBe(ApiErrorCode.UNEXPECTED) + } + }) +}) + +describe('saveField', () => { + let qb: ReturnType + + beforeEach(() => { + vi.clearAllMocks() + qb = createQueryBuilder({ maybeSingle: mockMaybeSingle, upsert: mockUpsert }) + mockFrom.mockReturnValue(qb) + }) + + it('calls upsert with correct data and onConflict', async () => { + mockUpsert.mockResolvedValueOnce({ data: null, error: null }) + + await saveField('user-1', 'note', { + encryptedBlob: 'aa'.repeat(16), + iv: 'bb'.repeat(12), + }) + + expect(mockFrom).toHaveBeenCalledWith('encrypted_fields') + expect(mockUpsert).toHaveBeenCalledWith( + { + user_id: 'user-1', + field_name: 'note', + encrypted_blob: 'aa'.repeat(16), + iv: 'bb'.repeat(12), + }, + { onConflict: 'user_id,field_name' }, + ) + }) + + it('throws ApiError on Supabase upsert error', async () => { + mockUpsert.mockResolvedValueOnce({ + data: null, + error: { message: 'Upsert failed' }, + }) + + try { + await saveField('user-1', 'note', { + encryptedBlob: 'aa'.repeat(16), + iv: 'bb'.repeat(12), + }) + expect.unreachable('should have thrown') + } catch (e) { + expect(e).toBeInstanceOf(ApiError) + expect((e as ApiError).code).toBe(ApiErrorCode.UNEXPECTED) + } + }) +}) \ No newline at end of file diff --git a/src/shared/api/supabase-fields.ts b/src/shared/api/supabase-fields.ts new file mode 100644 index 0000000..5be9e59 --- /dev/null +++ b/src/shared/api/supabase-fields.ts @@ -0,0 +1,46 @@ +import { getSupabase } from '@/shared/api/supabase-client' +import { wrapApiError } from '@/shared/api/api-errors' +import type { ServerEncryptedField, SaveFieldData } from '@/shared/types/api.types' + +/** + * Fetch a single encrypted field for a user. + * Returns null if the field does not exist. + */ +export async function fetchField(userId: string, fieldName: string): Promise { + const supabase = getSupabase() + const { data, error } = await supabase + .from('encrypted_fields') + .select('field_name, encrypted_blob, iv, updated_at') + .eq('user_id', userId) + .eq('field_name', fieldName) + .maybeSingle() + + if (error) throw wrapApiError(error) + if (!data) return null + + return { + fieldName: data.field_name, + encryptedBlob: data.encrypted_blob, + iv: data.iv, + updatedAt: data.updated_at, + } +} + +/** + * Upsert an encrypted field for a user. + * Uses onConflict to handle the unique (user_id, field_name) constraint. + */ +export async function saveField(userId: string, fieldName: string, data: SaveFieldData): Promise { + const supabase = getSupabase() + const { error } = await supabase.from('encrypted_fields').upsert( + { + user_id: userId, + field_name: fieldName, + encrypted_blob: data.encryptedBlob, + iv: data.iv, + }, + { onConflict: 'user_id,field_name' }, + ) + + if (error) throw wrapApiError(error) +} diff --git a/src/shared/api/supabase-keys.test.ts b/src/shared/api/supabase-keys.test.ts index 3318725..43fc205 100644 --- a/src/shared/api/supabase-keys.test.ts +++ b/src/shared/api/supabase-keys.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { AuthError, AuthErrorCode } from '@/shared/auth/auth-errors' +import { ApiError, ApiErrorCode } from '@/shared/api/api-errors' // Mock Supabase client const mockFrom = vi.fn() @@ -7,6 +8,7 @@ const mockRpc = vi.fn() const mockSelect = vi.fn() const mockEq = vi.fn() const mockSingle = vi.fn() +const mockUpsert = vi.fn() vi.mock('@/shared/api/supabase-client', () => ({ getSupabase: () => ({ @@ -15,7 +17,7 @@ vi.mock('@/shared/api/supabase-client', () => ({ }), })) -import { fetchLoginSalts, fetchMasterKeyEnvelope, fetchFieldKeys } from '@/shared/api/supabase-keys' +import { fetchLoginSalts, fetchMasterKeyEnvelope, fetchFieldKeys, saveWrappedKey } from '@/shared/api/supabase-keys' describe('fetchLoginSalts', () => { beforeEach(() => { @@ -134,16 +136,22 @@ describe('fetchMasterKeyEnvelope', () => { }) }) - it('throws when query returns error', async () => { + it('throws ApiError on query error', async () => { mockSingle.mockResolvedValueOnce({ data: null, error: { message: 'Query error' }, }) - await expect(fetchMasterKeyEnvelope('user-1')).rejects.toThrow() + try { + await fetchMasterKeyEnvelope('user-1') + expect.unreachable('should have thrown') + } catch (e) { + expect(e).toBeInstanceOf(ApiError) + expect((e as ApiError).code).toBe(ApiErrorCode.UNEXPECTED) + } }) - it('throws KEYS_NOT_FOUND when no data found', async () => { + it('throws NOT_FOUND when no data found', async () => { mockSingle.mockResolvedValueOnce({ data: null, error: null, @@ -153,8 +161,8 @@ describe('fetchMasterKeyEnvelope', () => { await fetchMasterKeyEnvelope('user-1') expect.unreachable('should have thrown') } catch (e) { - expect(e).toBeInstanceOf(AuthError) - expect((e as AuthError).code).toBe(AuthErrorCode.KEYS_NOT_FOUND) + expect(e).toBeInstanceOf(ApiError) + expect((e as ApiError).code).toBe(ApiErrorCode.NOT_FOUND) } }) }) @@ -203,7 +211,7 @@ describe('fetchFieldKeys', () => { expect(result).toEqual([]) }) - it('throws KEYS_NOT_FOUND when data is null', async () => { + it('throws NOT_FOUND when data is null', async () => { mockEq.mockResolvedValueOnce({ data: null, error: null, @@ -213,17 +221,76 @@ describe('fetchFieldKeys', () => { await fetchFieldKeys('user-1') expect.unreachable('should have thrown') } catch (e) { - expect(e).toBeInstanceOf(AuthError) - expect((e as AuthError).code).toBe(AuthErrorCode.KEYS_NOT_FOUND) + expect(e).toBeInstanceOf(ApiError) + expect((e as ApiError).code).toBe(ApiErrorCode.NOT_FOUND) } }) - it('throws when query returns error', async () => { + it('throws ApiError on query error', async () => { mockEq.mockResolvedValueOnce({ data: null, error: { message: 'Query error' }, }) - await expect(fetchFieldKeys('user-1')).rejects.toThrow() + try { + await fetchFieldKeys('user-1') + expect.unreachable('should have thrown') + } catch (e) { + expect(e).toBeInstanceOf(ApiError) + expect((e as ApiError).code).toBe(ApiErrorCode.UNEXPECTED) + } + }) +}) + +describe('saveWrappedKey', () => { + beforeEach(() => { + vi.clearAllMocks() + + mockFrom.mockReturnValue({ + upsert: mockUpsert, + }) + }) + + it('calls upsert with correct data and onConflict', async () => { + mockUpsert.mockResolvedValueOnce({ data: null, error: null }) + + await saveWrappedKey('user-1', { + fieldName: 'note', + version: 1, + wrappedKey: 'aa'.repeat(48), + keyIV: 'bb'.repeat(12), + }) + + expect(mockFrom).toHaveBeenCalledWith('field_keys') + expect(mockUpsert).toHaveBeenCalledWith( + { + user_id: 'user-1', + field_name: 'note', + version: 1, + wrapped_key: 'aa'.repeat(48), + key_iv: 'bb'.repeat(12), + }, + { onConflict: 'user_id,field_name,version' }, + ) + }) + + it('throws ApiError on upsert error', async () => { + mockUpsert.mockResolvedValueOnce({ + data: null, + error: { message: 'Upsert failed' }, + }) + + try { + await saveWrappedKey('user-1', { + fieldName: 'note', + version: 1, + wrappedKey: 'aa'.repeat(48), + keyIV: 'bb'.repeat(12), + }) + expect.unreachable('should have thrown') + } catch (e) { + expect(e).toBeInstanceOf(ApiError) + expect((e as ApiError).code).toBe(ApiErrorCode.UNEXPECTED) + } }) }) diff --git a/src/shared/api/supabase-keys.ts b/src/shared/api/supabase-keys.ts index 60927e7..8453470 100644 --- a/src/shared/api/supabase-keys.ts +++ b/src/shared/api/supabase-keys.ts @@ -1,7 +1,9 @@ import { getSupabase } from '@/shared/api/supabase-client' import { USERNAME_PATTERN } from '@/shared/auth/username-utils' -import { AuthError, AuthErrorCode, isNetworkError } from '@/shared/auth/auth-errors' -import type { ServerMasterKeyEnvelope, ServerFieldKey } from '@/shared/types/api.types' +import { AuthError, AuthErrorCode } from '@/shared/auth/auth-errors' +import { isNetworkError } from '@/shared/auth/auth-errors' +import { ApiError, ApiErrorCode, wrapApiError } from '@/shared/api/api-errors' +import type { ServerMasterKeyEnvelope, ServerFieldKey, SaveWrappedKeyData } from '@/shared/types/api.types' export interface LoginSalts { authSalt: string @@ -21,7 +23,7 @@ export async function fetchLoginSalts(username: string): Promise { const supabase = getSupabase() const { data, error } = await supabase.rpc('get_login_salts', { p_username: username }) - if (error) throw wrapApiError(error) + if (error) throw wrapAuthError(error) if (!data || data.length === 0) { throw new AuthError(AuthErrorCode.INVALID_CREDENTIALS) } @@ -43,7 +45,7 @@ export async function fetchMasterKeyEnvelope(userId: string): Promise if (error) throw wrapApiError(error) if (!data) { - throw new AuthError(AuthErrorCode.KEYS_NOT_FOUND) + throw new ApiError(ApiErrorCode.NOT_FOUND) } return data.map((row) => ({ @@ -76,7 +78,31 @@ export async function fetchFieldKeys(userId: string): Promise })) } -function wrapApiError(error: unknown): AuthError { +/** + * Upsert a wrapped field key for a user. + * Uses onConflict to handle the unique (user_id, field_name, version) constraint. + */ +export async function saveWrappedKey(userId: string, data: SaveWrappedKeyData): Promise { + const supabase = getSupabase() + const { error } = await supabase.from('field_keys').upsert( + { + user_id: userId, + field_name: data.fieldName, + version: data.version, + wrapped_key: data.wrappedKey, + key_iv: data.keyIV, + }, + { onConflict: 'user_id,field_name,version' }, + ) + + if (error) throw wrapApiError(error) +} + +/** + * Wrap a Supabase/unknown error as an AuthError for the pre-auth login salts flow. + * Only used by fetchLoginSalts — all other functions use wrapApiError from api-errors.ts. + */ +function wrapAuthError(error: unknown): AuthError { if (isNetworkError(error)) { return new AuthError(AuthErrorCode.NETWORK_ERROR, { cause: error instanceof Error ? error : undefined }) } diff --git a/src/shared/api/supabase-recovery.test.ts b/src/shared/api/supabase-recovery.test.ts new file mode 100644 index 0000000..c6dfcdb --- /dev/null +++ b/src/shared/api/supabase-recovery.test.ts @@ -0,0 +1,126 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { ApiError, ApiErrorCode } from '@/shared/api/api-errors' +import { createSupabaseQueryMocks, createQueryBuilder } from '@/test/supabase-mock' + +const { maybeSingle: mockMaybeSingle, upsert: mockUpsert } = createSupabaseQueryMocks() +const mockFrom = vi.fn() + +vi.mock('@/shared/api/supabase-client', () => ({ + getSupabase: () => ({ + from: mockFrom, + }), +})) + +import { fetchRecoveryData, saveRecoveryData } from '@/shared/api/supabase-recovery' + +describe('fetchRecoveryData', () => { + let qb: ReturnType + + beforeEach(() => { + vi.clearAllMocks() + qb = createQueryBuilder({ maybeSingle: mockMaybeSingle, upsert: mockUpsert }) + mockFrom.mockReturnValue(qb) + }) + + it('returns null when recovery data does not exist', async () => { + mockMaybeSingle.mockResolvedValueOnce({ data: null, error: null }) + + const result = await fetchRecoveryData('user-1') + + expect(result).toBeNull() + }) + + it('maps snake_case row to camelCase ServerRecoveryData', async () => { + mockMaybeSingle.mockResolvedValueOnce({ + data: { + recovery_salt: 'aa'.repeat(16), + wrapped_master_key: 'bb'.repeat(48), + recovery_iv: 'cc'.repeat(12), + }, + error: null, + }) + + const result = await fetchRecoveryData('user-1') + + expect(result).toEqual({ + recoverySalt: 'aa'.repeat(16), + wrappedMasterKey: 'bb'.repeat(48), + recoveryIV: 'cc'.repeat(12), + }) + }) + + it('queries with userId filter', async () => { + mockMaybeSingle.mockResolvedValueOnce({ data: null, error: null }) + + await fetchRecoveryData('user-1') + + expect(mockFrom).toHaveBeenCalledWith('recovery') + expect(qb.select).toHaveBeenCalledWith('recovery_salt, wrapped_master_key, recovery_iv') + expect(qb.eq).toHaveBeenCalledWith('user_id', 'user-1') + }) + + it('throws ApiError on Supabase query error', async () => { + mockMaybeSingle.mockResolvedValueOnce({ + data: null, + error: { message: 'Query error' }, + }) + + try { + await fetchRecoveryData('user-1') + expect.unreachable('should have thrown') + } catch (e) { + expect(e).toBeInstanceOf(ApiError) + expect((e as ApiError).code).toBe(ApiErrorCode.UNEXPECTED) + } + }) +}) + +describe('saveRecoveryData', () => { + let qb: ReturnType + + beforeEach(() => { + vi.clearAllMocks() + qb = createQueryBuilder({ maybeSingle: mockMaybeSingle, upsert: mockUpsert }) + mockFrom.mockReturnValue(qb) + }) + + it('calls upsert with correct data and onConflict', async () => { + mockUpsert.mockResolvedValueOnce({ data: null, error: null }) + + await saveRecoveryData('user-1', { + recoverySalt: 'aa'.repeat(16), + wrappedMasterKey: 'bb'.repeat(48), + recoveryIV: 'cc'.repeat(12), + }) + + expect(mockFrom).toHaveBeenCalledWith('recovery') + expect(mockUpsert).toHaveBeenCalledWith( + { + user_id: 'user-1', + recovery_salt: 'aa'.repeat(16), + wrapped_master_key: 'bb'.repeat(48), + recovery_iv: 'cc'.repeat(12), + }, + { onConflict: 'user_id' }, + ) + }) + + it('throws ApiError on Supabase upsert error', async () => { + mockUpsert.mockResolvedValueOnce({ + data: null, + error: { message: 'Upsert failed' }, + }) + + try { + await saveRecoveryData('user-1', { + recoverySalt: 'aa'.repeat(16), + wrappedMasterKey: 'bb'.repeat(48), + recoveryIV: 'cc'.repeat(12), + }) + expect.unreachable('should have thrown') + } catch (e) { + expect(e).toBeInstanceOf(ApiError) + expect((e as ApiError).code).toBe(ApiErrorCode.UNEXPECTED) + } + }) +}) \ No newline at end of file diff --git a/src/shared/api/supabase-recovery.ts b/src/shared/api/supabase-recovery.ts new file mode 100644 index 0000000..70697ab --- /dev/null +++ b/src/shared/api/supabase-recovery.ts @@ -0,0 +1,44 @@ +import { getSupabase } from '@/shared/api/supabase-client' +import { wrapApiError } from '@/shared/api/api-errors' +import type { ServerRecoveryData, SaveRecoveryData } from '@/shared/types/api.types' + +/** + * Fetch recovery data for a user. + * Returns null if the user has no recovery row. + */ +export async function fetchRecoveryData(userId: string): Promise { + const supabase = getSupabase() + const { data, error } = await supabase + .from('recovery') + .select('recovery_salt, wrapped_master_key, recovery_iv') + .eq('user_id', userId) + .maybeSingle() + + if (error) throw wrapApiError(error) + if (!data) return null + + return { + recoverySalt: data.recovery_salt, + wrappedMasterKey: data.wrapped_master_key, + recoveryIV: data.recovery_iv, + } +} + +/** + * Upsert recovery data for a user. + * Uses onConflict to handle the PK (user_id) constraint. + */ +export async function saveRecoveryData(userId: string, data: SaveRecoveryData): Promise { + const supabase = getSupabase() + const { error } = await supabase.from('recovery').upsert( + { + user_id: userId, + recovery_salt: data.recoverySalt, + wrapped_master_key: data.wrappedMasterKey, + recovery_iv: data.recoveryIV, + }, + { onConflict: 'user_id' }, + ) + + if (error) throw wrapApiError(error) +} diff --git a/src/shared/auth/auth-errors.ts b/src/shared/auth/auth-errors.ts index eacbb14..0173304 100644 --- a/src/shared/auth/auth-errors.ts +++ b/src/shared/auth/auth-errors.ts @@ -2,7 +2,6 @@ export const AuthErrorCode = { INVALID_CREDENTIALS: 'INVALID_CREDENTIALS', USERNAME_TAKEN: 'USERNAME_TAKEN', NETWORK_ERROR: 'NETWORK_ERROR', - KEYS_NOT_FOUND: 'KEYS_NOT_FOUND', UNEXPECTED: 'UNEXPECTED', } as const diff --git a/src/shared/ui/nav/PublicHeader.tsx b/src/shared/ui/nav/PublicHeader.tsx index 82315ac..e886615 100644 --- a/src/shared/ui/nav/PublicHeader.tsx +++ b/src/shared/ui/nav/PublicHeader.tsx @@ -9,7 +9,7 @@ function PublicHeader() { const { t } = useTranslation('common') return ( -
+
diff --git a/src/test/supabase-mock.ts b/src/test/supabase-mock.ts new file mode 100644 index 0000000..46d48f1 --- /dev/null +++ b/src/test/supabase-mock.ts @@ -0,0 +1,27 @@ +import { vi } from 'vitest' + +/** + * Create mock functions for Supabase query builder terminal methods. + * Returned mocks can be controlled per-test with `mockResolvedValueOnce`. + */ +export function createSupabaseQueryMocks() { + return { + maybeSingle: vi.fn(), + upsert: vi.fn(), + } +} + +/** + * Create a mock Supabase query builder for chaining `select().eq().maybeSingle()` + * and `upsert()` calls. Terminal methods delegate to the provided mocks. + */ +export function createQueryBuilder( + terminalMocks: ReturnType, +) { + const qb: Record> = {} + qb.select = vi.fn().mockReturnValue(qb) + qb.eq = vi.fn().mockReturnValue(qb) + qb.maybeSingle = terminalMocks.maybeSingle + qb.upsert = terminalMocks.upsert + return qb +} \ No newline at end of file From 1197b9ab3037a10335a9c0a93ae74a5db6eabf41 Mon Sep 17 00:00:00 2001 From: VitekHub Date: Sat, 30 May 2026 14:05:16 +0200 Subject: [PATCH 2/3] refactor: fix errors architecture, extract shared utilities and improve error boundaries --- src/app/ErrorBoundary.tsx | 40 +++++++++++++++- .../auth/model/auth-error-messages.ts | 3 +- .../encryption/model/crypto-error-messages.ts | 2 +- src/shared/api/api-errors.ts | 2 +- src/shared/api/supabase-fields.test.ts | 2 +- src/shared/api/supabase-keys.ts | 14 +----- src/shared/api/supabase-recovery.test.ts | 2 +- src/shared/api/supabase-registration.test.ts | 31 ++++++++++--- src/shared/api/supabase-registration.ts | 7 +-- src/shared/auth/auth-errors.test.ts | 46 +++++++++---------- src/shared/auth/auth-errors.ts | 19 ++++---- src/shared/auth/supabase-adapter.ts | 3 +- src/shared/i18n/locales/cs/common.json | 4 ++ src/shared/i18n/locales/en/common.json | 4 ++ src/shared/lib/network-errors.test.ts | 35 ++++++++++++++ src/shared/lib/network-errors.ts | 17 +++++++ src/test/supabase-mock.ts | 6 +-- 17 files changed, 171 insertions(+), 66 deletions(-) create mode 100644 src/shared/lib/network-errors.test.ts create mode 100644 src/shared/lib/network-errors.ts diff --git a/src/app/ErrorBoundary.tsx b/src/app/ErrorBoundary.tsx index 63d11e8..2031fe8 100644 --- a/src/app/ErrorBoundary.tsx +++ b/src/app/ErrorBoundary.tsx @@ -1,7 +1,9 @@ import { useTranslation } from 'react-i18next' import type { ErrorComponentProps } from '@tanstack/react-router' import { Button } from '@/shared/ui/button' -import { DecryptionError, CorruptedDataError } from '@/shared/crypto/errors' +import { Argon2Error, CorruptedDataError, DecryptionError, MnemonicError } from '@/shared/crypto/errors' +import { AuthError, AuthErrorCode } from '@/shared/auth/auth-errors' +import { ApiError, ApiErrorCode } from '@/shared/api/api-errors' function getErrorMessage(error: Error): { title: string; description: string } { if (error instanceof DecryptionError) { @@ -16,6 +18,42 @@ function getErrorMessage(error: Error): { title: string; description: string } { description: 'crypto:errors.corruptedData', } } + if (error instanceof Argon2Error) { + return { + title: 'common:status.error', + description: 'crypto:errors.argon2Failed', + } + } + if (error instanceof MnemonicError) { + return { + title: 'common:status.error', + description: 'crypto:errors.mnemonicFailed', + } + } + if (error instanceof AuthError) { + if (error.code === AuthErrorCode.NETWORK_ERROR) { + return { + title: 'common:status.error', + description: 'common:errors.networkError', + } + } + return { + title: 'common:status.error', + description: 'common:errors.unexpectedError', + } + } + if (error instanceof ApiError) { + if (error.code === ApiErrorCode.NETWORK_ERROR) { + return { + title: 'common:status.error', + description: 'common:errors.networkError', + } + } + return { + title: 'common:status.error', + description: 'common:errors.unexpectedError', + } + } return { title: 'common:status.error', description: error.message || 'common:status.error', diff --git a/src/features/auth/model/auth-error-messages.ts b/src/features/auth/model/auth-error-messages.ts index 06160fc..60dce7e 100644 --- a/src/features/auth/model/auth-error-messages.ts +++ b/src/features/auth/model/auth-error-messages.ts @@ -1,5 +1,6 @@ import type { TFunction } from 'i18next' -import { AuthErrorCode, isAuthError, isNetworkError } from '@/shared/auth/auth-errors' +import { AuthErrorCode, isAuthError } from '@/shared/auth/auth-errors' +import { isNetworkError } from '@/shared/lib/network-errors' import { ApiErrorCode, isApiError } from '@/shared/api/api-errors' export function getAuthErrorMessage(error: unknown, t: TFunction<'auth'>): string { diff --git a/src/features/encryption/model/crypto-error-messages.ts b/src/features/encryption/model/crypto-error-messages.ts index d090b87..fe634b1 100644 --- a/src/features/encryption/model/crypto-error-messages.ts +++ b/src/features/encryption/model/crypto-error-messages.ts @@ -1,7 +1,7 @@ import type { TFunction } from 'i18next' import { DecryptionError, CorruptedDataError, Argon2Error } from '@/shared/crypto/errors' -import { isNetworkError } from '@/shared/auth/auth-errors' +import { isNetworkError } from '@/shared/lib/network-errors' export function getCryptoErrorMessage(error: unknown, t: TFunction<'crypto'>): string { if (error instanceof DecryptionError) return t('errors.wrongPassword') diff --git a/src/shared/api/api-errors.ts b/src/shared/api/api-errors.ts index 2273fad..3f3bfb5 100644 --- a/src/shared/api/api-errors.ts +++ b/src/shared/api/api-errors.ts @@ -1,4 +1,4 @@ -import { isNetworkError } from '@/shared/auth/auth-errors' +import { isNetworkError } from '@/shared/lib/network-errors' export const ApiErrorCode = { NETWORK_ERROR: 'NETWORK_ERROR', diff --git a/src/shared/api/supabase-fields.test.ts b/src/shared/api/supabase-fields.test.ts index f5ac610..603cb7d 100644 --- a/src/shared/api/supabase-fields.test.ts +++ b/src/shared/api/supabase-fields.test.ts @@ -124,4 +124,4 @@ describe('saveField', () => { expect((e as ApiError).code).toBe(ApiErrorCode.UNEXPECTED) } }) -}) \ No newline at end of file +}) diff --git a/src/shared/api/supabase-keys.ts b/src/shared/api/supabase-keys.ts index 8453470..7c5103b 100644 --- a/src/shared/api/supabase-keys.ts +++ b/src/shared/api/supabase-keys.ts @@ -1,7 +1,6 @@ import { getSupabase } from '@/shared/api/supabase-client' import { USERNAME_PATTERN } from '@/shared/auth/username-utils' -import { AuthError, AuthErrorCode } from '@/shared/auth/auth-errors' -import { isNetworkError } from '@/shared/auth/auth-errors' +import { AuthError, AuthErrorCode, wrapAuthError } from '@/shared/auth/auth-errors' import { ApiError, ApiErrorCode, wrapApiError } from '@/shared/api/api-errors' import type { ServerMasterKeyEnvelope, ServerFieldKey, SaveWrappedKeyData } from '@/shared/types/api.types' @@ -97,14 +96,3 @@ export async function saveWrappedKey(userId: string, data: SaveWrappedKeyData): if (error) throw wrapApiError(error) } - -/** - * Wrap a Supabase/unknown error as an AuthError for the pre-auth login salts flow. - * Only used by fetchLoginSalts — all other functions use wrapApiError from api-errors.ts. - */ -function wrapAuthError(error: unknown): AuthError { - if (isNetworkError(error)) { - return new AuthError(AuthErrorCode.NETWORK_ERROR, { cause: error instanceof Error ? error : undefined }) - } - return new AuthError(AuthErrorCode.UNEXPECTED, { cause: error instanceof Error ? error : undefined }) -} diff --git a/src/shared/api/supabase-recovery.test.ts b/src/shared/api/supabase-recovery.test.ts index c6dfcdb..3ce813b 100644 --- a/src/shared/api/supabase-recovery.test.ts +++ b/src/shared/api/supabase-recovery.test.ts @@ -123,4 +123,4 @@ describe('saveRecoveryData', () => { expect((e as ApiError).code).toBe(ApiErrorCode.UNEXPECTED) } }) -}) \ No newline at end of file +}) diff --git a/src/shared/api/supabase-registration.test.ts b/src/shared/api/supabase-registration.test.ts index 97439df..2b902dc 100644 --- a/src/shared/api/supabase-registration.test.ts +++ b/src/shared/api/supabase-registration.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest' import { FIELD_KEY_VERSION } from '@/shared/types/crypto.types' import type { RegistrationResult, RecoveryData } from '@/shared/types/crypto.types' +import { ApiError, ApiErrorCode } from '@/shared/api/api-errors' vi.mock('@/shared/api/supabase-client', () => { const insert = vi.fn().mockResolvedValue({ error: null }) @@ -114,25 +115,37 @@ describe('uploadRegistrationData', () => { expect(recoveryRow.recovery_iv).toHaveLength(24) }) - it('throws on keys insert error', async () => { + it('throws ApiError on keys insert error', async () => { const insert = getMockInsert() insert.mockResolvedValueOnce({ error: new Error('keys insert failed') }) const data = makeRegistrationResult() - await expect(uploadRegistrationData(data, USER_ID)).rejects.toThrow('keys insert failed') + try { + await uploadRegistrationData(data, USER_ID) + expect.unreachable('should have thrown') + } catch (e) { + expect(e).toBeInstanceOf(ApiError) + expect((e as ApiError).code).toBe(ApiErrorCode.UNEXPECTED) + } }) - it('throws on field_keys insert error', async () => { + it('throws ApiError on field_keys insert error', async () => { const insert = getMockInsert() insert .mockResolvedValueOnce({ error: null }) .mockResolvedValueOnce({ error: new Error('field_keys insert failed') }) const data = makeRegistrationResult() - await expect(uploadRegistrationData(data, USER_ID)).rejects.toThrow('field_keys insert failed') + try { + await uploadRegistrationData(data, USER_ID) + expect.unreachable('should have thrown') + } catch (e) { + expect(e).toBeInstanceOf(ApiError) + expect((e as ApiError).code).toBe(ApiErrorCode.UNEXPECTED) + } }) - it('throws on recovery insert error', async () => { + it('throws ApiError on recovery insert error', async () => { const insert = getMockInsert() insert .mockResolvedValueOnce({ error: null }) @@ -140,6 +153,12 @@ describe('uploadRegistrationData', () => { .mockResolvedValueOnce({ error: new Error('recovery insert failed') }) const data = makeRegistrationResult() - await expect(uploadRegistrationData(data, USER_ID)).rejects.toThrow('recovery insert failed') + try { + await uploadRegistrationData(data, USER_ID) + expect.unreachable('should have thrown') + } catch (e) { + expect(e).toBeInstanceOf(ApiError) + expect((e as ApiError).code).toBe(ApiErrorCode.UNEXPECTED) + } }) }) diff --git a/src/shared/api/supabase-registration.ts b/src/shared/api/supabase-registration.ts index 1995802..bf9dfa3 100644 --- a/src/shared/api/supabase-registration.ts +++ b/src/shared/api/supabase-registration.ts @@ -1,4 +1,5 @@ import { getSupabase } from '@/shared/api/supabase-client' +import { wrapApiError } from '@/shared/api/api-errors' import { hexEncode } from '@/shared/crypto/crypto-utils' import type { RegistrationResult } from '@/shared/types/crypto.types' @@ -13,7 +14,7 @@ export async function uploadRegistrationData(data: RegistrationResult, userId: s wrapped_master_key: hexEncode(data.wrappedMasterKey), master_key_iv: hexEncode(data.masterKeyIV), }) - if (keysError) throw keysError + if (keysError) throw wrapApiError(keysError) // 2. Insert field_keys rows (3 wrapped field keys, version 1) const fieldKeysRows = data.wrappedFieldKeys.map((fk) => ({ @@ -24,7 +25,7 @@ export async function uploadRegistrationData(data: RegistrationResult, userId: s key_iv: hexEncode(fk.iv), })) const { error: fieldKeysError } = await supabase.from('field_keys').insert(fieldKeysRows) - if (fieldKeysError) throw fieldKeysError + if (fieldKeysError) throw wrapApiError(fieldKeysError) // 3. Insert recovery row (mnemonic-wrapped master key) const { error: recoveryError } = await supabase.from('recovery').insert({ @@ -33,5 +34,5 @@ export async function uploadRegistrationData(data: RegistrationResult, userId: s wrapped_master_key: hexEncode(data.recoveryData.wrappedMasterKey), recovery_iv: hexEncode(data.recoveryData.recoveryIV), }) - if (recoveryError) throw recoveryError + if (recoveryError) throw wrapApiError(recoveryError) } diff --git a/src/shared/auth/auth-errors.test.ts b/src/shared/auth/auth-errors.test.ts index d0f2b7b..be8f6ef 100644 --- a/src/shared/auth/auth-errors.test.ts +++ b/src/shared/auth/auth-errors.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest' -import { AuthError, AuthErrorCode, isAuthError, isNetworkError } from '@/shared/auth/auth-errors' +import { AuthError, AuthErrorCode, isAuthError, wrapAuthError } from '@/shared/auth/auth-errors' describe('AuthError', () => { it('sets name to AuthError', () => { @@ -47,35 +47,31 @@ describe('isAuthError', () => { }) }) -describe('isNetworkError', () => { - it('detects TypeError with "Failed to fetch" message', () => { - expect(isNetworkError(new TypeError('Failed to fetch'))).toBe(true) - }) - - it('detects Error with "Failed to fetch" message', () => { - expect(isNetworkError(new Error('Failed to fetch'))).toBe(true) - }) - - it('detects Error with "NetworkError" message', () => { - expect(isNetworkError(new Error('NetworkError'))).toBe(true) - }) - - it('detects Error with "network" (case-insensitive)', () => { - expect(isNetworkError(new Error('A Network failure occurred'))).toBe(true) +describe('wrapAuthError', () => { + it('maps network errors to NETWORK_ERROR', () => { + const error = wrapAuthError(new TypeError('Failed to fetch')) + expect(error).toBeInstanceOf(AuthError) + expect(error.code).toBe(AuthErrorCode.NETWORK_ERROR) }) - it('detects Error with "failed to fetch" in message (case-insensitive)', () => { - expect(isNetworkError(new Error('Request failed to fetch data'))).toBe(true) + it('maps non-network errors to UNEXPECTED', () => { + const original = new Error('something else') + const error = wrapAuthError(original) + expect(error).toBeInstanceOf(AuthError) + expect(error.code).toBe(AuthErrorCode.UNEXPECTED) + expect(error.cause).toBe(original) }) - it('returns false for non-network errors', () => { - expect(isNetworkError(new Error('Invalid credentials'))).toBe(false) - expect(isNetworkError(new Error('Something went wrong'))).toBe(false) + it('preserves cause for network errors', () => { + const original = new TypeError('Failed to fetch') + const error = wrapAuthError(original) + expect(error.cause).toBe(original) }) - it('returns false for non-Error values', () => { - expect(isNetworkError(null)).toBe(false) - expect(isNetworkError('string')).toBe(false) - expect(isNetworkError(42)).toBe(false) + it('maps non-Error values to UNEXPECTED without cause', () => { + const error = wrapAuthError('string error') + expect(error).toBeInstanceOf(AuthError) + expect(error.code).toBe(AuthErrorCode.UNEXPECTED) + expect(error.cause).toBeUndefined() }) }) diff --git a/src/shared/auth/auth-errors.ts b/src/shared/auth/auth-errors.ts index 0173304..e8a9ef2 100644 --- a/src/shared/auth/auth-errors.ts +++ b/src/shared/auth/auth-errors.ts @@ -1,3 +1,5 @@ +import { isNetworkError } from '@/shared/lib/network-errors' + export const AuthErrorCode = { INVALID_CREDENTIALS: 'INVALID_CREDENTIALS', USERNAME_TAKEN: 'USERNAME_TAKEN', @@ -21,13 +23,14 @@ export function isAuthError(error: unknown): error is AuthError { return error instanceof AuthError } -export function isNetworkError(error: unknown): boolean { - if (error instanceof TypeError && error.message === 'Failed to fetch') return true - if (error instanceof Error) { - const msg = error.message - if (msg === 'Failed to fetch' || msg === 'NetworkError') return true - const lower = msg.toLowerCase() - if (lower.includes('network') || lower.includes('failed to fetch')) return true +/** + * Wrap an unknown error as an AuthError. + * Network errors map to NETWORK_ERROR, everything else to UNEXPECTED. + */ +export function wrapAuthError(error: unknown): AuthError { + const cause = error instanceof Error ? error : undefined + if (isNetworkError(error)) { + return new AuthError(AuthErrorCode.NETWORK_ERROR, { cause }) } - return false + return new AuthError(AuthErrorCode.UNEXPECTED, { cause }) } diff --git a/src/shared/auth/supabase-adapter.ts b/src/shared/auth/supabase-adapter.ts index 9d09688..2c6ebda 100644 --- a/src/shared/auth/supabase-adapter.ts +++ b/src/shared/auth/supabase-adapter.ts @@ -8,7 +8,8 @@ import type { } from '@/shared/auth/auth.types' import { getSupabase } from '@/shared/api/supabase-client' import { toSupabaseEmail, fromSupabaseEmail } from '@/shared/auth/username-utils' -import { AuthError, AuthErrorCode, isNetworkError } from '@/shared/auth/auth-errors' +import { AuthError, AuthErrorCode } from '@/shared/auth/auth-errors' +import { isNetworkError } from '@/shared/lib/network-errors' class SupabaseAuthAdapter implements IAuthAdapter { async login(username: string, authHash: string): Promise { diff --git a/src/shared/i18n/locales/cs/common.json b/src/shared/i18n/locales/cs/common.json index 969b88a..c356e1a 100644 --- a/src/shared/i18n/locales/cs/common.json +++ b/src/shared/i18n/locales/cs/common.json @@ -16,6 +16,10 @@ "loading": "Načítání...", "error": "Něco se pokazilo" }, + "errors": { + "networkError": "Chyba sítě. Zkuste to prosím znovu.", + "unexpectedError": "Došlo k neočekávané chybě. Zkuste to prosím znovu." + }, "preAlpha": { "ariaLabel": "Upozornění na předběžnou verzi", "message": "Cipher Note je předběžná alfa verze. Očekávejte zásadní změny a občasné resety dat." diff --git a/src/shared/i18n/locales/en/common.json b/src/shared/i18n/locales/en/common.json index 8b6effd..5dcb1cd 100644 --- a/src/shared/i18n/locales/en/common.json +++ b/src/shared/i18n/locales/en/common.json @@ -16,6 +16,10 @@ "loading": "Loading...", "error": "Something went wrong" }, + "errors": { + "networkError": "Network error. Please try again.", + "unexpectedError": "An unexpected error occurred. Please try again." + }, "preAlpha": { "ariaLabel": "Pre-alpha software notice", "message": "Cipher Note is in pre-alpha version. Expect breaking changes and periodic data resets." diff --git a/src/shared/lib/network-errors.test.ts b/src/shared/lib/network-errors.test.ts new file mode 100644 index 0000000..8b07834 --- /dev/null +++ b/src/shared/lib/network-errors.test.ts @@ -0,0 +1,35 @@ +import { describe, it, expect } from 'vitest' +import { isNetworkError } from '@/shared/lib/network-errors' + +describe('isNetworkError', () => { + it('detects TypeError with "Failed to fetch" message', () => { + expect(isNetworkError(new TypeError('Failed to fetch'))).toBe(true) + }) + + it('detects Error with "Failed to fetch" message', () => { + expect(isNetworkError(new Error('Failed to fetch'))).toBe(true) + }) + + it('detects Error with "NetworkError" message', () => { + expect(isNetworkError(new Error('NetworkError'))).toBe(true) + }) + + it('detects Error with "network" (case-insensitive)', () => { + expect(isNetworkError(new Error('A Network failure occurred'))).toBe(true) + }) + + it('detects Error with "failed to fetch" in message (case-insensitive)', () => { + expect(isNetworkError(new Error('Request failed to fetch data'))).toBe(true) + }) + + it('returns false for non-network errors', () => { + expect(isNetworkError(new Error('Invalid credentials'))).toBe(false) + expect(isNetworkError(new Error('Something went wrong'))).toBe(false) + }) + + it('returns false for non-Error values', () => { + expect(isNetworkError(null)).toBe(false) + expect(isNetworkError('string')).toBe(false) + expect(isNetworkError(42)).toBe(false) + }) +}) diff --git a/src/shared/lib/network-errors.ts b/src/shared/lib/network-errors.ts new file mode 100644 index 0000000..27462c5 --- /dev/null +++ b/src/shared/lib/network-errors.ts @@ -0,0 +1,17 @@ +/** + * Check whether an error represents a network failure. + * + * Catches raw browser errors (TypeError "Failed to fetch") and Supabase + * network errors that can reach the UI without being wrapped by an adapter. + * Used by both AuthError and ApiError wrappers to classify network failures. + */ +export function isNetworkError(error: unknown): boolean { + if (error instanceof TypeError && error.message === 'Failed to fetch') return true + if (error instanceof Error) { + const msg = error.message + if (msg === 'Failed to fetch' || msg === 'NetworkError') return true + const lower = msg.toLowerCase() + if (lower.includes('network') || lower.includes('failed to fetch')) return true + } + return false +} diff --git a/src/test/supabase-mock.ts b/src/test/supabase-mock.ts index 46d48f1..617326c 100644 --- a/src/test/supabase-mock.ts +++ b/src/test/supabase-mock.ts @@ -15,13 +15,11 @@ export function createSupabaseQueryMocks() { * Create a mock Supabase query builder for chaining `select().eq().maybeSingle()` * and `upsert()` calls. Terminal methods delegate to the provided mocks. */ -export function createQueryBuilder( - terminalMocks: ReturnType, -) { +export function createQueryBuilder(terminalMocks: ReturnType) { const qb: Record> = {} qb.select = vi.fn().mockReturnValue(qb) qb.eq = vi.fn().mockReturnValue(qb) qb.maybeSingle = terminalMocks.maybeSingle qb.upsert = terminalMocks.upsert return qb -} \ No newline at end of file +} From c23ac623b5cd180f811e92cbcaf8c0eb6a3de329 Mon Sep 17 00:00:00 2001 From: VitekHub Date: Sat, 30 May 2026 14:41:10 +0200 Subject: [PATCH 3/3] fix: throw NOT_FOUND for empty field keys, refactor ErrorBoundary --- CLAUDE.md | 6 +- docs/implementation-plan/06-phase-6-data.md | 2 +- src/app/ErrorBoundary.tsx | 62 +++++++-------------- src/shared/api/supabase-keys.test.ts | 11 +++- src/shared/api/supabase-keys.ts | 2 +- src/shared/auth/supabase-adapter.ts | 2 + 6 files changed, 36 insertions(+), 49 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9141910..3c70956 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -171,6 +171,7 @@ See `IMPLEMENTATION-PLAN.md` for the full 36-step plan. - Step 21 (Login Crypto Flow) — complete - Step 22 (Login UI + Vault Unlock) — complete - Step 23 (Non-Extractable Key Vault + Zustand Store Refactor) — complete +- Step 24 (Supabase API Adapter) — complete ### Implementation Notes @@ -196,5 +197,6 @@ Non-obvious decisions not visible from code alone: - **`deriveRegistrationKeys` is a pure crypto function**: in `features/encryption/model/registration.ts`, it has no side effects (no auth, no DB, no store writes). The orchestration (signup + upload + store population) lives in `auth-flow.ts` `signUpUser`. Do not add side effects to this function. - **`signUpUser` error cleanup**: on any error after `deriveRegistrationKeys` succeeds, attempts `authAdapter.logout()` as best-effort cleanup (harmless if no session exists, since Supabase signOut with no session is a no-op). - **Login salt fetch is pre-auth**: `get_login_salts(p_username)` is a SECURITY DEFINER RPC callable by anonymous users, rate-limited (5 req/2 min/IP). Salts must be fetched before auth to derive `authHash` for Supabase Auth, but the `keys` table is RLS-protected. After auth succeeds, `fetchMasterKeyEnvelope` and `fetchFieldKeys` fetch wrapped key material through standard RLS-protected queries. -- **Auth error codes fold username format into invalid credentials**: `AuthErrorCode.INVALID_USERNAME_FORMAT` doesn't exist — `supabase-keys.ts` throws `INVALID_CREDENTIALS` for invalid username format. This is deliberate: showing a different error for "wrong format" vs "wrong password" would leak whether a username exists. -- **Network errors can bypass the adapter boundary**: `getAuthErrorMessage` in `auth-error-messages.ts` has an `isNetworkError` fallback because raw `TypeError('Failed to fetch')` from the browser can reach the UI without being wrapped by the adapter. The adapter wraps what it can, but the fallback catches the rest. +- **Auth error codes fold username format into invalid credentials**: `AuthErrorCode.INVALID_USERNAME_FORMAT` doesn't exist — `supabase-keys.ts` throws `INVALID_CREDENTIALS` for invalid username format. This is deliberate: showing a different error for "wrong format" vs "wrong password" would leak whether a username exists. Missing key data is `ApiError(NOT_FOUND)`, not an auth error — `KEYS_NOT_FOUND` was removed from `AuthErrorCode` because "data not found" is a data-layer concern, not an auth concern. +- **`AuthError` vs `ApiError` domain boundary**: `AuthError` (in `shared/auth/auth-errors.ts`) covers authentication errors (`INVALID_CREDENTIALS`, `USERNAME_TAKEN`, `NETWORK_ERROR`, `UNEXPECTED`). `ApiError` (in `shared/api/api-errors.ts`) covers data-layer errors (`NETWORK_ERROR`, `NOT_FOUND`, `UNEXPECTED`). `fetchLoginSalts` throws `AuthError` because it's a pre-auth RPC that's part of the login flow; all other data queries throw `ApiError`. Each domain has its own `wrapXxxError` that classifies raw errors using the shared `isNetworkError` helper. +- **Network errors can bypass the adapter boundary**: `isNetworkError` (in `shared/lib/network-errors.ts`) is shared by both `wrapAuthError` and `wrapApiError`. Raw `TypeError('Failed to fetch')` from the browser can reach the UI without being wrapped by any adapter, so `getAuthErrorMessage` in `auth-error-messages.ts` also calls `isNetworkError` as a final fallback. diff --git a/docs/implementation-plan/06-phase-6-data.md b/docs/implementation-plan/06-phase-6-data.md index 2e377eb..8e7842f 100644 --- a/docs/implementation-plan/06-phase-6-data.md +++ b/docs/implementation-plan/06-phase-6-data.md @@ -1,6 +1,6 @@ # Phase 6: Encrypted Data Layer -## Step 24 — Supabase API Adapter +## Step 24 — Supabase API Adapter ✅ **Goal:** Full CRUD implementation for all database operations, split into focused modules. diff --git a/src/app/ErrorBoundary.tsx b/src/app/ErrorBoundary.tsx index 2031fe8..d5e9905 100644 --- a/src/app/ErrorBoundary.tsx +++ b/src/app/ErrorBoundary.tsx @@ -5,55 +5,33 @@ import { Argon2Error, CorruptedDataError, DecryptionError, MnemonicError } from import { AuthError, AuthErrorCode } from '@/shared/auth/auth-errors' import { ApiError, ApiErrorCode } from '@/shared/api/api-errors' +/** Crypto errors with fixed descriptions — add new ones here instead of extending the if-chain. */ +const CRYPTO_ERRORS: readonly (readonly [new () => Error, string])[] = [ + [DecryptionError, 'crypto:errors.decryptFailed'], + [CorruptedDataError, 'crypto:errors.corruptedData'], + [Argon2Error, 'crypto:errors.argon2Failed'], + [MnemonicError, 'crypto:errors.mnemonicFailed'], +] + function getErrorMessage(error: Error): { title: string; description: string } { - if (error instanceof DecryptionError) { - return { - title: 'common:status.error', - description: 'crypto:errors.decryptFailed', - } - } - if (error instanceof CorruptedDataError) { - return { - title: 'common:status.error', - description: 'crypto:errors.corruptedData', - } - } - if (error instanceof Argon2Error) { - return { - title: 'common:status.error', - description: 'crypto:errors.argon2Failed', - } - } - if (error instanceof MnemonicError) { - return { - title: 'common:status.error', - description: 'crypto:errors.mnemonicFailed', + for (const [ErrorClass, description] of CRYPTO_ERRORS) { + if (error instanceof ErrorClass) { + return { title: 'common:status.error', description } } } + if (error instanceof AuthError) { - if (error.code === AuthErrorCode.NETWORK_ERROR) { - return { - title: 'common:status.error', - description: 'common:errors.networkError', - } - } - return { - title: 'common:status.error', - description: 'common:errors.unexpectedError', - } + return error.code === AuthErrorCode.NETWORK_ERROR + ? { title: 'common:status.error', description: 'common:errors.networkError' } + : { title: 'common:status.error', description: 'common:errors.unexpectedError' } } + if (error instanceof ApiError) { - if (error.code === ApiErrorCode.NETWORK_ERROR) { - return { - title: 'common:status.error', - description: 'common:errors.networkError', - } - } - return { - title: 'common:status.error', - description: 'common:errors.unexpectedError', - } + return error.code === ApiErrorCode.NETWORK_ERROR + ? { title: 'common:status.error', description: 'common:errors.networkError' } + : { title: 'common:status.error', description: 'common:errors.unexpectedError' } } + return { title: 'common:status.error', description: error.message || 'common:status.error', diff --git a/src/shared/api/supabase-keys.test.ts b/src/shared/api/supabase-keys.test.ts index 43fc205..1ed1216 100644 --- a/src/shared/api/supabase-keys.test.ts +++ b/src/shared/api/supabase-keys.test.ts @@ -201,14 +201,19 @@ describe('fetchFieldKeys', () => { ]) }) - it('returns empty array when no data', async () => { + it('throws NOT_FOUND when data is empty array', async () => { mockEq.mockResolvedValueOnce({ data: [], error: null, }) - const result = await fetchFieldKeys('user-1') - expect(result).toEqual([]) + try { + await fetchFieldKeys('user-1') + expect.unreachable('should have thrown') + } catch (e) { + expect(e).toBeInstanceOf(ApiError) + expect((e as ApiError).code).toBe(ApiErrorCode.NOT_FOUND) + } }) it('throws NOT_FOUND when data is null', async () => { diff --git a/src/shared/api/supabase-keys.ts b/src/shared/api/supabase-keys.ts index 7c5103b..23d5cf0 100644 --- a/src/shared/api/supabase-keys.ts +++ b/src/shared/api/supabase-keys.ts @@ -65,7 +65,7 @@ export async function fetchFieldKeys(userId: string): Promise .eq('user_id', userId) if (error) throw wrapApiError(error) - if (!data) { + if (!data || data.length === 0) { throw new ApiError(ApiErrorCode.NOT_FOUND) } diff --git a/src/shared/auth/supabase-adapter.ts b/src/shared/auth/supabase-adapter.ts index 2c6ebda..861ab3c 100644 --- a/src/shared/auth/supabase-adapter.ts +++ b/src/shared/auth/supabase-adapter.ts @@ -101,6 +101,8 @@ export function mapSupabaseToAuthResult( export const authAdapter = new SupabaseAuthAdapter() +// Supabase auth errors have their own status/code mapping (e.g. 400 invalid_credentials, +// 409 user_already_exists), so we can't use the generic wrapAuthError here. function wrapSupabaseAuthError(error: unknown): AuthError { if (isNetworkError(error)) { return new AuthError(AuthErrorCode.NETWORK_ERROR, { cause: error instanceof Error ? error : undefined })