refactor(auth): users table through UsersRepository; JWT and MFA lookups repository-backed

This commit is contained in:
jubnl
2026-09-22 19:39:01 +02:00
parent 7453c7dafa
commit 57f4e29b9a
42 changed files with 1746 additions and 225 deletions
+25
View File
@@ -74,3 +74,28 @@ export function columnIncrementedBy(platform: Platform, ref: string, amount: num
}
return unsupported(platform);
}
/**
* A case-insensitive column reference, for use as a filter's object key:
* `{ [lower(platform, 'email')]: value.toLowerCase() }` produces
* `WHERE LOWER(email) = ?`. This is the parity-preserving equivalent of the
* legacy `LOWER(col) = LOWER(?)` sites (`AuthService`/`OidcService`/
* `UserProfileService`'s username/email CI lookups): the caller lowercases
* the bound value itself (every one of this migration's call sites already
* does, via `String.prototype.toLowerCase`), so the comparison is between
* two already-lowercased values either way — SQLite's `LOWER()` is ASCII-only
* exactly like `toLowerCase()` is for the identifier characters these
* columns hold (usernames/emails), so the two spellings agree on every
* input this codebase stores.
*
* Typed `RawQueryFragment & symbol` (not the bare `RawQueryFragment` the
* other helpers in this file return): TypeScript only accepts a
* `string | number | symbol` as a computed property name, and a raw
* fragment is only usable as an object key because `raw()`'s default
* generic already brands it with a `[Symbol.toPrimitive]`. The other
* helpers here are only ever used as VALUES, where that brand isn't needed.
*/
export function lower(platform: Platform, ref: string): RawQueryFragment & symbol {
if (platform instanceof SqlitePlatform) return raw(`LOWER(${column(ref)})`);
return unsupported(platform);
}
@@ -37,12 +37,20 @@ export class InviteTokensRepository extends EntityRepository<InviteTokens> {
/**
* `SELECT * FROM invite_tokens WHERE token = ?`
*
* Named `findByToken`, not `findValid` (Task 0 review addendum, LOW item
* 1): the statement has no validity filter of its own — no `expires_at`,
* no `used_count < max_uses` in the WHERE — it is a plain lookup by the
* unique `token` column, and validity (expiry, capacity) is checked in the
* caller's JS afterward, identically for `AuthService.registerUser` (AU8)
* and `OidcService.findOrCreateUser` (O11). A name promising "valid" would
* describe a filter this statement doesn't have.
*
* Not a primary-key filter — `token` is a unique column, not `id` — so this
* always re-queries; no `{ refresh: true }` needed (Task 3a review's
* precision ruling: the identity-map short-circuit only fires for a
* PK-only filter).
*/
async findValid(token: string): Promise<InviteTokenRow | null> {
async findByToken(token: string): Promise<InviteTokenRow | null> {
const invite = await this.findOne({ token });
return invite ? (toRow(invite) as InviteTokenRow) : null;
}
@@ -1,4 +1,6 @@
import type { Users } from '../entities/Users.entity';
import { toRow, type AssertRowKeys } from './_shared/rows';
import { columnIncrementedBy, currentTimestamp, lower } from '../dialect/sql-functions';
import { EntityRepository } from '@mikro-orm/sql';
/**
@@ -13,6 +15,166 @@ import { EntityRepository } from '@mikro-orm/sql';
*/
export type InstanceApiKeyName = 'maps_api_key' | 'unsplash_api_key' | 'amap_api_key';
/**
* A full `users` row (`SELECT * FROM users WHERE ...`), as the API emits it.
* Every scalar column of the entity, hand-nullability-annotated the way
* `AppSettingsRepository`/`CategoriesRepository`'s row interfaces are —
* `AssertRowKeys` below fails `tsc` the moment this list and the entity's
* scalar columns drift.
*/
export interface UserRow {
id: number;
username: string;
email: string;
password_hash: string;
role: string;
maps_api_key: string | null;
unsplash_api_key: string | null;
amap_api_key: string | null;
openweather_api_key: string | null;
avatar: string | null;
oidc_sub: string | null;
oidc_issuer: string | null;
last_login: string | null;
mfa_enabled: number | null;
mfa_secret: string | null;
mfa_backup_codes: string | null;
immich_url: string | null;
immich_access_token: string | null;
synology_url: string | null;
synology_username: string | null;
synology_password: string | null;
synology_sid: string | null;
must_change_password: number | null;
password_version: number;
feed_token: string | null;
is_guest: number;
created_at: string | null;
updated_at: string | null;
immich_api_key: string | null;
synology_skip_ssl: number;
synology_did: string | null;
first_seen_version: string;
login_count: number;
immich_auto_upload: number;
airtrail_url: string | null;
airtrail_api_key: string | null;
airtrail_allow_insecure_tls: number | null;
airtrail_write_enabled: number | null;
display_name: string | null;
}
const _userRowKeys: AssertRowKeys<UserRow, Users> = true;
/** `AuthService.registerUser` (AU10) / `OidcService.findOrCreateUser` (O14) — the two column sets `insertUser` covers as one method (Task 1 brief's Required set: "one method, nullable oidc/avatar fields"). */
export interface NewUserRow {
username: string;
email: string;
password_hash: string;
role: string;
first_seen_version: string;
oidc_sub?: string | null;
oidc_issuer?: string | null;
avatar?: string | null;
}
/** `UserProfileService.updateApiKeys` (UP3) — the four encrypted key columns, already resolved by the service. */
export interface ApiKeysPatch {
maps_api_key: string | null;
openweather_api_key: string | null;
unsplash_api_key: string | null;
amap_api_key: string | null;
}
/** `UserProfileService.updateSettings`'s bounded dynamic SET clause (UP7) — a closed key union, never a built SQL string. */
export interface UserProfilePatch {
maps_api_key?: string | null;
openweather_api_key?: string | null;
unsplash_api_key?: string | null;
amap_api_key?: string | null;
username?: string;
email?: string;
}
/** `UserProfileService.getApiKeyColumns`/`currentKeys` (UP1). */
export interface UserApiKeyColumns {
role: string;
maps_api_key: string | null;
openweather_api_key: string | null;
unsplash_api_key: string | null;
amap_api_key: string | null;
}
/** `UserProfileService.updateApiKeys`/`updateSettings`'s re-select (UP4/UP8). */
export interface UserProfileWithKeys {
id: number;
username: string;
email: string;
role: string;
maps_api_key: string | null;
openweather_api_key: string | null;
unsplash_api_key: string | null;
amap_api_key: string | null;
avatar: string | null;
mfa_enabled: number | null;
}
/** `UserProfileService.saveAvatar`'s re-select (UP11). */
export interface UserProfileBasic {
id: number;
username: string;
email: string;
role: string;
avatar: string | null;
}
/** `UserProfileService.listUsers` (UP14). */
export interface UserDirectoryRow {
id: number;
username: string;
avatar: string | null;
}
/** `AuthService.getCurrentUser` (AU14). */
export interface CurrentUserRow {
id: number;
username: string;
email: string;
role: string;
avatar: string | null;
oidc_issuer: string | null;
created_at: string | null;
mfa_enabled: number | null;
must_change_password: number | null;
}
/** `jwt-verify.ts#verifyJwtAndLoadUser` (JV1). */
export interface UserWithPasswordVersion {
id: number;
username: string;
email: string;
role: string;
password_version: number;
}
/** `AuthService.requestPasswordReset` (AU36). */
export interface PasswordResetLookup {
id: number;
email: string;
password_hash: string;
oidc_sub: string | null;
}
/** `AuthService.resetPassword`'s reset-target read (AU40). */
export interface ResetTargetRow {
id: number;
email: string;
mfa_enabled: number | null;
mfa_secret: string | null;
mfa_backup_codes: string | null;
password_version: number;
}
export class UsersRepository extends EntityRepository<Users> {
/**
* `SELECT email FROM users WHERE id = ?`
@@ -42,4 +204,611 @@ export class UsersRepository extends EntityRepository<Users> {
const row = await this.findOne({ id: userId }, { fields: [name], refresh: true });
return row ? (row[name] ?? null) : null;
}
// ---------------------------------------------------------------------
// JV1 — jwt-verify.ts
// ---------------------------------------------------------------------
/**
* `SELECT id, username, email, role, password_version FROM users WHERE id = ?`
*
* The canonical session-invalidation read: `verifyJwtAndLoadUser` compares
* the returned `password_version` against the JWT's `pv` claim on every
* request. PK-only `findOne` → `refresh: true` (I1).
*/
async findByIdWithPasswordVersion(id: number): Promise<UserWithPasswordVersion | null> {
const row = await this.findOne(
{ id },
{ fields: ['id', 'username', 'email', 'role', 'password_version'], refresh: true },
);
return row ? { id: row.id, username: row.username, email: row.email, role: row.role, password_version: row.password_version } : null;
}
// ---------------------------------------------------------------------
// AU2 / TK7 / O2 — password_version (session-invalidation gate)
// ---------------------------------------------------------------------
/** `SELECT password_version FROM users WHERE id = ?`. `?? 0` stays in the caller (AU2/TK7/O2 each fall back differently in spirit, identically in value). */
async getPasswordVersion(id: number): Promise<number | null> {
const row = await this.findOne({ id }, { fields: ['password_version'], refresh: true });
return row?.password_version ?? null;
}
// ---------------------------------------------------------------------
// AU4 / AU7 / O10 — first-user / admin-role decisions
// ---------------------------------------------------------------------
/** `SELECT COUNT(*) as count FROM users WHERE COALESCE(is_guest, 0) = 0`. `is_guest` is `INTEGER NOT NULL DEFAULT 0` (verified in the baseline + guest-members migrations) — no row can store NULL there, so `{ is_guest: 0 }` is exact parity for the `COALESCE(is_guest, 0) = 0` guard, not an approximation. */
async countNonGuest(): Promise<number> {
return this.count({ is_guest: 0 });
}
// ---------------------------------------------------------------------
// AU6 — setup_complete
// ---------------------------------------------------------------------
/** `SELECT id FROM users WHERE role = 'admin' AND must_change_password = 1 LIMIT 1`. */
async findAdminNeedingPasswordChange(): Promise<number | null> {
const row = await this.findOne({ role: 'admin', must_change_password: 1 }, { fields: ['id'] });
return row?.id ?? null;
}
// ---------------------------------------------------------------------
// AU9 — registration collision check
// ---------------------------------------------------------------------
/**
* `SELECT id FROM users WHERE (LOWER(email) = LOWER(?) OR LOWER(username) = LOWER(?))
* AND COALESCE(is_guest, 0) = 0`
*/
async findIdByEmailOrUsernameCI(email: string, username: string): Promise<number | null> {
const platform = this.getEntityManager().getPlatform();
const row = await this.findOne(
{
$or: [{ [lower(platform, 'email')]: email.toLowerCase() }, { [lower(platform, 'username')]: username.toLowerCase() }],
is_guest: 0,
},
{ fields: ['id'] },
);
return row?.id ?? null;
}
// ---------------------------------------------------------------------
// AU10 / O14 — account creation
// ---------------------------------------------------------------------
/**
* `INSERT INTO users (username, email, password_hash, role, first_seen_version, login_count)
* VALUES (?, ?, ?, ?, ?, 0)` (AU10, local registration) and
* `INSERT INTO users (username, email, password_hash, role, oidc_sub, oidc_issuer, avatar, first_seen_version, login_count)
* VALUES (?, ?, ?, ?, ?, ?, ?, ?, 0)` (O14, SSO registration) — ONE method
* (Task 1 brief's Required set), the `oidc_sub`/`oidc_issuer`/`avatar`
* columns nullable so AU10's caller simply omits them. `login_count` is
* always the literal `0` on insert, never a parameter, matching both
* statements exactly.
*
* Followed by `refresh` (D4's create rule), not relied-on `RETURNING`:
* the returned row carries the generated `id` and the `defaultRaw`
* `created_at` the same way a legacy INSERT-then-reselect pair did.
*/
async insertUser(row: NewUserRow): Promise<UserRow> {
const user = this.create({
username: row.username,
email: row.email,
password_hash: row.password_hash,
role: row.role,
first_seen_version: row.first_seen_version,
login_count: 0,
oidc_sub: row.oidc_sub ?? null,
oidc_issuer: row.oidc_issuer ?? null,
avatar: row.avatar ?? null,
});
await this.getEntityManager().persist(user).flush();
await this.getEntityManager().refresh(user);
return toRow(user) as UserRow;
}
// ---------------------------------------------------------------------
// AU12 / O4 — email lookup (login + OIDC email-fallback)
// ---------------------------------------------------------------------
/** `SELECT * FROM users WHERE LOWER(email) = LOWER(?) AND COALESCE(is_guest, 0) = 0` */
async findByEmailCI(email: string): Promise<UserRow | null> {
const platform = this.getEntityManager().getPlatform();
const user = await this.findOne({ [lower(platform, 'email')]: email.toLowerCase(), is_guest: 0 });
return user ? (toRow(user) as UserRow) : null;
}
// ---------------------------------------------------------------------
// AU13 / AU34 / AU35 / PK12 / O16 — login bookkeeping
// ---------------------------------------------------------------------
/**
* `UPDATE users SET last_login = CURRENT_TIMESTAMP, login_count = login_count + 1
* WHERE id = ?` — ONE statement via `nativeUpdate` with a raw sibling-column
* increment (`columnIncrementedBy`), never a read-modify-write: two
* concurrent calls both read `login_count` from the row at commit time, not
* from a value fetched earlier, so both increments land (proven by
* `USERSREPO` `touchLastLogin` concurrency test — two concurrent calls →
* `+2`, never `+1`).
*/
async touchLastLogin(id: number): Promise<void> {
const platform = this.getEntityManager().getPlatform();
await this.nativeUpdate(
{ id },
{ last_login: currentTimestamp(platform), login_count: columnIncrementedBy(platform, 'login_count', 1) },
);
}
// ---------------------------------------------------------------------
// AU14 — /api/auth/me
// ---------------------------------------------------------------------
/** `SELECT id, username, email, role, avatar, oidc_issuer, created_at, mfa_enabled, must_change_password FROM users WHERE id = ?` */
async findMeRow(id: number): Promise<CurrentUserRow | null> {
const row = await this.findOne(
{ id },
{
fields: ['id', 'username', 'email', 'role', 'avatar', 'oidc_issuer', 'created_at', 'mfa_enabled', 'must_change_password'],
refresh: true,
},
);
return row
? {
id: row.id,
username: row.username,
email: row.email,
role: row.role,
avatar: row.avatar ?? null,
oidc_issuer: row.oidc_issuer ?? null,
created_at: row.created_at ?? null,
mfa_enabled: row.mfa_enabled ?? null,
must_change_password: row.must_change_password ?? null,
}
: null;
}
// ---------------------------------------------------------------------
// AU15 — current-password check
// ---------------------------------------------------------------------
/** `SELECT password_hash, password_version FROM users WHERE id = ?` */
async getPasswordHashAndVersion(id: number): Promise<{ password_hash: string; password_version: number } | null> {
const row = await this.findOne({ id }, { fields: ['password_hash', 'password_version'], refresh: true });
return row ? { password_hash: row.password_hash, password_version: row.password_version } : null;
}
// ---------------------------------------------------------------------
// AU16 / AU43 — password change / reset
// ---------------------------------------------------------------------
/**
* `UPDATE users SET password_hash = ?, must_change_password = 0,
* password_version = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?`
* (AU16 `changePassword`, AU43 `resetPassword` — identical statement).
*/
async setPassword(id: number, passwordHash: string, passwordVersion: number): Promise<void> {
const platform = this.getEntityManager().getPlatform();
await this.nativeUpdate(
{ id },
{ password_hash: passwordHash, must_change_password: 0, password_version: passwordVersion, updated_at: currentTimestamp(platform) },
);
}
// ---------------------------------------------------------------------
// AU19 / O7 — last-admin guard
// ---------------------------------------------------------------------
/** `SELECT COUNT(*) as count FROM users WHERE role = 'admin'` */
async countAdmins(): Promise<number> {
return this.count({ role: 'admin' });
}
// ---------------------------------------------------------------------
// AU20 / AU22 — admin gate
// ---------------------------------------------------------------------
/** `SELECT role FROM users WHERE id = ?` */
async getRole(id: number): Promise<string | null> {
const row = await this.findOne({ id }, { fields: ['role'], refresh: true });
return row?.role ?? null;
}
// ---------------------------------------------------------------------
// AU23 / AU27 / MP2 — MFA state
// ---------------------------------------------------------------------
/**
* `SELECT mfa_enabled FROM users WHERE id = ?`
*
* Returns the ROW, not the bare column: `mfa-policy.guard.ts` (MP2) needs
* to tell "no row for this id" (`null` — the user vanished mid-request,
* `MfaPolicyGuard` lets the request through) apart from "row present with
* `mfa_enabled` unset" (`{ mfa_enabled: null }` — still governed by the
* policy) — the same distinction the legacy `if (!row) return true;`
* before `row.mfa_enabled === 1` made. A bare `T | null` return would
* collapse both into the same value.
*/
async getMfaEnabled(id: number): Promise<{ mfa_enabled: number | null } | null> {
const row = await this.findOne({ id }, { fields: ['mfa_enabled'], refresh: true });
return row ? { mfa_enabled: row.mfa_enabled ?? null } : null;
}
// ---------------------------------------------------------------------
// AU28 — enable MFA
// ---------------------------------------------------------------------
/**
* `UPDATE users SET mfa_enabled = 1, mfa_secret = ?, mfa_backup_codes = ?,
* updated_at = CURRENT_TIMESTAMP WHERE id = ?`. `secret`/`codes` arrive
* already encrypted/hashed — this repository never touches either.
*/
async enableMfa(id: number, encryptedSecret: string, backupCodes: string): Promise<void> {
const platform = this.getEntityManager().getPlatform();
await this.nativeUpdate(
{ id },
{ mfa_enabled: 1, mfa_secret: encryptedSecret, mfa_backup_codes: backupCodes, updated_at: currentTimestamp(platform) },
);
}
// ---------------------------------------------------------------------
// AU30 / AU32 / PK4 / PK10 / O15 — full-row reads
// ---------------------------------------------------------------------
/**
* `SELECT * FROM users WHERE id = ?` — the full row, reused by every
* caller that needs more than a handful of columns (AU30 `disableMfa`,
* AU32 `verifyMfaLogin`, PK4 `passkeyRegisterOptions`, PK10
* `passkeyLoginVerify`, O15 `findOrCreateUser`'s post-transaction re-select).
* PK-only `findOne` → `refresh: true` (I1).
*/
async findById(id: number): Promise<UserRow | null> {
const user = await this.findOne({ id }, { refresh: true });
return user ? (toRow(user) as UserRow) : null;
}
// ---------------------------------------------------------------------
// AU31 — disable MFA
// ---------------------------------------------------------------------
/** `UPDATE users SET mfa_enabled = 0, mfa_secret = NULL, mfa_backup_codes = NULL, updated_at = CURRENT_TIMESTAMP WHERE id = ?` */
async disableMfa(id: number): Promise<void> {
const platform = this.getEntityManager().getPlatform();
await this.nativeUpdate(
{ id },
{ mfa_enabled: 0, mfa_secret: null, mfa_backup_codes: null, updated_at: currentTimestamp(platform) },
);
}
// ---------------------------------------------------------------------
// AU33 / AU44 — backup-code burn
// ---------------------------------------------------------------------
/**
* `UPDATE users SET mfa_backup_codes = ?, updated_at = CURRENT_TIMESTAMP
* WHERE id = ?` (AU33, `verifyMfaLogin`'s backup-code branch — inside the
* same transaction as `touchLastLogin`).
*/
async setBackupCodesAndTouch(id: number, backupCodes: string): Promise<void> {
const platform = this.getEntityManager().getPlatform();
await this.nativeUpdate({ id }, { mfa_backup_codes: backupCodes, updated_at: currentTimestamp(platform) });
}
/**
* `UPDATE users SET mfa_backup_codes = ? WHERE id = ?` (AU44,
* `resetPassword`'s conditional backup-code splice — no `updated_at`,
* unlike AU33's otherwise-identical statement; two methods, byte-exact
* parity per statement).
*/
async setBackupCodes(id: number, backupCodes: string): Promise<void> {
await this.nativeUpdate({ id }, { mfa_backup_codes: backupCodes });
}
// ---------------------------------------------------------------------
// AU36 — password-reset request lookup
// ---------------------------------------------------------------------
/**
* `SELECT id, email, password_hash, oidc_sub FROM users WHERE email = ?
* AND COALESCE(is_guest, 0) = 0` — case-SENSITIVE on `email`, unlike
* every other email lookup in this repository (no `LOWER()` in the legacy
* statement here); kept exactly as written, not "fixed" to match the
* others (parity is law).
*/
async findForPasswordReset(email: string): Promise<PasswordResetLookup | null> {
const row = await this.findOne(
{ email, is_guest: 0 },
{ fields: ['id', 'email', 'password_hash', 'oidc_sub'] },
);
return row ? { id: row.id, email: row.email, password_hash: row.password_hash, oidc_sub: row.oidc_sub ?? null } : null;
}
// ---------------------------------------------------------------------
// AU40 — reset-target read
// ---------------------------------------------------------------------
/** `SELECT id, email, mfa_enabled, mfa_secret, mfa_backup_codes, password_version FROM users WHERE id = ?` */
async findResetTarget(id: number): Promise<ResetTargetRow | null> {
const row = await this.findOne(
{ id },
{ fields: ['id', 'email', 'mfa_enabled', 'mfa_secret', 'mfa_backup_codes', 'password_version'], refresh: true },
);
return row
? {
id: row.id,
email: row.email,
mfa_enabled: row.mfa_enabled ?? null,
mfa_secret: row.mfa_secret ?? null,
mfa_backup_codes: row.mfa_backup_codes ?? null,
password_version: row.password_version,
}
: null;
}
// ---------------------------------------------------------------------
// PK15 — re-auth before passkey deletion
// ---------------------------------------------------------------------
/** `SELECT password_hash FROM users WHERE id = ?` */
async getPasswordHash(id: number): Promise<string | null> {
const row = await this.findOne({ id }, { fields: ['password_hash'], refresh: true });
return row?.password_hash ?? null;
}
// ---------------------------------------------------------------------
// PK17 — admin passkey-reset target
// ---------------------------------------------------------------------
/** `SELECT id, email FROM users WHERE id = ?` */
async findIdAndEmail(id: number): Promise<{ id: number; email: string } | null> {
const row = await this.findOne({ id }, { fields: ['id', 'email'], refresh: true });
return row ? { id: row.id, email: row.email } : null;
}
// ---------------------------------------------------------------------
// O3 — OIDC primary identity match
// ---------------------------------------------------------------------
/** `SELECT * FROM users WHERE oidc_sub = ? AND oidc_issuer = ?` */
async findByOidcIdentity(sub: string, issuer: string): Promise<UserRow | null> {
const user = await this.findOne({ oidc_sub: sub, oidc_issuer: issuer });
return user ? (toRow(user) as UserRow) : null;
}
// ---------------------------------------------------------------------
// O5 / O6 — OIDC identity linking
// ---------------------------------------------------------------------
/**
* `UPDATE users SET oidc_sub = ?, oidc_issuer = ? WHERE id = ?` (O5, the
* email-matched/unlinked/verified-email branch; O6, the provider-switch
* branch — identical statement, two call sites, no `updated_at`).
*/
async linkOidcIdentity(id: number, sub: string, issuer: string): Promise<void> {
await this.nativeUpdate({ id }, { oidc_sub: sub, oidc_issuer: issuer });
}
// ---------------------------------------------------------------------
// O8 — claims-based role change
// ---------------------------------------------------------------------
/** `UPDATE users SET role = ? WHERE id = ?` — no `updated_at` (legacy statement). */
async setRole(id: number, role: string): Promise<void> {
await this.nativeUpdate({ id }, { role });
}
// ---------------------------------------------------------------------
// O9 — OIDC avatar sync
// ---------------------------------------------------------------------
/**
* `UPDATE users SET avatar = ? WHERE id = ?` — no `updated_at`, unlike
* `setAvatar` (UP10/UP13) below, which is the profile-upload path's
* otherwise-identical statement. Two methods, byte-exact parity per
* statement (same rule as `setBackupCodes`/`setBackupCodesAndTouch`).
*/
async setAvatarRaw(id: number, avatar: string | null): Promise<void> {
await this.nativeUpdate({ id }, { avatar });
}
// ---------------------------------------------------------------------
// O12 / UP5 — username collision checks
// ---------------------------------------------------------------------
/**
* `SELECT id FROM users WHERE LOWER(username) = LOWER(?) AND id != ?
* AND COALESCE(is_guest, 0) = 0` (UP5, `updateSettings`'s own-username
* collision check).
*/
async findIdByUsernameCI(username: string, excludeId: number): Promise<number | null> {
const platform = this.getEntityManager().getPlatform();
const row = await this.findOne(
{ [lower(platform, 'username')]: username.toLowerCase(), id: { $ne: excludeId }, is_guest: 0 },
{ fields: ['id'] },
);
return row?.id ?? null;
}
/**
* `SELECT id FROM users WHERE LOWER(username) = LOWER(?)` (O12, OIDC
* derived-username collision avoidance — no `excludeId`, no guest filter:
* a genuinely different WHERE than UP5's, so a separate method per D4
* rather than an optional parameter that would silently change UP5's
* shape.
*/
async findIdByUsernameCIAny(username: string): Promise<number | null> {
const platform = this.getEntityManager().getPlatform();
const row = await this.findOne({ [lower(platform, 'username')]: username.toLowerCase() }, { fields: ['id'] });
return row?.id ?? null;
}
// ---------------------------------------------------------------------
// UP1 — API-key columns read (`currentKeys`)
// ---------------------------------------------------------------------
/** `SELECT role, maps_api_key, openweather_api_key, unsplash_api_key, amap_api_key FROM users WHERE id = ?` */
async getApiKeyColumns(id: number): Promise<UserApiKeyColumns | null> {
const row = await this.findOne(
{ id },
{ fields: ['role', 'maps_api_key', 'openweather_api_key', 'unsplash_api_key', 'amap_api_key'], refresh: true },
);
return row
? {
role: row.role,
maps_api_key: row.maps_api_key ?? null,
openweather_api_key: row.openweather_api_key ?? null,
unsplash_api_key: row.unsplash_api_key ?? null,
amap_api_key: row.amap_api_key ?? null,
}
: null;
}
// ---------------------------------------------------------------------
// UP2 — single-key (maps) update
// ---------------------------------------------------------------------
/** `UPDATE users SET maps_api_key = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?` */
async updateMapsKey(id: number, encryptedMapsApiKey: string | null): Promise<void> {
const platform = this.getEntityManager().getPlatform();
await this.nativeUpdate({ id }, { maps_api_key: encryptedMapsApiKey, updated_at: currentTimestamp(platform) });
}
// ---------------------------------------------------------------------
// UP3 — four-key update
// ---------------------------------------------------------------------
/**
* `UPDATE users SET maps_api_key = ?, openweather_api_key = ?,
* unsplash_api_key = ?, amap_api_key = ?, updated_at = CURRENT_TIMESTAMP
* WHERE id = ?`. The per-column "encrypt the new value or keep the
* current one" branching (a deleted-row race degrading to a 0-row UPDATE,
* not a crash) is the service's, per D4 — this writes exactly the four
* final values it is given.
*/
async updateApiKeys(id: number, keys: ApiKeysPatch): Promise<void> {
const platform = this.getEntityManager().getPlatform();
await this.nativeUpdate({ id }, { ...keys, updated_at: currentTimestamp(platform) });
}
// ---------------------------------------------------------------------
// UP4 / UP8 — profile-with-keys re-select
// ---------------------------------------------------------------------
/** `SELECT id, username, email, role, maps_api_key, openweather_api_key, unsplash_api_key, amap_api_key, avatar, mfa_enabled FROM users WHERE id = ?` */
async findProfileWithKeys(id: number): Promise<UserProfileWithKeys | null> {
const row = await this.findOne(
{ id },
{
fields: ['id', 'username', 'email', 'role', 'maps_api_key', 'openweather_api_key', 'unsplash_api_key', 'amap_api_key', 'avatar', 'mfa_enabled'],
refresh: true,
},
);
return row
? {
id: row.id,
username: row.username,
email: row.email,
role: row.role,
maps_api_key: row.maps_api_key ?? null,
openweather_api_key: row.openweather_api_key ?? null,
unsplash_api_key: row.unsplash_api_key ?? null,
amap_api_key: row.amap_api_key ?? null,
avatar: row.avatar ?? null,
mfa_enabled: row.mfa_enabled ?? null,
}
: null;
}
// ---------------------------------------------------------------------
// UP6 — email collision check
// ---------------------------------------------------------------------
/** `SELECT id FROM users WHERE LOWER(email) = LOWER(?) AND id != ? AND COALESCE(is_guest, 0) = 0` */
async findIdByEmailCI(email: string, excludeId: number): Promise<number | null> {
const platform = this.getEntityManager().getPlatform();
const row = await this.findOne(
{ [lower(platform, 'email')]: email.toLowerCase(), id: { $ne: excludeId }, is_guest: 0 },
{ fields: ['id'] },
);
return row?.id ?? null;
}
// ---------------------------------------------------------------------
// UP7 — bounded dynamic-column profile patch
// ---------------------------------------------------------------------
/**
* `` UPDATE users SET ${updates.join(', ')}, updated_at = CURRENT_TIMESTAMP
* WHERE id = ? `` — the SET clause is built at the CALL SITE from a fixed
* set of 6 possible literal column names, never here: `patchProfile` takes
* only the already-decided final columns as a typed partial and writes
* them in ONE `nativeUpdate` (never `assign`+`flush`, which would be a
* second statement shape for the identical intent) — the entity-API
* equivalent of the legacy's dynamic `SET` list, without ever building SQL
* text. `updated_at` is always stamped, matching the legacy: this method's
* only caller (`UserProfileService.updateSettings`) calls it only when at
* least one field changed (`updates.length > 0`), the same guard the
* legacy statement was built under.
*/
async patchProfile(id: number, changes: UserProfilePatch): Promise<void> {
const platform = this.getEntityManager().getPlatform();
await this.nativeUpdate({ id }, { ...changes, updated_at: currentTimestamp(platform) });
}
// ---------------------------------------------------------------------
// UP9 / UP12 — avatar read (before cleanup/replace)
// ---------------------------------------------------------------------
/** `SELECT avatar FROM users WHERE id = ?` */
async getAvatar(id: number): Promise<string | null> {
const row = await this.findOne({ id }, { fields: ['avatar'], refresh: true });
return row?.avatar ?? null;
}
// ---------------------------------------------------------------------
// UP10 / UP13 — avatar write
// ---------------------------------------------------------------------
/**
* `UPDATE users SET avatar = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?`
* (UP10 `saveAvatar` with a filename, UP13 `deleteAvatar` with `NULL` —
* same statement shape, `avatar` nullable either way).
*/
async setAvatar(id: number, avatar: string | null): Promise<void> {
const platform = this.getEntityManager().getPlatform();
await this.nativeUpdate({ id }, { avatar, updated_at: currentTimestamp(platform) });
}
// ---------------------------------------------------------------------
// UP11 — post-avatar-write re-select
// ---------------------------------------------------------------------
/** `SELECT id, username, email, role, avatar FROM users WHERE id = ?` */
async findProfileBasic(id: number): Promise<UserProfileBasic | null> {
const row = await this.findOne({ id }, { fields: ['id', 'username', 'email', 'role', 'avatar'], refresh: true });
return row ? { id: row.id, username: row.username, email: row.email, role: row.role, avatar: row.avatar ?? null } : null;
}
// ---------------------------------------------------------------------
// UP14 — user directory (trip member / contributor pickers)
// ---------------------------------------------------------------------
/** `SELECT id, username, avatar FROM users WHERE id != ? AND COALESCE(is_guest, 0) = 0 ORDER BY username ASC` */
async listOthersNonGuest(excludeId: number): Promise<UserDirectoryRow[]> {
const rows = await this.find(
{ id: { $ne: excludeId }, is_guest: 0 },
{ fields: ['id', 'username', 'avatar'], orderBy: { username: 'asc' } },
);
return rows.map((row) => ({ id: row.id, username: row.username, avatar: row.avatar ?? null }));
}
// ---------------------------------------------------------------------
// UP15 — key-validation probe
// ---------------------------------------------------------------------
/** `SELECT role, openweather_api_key FROM users WHERE id = ?` */
async getRoleAndWeatherKey(id: number): Promise<{ role: string; openweather_api_key: string | null } | null> {
const row = await this.findOne({ id }, { fields: ['role', 'openweather_api_key'], refresh: true });
return row ? { role: row.role, openweather_api_key: row.openweather_api_key ?? null } : null;
}
}
@@ -1,4 +1,28 @@
import type { WebauthnCredentials } from '../entities/WebauthnCredentials.entity';
import { EntityRepository } from '@mikro-orm/sql';
export class WebauthnCredentialsRepository extends EntityRepository<WebauthnCredentials> {}
/**
* Plan 3b Task 1 reserves this class (empty stub since Plan 3a) and implements
* exactly the one method the AUTH cluster's Task 1 needs NOW: `mfa-policy.guard.ts`
* (MP3) and, cross-service, `AuthService.updateAppSettings` (AU24, Task 5) both
* read "does this user own at least one passkey" — a passkey satisfies the
* `require_mfa` policy the same way TOTP does. Task 3 (`PasskeyService`)
* extends this class with the rest of the WebAuthn ceremony state
* (`listByUser`, `create`, `findByCredentialId`, `updateCounterAndLastUsed`,
* `renameOwned`, `deleteOwned`, `deleteAllForUser`) — this method's name and
* shape are chosen so nothing here needs to change when that lands.
*/
export class WebauthnCredentialsRepository extends EntityRepository<WebauthnCredentials> {
/**
* `SELECT 1 FROM webauthn_credentials WHERE user_id = ? LIMIT 1` (MP3/AU24
* — identical statement, two callers). `count` with an implicit `LIMIT 1`
* via `findOne` mirrors the legacy existence probe without pulling a row;
* MikroORM's `count()` has no built-in short-circuit, so `findOne` with a
* narrow `fields` selection is the closer match to "does one exist" than
* `count() > 0`, which would still scan/aggregate every matching row.
*/
async hasAny(userId: number): Promise<boolean> {
const row = await this.findOne({ user: userId }, { fields: ['id'] });
return row !== null;
}
}
+10
View File
@@ -1,5 +1,8 @@
import { trekMcpAccessPolicy, trekMcpValidateAccess } from '../mcp/nest-mcp-policy';
import mikroOrmConfig from '../mikro-orm.config';
import { AppSettings } from '../db/entities/AppSettings.entity';
import { Users } from '../db/entities/Users.entity';
import { WebauthnCredentials } from '../db/entities/WebauthnCredentials.entity';
import { McpModule } from '../nest-mcp';
import { AccommodationsModule } from './accommodations/accommodations.module';
import { AddonsModule } from './addons/addons.module';
@@ -155,6 +158,13 @@ import { APP_FILTER, APP_GUARD, APP_INTERCEPTOR, APP_PIPE } from '@nestjs/core';
// Not part of DatabaseModule: the e2e suites that compose `DatabaseModule`
// without the ORM would fail on an EntityManager-dependent provider there.
OrmModule,
// MfaPolicyGuard (Plan 3b Task 1) is registered directly below as this
// module's own APP_GUARD, never per-controller — so it is the one place
// that needs `forFeature` for its three repositories; `Users` is not
// otherwise global (JwtAuthGuard and friends inject `EntityManager`
// instead precisely to avoid needing this everywhere — see their own
// docstrings).
MikroOrmModule.forFeature([AppSettings, Users, WebauthnCredentials]),
],
providers: [
// Default-deny: a route is authenticated unless it carries @Public() or
+1 -1
View File
@@ -1097,6 +1097,6 @@ export class AuthService {
* route) should go through.
*/
async verifyJwtToken(token: string): Promise<User | null> {
return verifyJwtAndLoadUser(token);
return verifyJwtAndLoadUser(token, this.usersRepo);
}
}
+10 -1
View File
@@ -1,5 +1,7 @@
import { CanActivate, ExecutionContext, HttpException, Injectable } from '@nestjs/common';
import { EntityManager } from '@mikro-orm/core';
import type { Request } from 'express';
import { Users } from '../../db/entities/Users.entity';
import { verifyJwtAndLoadUser } from './jwt-verify';
/**
@@ -7,16 +9,23 @@ import { verifyJwtAndLoadUser } from './jwt-verify';
* trek_session cookie (never a Bearer token), so CSRF-sensitive state-changing
* OAuth endpoints (consent submit, client/session mutations) can't be driven by
* a leaked Bearer. Error bodies + codes match the legacy 401 shapes exactly.
*
* `EntityManager` injection, not `@InjectRepository(Users)` — same reasoning
* as `JwtAuthGuard` (Plan 3b Task 1 RULING): the guard is instantiated per the
* HOST controller's module, and using the always-global `EntityManager`
* avoids new `forFeature` wiring in every module that applies it.
*/
@Injectable()
export class CookieAuthGuard implements CanActivate {
constructor(private readonly em: EntityManager) {}
async canActivate(context: ExecutionContext): Promise<boolean> {
const req = context.switchToHttp().getRequest<Request & { cookies?: Record<string, string> }>();
const cookieToken = req.cookies?.trek_session;
if (!cookieToken) {
throw new HttpException({ error: 'Cookie session required for this endpoint', code: 'COOKIE_AUTH_REQUIRED' }, 401);
}
const user = await verifyJwtAndLoadUser(cookieToken);
const user = await verifyJwtAndLoadUser(cookieToken, this.em.getRepository(Users));
if (!user) {
throw new HttpException({ error: 'Invalid or expired session', code: 'AUTH_REQUIRED' }, 401);
}
+13 -4
View File
@@ -1,7 +1,9 @@
import { CanActivate, ExecutionContext, HttpException, Injectable } from '@nestjs/common';
import { GUARDS_METADATA } from '@nestjs/common/constants';
import { Reflector } from '@nestjs/core';
import { EntityManager } from '@mikro-orm/core';
import type { Request } from 'express';
import { Users } from '../../db/entities/Users.entity';
import { extractToken, verifyJwtAndLoadUser } from './jwt-verify';
import { IS_PUBLIC, OPTIONAL_AUTH } from './public.decorator';
@@ -28,7 +30,14 @@ import { IS_PUBLIC, OPTIONAL_AUTH } from './public.decorator';
*/
@Injectable()
export class GlobalAuthGuard implements CanActivate {
constructor(private readonly reflector: Reflector) {}
// EntityManager injection, not @InjectRepository(Users) — same reasoning
// as JwtAuthGuard (Plan 3b Task 1 RULING): kept uniform with the other
// three verifyJwtAndLoadUser callers even though this guard, registered
// only as AppModule's own APP_GUARD, would have tolerated either.
constructor(
private readonly reflector: Reflector,
private readonly em: EntityManager,
) {}
async canActivate(context: ExecutionContext): Promise<boolean> {
const handler = context.getHandler();
@@ -40,7 +49,7 @@ export class GlobalAuthGuard implements CanActivate {
if (this.reflector.getAllAndOverride(OPTIONAL_AUTH, [handler, controller])) {
const token = extractToken(req);
(req as { user: unknown }).user = (token ? await verifyJwtAndLoadUser(token) : null) || null;
(req as { user: unknown }).user = (token ? await verifyJwtAndLoadUser(token, this.em.getRepository(Users)) : null) || null;
return true;
}
@@ -57,7 +66,7 @@ export class GlobalAuthGuard implements CanActivate {
];
if (declared.length > 0) {
const declaredToken = extractToken(req);
(req as { user: unknown }).user = (declaredToken ? await verifyJwtAndLoadUser(declaredToken) : null) || null;
(req as { user: unknown }).user = (declaredToken ? await verifyJwtAndLoadUser(declaredToken, this.em.getRepository(Users)) : null) || null;
return true;
}
@@ -65,7 +74,7 @@ export class GlobalAuthGuard implements CanActivate {
if (!token) {
throw new HttpException({ error: 'Access token required', code: 'AUTH_REQUIRED' }, 401);
}
const user = await verifyJwtAndLoadUser(token);
const user = await verifyJwtAndLoadUser(token, this.em.getRepository(Users));
if (!user) {
throw new HttpException({ error: 'Invalid or expired token', code: 'AUTH_REQUIRED' }, 401);
}
+24 -1
View File
@@ -1,5 +1,7 @@
import { CanActivate, ExecutionContext, HttpException, Injectable } from '@nestjs/common';
import { EntityManager } from '@mikro-orm/core';
import type { Request } from 'express';
import { Users } from '../../db/entities/Users.entity';
import { extractToken, verifyJwtAndLoadUser } from './jwt-verify';
/**
@@ -9,16 +11,37 @@ import { extractToken, verifyJwtAndLoadUser } from './jwt-verify';
* and the loaded user are IDENTICAL to the Express middleware. No new tokens.
*
* Error bodies match the legacy 401 shape exactly so the client is unaffected.
*
* Injects `EntityManager`, not `@InjectRepository(Users)` (Plan 3b Task 1
* RULING on `verifyJwtAndLoadUser`'s callers): this guard is applied via
* `@UseGuards(JwtAuthGuard)` on ~70 controllers across nearly every domain
* module in the app, and Nest resolves a class-referenced guard's
* constructor dependencies from the HOST CONTROLLER'S OWN module graph, not
* from a single "the guard's module" — `@InjectRepository(Users)` would need
* `MikroOrmModule.forFeature([Users])` added to every one of those ~70
* modules (and to every partial e2e-testing-module harness that composes a
* guarded controller without the full app), which is the exact blast radius
* D5 exists to avoid. `EntityManager`/`MikroORM`, by contrast, come from
* `MikroOrmModule.forRoot`'s core module, which IS `@Global()` — already
* required wherever a JWT-guarded route is exercised for a real user (the
* ORM owns `users` everywhere else too) — so no module needs new wiring.
* `this.em.getRepository(Users)` inside `canActivate` is the same
* `orm.em.getRepository(Users)`-inside-the-request pattern
* `platform.routes.ts::servePhoto` uses (Task 0); `this.em` here is already
* the request-scoped fork, since `canActivate` only ever runs inside a real
* HTTP request Nest's `@mikro-orm/nestjs` middleware has already wrapped.
*/
@Injectable()
export class JwtAuthGuard implements CanActivate {
constructor(private readonly em: EntityManager) {}
async canActivate(context: ExecutionContext): Promise<boolean> {
const req = context.switchToHttp().getRequest<Request>();
const token = extractToken(req);
if (!token) {
throw new HttpException({ error: 'Access token required', code: 'AUTH_REQUIRED' }, 401);
}
const user = await verifyJwtAndLoadUser(token);
const user = await verifyJwtAndLoadUser(token, this.em.getRepository(Users));
if (!user) {
throw new HttpException({ error: 'Invalid or expired token', code: 'AUTH_REQUIRED' }, 401);
}
+13 -5
View File
@@ -1,8 +1,8 @@
import type { Request } from 'express';
import jwt from 'jsonwebtoken';
import { db } from '../../db/database';
import { JWT_SECRET } from '../../config';
import type { User } from '../../types';
import type { UsersRepository } from '../../db/repositories/Users.repository';
/**
* The canonical JWT session check. Every auth surface goes through here — the
@@ -15,6 +15,16 @@ import type { User } from '../../types';
* would have no way to inject one. Note JWT_SECRET is deliberately a live
* binding from src/config, not an app-config value: the admin panel rotates it
* at runtime and `export let` is what makes a rotation take effect in-process.
*
* `verifyJwtAndLoadUser` stays a free function (Plan 3b Task 1 RULING) and
* gains an explicit `users: UsersRepository` parameter — DI over a
* module-level accessor (CLAUDE.md "DI over global mutable module state").
* This file imports neither the ORM nor `RequestContext`: every caller
* resolves its own `UsersRepository` (through Nest DI, or — for the one
* pre-init caller, `platform.routes.ts::servePhoto` — `orm.em.getRepository(Users)`
* resolved INSIDE `applyPlatformUploads`'s `withRequestContext` wrapper,
* `orm` threaded in from `bootstrap.ts`) and passes it in. The legacy `db`
* proxy import is gone.
*/
export function extractToken(req: Request): string | null {
// Prefer httpOnly cookie; fall back to Authorization: Bearer (MCP, API clients)
@@ -54,16 +64,14 @@ export function decodeSessionClaims(token: string | undefined): SessionClaims |
return decoded as SessionClaims;
}
export async function verifyJwtAndLoadUser(token: string): Promise<User | null> {
export async function verifyJwtAndLoadUser(token: string, users: UsersRepository): Promise<User | null> {
try {
const decoded = jwt.verify(token, JWT_SECRET, { algorithms: ['HS256'] }) as { id: number; pv?: number; purpose?: string };
// Purpose-scoped tokens (e.g. the short-lived mfa_login token) share this
// secret but are not full session tokens — only their dedicated endpoint
// may accept them, so reject any token carrying a purpose claim here.
if (decoded.purpose) return null;
const row = db.prepare(
'SELECT id, username, email, role, password_version FROM users WHERE id = ?'
).get(decoded.id) as (User & { password_version?: number }) | undefined;
const row = await users.findByIdWithPasswordVersion(decoded.id);
if (!row) return null;
// Session invalidation: any token whose embedded password_version
// predates the user's current one is rejected. Tokens issued before
+15 -12
View File
@@ -1,11 +1,17 @@
import { CanActivate, ExecutionContext, HttpException, Injectable, SetMetadata } from '@nestjs/common';
import { Reflector } from '@nestjs/core';
import { InjectRepository } from '@mikro-orm/nestjs';
import type { Request } from 'express';
import { DatabaseService } from '../database/database.service';
import { RuntimeEnvService } from '../app-config/runtime-env.service';
import { DEMO_EMAILS } from '../common/demo';
import { IS_PUBLIC } from './public.decorator';
import type { User } from '../../types';
import { AppSettings } from '../../db/entities/AppSettings.entity';
import type { AppSettingsRepository } from '../../db/repositories/AppSettings.repository';
import { Users } from '../../db/entities/Users.entity';
import type { UsersRepository } from '../../db/repositories/Users.repository';
import { WebauthnCredentials } from '../../db/entities/WebauthnCredentials.entity';
import type { WebauthnCredentialsRepository } from '../../db/repositories/WebauthnCredentials.repository';
/** Metadata key `@MfaExempt()` writes. */
export const MFA_EXEMPT = 'trek:mfa-exempt';
@@ -44,7 +50,9 @@ type MfaRequest = Request & { user?: User };
@Injectable()
export class MfaPolicyGuard implements CanActivate {
constructor(
private readonly db: DatabaseService,
@InjectRepository(AppSettings) private readonly appSettings: AppSettingsRepository,
@InjectRepository(Users) private readonly users: UsersRepository,
@InjectRepository(WebauthnCredentials) private readonly webauthnCredentials: WebauthnCredentialsRepository,
private readonly env: RuntimeEnvService,
private readonly reflector: Reflector,
) {}
@@ -60,24 +68,19 @@ export class MfaPolicyGuard implements CanActivate {
// No session: whoever answers next decides, exactly as before.
if (!user) return true;
const requireRow = this.db.get<{ value: string }>(
"SELECT value FROM app_settings WHERE key = 'require_mfa'",
);
if (requireRow?.value !== 'true') return true;
const requireMfa = await this.appSettings.getValue('require_mfa');
if (requireMfa !== 'true') return true;
if (this.env.isDemoMode() && user.email && DEMO_EMAILS.has(user.email)) return true;
const row = this.db.get<{ mfa_enabled: number | boolean }>(
'SELECT mfa_enabled FROM users WHERE id = ?',
user.id,
);
const row = await this.users.getMfaEnabled(user.id);
if (!row) return true;
// A user-verified passkey is phishing-resistant and inherently two-factor, so
// owning at least one satisfies require_mfa exactly like TOTP does.
// (All stored passkeys were registered with userVerification required.)
const mfaOk = row.mfa_enabled === 1 || row.mfa_enabled === true;
const passkeyOk = !!this.db.get('SELECT 1 FROM webauthn_credentials WHERE user_id = ? LIMIT 1', user.id);
const mfaOk = row.mfa_enabled === 1;
const passkeyOk = await this.webauthnCredentials.hasAny(user.id);
if (mfaOk || passkeyOk) return true;
if (this.reflector.getAllAndOverride(MFA_EXEMPT, [handler, controller])) return true;
+8 -1
View File
@@ -1,5 +1,7 @@
import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common';
import { EntityManager } from '@mikro-orm/core';
import type { Request } from 'express';
import { Users } from '../../db/entities/Users.entity';
import { extractToken, verifyJwtAndLoadUser } from './jwt-verify';
/**
@@ -7,13 +9,18 @@ import { extractToken, verifyJwtAndLoadUser } from './jwt-verify';
* loaded user when a valid token is present, otherwise leaves it null — and
* always allows the request through (never 401). Used for endpoints whose
* response varies by auth state but don't require it (e.g. /app-config).
*
* `EntityManager` injection, not `@InjectRepository(Users)` — same reasoning
* as `JwtAuthGuard` (Plan 3b Task 1 RULING).
*/
@Injectable()
export class OptionalJwtGuard implements CanActivate {
constructor(private readonly em: EntityManager) {}
async canActivate(context: ExecutionContext): Promise<boolean> {
const req = context.switchToHttp().getRequest<Request>();
const token = extractToken(req);
(req as { user: unknown }).user = (token ? await verifyJwtAndLoadUser(token) : null) || null;
(req as { user: unknown }).user = (token ? await verifyJwtAndLoadUser(token, this.em.getRepository(Users)) : null) || null;
return true;
}
}
+55 -75
View File
@@ -1,7 +1,6 @@
import { Injectable } from '@nestjs/common';
import { InjectRepository } from '@mikro-orm/nestjs';
import { readEnv, getAppUrl } from '../../app-config';
import { DatabaseService } from '../database/database.service';
import { UnitOfWork } from '../database/unit-of-work';
import { StorageService } from '../storage/storage.service';
import { decrypt_api_key, maybe_encrypt_api_key } from '../common/crypto/apiKeyCrypto';
@@ -16,11 +15,10 @@ import {
type InstanceApiKeyName,
} from '../settings/instance-api-keys';
import { SEARCH_TEXT_FIELD_MASK } from '../maps/maps.helpers';
import { User } from '../../types';
import { AppSettings } from '../../db/entities/AppSettings.entity';
import type { AppSettingsRepository } from '../../db/repositories/AppSettings.repository';
import { Users } from '../../db/entities/Users.entity';
import type { UsersRepository } from '../../db/repositories/Users.repository';
import type { UsersRepository, UserApiKeyColumns, UserProfilePatch } from '../../db/repositories/Users.repository';
/**
* The account a user administers about themselves: display settings, avatar,
@@ -30,20 +28,19 @@ import type { UsersRepository } from '../../db/repositories/Users.repository';
* Split out of AuthService, which had grown to 1471 lines by owning identity,
* profile, settings and tokens together. None of this is identity: no password
* is checked here, no session is issued, no MFA secret is touched. Everything
* moved verbatim — same SQL, same validation order, same masking, same error
* strings and status codes.
* moved verbatim — same statement shapes, same validation order, same masking,
* same error strings and status codes.
*
* DatabaseService was the only injected dependency at first, which is what
* made this the second-cheapest cut after tokens; `AppSettingsRepository`/
* `UsersRepository` (Plan 3a Task 5) were added later purely so the three
* `instance-api-keys.ts` calls below pass their own repository rather than
* that file resolving one itself — every other read/write here is still
* `DatabaseService`'s raw SQL (auth is outside Plan 3a).
* Plan 3b Task 1: every read/write here now goes through `UsersRepository`
* (`AppSettingsRepository`/`UsersRepository` were already present, Plan 3a
* Task 5, purely so `instance-api-keys.ts` could take its own repository
* rather than resolving one itself); `DatabaseService` is gone from the
* constructor entirely — nothing in this file reads or writes raw SQL any
* more.
*/
@Injectable()
export class UserProfileService {
constructor(
private readonly db: DatabaseService,
private readonly storage: StorageService,
private readonly uow: UnitOfWork,
@InjectRepository(AppSettings) private readonly appSettings: AppSettingsRepository,
@@ -64,12 +61,12 @@ export class UserProfileService {
return readEnv().managed.enabled;
}
/** The three key columns plus the role that decides where a save lands. */
private async currentKeys(userId: number) {
return this.db.get<Pick<User, 'role' | 'maps_api_key' | 'openweather_api_key' | 'unsplash_api_key' | 'amap_api_key'>>(
'SELECT role, maps_api_key, openweather_api_key, unsplash_api_key, amap_api_key FROM users WHERE id = ?',
userId
);
/**
* The three key columns plus the role that decides where a save lands.
* `SELECT role, maps_api_key, openweather_api_key, unsplash_api_key, amap_api_key FROM users WHERE id = ?` (UP1).
*/
private async currentKeys(userId: number): Promise<UserApiKeyColumns | null> {
return this.usersRepo.getApiKeyColumns(userId);
}
/**
@@ -81,7 +78,7 @@ export class UserProfileService {
*/
private async storedKeyPlaintext(
name: 'maps_api_key' | 'openweather_api_key' | 'unsplash_api_key' | 'amap_api_key',
current: Pick<User, 'maps_api_key' | 'openweather_api_key' | 'unsplash_api_key' | 'amap_api_key'> | undefined,
current: UserApiKeyColumns | null,
isAdmin: boolean,
): Promise<string> {
if (isAdmin && (INSTANCE_API_KEY_NAMES as readonly string[]).includes(name)) {
@@ -104,7 +101,7 @@ export class UserProfileService {
*/
private async changedKeyNames(
body: Record<string, unknown>,
current: Pick<User, 'maps_api_key' | 'openweather_api_key' | 'unsplash_api_key' | 'amap_api_key'> | undefined,
current: UserApiKeyColumns | null,
isAdmin: boolean,
skipped: string[] = [],
): Promise<string[]> {
@@ -143,10 +140,7 @@ export class UserProfileService {
const isAdmin = current?.role === 'admin';
const changedKeys = await this.changedKeyNames({ maps_api_key }, current, isAdmin);
await this.uow.transactional(async () => {
this.db.run(
'UPDATE users SET maps_api_key = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?',
maybe_encrypt_api_key(maps_api_key), userId
);
await this.usersRepo.updateMapsKey(userId, maybe_encrypt_api_key(maps_api_key));
await this.mirrorInstanceKeys({ maps_api_key }, isAdmin);
});
return { success: true, maps_api_key: mask_stored_api_key(maps_api_key), changedKeys };
@@ -163,23 +157,18 @@ export class UserProfileService {
await this.uow.transactional(async () => {
// `?? null` instead of the former non-null assertions: a user row deleted
// mid-request must degrade to a 0-row UPDATE, not a TypeError/500.
this.db.run(
'UPDATE users SET maps_api_key = ?, openweather_api_key = ?, unsplash_api_key = ?, amap_api_key = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?',
body.maps_api_key !== undefined ? maybe_encrypt_api_key(body.maps_api_key) : current?.maps_api_key ?? null,
body.openweather_api_key !== undefined ? maybe_encrypt_api_key(body.openweather_api_key) : current?.openweather_api_key ?? null,
body.unsplash_api_key !== undefined ? maybe_encrypt_api_key(body.unsplash_api_key) : current?.unsplash_api_key ?? null,
body.amap_api_key !== undefined ? maybe_encrypt_api_key(body.amap_api_key) : current?.amap_api_key ?? null,
userId
);
await this.usersRepo.updateApiKeys(userId, {
maps_api_key: body.maps_api_key !== undefined ? maybe_encrypt_api_key(body.maps_api_key) : current?.maps_api_key ?? null,
openweather_api_key: body.openweather_api_key !== undefined ? maybe_encrypt_api_key(body.openweather_api_key) : current?.openweather_api_key ?? null,
unsplash_api_key: body.unsplash_api_key !== undefined ? maybe_encrypt_api_key(body.unsplash_api_key) : current?.unsplash_api_key ?? null,
amap_api_key: body.amap_api_key !== undefined ? maybe_encrypt_api_key(body.amap_api_key) : current?.amap_api_key ?? null,
});
await this.mirrorInstanceKeys(body, isAdmin);
});
const updated = this.db.get<Pick<User, 'id' | 'username' | 'email' | 'role' | 'maps_api_key' | 'openweather_api_key' | 'unsplash_api_key' | 'amap_api_key' | 'avatar' | 'mfa_enabled'>>(
'SELECT id, username, email, role, maps_api_key, openweather_api_key, unsplash_api_key, amap_api_key, avatar, mfa_enabled FROM users WHERE id = ?',
userId
);
const updated = await this.usersRepo.findProfileWithKeys(userId);
const u = updated ? { ...updated, mfa_enabled: !!(updated.mfa_enabled === 1 || updated.mfa_enabled === true) } : undefined;
const u = updated ? { ...updated, mfa_enabled: !!(updated.mfa_enabled === 1) } : undefined;
return {
success: true,
...(blocked.length ? { managed_keys: blocked } : {}),
@@ -203,7 +192,7 @@ export class UserProfileService {
if (!/^[a-zA-Z0-9_.-]+$/.test(trimmed)) {
return { error: 'Username can only contain letters, numbers, underscores, dots and hyphens', status: 400 };
}
const conflict = this.db.get('SELECT id FROM users WHERE LOWER(username) = LOWER(?) AND id != ? AND COALESCE(is_guest, 0) = 0', trimmed, userId);
const conflict = await this.usersRepo.findIdByUsernameCI(trimmed, userId);
if (conflict) return { error: 'Username already taken', status: 409 };
}
@@ -212,24 +201,26 @@ export class UserProfileService {
if (!trimmed || !EMAIL_REGEX.test(trimmed)) {
return { error: 'Invalid email format', status: 400 };
}
const conflict = this.db.get('SELECT id FROM users WHERE LOWER(email) = LOWER(?) AND id != ? AND COALESCE(is_guest, 0) = 0', trimmed, userId);
const conflict = await this.usersRepo.findIdByEmailCI(trimmed, userId);
if (conflict) return { error: 'Email already taken', status: 409 };
}
const updates: string[] = [];
const params: (string | number | null)[] = [];
// The name and email half of this body stays the user's own in every mode;
// only the three key columns answer to the operator.
const { blocked } = splitManagedKeys(body, this.managed);
const keyLocked = blocked.length > 0;
if (maps_api_key !== undefined && !keyLocked) { updates.push('maps_api_key = ?'); params.push(maybe_encrypt_api_key(maps_api_key)); }
if (openweather_api_key !== undefined && !keyLocked) { updates.push('openweather_api_key = ?'); params.push(maybe_encrypt_api_key(openweather_api_key)); }
if (unsplash_api_key !== undefined && !keyLocked) { updates.push('unsplash_api_key = ?'); params.push(maybe_encrypt_api_key(unsplash_api_key)); }
if (amap_api_key !== undefined && !keyLocked) { updates.push('amap_api_key = ?'); params.push(maybe_encrypt_api_key(amap_api_key)); }
if (username !== undefined) { updates.push('username = ?'); params.push(username.trim()); }
if (email !== undefined) { updates.push('email = ?'); params.push(email.trim()); }
// The bounded set of columns UP7's dynamic SET clause could touch — built
// here, at the call site, exactly as the legacy `updates`/`params` arrays
// were; `UsersRepository.patchProfile` writes this typed partial in ONE
// statement, never a built SQL string.
const changes: UserProfilePatch = {};
if (maps_api_key !== undefined && !keyLocked) changes.maps_api_key = maybe_encrypt_api_key(maps_api_key);
if (openweather_api_key !== undefined && !keyLocked) changes.openweather_api_key = maybe_encrypt_api_key(openweather_api_key);
if (unsplash_api_key !== undefined && !keyLocked) changes.unsplash_api_key = maybe_encrypt_api_key(unsplash_api_key);
if (amap_api_key !== undefined && !keyLocked) changes.amap_api_key = maybe_encrypt_api_key(amap_api_key);
if (username !== undefined) changes.username = username.trim();
if (email !== undefined) changes.email = email.trim();
// Read before the write, so the comparison sees the old value; the role in
// the same row decides whether the two instance-wide names travel with it.
@@ -237,21 +228,16 @@ export class UserProfileService {
const isAdmin = current?.role === 'admin';
const changedKeys = keyLocked ? [] : await this.changedKeyNames(body, current, isAdmin, blocked);
if (updates.length > 0) {
updates.push('updated_at = CURRENT_TIMESTAMP');
params.push(userId);
if (Object.keys(changes).length > 0) {
await this.uow.transactional(async () => {
this.db.run(`UPDATE users SET ${updates.join(', ')} WHERE id = ?`, ...params);
await this.usersRepo.patchProfile(userId, changes);
if (!keyLocked) await this.mirrorInstanceKeys(body, isAdmin);
});
}
const updated = this.db.get<Pick<User, 'id' | 'username' | 'email' | 'role' | 'maps_api_key' | 'openweather_api_key' | 'unsplash_api_key' | 'amap_api_key' | 'avatar' | 'mfa_enabled'>>(
'SELECT id, username, email, role, maps_api_key, openweather_api_key, unsplash_api_key, amap_api_key, avatar, mfa_enabled FROM users WHERE id = ?',
userId
);
const updated = await this.usersRepo.findProfileWithKeys(userId);
const u = updated ? { ...updated, mfa_enabled: !!(updated.mfa_enabled === 1 || updated.mfa_enabled === true) } : undefined;
const u = updated ? { ...updated, mfa_enabled: !!(updated.mfa_enabled === 1) } : undefined;
return {
success: true,
...(blocked.length ? { managed_keys: blocked } : {}),
@@ -261,10 +247,7 @@ export class UserProfileService {
}
async getSettings(userId: number): Promise<{ error?: string; status?: number; settings?: Record<string, unknown> }> {
const user = this.db.get<Pick<User, 'role' | 'maps_api_key' | 'openweather_api_key' | 'unsplash_api_key' | 'amap_api_key'>>(
'SELECT role, maps_api_key, openweather_api_key, unsplash_api_key, amap_api_key FROM users WHERE id = ?',
userId
);
const user = await this.usersRepo.getApiKeyColumns(userId);
if (user?.role !== 'admin') return { error: 'Admin access required', status: 403 };
// The one endpoint in the codebase that hands back a stored key in the
@@ -303,29 +286,29 @@ export class UserProfileService {
// -------------------------------------------------------------------------
async saveAvatar(userId: number, filename: string) {
const current = this.db.get<{ avatar: string | null }>('SELECT avatar FROM users WHERE id = ?', userId);
const current = await this.usersRepo.getAvatar(userId);
// Only a locally uploaded file has something to clean up. An OIDC picture URL
// (#1399) has no storage object, so skip the delete entirely.
if (current?.avatar && !/^https:\/\//i.test(current.avatar)) {
if (current && !/^https:\/\//i.test(current)) {
// Fire-and-forget parity: leftover objects are harmless; the DB update is
// the source of truth for which avatar is current. The catch also
// swallows a hostile stored value the central key validation rejects.
await this.storage.delete('avatars', current.avatar).catch(() => {});
await this.storage.delete('avatars', current).catch(() => {});
}
this.db.run('UPDATE users SET avatar = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?', filename, userId);
await this.usersRepo.setAvatar(userId, filename);
const updated = this.db.get<Pick<User, 'id' | 'username' | 'email' | 'role' | 'avatar'>>('SELECT id, username, email, role, avatar FROM users WHERE id = ?', userId);
const updated = await this.usersRepo.findProfileBasic(userId);
return { success: true, avatar_url: avatarUrl(updated || {}) };
}
async deleteAvatar(userId: number) {
const current = this.db.get<{ avatar: string | null }>('SELECT avatar FROM users WHERE id = ?', userId);
const current = await this.usersRepo.getAvatar(userId);
// An OIDC picture URL (#1399) has no storage object — only delete an uploaded one.
if (current?.avatar && !/^https:\/\//i.test(current.avatar)) {
await this.storage.delete('avatars', current.avatar).catch(() => {});
if (current && !/^https:\/\//i.test(current)) {
await this.storage.delete('avatars', current).catch(() => {});
}
this.db.run('UPDATE users SET avatar = NULL, updated_at = CURRENT_TIMESTAMP WHERE id = ?', userId);
await this.usersRepo.setAvatar(userId, null);
return { success: true };
}
@@ -336,10 +319,7 @@ export class UserProfileService {
async listUsers(excludeUserId: number) {
// The global user directory feeds the trip member-add / contributor pickers —
// guests (#1362) are trip-scoped and must never be selectable here.
const users = this.db.all<Pick<User, 'id' | 'username' | 'avatar'>>(
'SELECT id, username, avatar FROM users WHERE id != ? AND COALESCE(is_guest, 0) = 0 ORDER BY username ASC',
excludeUserId
);
const users = await this.usersRepo.listOthersNonGuest(excludeUserId);
return users.map(u => ({ ...u, avatar_url: avatarUrl(u) }));
}
@@ -348,7 +328,7 @@ export class UserProfileService {
// -------------------------------------------------------------------------
async validateKeys(userId: number): Promise<{ error?: string; status?: number; maps: boolean; weather: boolean; maps_details: null | { ok: boolean; status: number | null; status_text: string | null; error_message: string | null; error_status: string | null; error_raw: string | null } }> {
const user = this.db.get<Pick<User, 'role' | 'openweather_api_key'>>('SELECT role, openweather_api_key FROM users WHERE id = ?', userId);
const user = await this.usersRepo.getRoleAndWeatherKey(userId);
if (user?.role !== 'admin') return { error: 'Admin access required', status: 403, maps: false, weather: false, maps_details: null };
const result: {
@@ -1,6 +1,8 @@
import { Injectable } from '@nestjs/common';
import { InjectRepository } from '@mikro-orm/nestjs';
import { getAppUrl, readEnv } from '../../app-config';
import { DatabaseService } from '../database/database.service';
import { AppSettings } from '../../db/entities/AppSettings.entity';
import type { AppSettingsRepository } from '../../db/repositories/AppSettings.repository';
function hostOf(url: string): string | null {
try {
@@ -68,10 +70,11 @@ export interface WebauthnConfig {
*/
@Injectable()
export class WebauthnConfigService {
constructor(private readonly db: DatabaseService) {}
constructor(@InjectRepository(AppSettings) private readonly appSettings: AppSettingsRepository) {}
/** `SELECT value FROM app_settings WHERE key = ?` (WC1), through `AppSettingsRepository.getValue` (Plan 3a). */
private async setting(key: string): Promise<string | null> {
const raw = this.db.get<{ value: string }>('SELECT value FROM app_settings WHERE key = ?', key)?.value;
const raw = (await this.appSettings.getValue(key)) ?? undefined;
const trimmed = raw?.trim();
return trimmed ? trimmed : null;
}
+8 -1
View File
@@ -8,6 +8,8 @@ import { PermissionsService } from '../permissions/permissions.service';
import { avatarUrl } from '../common/avatarUrl';
import { EphemeralTokenService } from '../auth/ephemeral-token.service';
import { verifyJwtAndLoadUser } from '../auth/jwt-verify';
import { EntityManager } from '@mikro-orm/core';
import { Users } from '../../db/entities/Users.entity';
import type { User, TripFile } from '../../types';
import { DatabaseService, type TripAccess } from '../database/database.service';
import { DEFAULT_ALLOWED_EXTENSIONS } from './files.constants';
@@ -79,6 +81,11 @@ export class FilesService {
private readonly realtime: RealtimeService,
private readonly tokens: EphemeralTokenService,
private readonly storage: StorageService,
// EntityManager, not @InjectRepository(Users) — same reasoning as
// JwtAuthGuard (Plan 3b Task 1 RULING on verifyJwtAndLoadUser's callers):
// kept uniform with the guards rather than adding a one-off
// MikroOrmModule.forFeature([Users]) to FilesModule for this single call.
private readonly em: EntityManager,
) {}
async verifyTripAccess(tripId: string | number, userId: number) {
@@ -110,7 +117,7 @@ export class FilesService {
// Use the shared helper so the password_version gate applies here too;
// previously this bypassed the check and stolen download tokens stayed
// valid across a password reset.
const user = await verifyJwtAndLoadUser(jwtToken);
const user = await verifyJwtAndLoadUser(jwtToken, this.em.getRepository(Users));
if (!user) return { error: 'Invalid or expired token', status: 401 };
return { userId: user.id };
}
+18 -11
View File
@@ -8,6 +8,8 @@ import { db } from '../../db/database';
import { withRequestContext } from '../database/request-context';
import { StorageService } from '../storage/storage.service';
import { StorageInvalidKeyError, StorageNotFoundError, type StorageCategory } from '../storage/storage.types';
import { Users } from '../../db/entities/Users.entity';
import type { UsersRepository } from '../../db/repositories/Users.repository';
// Platform / transport routes extracted verbatim from createApp() (app.ts) so they can be
// mounted on either the legacy Express app or the NestJS Express instance (strangler A6/A8).
@@ -72,7 +74,7 @@ export function storageStaticHandler(storage: StorageService, category: StorageC
};
}
async function servePhoto(storage: StorageService, req: Request, res: Response): Promise<void> {
async function servePhoto(storage: StorageService, req: Request, res: Response, users: UsersRepository): Promise<void> {
const safeName = path.basename(req.params.filename);
// Parity: after basename(), the old resolve()+startsWith guard could only
// fire when the remaining segment was '..' — keep that exact 403.
@@ -104,7 +106,7 @@ async function servePhoto(storage: StorageService, req: Request, res: Response):
}
// JWT session path (with pv check).
const user = await verifyJwtAndLoadUser(rawToken);
const user = await verifyJwtAndLoadUser(rawToken, users);
if (user) return sendPhoto();
// Share-token path: require the token to cover the exact trip the
@@ -132,14 +134,19 @@ async function servePhoto(storage: StorageService, req: Request, res: Response):
* mounted here, on the raw Express instance, BEFORE `app.init()` — every
* `/uploads/photos/*` request runs this handler entirely outside Nest's
* per-request EntityManager fork, forever, not just during a boot window.
* `verifyJwtAndLoadUser` (`jwt-verify.ts`) reads `users` through the legacy
* `db` proxy today, so nothing here needs the ORM yet — but the inventory's
* proposed conversion (`UsersRepository.findByIdWithPasswordVersion`, Task 1)
* would throw `cannotUseGlobalContext` on every photo request the instant it
* lands, because nothing forks a context for this pre-init route. Wrapping
* the handler in `withRequestContext` NOW, before any repository call exists
* on this path, means Task 1 converts `jwt-verify.ts` without touching this
* file again and without a single broken photo request in between.
* `verifyJwtAndLoadUser` (`jwt-verify.ts`, JV1) now reads `users` through
* `UsersRepository` (Plan 3b Task 1) rather than the legacy `db` proxy — the
* exact case Task 0's wrap was built for. `orm.em.getRepository(Users)` is
* resolved inside the arrow passed to `withRequestContext` below: `orm.em`
* is the global, context-resolving EntityManager, and MikroORM's repository
* object holds a reference to that same proxy rather than a snapshot, so a
* query issued through it later resolves whatever `AsyncLocalStorage`
* context is active AT QUERY TIME — not at the moment `getRepository()` was
* called. What actually gates this is `withRequestContext` wrapping the
* whole `servePhoto(...)` call (query included): removing the wrapper
* entirely reproduces Task 0's `ValidationError: Using global EntityManager
* instance...` (verified directly — mutation-proof in `task-1-report.md`),
* exactly the failure `PHOTOCTX-002`/`SEAM-002` guard against.
*
* Optional at the type level only, the same shape `TrekWsAdapter` uses for
* its own D6 wrapper (`src/nest/realtime/trek-ws.adapter.ts`): `bootstrap.ts`
@@ -184,7 +191,7 @@ export function applyPlatformUploads(app: express.Application, storage: StorageS
next(new Error('applyPlatformUploads: no MikroORM available to build a request context for /uploads/photos/*'));
return;
}
return withRequestContext(orm, () => servePhoto(storage, req, res)).catch(next);
return withRequestContext(orm, () => servePhoto(storage, req, res, orm.em.getRepository(Users))).catch(next);
});
// Block direct access to /uploads/files
@@ -1,9 +1,11 @@
import { All, Controller, Param, Req, Res } from '@nestjs/common';
import { EntityManager } from '@mikro-orm/core';
import type { Request, Response } from 'express';
import { extractToken, verifyJwtAndLoadUser } from '../auth/jwt-verify';
import { pluginsEnabled } from './kill-switch';
import { PluginRuntimeService } from './plugin-runtime.service';
import { Public } from '../auth/public.decorator';
import { Users } from '../../db/entities/Users.entity';
/**
* Proxies a plugin's own HTTP routes at /api/plugins/:id/* (#plugins, M2).
@@ -74,7 +76,12 @@ function toRelativeLocation(loc: unknown): string | null {
@Public('auth is data-driven per plugin route (route.auth in the manifest), asserted in the handler')
@Controller('api/plugins/:pluginId')
export class PluginsProxyController {
constructor(private readonly runtime: PluginRuntimeService) {}
// EntityManager, not @InjectRepository(Users) — same reasoning as
// JwtAuthGuard (Plan 3b Task 1 RULING on verifyJwtAndLoadUser's callers).
constructor(
private readonly runtime: PluginRuntimeService,
private readonly em: EntityManager,
) {}
@All('*path')
async proxy(@Param('pluginId') pluginId: string, @Req() req: Request, @Res() res: Response): Promise<void> {
@@ -95,7 +102,7 @@ export class PluginsProxyController {
let user: { id: number; username: string; role?: 'admin' | 'user' } | null = null;
if (route.auth) {
const token = extractToken(req);
const loaded = token ? await verifyJwtAndLoadUser(token) : null;
const loaded = token ? await verifyJwtAndLoadUser(token, this.em.getRepository(Users)) : null;
if (!loaded) {
res.status(401).json({ error: 'Access token required', code: 'AUTH_REQUIRED' });
return;
@@ -89,6 +89,15 @@ function withinBurst(socket: TrekWebSocket): boolean {
* `error` event on the socket; unhandled, Node rethrows it and the process
* dies. That is #1576, and it only reproduces against a hostile client, so
* nothing in CI would catch its removal.
*
* All three per-connection lifecycle hooks the base adapter offers —
* `bindClientConnect` (`OnGatewayConnection`), `bindMessageHandlers`
* (`@SubscribeMessage`) and `bindClientDisconnect` (`OnGatewayDisconnect`) —
* are overridden here to run their callback inside `withRequestContext`
* (D6): none of the three has an HTTP request behind it, so none gets a
* forked EntityManager unless this adapter forks one itself. `create`
* above and `close` below are the only other overrides, and neither
* dispatches into gateway/handler code, so neither needs the wrap.
*/
export class TrekWsAdapter extends WsAdapter {
private readonly httpServerRef: HttpServer;
@@ -184,6 +193,26 @@ export class TrekWsAdapter extends WsAdapter {
});
}
/**
* Dispatches Nest's `OnGatewayDisconnect` hook (`RealtimeGateway.handleDisconnect`)
* the same way `bindClientConnect` above dispatches `OnGatewayConnection` —
* the base `WsAdapter.bindClientDisconnect` just does `client.on(CLOSE_EVENT,
* callback)` with no context of any kind. `handleDisconnect` reads no
* repository today (Task 0 review addendum, LOW item 2), but it is a
* lifecycle hook with no HTTP request behind it exactly like the connect and
* message hooks are, so it gets the same wrap NOW rather than becoming the
* next "the adapter is done" surprise the moment a future change makes it
* read one. Same fail-closed shape as the other two: throw before calling.
*/
bindClientDisconnect(client: TrekWebSocket, callback: (...args: unknown[]) => void): void {
client.on('close', (...args: unknown[]) => {
if (!this.orm) {
throw new Error('TrekWsAdapter: no MikroORM available to build a request context for this disconnect');
}
withRequestContext(this.orm, () => callback(...args));
});
}
bindMessageHandlers(
socket: TrekWebSocket,
handlers: MessageMappingProperties[],
+17 -1
View File
@@ -79,6 +79,10 @@ import { GlobalAuthGuard } from '../../src/nest/auth/global-auth.guard';
import { MfaPolicyGuard } from '../../src/nest/auth/mfa-policy.guard';
import { TestUnitOfWorkModule } from '../helpers/test-uow';
import { createTestMikroOrmModule } from '../helpers/test-orm';
import { MikroOrmModule } from '@mikro-orm/nestjs';
import { AppSettings } from '../../src/db/entities/AppSettings.entity';
import { Users } from '../../src/db/entities/Users.entity';
import { WebauthnCredentials } from '../../src/db/entities/WebauthnCredentials.entity';
const BASE = 'https://trek.example.test';
@@ -94,7 +98,19 @@ describe('Calendar-feed e2e (real auth guard + temp SQLite)', () => {
// on the feed controller passed here while production 401'd every calendar
// client).
const moduleRef = await Test.createTestingModule({
imports: [await TestUnitOfWorkModule.forRoot(db), await createTestMikroOrmModule(db), AppConfigModule, DatabaseModule, RealtimeModule, FeedsModule],
imports: [
await TestUnitOfWorkModule.forRoot(db),
await createTestMikroOrmModule(db),
// MfaPolicyGuard (Plan 3b Task 1) needs these three repositories —
// mirrors app.module.ts's own forFeature, added there for the same
// reason (MfaPolicyGuard is provided only as an APP_GUARD, never
// per-controller, so this is the one place that needs it).
MikroOrmModule.forFeature([AppSettings, Users, WebauthnCredentials]),
AppConfigModule,
DatabaseModule,
RealtimeModule,
FeedsModule,
],
providers: [
{ provide: APP_GUARD, useClass: GlobalAuthGuard },
{ provide: APP_GUARD, useClass: MfaPolicyGuard },
+7 -1
View File
@@ -41,6 +41,7 @@ vi.mock('../../src/db/database', () => ({
import { PlaceShadowModule } from '../../src/nest/place-shadow/place-shadow.module';
import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter';
import { TestUnitOfWorkModule } from '../helpers/test-uow';
import { createTestMikroOrmModule } from '../helpers/test-orm';
const PICK = {
query: 'kaffee bar am dobi',
@@ -66,8 +67,13 @@ describe('/api/place-shadow e2e (real guards + temp SQLite)', () => {
let app: Awaited<ReturnType<typeof build>>;
async function build() {
// JwtAuthGuard (Plan 3b Task 1) injects EntityManager — needs
// MikroOrmModule.forRoot in the graph, same as every other e2e harness
// guarding a route with it; this suite's minimal hand-rolled `users`
// table already carries the five columns
// `findByIdWithPasswordVersion` selects.
const moduleRef = await Test.createTestingModule({
imports: [await TestUnitOfWorkModule.forRoot(db), DatabaseModule, PlaceShadowModule],
imports: [await TestUnitOfWorkModule.forRoot(db), await createTestMikroOrmModule(db), DatabaseModule, PlaceShadowModule],
providers: [{ provide: APP_PIPE, useClass: ZodValidationPipe }],
}).compile();
const nest = moduleRef.createNestApplication();
+9 -1
View File
@@ -31,13 +31,21 @@ vi.mock('../../src/nest/weather/weather.impl', async (importActual) => {
import { WeatherModule } from '../../src/nest/weather/weather.module';
import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter';
import { TestUnitOfWorkModule } from '../helpers/test-uow';
import { createTestMikroOrmModule } from '../helpers/test-orm';
describe('Weather e2e (real auth guard + temp SQLite)', () => {
let server: Server;
let app: Awaited<ReturnType<typeof build>>;
async function build() {
const moduleRef = await Test.createTestingModule({ imports: [await TestUnitOfWorkModule.forRoot(db), WeatherModule] }).compile();
// JwtAuthGuard (Plan 3b Task 1) injects EntityManager — needs
// MikroOrmModule.forRoot in the graph, same as every other e2e harness
// guarding a route with it; this suite's minimal hand-rolled `users`
// table already carries the five columns
// `findByIdWithPasswordVersion` selects.
const moduleRef = await Test.createTestingModule({
imports: [await TestUnitOfWorkModule.forRoot(db), await createTestMikroOrmModule(db), WeatherModule],
}).compile();
const nest = moduleRef.createNestApplication();
nest.use(cookieParser());
nest.useGlobalFilters(new TrekExceptionFilter());
+3 -3
View File
@@ -148,7 +148,7 @@ export async function createMcpTestRegistry(): Promise<McpRegistry> {
dbService,
permissionsService,
new TripMembershipService(dbService),
new WebauthnConfigService(dbService),
new WebauthnConfigService(appSettings),
new UserCleanupService(dbService, budgetService, await createTestUnitOfWork(dbService.connection)),
new MailerService(dbService),
new EphemeralTokenService(),
@@ -201,7 +201,7 @@ export async function createMcpTestRegistry(): Promise<McpRegistry> {
const readModelService = new TripReadModelService(
dbService, membersService, daysService, accommodationsService, budgetService,
packingService, reservationsService, collabService, placesService, todoService,
new FilesService(dbService, permissionsService, realtimeService, new EphemeralTokenService(), generalStorage),
new FilesService(dbService, permissionsService, realtimeService, new EphemeralTokenService(), generalStorage, mcpOrm.em),
);
const calendarService = new CalendarService(dbService, reservationsService);
// The nine addon-gated surfaces read their toggle off an injected service now
@@ -233,7 +233,7 @@ export async function createMcpTestRegistry(): Promise<McpRegistry> {
new DayNotesMcp(new DayNotesService(dbService, permissionsService, realtimeService), authService, guards),
new DaysMcp(daysService, authService, guards),
new RoadtripMcp(new RoadtripService(dbService, realtimeService, await createTestUnitOfWork(dbService.connection)), dbService, guards, authService, addonsService),
new FilesMcp(new FilesService(dbService, permissionsService, realtimeService, new EphemeralTokenService(), generalStorage), authService, guards),
new FilesMcp(new FilesService(dbService, permissionsService, realtimeService, new EphemeralTokenService(), generalStorage, mcpOrm.em), authService, guards),
new AccommodationsMcp(accommodationsService, dbService, placesService, authService, guards, await createTestUnitOfWork(dbService.connection)),
new AssignmentsMcp(assignmentsService, daysService, authService, guards),
new CollabMcp(collabService, authService, addonsService, guards),
+1 -1
View File
@@ -99,7 +99,7 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise<
const queryHelpers = new QueryHelpersService(dbs);
const todos = new TodoService(dbs, permissions, realtime, await createTestUnitOfWork(dbs.connection));
const packing = new PackingService(dbs, permissions, realtime, notificationsStub(), await createTestUnitOfWork(dbs.connection));
const files = new FilesService(dbs, permissions, realtime, new EphemeralTokenService(), generalStorage);
const files = new FilesService(dbs, permissions, realtime, new EphemeralTokenService(), generalStorage, (await sharedTestOrm(dbs.connection)).em);
const collab = new CollabService(dbs, permissions, realtime, notificationsStub(), generalStorage, new RateLimitService(), await createTestUnitOfWork(dbs.connection));
const vacay = new VacayService(dbs, realtime, notificationsStub(), await createTestUnitOfWork(dbs.connection));
const days = new DaysService(dbs, permissions, realtime, queryHelpers, await createTestUnitOfWork(dbs.connection));
@@ -5,7 +5,7 @@ import { resetTestDb } from '../../../helpers/test-db';
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
import { createUser } from '../../../helpers/factories';
import { Users } from '../../../../src/db/entities/Users.entity';
import { columnIncrementedBy, columnRef, currentTimestamp, dateAdd, dateOf } from '../../../../src/db/dialect/sql-functions';
import { columnIncrementedBy, columnRef, currentTimestamp, dateAdd, dateOf, lower } from '../../../../src/db/dialect/sql-functions';
const testDb = createSnapshotTestDb();
let t: TestOrm;
@@ -73,6 +73,32 @@ describe('sql-functions (sqlite)', () => {
expect(() => currentTimestamp(foreign)).toThrow(/no implementation for platform FakePlatform/);
expect(() => columnRef(foreign, 'u.max_uses')).toThrow(/no implementation for platform FakePlatform/);
expect(() => columnIncrementedBy(foreign, 'u.used_count', 1)).toThrow(/no implementation for platform FakePlatform/);
expect(() => lower(foreign, 'u.email')).toThrow(/no implementation for platform FakePlatform/);
});
// Plan 3b Task 1: UsersRepository's CI (case-insensitive) lookups
// (findByEmailCI, findIdByUsernameCI, ...) need `LOWER(col) = ?` as a
// `find`/`findOne` filter KEY, not just inside a QueryBuilder `.where()` —
// proving it works through `em.find`/`findOne` (the API the repository
// actually calls) rather than only through `createQueryBuilder`.
it('SQLF-012: lower is usable as a find() filter key, matching case-insensitively', async () => {
const { user } = createUser(testDb, { email: 'Mixed.Case@Example.com' });
const rows = await t.em.find(Users, { [lower(t.em.getPlatform(), 'email')]: 'mixed.case@example.com' });
expect(rows.map((r) => r.id)).toEqual([user.id]);
// A differently-cased query value that still lowercases to the same
// string finds the same row; one that does not, finds nothing.
const none = await t.em.find(Users, { [lower(t.em.getPlatform(), 'email')]: 'nobody@example.com' });
expect(none).toEqual([]);
});
it('SQLF-013: lower composes with other filter keys in one findOne call', async () => {
const { user } = createUser(testDb, { username: 'MixedCaseName' });
const row = await t.em.findOne(Users, {
[lower(t.em.getPlatform(), 'username')]: 'mixedcasename',
id: { $ne: -1 },
});
expect(row?.id).toBe(user.id);
});
// Plan 3b Task 0: InviteTokensRepository.incrementUsedCount needs a
@@ -22,15 +22,15 @@ function rawInvite(id: number): unknown {
}
describe('InviteTokensRepository', () => {
describe('findValid', () => {
it('INVREPO-001: a row read through findValid is the SELECT * row, key for key', async () => {
describe('findByToken', () => {
it('INVREPO-001: a row read through findByToken is the SELECT * row, key for key', async () => {
const created = createInviteToken(testDb, { token: 'find-me' });
const row = await invites.findValid('find-me');
const row = await invites.findByToken('find-me');
expect(row).toStrictEqual(rawInvite(created.id));
});
it('INVREPO-002: an unknown token is null', async () => {
expect(await invites.findValid('does-not-exist')).toBeNull();
expect(await invites.findByToken('does-not-exist')).toBeNull();
});
});
@@ -2,7 +2,7 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vites
import { createSnapshotTestDb } from '../../../helpers/db-mock';
import { resetTestDb } from '../../../helpers/test-db';
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
import { createUser } from '../../../helpers/factories';
import { createUser, createAdmin } from '../../../helpers/factories';
import { Users } from '../../../../src/db/entities/Users.entity';
import type { UsersRepository } from '../../../../src/db/repositories/Users.repository';
@@ -94,4 +94,451 @@ describe('UsersRepository', () => {
expect(queries).toBe(1);
});
});
// ---------------------------------------------------------------------
// Plan 3b Task 1 — every users-table method the auth/oidc/oauth/tokens
// cluster needs (see Users.repository.ts for the legacy-statement mapping
// per method). One `it` (or a focused group) per method, on real rows.
// ---------------------------------------------------------------------
it('USERSREPO-009: findByIdWithPasswordVersion (JV1) reads id/username/email/role/password_version', async () => {
const { user } = createUser(testDb);
testDb.prepare('UPDATE users SET password_version = 3 WHERE id = ?').run(user.id);
const row = await users.findByIdWithPasswordVersion(user.id);
expect(row).toEqual({ id: user.id, username: user.username, email: user.email, role: 'user', password_version: 3 });
expect(await users.findByIdWithPasswordVersion(999999)).toBeNull();
});
it('USERSREPO-010: getPasswordVersion reads password_version, null for a missing user', async () => {
const { user } = createUser(testDb);
expect(await users.getPasswordVersion(user.id)).toBe(0);
testDb.prepare('UPDATE users SET password_version = 7 WHERE id = ?').run(user.id);
expect(await users.getPasswordVersion(user.id)).toBe(7);
expect(await users.getPasswordVersion(999999)).toBeNull();
});
describe('countNonGuest (AU4/AU7/O10) — is_guest is INTEGER NOT NULL DEFAULT 0, never observed NULL', () => {
it('USERSREPO-011: counts only non-guest users', async () => {
createUser(testDb);
createUser(testDb);
const { user: guest } = createUser(testDb);
testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(guest.id);
expect(await users.countNonGuest()).toBe(2);
});
it('USERSREPO-012: a NULL is_guest row cannot exist — the column is NOT NULL DEFAULT 0 (verified against the baseline + guest-members migrations), so a direct INSERT that omits the column, or an explicit NULL, both resolve to 0, never leaving a row this filter would miscount', () => {
testDb.prepare('INSERT INTO users (username, email, password_hash, role) VALUES (?, ?, ?, ?)').run('nog', 'nog@test.example.com', 'x', 'user');
const stored = testDb.prepare("SELECT is_guest FROM users WHERE username = 'nog'").get() as { is_guest: number };
expect(stored.is_guest).toBe(0); // the DEFAULT 0, not a nullable column silently coercing
expect(() => testDb.prepare('UPDATE users SET is_guest = NULL WHERE username = ?').run('nog')).toThrow(/NOT NULL constraint failed/);
});
});
it('USERSREPO-013: findAdminNeedingPasswordChange finds an admin flagged must_change_password, ignores a matching non-admin or an unflagged admin', async () => {
expect(await users.findAdminNeedingPasswordChange()).toBeNull();
const { user: plainAdmin } = createAdmin(testDb);
expect(await users.findAdminNeedingPasswordChange()).toBeNull();
const { user: flaggedUser } = createUser(testDb);
testDb.prepare('UPDATE users SET must_change_password = 1 WHERE id = ?').run(flaggedUser.id);
expect(await users.findAdminNeedingPasswordChange()).toBeNull(); // not an admin
testDb.prepare('UPDATE users SET must_change_password = 1 WHERE id = ?').run(plainAdmin.id);
expect(await users.findAdminNeedingPasswordChange()).toBe(plainAdmin.id);
});
describe('findIdByEmailOrUsernameCI (AU9)', () => {
it('USERSREPO-014: matches by email OR username, case-insensitively, excluding guests', async () => {
const { user } = createUser(testDb, { email: 'Someone@Example.com', username: 'SomeoneName' });
expect(await users.findIdByEmailOrUsernameCI('someone@example.com', 'no-match')).toBe(user.id);
expect(await users.findIdByEmailOrUsernameCI('no-match@example.com', 'someonename')).toBe(user.id);
expect(await users.findIdByEmailOrUsernameCI('nope@example.com', 'nope')).toBeNull();
});
it('USERSREPO-015: excludes a guest even on an exact match', async () => {
const { user } = createUser(testDb, { email: 'guest1@example.com', username: 'guest1' });
testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(user.id);
expect(await users.findIdByEmailOrUsernameCI('guest1@example.com', 'guest1')).toBeNull();
});
});
describe('insertUser (AU10/O14)', () => {
it('USERSREPO-016: local registration shape (AU10) — no oidc/avatar columns', async () => {
const row = await users.insertUser({
username: 'localuser',
email: 'local@example.com',
password_hash: 'hashed',
role: 'user',
first_seen_version: '1.2.3',
});
expect(row.username).toBe('localuser');
expect(row.email).toBe('local@example.com');
expect(row.password_hash).toBe('hashed');
expect(row.role).toBe('user');
expect(row.first_seen_version).toBe('1.2.3');
expect(row.login_count).toBe(0);
expect(row.oidc_sub).toBeNull();
expect(row.oidc_issuer).toBeNull();
expect(row.avatar).toBeNull();
expect(row.created_at).not.toBeNull();
const raw = testDb.prepare('SELECT * FROM users WHERE id = ?').get(row.id);
expect(raw).toMatchObject({ username: 'localuser', email: 'local@example.com', role: 'user', login_count: 0 });
});
it('USERSREPO-017: SSO registration shape (O14) — oidc_sub/oidc_issuer/avatar set', async () => {
const row = await users.insertUser({
username: 'ssouser',
email: 'sso@example.com',
password_hash: 'unusable-hash',
role: 'admin',
first_seen_version: '1.2.3',
oidc_sub: 'sub-123',
oidc_issuer: 'https://idp.example.com',
avatar: 'https://idp.example.com/pic.png',
});
expect(row.oidc_sub).toBe('sub-123');
expect(row.oidc_issuer).toBe('https://idp.example.com');
expect(row.avatar).toBe('https://idp.example.com/pic.png');
expect(row.role).toBe('admin');
expect(row.login_count).toBe(0);
});
});
describe('findByEmailCI (AU12/O4)', () => {
it('USERSREPO-018: matches case-insensitively, excludes guests, returns the full row', async () => {
const { user } = createUser(testDb, { email: 'Case.Test@Example.com' });
const row = await users.findByEmailCI('case.test@example.com');
expect(row?.id).toBe(user.id);
expect(row?.username).toBe(user.username);
expect(await users.findByEmailCI('nope@example.com')).toBeNull();
});
it('USERSREPO-019: a guest with a matching email is not returned', async () => {
const { user } = createUser(testDb, { email: 'guest2@example.com' });
testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(user.id);
expect(await users.findByEmailCI('guest2@example.com')).toBeNull();
});
});
describe('touchLastLogin (AU13/AU34/AU35/PK12/O16)', () => {
it('USERSREPO-020: bumps login_count by exactly 1 and stamps last_login, in one statement', async () => {
const { user } = createUser(testDb);
testDb.prepare('UPDATE users SET login_count = 5 WHERE id = ?').run(user.id);
await users.touchLastLogin(user.id);
const row = testDb.prepare('SELECT login_count, last_login FROM users WHERE id = ?').get(user.id) as { login_count: number; last_login: string };
expect(row.login_count).toBe(6);
expect(row.last_login).toMatch(/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/);
});
it('USERSREPO-021: two concurrent calls both land — +2, never +1 (no read-modify-write)', async () => {
const { user } = createUser(testDb);
testDb.prepare('UPDATE users SET login_count = 10 WHERE id = ?').run(user.id);
await Promise.all([users.touchLastLogin(user.id), users.touchLastLogin(user.id)]);
const row = testDb.prepare('SELECT login_count FROM users WHERE id = ?').get(user.id) as { login_count: number };
expect(row.login_count).toBe(12);
});
});
it('USERSREPO-022: findMeRow (AU14) reads the /api/auth/me projection', async () => {
const { user } = createUser(testDb);
testDb.prepare('UPDATE users SET avatar = ?, mfa_enabled = 1, must_change_password = 1 WHERE id = ?').run('pic.png', user.id);
const row = await users.findMeRow(user.id);
expect(row).toMatchObject({ id: user.id, username: user.username, email: user.email, role: 'user', avatar: 'pic.png', mfa_enabled: 1, must_change_password: 1 });
expect(row?.created_at).not.toBeNull();
expect(await users.findMeRow(999999)).toBeNull();
});
it('USERSREPO-023: getPasswordHashAndVersion (AU15) reads both columns together', async () => {
const { user, password: _password } = createUser(testDb);
testDb.prepare('UPDATE users SET password_version = 2 WHERE id = ?').run(user.id);
const row = await users.getPasswordHashAndVersion(user.id);
expect(row?.password_hash).toBe(user.password_hash);
expect(row?.password_version).toBe(2);
});
it('USERSREPO-024: setPassword (AU16/AU43) writes hash+version, clears must_change_password, stamps updated_at', async () => {
const { user } = createUser(testDb);
testDb.prepare('UPDATE users SET must_change_password = 1 WHERE id = ?').run(user.id);
await users.setPassword(user.id, 'new-hash', 5);
const row = testDb.prepare('SELECT password_hash, password_version, must_change_password, updated_at FROM users WHERE id = ?').get(user.id) as {
password_hash: string; password_version: number; must_change_password: number; updated_at: string;
};
expect(row.password_hash).toBe('new-hash');
expect(row.password_version).toBe(5);
expect(row.must_change_password).toBe(0);
expect(row.updated_at).toMatch(/^\d{4}-\d{2}-\d{2}/);
});
it('USERSREPO-025: countAdmins (AU19/O7) counts only role=admin', async () => {
createUser(testDb);
createAdmin(testDb);
createAdmin(testDb);
expect(await users.countAdmins()).toBe(2);
});
it('USERSREPO-026: getRole (AU20/AU22) reads the role column', async () => {
const { user } = createAdmin(testDb);
expect(await users.getRole(user.id)).toBe('admin');
expect(await users.getRole(999999)).toBeNull();
});
describe('getMfaEnabled (AU23/AU27/MP2) — distinguishes "no row" from "row present, mfa_enabled unset"', () => {
it('USERSREPO-027: returns the row wrapper when the user exists', async () => {
const { user } = createUser(testDb);
expect(await users.getMfaEnabled(user.id)).toEqual({ mfa_enabled: 0 });
testDb.prepare('UPDATE users SET mfa_enabled = 1 WHERE id = ?').run(user.id);
expect(await users.getMfaEnabled(user.id)).toEqual({ mfa_enabled: 1 });
});
it('USERSREPO-028: returns null (not a row with a null field) when the user does not exist', async () => {
expect(await users.getMfaEnabled(999999)).toBeNull();
});
});
it('USERSREPO-029: enableMfa (AU28) sets mfa_enabled=1, writes the already-encrypted secret and codes, stamps updated_at', async () => {
const { user } = createUser(testDb);
await users.enableMfa(user.id, 'encrypted-secret', '["hash1","hash2"]');
const row = testDb.prepare('SELECT mfa_enabled, mfa_secret, mfa_backup_codes, updated_at FROM users WHERE id = ?').get(user.id) as {
mfa_enabled: number; mfa_secret: string; mfa_backup_codes: string; updated_at: string;
};
expect(row.mfa_enabled).toBe(1);
expect(row.mfa_secret).toBe('encrypted-secret');
expect(row.mfa_backup_codes).toBe('["hash1","hash2"]');
expect(row.updated_at).toMatch(/^\d{4}-\d{2}-\d{2}/);
});
describe('findById (AU30/AU32/PK4/PK10/O15) — full row, PK-only findOne', () => {
it('USERSREPO-030: reads every scalar column', async () => {
const { user } = createUser(testDb);
const row = await users.findById(user.id);
expect(row?.id).toBe(user.id);
expect(row?.username).toBe(user.username);
expect(row?.password_hash).toBe(user.password_hash);
expect(await users.findById(999999)).toBeNull();
});
it('USERSREPO-031: sees a raw UPDATE on the same id in the same request (I1, identity-map regression)', async () => {
const { user } = createUser(testDb, { username: 'before' });
await users.findById(user.id); // populate the identity map
testDb.prepare('UPDATE users SET username = ? WHERE id = ?').run('after', user.id);
const { value, queries } = await withQueryCount(() => users.findById(user.id));
expect(value?.username).toBe('after');
expect(queries).toBe(1);
});
});
it('USERSREPO-032: disableMfa (AU31) clears mfa_enabled/secret/codes, stamps updated_at', async () => {
const { user } = createUser(testDb);
testDb.prepare('UPDATE users SET mfa_enabled = 1, mfa_secret = ?, mfa_backup_codes = ? WHERE id = ?').run('s', 'c', user.id);
await users.disableMfa(user.id);
const row = testDb.prepare('SELECT mfa_enabled, mfa_secret, mfa_backup_codes, updated_at FROM users WHERE id = ?').get(user.id) as {
mfa_enabled: number; mfa_secret: string | null; mfa_backup_codes: string | null; updated_at: string;
};
expect(row.mfa_enabled).toBe(0);
expect(row.mfa_secret).toBeNull();
expect(row.mfa_backup_codes).toBeNull();
expect(row.updated_at).toMatch(/^\d{4}-\d{2}-\d{2}/);
});
describe('setBackupCodesAndTouch (AU33) vs setBackupCodes (AU44) — one stamps updated_at, the other does not', () => {
it('USERSREPO-033: setBackupCodesAndTouch writes codes and stamps updated_at', async () => {
const { user } = createUser(testDb);
testDb.prepare("UPDATE users SET updated_at = '2020-01-01 00:00:00' WHERE id = ?").run(user.id);
await users.setBackupCodesAndTouch(user.id, '["a"]');
const row = testDb.prepare('SELECT mfa_backup_codes, updated_at FROM users WHERE id = ?').get(user.id) as { mfa_backup_codes: string; updated_at: string };
expect(row.mfa_backup_codes).toBe('["a"]');
expect(row.updated_at).not.toBe('2020-01-01 00:00:00');
});
it('USERSREPO-034: setBackupCodes writes codes WITHOUT touching updated_at', async () => {
const { user } = createUser(testDb);
testDb.prepare("UPDATE users SET updated_at = '2020-01-01 00:00:00' WHERE id = ?").run(user.id);
await users.setBackupCodes(user.id, '["b"]');
const row = testDb.prepare('SELECT mfa_backup_codes, updated_at FROM users WHERE id = ?').get(user.id) as { mfa_backup_codes: string; updated_at: string };
expect(row.mfa_backup_codes).toBe('["b"]');
expect(row.updated_at).toBe('2020-01-01 00:00:00');
});
});
describe('findForPasswordReset (AU36) — case-SENSITIVE email, unlike every other email lookup here', () => {
it('USERSREPO-035: matches the exact case only', async () => {
const { user } = createUser(testDb, { email: 'Exact@Example.com' });
const row = await users.findForPasswordReset('Exact@Example.com');
expect(row?.id).toBe(user.id);
expect(await users.findForPasswordReset('exact@example.com')).toBeNull(); // no LOWER() in the legacy statement
});
it('USERSREPO-036: excludes a guest', async () => {
const { user } = createUser(testDb, { email: 'guest3@example.com' });
testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(user.id);
expect(await users.findForPasswordReset('guest3@example.com')).toBeNull();
});
});
it('USERSREPO-037: findResetTarget (AU40) reads the reset-branch projection', async () => {
const { user } = createUser(testDb);
testDb.prepare('UPDATE users SET mfa_enabled = 1, mfa_secret = ?, mfa_backup_codes = ?, password_version = 4 WHERE id = ?').run('s', 'c', user.id);
const row = await users.findResetTarget(user.id);
expect(row).toEqual({ id: user.id, email: user.email, mfa_enabled: 1, mfa_secret: 's', mfa_backup_codes: 'c', password_version: 4 });
});
it('USERSREPO-038: getPasswordHash (PK15) reads password_hash', async () => {
const { user } = createUser(testDb);
expect(await users.getPasswordHash(user.id)).toBe(user.password_hash);
expect(await users.getPasswordHash(999999)).toBeNull();
});
it('USERSREPO-039: findIdAndEmail (PK17) reads id+email', async () => {
const { user } = createUser(testDb);
expect(await users.findIdAndEmail(user.id)).toEqual({ id: user.id, email: user.email });
});
describe('findByOidcIdentity (O3)', () => {
it('USERSREPO-040: matches on the (sub, issuer) pair', async () => {
const { user } = createUser(testDb);
testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ? WHERE id = ?').run('sub-1', 'https://idp.example.com', user.id);
const row = await users.findByOidcIdentity('sub-1', 'https://idp.example.com');
expect(row?.id).toBe(user.id);
expect(await users.findByOidcIdentity('sub-1', 'https://other-idp.example.com')).toBeNull();
});
});
it('USERSREPO-041: linkOidcIdentity (O5/O6) writes sub+issuer WITHOUT touching updated_at', async () => {
const { user } = createUser(testDb);
testDb.prepare("UPDATE users SET updated_at = '2020-01-01 00:00:00' WHERE id = ?").run(user.id);
await users.linkOidcIdentity(user.id, 'sub-2', 'https://idp2.example.com');
const row = testDb.prepare('SELECT oidc_sub, oidc_issuer, updated_at FROM users WHERE id = ?').get(user.id) as {
oidc_sub: string; oidc_issuer: string; updated_at: string;
};
expect(row.oidc_sub).toBe('sub-2');
expect(row.oidc_issuer).toBe('https://idp2.example.com');
expect(row.updated_at).toBe('2020-01-01 00:00:00');
});
it('USERSREPO-042: setRole (O8) writes role WITHOUT touching updated_at', async () => {
const { user } = createUser(testDb);
testDb.prepare("UPDATE users SET updated_at = '2020-01-01 00:00:00' WHERE id = ?").run(user.id);
await users.setRole(user.id, 'admin');
const row = testDb.prepare('SELECT role, updated_at FROM users WHERE id = ?').get(user.id) as { role: string; updated_at: string };
expect(row.role).toBe('admin');
expect(row.updated_at).toBe('2020-01-01 00:00:00');
});
it('USERSREPO-043: setAvatarRaw (O9) writes avatar WITHOUT touching updated_at, unlike setAvatar', async () => {
const { user } = createUser(testDb);
testDb.prepare("UPDATE users SET updated_at = '2020-01-01 00:00:00' WHERE id = ?").run(user.id);
await users.setAvatarRaw(user.id, 'https://idp.example.com/pic.png');
const row = testDb.prepare('SELECT avatar, updated_at FROM users WHERE id = ?').get(user.id) as { avatar: string; updated_at: string };
expect(row.avatar).toBe('https://idp.example.com/pic.png');
expect(row.updated_at).toBe('2020-01-01 00:00:00');
});
describe('findIdByUsernameCI (UP5) vs findIdByUsernameCIAny (O12) — different WHERE, two methods', () => {
it('USERSREPO-044: findIdByUsernameCI excludes the given id and any guest', async () => {
const { user: a } = createUser(testDb, { username: 'DupeName' });
const { user: b } = createUser(testDb, { username: 'other' });
expect(await users.findIdByUsernameCI('dupename', b.id)).toBe(a.id);
expect(await users.findIdByUsernameCI('dupename', a.id)).toBeNull(); // excludes self
testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(a.id);
expect(await users.findIdByUsernameCI('dupename', b.id)).toBeNull(); // guest excluded
});
it('USERSREPO-045: findIdByUsernameCIAny has no exclude and no guest filter', async () => {
const { user } = createUser(testDb, { username: 'AnyName' });
testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(user.id);
expect(await users.findIdByUsernameCIAny('anyname')).toBe(user.id); // a guest still matches here
});
});
it('USERSREPO-046: getApiKeyColumns (UP1) reads role + the four key columns', async () => {
const { user } = createAdmin(testDb);
testDb.prepare('UPDATE users SET maps_api_key = ?, openweather_api_key = ?, unsplash_api_key = ?, amap_api_key = ? WHERE id = ?')
.run('m', 'o', 'u', 'a', user.id);
expect(await users.getApiKeyColumns(user.id)).toEqual({ role: 'admin', maps_api_key: 'm', openweather_api_key: 'o', unsplash_api_key: 'u', amap_api_key: 'a' });
});
it('USERSREPO-047: updateMapsKey (UP2) writes maps_api_key + updated_at', async () => {
const { user } = createUser(testDb);
await users.updateMapsKey(user.id, 'encrypted');
const row = testDb.prepare('SELECT maps_api_key, updated_at FROM users WHERE id = ?').get(user.id) as { maps_api_key: string; updated_at: string };
expect(row.maps_api_key).toBe('encrypted');
expect(row.updated_at).toMatch(/^\d{4}-\d{2}-\d{2}/);
});
it('USERSREPO-048: updateApiKeys (UP3) writes all four columns + updated_at in one statement', async () => {
const { user } = createUser(testDb);
await users.updateApiKeys(user.id, { maps_api_key: 'm2', openweather_api_key: 'o2', unsplash_api_key: 'u2', amap_api_key: null });
const row = testDb.prepare('SELECT maps_api_key, openweather_api_key, unsplash_api_key, amap_api_key, updated_at FROM users WHERE id = ?').get(user.id) as Record<string, unknown>;
expect(row).toMatchObject({ maps_api_key: 'm2', openweather_api_key: 'o2', unsplash_api_key: 'u2', amap_api_key: null });
expect(row.updated_at).toMatch(/^\d{4}-\d{2}-\d{2}/);
});
it('USERSREPO-049: findProfileWithKeys (UP4/UP8) reads the profile-with-keys projection', async () => {
const { user } = createUser(testDb);
testDb.prepare('UPDATE users SET maps_api_key = ?, mfa_enabled = 1 WHERE id = ?').run('m', user.id);
const row = await users.findProfileWithKeys(user.id);
expect(row).toMatchObject({ id: user.id, username: user.username, email: user.email, role: 'user', maps_api_key: 'm', mfa_enabled: 1 });
});
it('USERSREPO-050: findIdByEmailCI (UP6) matches case-insensitively, excludes self and guests', async () => {
const { user: a } = createUser(testDb, { email: 'Dup@Example.com' });
const { user: b } = createUser(testDb, { email: 'other@example.com' });
expect(await users.findIdByEmailCI('dup@example.com', b.id)).toBe(a.id);
expect(await users.findIdByEmailCI('dup@example.com', a.id)).toBeNull();
});
it('USERSREPO-051: patchProfile (UP7) writes only the given bounded columns + updated_at, in one statement', async () => {
const { user } = createUser(testDb, { username: 'patchme', email: 'patchme@example.com' });
await users.patchProfile(user.id, { username: 'patched', maps_api_key: 'newkey' });
const row = testDb.prepare('SELECT username, email, maps_api_key, updated_at FROM users WHERE id = ?').get(user.id) as Record<string, unknown>;
expect(row.username).toBe('patched');
expect(row.email).toBe('patchme@example.com'); // untouched — not in `changes`
expect(row.maps_api_key).toBe('newkey');
expect(row.updated_at).toMatch(/^\d{4}-\d{2}-\d{2}/);
});
describe('getAvatar (UP9/UP12) / setAvatar (UP10/UP13)', () => {
it('USERSREPO-052: getAvatar reads the avatar column', async () => {
const { user } = createUser(testDb);
expect(await users.getAvatar(user.id)).toBeNull();
testDb.prepare('UPDATE users SET avatar = ? WHERE id = ?').run('pic.png', user.id);
expect(await users.getAvatar(user.id)).toBe('pic.png');
});
it('USERSREPO-053: setAvatar writes avatar + updated_at (both a filename and NULL for delete)', async () => {
const { user } = createUser(testDb);
await users.setAvatar(user.id, 'new.png');
let row = testDb.prepare('SELECT avatar, updated_at FROM users WHERE id = ?').get(user.id) as { avatar: string; updated_at: string };
expect(row.avatar).toBe('new.png');
expect(row.updated_at).toMatch(/^\d{4}-\d{2}-\d{2}/);
testDb.prepare("UPDATE users SET updated_at = '2020-01-01 00:00:00' WHERE id = ?").run(user.id);
await users.setAvatar(user.id, null);
row = testDb.prepare('SELECT avatar, updated_at FROM users WHERE id = ?').get(user.id) as { avatar: string | null; updated_at: string };
expect(row.avatar).toBeNull();
expect(row.updated_at).not.toBe('2020-01-01 00:00:00');
});
});
it('USERSREPO-054: findProfileBasic (UP11) reads id/username/email/role/avatar', async () => {
const { user } = createUser(testDb);
testDb.prepare('UPDATE users SET avatar = ? WHERE id = ?').run('a.png', user.id);
expect(await users.findProfileBasic(user.id)).toEqual({ id: user.id, username: user.username, email: user.email, role: 'user', avatar: 'a.png' });
});
describe('listOthersNonGuest (UP14)', () => {
it('USERSREPO-055: excludes the given id and every guest, ordered by username', async () => {
const { user: me } = createUser(testDb, { username: 'me' });
const { user: bravo } = createUser(testDb, { username: 'bravo' });
const { user: alpha } = createUser(testDb, { username: 'alpha' });
const { user: guest } = createUser(testDb, { username: 'zzz-guest' });
testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(guest.id);
const rows = await users.listOthersNonGuest(me.id);
expect(rows.map((r) => r.id)).toEqual([alpha.id, bravo.id]);
});
});
it('USERSREPO-056: getRoleAndWeatherKey (UP15) reads role + openweather_api_key', async () => {
const { user } = createAdmin(testDb);
testDb.prepare('UPDATE users SET openweather_api_key = ? WHERE id = ?').run('w', user.id);
expect(await users.getRoleAndWeatherKey(user.id)).toEqual({ role: 'admin', openweather_api_key: 'w' });
});
});
+2 -1
View File
@@ -87,7 +87,7 @@ import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo }
const dbs = new DatabaseService(testDb);
const realtime = new RealtimeService();
const webauthn = new WebauthnConfigService(dbs);
let webauthn: WebauthnConfigService;
// Positional and previously wrong: an AtlasService sat in the membership slot
// and the mailer was missing entirely, so `auth` was built with its last four
@@ -99,6 +99,7 @@ let userCleanup: UserCleanupService;
let auth: AuthService;
let svc: AdminService;
beforeAll(async () => {
webauthn = new WebauthnConfigService(await createTestAppSettingsRepo(dbs.connection));
permissions = new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection));
userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), realtime, await createTestUnitOfWork(dbs.connection)), await createTestUnitOfWork(dbs.connection));
auth = new AuthService(dbs, permissions, new TripMembershipService(dbs), webauthn, userCleanup, new MailerService(dbs), new EphemeralTokenService(), new AllowedFileTypesService(dbs), await createTestUnitOfWork(dbs.connection), await createTestAppSettingsRepo(dbs.connection), await createTestUsersRepo(dbs.connection));
@@ -65,7 +65,7 @@ import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo }
const dbs = new DatabaseService(testDb);
const realtime = new RealtimeService();
const webauthn = new WebauthnConfigService(dbs);
let webauthn: WebauthnConfigService;
// Positional and previously wrong: an AtlasService sat in the membership slot
// and the mailer was missing entirely, so `auth` was built with its last four
@@ -77,6 +77,7 @@ let userCleanup: UserCleanupService;
let auth: AuthService;
let svc: AdminService;
beforeAll(async () => {
webauthn = new WebauthnConfigService(await createTestAppSettingsRepo(dbs.connection));
permissions = new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection));
userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), realtime, await createTestUnitOfWork(dbs.connection)), await createTestUnitOfWork(dbs.connection));
auth = new AuthService(dbs, permissions, new TripMembershipService(dbs), webauthn, userCleanup, new MailerService(dbs), new EphemeralTokenService(), new AllowedFileTypesService(dbs), await createTestUnitOfWork(dbs.connection), await createTestAppSettingsRepo(dbs.connection), await createTestUsersRepo(dbs.connection));
+11 -3
View File
@@ -7,6 +7,7 @@ vi.mock('../../../src/nest/common/cookie', () => ({ setAuthCookie: vi.fn() }));
vi.mock('../../../src/nest/audit/client-ip', () => ({ getClientIp: vi.fn(() => '1.2.3.4') }));
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 { EntityManager } from '@mikro-orm/core';
import { JwtAuthGuard } from '../../../src/nest/auth/jwt-auth.guard';
import { CookieAuthGuard } from '../../../src/nest/auth/cookie-auth.guard';
import { OptionalJwtGuard } from '../../../src/nest/auth/optional-jwt.guard';
@@ -41,6 +42,13 @@ import type { User } from '../../../src/types';
const user = { id: 1, username: 'u', role: 'user', email: 'u@example.test' } as User;
// JwtAuthGuard/CookieAuthGuard/OptionalJwtGuard now inject EntityManager
// (Plan 3b Task 1 RULING) rather than reading the legacy `db` proxy through
// the mocked jwt-verify.ts — verifyJwtAndLoadUser itself is fully mocked
// above, so `em.getRepository` never needs to return anything meaningful;
// it just has to not throw when the guard calls it.
const emStub = { getRepository: () => ({}) } as unknown as EntityManager;
function context(req: unknown) {
return { switchToHttp: () => ({ getRequest: () => req }) } as never;
}
@@ -64,7 +72,7 @@ async function thrownAsync(fn: () => Promise<unknown>): Promise<{ status: number
beforeEach(() => vi.clearAllMocks());
describe('JwtAuthGuard', () => {
const guard = new JwtAuthGuard();
const guard = new JwtAuthGuard(emStub);
it('rejects with the legacy 401 { error, code } when no token is present', async () => {
vi.mocked(extractToken).mockReturnValue(null);
@@ -93,7 +101,7 @@ describe('JwtAuthGuard', () => {
});
describe('CookieAuthGuard', () => {
const guard = new CookieAuthGuard();
const guard = new CookieAuthGuard(emStub);
it('401s when the trek_session cookie is missing', async () => {
expect(await thrownAsync(() => guard.canActivate(context({ cookies: {} })))).toEqual({
@@ -124,7 +132,7 @@ describe('CookieAuthGuard', () => {
});
describe('OptionalJwtGuard', () => {
const guard = new OptionalJwtGuard();
const guard = new OptionalJwtGuard(emStub);
it('always allows; sets req.user to null when no token', async () => {
const req: Record<string, unknown> = { headers: {} };
+3 -3
View File
@@ -104,7 +104,7 @@ beforeAll(async () => {
new DatabaseService(testDb),
new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)),
membershipStub,
new WebauthnConfigService(new DatabaseService(testDb)),
new WebauthnConfigService(await createTestAppSettingsRepo(testDb)),
new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb)), await createTestUnitOfWork(testDb)),
mailerStub,
new EphemeralTokenService(),
@@ -482,11 +482,11 @@ describe('changePassword — session invalidation', () => {
const { user, password } = createUser(testDb);
const stolen = await svc.generateToken({ id: user.id }); // pv=0 at mint time
expect(await verifyJwtAndLoadUser(stolen)).not.toBeNull();
expect(await verifyJwtAndLoadUser(stolen, await createTestUsersRepo(testDb))).not.toBeNull();
await svc.changePassword(user.id, user.email, { current_password: password, new_password: 'New1234!' });
expect(await verifyJwtAndLoadUser(stolen)).toBeNull(); // invalidated by the pv bump
expect(await verifyJwtAndLoadUser(stolen, await createTestUsersRepo(testDb))).toBeNull(); // invalidated by the pv bump
});
it('AUTH-DB-036d: preserves the remember choice in the re-issued session (#1927)', async () => {
+30 -24
View File
@@ -10,17 +10,21 @@
* The password_version gate gets direct cases here for the first time. It is the
* reason this function exists at all: a reset bumps users.password_version and
* every JWT carrying the prior value has to stop working.
*
* Plan 3b Task 1: `verifyJwtAndLoadUser` now takes an explicit `UsersRepository`
* parameter instead of reading the legacy `db` proxy — this file stubs
* `findByIdWithPasswordVersion` directly (a fake repository object, exactly
* the shape `Users.repository.test.ts` proves against real rows) rather than
* mocking `src/db/database`'s module-level `db` export, which this file no
* longer imports at all.
*/
import { describe, it, expect, vi, afterEach } from 'vitest';
import jwt from 'jsonwebtoken';
vi.mock('../../../../src/db/database', () => ({
db: { prepare: vi.fn(() => ({ get: vi.fn(), all: vi.fn() })) },
}));
vi.mock('../../../../src/config', () => ({ JWT_SECRET: 'test-secret' }));
import { extractToken, verifyJwtAndLoadUser } from '../../../../src/nest/auth/jwt-verify';
import { db } from '../../../../src/db/database';
import type { UsersRepository, UserWithPasswordVersion } from '../../../../src/db/repositories/Users.repository';
import type { Request } from 'express';
function makeReq(overrides: {
@@ -33,9 +37,9 @@ function makeReq(overrides: {
} as unknown as Request;
}
/** Stub the single users row the verify reads. */
function userRow(row: Record<string, unknown> | undefined): void {
vi.mocked(db.prepare).mockReturnValue({ get: vi.fn(() => row), all: vi.fn() } as never);
/** A fake `UsersRepository` whose `findByIdWithPasswordVersion` returns the given row. */
function usersRepo(row: UserWithPasswordVersion | null): UsersRepository {
return { findByIdWithPasswordVersion: vi.fn(async () => row) } as unknown as UsersRepository;
}
afterEach(() => vi.clearAllMocks());
@@ -72,64 +76,66 @@ describe('extractToken', () => {
describe('verifyJwtAndLoadUser', () => {
it('AUTH-JWT-001: returns the user for a valid token, without password_version', async () => {
userRow({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 0 });
const users = usersRepo({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 0 });
const token = jwt.sign({ id: 1 }, 'test-secret', { algorithm: 'HS256' });
expect(await verifyJwtAndLoadUser(token)).toEqual({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user' });
expect(await verifyJwtAndLoadUser(token, users)).toEqual({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user' });
expect(users.findByIdWithPasswordVersion).toHaveBeenCalledWith(1);
});
it('AUTH-JWT-002: returns null for a malformed token', async () => {
expect(await verifyJwtAndLoadUser('invalid.jwt.token')).toBeNull();
expect(await verifyJwtAndLoadUser('invalid.jwt.token', usersRepo(null))).toBeNull();
});
it('AUTH-JWT-003: returns null when the user no longer exists', async () => {
userRow(undefined);
expect(await verifyJwtAndLoadUser(jwt.sign({ id: 99999 }, 'test-secret', { algorithm: 'HS256' }))).toBeNull();
const users = usersRepo(null);
expect(await verifyJwtAndLoadUser(jwt.sign({ id: 99999 }, 'test-secret', { algorithm: 'HS256' }), users)).toBeNull();
});
it('AUTH-JWT-004: returns null for an expired token', async () => {
const expired = jwt.sign({ id: 1, exp: Math.floor(Date.now() / 1000) - 3600 }, 'test-secret', { algorithm: 'HS256' });
expect(await verifyJwtAndLoadUser(expired)).toBeNull();
expect(await verifyJwtAndLoadUser(expired, usersRepo(null))).toBeNull();
});
it('AUTH-JWT-005: returns null for a token signed with the wrong secret', async () => {
expect(await verifyJwtAndLoadUser(jwt.sign({ id: 1 }, 'wrong-secret', { algorithm: 'HS256' }))).toBeNull();
expect(await verifyJwtAndLoadUser(jwt.sign({ id: 1 }, 'wrong-secret', { algorithm: 'HS256' }), usersRepo(null))).toBeNull();
});
it('AUTH-JWT-006: rejects a purpose-scoped mfa_login token even when the user is valid', async () => {
// Issued after the password check but before TOTP, signed with the same
// secret. It must never authenticate an ordinary request.
userRow({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 0 });
const users = usersRepo({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 0 });
const mfaToken = jwt.sign({ id: 1, purpose: 'mfa_login' }, 'test-secret', { algorithm: 'HS256' });
expect(await verifyJwtAndLoadUser(mfaToken)).toBeNull();
expect(await verifyJwtAndLoadUser(mfaToken, users)).toBeNull();
expect(users.findByIdWithPasswordVersion).not.toHaveBeenCalled();
});
it('AUTH-JWT-007: rejects a token whose password_version predates the user row', async () => {
userRow({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 2 });
const users = usersRepo({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 2 });
const stale = jwt.sign({ id: 1, pv: 1 }, 'test-secret', { algorithm: 'HS256' });
expect(await verifyJwtAndLoadUser(stale)).toBeNull();
expect(await verifyJwtAndLoadUser(stale, users)).toBeNull();
});
it('AUTH-JWT-008: accepts a token whose password_version matches', async () => {
userRow({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 2 });
const users = usersRepo({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 2 });
const current = jwt.sign({ id: 1, pv: 2 }, 'test-secret', { algorithm: 'HS256' });
expect(await verifyJwtAndLoadUser(current)).not.toBeNull();
expect(await verifyJwtAndLoadUser(current, users)).not.toBeNull();
});
it('AUTH-JWT-009: a pre-pv token still works against a never-reset user (both read as 0)', async () => {
userRow({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: null });
const users = usersRepo({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 0 });
const legacy = jwt.sign({ id: 1 }, 'test-secret', { algorithm: 'HS256' });
expect(await verifyJwtAndLoadUser(legacy)).not.toBeNull();
expect(await verifyJwtAndLoadUser(legacy, users)).not.toBeNull();
});
it('AUTH-JWT-010: but a pre-pv token stops working once the user has reset', async () => {
userRow({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 1 });
const users = usersRepo({ id: 1, username: 'alice', email: 'alice@example.com', role: 'user', password_version: 1 });
const legacy = jwt.sign({ id: 1 }, 'test-secret', { algorithm: 'HS256' });
expect(await verifyJwtAndLoadUser(legacy)).toBeNull();
expect(await verifyJwtAndLoadUser(legacy, users)).toBeNull();
});
});
+8 -3
View File
@@ -74,10 +74,15 @@ import {
import type { TripFile, User } from '../../../src/types';
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service';
import type { EntityManager } from '@mikro-orm/core';
const storageDelete = vi.fn();
const storageStub = { delete: storageDelete } as unknown as import('../../../src/nest/storage/storage.service').StorageService;
const svc = new FilesService(new DatabaseService(testDb), permissionsStub, new RealtimeService(), new EphemeralTokenService(), storageStub);
// EntityManager stub (Plan 3b Task 1 RULING): verifyJwtAndLoadUser is
// fully mocked above, so `em.getRepository` never needs to return
// anything meaningful; it just has to not throw when the service calls it.
const emStub = { getRepository: () => ({}) } as unknown as EntityManager;
const svc = new FilesService(new DatabaseService(testDb), permissionsStub, new RealtimeService(), new EphemeralTokenService(), storageStub, emStub);
beforeAll(() => {
createTables(testDb);
@@ -555,13 +560,13 @@ describe('authenticateDownload', () => {
verifyJwtAndLoadUser.mockReturnValue({ id: 42 });
const result = await svc.authenticateDownload(req({ cookie: 'cookie-jwt', bearer: 'bearer-jwt' }));
expect(result).toEqual({ userId: 42 });
expect(verifyJwtAndLoadUser).toHaveBeenCalledWith('cookie-jwt');
expect(verifyJwtAndLoadUser).toHaveBeenCalledWith('cookie-jwt', expect.anything());
});
it('FILE-SVC-033: a bearer token is used when no cookie is present; invalid JWTs 401', async () => {
verifyJwtAndLoadUser.mockReturnValue({ id: 7 });
expect(await svc.authenticateDownload(req({ bearer: 'bearer-jwt' }))).toEqual({ userId: 7 });
expect(verifyJwtAndLoadUser).toHaveBeenCalledWith('bearer-jwt');
expect(verifyJwtAndLoadUser).toHaveBeenCalledWith('bearer-jwt', expect.anything());
verifyJwtAndLoadUser.mockReturnValue(null);
expect(await svc.authenticateDownload(req({ bearer: 'stale' }))).toEqual({ error: 'Invalid or expired token', status: 401 });
+36 -19
View File
@@ -6,13 +6,22 @@
* /api endpoint added after those lists were written answered a logged-in user
* without MFA with a 403 while answering a stranger fine. MFA-011 pins the
* correction.
*
* Plan 3b Task 1 (Task 0 review flag — the one guard test needing real
* rework): `MfaPolicyGuard` now takes `AppSettingsRepository`/`UsersRepository`/
* `WebauthnCredentialsRepository` instead of `DatabaseService`, so this file
* stubs the three REPOSITORY METHODS directly (`getValue`/`getMfaEnabled`/
* `hasAny`) rather than hand-rolling a `DatabaseService.get` stub that
* branched on SQL text substrings.
*/
import { describe, it, expect, vi } from 'vitest';
import { HttpException } from '@nestjs/common';
import { Reflector } from '@nestjs/core';
import { MfaExempt, MFA_EXEMPT, MfaPolicyGuard } from '../../../src/nest/auth/mfa-policy.guard';
import { Public, IS_PUBLIC } from '../../../src/nest/auth/public.decorator';
import type { DatabaseService } from '../../../src/nest/database/database.service';
import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository';
import type { UsersRepository } from '../../../src/db/repositories/Users.repository';
import type { WebauthnCredentialsRepository } from '../../../src/db/repositories/WebauthnCredentials.repository';
import type { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service';
import { DEMO_EMAIL_PRIMARY } from '../../../src/nest/common/demo';
import type { User } from '../../../src/types';
@@ -21,29 +30,34 @@ const user = { id: 7, email: 'u@example.test', role: 'user' } as User;
interface Rows {
requireMfa?: string;
mfaEnabled?: number | boolean | undefined;
mfaEnabled?: number | undefined;
hasPasskey?: boolean;
userMissing?: boolean;
}
function makeGuard(rows: Rows = {}, meta: { public?: boolean; exempt?: boolean } = {}, demo = false) {
const get = vi.fn((sql: string) => {
if (sql.includes('app_settings')) return rows.requireMfa === undefined ? undefined : { value: rows.requireMfa };
if (sql.includes('FROM users')) return rows.userMissing ? undefined : { mfa_enabled: rows.mfaEnabled ?? 0 };
if (sql.includes('webauthn_credentials')) return rows.hasPasskey ? { 1: 1 } : undefined;
return undefined;
});
const getValue = vi.fn(async (key: string) => (key === 'require_mfa' ? (rows.requireMfa ?? null) : null));
const appSettings = { getValue } as unknown as AppSettingsRepository;
const getMfaEnabled = vi.fn(async () => (rows.userMissing ? null : { mfa_enabled: rows.mfaEnabled ?? 0 }));
const users = { getMfaEnabled } as unknown as UsersRepository;
const hasAny = vi.fn(async () => !!rows.hasPasskey);
const webauthnCredentials = { hasAny } as unknown as WebauthnCredentialsRepository;
const reflector = {
getAllAndOverride: vi.fn((key: string) =>
(key === IS_PUBLIC && meta.public) || (key === MFA_EXEMPT && meta.exempt) ? { reason: 'x' } : undefined,
),
} as unknown as Reflector;
const guard = new MfaPolicyGuard(
{ get } as unknown as DatabaseService,
appSettings,
users,
webauthnCredentials,
{ isDemoMode: () => demo } as unknown as RuntimeEnvService,
reflector,
);
return { guard, get };
return { guard, getValue, getMfaEnabled, hasAny };
}
function ctx(request: Record<string, unknown>) {
@@ -90,9 +104,9 @@ describe('MfaPolicyGuard', () => {
});
it('MFA-004: does nothing when the policy is off, and never reads the users row', async () => {
const { guard, get } = makeGuard({ requireMfa: 'false', mfaEnabled: 0 });
const { guard, getMfaEnabled } = makeGuard({ requireMfa: 'false', mfaEnabled: 0 });
expect(await guard.canActivate(ctx({ user }))).toBe(true);
expect(get.mock.calls.some((c) => String(c[0]).includes('FROM users'))).toBe(false);
expect(getMfaEnabled).not.toHaveBeenCalled();
});
it('MFA-005: does nothing when the setting row is absent at all', async () => {
@@ -101,9 +115,9 @@ describe('MfaPolicyGuard', () => {
});
it('MFA-006: an anonymous request is not the policy business', async () => {
const { guard, get } = makeGuard(ENFORCED);
const { guard, getValue } = makeGuard(ENFORCED);
expect(await guard.canActivate(ctx({}))).toBe(true);
expect(get).not.toHaveBeenCalled();
expect(getValue).not.toHaveBeenCalled();
});
it('MFA-007: the demo account is exempt while demo mode is on', async () => {
@@ -128,10 +142,13 @@ describe('MfaPolicyGuard', () => {
it('MFA-010: it reads req.user rather than verifying the token again', async () => {
// The middleware this replaces called jwt.verify and SELECTed the users row a
// second time on every /api request, then discarded the result.
const { guard, get } = makeGuard({ requireMfa: 'true', mfaEnabled: 1 });
// second time on every /api request, then discarded the result. Now it reads
// exactly the two repository methods the policy needs — never a third,
// token-shaped read.
const { guard, getValue, getMfaEnabled } = makeGuard({ requireMfa: 'true', mfaEnabled: 1 });
await guard.canActivate(ctx({ user, headers: { authorization: 'Bearer nonsense' } }));
expect(get.mock.calls.map((c) => c[0]).every((sql) => !String(sql).includes('password_version'))).toBe(true);
expect(getValue).toHaveBeenCalledWith('require_mfa');
expect(getMfaEnabled).toHaveBeenCalledWith(user.id);
});
it('MFA-011: a @Public route no longer 403s a logged-in user without MFA', async () => {
@@ -139,9 +156,9 @@ describe('MfaPolicyGuard', () => {
// routes and /api/health/features all answered a stranger fine and answered
// this user with a 403, because they were added after the middleware path
// lists were written and nobody noticed.
const { guard, get } = makeGuard(ENFORCED, { public: true });
const { guard, getValue } = makeGuard(ENFORCED, { public: true });
expect(await guard.canActivate(ctx({ user }))).toBe(true);
expect(get).not.toHaveBeenCalled();
expect(getValue).not.toHaveBeenCalled();
});
it('MFA-012: the decorators write what the guard reads', () => {
+1 -1
View File
@@ -125,7 +125,7 @@ beforeAll(async () => {
new DatabaseService(testDb),
new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)),
membership,
new WebauthnConfigService(new DatabaseService(testDb)),
new WebauthnConfigService(await createTestAppSettingsRepo(testDb)),
new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb)), await createTestUnitOfWork(testDb)),
mailerStub,
new EphemeralTokenService(),
@@ -118,7 +118,7 @@ beforeAll(async () => {
new DatabaseService(testDb),
new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)),
new TripMembershipService(new DatabaseService(testDb)),
new WebauthnConfigService(new DatabaseService(testDb)),
new WebauthnConfigService(await createTestAppSettingsRepo(testDb)),
new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(testDb)), await createTestUnitOfWork(testDb)),
mailerStub,
new EphemeralTokenService(),
@@ -178,7 +178,7 @@ describe('applyPlatformUploads', () => {
res,
next,
);
expect(h.verifyJwtAndLoadUser).toHaveBeenCalledWith('jwt123');
expect(h.verifyJwtAndLoadUser).toHaveBeenCalledWith('jwt123', expect.anything());
expect(h.sendToResponse).toHaveBeenCalledWith('photos', 'a.jpg', res);
});
@@ -188,7 +188,7 @@ describe('applyPlatformUploads', () => {
h.verifyJwtAndLoadUser.mockReturnValue({ id: 1 });
const res = makeRes();
await photoHandler()({ params: { filename: 'a.jpg' }, headers: {}, query: { token: 'qtok' } }, res, next);
expect(h.verifyJwtAndLoadUser).toHaveBeenCalledWith('qtok');
expect(h.verifyJwtAndLoadUser).toHaveBeenCalledWith('qtok', expect.anything());
expect(h.sendToResponse).toHaveBeenCalledWith('photos', 'a.jpg', res);
});
@@ -315,6 +315,54 @@ describe('TrekWsAdapter D6 request context on connect (Plan 3b Task 0)', () => {
});
});
/**
* D6 request context on the DISCONNECT dispatch (Task 0 review addendum,
* LOW item 2) — the same property WSAD-050/051 pin for the connect dispatch,
* now for `bindClientDisconnect` (`OnGatewayDisconnect` /
* `RealtimeGateway.handleDisconnect`). The base `WsAdapter.bindClientDisconnect`
* is a bare `client.on('close', callback)` with no context of any kind.
*/
describe('TrekWsAdapter D6 request context on disconnect (Task 0 review addendum)', () => {
it('WSAD-052: without MikroORM passed to the adapter, the disconnect dispatch THROWS rather than running handleDisconnect unwrapped', () => {
const ad = new TrekWsAdapter({} as HttpServer); // no orm
const socket = fakeSocket();
let handlerRan = false;
ad.bindClientDisconnect(socket as never, () => {
handlerRan = true;
});
expect(() => socket.emit('close', Buffer.from(''))).toThrow(/no MikroORM available/i);
// Same fail-closed shape as WSAD-050: the wrapper throws BEFORE the
// disconnect callback ever runs.
expect(handlerRan).toBe(false);
});
it('WSAD-053: with MikroORM passed to the adapter, a repository read inside the disconnect callback succeeds — the wrapper is load-bearing', async () => {
const t = await createTestOrm(testDb, { allowGlobalContext: false });
try {
let result: unknown;
let caught: unknown;
const ad = new TrekWsAdapter({} as HttpServer, t.orm);
const socket = fakeSocket();
let captured: Promise<unknown> | undefined;
ad.bindClientDisconnect(socket as never, () => {
captured = (async () => {
try {
result = await t.orm.em.find(Users, {});
} catch (e) {
caught = e;
}
})();
});
socket.emit('close', Buffer.from(''));
await captured;
expect(caught).toBeUndefined();
expect(Array.isArray(result)).toBe(true);
} finally {
await t.close();
}
});
});
/**
* The pointer exemption (#1973).
*
@@ -78,7 +78,7 @@ import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.serv
import { makeStorageFixture } from '../../helpers/storage-fixture';
import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service';
import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository';
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo } from '../../helpers/test-uow';
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow';
// Real sibling services over the same in-memory DB — the aggregation runs the
// actual SQL of every domain it fans out to, so a shape change downstream shows
@@ -120,7 +120,7 @@ const buildReadModel = async (database: DatabaseService, roster: TripMembersServ
new CollabService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), makeStorageFixture('').storage, new RateLimitService(), await createTestUnitOfWork(dbs().connection)),
placesSvc,
new TodoService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), await createTestUnitOfWork(dbs().connection)),
new FilesService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), new EphemeralTokenService(), makeStorageFixture('').storage),
new FilesService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), new EphemeralTokenService(), makeStorageFixture('').storage, (await sharedTestOrm(testDb)).em),
);
let svc: Awaited<ReturnType<typeof buildReadModel>>;
+2 -2
View File
@@ -88,7 +88,7 @@ import fs from 'fs';
import path from 'path';
import { notificationsStub } from '../../helpers/notifications';
import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service';
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo } from '../../helpers/test-uow';
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow';
// Real sibling services over the same in-memory DB — updateTrip's date-shift
// resyncs and the summary/bundle aggregation run their actual SQL.
@@ -156,7 +156,7 @@ beforeAll(async () => {
new CollabService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), coversFx.storage, new RateLimitService(), await createTestUnitOfWork(dbs().connection)),
placesSvc,
new TodoService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), await createTestUnitOfWork(dbs().connection)),
new FilesService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), new EphemeralTokenService(), coversFx.storage),
new FilesService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), new EphemeralTokenService(), coversFx.storage, (await sharedTestOrm(testDb)).em),
);
});
@@ -42,7 +42,6 @@ import fs from 'node:fs';
import path from 'node:path';
import { UserProfileService } from '../../../src/nest/auth/user-profile.service';
import { makeStorageFixture } from '../../helpers/storage-fixture';
import { DatabaseService } from '../../../src/nest/database/database.service';
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo } from '../../helpers/test-uow';
import { SEARCH_TEXT_FIELD_MASK } from '../../../src/nest/maps/maps.helpers';
@@ -53,7 +52,6 @@ beforeAll(async () => {
createTables(testDb);
runMigrations(testDb);
profile = new UserProfileService(
new DatabaseService(testDb),
avatarsFx.storage,
await createTestUnitOfWork(testDb),
await createTestAppSettingsRepo(testDb),
@@ -22,22 +22,19 @@ vi.mock('../../../src/app-config', async (importOriginal) => {
});
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { DatabaseService } from '../../../src/nest/database/database.service';
import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository';
import { WebauthnConfigService, originWithinRpScope } from '../../../src/nest/auth/webauthn-config.service';
// Injected instead of vi.mocking src/db/database: the service reads exactly one
// row shape (app_settings.value), so a stub connection keeps the suite free of
// a schema and states the only DB contact the resolver has.
const conn = {
prepare: (_sql: string) => ({
get: (key: string) => {
const v = settingsStore.get(key);
return v === undefined ? undefined : { value: v };
},
}),
} as unknown as ConstructorParameters<typeof DatabaseService>[0];
// Injected instead of vi.mocking src/db/database (Plan 3b Task 1: the service
// now takes an AppSettingsRepository, not DatabaseService): the service reads
// exactly one method (`getValue`), so a fake repository over the same real
// Map keeps the suite free of a schema and states the only contact the
// resolver has, unchanged from the DatabaseService-stub version.
const appSettings = {
getValue: async (key: string) => settingsStore.get(key) ?? null,
} as unknown as AppSettingsRepository;
const svc = new WebauthnConfigService(new DatabaseService(conn));
const svc = new WebauthnConfigService(appSettings);
beforeEach(() => {
settingsStore.clear();
+25 -16
View File
@@ -13,9 +13,18 @@ const { pluginsEnabledMock, extractTokenMock, verifyMock } = vi.hoisted(() => ({
vi.mock('../../../src/nest/plugins/kill-switch', () => ({ pluginsEnabled: pluginsEnabledMock }));
vi.mock('../../../src/nest/auth/jwt-verify', () => ({ extractToken: extractTokenMock, verifyJwtAndLoadUser: verifyMock }));
import type { EntityManager } from '@mikro-orm/core';
import { PluginsProxyController } from '../../../src/nest/plugins/plugins-proxy.controller';
import type { PluginRuntimeService } from '../../../src/nest/plugins/plugin-runtime.service';
// EntityManager injection (Plan 3b Task 1 RULING) — verifyJwtAndLoadUser is
// fully mocked above, so `em.getRepository` never needs to return anything
// meaningful; it just has to not throw when the controller calls it.
const emStub = { getRepository: () => ({}) } as unknown as EntityManager;
function ctrl(runtime: PluginRuntimeService): PluginsProxyController {
return new PluginsProxyController(runtime, emStub);
}
function fakeRes() {
const res = {
statusCode: 0,
@@ -52,33 +61,33 @@ describe('PluginsProxyController', () => {
it('404 when the runtime is disabled', async () => {
pluginsEnabledMock.mockReturnValue(false);
const res = fakeRes();
await new PluginsProxyController(makeRuntime()).proxy('p', fakeReq('GET', '/status'), res as never);
await ctrl(makeRuntime()).proxy('p', fakeReq('GET', '/status'), res as never);
expect(res.statusCode).toBe(404);
});
it('404 when the plugin is not active', async () => {
const res = fakeRes();
await new PluginsProxyController(makeRuntime({ isActive: vi.fn(() => false) } as never)).proxy('p', fakeReq('GET', '/status'), res as never);
await ctrl(makeRuntime({ isActive: vi.fn(() => false) } as never)).proxy('p', fakeReq('GET', '/status'), res as never);
expect(res.statusCode).toBe(404);
});
it('404 when no declared route matches', async () => {
const res = fakeRes();
await new PluginsProxyController(makeRuntime()).proxy('p', fakeReq('GET', '/nope'), res as never);
await ctrl(makeRuntime()).proxy('p', fakeReq('GET', '/nope'), res as never);
expect(res.statusCode).toBe(404);
});
it('401 on an auth route without a valid session', async () => {
verifyMock.mockReturnValue(null as never);
const res = fakeRes();
await new PluginsProxyController(makeRuntime()).proxy('p', fakeReq('GET', '/status'), res as never);
await ctrl(makeRuntime()).proxy('p', fakeReq('GET', '/status'), res as never);
expect(res.statusCode).toBe(401);
});
it('forwards an authenticated request and returns the child response', async () => {
const runtime = makeRuntime();
const res = fakeRes();
await new PluginsProxyController(runtime).proxy('p', fakeReq('GET', '/status'), res as never);
await ctrl(runtime).proxy('p', fakeReq('GET', '/status'), res as never);
expect(res.statusCode).toBe(200);
expect(res.headers['content-type']).toBe('application/json');
expect(res.body).toBe('{"ok":true}');
@@ -96,7 +105,7 @@ describe('PluginsProxyController', () => {
verifyMock.mockReturnValue({ id: 7, username: 'root', role: 'admin' } as never);
const runtime = makeRuntime();
const res = fakeRes();
await new PluginsProxyController(runtime).proxy('p', fakeReq('GET', '/status'), res as never);
await ctrl(runtime).proxy('p', fakeReq('GET', '/status'), res as never);
expect(runtime.invoke).toHaveBeenCalledWith('p', 'invoke.route', expect.objectContaining({
req: expect.objectContaining({ user: { id: 7, username: 'root', isAdmin: true } }),
}), 7);
@@ -105,7 +114,7 @@ describe('PluginsProxyController', () => {
it('a public (auth:false) route skips the session check', async () => {
const runtime = makeRuntime({ routesOf: vi.fn(() => [{ i: 1, method: 'POST', path: '/webhook', auth: false }]) } as never);
const res = fakeRes();
await new PluginsProxyController(runtime).proxy('p', fakeReq('POST', '/webhook'), res as never);
await ctrl(runtime).proxy('p', fakeReq('POST', '/webhook'), res as never);
expect(res.statusCode).toBe(200);
expect(extractTokenMock).not.toHaveBeenCalled();
// a public route has no session user → no bound acting user (undefined)
@@ -117,7 +126,7 @@ describe('PluginsProxyController', () => {
it('forwards the raw request bytes to a webhook (auth:false) route, but withholds them from an authenticated route', async () => {
// Webhook route → the plugin gets the raw payload so it can verify an HMAC.
const wh = makeRuntime({ routesOf: vi.fn(() => [{ i: 1, method: 'POST', path: '/webhook', auth: false }]) } as never);
await new PluginsProxyController(wh).proxy('p', fakeReq('POST', '/webhook', { rawBody: Buffer.from('{"a":1}') }), fakeRes() as never);
await ctrl(wh).proxy('p', fakeReq('POST', '/webhook', { rawBody: Buffer.from('{"a":1}') }), fakeRes() as never);
// forwarded as base64 so a non-UTF-8 signed body survives
expect(wh.invoke).toHaveBeenCalledWith('p', 'invoke.route', expect.objectContaining({
req: expect.objectContaining({ rawBodyBase64: Buffer.from('{"a":1}').toString('base64') }),
@@ -125,7 +134,7 @@ describe('PluginsProxyController', () => {
// Authenticated route → raw bytes are never handed to the plugin.
const auth = makeRuntime();
await new PluginsProxyController(auth).proxy('p', fakeReq('GET', '/status', { rawBody: Buffer.from('secret') }), fakeRes() as never);
await ctrl(auth).proxy('p', fakeReq('GET', '/status', { rawBody: Buffer.from('secret') }), fakeRes() as never);
const fwd = (auth.invoke as unknown as { mock: { calls: unknown[][] } }).mock.calls[0][2] as { req: { rawBodyBase64?: unknown } };
expect(fwd.req.rawBodyBase64).toBeUndefined();
});
@@ -141,7 +150,7 @@ describe('PluginsProxyController', () => {
authorization: 'Bearer leak', // must be dropped
'x-socket-id': 'sock-1', // must be dropped
};
await new PluginsProxyController(runtime).proxy('p', fakeReq('POST', '/webhook', { headers }), res as never);
await ctrl(runtime).proxy('p', fakeReq('POST', '/webhook', { headers }), res as never);
const forwarded = (runtime.invoke as unknown as { mock: { calls: unknown[][] } }).mock.calls[0][2] as { req: { headers: Record<string, string> } };
expect(forwarded.req.headers).toEqual({ 'content-type': 'application/json', 'stripe-signature': 't=1,v1=abc', 'x-hub-signature-256': 'sha256=deadbeef' });
expect(forwarded.req.headers.cookie).toBeUndefined();
@@ -152,7 +161,7 @@ describe('PluginsProxyController', () => {
it('an authenticated route gets NO inbound headers (even safe ones)', async () => {
const runtime = makeRuntime(); // /status, auth:true
const res = fakeRes();
await new PluginsProxyController(runtime).proxy('p', fakeReq('GET', '/status', { headers: { 'content-type': 'application/json', 'stripe-signature': 'x' } }), res as never);
await ctrl(runtime).proxy('p', fakeReq('GET', '/status', { headers: { 'content-type': 'application/json', 'stripe-signature': 'x' } }), res as never);
const forwarded = (runtime.invoke as unknown as { mock: { calls: unknown[][] } }).mock.calls[0][2] as { req: { headers: Record<string, string> } };
expect(forwarded.req.headers).toEqual({});
});
@@ -162,7 +171,7 @@ describe('PluginsProxyController', () => {
invoke: vi.fn(async () => ({ status: 200, headers: { 'content-type': 'text/plain', 'set-cookie': 'evil=1' }, body: 'ok' })),
} as never);
const res = fakeRes();
await new PluginsProxyController(runtime).proxy('p', fakeReq('GET', '/status'), res as never);
await ctrl(runtime).proxy('p', fakeReq('GET', '/status'), res as never);
expect(res.headers['content-type']).toBe('text/plain');
expect(res.headers['set-cookie']).toBeUndefined(); // a plugin cannot set cookies
});
@@ -170,7 +179,7 @@ describe('PluginsProxyController', () => {
it('502 when the plugin invoke throws', async () => {
const runtime = makeRuntime({ invoke: vi.fn(async () => { throw new Error('down'); }) } as never);
const res = fakeRes();
await new PluginsProxyController(runtime).proxy('p', fakeReq('GET', '/status'), res as never);
await ctrl(runtime).proxy('p', fakeReq('GET', '/status'), res as never);
expect(res.statusCode).toBe(502);
});
@@ -179,7 +188,7 @@ describe('PluginsProxyController', () => {
invoke: vi.fn(async () => ({ status: 302, headers: { location: '/trips/5?tab=plan' } })),
} as never);
const res = fakeRes();
await new PluginsProxyController(runtime).proxy('p', fakeReq('GET', '/status'), res as never);
await ctrl(runtime).proxy('p', fakeReq('GET', '/status'), res as never);
expect(res.statusCode).toBe(302);
// normalised to path+query, still same-origin
expect(res.headers['Location']).toBe('/trips/5?tab=plan');
@@ -200,7 +209,7 @@ describe('PluginsProxyController', () => {
invoke: vi.fn(async () => ({ status: 302, headers: { location: loc } })),
} as never);
const res = fakeRes();
await new PluginsProxyController(runtime).proxy('p', fakeReq('GET', '/status'), res as never);
await ctrl(runtime).proxy('p', fakeReq('GET', '/status'), res as never);
expect(res.statusCode).toBe(502);
expect(res.body).toMatchObject({ detail: 'unsafe redirect target' });
expect(res.headers['Location']).toBeUndefined();
@@ -211,7 +220,7 @@ describe('PluginsProxyController', () => {
invoke: vi.fn(async () => ({ status: 301, headers: {} })),
} as never);
const res = fakeRes();
await new PluginsProxyController(runtime).proxy('p', fakeReq('GET', '/status'), res as never);
await ctrl(runtime).proxy('p', fakeReq('GET', '/status'), res as never);
expect(res.statusCode).toBe(502);
});
});