From 8e40445a763b46a0953ca4d316f9d01aa3da21c8 Mon Sep 17 00:00:00 2001 From: ardakrt <81361836+ardakrt@users.noreply.github.com> Date: Wed, 28 Oct 2026 23:49:07 +0300 Subject: [PATCH 1/3] fix: validate account secrets and make QR imports atomic --- .github/readme/DEVELOPMENT.md | 10 + SECURITY.md | 13 + .../repositories/AccountRepository.ts | 6 + src/main/ipc/registerAppHandlers.ts | 19 +- src/main/services/AccountService.ts | 59 ++-- src/main/services/TotpService.ts | 4 +- src/main/window/createMainWindow.ts | 38 +++ src/preload/index.ts | 11 + src/renderer/src/pages/AddAccountPage.tsx | 14 +- src/renderer/src/pages/HomePage.tsx | 18 +- src/renderer/src/utils/otpMigrationParser.ts | 284 ++++++++---------- src/renderer/src/utils/qrDecoder.ts | 7 +- src/renderer/src/utils/translations.ts | 7 + src/shared/ipc/channels.ts | 1 + src/shared/schemas/account.ts | 30 +- src/shared/schemas/backup.ts | 4 +- src/shared/types/electron-api.ts | 2 + tests/renderer/accountFlows.test.tsx | 87 ++++++ tests/renderer/otpMigrationParser.test.ts | 75 +++++ tests/setup.ts | 1 + tests/unit/accountSchema.test.ts | 47 +++ tests/unit/accountStorage.test.ts | 113 +++++++ tests/unit/ipcLockGuards.test.ts | 88 ++++++ 23 files changed, 733 insertions(+), 205 deletions(-) create mode 100644 SECURITY.md create mode 100644 tests/renderer/accountFlows.test.tsx create mode 100644 tests/unit/accountSchema.test.ts diff --git a/.github/readme/DEVELOPMENT.md b/.github/readme/DEVELOPMENT.md index 699c5d2..b149d3d 100644 --- a/.github/readme/DEVELOPMENT.md +++ b/.github/readme/DEVELOPMENT.md @@ -40,6 +40,16 @@ npm run smoke:visual `smoke:visual` additionally captures the runtime smoke views into a temporary directory reported by the script. Passing static checks alone does not prove packaged Windows/Linux behavior. +The Electron smoke also exercises the real sandboxed preload and main-process account handlers with an isolated in-memory database. It verifies encrypted batch import, TOTP generation, invalid-secret rejection, database rollback after a later insert fails, and account cleanup. It does not access the system clipboard or the user's vault. + +### Account input and import boundaries + +- Creation, secret updates and backup payloads share canonical Base32 validation. Spaces, hyphens, case and valid padding are normalized; invalid characters, padding and trailing bits are rejected before storage. +- A legacy unreadable account remains visible with an unavailable-code state. Its failure does not prevent healthy accounts from generating or copying codes. +- Google Authenticator migration QR input is bounded to 128 KiB and 100 accounts. Truncated protobuf data, overflowing lengths, unknown algorithms/digit counts and non-TOTP accounts are rejected before the confirmation preview. +- The named `importAccounts` bridge validates both sides of IPC, checks the renderer origin and PIN lock, and rechecks the lock after encryption. Every selected account is inserted in one SQLite transaction; any failure leaves the batch unsaved. +- These changes do not migrate the database or delete existing accounts. A malformed legacy secret must be replaced from the original provider or the affected account removed. + ## Packaging Windows NSIS installer: diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..ce87b53 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,13 @@ +# Security + +To report a potential vulnerability, use [GitHub private vulnerability reporting](https://github.com/ardakrt/authly/security/advisories/new) when available. Otherwise, open an issue requesting a private contact without including account secrets, OTP codes, PINs, passwords or backup contents. + +## Boundaries + +- Electron main owns database access, OS-backed secret encryption and TOTP generation. The sandboxed renderer receives safe metadata and display codes through named IPC methods; stored secrets are not returned by account-list or import responses. +- Account writes validate Base32 keys. QR migration parsing bounds input and rejects malformed or unsupported data. Selected QR accounts are encrypted before one atomic database transaction, with PIN lock checks before and after asynchronous encryption. +- Corrupt or unreadable legacy accounts remain in the vault. Code generation isolates their failure so healthy accounts remain usable. Unavailable codes cannot be copied from the interface. +- The application PIN gates access; it is not an independent encryption key for the vault. An unlocked OS session or compromised operating system is outside this protection boundary. +- TOTP is generated offline. Update operations contact GitHub, and the stylesheet currently references Google Fonts. See the README for platform-specific storage and backup details. + +Never attach a real vault, unencrypted backup, raw QR content, secret key or generated code to an issue or log. Automated tests and runtime smoke use synthetic fixtures and isolated storage. diff --git a/src/main/database/repositories/AccountRepository.ts b/src/main/database/repositories/AccountRepository.ts index 68e525b..05f08a6 100644 --- a/src/main/database/repositories/AccountRepository.ts +++ b/src/main/database/repositories/AccountRepository.ts @@ -83,6 +83,12 @@ export class AccountRepository { ); } + createMany(records: AccountRecord[]): void { + this.database.transaction(() => { + for (const record of records) this.create(record); + })(); + } + update(record: AccountRecord): void { const result = this.database .prepare( diff --git a/src/main/ipc/registerAppHandlers.ts b/src/main/ipc/registerAppHandlers.ts index 1bd8d10..5e1e628 100644 --- a/src/main/ipc/registerAppHandlers.ts +++ b/src/main/ipc/registerAppHandlers.ts @@ -11,6 +11,7 @@ import { IPC_CHANNELS } from '@shared/ipc/channels'; import { accountDtoSchema, createAccountRequestSchema, + importAccountsRequestSchema, deleteAccountRequestSchema, listAccountsRequestSchema, updateAccountRequestSchema, @@ -101,6 +102,19 @@ export function registerAppHandlers( } }); + ipcMain.handle(IPC_CHANNELS.importAccounts, async (event, rawRequest: unknown) => { + try { + assertTrustedSender(event, developmentUrl); + lockService.assertNotLocked(); + const request = importAccountsRequestSchema.parse(rawRequest); + return accountDtoSchema + .array() + .parse(await accountService.importAccounts(request, () => lockService.assertNotLocked())); + } catch { + throw new Error('Hesaplar içe aktarılamadı. Hiçbir hesap kaydedilmedi.'); + } + }); + ipcMain.handle(IPC_CHANNELS.updateAccount, async (event, rawRequest: unknown) => { try { assertTrustedSender(event, developmentUrl); @@ -149,10 +163,7 @@ export function registerAppHandlers( assertTrustedSender(event, developmentUrl); lockService.assertNotLocked(); const { accountId } = copyTotpRequestSchema.parse(rawRequest); - const value = (await accountService.getTotpCodes()).find( - (item) => item.accountId === accountId, - ); - if (!value) throw new Error('Account not found.'); + const value = await accountService.getTotpCode(accountId); clipboard.writeText(value.code); setTimeout(() => { if (clipboard.readText() === value.code) clipboard.clear(); diff --git a/src/main/services/AccountService.ts b/src/main/services/AccountService.ts index 39793b7..8394ebb 100644 --- a/src/main/services/AccountService.ts +++ b/src/main/services/AccountService.ts @@ -2,9 +2,15 @@ import { randomUUID } from 'node:crypto'; import type { AccountDto, CreateAccountRequest, + ImportAccountsRequest, UpdateAccountRequest, } from '@shared/schemas/account'; -import { accountDtoSchema } from '@shared/schemas/account'; +import { + accountDtoSchema, + createAccountRequestSchema, + importAccountsRequestSchema, + updateAccountRequestSchema, +} from '@shared/schemas/account'; import { AccountRepository, type AccountRecord } from '../database/repositories/AccountRepository'; import type { VaultService } from '../security/VaultService'; import type { TotpCode } from '@shared/schemas/totp'; @@ -39,23 +45,16 @@ export class AccountService { async getTotpCodes(): Promise { if (!this.totpService) throw new Error('TOTP service unavailable.'); - return Promise.all( - this.repository.list().map(async (account) => { - const decrypted = await this.vault.decryptSecret(account.encryptedSecret); - if (decrypted.reEncrypted) { - this.repository.updateEncryptedSecret(account.id, decrypted.reEncrypted); - } - return { - accountId: account.id, - ...this.totpService!.generate( - decrypted.secret, - account.algorithm, - account.digits, - account.period, - ), - }; - }), + const results = await Promise.allSettled( + this.repository.list().map((account) => this.getTotpCode(account.id)), ); + return results.flatMap((result) => (result.status === 'fulfilled' ? [result.value] : [])); + } + + async getTotpCode(id: string): Promise { + if (!this.totpService) throw new Error('TOTP service unavailable.'); + const { secret, algorithm, digits, period } = await this.getTotpSecret(id); + return { accountId: id, ...this.totpService.generate(secret, algorithm, digits, period) }; } async getTotpSecret(id: string): Promise<{ @@ -79,6 +78,7 @@ export class AccountService { } async create(input: CreateAccountRequest): Promise { + input = createAccountRequestSchema.parse(input); const now = Date.now(); const record: AccountRecord = { id: randomUUID(), @@ -98,7 +98,32 @@ export class AccountService { return toDto(record); } + async importAccounts( + input: ImportAccountsRequest, + assertCanWrite: () => void = () => {}, + ): Promise { + const { accounts } = importAccountsRequestSchema.parse(input); + const now = Date.now(); + const sortOrder = this.repository.list().length; + // Finish every asynchronous encryption before opening the synchronous SQLite transaction. + const records = await Promise.all( + accounts.map(async ({ secret, ...metadata }, index): Promise => ({ + ...metadata, + id: randomUUID(), + encryptedSecret: await this.vault.encryptSecret(secret), + sortOrder: sortOrder + index, + createdAt: now, + updatedAt: now, + })), + ); + const dtos = records.map(toDto); + assertCanWrite(); + this.repository.createMany(records); + return dtos; + } + async update(input: UpdateAccountRequest): Promise { + input = updateAccountRequestSchema.parse(input); const current = this.repository.getById(input.id); if (!current) throw new Error('Account not found.'); const updated: AccountRecord = { diff --git a/src/main/services/TotpService.ts b/src/main/services/TotpService.ts index 6efcea2..70700f5 100644 --- a/src/main/services/TotpService.ts +++ b/src/main/services/TotpService.ts @@ -1,6 +1,6 @@ import * as OTPAuth from 'otpauth'; import type { CreateAccountRequest } from '@shared/schemas/account'; -import { createAccountRequestSchema } from '@shared/schemas/account'; +import { base32SecretSchema, createAccountRequestSchema } from '@shared/schemas/account'; export interface TotpValue { code: string; @@ -16,7 +16,7 @@ export class TotpService { period: number, timestamp = Date.now(), ): TotpValue { - const normalized = secret.replace(/[\s-]/g, '').toUpperCase(); + const normalized = base32SecretSchema.parse(secret); const otpSecret = OTPAuth.Secret.fromBase32(normalized); return { code: OTPAuth.TOTP.generate({ secret: otpSecret, algorithm, digits, period, timestamp }), diff --git a/src/main/window/createMainWindow.ts b/src/main/window/createMainWindow.ts index 61a8cec..0cdb2d3 100644 --- a/src/main/window/createMainWindow.ts +++ b/src/main/window/createMainWindow.ts @@ -81,6 +81,44 @@ export async function createMainWindow(developmentUrl?: string): Promise { + const api = window.authapp; + const fixture = { + issuer: 'Smoke fixture', accountName: 'fixture', secret: 'JBSWY3DPEHPK3PXP', + algorithm: 'SHA1', digits: 6, period: 30, favorite: false, groupId: null, + }; + const imported = await api.importAccounts({ + accounts: [fixture, { ...fixture, accountName: 'second fixture' }], + }); + if (imported.length !== 2 || imported.some((item) => 'secret' in item || 'encryptedSecret' in item)) { + throw new Error('Account import contract failed.'); + } + const codes = await api.getTotpCodes(); + if (codes.length !== 2 || codes.some((item) => !/^\\d{6}$/.test(item.code))) { + throw new Error('Account generation failed.'); + } + let invalidRejected = false; + try { await api.createAccount({ ...fixture, secret: 'INVALID!' }); } + catch { invalidRejected = true; } + if (!invalidRejected) throw new Error('Invalid secret was accepted.'); + let rollbackRejected = false; + try { + await api.importAccounts({ accounts: [fixture, { + ...fixture, groupId: 'a4ff59c6-2df2-4136-86ba-78bd57a617d8', + }] }); + } catch { rollbackRejected = true; } + if (!rollbackRejected || (await api.listAccounts()).length !== 2) { + throw new Error('Atomic account import failed.'); + } + for (const account of imported) await api.deleteAccount({ id: account.id }); + const single = await api.createAccount(fixture); + await api.deleteAccount({ id: single.id }); + return (await api.listAccounts()).length === 0; + })() + `); + if (accountFlowReady !== true) throw new Error('Account flow smoke failed.'); + const screenshotDirectory = process.env['AUTHAPP_SMOKE_SCREENSHOT_DIR']; if (screenshotDirectory) { await mkdir(screenshotDirectory, { recursive: true }); diff --git a/src/preload/index.ts b/src/preload/index.ts index 0c44ffd..ee35a7e 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -9,9 +9,11 @@ import { IPC_CHANNELS, UPDATE_STATE_EVENT } from '@shared/ipc/channels'; import { accountDtoSchema, createAccountRequestSchema, + importAccountsRequestSchema, deleteAccountRequestSchema, updateAccountRequestSchema, type CreateAccountRequest, + type ImportAccountsRequest, type DeleteAccountRequest, type UpdateAccountRequest, } from '@shared/schemas/account'; @@ -58,6 +60,15 @@ const api: AuthappApi = Object.freeze({ createAccountRequestSchema.parse(request), ), ), + importAccounts: async (request: ImportAccountsRequest) => + accountDtoSchema + .array() + .parse( + await ipcRenderer.invoke( + IPC_CHANNELS.importAccounts, + importAccountsRequestSchema.parse(request), + ), + ), updateAccount: async (request: UpdateAccountRequest) => accountDtoSchema.parse( await ipcRenderer.invoke( diff --git a/src/renderer/src/pages/AddAccountPage.tsx b/src/renderer/src/pages/AddAccountPage.tsx index abc5efe..8505cca 100644 --- a/src/renderer/src/pages/AddAccountPage.tsx +++ b/src/renderer/src/pages/AddAccountPage.tsx @@ -13,6 +13,7 @@ import { useCallback, useEffect, useState, type FormEvent } from 'react'; import { Link, useNavigate } from 'react-router-dom'; import type { CreateAccountRequest } from '@shared/schemas/account'; +import { createAccountRequestSchema } from '@shared/schemas/account'; import { PageHeading } from '../components/PageHeading'; import { decodeQrFromImageFile, processScannedQrContent } from '../utils/qrDecoder'; import { useLanguage } from '../hooks/useLanguage'; @@ -167,15 +168,16 @@ export function AddAccountPage(): React.JSX.Element { setError('Lütfen bir hesap adı girin.'); return; } - if (!form.secret.trim()) { - setError('Lütfen geçerli bir kurulum anahtarı girin.'); + const parsed = createAccountRequestSchema.safeParse(form); + if (!parsed.success) { + setError(t('invalidAccountInput')); return; } setSaving(true); setError(''); try { - await window.authapp.createAccount(form); + await window.authapp.createAccount(parsed.data); navigate('/'); } catch { setError('Hesap kaydedilemedi. Kurulum anahtarını kontrol edin.'); @@ -194,12 +196,10 @@ export function AddAccountPage(): React.JSX.Element { setSaving(true); setError(''); try { - for (const account of selectedList) { - await window.authapp.createAccount(account); - } + await window.authapp.importAccounts({ accounts: selectedList }); navigate('/'); } catch { - setError('Hesaplar içe aktarılırken bir hata oluştu.'); + setError(t('accountImportFailed')); setSaving(false); } } diff --git a/src/renderer/src/pages/HomePage.tsx b/src/renderer/src/pages/HomePage.tsx index ef23316..b40752a 100644 --- a/src/renderer/src/pages/HomePage.tsx +++ b/src/renderer/src/pages/HomePage.tsx @@ -91,6 +91,7 @@ export function HomePage(): React.JSX.Element { ); } setError(true); + setCodes([]); } finally { setLoading(false); } @@ -126,7 +127,12 @@ export function HomePage(): React.JSX.Element { .then((nextCodes) => { if (isMounted) setCodes(nextCodes); }) - .catch(() => {}); + .catch(() => { + if (isMounted) { + setCodes([]); + setError(true); + } + }); } } lastSec = currentSec; @@ -207,7 +213,12 @@ export function HomePage(): React.JSX.Element { }; async function handleCopy(accountId: string): Promise { - await window.authapp.copyTotp(accountId); + try { + await window.authapp.copyTotp(accountId); + } catch { + setError(true); + return; + } setCopiedAccountId(accountId); window.setTimeout(() => { setCopiedAccountId((prev) => (prev === accountId ? null : prev)); @@ -304,10 +315,11 @@ export function HomePage(): React.JSX.Element {
- {value ? value.code.replace(/(.{3})/g, '$1 ').trim() : '------'} + {value ? value.code.replace(/(.{3})/g, '$1 ').trim() : t('codeUnavailable')} + )} +
+ + ); + if (!status.isLocked) return null; return (
diff --git a/src/renderer/src/pages/SettingsPage.tsx b/src/renderer/src/pages/SettingsPage.tsx index 572b6a0..c5a0428 100644 --- a/src/renderer/src/pages/SettingsPage.tsx +++ b/src/renderer/src/pages/SettingsPage.tsx @@ -144,13 +144,15 @@ export function SettingsPage(): React.JSX.Element { setStatusMsg(null); try { const result = await window.authapp.importBackup({ password: importPassword }); - if (result.success) { + if (result.success && result.skippedCount === 0) { setStatusMsg({ type: 'success', - message: `${result.importedCount} ${t('importSuccess')}`, + message: `${result.importedCount} ${t('importSuccess')}${result.ungroupedCount ? ` ${t('legacyGroupsRestored')}` : ''}`, }); setShowImportForm(false); setImportPassword(''); + } else if (result.skippedCount > 0) { + setStatusMsg({ type: 'error', message: t('backupIncomplete') }); } } catch (err) { setStatusMsg({ diff --git a/src/renderer/src/utils/translations.ts b/src/renderer/src/utils/translations.ts index 4f2b2bd..b3e8adc 100644 --- a/src/renderer/src/utils/translations.ts +++ b/src/renderer/src/utils/translations.ts @@ -110,6 +110,12 @@ export const translations = { // Lock Overlay lockTitle: 'Authly Kilitli', + lockStatusLoading: 'Kilit durumu kontrol ediliyor…', + lockStatusUnavailable: 'Kilit durumu alınamadı. Yeniden deneyin.', + legacyGroupsRestored: + 'Eski yedekteki grup bilgisi bulunamadığı için bazı hesaplar gruplandırılmadan geri yüklendi.', + backupIncomplete: + 'Yedek tamamen geri yüklenemedi. Atlanan hesaplar var; yedek dosyasını saklayın.', lockSubtitle: 'Devam etmek için Master PIN kodunuzu girin.', lockUnlock: 'Kilidi Aç', lockClear: 'Sil', @@ -222,6 +228,12 @@ export const translations = { // Lock Overlay lockTitle: 'Authly Locked', + lockStatusLoading: 'Checking lock status…', + lockStatusUnavailable: 'Lock status could not be checked. Try again.', + legacyGroupsRestored: + 'Some accounts were restored without a group because the older backup did not include group details.', + backupIncomplete: + 'The backup was not fully restored. Some accounts were skipped; keep the backup file.', lockSubtitle: 'Enter your Master PIN code to continue.', lockUnlock: 'Unlock', lockClear: 'Clear', diff --git a/src/shared/ipc/channels.ts b/src/shared/ipc/channels.ts index 2fba6c7..a82e0b2 100644 --- a/src/shared/ipc/channels.ts +++ b/src/shared/ipc/channels.ts @@ -17,9 +17,11 @@ export const IPC_CHANNELS = { setPin: 'lock:set-pin', removePin: 'lock:remove-pin', lockApp: 'lock:lock-app', + recordActivity: 'lock:activity', checkUpdate: 'app:check-update', getUpdateState: 'app:get-update-state', installUpdate: 'app:install-update', } as const; export const UPDATE_STATE_EVENT = 'app:update-state'; +export const LOCK_STATUS_EVENT = 'lock:status-changed'; diff --git a/src/shared/schemas/backup.ts b/src/shared/schemas/backup.ts index 0cc16c6..2782833 100644 --- a/src/shared/schemas/backup.ts +++ b/src/shared/schemas/backup.ts @@ -39,6 +39,7 @@ export const importBackupResultSchema = z success: z.boolean(), importedCount: z.number().int().nonnegative(), skippedCount: z.number().int().nonnegative(), + ungroupedCount: z.number().int().nonnegative().default(0), }) .strict(); @@ -53,7 +54,17 @@ export const backupAccountItemSchema = z.object({ groupId: z.string().uuid().nullable().default(null), }); -export const backupPayloadSchema = z +export const backupGroupSchema = z + .object({ + id: z.string().uuid(), + name: z.string().trim().min(1).max(120), + sortOrder: z.number().int().nonnegative(), + createdAt: z.number().int().positive(), + updatedAt: z.number().int().positive(), + }) + .strict(); + +const legacyBackupPayloadSchema = z .object({ version: z.literal(1), exportedAt: z.number().int().positive(), @@ -61,6 +72,16 @@ export const backupPayloadSchema = z }) .strict(); +export const backupPayloadSchema = z.discriminatedUnion('version', [ + legacyBackupPayloadSchema, + legacyBackupPayloadSchema + .extend({ + version: z.literal(2), + groups: z.array(backupGroupSchema).max(MAX_BACKUP_ACCOUNTS), + }) + .strict(), +]); + export const backupFileEnvelopeSchema = z .object({ format: z.literal('authapp-backup-v1'), diff --git a/src/shared/schemas/lock.ts b/src/shared/schemas/lock.ts index 4948fe0..eeb51c4 100644 --- a/src/shared/schemas/lock.ts +++ b/src/shared/schemas/lock.ts @@ -1,5 +1,7 @@ import { z } from 'zod'; +export const lockActivityRequestSchema = z.object({}).strict(); + export const lockStatusSchema = z .object({ isPinSet: z.boolean(), diff --git a/src/shared/types/electron-api.ts b/src/shared/types/electron-api.ts index b61dc50..5728f15 100644 --- a/src/shared/types/electron-api.ts +++ b/src/shared/types/electron-api.ts @@ -41,6 +41,8 @@ export interface AuthappApi { setPin: (request: SetPinRequest) => Promise; removePin: (request: RemovePinRequest) => Promise; lockApp: () => Promise; + recordActivity: () => Promise; + onLockStatus: (listener: (status: LockStatus) => void) => () => void; checkUpdate: () => Promise; getUpdateState: () => Promise; installUpdate: () => Promise; diff --git a/tests/renderer/App.test.tsx b/tests/renderer/App.test.tsx index ea0f66b..5c66eab 100644 --- a/tests/renderer/App.test.tsx +++ b/tests/renderer/App.test.tsx @@ -19,7 +19,7 @@ describe('App shell and user interface', () => { it('uses the packaged Authly logo in the application header', async () => { renderApp(); - const brandButton = screen.getByRole('button', { name: 'Authly Ana Sayfası' }); + const brandButton = await screen.findByRole('button', { name: 'Authly Ana Sayfası' }); expect(brandButton.querySelector('img')).toHaveAttribute( 'src', expect.stringContaining('logo.png'), @@ -55,7 +55,7 @@ describe('App shell and user interface', () => { , ); - await user.click(screen.getByRole('button', { name: 'Koyu' })); + await user.click(await screen.findByRole('button', { name: 'Koyu' })); expect(document.documentElement).toHaveAttribute('data-theme', 'dark'); expect(screen.getByRole('button', { name: 'Koyu' })).toHaveAttribute('aria-pressed', 'true'); }); diff --git a/tests/renderer/lockFlows.test.tsx b/tests/renderer/lockFlows.test.tsx new file mode 100644 index 0000000..694cbb4 --- /dev/null +++ b/tests/renderer/lockFlows.test.tsx @@ -0,0 +1,97 @@ +import { act, render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MemoryRouter } from 'react-router-dom'; +import { beforeEach, expect, it, vi } from 'vitest'; +import { App } from '../../src/renderer/src/app/App'; +import { ThemeProvider } from '../../src/renderer/src/app/ThemeProvider'; +import type { LockStatus } from '../../src/shared/schemas/lock'; + +let listener: (status: LockStatus) => void; +const account = { + id: 'a4ff59c6-2df2-4136-86ba-78bd57a617d8', + issuer: 'Existing fixture account', + accountName: 'fixture', + algorithm: 'SHA1' as const, + digits: 6, + period: 30, + favorite: false, + groupId: null, + sortOrder: 0, + createdAt: 1, + updatedAt: 1, +}; +const unlocked = { isLocked: false, isPinSet: true, autoLockTimeout: 5 }; + +beforeEach(() => { + vi.mocked(window.authapp.onLockStatus).mockImplementation((callback) => { + listener = callback; + return () => {}; + }); + vi.mocked(window.authapp.getLockStatus).mockResolvedValue(unlocked); + vi.mocked(window.authapp.listAccounts).mockResolvedValue([account]); + vi.mocked(window.authapp.getTotpCodes).mockResolvedValue([ + { accountId: account.id, code: '000000', remaining: 15, period: 30 }, + ]); +}); + +function renderApp() { + return render( + + + + + , + ); +} + +it('loads accounts immediately after PIN unlock without changing window focus', async () => { + let locked = true; + vi.mocked(window.authapp.getLockStatus).mockImplementation(async () => ({ + ...unlocked, + isLocked: locked, + })); + vi.mocked(window.authapp.verifyPin).mockImplementation(async () => { + locked = false; + return true; + }); + const user = userEvent.setup(); + renderApp(); + expect(await screen.findByRole('dialog', { name: 'Authly Kilitli' })).toBeVisible(); + expect(window.authapp.listAccounts).not.toHaveBeenCalled(); + await user.keyboard('1234{Enter}'); + expect(await screen.findByText(account.issuer)).toBeVisible(); + expect(screen.queryByRole('dialog', { name: 'Authly Kilitli' })).not.toBeInTheDocument(); +}); + +it('unmounts sensitive account and form state on lock and reloads on the next unlock', async () => { + renderApp(); + expect(await screen.findByText(account.issuer)).toBeVisible(); + act(() => listener({ ...unlocked, isLocked: true })); + expect(screen.queryByText(account.issuer)).not.toBeInTheDocument(); + expect(screen.getByRole('dialog', { name: 'Authly Kilitli' })).toBeVisible(); + act(() => listener(unlocked)); + expect(await screen.findByText(account.issuer)).toBeVisible(); + expect(window.authapp.listAccounts).toHaveBeenCalledTimes(2); +}); + +it('fails closed when the initial lock status cannot be obtained', async () => { + vi.mocked(window.authapp.getLockStatus).mockRejectedValue(new Error('Fixture unavailable')); + renderApp(); + expect(await screen.findByText('Kilit durumu alınamadı. Yeniden deneyin.')).toBeVisible(); + expect(window.authapp.listAccounts).not.toHaveBeenCalled(); + expect(screen.queryByText(account.issuer)).not.toBeInTheDocument(); +}); + +it('ignores an initial unlocked snapshot that arrives after a newer locked event', async () => { + let finish!: (status: LockStatus) => void; + vi.mocked(window.authapp.getLockStatus).mockReturnValue( + new Promise((resolve) => { + finish = resolve; + }), + ); + renderApp(); + act(() => listener({ ...unlocked, isLocked: true })); + await act(async () => finish(unlocked)); + await waitFor(() => expect(screen.getByRole('dialog', { name: 'Authly Kilitli' })).toBeVisible()); + expect(window.authapp.listAccounts).not.toHaveBeenCalled(); +}); diff --git a/tests/setup.ts b/tests/setup.ts index 38d6f85..b6246bf 100644 --- a/tests/setup.ts +++ b/tests/setup.ts @@ -66,6 +66,8 @@ Object.defineProperty(window, 'authapp', { setPin: vi.fn().mockResolvedValue(undefined), removePin: vi.fn().mockResolvedValue(undefined), lockApp: vi.fn().mockResolvedValue(undefined), + recordActivity: vi.fn().mockResolvedValue(undefined), + onLockStatus: vi.fn().mockReturnValue(() => {}), checkUpdate: vi.fn().mockResolvedValue({ hasUpdate: false, currentVersion: '0.3.5', diff --git a/tests/unit/accountStorage.test.ts b/tests/unit/accountStorage.test.ts index dbbbd2f..97c3c6c 100644 --- a/tests/unit/accountStorage.test.ts +++ b/tests/unit/accountStorage.test.ts @@ -29,6 +29,31 @@ const input = { }; describe('local encrypted account storage', () => { + it('checks the captured session after async encryption and before single-account writes', async () => { + const database = new LocalDatabase(':memory:'); + try { + let finish!: (value: Buffer) => void; + const vault = new TestVault(); + const service = new AccountService(new AccountRepository(database.connection), { + encryptSecret: () => + new Promise((resolve) => { + finish = resolve; + }), + decryptSecret: (value) => vault.decryptSecret(value), + }); + let allowed = true; + const pending = service.create(input, () => { + if (!allowed) throw new Error('Session revoked'); + }); + allowed = false; + finish(await vault.encryptSecret(input.secret)); + await expect(pending).rejects.toThrow('Session revoked'); + expect(service.list()).toHaveLength(0); + } finally { + database.close(); + } + }); + it('rejects malformed secrets on create and update without changing storage', async () => { const database = new LocalDatabase(':memory:'); try { diff --git a/tests/unit/backupRecovery.test.ts b/tests/unit/backupRecovery.test.ts new file mode 100644 index 0000000..dad50fd --- /dev/null +++ b/tests/unit/backupRecovery.test.ts @@ -0,0 +1,210 @@ +import { createCipheriv, pbkdf2Sync, randomBytes } from 'node:crypto'; +import { afterEach, beforeEach, expect, it, vi } from 'vitest'; +import { LocalDatabase } from '../../src/main/database/LocalDatabase'; +import { AccountRepository } from '../../src/main/database/repositories/AccountRepository'; +import { GroupRepository } from '../../src/main/database/repositories/GroupRepository'; +import { AccountService } from '../../src/main/services/AccountService'; +import { BackupService } from '../../src/main/services/BackupService'; +import type { VaultService } from '../../src/main/security/VaultService'; +import { dialog } from 'electron'; +import * as fs from 'node:fs'; + +vi.mock('electron', () => ({ dialog: { showOpenDialog: vi.fn(), showSaveDialog: vi.fn() } })); +vi.mock('node:fs', async (original) => ({ + ...(await original()), + writeFileSync: vi.fn(), + readFileSync: vi.fn(), + statSync: vi.fn(), +})); + +const databases: LocalDatabase[] = []; +const password = 'synthetic-password'; +const group = { + id: 'a4ff59c6-2df2-4136-86ba-78bd57a617d8', + name: 'Fixture group', + sortOrder: 0, + createdAt: 1, + updatedAt: 1, +}; +const account = { + issuer: 'Fixture', + accountName: 'fixture', + secret: 'JBSWY3DPEHPK3PXP', + algorithm: 'SHA1' as const, + digits: 6, + period: 30, + favorite: false, + groupId: null, +}; +const vault: VaultService = { + encryptSecret: async (value) => Buffer.from([...Buffer.from(value)].map((byte) => byte ^ 0xaa)), + decryptSecret: async (value) => ({ + secret: Buffer.from([...value].map((byte) => byte ^ 0xaa)).toString(), + }), +}; + +function fixture(storage = vault) { + const database = new LocalDatabase(':memory:'); + databases.push(database); + const groups = new GroupRepository(database.connection); + const accounts = new AccountService( + new AccountRepository(database.connection), + storage, + undefined, + groups, + ); + return { database, groups, accounts, backup: new BackupService(accounts) }; +} + +function provideFile(content: string) { + vi.mocked(fs.statSync).mockReturnValue({ size: Buffer.byteLength(content) } as fs.Stats); + vi.mocked(fs.readFileSync).mockReturnValue(content); +} + +function providePayload(payload: unknown) { + const salt = randomBytes(16), + iv = randomBytes(12); + const key = pbkdf2Sync(password, salt, 100_000, 32, 'sha256'); + const cipher = createCipheriv('aes-256-gcm', key, iv); + const encrypted = Buffer.concat([cipher.update(JSON.stringify(payload)), cipher.final()]); + provideFile( + JSON.stringify({ + format: 'authapp-backup-v1', + kdf: { algorithm: 'pbkdf2', iterations: 100_000, hash: 'sha256', salt: salt.toString('hex') }, + cipher: { + algorithm: 'aes-256-gcm', + iv: iv.toString('hex'), + authTag: cipher.getAuthTag().toString('hex'), + }, + encryptedData: encrypted.toString('hex'), + }), + ); +} + +beforeEach(() => { + vi.mocked(dialog.showOpenDialog).mockResolvedValue({ + canceled: false, + filePaths: ['fixture.authapp'], + }); + vi.mocked(dialog.showSaveDialog).mockResolvedValue({ + canceled: false, + filePath: 'fixture.authapp', + }); +}); +afterEach(() => { + for (const database of databases.splice(0)) database.close(); +}); + +it('round-trips encrypted grouped and ungrouped accounts onto a fresh database', async () => { + const source = fixture(), + target = fixture(); + source.groups.create(group); + await source.accounts.create({ ...account, groupId: group.id }); + await source.accounts.create({ ...account, accountName: 'ungrouped' }); + let encryptedFile = ''; + vi.mocked(fs.writeFileSync).mockImplementation((_path, content) => { + encryptedFile = String(content); + }); + await source.backup.exportBackup(password); + provideFile(encryptedFile); + expect(await target.backup.importBackup(password)).toEqual({ + success: true, + importedCount: 2, + skippedCount: 0, + ungroupedCount: 0, + }); + const restoredGroup = target.groups.list()[0]!; + expect(restoredGroup.name).toBe(group.name); + expect(target.accounts.list().find((item) => item.accountName === 'fixture')?.groupId).toBe( + restoredGroup.id, + ); +}); + +it('restores every legacy v1 account and reports missing group metadata instead of skipping it', async () => { + const target = fixture(); + providePayload({ + version: 1, + exportedAt: 1, + accounts: [ + { ...account, groupId: group.id }, + { ...account, accountName: 'second' }, + ], + }); + expect(await target.backup.importBackup(password)).toEqual({ + success: true, + importedCount: 2, + skippedCount: 0, + ungroupedCount: 1, + }); + expect(target.accounts.list()).toHaveLength(2); + expect(target.accounts.list().every((item) => item.groupId === null)).toBe(true); +}); + +it('rolls back groups and earlier accounts when a later account insert fails', async () => { + const target = fixture(); + target.database.connection.exec( + "CREATE TRIGGER reject_fixture BEFORE INSERT ON accounts WHEN NEW.account_name = 'fail' BEGIN SELECT RAISE(ABORT, 'fixture'); END", + ); + providePayload({ + version: 2, + exportedAt: 1, + groups: [group], + accounts: [ + { ...account, groupId: group.id }, + { ...account, accountName: 'fail' }, + ], + }); + await expect(target.backup.importBackup(password)).rejects.toThrow(); + expect(target.accounts.list()).toHaveLength(0); + expect(target.groups.list()).toHaveLength(0); +}); + +it('does not partially restore when encryption is unavailable', async () => { + let attempts = 0; + const target = fixture({ + ...vault, + encryptSecret: async (value) => { + if (++attempts === 2) throw new Error('Fixture encryption unavailable'); + return vault.encryptSecret(value); + }, + }); + providePayload({ + version: 2, + exportedAt: 1, + groups: [group], + accounts: [{ ...account, groupId: group.id }, account], + }); + await expect(target.backup.importBackup(password)).rejects.toThrow(); + expect(target.accounts.list()).toHaveLength(0); + expect(target.groups.list()).toHaveLength(0); +}); + +it('rejects incomplete v2 group references before any database write', async () => { + const target = fixture(); + providePayload({ + version: 2, + exportedAt: 1, + groups: [], + accounts: [{ ...account, groupId: group.id }], + }); + await expect(target.backup.importBackup(password)).rejects.toThrow('eksik grup'); + expect(target.accounts.list()).toHaveLength(0); +}); + +it('rechecks access after a native dialog and never reads or writes backup data while locked', async () => { + const target = fixture(); + let finish!: (value: Electron.SaveDialogReturnValue) => void; + vi.mocked(dialog.showSaveDialog).mockReturnValue( + new Promise((resolve) => { + finish = resolve; + }), + ); + let locked = false; + const result = target.backup.exportBackup(password, () => { + if (locked) throw new Error('Locked'); + }); + locked = true; + finish({ canceled: false, filePath: 'fixture.authapp' }); + await expect(result).rejects.toThrow('Locked'); + expect(fs.writeFileSync).not.toHaveBeenCalled(); +}); diff --git a/tests/unit/backupService.test.ts b/tests/unit/backupService.test.ts index 6efe7a7..5a0cefb 100644 --- a/tests/unit/backupService.test.ts +++ b/tests/unit/backupService.test.ts @@ -36,6 +36,7 @@ describe('BackupService', () => { groupId: null, }, ]), + listGroups: vi.fn().mockReturnValue([]), getTotpSecret: vi.fn().mockResolvedValue({ secret: 'JBSWY3DPEHPK3PXP', algorithm: 'SHA1', diff --git a/tests/unit/ipcLockGuards.test.ts b/tests/unit/ipcLockGuards.test.ts index b45f7b1..5e03ad5 100644 --- a/tests/unit/ipcLockGuards.test.ts +++ b/tests/unit/ipcLockGuards.test.ts @@ -1,6 +1,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { IPC_CHANNELS } from '../../src/shared/ipc/channels'; import { clipboard } from 'electron'; +import { LockService } from '../../src/main/services/LockService'; +import type { SettingsRepository } from '../../src/main/database/repositories/SettingsRepository'; type InvokeHandler = ( event: { senderFrame?: { url: string } }, @@ -33,7 +35,57 @@ vi.mock('electron', () => ({ import { registerAppHandlers, removeAppHandlers } from '../../src/main/ipc/registerAppHandlers'; +function lockMock(assertNotLocked = vi.fn()) { + return { + assertNotLocked, + captureSession: () => { + assertNotLocked(); + return assertNotLocked; + }, + }; +} + describe('sensitive IPC lock guards', () => { + it.each([IPC_CHANNELS.copyTotp, IPC_CHANNELS.getTotpCodes])( + 'revokes pending %s work when the lock session changes', + async (channel) => { + const store = new Map(); + const repository = { + get: (key: string) => store.get(key) ?? null, + set: (key: string, value: string) => store.set(key, value), + delete: (key: string) => store.delete(key), + } as unknown as SettingsRepository; + const lock = new LockService(repository); + lock.setPin('1234'); + let finish!: (value: unknown) => void; + const pending = new Promise((resolve) => { + finish = resolve; + }); + const services = [ + { getTotpCode: () => pending, getTotpCodes: () => pending }, + {}, + {}, + {}, + lock, + {}, + ] as unknown as Parameters; + registerAppHandlers(...services); + const request = + channel === IPC_CHANNELS.copyTotp + ? { accountId: 'a4ff59c6-2df2-4136-86ba-78bd57a617d8' } + : {}; + const result = handlers.get(channel)!( + { senderFrame: { url: 'authapp://app/index.html' } }, + request, + ); + lock.lockApp(); + lock.verifyPin('1234'); + finish(channel === IPC_CHANNELS.copyTotp ? { code: '000000' } : []); + await expect(result).rejects.toThrow(); + expect(clipboard.writeText).not.toHaveBeenCalled(); + }, + ); + const importRequest = { accounts: [{ issuer: 'Fixture', accountName: 'fixture', secret: 'JBSWY3DPEHPK3PXP' }], }; @@ -51,7 +103,7 @@ describe('sensitive IPC lock guards', () => { {}, {}, {}, - { assertNotLocked }, + lockMock(assertNotLocked), {}, ] as unknown as Parameters; registerAppHandlers(...services); @@ -82,7 +134,7 @@ describe('sensitive IPC lock guards', () => { {}, {}, {}, - { assertNotLocked }, + lockMock(assertNotLocked), {}, ] as unknown as Parameters; registerAppHandlers(...services); @@ -109,7 +161,7 @@ describe('sensitive IPC lock guards', () => { {}, {}, {}, - { assertNotLocked: vi.fn() }, + lockMock(), {}, ] as unknown as Parameters; registerAppHandlers(...services); @@ -135,7 +187,7 @@ describe('sensitive IPC lock guards', () => { {}, { exportBackup: vi.fn(), importBackup: vi.fn() }, {}, - { assertNotLocked }, + lockMock(assertNotLocked), {}, ] as unknown as Parameters; diff --git a/tests/unit/lockService.test.ts b/tests/unit/lockService.test.ts index 8a9eb98..9595a72 100644 --- a/tests/unit/lockService.test.ts +++ b/tests/unit/lockService.test.ts @@ -3,6 +3,86 @@ import { LockService } from '../../src/main/services/LockService'; import type { SettingsRepository } from '../../src/main/database/repositories/SettingsRepository'; describe('LockService', () => { + function fixture() { + const store = new Map(); + const repository = { + get: (key: string) => store.get(key) ?? null, + set: (key: string, value: string) => store.set(key, value), + delete: (key: string) => store.delete(key), + } as unknown as SettingsRepository; + let now = 1000; + let systemIdle = 0; + const service = new LockService( + repository, + () => now, + () => systemIdle, + ); + return { + service, + store, + advance: (ms: number) => { + now += ms; + }, + idle: (seconds: number) => { + systemIdle = seconds; + }, + }; + } + + it('enforces inactivity in main and reports a single lock transition', () => { + const f = fixture(); + f.service.setPin('1234'); + const changes: boolean[] = []; + f.service.onStatusChange((status) => changes.push(status.isLocked)); + f.advance(299_999); + expect(f.service.getStatus().isLocked).toBe(false); + f.advance(1); + f.service.checkAutoLock(); + f.service.checkAutoLock(); + expect(changes).toEqual([true]); + expect(() => f.service.assertNotLocked()).toThrow(); + }); + + it('extends a live session on activity, but never unlocks an expired session with activity', () => { + const f = fixture(); + f.service.setPin('1234'); + f.advance(240_000); + f.service.recordActivity(); + f.advance(240_000); + expect(f.service.getStatus().isLocked).toBe(false); + f.advance(60_000); + expect(() => f.service.recordActivity()).toThrow(); + expect(f.service.getStatus().isLocked).toBe(true); + }); + + it('rejects pending work even if the user has unlocked again', () => { + const f = fixture(); + f.service.setPin('1234'); + const guard = f.service.captureSession(); + f.service.lockApp(); + f.service.verifyPin('1234'); + expect(() => guard()).toThrow('oturumu değişti'); + expect(() => f.service.captureSession()()).not.toThrow(); + }); + + it('uses OS idle time so synthetic renderer activity cannot keep an idle machine unlocked', () => { + const f = fixture(); + f.service.setPin('1234'); + f.idle(300); + expect(() => f.service.recordActivity()).toThrow(); + expect(f.service.getStatus().isLocked).toBe(true); + }); + + it('honors disabled inactivity locking and leaves accounts without a PIN available', () => { + const f = fixture(); + f.advance(600_000); + expect(f.service.getStatus().isLocked).toBe(false); + f.service.setPin('1234'); + f.store.set('autoLockTimeout', '0'); + f.advance(600_000); + expect(f.service.getStatus().isLocked).toBe(false); + }); + it('starts unlocked when no PIN is set in repository', () => { const store = new Map(); const mockRepo = { From b414121c8c2188d335e5e7969675f41970ce1186 Mon Sep 17 00:00:00 2001 From: ardakrt <81361836+ardakrt@users.noreply.github.com> Date: Thu, 29 Oct 2026 00:12:05 +0300 Subject: [PATCH 3/3] test: isolate smoke lock checks from unattended OS idle time --- src/main/index.ts | 3 ++- src/main/window/createMainWindow.ts | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/index.ts b/src/main/index.ts index b53d953..c443cd7 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -69,7 +69,8 @@ app.whenReady().then(async () => { const lockService = new LockService( new SettingsRepository(localDatabase.connection), Date.now, - () => powerMonitor.getSystemIdleTime(), + // Isolated smoke has no physical user input; OS-idle behavior is covered by service tests. + () => (isSmokeTest ? 0 : powerMonitor.getSystemIdleTime()), ); lockService.onStatusChange((status) => { for (const window of BrowserWindow.getAllWindows()) { diff --git a/src/main/window/createMainWindow.ts b/src/main/window/createMainWindow.ts index c0e9dd8..4e46c67 100644 --- a/src/main/window/createMainWindow.ts +++ b/src/main/window/createMainWindow.ts @@ -118,6 +118,7 @@ export async function createMainWindow(developmentUrl?: string): Promise setTimeout(resolve, 25));