Commit Graph

7 Commits

Author SHA1 Message Date
Maurice a876fb2634 feat: Passkey (WebAuthn) login (#1111)
* feat(auth): passkey (WebAuthn) login — server endpoints, schema + admin toggle

Add @simplewebauthn/server registration and primary (discoverable) login ceremonies under /api/auth/passkey, a webauthn_credentials + single-use webauthn_challenges schema (migration), the instance-wide passkey_login toggle (default off) enforced before auth by a guard, and require_mfa satisfaction via a verified passkey. RP ID/origin come only from server config (webauthn_rp_id/origins -> APP_URL), never request headers.

* feat(auth): passkey enrolment, login button + admin settings UI

PasskeysSection in account settings (add/rename/remove with a current-password step-up), a 'Sign in with a passkey' button on the login page, the admin enable + RP-ID/origins controls, and a per-user admin reset action.

* i18n(auth): passkey strings across all locales

Add login/settings/admin passkey keys to en and all 19 translated locales.
2026-06-05 18:54:13 +02:00
Julien G. 1f5deeba6c Bug fixes - April 27th 2026 (#907)
* fix: clean up dangling FK references before deleting a user

Resolves FOREIGN KEY constraint failed (500) on DELETE /api/admin/users/:id
and DELETE /api/auth/me when the target user had rows in trip_members.invited_by,
share_tokens.created_by, budget_items.paid_by_user_id, journeys.user_id,
journey_entries.author_id, journey_contributors.user_id, or
journey_share_tokens.created_by — none of which had ON DELETE clauses.

Introduces deleteUserCompletely() in userCleanupService.ts which wraps all
cleanup and the final DELETE FROM users in a single transaction. Both
adminService.deleteUser and authService.deleteAccount now call it instead of
the bare DELETE. Tests ADMIN-005b and AUTH-040 cover all reference types
including notification sender/recipient and notice dismissals.

* test: extend FK deletion tests to cover journeys, files, and photos

ADMIN-005b and AUTH-040 now also seed and assert:
- owned journey with entries (cascade-deleted via journeys.user_id cleanup)
- trip_files.uploaded_by (SET NULL — file survives, attribution cleared)
- trek_photos.owner_id (SET NULL — photo record survives, owner cleared)
- trip_photos.user_id (CASCADE — photo association removed)

* test: extend user deletion tests to cover all FK relationships

ADMIN-005b and AUTH-040 now seed and assert every user FK relationship:

CASCADE (row deleted): trips, trip_members, tags, mcp_tokens, oauth_tokens,
oauth_consents, vacay_plans, vacay_plan_members, bucket_list,
visited_countries, visited_regions, packing_templates, invite_tokens,
collab_notes, settings, password_reset_tokens, notification_channel_preferences

SET NULL (row survives, column nulled): categories, todo_items.assigned_user_id,
packing_bags, audit_log

Caught and fixed: notification_preferences was dropped in migration 72;
correct table is notification_channel_preferences.

* fix: preserve URL hash and OIDC redirect target through login flow

- Include location.hash in redirect param at all three producer sites
  (ProtectedRoute, axios 401 interceptor, OAuthAuthorizePage) so
  hash fragments survive the login bounce
- Stash redirectTarget in sessionStorage before any OIDC provider
  redirect and restore it after the code exchange, since the IdP
  strips the original ?redirect= param during the roundtrip
- Clear sessionStorage on OIDC error to avoid stale state
- Add tests covering sessionStorage stash on mount, navigate to saved
  redirect after OIDC exchange, fallback to /dashboard, and cleanup
  on error

* fix: use day position instead of ID for accommodation date range clamping

Math.min/Math.max over raw day IDs breaks the start/end picker when a
trip's day IDs are non-monotonic relative to day_number (normal after
repeated generateDays extend/shrink cycles). Replaced with findIndex
lookups so clamping is always based on positional order.

Closes #889

* fix: normalize env var comparisons to be case-insensitive

All NODE_ENV, DEMO_MODE, OIDC_ONLY, FORCE_HTTPS, COOKIE_SECURE, and
ALLOW_INTERNAL_NETWORK checks now use .toLowerCase() so values like
'Production' or 'True' behave identically to their lowercase forms.
Also adds APP_VERSION to the startup banner.

* fix: delete surplus days when shortening a trip

When shrinking a trip's date range, surplus days are now deleted along
with their assignments, notes, and accommodations (cascade). Places
remain in the trip pool; reservations keep their day reference nulled
by the existing ON DELETE SET NULL constraint (issue #909).

Updates TRIP-SVC-011 to reflect the new behaviour; adds TRIP-SVC-016
as a regression test for the empty-day case.

* fix: auto-backup retention deletes itself and manual backups on Docker

Two bugs in cleanupOldBackups:
1. Filter was .endsWith('.zip') — swept manual backup-*.zip files too.
   Now restricted to auto-backup-* prefix.
2. Age was derived from stat.birthtimeMs, which is 0 on overlayfs
   (Docker default), making every backup appear epoch-old and get
   deleted immediately. Age is now parsed from the filename timestamp
   and falls back to mtimeMs (reliable on overlayfs).

Also converts inline require('./services/auditLog') calls to a static
import throughout scheduler.ts, and adds 8 unit tests covering the
fixed retention logic including the overlayfs regression case.

* test: update TRIP-024 to match delete behavior on trip shrink

* feat: add bypass-branch-check label to skip branch enforcement
2026-04-28 05:17:20 +02:00
Maurice 292e443dbe security: address silent-failure review findings on top of batch 1
Second-pass fixes caught by a self-review after the initial commit — each
one would have undermined a fix from the previous commit.

- mfaPolicy now goes through `verifyJwtAndLoadUser` too. Without this,
  a JWT stolen before a password reset still satisfied `require_mfa`
  until its natural 24h expiry, defeating the whole point of the
  password_version bump.
- Drop the `?? keys[0]` fallback in OIDC JWKS key selection. When the
  token carries a `kid` that is not in the current JWKS, refuse
  outright instead of picking an arbitrary key and letting the
  signature check produce a generic failure — the real failure mode
  deserves a specific error code.
- Tighten OAuth DCR custom-scheme rule so `javascript:`, `data:`,
  `vbscript:`, `file:`, `blob:`, `about:`, `chrome:` are all rejected.
  Previously the catch-all "not http/https" check admitted them; the
  authorize flow later 302s the browser to whatever is registered,
  which with a `javascript:` URI would execute attacker script on
  redirect. Also require the private-use scheme body to be reverse-DNS
  (contain a dot), matching RFC 8252 §7.1.
- permanentDeleteFile / emptyTrash only delete the trip_files row when
  the on-disk unlink actually succeeded. Previously Promise.all
  swallowed individual unlink failures and DELETE ran unconditionally,
  so a permission / ENOSPC failure would orphan bytes on disk.
- restoreFromZip also invalidates the permissions cache in the outer
  catch. If extraction threw before the DB swap even started, the
  cache wasn't stale, but belt-and-braces is cheap and guarantees no
  failed-restore path leaves stale cache behind.
2026-04-20 20:44:57 +02:00
Maurice 2d0414b4a3 security: internal audit — batch 1
Fixes the critical + high + medium findings from our internal security
review. Bundled into one PR because the changes overlap heavily (JWT
verification unifies across three call sites; backup-code hashing and
demo-email handling cross-cut several services); splitting them out
would mean redundant reviews of the same files.

Critical
- CI-C1 — .github/workflows/test.yml: restore actions/{checkout,setup-
  node,upload-artifact} to @v4. The @v6 refs don't exist, so the test
  workflow was errorring before a single test ran.
- SEC-C1 — mfaPolicy now extracts the token via extractToken() (cookie-
  first, Bearer fallback). Previously it only read Authorization, so
  every cookie-authenticated SPA session bypassed require_mfa entirely.
- SEC-C2/C4/C6 — all JWT verification paths (MCP bearer, file download,
  photo route) now go through the shared verifyJwtAndLoadUser that
  checks password_version. resetPassword additionally deletes every
  mcp_tokens row and marks outstanding oauth_tokens revoked, so a
  password reset invalidates ALL credential classes — not just the
  cookie JWT.

High
- SEC-H2 — reset email URL is built from server-side APP_URL /
  ALLOWED_ORIGINS (via existing getAppUrl()), not request headers.
  Closes the host-header-injection vector into reset links.
- SEC-H3 — OIDC findOrCreateUser wraps the invite-redemption UPDATE +
  user INSERT in a transaction. The UPDATE is the capacity check; if
  a concurrent callback takes the last slot, the whole transaction
  aborts with registration_disabled instead of double-creating users.
- SEC-H4 — new verifyIdToken() performs full JWT signature
  verification via the provider's JWKS (Node's crypto.createPublicKey
  accepts JWK directly — no extra dependency), plus iss/aud/exp
  checks. The callback also rejects the login when userinfo.sub does
  not match id_token.sub.
- SEC-H5 — OAuth DCR now validates redirect_uris against an allowlist
  of schemes: https, http-loopback, or a private custom scheme. Plain
  http://non-loopback is rejected.
- SEC-H6 — oauthService audience defaults to mcpResource when the
  `resource` parameter is missing, so tokens are always audience-bound
  to /mcp instead of being issued with audience=null.
- SEC-H7 — HSTS is enabled any time NODE_ENV=production (previously
  required FORCE_HTTPS=true), includeSubDomains defaults on and can
  be disabled with HSTS_INCLUDE_SUBDOMAINS=false.
- SEC-H8 — trek_session cookie Secure flag is also driven by
  req.secure (which Express resolves from X-Forwarded-Proto once
  trust proxy is set), so instances behind a TLS-terminating proxy
  get Secure cookies without needing FORCE_HTTPS.

Medium
- SEC-M1 — permanentDeleteFile / emptyTrash / avatar unlink now use
  fs.promises.rm with { force: true } (one async op vs the previous
  existsSync + unlinkSync pair per file).
- SEC-M2 — invalidatePermissionsCache() is called inside restoreFromZip
  so a restored DB with different permission rows is honoured
  immediately.
- SEC-M3 + C1 — idempotency store bounds the key at 128 chars, caches
  only responses ≤ 256 KiB, and scopes the lookup by (key, user_id,
  method, path) rather than (key, user_id). Same key replayed against
  a different endpoint no longer returns a stale unrelated body.
- SEC-M4 — share_tokens gets an expires_at column; new tokens default
  to 90-day TTL, expired tokens are denied at lookup. Existing tokens
  stay NULL = no expiry so already-published links don't break.
- SEC-M5 — /uploads/photos/:filename now resolves the photo to its
  trip_id and requires the share token to cover THAT trip. Previously
  any share token for any trip would unlock any photo filename.
- SEC-M6 — BLOCKED_EXTENSIONS is the single source of truth shared
  between fileService and collab uploads. The '*' allowed_file_types
  wildcard now still rejects executables/scripts.
- SEC-M7 — single DEMO_EMAILS constant (services/demo.ts) used by
  demoUploadBlock, mfaPolicy, and every demo-mode guard in
  authService. The old demoUploadBlock only matched 'demo@nomad.app'
  so the seed 'demo@trek.app' could in fact upload in demo mode.
- SEC-M8 — MFA backup codes are now bcrypt-hashed at rest
  (hashBackupCodeBcrypt). matchBackupCode accepts both bcrypt and
  legacy SHA-256 hex hashes, so existing installs keep working until
  the user regenerates codes via enableMfa.
- SEC-M9 — document the "security via UUID v4 filename" model for
  /uploads/avatars|covers|journey. Requires no code change but
  captures the decision so future reviewers don't re-flag it.
- SEC-M10 — already covered by the resetPassword revocation logic
  above: mcp_tokens DELETE + oauth_tokens UPDATE … SET revoked_at.

Performance
- PERF-H1 — new migration adds the indexes flagged in the audit:
  trips(user_id), trips(created_at DESC), photos(day_id),
  photos(place_id), reservations(day_id), share_tokens(token), plus
  conditional day_accommodations and notifications indexes depending
  on which columns are present.

Tests
- tests/integration/oidc.test.ts now mocks verifyIdToken and passes
  an id_token in the exchangeCodeForToken stub for the three flows
  that exercise a successful callback. The three remaining failures
  tests pointed out were all pre-existing (file-upload flakes +
  notificationPreferences event_types count drift), none introduced
  by this PR.
2026-04-20 20:36:52 +02:00
Julien G. 905c7d460b Add comprehensive backend test suite (#339)
* add test suite, mostly covers integration testing, tests are only backend side

* workflow runs the correct script

* workflow runs the correct script

* workflow runs the correct script

* unit tests incoming

* Fix multer silent rejections and error handler info leak

- Revert cb(null, false) to cb(new Error(...)) in auth.ts, collab.ts,
  and files.ts so invalid uploads return an error instead of silently
  dropping the file
- Error handler in app.ts now always returns 500 / "Internal server
  error" instead of forwarding err.message to the client

* Use statusCode consistently for multer errors and error handler

- Error handler in app.ts reads err.statusCode to forward the correct
  HTTP status while keeping the response body generic
2026-04-03 13:17:53 +02:00
Claude fedd559fd6 fix: pin JWT algorithm to HS256 and harden token security
- Add { algorithms: ['HS256'] } to all jwt.verify() calls to prevent
  algorithm confusion attacks (including the 'none' algorithm)
- Add { algorithm: 'HS256' } to all jwt.sign() calls for consistency
- Reduce OIDC token payload to only { id } (was leaking username, email, role)
- Validate OIDC redirect URI against APP_URL env var when configured
- Add startup warning when JWT_SECRET is auto-generated

https://claude.ai/code/session_01SoQKcF5Rz9Y8Nzo4PzkxY8
2026-03-31 00:33:53 +00:00
fgbona 66f5ea50c5 feat(require-mfa): #155 enforce MFA via admin policy toggle across app access
Add an admin-controlled `require_mfa` policy in App Settings and expose it via `/auth/app-config` so the client can enforce it globally. Users without MFA are redirected to Settings after login and blocked from protected API/WebSocket access until setup is completed, while preserving MFA setup endpoints and admin recovery paths. Also prevent enabling the policy unless the acting admin already has MFA enabled, and block MFA disable while the policy is active. Includes UI toggle in Admin > Settings, required-policy notice in Settings, client-side 403 `MFA_REQUIRED` handling, and i18n updates for all supported locales.
2026-03-30 17:42:40 -03:00