mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-20 05:56:53 +00:00
1a0834b866
Searching worked for the admin and 403'd for every other member. getMapsKey read the calling user's own row first and then fell through to "any admin" via LIMIT 1 with no ORDER BY, so the admin who pasted the key hit their own row while everyone else got a non-deterministic stranger's. That fallback is what CLAUDE.md forbids. The Places and Unsplash keys bill the install, not the account that pasted them in, so they become instance configuration: an encrypted app_settings row, resolved as env -> instance -> the caller's own row, the same shape #1772 chose for the LLM endpoint. The per-user column stays as the last step because /me/api-keys has always accepted a personal key. The trailing migration carries an existing install's key into the instance row so nothing degrades to OpenStreetMap on upgrade, but only where that key already was the whole install's: as soon as a second row holds one, it is left alone, because the instance row outranks everybody's own column and an upgrade must not move a member onto someone else's key and bill. Two things from the same report: The key test button probed with field mask places.displayName while the search asks for places.rating and more, which is a separate SKU and can be restricted on its own, so the button could pass while every search failed. It now probes the resolved key with the search's own mask. Saving API keys wrote no audit line. It now writes one settings.api_keys_update row naming which fields changed, never their values, comparing cleartext because the random IV makes every ciphertext look different. Closes #1939
200 lines
9.1 KiB
TypeScript
200 lines
9.1 KiB
TypeScript
/**
|
||
* Unit tests for the DI-native audit domain — AUDIT-SVC-001 through
|
||
* AUDIT-SVC-017 (001–007 are the getClientIp cases moved 1:1 from the legacy
|
||
* tests/unit/services/auditLog.test.ts, which had no case IDs — the IDs are
|
||
* introduced with the move; 008–014 cover writeAudit over a real in-memory
|
||
* SQLite DB, which the legacy suite never exercised; 015–017 pin the
|
||
* audit.bridge delegation). The logger module is mocked (it replaces the old
|
||
* suite's fs mock: the import-time mkdir lives there now, and mocking it lets
|
||
* the exact log-line formats be asserted).
|
||
*/
|
||
import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest';
|
||
|
||
// ── DB setup ──────────────────────────────────────────────────────────────────
|
||
|
||
const { testDb, dbMock } = vi.hoisted(() => {
|
||
const Database = require('better-sqlite3');
|
||
const db = new Database(':memory:');
|
||
db.exec('PRAGMA journal_mode = WAL');
|
||
db.exec('PRAGMA foreign_keys = ON');
|
||
db.exec('PRAGMA busy_timeout = 5000');
|
||
const mock = {
|
||
db,
|
||
closeDb: () => {},
|
||
reinitialize: () => {},
|
||
};
|
||
return { testDb: db, dbMock: mock };
|
||
});
|
||
|
||
vi.mock('../../../src/db/database', () => dbMock);
|
||
vi.mock('../../../src/nest/audit/audit-log.logger', () => ({
|
||
LOG_LEVEL: 'error',
|
||
logInfo: vi.fn(),
|
||
logDebug: vi.fn(),
|
||
logError: vi.fn(),
|
||
logWarn: vi.fn(),
|
||
}));
|
||
|
||
import type { Request } from 'express';
|
||
import { createTables } from '../../../src/db/schema';
|
||
import { runMigrations } from '../../../src/db/migrations';
|
||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||
import { AuditService } from '../../../src/nest/audit/audit.service';
|
||
import { getClientIp } from '../../../src/nest/audit/client-ip';
|
||
import { logInfo, logDebug, logError } from '../../../src/nest/audit/audit-log.logger';
|
||
|
||
const svc = new AuditService(new DatabaseService(testDb));
|
||
|
||
beforeAll(() => {
|
||
createTables(testDb);
|
||
runMigrations(testDb);
|
||
});
|
||
|
||
beforeEach(() => {
|
||
vi.clearAllMocks();
|
||
testDb.prepare('DELETE FROM audit_log').run();
|
||
testDb.prepare('DELETE FROM users').run();
|
||
});
|
||
|
||
afterAll(() => {
|
||
testDb.close();
|
||
});
|
||
|
||
function makeReq(options: {
|
||
xff?: string | string[];
|
||
remoteAddress?: string;
|
||
} = {}): Request {
|
||
return {
|
||
headers: {
|
||
...(options.xff !== undefined ? { 'x-forwarded-for': options.xff } : {}),
|
||
},
|
||
socket: { remoteAddress: options.remoteAddress ?? undefined },
|
||
} as unknown as Request;
|
||
}
|
||
|
||
// ── getClientIp (pure, moved 1:1) ─────────────────────────────────────────────
|
||
|
||
describe('getClientIp', () => {
|
||
it('AUDIT-SVC-001: returns first IP from comma-separated X-Forwarded-For string', () => {
|
||
expect(getClientIp(makeReq({ xff: '1.2.3.4, 5.6.7.8, 9.10.11.12' }))).toBe('1.2.3.4');
|
||
});
|
||
|
||
it('AUDIT-SVC-002: returns single IP when X-Forwarded-For has no comma', () => {
|
||
expect(getClientIp(makeReq({ xff: '10.0.0.1' }))).toBe('10.0.0.1');
|
||
});
|
||
|
||
it('AUDIT-SVC-003: returns first element when X-Forwarded-For is an array', () => {
|
||
expect(getClientIp(makeReq({ xff: ['203.0.113.1', '10.0.0.1'] }))).toBe('203.0.113.1');
|
||
});
|
||
|
||
it('AUDIT-SVC-004: trims whitespace from extracted IP', () => {
|
||
expect(getClientIp(makeReq({ xff: ' 192.168.1.1 , 10.0.0.1' }))).toBe('192.168.1.1');
|
||
});
|
||
|
||
it('AUDIT-SVC-005: falls back to req.socket.remoteAddress when no X-Forwarded-For', () => {
|
||
expect(getClientIp(makeReq({ remoteAddress: '172.16.0.1' }))).toBe('172.16.0.1');
|
||
});
|
||
|
||
it('AUDIT-SVC-006: returns null when no forwarded header and no socket address', () => {
|
||
expect(getClientIp(makeReq({}))).toBeNull();
|
||
});
|
||
|
||
it('AUDIT-SVC-007: returns null for empty string X-Forwarded-For', () => {
|
||
const req = {
|
||
headers: { 'x-forwarded-for': '' },
|
||
socket: { remoteAddress: undefined },
|
||
} as unknown as Request;
|
||
expect(getClientIp(req)).toBeNull();
|
||
});
|
||
});
|
||
|
||
// ── writeAudit (real DB) ──────────────────────────────────────────────────────
|
||
|
||
function seedUser(id: number, email: string): void {
|
||
testDb.prepare(
|
||
"INSERT INTO users (id, username, email, password_hash, role) VALUES (?, ?, ?, 'x', 'user')"
|
||
).run(id, `u${id}`, email);
|
||
}
|
||
|
||
describe('writeAudit', () => {
|
||
it('AUDIT-SVC-008: inserts the row and logs the labeled summary line', () => {
|
||
seedUser(1, 'a@b.c');
|
||
svc.writeAudit({ userId: 1, action: 'trip.create', resource: 'trip', details: { title: 'Rome' }, ip: '1.2.3.4' });
|
||
const row = testDb.prepare('SELECT user_id, action, resource, details, ip FROM audit_log').get();
|
||
expect(row).toEqual({ user_id: 1, action: 'trip.create', resource: 'trip', details: '{"title":"Rome"}', ip: '1.2.3.4' });
|
||
expect(logInfo).toHaveBeenCalledWith('a@b.c created trip "Rome" ip=1.2.3.4');
|
||
});
|
||
|
||
it('AUDIT-SVC-009: empty details object stores NULL details and skips the debug fallback', () => {
|
||
seedUser(1, 'a@b.c');
|
||
svc.writeAudit({ userId: 1, action: 'user.login', details: {}, ip: '1.2.3.4' });
|
||
const row = testDb.prepare('SELECT details FROM audit_log').get() as { details: string | null };
|
||
expect(row.details).toBeNull();
|
||
expect(logDebug).not.toHaveBeenCalled();
|
||
});
|
||
|
||
it('AUDIT-SVC-010: unknown action keeps the raw key; empty-email/zero/null userIds resolve', () => {
|
||
seedUser(1, 'a@b.c');
|
||
svc.writeAudit({ userId: 1, action: 'custom.thing', ip: '9.9.9.9' });
|
||
expect(logInfo).toHaveBeenLastCalledWith('a@b.c custom.thing ip=9.9.9.9');
|
||
seedUser(42, ''); // falsy email → the `row?.email || uid:N` fallback
|
||
svc.writeAudit({ userId: 42, action: 'user.login', ip: '9.9.9.9' });
|
||
expect(logInfo).toHaveBeenLastCalledWith('uid:42 logged in ip=9.9.9.9');
|
||
seedUser(0, 'zero@b.c'); // since the quirk fix, a real id 0 resolves via the DB
|
||
svc.writeAudit({ userId: 0, action: 'user.login', ip: '9.9.9.9' });
|
||
expect(logInfo).toHaveBeenLastCalledWith('zero@b.c logged in ip=9.9.9.9');
|
||
svc.writeAudit({ userId: null, action: 'user.login', ip: '9.9.9.9' });
|
||
expect(logInfo).toHaveBeenLastCalledWith('anonymous logged in ip=9.9.9.9');
|
||
});
|
||
|
||
it('AUDIT-SVC-011: omitted resource/ip store NULL and the log line ends ip=-', () => {
|
||
seedUser(1, 'a@b.c');
|
||
svc.writeAudit({ userId: 1, action: 'user.login' });
|
||
const row = testDb.prepare('SELECT resource, ip FROM audit_log').get();
|
||
expect(row).toEqual({ resource: null, ip: null });
|
||
expect(logInfo).toHaveBeenCalledWith('a@b.c logged in ip=-');
|
||
});
|
||
|
||
it('AUDIT-SVC-012: debugDetails wins the debug line; detailsJson is the fallback', () => {
|
||
seedUser(1, 'a@b.c');
|
||
svc.writeAudit({ userId: 1, action: 'settings.app_update', details: { require_mfa: true }, debugDetails: { raw: 1 } });
|
||
expect(logDebug).toHaveBeenLastCalledWith('AUDIT settings.app_update userId=1 {"raw":1}');
|
||
svc.writeAudit({ userId: 1, action: 'settings.app_update', details: { require_mfa: true } });
|
||
expect(logDebug).toHaveBeenLastCalledWith('AUDIT settings.app_update userId=1 {"require_mfa":true}');
|
||
});
|
||
|
||
it('AUDIT-SVC-013: never throws — a failed insert reduces to a logError line', () => {
|
||
testDb.prepare('ALTER TABLE audit_log RENAME TO audit_log_gone').run();
|
||
expect(() => svc.writeAudit({ userId: 1, action: 'user.login' })).not.toThrow();
|
||
expect(logError).toHaveBeenCalledWith(expect.stringMatching(/^Audit write failed: /));
|
||
testDb.prepare('ALTER TABLE audit_log_gone RENAME TO audit_log').run();
|
||
});
|
||
|
||
it('AUDIT-SVC-018: settings.api_keys_update names the changed keys and nothing else (#1939)', () => {
|
||
seedUser(1, 'admin@b.c');
|
||
svc.writeAudit({
|
||
userId: 1,
|
||
action: 'settings.api_keys_update',
|
||
resource: 'api_keys',
|
||
details: { changed: ['maps_api_key', 'unsplash_api_key'] },
|
||
ip: '1.2.3.4',
|
||
});
|
||
expect(logInfo).toHaveBeenLastCalledWith('admin@b.c updated API keys (maps_api_key, unsplash_api_key) ip=1.2.3.4');
|
||
// A details blob without the array (or with an empty one) leaves the brief off
|
||
// rather than stringifying whatever else is in there.
|
||
svc.writeAudit({ userId: 1, action: 'settings.api_keys_update', details: { changed: [] }, ip: '1.2.3.4' });
|
||
expect(logInfo).toHaveBeenLastCalledWith('admin@b.c updated API keys ip=1.2.3.4');
|
||
svc.writeAudit({ userId: 1, action: 'settings.api_keys_update', details: { other: 'x' }, ip: '1.2.3.4' });
|
||
expect(logInfo).toHaveBeenLastCalledWith('admin@b.c updated API keys ip=1.2.3.4');
|
||
});
|
||
|
||
it('AUDIT-SVC-014: buildInfoSummary variants (settings parts, login empty brief)', () => {
|
||
seedUser(1, 'a@b.c');
|
||
svc.writeAudit({ userId: 1, action: 'settings.app_update', details: { notification_channel: 'smtp', require_mfa: false }, ip: '1.1.1.1' });
|
||
expect(logInfo).toHaveBeenLastCalledWith('a@b.c updated settings (channel=smtp, mfa=false) ip=1.1.1.1');
|
||
svc.writeAudit({ userId: 1, action: 'user.login', details: { anything: true }, ip: '1.1.1.1' });
|
||
expect(logInfo).toHaveBeenLastCalledWith('a@b.c logged in ip=1.1.1.1');
|
||
});
|
||
});
|
||
|