Clean up dead code, dedupe helpers, fix the reset-password contract

- Remove server exports orphaned by the Express removal: the immich
  album-link helpers, seven route-only service exports, getFileByIdFull;
  de-export internal-only helpers (utcSuffix).
- De-duplicate verifyTripAccess (9 identical copies -> services/tripAccess.ts)
  and avatarUrl (3 -> services/avatarUrl.ts); name the bcrypt cost
  (BCRYPT_COST) and the email regex (EMAIL_REGEX). Public API unchanged.
- resetPasswordRequestSchema declared `password`, but the client sends and
  the service reads `new_password` — rename it so the contract matches and
  the client types resolve.
- Make ATLAS-013 deterministic: stub the admin-1 GeoJSON download instead of
  fetching ~4600 features from GitHub during the test (it hung the suite).
This commit is contained in:
Maurice
2026-05-31 14:13:04 +02:00
parent 4c9631998f
commit 4cb4454d9f
24 changed files with 94 additions and 145 deletions
+4 -1
View File
@@ -1,6 +1,9 @@
import Database from 'better-sqlite3';
import crypto from 'crypto';
// bcrypt cost factor for the seeded admin password — kept in sync with authService.
const BCRYPT_COST = 12;
// Seeds run at startup before the DB admin panel can be used, so only env vars
// are checked here. The granular password_login/password_registration DB toggles
// are only relevant after the first user exists; at that point seeds have already
@@ -40,7 +43,7 @@ function seedAdminAccount(db: Database.Database): void {
email = 'admin@trek.local';
}
const hash = bcrypt.hashSync(password, 12);
const hash = bcrypt.hashSync(password, BCRYPT_COST);
const username = 'admin';
db.prepare('INSERT INTO users (username, email, password_hash, role, must_change_password) VALUES (?, ?, ?, ?, 1)').run(username, email, hash, 'admin');
+6 -3
View File
@@ -16,7 +16,10 @@ import { resolveAuthToggles } from './authService';
// ── Helpers ────────────────────────────────────────────────────────────────
export function utcSuffix(ts: string | null | undefined): string | null {
// bcrypt cost factor for user passwords — kept in sync with authService.
const BCRYPT_COST = 12;
function utcSuffix(ts: string | null | undefined): string | null {
if (!ts) return null;
return ts.endsWith('Z') ? ts : ts.replace(' ', 'T') + 'Z';
}
@@ -94,7 +97,7 @@ export function createUser(data: { username: string; email: string; password: st
const existingEmail = db.prepare('SELECT id FROM users WHERE email = ?').get(email);
if (existingEmail) return { error: 'Email already taken', status: 409 };
const passwordHash = bcrypt.hashSync(password, 12);
const passwordHash = bcrypt.hashSync(password, BCRYPT_COST);
const result = db.prepare(
'INSERT INTO users (username, email, password_hash, role) VALUES (?, ?, ?, ?)'
@@ -136,7 +139,7 @@ export function updateUser(id: string, data: { username?: string; email?: string
const pwCheck = validatePassword(password);
if (!pwCheck.ok) return { error: pwCheck.reason, status: 400 };
}
const passwordHash = password ? bcrypt.hashSync(password, 12) : null;
const passwordHash = password ? bcrypt.hashSync(password, BCRYPT_COST) : null;
db.prepare(`
UPDATE users SET
+16 -13
View File
@@ -20,6 +20,9 @@ import { getFlightDistanceKm } from './distanceService';
import { verifyJwtAndLoadUser } from '../middleware/auth';
import { User } from '../types';
import { DEMO_EMAIL_PRIMARY, isDemoEmail } from './demo';
import { avatarUrl } from './avatarUrl';
export { avatarUrl };
// ---------------------------------------------------------------------------
// Constants
@@ -27,10 +30,16 @@ import { DEMO_EMAIL_PRIMARY, isDemoEmail } from './demo';
authenticator.options = { window: 1 };
// bcrypt cost factor for user passwords. Shared by register/changePassword/
// resetPassword and the dummy-hash timing equaliser below — must stay in sync.
const BCRYPT_COST = 12;
// Shape check for email input on register and profile update.
const EMAIL_REGEX = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
// Pre-computed bcrypt hash to equalise timing of "unknown email" and
// "OIDC-only account" branches with the real verification path (CWE-208).
// Cost factor 12 matches register/changePassword/resetPassword — must stay in sync.
const DUMMY_PASSWORD_HASH = bcrypt.hashSync('__trek_no_such_user__', 12);
const DUMMY_PASSWORD_HASH = bcrypt.hashSync('__trek_no_such_user__', BCRYPT_COST);
const MFA_SETUP_TTL_MS = 15 * 60 * 1000;
const mfaSetupPending = new Map<number, { secret: string; exp: number }>();
@@ -114,10 +123,6 @@ export function mask_stored_api_key(key: string | null | undefined): string | nu
return maskKey(plain);
}
export function avatarUrl(user: { avatar?: string | null }): string | null {
return user.avatar ? `/uploads/avatars/${user.avatar}` : null;
}
export function resolveAuthToggles(): {
password_login: boolean;
password_registration: boolean;
@@ -377,8 +382,7 @@ export function registerUser(body: {
const pwCheck = validatePassword(password);
if (!pwCheck.ok) return { error: pwCheck.reason, status: 400 };
const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
if (!emailRegex.test(email)) {
if (!EMAIL_REGEX.test(email)) {
return { error: 'Invalid email format', status: 400 };
}
@@ -387,7 +391,7 @@ export function registerUser(body: {
return { error: 'Registration failed. Please try different credentials.', status: 409 };
}
const password_hash = bcrypt.hashSync(password, 12);
const password_hash = bcrypt.hashSync(password, BCRYPT_COST);
const isFirstUser = userCount === 0;
const role = isFirstUser ? 'admin' : 'user';
@@ -533,7 +537,7 @@ export function changePassword(
return { error: 'Current password is incorrect', status: 401 };
}
const hash = bcrypt.hashSync(new_password, 12);
const hash = bcrypt.hashSync(new_password, BCRYPT_COST);
db.prepare('UPDATE users SET password_hash = ?, must_change_password = 0, updated_at = CURRENT_TIMESTAMP WHERE id = ?').run(hash, userId);
return { success: true };
}
@@ -608,8 +612,7 @@ export function updateSettings(
if (email !== undefined) {
const trimmed = email.trim();
const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
if (!trimmed || !emailRegex.test(trimmed)) {
if (!trimmed || !EMAIL_REGEX.test(trimmed)) {
return { error: 'Invalid email format', status: 400 };
}
const conflict = db.prepare('SELECT id FROM users WHERE LOWER(email) = LOWER(?) AND id != ?').get(trimmed, userId);
@@ -1249,7 +1252,7 @@ export function resetPassword(body: {
}
}
const newHash = bcrypt.hashSync(new_password, 12);
const newHash = bcrypt.hashSync(new_password, BCRYPT_COST);
const newPv = (user.password_version ?? 0) + 1;
db.transaction(() => {
+3
View File
@@ -0,0 +1,3 @@
export function avatarUrl(user: { avatar?: string | null }): string | null {
return user.avatar ? `/uploads/avatars/${user.avatar}` : null;
}
+4 -8
View File
@@ -1,17 +1,13 @@
import { db, canAccessTrip } from '../db/database';
import { db } from '../db/database';
import { BudgetItem, BudgetItemMember } from '../types';
import { avatarUrl } from './avatarUrl';
// ---------------------------------------------------------------------------
// Helpers
// ---------------------------------------------------------------------------
export function avatarUrl(user: { avatar?: string | null }): string | null {
return user.avatar ? `/uploads/avatars/${user.avatar}` : null;
}
export function verifyTripAccess(tripId: string | number, userId: number) {
return canAccessTrip(tripId, userId);
}
export { avatarUrl };
export { verifyTripAccess } from './tripAccess';
function loadItemMembers(itemId: number | string) {
const rows = db.prepare(`
+4 -8
View File
@@ -1,8 +1,9 @@
import path from 'path';
import fs from 'fs';
import { db, canAccessTrip } from '../db/database';
import { db } from '../db/database';
import { CollabNote, CollabPoll, CollabMessage, TripFile } from '../types';
import { checkSsrf, createPinnedDispatcher } from '../utils/ssrfGuard';
import { avatarUrl } from './avatarUrl';
/* ------------------------------------------------------------------ */
/* Internal row types */
@@ -48,13 +49,8 @@ export interface LinkPreviewResult {
/* Helpers */
/* ------------------------------------------------------------------ */
export function avatarUrl(user: { avatar?: string | null }): string | null {
return user.avatar ? `/uploads/avatars/${user.avatar}` : null;
}
export function verifyTripAccess(tripId: string | number, userId: number) {
return canAccessTrip(tripId, userId);
}
export { avatarUrl };
export { verifyTripAccess } from './tripAccess';
/* ------------------------------------------------------------------ */
/* Reactions */
+2 -4
View File
@@ -1,9 +1,7 @@
import { db, canAccessTrip } from '../db/database';
import { db } from '../db/database';
import { DayNote } from '../types';
export function verifyTripAccess(tripId: string | number, userId: number) {
return canAccessTrip(tripId, userId);
}
export { verifyTripAccess } from './tripAccess';
export function listNotes(dayId: string | number, tripId: string | number) {
return db.prepare(
+2 -4
View File
@@ -1,10 +1,8 @@
import { db, canAccessTrip } from '../db/database';
import { db } from '../db/database';
import { loadTagsByPlaceIds, loadParticipantsByAssignmentIds, formatAssignmentWithPlace } from './queryHelpers';
import { AssignmentRow, Day, DayNote } from '../types';
export function verifyTripAccess(tripId: string | number, userId: number) {
return canAccessTrip(tripId, userId);
}
export { verifyTripAccess } from './tripAccess';
// ---------------------------------------------------------------------------
// Day assignment helpers
+2 -4
View File
@@ -1,7 +1,7 @@
import path from 'path';
import fs from 'fs';
import type { Request } from 'express';
import { db, canAccessTrip } from '../db/database';
import { db } from '../db/database';
import { consumeEphemeralToken } from './ephemeralTokens';
import { verifyJwtAndLoadUser } from '../middleware/auth';
import { TripFile } from '../types';
@@ -30,9 +30,7 @@ export const filesDir = path.join(__dirname, '../../uploads/files');
// Helpers
// ---------------------------------------------------------------------------
export function verifyTripAccess(tripId: string | number, userId: number) {
return canAccessTrip(tripId, userId);
}
export { verifyTripAccess } from './tripAccess';
export function getAllowedExtensions(): string {
try {
-3
View File
@@ -7,9 +7,6 @@ import { getAppUrl } from './notifications';
let googleApiCallCount = 0;
export function getGoogleApiCallCount(): number { return googleApiCallCount; }
export function resetGoogleApiCallCount(): void { googleApiCallCount = 0; }
function googleFetch(endpoint: string, label: string, init?: RequestInit): Promise<Response> {
googleApiCallCount++;
console.debug(`[Google API] #${googleApiCallCount} ${label}${endpoint}`);
@@ -349,42 +349,6 @@ export async function getAlbumPhotos(
}
}
export function listAlbumLinks(tripId: string) {
return db.prepare(`
SELECT tal.*, u.username
FROM trip_album_links tal
JOIN users u ON tal.user_id = u.id
WHERE tal.trip_id = ?
ORDER BY tal.created_at ASC
`).all(tripId);
}
export function createAlbumLink(
tripId: string,
userId: number,
albumId: string,
albumName: string
): { success: boolean; error?: string } {
try {
db.prepare(
"INSERT OR IGNORE INTO trip_album_links (trip_id, user_id, album_id, album_name, provider) VALUES (?, ?, ?, ?, 'immich')"
).run(tripId, userId, albumId, albumName || '');
return { success: true };
} catch {
return { success: false, error: 'Album already linked' };
}
}
export function deleteAlbumLink(linkId: string, tripId: string, userId: number) {
db.transaction(() => {
const link = db.prepare('SELECT id FROM trip_album_links WHERE id = ? AND trip_id = ? AND user_id = ?').get(linkId, tripId, userId);
if (link) {
db.prepare('DELETE FROM trip_photos WHERE trip_id = ? AND album_link_id = ?').run(tripId, linkId);
db.prepare('DELETE FROM trip_album_links WHERE id = ?').run(linkId);
}
})();
}
export async function syncAlbumAssets(
tripId: string,
linkId: string,
@@ -230,10 +230,3 @@ export function deleteTrekPhotoIfOrphan(photoId: number): void {
db.prepare("DELETE FROM trek_photos WHERE id = ? AND provider != 'local'").run(photoId);
}
// ── Delete local file for a trek_photo ──────────────────────────────────
export function getTrekPhotoFilePath(photoId: number): string | null {
const photo = resolveTrekPhoto(photoId);
if (!photo || photo.provider !== 'local' || !photo.file_path) return null;
return path.join(__dirname, '../../../uploads', photo.file_path);
}
@@ -458,11 +458,12 @@ export async function listSynologyAlbums(userId: number): Promise<ServiceResult<
const addAlbums = (result: PromiseSettledResult<ServiceResult<any[]>>, extractPassphrase: (a: any) => string | undefined) => {
if (result.status === 'rejected') return;
if (!result.value.success) {
console.warn('[Synology] album list partial failure:', (result.value as any).error?.message);
const value = result.value;
if ('error' in value) {
console.warn('[Synology] album list partial failure:', value.error.message);
return;
}
for (const album of result.value.data ?? []) {
for (const album of value.data ?? []) {
const id = String(album.id);
const passphrase = extractPassphrase(album);
map.set(id, { id, albumName: album.name || '', assetCount: album.item_count || 0, passphrase });
@@ -299,7 +299,7 @@ async function _notifySharedTripPhotos(
actorUserId: number,
added: number,
): Promise<ServiceResult<void>> {
if (added <= 0) return fail('No photos shared, skipping notifications', 200);
if (added <= 0) return success(undefined);
try {
const actorRow = db.prepare('SELECT username, email FROM users WHERE id = ?').get(actorUserId) as { username: string | null, email: string | null };
-9
View File
@@ -432,15 +432,6 @@ export function resolveNtfyUrl(adminCfg: NtfyConfig, userCfg: NtfyConfig | null)
return `${base}/${encodeURIComponent(topic)}`;
}
export function isNtfyConfiguredForUser(userId: number): boolean {
const cfg = getUserNtfyConfig(userId);
return !!(cfg?.topic);
}
export function isNtfyConfiguredAdmin(): boolean {
return !!(getAppSetting('admin_ntfy_topic'));
}
function encodeHeaderValue(value: string): string {
for (let i = 0; i < value.length; i++) {
if (value.charCodeAt(i) > 0xFF) {
-10
View File
@@ -118,16 +118,6 @@ export function listOAuthClients(userId: number): Record<string, unknown>[] {
}));
}
/** Returns true if the URI is a valid OAuth redirect target (HTTPS or localhost). */
export function isValidRedirectUri(uri: string): boolean {
try {
const url = new URL(uri);
return url.protocol === 'https:' || url.hostname === 'localhost' || url.hostname === '127.0.0.1';
} catch {
return false;
}
}
export function createOAuthClient(
userId: number | null,
name: string,
+2 -4
View File
@@ -1,11 +1,9 @@
import { db, canAccessTrip } from '../db/database';
import { db } from '../db/database';
import { avatarUrl } from './authService';
const BAG_COLORS = ['#6366f1', '#ec4899', '#f97316', '#10b981', '#06b6d4', '#8b5cf6', '#ef4444', '#f59e0b'];
export function verifyTripAccess(tripId: string | number, userId: number) {
return canAccessTrip(tripId, userId);
}
export { verifyTripAccess } from './tripAccess';
// ── Items ──────────────────────────────────────────────────────────────────
+3 -14
View File
@@ -1,6 +1,8 @@
import { db, canAccessTrip } from '../db/database';
import { db } from '../db/database';
import { Reservation } from '../types';
export { verifyTripAccess } from './tripAccess';
export interface ReservationEndpoint {
id?: number;
reservation_id?: number;
@@ -17,10 +19,6 @@ export interface ReservationEndpoint {
type EndpointInput = Omit<ReservationEndpoint, 'id' | 'reservation_id' | 'sequence'> & { sequence?: number };
export function verifyTripAccess(tripId: string | number, userId: number) {
return canAccessTrip(tripId, userId);
}
function loadEndpointsByTrip(tripId: string | number): Map<number, ReservationEndpoint[]> {
const rows = db.prepare(`
SELECT e.* FROM reservation_endpoints e
@@ -298,15 +296,6 @@ export function updatePositions(tripId: string | number, positions: { id: number
}
}
export function getDayPositions(tripId: string | number, dayId: number | string) {
return db.prepare(`
SELECT rdp.reservation_id, rdp.position
FROM reservation_day_positions rdp
JOIN reservations r ON rdp.reservation_id = r.id
WHERE r.trip_id = ? AND rdp.day_id = ?
`).all(tripId, dayId) as { reservation_id: number; position: number }[];
}
export function getReservation(id: string | number, tripId: string | number) {
return db.prepare('SELECT * FROM reservations WHERE id = ? AND trip_id = ?').get(id, tripId) as Reservation | undefined;
}
+2 -4
View File
@@ -1,8 +1,6 @@
import { db, canAccessTrip } from '../db/database';
import { db } from '../db/database';
export function verifyTripAccess(tripId: string | number, userId: number) {
return canAccessTrip(tripId, userId);
}
export { verifyTripAccess } from './tripAccess';
// ── Items ──────────────────────────────────────────────────────────────────
+9
View File
@@ -0,0 +1,9 @@
import { canAccessTrip } from '../db/database';
/**
* Returns the trip row if the user is the owner or a member, otherwise undefined.
* Shared by the domain services so each one exposes the same access check.
*/
export function verifyTripAccess(tripId: string | number, userId: number) {
return canAccessTrip(tripId, userId);
}
+2 -5
View File
@@ -1,6 +1,6 @@
import path from 'path';
import fs from 'fs';
import { db, canAccessTrip, isOwner } from '../db/database';
import { db, isOwner } from '../db/database';
import { Trip, User } from '../types';
import { listDays, listAccommodations } from './dayService';
import { listBudgetItems } from './budgetService';
@@ -25,10 +25,7 @@ export const TRIP_SELECT = `
// ── Access helpers ────────────────────────────────────────────────────────
export function verifyTripAccess(tripId: string | number, userId: number) {
return canAccessTrip(tripId, userId);
}
export { verifyTripAccess } from './tripAccess';
export { isOwner };
// ── Day generation ────────────────────────────────────────────────────────
+22
View File
@@ -51,6 +51,27 @@ let nestApp: INestApplication;
let app: Application;
beforeAll(async () => {
// Stub the admin-1 GeoJSON download so /regions/geo is deterministic and never
// hits the real network (the un-stubbed fetch of a ~4600-feature file from
// raw.githubusercontent.com is what made ATLAS-013 hang/time out under load).
// Any other outbound fetch (e.g. background reverse-geocoding) returns empty so
// no test depends on live network.
vi.stubGlobal('fetch', async (url: unknown) => {
if (String(url).includes('natural-earth-vector')) {
return new Response(
JSON.stringify({
type: 'FeatureCollection',
features: [
{ type: 'Feature', properties: { iso_a2: 'DE' }, geometry: { type: 'Point', coordinates: [10, 51] } },
{ type: 'Feature', properties: { iso_a2: 'FR' }, geometry: { type: 'Point', coordinates: [2, 47] } },
],
}),
{ status: 200, headers: { 'Content-Type': 'application/json' } },
);
}
return new Response('{}', { status: 200, headers: { 'Content-Type': 'application/json' } });
});
createTables(testDb);
runMigrations(testDb);
nestApp = await buildApp();
@@ -63,6 +84,7 @@ beforeEach(() => {
});
afterAll(async () => {
vi.unstubAllGlobals();
await nestApp.close();
testDb.close();
});
+3 -3
View File
@@ -28,9 +28,9 @@ describe('loginRequestSchema', () => {
describe('forgot/reset/change password schemas', () => {
it('validate their required fields', () => {
expect(forgotPasswordRequestSchema.safeParse({ email: 'a@b.c' }).success).toBe(true);
expect(resetPasswordRequestSchema.safeParse({ token: 't', password: 'pw' }).success).toBe(true);
expect(resetPasswordRequestSchema.safeParse({ token: 't', password: 'pw', mfa_code: '123456' }).success).toBe(true);
expect(resetPasswordRequestSchema.safeParse({ password: 'pw' }).success).toBe(false);
expect(resetPasswordRequestSchema.safeParse({ token: 't', new_password: 'pw' }).success).toBe(true);
expect(resetPasswordRequestSchema.safeParse({ token: 't', new_password: 'pw', mfa_code: '123456' }).success).toBe(true);
expect(resetPasswordRequestSchema.safeParse({ new_password: 'pw' }).success).toBe(false);
expect(changePasswordRequestSchema.safeParse({ current_password: 'a', new_password: 'b' }).success).toBe(true);
expect(changePasswordRequestSchema.safeParse({ new_password: 'b' }).success).toBe(false);
});
+3 -1
View File
@@ -29,7 +29,9 @@ export type ForgotPasswordRequest = z.infer<typeof forgotPasswordRequestSchema>;
export const resetPasswordRequestSchema = z.object({
token: z.string(),
password: z.string(),
// The client sends `new_password` and the service reads `body.new_password`;
// the field was misnamed `password` here, which broke the client's typing.
new_password: z.string(),
mfa_code: z.string().optional(),
});
export type ResetPasswordRequest = z.infer<typeof resetPasswordRequestSchema>;