Commit Graph

7 Commits

Author SHA1 Message Date
Paddy 33489585a0 Model permissions as (app, role) and name passkeys from their AAGUID
Two changes to the admin module, both made now because it is not deployed yet
and neither is free later.

Permissions were "an app", with a `role` column reserved for a future
fine-grained model. Reviewing whether that reservation was enough found three
problems:

- Every row was written with role = 'admin', hardcoded, and the column
  defaulted to it. On a `tickets` row that reads as "tickets administrator"
  when it only ever meant "has access", and once real roles existed there
  would have been no way to tell an old plain grant from a deliberate one.
- The role never left the database. /admin/me, the user list, the user detail
  and both write endpoints all spoke apps: AppName[]. Adding roles would have
  been a breaking change to /admin/me - and after the cutover that endpoint
  has two more consumers, turning a local edit into a coordinated deploy of
  three apps.
- The key (user_id, app) allowed one role per app, i.e. a tier rather than a
  set of capabilities. Choosing later means an ALTER on a live table.

So: the key is now (user_id, app, role), the role is `access`, and APP_ROLES
in admin.schema.ts is the contract - a role not listed there is rejected with
400 rather than written. permissions: [{app, role}] is on the wire alongside
the derived apps: AppName[], which is kept because the three frontends only
ever ask "may I show this app?". Both write endpoints accept either shape, and
the invitation column (now `permissions`) is parsed leniently: invitations live
seven days, so a deploy that changes the shape has in-flight rows in the old
one. requireAppAccess(app, role?) takes an optional role; nothing passes one
yet.

countActiveAdminsForUpdate now counts DISTINCT users rather than rows. With
several roles per app, counting rows would make a single admin holding two
roles look like two admins and defeat the last-admin guard at exactly the
moment it matters.

Separately, passkey registration now fills `name` from the authenticator's
AAGUID via registration.afterVerification and better-auth's own
getAuthenticatorName, yielding "1Password", "iCloud Keychain", "Windows Hello".
Without it the column stayed NULL and the account page could only label every
passkey "Passkey" - useless when someone has to remove the one on the device
they just lost. A client-supplied name still wins; an unknown AAGUID still
leaves it blank.

148 unit tests (up from 131, including the new admin.schema.test.ts) and 41
integration tests pass. The integration suite applies sql/admin/001_init.sql,
so the new key is exercised rather than trusted.

No production migration is needed - the module is not deployed. An existing dev
database needs three statements: set role = 'access', drop and re-add the
primary key, rename invitations.apps to permissions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-06 11:23:08 +02:00
Paddy dbcd5b56f6 Harden the admin module after a fresh-context review
Six defects found by an independent review of 7aac07a.

Environment handling now fails safe. NODE_ENV=production was gating the
signing key, the cookie domain, the CORS origin list and invitation-token
logging all at once, and it was documented nowhere - an unset value, which is
what a fresh Plesk vhost gives you, silently degraded all four. Only
'development' and 'test' relax anything now; everything else, unset included,
is strict. The hardcoded fallback secret is gone (dev gets a random
per-process one, so no committed value can ever sign a production cookie),
and invitation-link logging is an explicit ADMIN_LOG_INVITE_LINKS opt-in that
is refused in strict mode.

Rate limiting no longer collapses into a single global bucket. Without
trustedProxies, better-auth rejects a multi-value x-forwarded-for, resolves no
client IP, and keys every request to "no-trusted-ip" - where /sign-in/*
allows 3 requests per 10 seconds, so one noisy client could lock the whole
organisation out. CLIENT_IP_HEADERS and TRUSTED_PROXY_IPS make this explicit,
the unspecified x-forwarded-for fallback is gone, and strict mode warns at
boot when no trusted proxy is configured.

Invite acceptance is transactional. The user and its credential account go in
one runWithTransaction, as better-auth's own sign-up route does. A transaction
cannot span the permission and invitation writes - those use this module's own
pool - so a failure there is compensated: the user row is deleted and the
invitation un-marked, so the link works again instead of leaving the invitee
with a burnt token and an account no route can repair.

The last-admin guards were check-then-act. Two admins each removing the
other's admin permission could both pass the check and both commit, leaving
nobody able to administer anything. The count now runs inside the write
transaction under SELECT ... FOR UPDATE.

Also: lastSignInAt filtered expired sessions in the detail endpoint but not
the list, so the two disagreed; and the integration suite never reset
rateLimit, leaving it one added sign-in away from 429s that look like auth
bugs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-05 18:28:52 +02:00
Paddy 7aac07a013 Add admin identity module: better-auth, per-app permissions, invitations
Introduces src/models/admin/, a dedicated identity and permissions module on
its own nachklang_admin database, and the shared authenticator that feedback
and tickets will move onto in the cutover step. Nothing swaps over yet:
feedback.auth.ts and tickets.auth.ts still authenticate against the legacy
calendar sessions, so production behaviour is unchanged.

- better-auth 1.7 mounted at /admin/auth/*, sessions as httpOnly cookies
  scoped to .nachklang.art so one sign-in covers every *.nachklang.art app.
- Accounts are invite-only: public sign-up is disabled, and the invitations
  plugin is the only code that creates users. Tokens are stored as SHA-256
  hashes and travel in the request body, never in a URL.
- Per-app permissions in user_app_permissions; requireAppAccess(app) queries
  the database on every request (no cookie cache) so disabling a user or
  revoking a session takes effect immediately.
- ADMIN_BOOTSTRAP_EMAIL guarantees a way in on an empty database, idempotently
  and without crashing the API if the database is unreachable at boot.
- Guards prevent an admin from removing their own admin permission, disabling
  themselves, or stripping the last active admin.

The admin pool uses the callback-style mysql2, not mysql2/promise: Kysely's
MysqlDialect drives the pool with callbacks, and the promise wrapper ignores
them, so every query hangs silently. Only the integration tests caught this.

Schema in sql/admin/001_init.sql, derived from getAuthTables() on the
installed better-auth rather than the published CLI, which lags the library
and omits account.issuer.

app.ts is split into src/app.factory.ts so the integration tests drive the
real middleware order rather than a copy of it.

Tests: 131 unit, plus 41 integration tests against a throwaway MariaDB
started by test/integration/setup.ts (docker or podman).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-05 18:03:08 +02:00
Paddy 3ea9e630ed Migrate the API to native ESM and vitest; pin Node 26
Prep PR for the admin auth module (docs/plan-admin-auth.md step 1).
better-auth 1.7 ships ESM only, so the API moves off CommonJS:

- "type": "module", module nodenext, target ES2024, .js suffixes on all
  relative imports, require('mariadb'|'cors') replaced by imports, and
  export= packages (winston, app-root-path, bcrypt) consumed via default
  imports. The logger now uses appRoot.path explicitly.
- TypeScript 5.9, @types/node 26, tslint removed. Node 26 pinned via
  engines and .nvmrc (Plesk runs 26).
- Jest 28 + ts-jest replaced by vitest 5. Eight test files depend on
  hoisted module mocks with static imports and resetModules + require,
  which Jest's ESM mode does not support; vitest keeps them nearly
  verbatim. Coverage via @vitest/coverage-v8 (lcov), Sonar generic report
  via vitest-sonar-reporter, so sonar-project.properties is unchanged.
  vitest.config.ts sets FEEDBACK_IP_SALT so the suite passes without a
  local .env.
- dotenv 8 -> 16 and axios 0.24 -> 1.x: their old typings are not
  resolvable under nodenext.
- autoCommit: false dropped from the pool configs; it is not a mariadb
  connector option and was silently ignored.

tsc clean, 96/96 tests green, compiled app boots and serves /, /docs and
CORS under Node ESM.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-05 16:02:26 +02:00
Paddy 449edd6c68 Relay transactional email through Salesforce instead of SMTP (#9)
Jenkins Production Deployment
Reviewed-on: #9
Co-authored-by: Patrick Müller <patrick@mueller-patrick.tech>
Co-committed-by: Patrick Müller <patrick@mueller-patrick.tech>
2026-08-31 16:17:59 +00:00
Paddy b05f6b9da0 Add Feedback domain module: public submission flow, admin CRUD, reporting (#7)
Jenkins Production Deployment
Co-authored-by: Patrick Müller <mail@pmueller.me>
Reviewed-on: #7
Co-authored-by: Patrick Mueller <patrick@mueller-patrick.tech>
Co-committed-by: Patrick Mueller <patrick@mueller-patrick.tech>
2026-08-23 09:39:02 +00:00
Paddy 3996e37682 Initial commit 2022-12-24 14:32:13 +01:00