2 Commits

Author SHA1 Message Date
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
40 changed files with 436 additions and 1222 deletions
-6
View File
@@ -52,12 +52,6 @@ ADMIN_BOOTSTRAP_EMAIL=
# request shares ONE rate-limit bucket (/sign-in/* allows 3 per 10 seconds, so # request shares ONE rate-limit bucket (/sign-in/* allows 3 per 10 seconds, so
# one noisy client locks everyone out). Check with: # one noisy client locks everyone out). Check with:
# SELECT `key` FROM rateLimit; -- a "no-trusted-ip" row means it is happening. # SELECT `key` FROM rateLimit; -- a "no-trusted-ip" row means it is happening.
# The header the reverse proxy puts the real client IP in. Must be one the proxy
# actually overwrites - trusting a header it does not set lets any client send its
# own value and bypass the sign-in rate limit entirely.
# Set to "none" to trust no header at all: every request then shares one rate-limit
# bucket, which is the safe fallback if the check below fails. Verify after deploy
# with: SELECT ipAddress FROM session ORDER BY createdAt DESC LIMIT 3;
CLIENT_IP_HEADERS=x-real-ip CLIENT_IP_HEADERS=x-real-ip
TRUSTED_PROXY_IPS= TRUSTED_PROXY_IPS=
+3 -11
View File
@@ -46,17 +46,9 @@ other domain keeps the `mariadb` driver), mounted at `/admin/auth/*` for the aut
and `/admin` for the JSON routes. Sessions are httpOnly cookies scoped to and `/admin` for the JSON routes. Sessions are httpOnly cookies scoped to
`.nachklang.art`, so one sign-in covers every app. Accounts are **invite-only** — public `.nachklang.art`, so one sign-in covers every app. Accounts are **invite-only** — public
sign-up is disabled, and `invitations.plugin.ts` is the only code that creates users. sign-up is disabled, and `invitations.plugin.ts` is the only code that creates users.
A permission is **(app, role)** in `user_app_permissions`, keyed on Permissions are per app in `user_app_permissions`; `requireAppAccess(app)` in
`(user_id, app, role)` so one user can hold several roles per app. `access` is the only role `admin.middleware.ts` is the single authenticator, and it queries the database on every
today and means "may use this app at all"; `APP_ROLES` in `admin.schema.ts` is the contract, request (no cookie cache) so disabling a user takes effect at once. `ADMIN_BOOTSTRAP_EMAIL`
and a role not listed there is rejected rather than written. `requireAppAccess(app)` in
`admin.middleware.ts` is the single authenticator - it takes an optional second argument to
narrow to one role, and queries the database on every request (no cookie cache) so disabling
a user takes effect at once. Two things to know before touching this: any count of admins
must count **distinct users**, not permission rows, or a single admin with two roles reads as
two and the last-admin guard stops guarding; and both write endpoints accept
`{permissions: [{app, role}]}` as well as the older `{apps: ['tickets']}`, which means the
same at the `access` role. `ADMIN_BOOTSTRAP_EMAIL`
makes sure someone can always get in on a fresh database. makes sure someone can always get in on a fresh database.
*Legacy calendar* — unchanged: users need a `@nachklang.art` email, and after activation *Legacy calendar* — unchanged: users need a `@nachklang.art` email, and after activation
+1 -13
View File
@@ -11,19 +11,7 @@ These items were identified during a security review on 2026-05-02 and conscious
`sessionId` and `sessionKey` are currently read from query parameters, which means they appear in server access logs, browser history, proxy logs, and `Referer` headers. `sessionId` and `sessionKey` are currently read from query parameters, which means they appear in server access logs, browser history, proxy logs, and `Referer` headers.
**Fix (updated 2026-09-06):** Move the calendar onto the shared admin identity - **Fix:** Move to request headers (`X-Session-Id` / `X-Session-Key`) or the request body. Requires a corresponding frontend update.
`requireAppAccess('calendar')` against the better-auth session cookie, per
`docs/calendar-auth-migration.md`. That closes this item outright rather than moving the
credential to a safer place, and it is now the cheaper of the two: the feedback and tickets
modules made the same move on 2026-09-06 for one line each.
~~Move to request headers (`X-Session-Id` / `X-Session-Key`) or the request body.~~ No longer
the recommendation. Nothing on the server reads those two headers any more - the calendar's
query parameters are the last legacy credential path in the API - so this would build a second
mechanism just as the first is being retired. They survive only in the CORS `allowedHeaders`
list, and only until both frontends are redeployed.
Either fix requires a corresponding frontend update.
> Note: the shared calendar `password` parameter in query params is intentional (iCal clients don't support headers) and is acceptable for the current setup. > Note: the shared calendar `password` parameter in query params is intentional (iCal clients don't support headers) and is acceptable for the current setup.
+9 -15
View File
@@ -103,21 +103,15 @@ CREATE TABLE IF NOT EXISTS `rateLimit` (
-- --------------------------------------------------------------------------- -- ---------------------------------------------------------------------------
-- Which apps a user may administer. `admin` is just another app: holding it is -- Which apps a user may administer. `admin` is just another app: holding it is
-- what lets someone manage users and invitations. A permission is (app, role); -- what lets someone manage users and invitations. `role` is reserved for
-- `access` is the only role today, and the key admits several per app so finer -- per-app roles later and is 'admin' for every row today.
-- ones can be added by inserting rows rather than by migrating this table.
CREATE TABLE IF NOT EXISTS `user_app_permissions` ( CREATE TABLE IF NOT EXISTS `user_app_permissions` (
`user_id` VARCHAR(36) NOT NULL, `user_id` VARCHAR(36) NOT NULL,
`app` ENUM('calendar','feedback','tickets','admin') NOT NULL, `app` ENUM('calendar','feedback','tickets','admin') NOT NULL,
-- One row per (user, app, role). `access` means "may use this app at all" `role` VARCHAR(32) NOT NULL DEFAULT 'admin',
-- and is the only role today; the key allows several per app so a finer
-- permission can be added later by inserting rows, not by migrating.
`role` VARCHAR(32) NOT NULL DEFAULT 'access',
`granted_by` VARCHAR(36) DEFAULT NULL, `granted_by` VARCHAR(36) DEFAULT NULL,
`granted_at` DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, `granted_at` DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
-- (user_id, app) is the leftmost prefix of this key, so the per-request PRIMARY KEY (`user_id`, `app`),
-- permission lookup needs no separate index.
PRIMARY KEY (`user_id`, `app`, `role`),
CONSTRAINT `uap_user_fk` FOREIGN KEY (`user_id`) REFERENCES `user` (`id`) ON DELETE CASCADE CONSTRAINT `uap_user_fk` FOREIGN KEY (`user_id`) REFERENCES `user` (`id`) ON DELETE CASCADE
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
@@ -128,7 +122,7 @@ CREATE TABLE IF NOT EXISTS `invitations` (
`email` VARCHAR(255) NOT NULL, `email` VARCHAR(255) NOT NULL,
`name` VARCHAR(255) NOT NULL, `name` VARCHAR(255) NOT NULL,
`token_hash` CHAR(64) NOT NULL, `token_hash` CHAR(64) NOT NULL,
`permissions` JSON NOT NULL, `apps` JSON NOT NULL,
`invited_by` VARCHAR(36) DEFAULT NULL, `invited_by` VARCHAR(36) DEFAULT NULL,
`created_at` DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, `created_at` DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
`expires_at` DATETIME NOT NULL, `expires_at` DATETIME NOT NULL,
@@ -164,7 +158,7 @@ VALUES (
); );
INSERT INTO `user_app_permissions` (`user_id`, `app`, `role`) VALUES INSERT INTO `user_app_permissions` (`user_id`, `app`, `role`) VALUES
('dev-user-0000-0000-0000-000000000001', 'calendar', 'access'), ('dev-user-0000-0000-0000-000000000001', 'calendar', 'admin'),
('dev-user-0000-0000-0000-000000000001', 'feedback', 'access'), ('dev-user-0000-0000-0000-000000000001', 'feedback', 'admin'),
('dev-user-0000-0000-0000-000000000001', 'tickets', 'access'), ('dev-user-0000-0000-0000-000000000001', 'tickets', 'admin'),
('dev-user-0000-0000-0000-000000000001', 'admin', 'access'); ('dev-user-0000-0000-0000-000000000001', 'admin', 'admin');
+5 -11
View File
@@ -114,21 +114,15 @@ CREATE TABLE IF NOT EXISTS `rateLimit` (
-- --------------------------------------------------------------------------- -- ---------------------------------------------------------------------------
-- Which apps a user may administer. `admin` is just another app: holding it is -- Which apps a user may administer. `admin` is just another app: holding it is
-- what lets someone manage users and invitations. A permission is (app, role); -- what lets someone manage users and invitations. `role` is reserved for
-- `access` is the only role today, and the key admits several per app so finer -- per-app roles later and is 'admin' for every row today.
-- ones can be added by inserting rows rather than by migrating this table.
CREATE TABLE IF NOT EXISTS `user_app_permissions` ( CREATE TABLE IF NOT EXISTS `user_app_permissions` (
`user_id` VARCHAR(36) NOT NULL, `user_id` VARCHAR(36) NOT NULL,
`app` ENUM('calendar','feedback','tickets','admin') NOT NULL, `app` ENUM('calendar','feedback','tickets','admin') NOT NULL,
-- One row per (user, app, role). `access` means "may use this app at all" `role` VARCHAR(32) NOT NULL DEFAULT 'admin',
-- and is the only role today; the key allows several per app so a finer
-- permission can be added later by inserting rows, not by migrating.
`role` VARCHAR(32) NOT NULL DEFAULT 'access',
`granted_by` VARCHAR(36) DEFAULT NULL, `granted_by` VARCHAR(36) DEFAULT NULL,
`granted_at` DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, `granted_at` DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
-- (user_id, app) is the leftmost prefix of this key, so the per-request PRIMARY KEY (`user_id`, `app`),
-- permission lookup needs no separate index.
PRIMARY KEY (`user_id`, `app`, `role`),
CONSTRAINT `uap_user_fk` FOREIGN KEY (`user_id`) REFERENCES `user` (`id`) ON DELETE CASCADE CONSTRAINT `uap_user_fk` FOREIGN KEY (`user_id`) REFERENCES `user` (`id`) ON DELETE CASCADE
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
@@ -139,7 +133,7 @@ CREATE TABLE IF NOT EXISTS `invitations` (
`email` VARCHAR(255) NOT NULL, `email` VARCHAR(255) NOT NULL,
`name` VARCHAR(255) NOT NULL, `name` VARCHAR(255) NOT NULL,
`token_hash` CHAR(64) NOT NULL, `token_hash` CHAR(64) NOT NULL,
`permissions` JSON NOT NULL, `apps` JSON NOT NULL,
`invited_by` VARCHAR(36) DEFAULT NULL, `invited_by` VARCHAR(36) DEFAULT NULL,
`created_at` DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, `created_at` DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
`expires_at` DATETIME NOT NULL, `expires_at` DATETIME NOT NULL,
+5 -38
View File
@@ -4,7 +4,6 @@ import swaggerUi from 'swagger-ui-express';
import swaggerJSDoc from 'swagger-jsdoc'; import swaggerJSDoc from 'swagger-jsdoc';
import cors from 'cors'; import cors from 'cors';
import {toNodeHandler} from 'better-auth/node'; import {toNodeHandler} from 'better-auth/node';
import logger from './middleware/logger.js';
// Router imports // Router imports
import {calendarRouter} from './models/calendar/Calendar.router.js'; import {calendarRouter} from './models/calendar/Calendar.router.js';
@@ -12,7 +11,7 @@ import {feedbackRouter} from './models/feedback/Feedback.router.js';
import {ticketsRouter} from './models/tickets/Tickets.router.js'; import {ticketsRouter} from './models/tickets/Tickets.router.js';
import {adminRouter} from './models/admin/Admin.router.js'; import {adminRouter} from './models/admin/Admin.router.js';
import {auth} from './models/admin/admin.auth.js'; import {auth} from './models/admin/admin.auth.js';
import {ADMIN_ALLOWED_ORIGINS, isProd} from './models/admin/admin.config.js'; import {ADMIN_ALLOWED_ORIGINS} from './models/admin/admin.config.js';
dotenv.config(); dotenv.config();
@@ -48,28 +47,15 @@ export const createApp = (): express.Application => {
// staging host does not need a code change here. // staging host does not need a code change here.
...ADMIN_ALLOWED_ORIGINS ...ADMIN_ALLOWED_ORIGINS
]; ];
// `isProd` from admin.config, NOT `NODE_ENV !== 'production'`. The two are not const isDev = process.env.NODE_ENV !== 'production';
// the same when NODE_ENV is unset, which is exactly what a fresh Plesk vhost
// gives you: the old test called that "dev" and opened the loopback and
// private-LAN exceptions below. With `credentials: true` on this CORS config
// and a session cookie scoped to .nachklang.art, that let any page served
// from localhost read a signed-in admin's data cross-origin. admin.config
// treats anything but an explicit 'development'/'test' as production, so an
// unset value now fails closed.
const isDev = !isProd;
const localhostRegex = /^http:\/\/localhost:\d+$/; const localhostRegex = /^http:\/\/localhost:\d+$/;
// Matches http://<private-LAN-IPv4>:<port> - needed so the feedback form can // Matches http://<private-LAN-IPv4>:<port> - needed so the feedback form can
// be reached from a real phone over WiFi during dev (the phone's Origin is // be reached from a real phone over WiFi during dev (the phone's Origin is
// the dev machine's LAN IP, never "localhost"). Dev-only, same as above. // the dev machine's LAN IP, never "localhost"). Dev-only, same as above.
const lanIpRegex = /^http:\/\/(192\.168\.\d{1,3}\.\d{1,3}|10\.\d{1,3}\.\d{1,3}\.\d{1,3}|172\.(1[6-9]|2\d|3[01])\.\d{1,3}\.\d{1,3}):\d+$/; const lanIpRegex = /^http:\/\/(192\.168\.\d{1,3}\.\d{1,3}|10\.\d{1,3}\.\d{1,3}\.\d{1,3}|172\.(1[6-9]|2\d|3[01])\.\d{1,3}\.\d{1,3}):\d+$/;
app.use(cors({ app.use(cors({
// X-Session-* are no longer read by anything on this side: the step 4 // X-Session-* stay allowed until the calendar module is migrated off the
// cutover took the last two readers (feedback.auth.ts, tickets.auth.ts) // legacy header sessions (see docs/calendar-auth-migration.md).
// off them, and the calendar module passes its session in query
// parameters (DEFERRED_SECURITY.md item 1). They stay allowed only so a
// browser still running the pre-cutover tickets or feedback bundle gets
// a clean 401 rather than a CORS preflight failure. Drop them once both
// frontends are deployed - see docs/calendar-auth-migration.md step 5.
allowedHeaders: ['Content-Type', 'X-Session-Id', 'X-Session-Key'], allowedHeaders: ['Content-Type', 'X-Session-Id', 'X-Session-Key'],
// The admin session lives in a cookie, so browsers must be allowed to send // The admin session lives in a cookie, so browsers must be allowed to send
// it cross-origin - this is what makes credentials: 'include' work. // it cross-origin - this is what makes credentials: 'include' work.
@@ -97,26 +83,7 @@ export const createApp = (): express.Application => {
// better-auth's own handler, mounted before express.json(): it reads the raw // better-auth's own handler, mounted before express.json(): it reads the raw
// request body stream itself and a parsed body would leave it hanging. // request body stream itself and a parsed body would leave it hanging.
// app.all('/admin/auth/*', toNodeHandler(auth));
// Wrapped, because Express 4 does not await an async handler: a rejected
// promise escapes as an unhandled rejection instead of becoming a response.
// Nearly every better-auth route touches the admin database, so a database
// blip would leave the request hanging with no answer at all while the
// process logged an uncaughtException - observed by pointing ADMIN_DB at a
// database the user cannot open. Answer 503 instead: the caller learns, and
// the other domains keep serving.
const authHandler = toNodeHandler(auth);
app.all('/admin/auth/*', (req, res) => {
Promise.resolve(authHandler(req, res)).catch((e: any) => {
logger.error('Admin auth handler failed', {path: req.path, detail: e?.message});
if (!res.headersSent) {
res.status(503).send({
status: 'SERVICE_UNAVAILABLE',
message: 'Die Anmeldung ist derzeit nicht verfügbar. Bitte versuche es später erneut.'
});
}
});
});
// here we are adding middleware to parse all incoming requests as JSON // here we are adding middleware to parse all incoming requests as JSON
app.use(express.json()); app.use(express.json());
-5
View File
@@ -35,11 +35,6 @@ adminRouter.get('/me', requireSignedIn, (req: Request, res: Response) => {
id: res.locals.admin.id, id: res.locals.admin.id,
email: res.locals.admin.email, email: res.locals.admin.email,
fullName: res.locals.admin.displayName, fullName: res.locals.admin.displayName,
// `permissions` is the full (app, role) truth; `apps` is the distinct
// apps within it. Both are sent because the three frontends only ever ask
// "may I show this app?", and keeping `apps` means a finer permission can
// land here without a coordinated deploy of all of them.
permissions: res.locals.admin.permissions,
apps: res.locals.admin.apps apps: res.locals.admin.apps
}); });
}); });
+2 -19
View File
@@ -1,6 +1,6 @@
import {betterAuth} from 'better-auth'; import {betterAuth} from 'better-auth';
import {APIError} from 'better-auth/api'; import {APIError} from 'better-auth/api';
import {passkey, getAuthenticatorName} from '@better-auth/passkey'; import {passkey} from '@better-auth/passkey';
import {NachklangAdminDB} from './Admin.db.js'; import {NachklangAdminDB} from './Admin.db.js';
import {invitationsPlugin} from './invitations/invitations.plugin.js'; import {invitationsPlugin} from './invitations/invitations.plugin.js';
import {sendPasswordResetMail} from './admin.mail.js'; import {sendPasswordResetMail} from './admin.mail.js';
@@ -116,24 +116,7 @@ export const auth = betterAuth({
passkey({ passkey({
rpID: PASSKEY_RP_ID, rpID: PASSKEY_RP_ID,
rpName: 'Nachklang', rpName: 'Nachklang',
origin: ADMIN_ALLOWED_ORIGINS, origin: ADMIN_ALLOWED_ORIGINS
registration: {
// Without this, every passkey is stored with name = NULL and the
// account page can only label them all "Passkey" - useless at the
// one moment that list matters, when someone has to remove the
// passkey on the device they just lost.
//
// The AAGUID identifies the authenticator *model* (not a device
// and not a person), and better-auth ships the lookup table, so
// this yields "1Password", "iCloud Keychain", "Windows Hello".
// It only fills a blank: a name the client sent always wins, and
// an unknown AAGUID leaves the column NULL as before.
afterVerification: async ({verification}) => {
const name = getAuthenticatorName(verification.registrationInfo?.aaguid);
return name ? {name} : undefined;
}
}
}), }),
invitationsPlugin() invitationsPlugin()
], ],
+1 -2
View File
@@ -1,7 +1,6 @@
import * as UsersService from './users/users.admin.service.js'; import * as UsersService from './users/users.admin.service.js';
import * as InvitationsService from './invitations/invitations.service.js'; import * as InvitationsService from './invitations/invitations.service.js';
import {sendInvitationMail} from './admin.mail.js'; import {sendInvitationMail} from './admin.mail.js';
import {ACCESS_ROLE} from './admin.schema.js';
import {ADMIN_APP_URL, ADMIN_BOOTSTRAP_EMAIL, LOG_INVITE_LINKS} from './admin.config.js'; import {ADMIN_APP_URL, ADMIN_BOOTSTRAP_EMAIL, LOG_INVITE_LINKS} from './admin.config.js';
import logger from '../../middleware/logger.js'; import logger from '../../middleware/logger.js';
@@ -49,7 +48,7 @@ export const bootstrapAdmin = async (): Promise<void> => {
const invitation = await InvitationsService.createInvitation( const invitation = await InvitationsService.createInvitation(
email, email,
'Nachklang Admin', 'Nachklang Admin',
[{app: 'admin', role: ACCESS_ROLE}], ['admin'],
null null
); );
+5 -61
View File
@@ -73,27 +73,8 @@ const parseList = (value: string | undefined, fallback: string[]): string[] => {
return parsed.length > 0 ? parsed : fallback; return parsed.length > 0 ? parsed : fallback;
}; };
/** // The apps whose frontends may talk to /admin/* with credentials.
* The apps whose frontends may talk to /admin/* with credentials. export const APP_ORIGINS = parseList(process.env.APP_ORIGINS, []).map(origin => origin.replace(/\/$/, ''));
*
* They feed better-auth's `trustedOrigins`, which is what lets the tickets and
* feedback admin areas call /admin/auth/sign-out from their own origin. That
* became load-bearing with the step 4 cutover: before it, the only browser
* origin that ever reached /admin/auth was the admin app itself.
*
* Hence the production default rather than an empty list. An origin missing
* here fails in a way that is easy to misread - sign-in works, the app works,
* and only sign-out returns an origin error - so the two frontends we know
* about are named here and APP_ORIGINS overrides them for a staging host.
* Dev adds the localhost ports separately (see admin.auth.ts).
*/
const DEFAULT_APP_ORIGINS = [
'https://tickets.nachklang.art',
'https://feedback.nachklang.art'
];
export const APP_ORIGINS = parseList(process.env.APP_ORIGINS, DEFAULT_APP_ORIGINS)
.map(origin => origin.replace(/\/$/, ''));
// Kept in sync by construction rather than by three separate lists: the admin // Kept in sync by construction rather than by three separate lists: the admin
// app itself always counts, and dev adds the local ports. // app itself always counts, and dev adds the local ports.
@@ -120,53 +101,16 @@ export const ADMIN_ALLOWED_ORIGINS = Array.from(new Set([
* brute-force budget - so the default is the single header nginx sets, not a * brute-force budget - so the default is the single header nginx sets, not a
* permissive list. * permissive list.
*/ */
export const CLIENT_IP_HEADERS = parseList(process.env.CLIENT_IP_HEADERS, ['x-real-ip']);
/**
* `CLIENT_IP_HEADERS=none` trusts no header at all.
*
* This is the escape hatch for the one case where the wrong setting is worse
* than no setting: if the proxy turns out NOT to overwrite the header we are
* trusting, any client can send it and mint itself an unlimited brute-force
* budget against /sign-in. Falling back to the shared bucket is bad (one noisy
* client can lock the organisation out for ten seconds at a time) but it is
* bad in a way that fails closed, and it can be reverted from the environment
* without a deploy.
*
* Reach for it only after a check has actually failed - `SELECT ipAddress FROM
* session ORDER BY createdAt DESC` showing 127.0.0.1 or NULL for a real remote
* sign-in - and take it back out once the header is configured.
*
* An empty or unset value still means "use the default", not "trust nothing":
* a stray blank line in a .env must not silently change how requests are
* bucketed. Only the explicit word does that.
*/
const TRUST_NO_HEADER = 'none';
export const TRUST_NO_CLIENT_IP_HEADER =
(process.env.CLIENT_IP_HEADERS || '').trim().toLowerCase() === TRUST_NO_HEADER;
// An empty array is what better-auth reads as "no headers": it only falls back
// to its own default when the option is absent, and `[]` is truthy.
export const CLIENT_IP_HEADERS = TRUST_NO_CLIENT_IP_HEADER
? []
: parseList(process.env.CLIENT_IP_HEADERS, ['x-real-ip']);
export const TRUSTED_PROXY_IPS = parseList(process.env.TRUSTED_PROXY_IPS, []); export const TRUSTED_PROXY_IPS = parseList(process.env.TRUSTED_PROXY_IPS, []);
if (isProd && TRUST_NO_CLIENT_IP_HEADER) { if (isProd && TRUSTED_PROXY_IPS.length === 0) {
logger.warn(
'Admin module: CLIENT_IP_HEADERS=none - no client-IP header is trusted, so every ' +
'request shares one rate-limit bucket and /sign-in allows 3 attempts per 10 seconds ' +
'for everyone combined. This is the safe fallback, not a destination: configure the ' +
'header the proxy actually sets and remove it.'
);
} else if (isProd && TRUSTED_PROXY_IPS.length === 0) {
logger.warn( logger.warn(
'Admin module: TRUSTED_PROXY_IPS is not set. If the proxy sends a multi-value ' + 'Admin module: TRUSTED_PROXY_IPS is not set. If the proxy sends a multi-value ' +
`${CLIENT_IP_HEADERS.join('/')}, better-auth cannot resolve a client IP and every ` + `${CLIENT_IP_HEADERS.join('/')}, better-auth cannot resolve a client IP and every ` +
'request shares one rate-limit bucket. Verify with: SELECT `key` FROM rateLimit - ' + 'request shares one rate-limit bucket. Verify with: SELECT `key` FROM rateLimit - ' +
'a "no-trusted-ip" row means this is happening. A single-value header needs no ' + 'a "no-trusted-ip" row means this is happening.'
'trusted proxies, so this warning is expected on a plain single-proxy setup.'
); );
} }
+3 -19
View File
@@ -2,7 +2,7 @@ import express from 'express';
import {fromNodeHeaders} from 'better-auth/node'; import {fromNodeHeaders} from 'better-auth/node';
import {auth} from './admin.auth.js'; import {auth} from './admin.auth.js';
import * as UsersService from './users/users.admin.service.js'; import * as UsersService from './users/users.admin.service.js';
import {AppName, AppPermission, AppRole} from './admin.schema.js'; import {AppName} from './admin.schema.js';
import {sendServerError} from './admin.errors.js'; import {sendServerError} from './admin.errors.js';
/** /**
@@ -28,9 +28,6 @@ export interface AdminIdentity {
export interface AdminAccess extends AdminIdentity { export interface AdminAccess extends AdminIdentity {
disabled: boolean; disabled: boolean;
/** Every (app, role) grant. */
permissions: AppPermission[];
/** The distinct apps those grants cover. */
apps: AppName[]; apps: AppName[];
} }
@@ -62,7 +59,6 @@ export const resolveAccess = async (req: express.Request): Promise<AdminAccess |
email: access.email, email: access.email,
displayName: access.displayName, displayName: access.displayName,
disabled: access.disabled, disabled: access.disabled,
permissions: access.permissions,
apps: access.apps apps: access.apps
}; };
}; };
@@ -101,12 +97,8 @@ export const requireSignedIn: express.RequestHandler = async (req, res, next) =>
* what feedback.auth.ts's requireAdminAuth used to be, except that it now * what feedback.auth.ts's requireAdminAuth used to be, except that it now
* answers 403 for a signed-in user without that app's permission instead of * answers 403 for a signed-in user without that app's permission instead of
* letting any activated @nachklang.art account in. * letting any activated @nachklang.art account in.
*
* The optional second argument narrows it to one role within the app. Nothing
* passes it today - every app has exactly the `access` role - but it is the
* seam a finer permission arrives through.
*/ */
export const requireAppAccess = (app: AppName, role?: AppRole): express.RequestHandler => { export const requireAppAccess = (app: AppName): express.RequestHandler => {
return async (req, res, next) => { return async (req, res, next) => {
try { try {
const access = await resolveAccess(req); const access = await resolveAccess(req);
@@ -118,15 +110,7 @@ export const requireAppAccess = (app: AppName, role?: AppRole): express.RequestH
forbidden(res, 'Dieses Konto ist deaktiviert.'); forbidden(res, 'Dieses Konto ist deaktiviert.');
return; return;
} }
if (!access.apps.includes(app)) {
// Without a role this asks "may they open this app at all?", which is
// any grant on it. With one it asks for that specific grant - the hook
// a finer permission plugs into, without touching existing call sites.
const allowed = role === undefined
? access.apps.includes(app)
: access.permissions.some(permission => permission.app === app && permission.role === role);
if (!allowed) {
forbidden(res, 'Für diesen Bereich fehlt dir die Berechtigung.'); forbidden(res, 'Für diesen Bereich fehlt dir die Berechtigung.');
return; return;
} }
+3 -85
View File
@@ -19,87 +19,6 @@ export const isAppName = (value: unknown): value is AppName => {
return typeof value === 'string' && (APP_NAMES as string[]).includes(value); return typeof value === 'string' && (APP_NAMES as string[]).includes(value);
}; };
/**
* A permission is (app, role), not just an app. Today every app has exactly one
* role - `access`, "may use this app at all" - so the model looks like a plain
* list of apps and the UI renders one checkbox each. It is written this way
* anyway because the alternative gets expensive fast: `user_app_permissions`
* has primary key (user_id, app, role), so a user can hold several roles for
* the same app, and adding one later is a string in APP_ROLES plus rows - never
* a schema migration and never a change to the shape on the wire.
*
* Note the role is deliberately NOT called `admin`, which is what the column
* defaulted to before: on a `tickets` row that reads as "tickets administrator"
* when it only ever meant "has access", and once real roles exist there would
* be no way to tell the two apart.
*/
export const ACCESS_ROLE = 'access';
export type AppRole = string;
/** Every role that exists, per app, in display order. Extend to add one. */
export const APP_ROLES: Record<AppName, readonly AppRole[]> = {
calendar: [ACCESS_ROLE],
feedback: [ACCESS_ROLE],
tickets: [ACCESS_ROLE],
admin: [ACCESS_ROLE]
};
export interface AppPermission {
app: AppName;
role: AppRole;
}
export const isAppRole = (app: AppName, role: unknown): role is AppRole => {
return typeof role === 'string' && APP_ROLES[app].includes(role);
};
export const isAppPermission = (value: unknown): value is AppPermission => {
if (typeof value !== 'object' || value === null) {
return false;
}
const candidate = value as {app?: unknown; role?: unknown};
return isAppName(candidate.app) && isAppRole(candidate.app, candidate.role);
};
/**
* Normalises whatever a caller sent into a valid, duplicate-free permission
* list. Accepts the richer `{app, role}` form and the plain `AppName` form,
* because `{apps: ['tickets']}` is still what the older callers send and it
* means exactly "tickets at the access role".
*/
export const toPermissions = (value: unknown): AppPermission[] | null => {
if (!Array.isArray(value)) {
return null;
}
const permissions: AppPermission[] = [];
for (const entry of value) {
if (isAppName(entry)) {
permissions.push({app: entry, role: ACCESS_ROLE});
} else if (isAppPermission(entry)) {
permissions.push({app: entry.app, role: entry.role});
} else {
return null;
}
}
const seen = new Set<string>();
return permissions.filter(permission => {
const key = `${permission.app}:${permission.role}`;
if (seen.has(key)) {
return false;
}
seen.add(key);
return true;
});
};
/** The distinct apps a permission list grants any access to. */
export const appsOf = (permissions: AppPermission[]): AppName[] => {
return APP_NAMES.filter(app => permissions.some(permission => permission.app === app));
};
export interface UserTable { export interface UserTable {
id: string; id: string;
name: string; name: string;
@@ -133,7 +52,7 @@ export interface PasskeyTable {
export interface UserAppPermissionTable { export interface UserAppPermissionTable {
user_id: string; user_id: string;
app: AppName; app: AppName;
role: AppRole; role: string;
granted_by: string | null; granted_by: string | null;
granted_at: Generated<Date>; granted_at: Generated<Date>;
} }
@@ -144,9 +63,8 @@ export interface InvitationTable {
email: string; email: string;
name: string; name: string;
token_hash: string; token_hash: string;
// JSON column holding an AppPermission[]. Older rows may hold a plain // JSON column holding an AppName[].
// AppName[]; `parsePermissions` reads both. apps: string;
permissions: string;
invited_by: string | null; invited_by: string | null;
created_at: Generated<Date>; created_at: Generated<Date>;
expires_at: Date; expires_at: Date;
@@ -147,7 +147,7 @@ export const invitationsPlugin = () => {
return created; return created;
}); });
await UsersService.setPermissions(user.id, invitation.permissions, null); await UsersService.setPermissions(user.id, invitation.apps, null);
const session = await ctx.context.internalAdapter.createSession(user.id); const session = await ctx.context.internalAdapter.createSession(user.id);
await setSessionCookie(ctx, {session, user}); await setSessionCookie(ctx, {session, user});
@@ -1,7 +1,7 @@
import express, {Request, Response} from 'express'; import express, {Request, Response} from 'express';
import * as InvitationsService from './invitations.service.js'; import * as InvitationsService from './invitations.service.js';
import * as UsersService from '../users/users.admin.service.js'; import * as UsersService from '../users/users.admin.service.js';
import {toPermissions} from '../admin.schema.js'; import {isAppName, AppName} from '../admin.schema.js';
import {sendInvitationMail} from '../admin.mail.js'; import {sendInvitationMail} from '../admin.mail.js';
import {ADMIN_APP_URL, LOG_INVITE_LINKS} from '../admin.config.js'; import {ADMIN_APP_URL, LOG_INVITE_LINKS} from '../admin.config.js';
import {sendServerError} from '../admin.errors.js'; import {sendServerError} from '../admin.errors.js';
@@ -64,24 +64,16 @@ invitationsRouter.get('/', async (req: Request, res: Response) => {
* application/json: * application/json:
* schema: * schema:
* type: object * type: object
* required: [email, name, permissions] * required: [email, name, apps]
* properties: * properties:
* email: * email:
* type: string * type: string
* name: * name:
* type: string * type: string
* permissions: * apps:
* type: array * type: array
* description: >
* One entry per (app, role). A plain array of app names is
* accepted too and means the same at the `access` role.
* items: * items:
* type: object * type: string
* properties:
* app:
* type: string
* role:
* type: string
* responses: * responses:
* 201: * 201:
* description: Invitation created and mailed * description: Invitation created and mailed
@@ -94,15 +86,10 @@ invitationsRouter.post('/', async (req: Request, res: Response) => {
try { try {
const email = String(req.body?.email || '').trim().toLowerCase(); const email = String(req.body?.email || '').trim().toLowerCase();
const name = String(req.body?.name || '').trim(); const name = String(req.body?.name || '').trim();
const apps: unknown = req.body?.apps;
// Same two accepted shapes as PUT /admin/users/:id/permissions. if (!EMAIL_PATTERN.test(email) || name.length === 0 || !Array.isArray(apps) || !apps.every(isAppName)) {
const permissions = toPermissions(req.body?.permissions ?? req.body?.apps); res.status(400).send({status: 'BAD_REQUEST', message: 'E-Mail, Name und App-Liste sind erforderlich.'});
if (!EMAIL_PATTERN.test(email) || name.length === 0 || !permissions) {
res.status(400).send({
status: 'BAD_REQUEST',
message: 'E-Mail, Name und Berechtigungen sind erforderlich.'
});
return; return;
} }
@@ -120,7 +107,7 @@ invitationsRouter.post('/', async (req: Request, res: Response) => {
const invitation = await InvitationsService.createInvitation( const invitation = await InvitationsService.createInvitation(
email, email,
name, name,
permissions, apps as AppName[],
res.locals.admin.id res.locals.admin.id
); );
@@ -1,6 +1,6 @@
import * as crypto from 'crypto'; import * as crypto from 'crypto';
import {NachklangAdminDB} from '../Admin.db.js'; import {NachklangAdminDB} from '../Admin.db.js';
import {AppPermission, isAppName, isAppPermission, ACCESS_ROLE} from '../admin.schema.js'; import {AppName, APP_NAMES, isAppName} from '../admin.schema.js';
const db = NachklangAdminDB.db; const db = NachklangAdminDB.db;
@@ -19,7 +19,7 @@ export interface OpenInvitation {
id: number; id: number;
email: string; email: string;
name: string; name: string;
permissions: AppPermission[]; apps: AppName[];
invitedBy: string | null; invitedBy: string | null;
createdAt: Date; createdAt: Date;
expiresAt: Date; expiresAt: Date;
@@ -29,7 +29,7 @@ export interface AcceptableInvitation {
id: number; id: number;
email: string; email: string;
name: string; name: string;
permissions: AppPermission[]; apps: AppName[];
} }
const hashToken = (token: string): string => { const hashToken = (token: string): string => {
@@ -41,28 +41,11 @@ const generateToken = (): string => {
return crypto.randomBytes(32).toString('base64url'); return crypto.randomBytes(32).toString('base64url');
}; };
/** const parseApps = (value: unknown): AppName[] => {
* Reads the stored permission list. Two shapes are accepted: the current // mysql2 hands back a JSON column already parsed; a driver or column-type
* `[{app, role}]`, and a bare `['tickets', ...]` from before roles existed, // change that turns it into a string must not break the read path.
* which means the same thing at the `access` role. Invitations live for seven
* days, so a deploy that changes the shape has in-flight rows in the old one -
* tolerating both is what stops those invitees from being stranded.
*
* mysql2 hands back a JSON column already parsed; a driver or column-type
* change that turns it into a string must not break the read path either.
*/
const parsePermissions = (value: unknown): AppPermission[] => {
const raw = typeof value === 'string' ? JSON.parse(value) : value; const raw = typeof value === 'string' ? JSON.parse(value) : value;
if (!Array.isArray(raw)) { return Array.isArray(raw) ? raw.filter(isAppName) : [];
return [];
}
return raw.flatMap((entry): AppPermission[] => {
if (isAppName(entry)) {
return [{app: entry, role: ACCESS_ROLE}];
}
return isAppPermission(entry) ? [{app: entry.app, role: entry.role}] : [];
});
}; };
const expiryFromNow = (): Date => { const expiryFromNow = (): Date => {
@@ -78,12 +61,12 @@ const expiryFromNow = (): Date => {
export const createInvitation = async ( export const createInvitation = async (
email: string, email: string,
name: string, name: string,
permissions: AppPermission[], apps: AppName[],
invitedBy: string | null invitedBy: string | null
): Promise<{id: number; token: string; expiresAt: Date}> => { ): Promise<{id: number; token: string; expiresAt: Date}> => {
const token = generateToken(); const token = generateToken();
const expiresAt = expiryFromNow(); const expiresAt = expiryFromNow();
const valid = permissions.filter(isAppPermission); const validApps = Array.from(new Set(apps)).filter(app => APP_NAMES.includes(app));
const id = await db.transaction().execute(async trx => { const id = await db.transaction().execute(async trx => {
await trx await trx
@@ -100,7 +83,7 @@ export const createInvitation = async (
email, email,
name, name,
token_hash: hashToken(token), token_hash: hashToken(token),
permissions: JSON.stringify(valid), apps: JSON.stringify(validApps),
invited_by: invitedBy, invited_by: invitedBy,
created_at: new Date(), created_at: new Date(),
expires_at: expiresAt expires_at: expiresAt
@@ -122,7 +105,7 @@ export const createInvitation = async (
export const findByToken = async (token: string): Promise<AcceptableInvitation | null> => { export const findByToken = async (token: string): Promise<AcceptableInvitation | null> => {
const row = await db const row = await db
.selectFrom('invitations') .selectFrom('invitations')
.select(['id', 'email', 'name', 'permissions']) .select(['id', 'email', 'name', 'apps'])
.where('token_hash', '=', hashToken(token)) .where('token_hash', '=', hashToken(token))
.where('accepted_at', 'is', null) .where('accepted_at', 'is', null)
.where('revoked_at', 'is', null) .where('revoked_at', 'is', null)
@@ -133,7 +116,7 @@ export const findByToken = async (token: string): Promise<AcceptableInvitation |
return null; return null;
} }
return {id: row.id, email: row.email, name: row.name, permissions: parsePermissions(row.permissions)}; return {id: row.id, email: row.email, name: row.name, apps: parseApps(row.apps)};
}; };
/** Marks the invitation accepted. Conditional on it still being open so two /** Marks the invitation accepted. Conditional on it still being open so two
@@ -166,7 +149,7 @@ export const unmarkAccepted = async (invitationId: number): Promise<void> => {
export const listOpenInvitations = async (): Promise<OpenInvitation[]> => { export const listOpenInvitations = async (): Promise<OpenInvitation[]> => {
const rows = await db const rows = await db
.selectFrom('invitations') .selectFrom('invitations')
.select(['id', 'email', 'name', 'permissions', 'invited_by', 'created_at', 'expires_at']) .select(['id', 'email', 'name', 'apps', 'invited_by', 'created_at', 'expires_at'])
.where('accepted_at', 'is', null) .where('accepted_at', 'is', null)
.where('revoked_at', 'is', null) .where('revoked_at', 'is', null)
.where('expires_at', '>', new Date()) .where('expires_at', '>', new Date())
@@ -177,7 +160,7 @@ export const listOpenInvitations = async (): Promise<OpenInvitation[]> => {
id: row.id, id: row.id,
email: row.email, email: row.email,
name: row.name, name: row.name,
permissions: parsePermissions(row.permissions), apps: parseApps(row.apps),
invitedBy: row.invited_by, invitedBy: row.invited_by,
createdAt: row.created_at, createdAt: row.created_at,
expiresAt: row.expires_at expiresAt: row.expires_at
+10 -25
View File
@@ -1,6 +1,6 @@
import express, {Request, Response} from 'express'; import express, {Request, Response} from 'express';
import * as UsersService from './users.admin.service.js'; import * as UsersService from './users.admin.service.js';
import {toPermissions} from '../admin.schema.js'; import {AppName, isAppName} from '../admin.schema.js';
import {sendServerError} from '../admin.errors.js'; import {sendServerError} from '../admin.errors.js';
export const usersAdminRouter = express.Router(); export const usersAdminRouter = express.Router();
@@ -90,40 +90,26 @@ usersAdminRouter.get('/:userId', async (req: Request, res: Response) => {
* schema: * schema:
* type: object * type: object
* properties: * properties:
* permissions: * apps:
* type: array * type: array
* description: >
* One entry per (app, role). `access` is the only role today.
* A plain array of app names is also accepted and means the
* same at the `access` role.
* items: * items:
* type: object * type: string
* properties: * enum: [calendar, feedback, tickets, admin]
* app:
* type: string
* enum: [calendar, feedback, tickets, admin]
* role:
* type: string
* enum: [access]
* responses: * responses:
* 200: * 200:
* description: Success * description: Success
* 400: * 400:
* description: Invalid app or role * description: Invalid app name
* 409: * 409:
* description: Would lock the last admin out * description: Would lock the last admin out
*/ */
usersAdminRouter.put('/:userId/permissions', async (req: Request, res: Response) => { usersAdminRouter.put('/:userId/permissions', async (req: Request, res: Response) => {
try { try {
const userId = req.params.userId; const userId = req.params.userId;
const apps: unknown = req.body?.apps;
// `permissions: [{app, role}]` is the real shape; `apps: ['tickets']` is if (!Array.isArray(apps) || !apps.every(isAppName)) {
// accepted as shorthand for the same thing at the `access` role, so a res.status(400).send({status: 'BAD_REQUEST', message: 'Ungültige App-Liste.'});
// caller that predates roles keeps working.
const permissions = toPermissions(req.body?.permissions ?? req.body?.apps);
if (!permissions) {
res.status(400).send({status: 'BAD_REQUEST', message: 'Ungültige Berechtigungsliste.'});
return; return;
} }
@@ -133,8 +119,7 @@ usersAdminRouter.put('/:userId/permissions', async (req: Request, res: Response)
} }
const target = await UsersService.loadAccess(userId); const target = await UsersService.loadAccess(userId);
const keepsAdmin = permissions.some(permission => permission.app === 'admin'); const losesAdmin = Boolean(target?.apps.includes('admin')) && !(apps as AppName[]).includes('admin');
const losesAdmin = Boolean(target?.apps.includes('admin')) && !keepsAdmin;
// Self-lockout is checked here because it needs the caller's identity, // Self-lockout is checked here because it needs the caller's identity,
// which the service has no business knowing. The last-admin check is // which the service has no business knowing. The last-admin check is
@@ -145,7 +130,7 @@ usersAdminRouter.put('/:userId/permissions', async (req: Request, res: Response)
return; return;
} }
const result = await UsersService.setPermissionsGuarded(userId, permissions, res.locals.admin.id); const result = await UsersService.setPermissionsGuarded(userId, apps as AppName[], res.locals.admin.id);
if (result === 'last-admin') { if (result === 'last-admin') {
conflict(res, 'Die letzte Admin-Berechtigung kann nicht entzogen werden.'); conflict(res, 'Die letzte Admin-Berechtigung kann nicht entzogen werden.');
return; return;
+44 -120
View File
@@ -1,15 +1,6 @@
import {Transaction} from 'kysely'; import {Transaction} from 'kysely';
import {NachklangAdminDB} from '../Admin.db.js'; import {NachklangAdminDB} from '../Admin.db.js';
import { import {AdminDatabase, AppName, APP_NAMES} from '../admin.schema.js';
AdminDatabase,
AppName,
AppPermission,
AppRole,
ACCESS_ROLE,
appsOf,
isAppName,
isAppRole
} from '../admin.schema.js';
const db = NachklangAdminDB.db; const db = NachklangAdminDB.db;
@@ -27,10 +18,6 @@ export interface UserAccess {
email: string; email: string;
displayName: string; displayName: string;
disabled: boolean; disabled: boolean;
/** Every (app, role) grant this user holds. */
permissions: AppPermission[];
/** The distinct apps the above grants any access to. Derived, kept because
* most callers only ever ask "may they open this app at all?". */
apps: AppName[]; apps: AppName[];
} }
@@ -40,7 +27,6 @@ export interface UserListEntry {
id: string; id: string;
email: string; email: string;
name: string; name: string;
permissions: AppPermission[];
apps: AppName[]; apps: AppName[];
status: UserStatus; status: UserStatus;
createdAt: Date; createdAt: Date;
@@ -75,8 +61,7 @@ export const loadAccess = async (userId: string): Promise<UserAccess | null> =>
'user.email as email', 'user.email as email',
'user.name as name', 'user.name as name',
'user.disabled as disabled', 'user.disabled as disabled',
'user_app_permissions.app as app', 'user_app_permissions.app as app'
'user_app_permissions.role as role'
]) ])
.execute(); .execute();
@@ -84,32 +69,16 @@ export const loadAccess = async (userId: string): Promise<UserAccess | null> =>
return null; return null;
} }
const permissions = toPermissionRows(rows);
return { return {
id: rows[0].id, id: rows[0].id,
email: rows[0].email, email: rows[0].email,
displayName: rows[0].name, displayName: rows[0].name,
// MySQL TINYINT(1) comes back as 0/1 through mysql2. // MySQL TINYINT(1) comes back as 0/1 through mysql2.
disabled: Boolean(rows[0].disabled), disabled: Boolean(rows[0].disabled),
permissions, apps: rows.map(row => row.app).filter((app): app is AppName => app !== null)
apps: appsOf(permissions)
}; };
}; };
/**
* Turns joined permission rows into AppPermission[]. The left join produces one
* row with a null app for a user who holds nothing, and a role written directly
* into the database that no longer appears in APP_ROLES is dropped rather than
* trusted - the table is the store, APP_ROLES is the contract.
*/
const toPermissionRows = (rows: {app: AppName | null; role: string | null}[]): AppPermission[] => {
return rows
.filter((row): row is {app: AppName; role: string} =>
isAppName(row.app) && isAppRole(row.app, row.role))
.map(row => ({app: row.app, role: row.role}));
};
export const listUsers = async (): Promise<UserListEntry[]> => { export const listUsers = async (): Promise<UserListEntry[]> => {
const users = await db const users = await db
.selectFrom('user') .selectFrom('user')
@@ -119,7 +88,7 @@ export const listUsers = async (): Promise<UserListEntry[]> => {
const permissions = await db const permissions = await db
.selectFrom('user_app_permissions') .selectFrom('user_app_permissions')
.select(['user_id', 'app', 'role']) .select(['user_id', 'app'])
.execute(); .execute();
// Last sign-in is derived from the newest session rather than stored: a // Last sign-in is derived from the newest session rather than stored: a
@@ -138,33 +107,26 @@ export const listUsers = async (): Promise<UserListEntry[]> => {
.groupBy('userId') .groupBy('userId')
.execute(); .execute();
const permissionsByUser = new Map<string, AppPermission[]>(); const appsByUser = new Map<string, AppName[]>();
for (const row of permissions) { for (const row of permissions) {
if (!isAppRole(row.app, row.role)) { const apps = appsByUser.get(row.user_id) || [];
continue; apps.push(row.app);
} appsByUser.set(row.user_id, apps);
const held = permissionsByUser.get(row.user_id) || [];
held.push({app: row.app, role: row.role});
permissionsByUser.set(row.user_id, held);
} }
const lastSignInByUser = new Map<string, Date | null>( const lastSignInByUser = new Map<string, Date | null>(
lastSessions.map(row => [row.userId, row.lastSignInAt as Date | null]) lastSessions.map(row => [row.userId, row.lastSignInAt as Date | null])
); );
return users.map(user => { return users.map(user => ({
const held = permissionsByUser.get(user.id) || []; id: user.id,
return { email: user.email,
id: user.id, name: user.name,
email: user.email, apps: appsByUser.get(user.id) || [],
name: user.name, status: user.disabled ? 'deaktiviert' : 'aktiv',
permissions: held, createdAt: user.createdAt,
apps: appsOf(held), lastSignInAt: lastSignInByUser.get(user.id) ?? null
status: user.disabled ? ('deaktiviert' as const) : ('aktiv' as const), }));
createdAt: user.createdAt,
lastSignInAt: lastSignInByUser.get(user.id) ?? null
};
});
}; };
export const getUserDetail = async (userId: string): Promise<UserDetail | null> => { export const getUserDetail = async (userId: string): Promise<UserDetail | null> => {
@@ -179,11 +141,7 @@ export const getUserDetail = async (userId: string): Promise<UserDetail | null>
} }
const [permissions, sessions, passkeys] = await Promise.all([ const [permissions, sessions, passkeys] = await Promise.all([
db db.selectFrom('user_app_permissions').select('app').where('user_id', '=', userId).execute(),
.selectFrom('user_app_permissions')
.select(['app', 'role'])
.where('user_id', '=', userId)
.execute(),
db db
.selectFrom('session') .selectFrom('session')
.select(['id', 'createdAt', 'expiresAt', 'ipAddress', 'userAgent']) .select(['id', 'createdAt', 'expiresAt', 'ipAddress', 'userAgent'])
@@ -198,14 +156,11 @@ export const getUserDetail = async (userId: string): Promise<UserDetail | null>
.executeTakeFirst() .executeTakeFirst()
]); ]);
const held = toPermissionRows(permissions);
return { return {
id: user.id, id: user.id,
email: user.email, email: user.email,
name: user.name, name: user.name,
permissions: held, apps: permissions.map(row => row.app),
apps: appsOf(held),
status: user.disabled ? 'deaktiviert' : 'aktiv', status: user.disabled ? 'deaktiviert' : 'aktiv',
createdAt: user.createdAt, createdAt: user.createdAt,
lastSignInAt: sessions.length > 0 ? sessions[0].createdAt : null, lastSignInAt: sessions.length > 0 ? sessions[0].createdAt : null,
@@ -219,51 +174,25 @@ export const getUserDetail = async (userId: string): Promise<UserDetail | null>
* transaction rather than a diff: the set is at most four rows, and a diff * transaction rather than a diff: the set is at most four rows, and a diff
* would only add branches for no measurable gain. * would only add branches for no measurable gain.
*/ */
/** The rows a permission list becomes. One row per (app, role). */
const permissionRows = (
userId: string,
permissions: AppPermission[],
grantedBy: string | null
) => {
return permissions.map(permission => ({
user_id: userId,
app: permission.app,
role: permission.role,
granted_by: grantedBy,
granted_at: new Date()
}));
};
/** Drops anything not in APP_ROLES and de-duplicates on (app, role). */
const validPermissions = (permissions: AppPermission[]): AppPermission[] => {
const seen = new Set<string>();
return permissions.filter(permission => {
if (!isAppName(permission.app) || !isAppRole(permission.app, permission.role)) {
return false;
}
const key = `${permission.app}:${permission.role}`;
if (seen.has(key)) {
return false;
}
seen.add(key);
return true;
});
};
export const setPermissions = async ( export const setPermissions = async (
userId: string, userId: string,
permissions: AppPermission[], apps: AppName[],
grantedBy: string | null grantedBy: string | null
): Promise<void> => { ): Promise<void> => {
const valid = validPermissions(permissions); const unique = Array.from(new Set(apps)).filter(app => APP_NAMES.includes(app));
await db.transaction().execute(async trx => { await db.transaction().execute(async trx => {
await trx.deleteFrom('user_app_permissions').where('user_id', '=', userId).execute(); await trx.deleteFrom('user_app_permissions').where('user_id', '=', userId).execute();
if (valid.length > 0) { if (unique.length > 0) {
await trx await trx
.insertInto('user_app_permissions') .insertInto('user_app_permissions')
.values(permissionRows(userId, valid, grantedBy)) .values(unique.map(app => ({
user_id: userId,
app,
role: 'admin',
granted_by: grantedBy,
granted_at: new Date()
})))
.execute(); .execute();
} }
}); });
@@ -288,11 +217,7 @@ const countActiveAdminsForUpdate = async (trx: Transaction<AdminDatabase>): Prom
.innerJoin('user', 'user.id', 'user_app_permissions.user_id') .innerJoin('user', 'user.id', 'user_app_permissions.user_id')
.where('user_app_permissions.app', '=', 'admin') .where('user_app_permissions.app', '=', 'admin')
.where('user.disabled', '=', false) .where('user.disabled', '=', false)
// countDistinct, not countAll: with (user_id, app, role) as the key one .select(({fn}) => fn.countAll<number>().as('count'))
// user can hold several roles on `admin`, and counting rows would make a
// single admin with two roles look like two admins - defeating the guard
// at exactly the moment it matters.
.select(({fn}) => fn.count<number>('user_app_permissions.user_id').distinct().as('count'))
.forUpdate() .forUpdate()
.executeTakeFirst(); .executeTakeFirst();
@@ -305,11 +230,10 @@ const countActiveAdminsForUpdate = async (trx: Transaction<AdminDatabase>): Prom
*/ */
export const setPermissionsGuarded = async ( export const setPermissionsGuarded = async (
userId: string, userId: string,
permissions: AppPermission[], apps: AppName[],
grantedBy: string | null grantedBy: string | null
): Promise<LastAdminGuardResult> => { ): Promise<LastAdminGuardResult> => {
const valid = validPermissions(permissions); const unique = Array.from(new Set(apps)).filter(app => APP_NAMES.includes(app));
const keepsAdmin = valid.some(permission => permission.app === 'admin');
return db.transaction().execute(async trx => { return db.transaction().execute(async trx => {
const target = await trx const target = await trx
@@ -318,20 +242,25 @@ export const setPermissionsGuarded = async (
.where('user_app_permissions.user_id', '=', userId) .where('user_app_permissions.user_id', '=', userId)
.where('user_app_permissions.app', '=', 'admin') .where('user_app_permissions.app', '=', 'admin')
.select(['user.disabled as disabled']) .select(['user.disabled as disabled'])
.limit(1)
.forUpdate() .forUpdate()
.executeTakeFirst(); .executeTakeFirst();
const losesAdmin = Boolean(target) && !keepsAdmin; const losesAdmin = Boolean(target) && !unique.includes('admin');
if (losesAdmin && !target?.disabled && (await countActiveAdminsForUpdate(trx)) <= 1) { if (losesAdmin && !target?.disabled && (await countActiveAdminsForUpdate(trx)) <= 1) {
return 'last-admin'; return 'last-admin';
} }
await trx.deleteFrom('user_app_permissions').where('user_id', '=', userId).execute(); await trx.deleteFrom('user_app_permissions').where('user_id', '=', userId).execute();
if (valid.length > 0) { if (unique.length > 0) {
await trx await trx
.insertInto('user_app_permissions') .insertInto('user_app_permissions')
.values(permissionRows(userId, valid, grantedBy)) .values(unique.map(app => ({
user_id: userId,
app,
role: 'admin',
granted_by: grantedBy,
granted_at: new Date()
})))
.execute(); .execute();
} }
@@ -352,7 +281,6 @@ export const disableUserGuarded = async (userId: string): Promise<LastAdminGuard
.where('user_app_permissions.app', '=', 'admin') .where('user_app_permissions.app', '=', 'admin')
.where('user.disabled', '=', false) .where('user.disabled', '=', false)
.select('user_app_permissions.user_id') .select('user_app_permissions.user_id')
.limit(1)
.forUpdate() .forUpdate()
.executeTakeFirst(); .executeTakeFirst();
@@ -370,16 +298,12 @@ export const disableUserGuarded = async (userId: string): Promise<LastAdminGuard
export const grantPermission = async ( export const grantPermission = async (
userId: string, userId: string,
app: AppName, app: AppName,
grantedBy: string | null, grantedBy: string | null
role: AppRole = ACCESS_ROLE
): Promise<void> => { ): Promise<void> => {
await db await db
.insertInto('user_app_permissions') .insertInto('user_app_permissions')
.values({user_id: userId, app, role, granted_by: grantedBy, granted_at: new Date()}) .values({user_id: userId, app, role: 'admin', granted_by: grantedBy, granted_at: new Date()})
// The row already existing is the success case - this is "make sure they .onDuplicateKeyUpdate({role: 'admin'})
// hold it", not "re-grant it" - so nothing is overwritten and granted_by
// keeps naming whoever granted it first.
.onDuplicateKeyUpdate({role})
.execute(); .execute();
}; };
@@ -1,6 +1,19 @@
/** /**
* @swagger * @swagger
* components: * components:
* parameters:
* SessionIdHeader:
* in: header
* name: X-Session-Id
* required: true
* schema:
* type: string
* SessionKeyHeader:
* in: header
* name: X-Session-Key
* required: true
* schema:
* type: string
* schemas: * schemas:
* EventAdminSummary: * EventAdminSummary:
* type: object * type: object
+5 -8
View File
@@ -26,8 +26,9 @@ adminRouter.use(requireAdminAuth);
* summary: Validate the current admin session * summary: Validate the current admin session
* description: Used by the Next.js middleware/proxy to gate /admin. Returns the authenticated admin's identity. * description: Used by the Next.js middleware/proxy to gate /admin. Returns the authenticated admin's identity.
* tags: [feedback-admin] * tags: [feedback-admin]
* security: * parameters:
* - AdminSessionCookie: [] * - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* responses: * responses:
* 200: * 200:
* description: Success * description: Success
@@ -42,8 +43,6 @@ adminRouter.use(requireAdminAuth);
* type: string * type: string
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
adminRouter.get('/me', (req: Request, res: Response) => { adminRouter.get('/me', (req: Request, res: Response) => {
res.status(200).send({email: res.locals.admin.email, fullName: res.locals.admin.displayName}); res.status(200).send({email: res.locals.admin.email, fullName: res.locals.admin.displayName});
@@ -56,9 +55,9 @@ adminRouter.get('/me', (req: Request, res: Response) => {
* summary: Delete a single submission * summary: Delete a single submission
* description: Removes the submission and everything under it (its answers, guest book entry, newsletter signup) - for removing an individual abusive or inappropriate entry. Not a bulk moderation tool. * description: Removes the submission and everything under it (its answers, guest book entry, newsletter signup) - for removing an individual abusive or inappropriate entry. Not a bulk moderation tool.
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: submissionId * name: submissionId
* required: true * required: true
@@ -71,8 +70,6 @@ adminRouter.get('/me', (req: Request, res: Response) => {
* description: Unknown submission * description: Unknown submission
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
adminRouter.delete('/submissions/:submissionId', async (req: Request, res: Response) => { adminRouter.delete('/submissions/:submissionId', async (req: Request, res: Response) => {
try { try {
@@ -18,8 +18,9 @@ export const eventsAdminRouter = express.Router();
* summary: List all events (admin) * summary: List all events (admin)
* description: All events, published or not, past or future, with submission counts. * description: All events, published or not, past or future, with submission counts.
* tags: [feedback-admin] * tags: [feedback-admin]
* security: * parameters:
* - AdminSessionCookie: [] * - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* responses: * responses:
* 200: * 200:
* description: Success * description: Success
@@ -31,14 +32,13 @@ export const eventsAdminRouter = express.Router();
* $ref: '#/components/schemas/EventAdminSummary' * $ref: '#/components/schemas/EventAdminSummary'
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
* post: * post:
* summary: Create an event * summary: Create an event
* description: Auto-generates the slug from the name and event year; defaults feedback_deadline to event_date + 14 days 23:59:59 unless supplied. * description: Auto-generates the slug from the name and event year; defaults feedback_deadline to event_date + 14 days 23:59:59 unless supplied.
* tags: [feedback-admin] * tags: [feedback-admin]
* security: * parameters:
* - AdminSessionCookie: [] * - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* requestBody: * requestBody:
* required: true * required: true
* content: * content:
@@ -68,8 +68,6 @@ export const eventsAdminRouter = express.Router();
* description: Missing required fields * description: Missing required fields
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
eventsAdminRouter.get('/', async (req: Request, res: Response) => { eventsAdminRouter.get('/', async (req: Request, res: Response) => {
try { try {
@@ -103,9 +101,9 @@ eventsAdminRouter.post('/', async (req: Request, res: Response) => {
* summary: Get one event (admin) * summary: Get one event (admin)
* description: Full event detail including setlist and assigned questions. * description: Full event detail including setlist and assigned questions.
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: eventId * name: eventId
* required: true * required: true
@@ -122,14 +120,12 @@ eventsAdminRouter.post('/', async (req: Request, res: Response) => {
* description: Unknown event * description: Unknown event
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
* put: * put:
* summary: Update an event * summary: Update an event
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: eventId * name: eventId
* required: true * required: true
@@ -142,15 +138,13 @@ eventsAdminRouter.post('/', async (req: Request, res: Response) => {
* description: Unknown event * description: Unknown event
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
* delete: * delete:
* summary: Delete an event * summary: Delete an event
* description: Refuses with 409 if submissions exist unless ?force=true is passed. * description: Refuses with 409 if submissions exist unless ?force=true is passed.
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: eventId * name: eventId
* required: true * required: true
@@ -169,8 +163,6 @@ eventsAdminRouter.post('/', async (req: Request, res: Response) => {
* description: Submissions exist and force was not set * description: Submissions exist and force was not set
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
eventsAdminRouter.get('/:eventId', async (req: Request, res: Response) => { eventsAdminRouter.get('/:eventId', async (req: Request, res: Response) => {
try { try {
@@ -222,9 +214,9 @@ eventsAdminRouter.delete('/:eventId', async (req: Request, res: Response) => {
* get: * get:
* summary: Get an event's setlist * summary: Get an event's setlist
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: eventId * name: eventId
* required: true * required: true
@@ -235,14 +227,12 @@ eventsAdminRouter.delete('/:eventId', async (req: Request, res: Response) => {
* description: Success * description: Success
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
* post: * post:
* summary: Add a song to an event's setlist * summary: Add a song to an event's setlist
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: eventId * name: eventId
* required: true * required: true
@@ -267,8 +257,6 @@ eventsAdminRouter.delete('/:eventId', async (req: Request, res: Response) => {
* description: Missing title * description: Missing title
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
eventsAdminRouter.get('/:eventId/songs', async (req: Request, res: Response) => { eventsAdminRouter.get('/:eventId/songs', async (req: Request, res: Response) => {
try { try {
@@ -304,9 +292,9 @@ eventsAdminRouter.post('/:eventId/songs', async (req: Request, res: Response) =>
* summary: Bulk reorder an event's setlist * summary: Bulk reorder an event's setlist
* description: Rewrites song positions as a dense 0..n-1 sequence in one transaction. * description: Rewrites song positions as a dense 0..n-1 sequence in one transaction.
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: eventId * name: eventId
* required: true * required: true
@@ -329,8 +317,6 @@ eventsAdminRouter.post('/:eventId/songs', async (req: Request, res: Response) =>
* description: Reordered * description: Reordered
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
eventsAdminRouter.put('/:eventId/songs/order', async (req: Request, res: Response) => { eventsAdminRouter.put('/:eventId/songs/order', async (req: Request, res: Response) => {
try { try {
@@ -348,9 +334,9 @@ eventsAdminRouter.put('/:eventId/songs/order', async (req: Request, res: Respons
* get: * get:
* summary: Get an event's assigned questions * summary: Get an event's assigned questions
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: eventId * name: eventId
* required: true * required: true
@@ -361,15 +347,13 @@ eventsAdminRouter.put('/:eventId/songs/order', async (req: Request, res: Respons
* description: Success * description: Success
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
* put: * put:
* summary: Bulk-set an event's assigned questions * summary: Bulk-set an event's assigned questions
* description: One transaction - inserts new, updates existing, deletes removed. Keeps the admin UI a simple save-the-whole-list form. * description: One transaction - inserts new, updates existing, deletes removed. Keeps the admin UI a simple save-the-whole-list form.
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: eventId * name: eventId
* required: true * required: true
@@ -399,8 +383,6 @@ eventsAdminRouter.put('/:eventId/songs/order', async (req: Request, res: Respons
* description: Saved * description: Saved
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
eventsAdminRouter.get('/:eventId/questions', async (req: Request, res: Response) => { eventsAdminRouter.get('/:eventId/questions', async (req: Request, res: Response) => {
try { try {
@@ -16,9 +16,9 @@ export const questionsAdminRouter = express.Router();
* get: * get:
* summary: List the question library * summary: List the question library
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: query * - in: query
* name: includeArchived * name: includeArchived
* schema: * schema:
@@ -34,13 +34,12 @@ export const questionsAdminRouter = express.Router();
* $ref: '#/components/schemas/AdminQuestion' * $ref: '#/components/schemas/AdminQuestion'
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
* post: * post:
* summary: Create a question * summary: Create a question
* tags: [feedback-admin] * tags: [feedback-admin]
* security: * parameters:
* - AdminSessionCookie: [] * - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* requestBody: * requestBody:
* required: true * required: true
* content: * content:
@@ -62,8 +61,6 @@ export const questionsAdminRouter = express.Router();
* description: Missing or invalid fields * description: Missing or invalid fields
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
questionsAdminRouter.get('/', async (req: Request, res: Response) => { questionsAdminRouter.get('/', async (req: Request, res: Response) => {
try { try {
@@ -97,9 +94,9 @@ questionsAdminRouter.post('/', async (req: Request, res: Response) => {
* summary: Edit a question's label/help text * summary: Edit a question's label/help text
* description: question_type is immutable after creation - the admin UI offers "archive and create new" instead. * description: question_type is immutable after creation - the admin UI offers "archive and create new" instead.
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: questionId * name: questionId
* required: true * required: true
@@ -126,15 +123,13 @@ questionsAdminRouter.post('/', async (req: Request, res: Response) => {
* description: Unknown question * description: Unknown question
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
* delete: * delete:
* summary: Archive (or hard-delete) a question * summary: Archive (or hard-delete) a question
* description: Archives the question if it has ever been used; hard-deletes it if it has never been assigned to any event. * description: Archives the question if it has ever been used; hard-deletes it if it has never been assigned to any event.
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: questionId * name: questionId
* required: true * required: true
@@ -147,8 +142,6 @@ questionsAdminRouter.post('/', async (req: Request, res: Response) => {
* description: Unknown question * description: Unknown question
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
questionsAdminRouter.put('/:questionId', async (req: Request, res: Response) => { questionsAdminRouter.put('/:questionId', async (req: Request, res: Response) => {
try { try {
@@ -19,9 +19,9 @@ export const reportsAdminRouter = express.Router();
* summary: Aggregated feedback report for one event * summary: Aggregated feedback report for one event
* description: Song-pick vote counts, song-rating averages, capped free-text list, guest book count, and newsletter sync counts. * description: Song-pick vote counts, song-rating averages, capped free-text list, guest book count, and newsletter sync counts.
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: eventId * name: eventId
* required: true * required: true
@@ -34,8 +34,6 @@ export const reportsAdminRouter = express.Router();
* description: Unknown event * description: Unknown event
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
reportsAdminRouter.get('/:eventId/report', async (req: Request, res: Response) => { reportsAdminRouter.get('/:eventId/report', async (req: Request, res: Response) => {
try { try {
@@ -57,9 +55,9 @@ reportsAdminRouter.get('/:eventId/report', async (req: Request, res: Response) =
* summary: Guest Book entries for one event * summary: Guest Book entries for one event
* description: Newest first, paginated. * description: Newest first, paginated.
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: eventId * name: eventId
* required: true * required: true
@@ -83,8 +81,6 @@ reportsAdminRouter.get('/:eventId/report', async (req: Request, res: Response) =
* description: Success * description: Success
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
reportsAdminRouter.get('/:eventId/guestbook', async (req: Request, res: Response) => { reportsAdminRouter.get('/:eventId/guestbook', async (req: Request, res: Response) => {
try { try {
@@ -105,9 +101,9 @@ reportsAdminRouter.get('/:eventId/guestbook', async (req: Request, res: Response
* summary: Newsletter signups for one event * summary: Newsletter signups for one event
* description: Includes sync_status, so failures can be handled manually. Newest first, paginated. * description: Includes sync_status, so failures can be handled manually. Newest first, paginated.
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: eventId * name: eventId
* required: true * required: true
@@ -131,8 +127,6 @@ reportsAdminRouter.get('/:eventId/guestbook', async (req: Request, res: Response
* description: Success * description: Success
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
reportsAdminRouter.get('/:eventId/newsletter', async (req: Request, res: Response) => { reportsAdminRouter.get('/:eventId/newsletter', async (req: Request, res: Response) => {
try { try {
@@ -153,9 +147,9 @@ reportsAdminRouter.get('/:eventId/newsletter', async (req: Request, res: Respons
* summary: CSV export of all answers for one event * summary: CSV export of all answers for one event
* description: Long format, one row per answer. UTF-8 BOM, `;` separator, RFC 4180 escaping, formula-injection guard. * description: Long format, one row per answer. UTF-8 BOM, `;` separator, RFC 4180 escaping, formula-injection guard.
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: eventId * name: eventId
* required: true * required: true
@@ -168,8 +162,6 @@ reportsAdminRouter.get('/:eventId/newsletter', async (req: Request, res: Respons
* text/csv: {} * text/csv: {}
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
reportsAdminRouter.get('/:eventId/export/responses.csv', async (req: Request, res: Response) => { reportsAdminRouter.get('/:eventId/export/responses.csv', async (req: Request, res: Response) => {
try { try {
@@ -195,9 +187,9 @@ reportsAdminRouter.get('/:eventId/export/responses.csv', async (req: Request, re
* get: * get:
* summary: CSV export of Guest Book entries for one event * summary: CSV export of Guest Book entries for one event
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: eventId * name: eventId
* required: true * required: true
@@ -210,8 +202,6 @@ reportsAdminRouter.get('/:eventId/export/responses.csv', async (req: Request, re
* text/csv: {} * text/csv: {}
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
reportsAdminRouter.get('/:eventId/export/guestbook.csv', async (req: Request, res: Response) => { reportsAdminRouter.get('/:eventId/export/guestbook.csv', async (req: Request, res: Response) => {
try { try {
@@ -16,9 +16,9 @@ export const songsAdminRouter = express.Router();
* put: * put:
* summary: Edit a song's title/composer * summary: Edit a song's title/composer
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: songId * name: songId
* required: true * required: true
@@ -45,15 +45,13 @@ export const songsAdminRouter = express.Router();
* description: Unknown song * description: Unknown song
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
* delete: * delete:
* summary: Remove a song * summary: Remove a song
* description: Past answers keep their song_title_snapshot even after the song is removed. * description: Past answers keep their song_title_snapshot even after the song is removed.
* tags: [feedback-admin] * tags: [feedback-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: songId * name: songId
* required: true * required: true
@@ -66,8 +64,6 @@ export const songsAdminRouter = express.Router();
* description: Unknown song * description: Unknown song
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
songsAdminRouter.put('/:songId', async (req: Request, res: Response) => { songsAdminRouter.put('/:songId', async (req: Request, res: Response) => {
try { try {
+64 -24
View File
@@ -1,38 +1,78 @@
import {requireAppAccess} from '../admin/admin.middleware.js'; import express from 'express';
import * as UserService from '../calendar/users/users.service.js';
import {sendServerError} from './feedback.errors.js';
/** /**
* This file is the ONLY place in the feedback module that knows how admin * This file is the ONLY place in the feedback module that knows how admin
* authentication works. No route handler and no service outside this file may * authentication works today. No route handler and no service outside this
* read session headers or resolve a user itself. * file may import users.service, read session headers, or touch bcrypt.
* *
* Today: the shared admin identity in `src/models/admin/`. A session cookie * Today: reuses the existing Calendar users/sessions mechanism. Any
* set by /admin/auth on admin.nachklang.art, plus a `feedback` permission on * activated @nachklang.art account may administer feedback — no roles.
* the account. Both are re-checked on every request, so disabling a user or * Migrating to Keycloak later means writing a keycloakJwtAuthenticator
* taking their feedback permission away takes effect immediately. * below and changing the one `activeAuthenticator` binding (plus the
* frontend's login route handler) — nothing else in the feedback module
* needs to change.
* *
* Before 2026-09-06 this was a header session against the calendar users * Explicitly forbidden: accepting sessionId/sessionKey from query
* table, and any activated @nachklang.art account could administer feedback. * parameters, even "temporarily". That is the exact mistake documented in
* That is why the swap is a one-line binding: everything downstream only ever * DEFERRED_SECURITY.md item 1 for the Calendar domain, where credentials
* saw `requireAdminAuth` and `res.locals.admin`, and both still mean what * end up in access logs, browser history, proxy logs, and Referer headers.
* they meant. What changed is that access is now granted per user rather than * Headers only.
* implied by having an account.
*
* Explicitly forbidden: accepting session credentials from query parameters,
* even "temporarily". That is the exact mistake documented in
* DEFERRED_SECURITY.md item 1 for the Calendar domain, where credentials end
* up in access logs, browser history, proxy logs, and Referer headers.
*/ */
// The only thing the rest of the feedback module knows about an admin. The // The only thing the rest of the feedback module knows about an admin.
// shared middleware puts a superset of this on res.locals.admin.
export interface AdminIdentity { export interface AdminIdentity {
id: string; id: string;
email: string; email: string;
displayName: string; displayName: string;
} }
// Pluggable strategy: extract + verify credentials from a request.
// Returns the identity, or null if unauthenticated. Throws only on
// infrastructure errors (e.g. the DB being unreachable).
export type AdminAuthenticator = (req: express.Request) => Promise<AdminIdentity | null>;
// Current implementation: reads X-Session-Id / X-Session-Key headers,
// delegates to the existing calendar UserService.checkSession(...).
export const sessionHeaderAuthenticator: AdminAuthenticator = async (req) => {
const sessionId = req.header('X-Session-Id');
const sessionKey = req.header('X-Session-Key');
if (!sessionId || !sessionKey) {
return null;
}
const ip = req.ip || '';
const user = await UserService.checkSession(sessionId, sessionKey, ip);
// Mirrors the Calendar domain's own convention: a valid session on an
// inactive (not yet activated) account is not sufficient.
if (!user || !user.isActive) {
return null;
}
return {
id: String(user.userId),
email: user.email,
displayName: user.fullName
};
};
// Swap point: change this one binding to migrate to Keycloak.
export const activeAuthenticator: AdminAuthenticator = sessionHeaderAuthenticator;
// Express middleware used by every admin route. On success: // Express middleware used by every admin route. On success:
// res.locals.admin = AdminAccess (an AdminIdentity plus permissions), calls // res.locals.admin = AdminIdentity, calls next(). On failure: 401.
// next(). On failure: 401 when not signed in, 403 when signed in without the export const requireAdminAuth: express.RequestHandler = async (req, res, next) => {
// feedback permission. try {
export const requireAdminAuth = requireAppAccess('feedback'); const identity = await activeAuthenticator(req);
if (!identity) {
res.status(401).send({status: 'UNAUTHORIZED', message: 'Anmeldung erforderlich.'});
return;
}
res.locals.admin = identity;
next();
} catch (e: any) {
sendServerError(res, e);
}
};
+3 -4
View File
@@ -16,15 +16,14 @@ adminRouter.use(requireAdminAuth);
* get: * get:
* summary: Validate the current admin session * summary: Validate the current admin session
* tags: [tickets-admin] * tags: [tickets-admin]
* security: * parameters:
* - AdminSessionCookie: [] * - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* responses: * responses:
* 200: * 200:
* description: Success * description: Success
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
adminRouter.get('/me', (req: Request, res: Response) => { adminRouter.get('/me', (req: Request, res: Response) => {
res.status(200).send({email: res.locals.admin.email, fullName: res.locals.admin.displayName}); res.status(200).send({email: res.locals.admin.email, fullName: res.locals.admin.displayName});
+12 -20
View File
@@ -11,15 +11,14 @@ export const eventsAdminRouter = express.Router();
* summary: List concerts for the admin event picker * summary: List concerts for the admin event picker
* description: Wraps the Calendar module's public-calendar admin listing (includes DRAFT events). * description: Wraps the Calendar module's public-calendar admin listing (includes DRAFT events).
* tags: [tickets-admin] * tags: [tickets-admin]
* security: * parameters:
* - AdminSessionCookie: [] * - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* responses: * responses:
* 200: * 200:
* description: Success * description: Success
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
eventsAdminRouter.get('/', async (req: Request, res: Response) => { eventsAdminRouter.get('/', async (req: Request, res: Response) => {
try { try {
@@ -36,15 +35,14 @@ eventsAdminRouter.get('/', async (req: Request, res: Response) => {
* summary: List public-calendar events not yet added to the ticket shop * summary: List public-calendar events not yet added to the ticket shop
* description: Source list for the "add a concert" picker - the public calendar holds more than concerts, so events only appear in the ticket shop once explicitly added. * description: Source list for the "add a concert" picker - the public calendar holds more than concerts, so events only appear in the ticket shop once explicitly added.
* tags: [tickets-admin] * tags: [tickets-admin]
* security: * parameters:
* - AdminSessionCookie: [] * - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* responses: * responses:
* 200: * 200:
* description: Success * description: Success
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
eventsAdminRouter.get('/available', async (req: Request, res: Response) => { eventsAdminRouter.get('/available', async (req: Request, res: Response) => {
try { try {
@@ -60,9 +58,9 @@ eventsAdminRouter.get('/available', async (req: Request, res: Response) => {
* get: * get:
* summary: Get a concert's voucher/capacity stats * summary: Get a concert's voucher/capacity stats
* tags: [tickets-admin] * tags: [tickets-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: eventId * name: eventId
* required: true * required: true
@@ -77,8 +75,6 @@ eventsAdminRouter.get('/available', async (req: Request, res: Response) => {
* $ref: '#/components/schemas/EventStats' * $ref: '#/components/schemas/EventStats'
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
eventsAdminRouter.get('/:eventId/stats', async (req: Request, res: Response) => { eventsAdminRouter.get('/:eventId/stats', async (req: Request, res: Response) => {
try { try {
@@ -95,9 +91,9 @@ eventsAdminRouter.get('/:eventId/stats', async (req: Request, res: Response) =>
* summary: Set a concert's voucher settings * summary: Set a concert's voucher settings
* description: Upserts capacity (null = uncapped), redemption deadline (null = none), and whether to collect a mailing address. * description: Upserts capacity (null = uncapped), redemption deadline (null = none), and whether to collect a mailing address.
* tags: [tickets-admin] * tags: [tickets-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: eventId * name: eventId
* required: true * required: true
@@ -127,8 +123,6 @@ eventsAdminRouter.get('/:eventId/stats', async (req: Request, res: Response) =>
* description: Saved * description: Saved
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
eventsAdminRouter.put('/:eventId/settings', async (req: Request, res: Response) => { eventsAdminRouter.put('/:eventId/settings', async (req: Request, res: Response) => {
try { try {
@@ -155,9 +149,9 @@ eventsAdminRouter.put('/:eventId/settings', async (req: Request, res: Response)
* summary: Remove an event from the ticket shop * summary: Remove an event from the ticket shop
* description: Deletes its settings row, so it drops out of the picker and reappears in the "add" list. Refused with 409 if vouchers already reference the event - existing vouchers/redemptions stay valid either way. * description: Deletes its settings row, so it drops out of the picker and reappears in the "add" list. Refused with 409 if vouchers already reference the event - existing vouchers/redemptions stay valid either way.
* tags: [tickets-admin] * tags: [tickets-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: eventId * name: eventId
* required: true * required: true
@@ -170,8 +164,6 @@ eventsAdminRouter.put('/:eventId/settings', async (req: Request, res: Response)
* description: Vouchers already reference this event * description: Vouchers already reference this event
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
eventsAdminRouter.delete('/:eventId/settings', async (req: Request, res: Response) => { eventsAdminRouter.delete('/:eventId/settings', async (req: Request, res: Response) => {
try { try {
@@ -11,9 +11,9 @@ export const redemptionsAdminRouter = express.Router();
* summary: List redemptions (admin) * summary: List redemptions (admin)
* description: Filterable by event and status (ACTIVE/UNDONE). * description: Filterable by event and status (ACTIVE/UNDONE).
* tags: [tickets-admin] * tags: [tickets-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: query * - in: query
* name: eventId * name: eventId
* schema: * schema:
@@ -33,8 +33,6 @@ export const redemptionsAdminRouter = express.Router();
* $ref: '#/components/schemas/RedemptionSummary' * $ref: '#/components/schemas/RedemptionSummary'
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
redemptionsAdminRouter.get('/', async (req: Request, res: Response) => { redemptionsAdminRouter.get('/', async (req: Request, res: Response) => {
try { try {
@@ -52,9 +50,9 @@ redemptionsAdminRouter.get('/', async (req: Request, res: Response) => {
* get: * get:
* summary: Get a single redemption (admin) * summary: Get a single redemption (admin)
* tags: [tickets-admin] * tags: [tickets-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: redemptionId * name: redemptionId
* required: true * required: true
@@ -67,15 +65,13 @@ redemptionsAdminRouter.get('/', async (req: Request, res: Response) => {
* description: Unknown redemption * description: Unknown redemption
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
* patch: * patch:
* summary: Edit a redemption's contact info and/or guest list * summary: Edit a redemption's contact info and/or guest list
* description: Only fields present in the body are changed. Growing the guest count is re-checked against the voucher's max guests and the event's remaining capacity. Logs to the audit trail with an optional admin-supplied reason. * description: Only fields present in the body are changed. Growing the guest count is re-checked against the voucher's max guests and the event's remaining capacity. Logs to the audit trail with an optional admin-supplied reason.
* tags: [tickets-admin] * tags: [tickets-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: redemptionId * name: redemptionId
* required: true * required: true
@@ -109,8 +105,6 @@ redemptionsAdminRouter.get('/', async (req: Request, res: Response) => {
* description: Not active, exceeds max guests, or exceeds remaining capacity * description: Not active, exceeds max guests, or exceeds remaining capacity
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
redemptionsAdminRouter.get('/:redemptionId', async (req: Request, res: Response) => { redemptionsAdminRouter.get('/:redemptionId', async (req: Request, res: Response) => {
try { try {
@@ -167,9 +161,9 @@ redemptionsAdminRouter.patch('/:redemptionId', async (req: Request, res: Respons
* summary: Undo a redemption * summary: Undo a redemption
* description: Reopens the code (back to UNUSED) and marks the redemption UNDONE. Guest data is kept for the audit trail; a later re-redemption creates a new redemption record. * description: Reopens the code (back to UNUSED) and marks the redemption UNDONE. Guest data is kept for the audit trail; a later re-redemption creates a new redemption record.
* tags: [tickets-admin] * tags: [tickets-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: redemptionId * name: redemptionId
* required: true * required: true
@@ -192,8 +186,6 @@ redemptionsAdminRouter.patch('/:redemptionId', async (req: Request, res: Respons
* description: Redemption is not active * description: Redemption is not active
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
redemptionsAdminRouter.post('/:redemptionId/undo', async (req: Request, res: Response) => { redemptionsAdminRouter.post('/:redemptionId/undo', async (req: Request, res: Response) => {
try { try {
@@ -219,9 +211,9 @@ redemptionsAdminRouter.post('/:redemptionId/undo', async (req: Request, res: Res
* summary: Resend the redemption confirmation email * summary: Resend the redemption confirmation email
* description: Rebuilds the confirmation email from the stored redemption data and sends it again, then records the outcome on the redemption. Intended for redemptions whose original confirmation email failed. * description: Rebuilds the confirmation email from the stored redemption data and sends it again, then records the outcome on the redemption. Intended for redemptions whose original confirmation email failed.
* tags: [tickets-admin] * tags: [tickets-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: redemptionId * name: redemptionId
* required: true * required: true
@@ -238,8 +230,6 @@ redemptionsAdminRouter.post('/:redemptionId/undo', async (req: Request, res: Res
* description: The email relay rejected the send * description: The email relay rejected the send
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
redemptionsAdminRouter.post('/:redemptionId/resend-confirmation', async (req: Request, res: Response) => { redemptionsAdminRouter.post('/:redemptionId/resend-confirmation', async (req: Request, res: Response) => {
try { try {
@@ -269,9 +259,9 @@ redemptionsAdminRouter.post('/:redemptionId/resend-confirmation', async (req: Re
* get: * get:
* summary: Get a voucher's admin-action audit trail * summary: Get a voucher's admin-action audit trail
* tags: [tickets-admin] * tags: [tickets-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: code * name: code
* required: true * required: true
@@ -288,8 +278,6 @@ redemptionsAdminRouter.post('/:redemptionId/resend-confirmation', async (req: Re
* $ref: '#/components/schemas/AuditLogEntry' * $ref: '#/components/schemas/AuditLogEntry'
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
export const voucherHistoryRouter = express.Router(); export const voucherHistoryRouter = express.Router();
voucherHistoryRouter.get('/:code/history', async (req: Request, res: Response) => { voucherHistoryRouter.get('/:code/history', async (req: Request, res: Response) => {
@@ -11,9 +11,9 @@ export const vouchersAdminRouter = express.Router();
* summary: List vouchers (admin) * summary: List vouchers (admin)
* description: Filterable by event and status. * description: Filterable by event and status.
* tags: [tickets-admin] * tags: [tickets-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: query * - in: query
* name: eventId * name: eventId
* schema: * schema:
@@ -33,8 +33,6 @@ export const vouchersAdminRouter = express.Router();
* $ref: '#/components/schemas/VoucherCode' * $ref: '#/components/schemas/VoucherCode'
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
vouchersAdminRouter.get('/', async (req: Request, res: Response) => { vouchersAdminRouter.get('/', async (req: Request, res: Response) => {
try { try {
@@ -53,8 +51,9 @@ vouchersAdminRouter.get('/', async (req: Request, res: Response) => {
* summary: Batch-generate wildcard codes * summary: Batch-generate wildcard codes
* description: Generates `quantity` codes sharing the same eligible events and max-guest count, grouped under one batchId. * description: Generates `quantity` codes sharing the same eligible events and max-guest count, grouped under one batchId.
* tags: [tickets-admin] * tags: [tickets-admin]
* security: * parameters:
* - AdminSessionCookie: [] * - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* requestBody: * requestBody:
* required: true * required: true
* content: * content:
@@ -88,8 +87,6 @@ vouchersAdminRouter.get('/', async (req: Request, res: Response) => {
* description: Invalid input * description: Invalid input
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
vouchersAdminRouter.post('/wildcard', async (req: Request, res: Response) => { vouchersAdminRouter.post('/wildcard', async (req: Request, res: Response) => {
try { try {
@@ -115,8 +112,9 @@ vouchersAdminRouter.post('/wildcard', async (req: Request, res: Response) => {
* summary: Bulk-create personalized codes * summary: Bulk-create personalized codes
* description: One code per row (name, email, eligible events, max guests), grouped under one batchId. * description: One code per row (name, email, eligible events, max guests), grouped under one batchId.
* tags: [tickets-admin] * tags: [tickets-admin]
* security: * parameters:
* - AdminSessionCookie: [] * - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* requestBody: * requestBody:
* required: true * required: true
* content: * content:
@@ -149,8 +147,6 @@ vouchersAdminRouter.post('/wildcard', async (req: Request, res: Response) => {
* description: Invalid input * description: Invalid input
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
vouchersAdminRouter.post('/personalized', async (req: Request, res: Response) => { vouchersAdminRouter.post('/personalized', async (req: Request, res: Response) => {
try { try {
@@ -173,9 +169,9 @@ vouchersAdminRouter.post('/personalized', async (req: Request, res: Response) =>
* get: * get:
* summary: Get a single voucher (admin) * summary: Get a single voucher (admin)
* tags: [tickets-admin] * tags: [tickets-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: code * name: code
* required: true * required: true
@@ -188,8 +184,6 @@ vouchersAdminRouter.post('/personalized', async (req: Request, res: Response) =>
* description: Unknown code * description: Unknown code
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
vouchersAdminRouter.get('/:code', async (req: Request, res: Response) => { vouchersAdminRouter.get('/:code', async (req: Request, res: Response) => {
try { try {
@@ -211,9 +205,9 @@ vouchersAdminRouter.get('/:code', async (req: Request, res: Response) => {
* summary: Void an unredeemed code * summary: Void an unredeemed code
* description: Only allowed while the code is UNUSED. Logs to the voucher's audit trail. * description: Only allowed while the code is UNUSED. Logs to the voucher's audit trail.
* tags: [tickets-admin] * tags: [tickets-admin]
* security:
* - AdminSessionCookie: []
* parameters: * parameters:
* - $ref: '#/components/parameters/SessionIdHeader'
* - $ref: '#/components/parameters/SessionKeyHeader'
* - in: path * - in: path
* name: code * name: code
* required: true * required: true
@@ -236,8 +230,6 @@ vouchersAdminRouter.get('/:code', async (req: Request, res: Response) => {
* description: Code is not in UNUSED status * description: Code is not in UNUSED status
* 401: * 401:
* description: Unauthorized * description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/ */
vouchersAdminRouter.post('/:code/void', async (req: Request, res: Response) => { vouchersAdminRouter.post('/:code/void', async (req: Request, res: Response) => {
try { try {
+49 -17
View File
@@ -1,23 +1,20 @@
import {requireAppAccess} from '../admin/admin.middleware.js'; import express from 'express';
import * as UserService from '../calendar/users/users.service.js';
import {sendServerError} from './tickets.errors.js';
/** /**
* Mirrors the Feedback module's feedback.auth.ts: this is the ONLY place in * Mirrors the Feedback module's feedback.auth.ts: this is the ONLY place in
* the tickets module that knows how admin authentication works. No route * the tickets module that knows how admin authentication works. No route
* handler and no service outside this file may read session headers or * handler and no service outside this file may import users.service, read
* resolve a user itself. * session headers, or touch bcrypt.
* *
* Today: the shared admin identity in `src/models/admin/`. A session cookie * Today: reuses the existing Calendar users/sessions mechanism. Any
* set by /admin/auth on admin.nachklang.art, plus a `tickets` permission on * activated @nachklang.art account may administer vouchers - no roles, same
* the account. Both are re-checked on every request, so disabling a user or * policy as Feedback (see docs/plan-ticket-shop.md). A dedicated
* taking their tickets permission away takes effect immediately. * roles/permissions model is explicitly out of scope for v1.
* *
* Before 2026-09-06 this was a header session against the calendar users * Explicitly forbidden: accepting sessionId/sessionKey from query
* table, and any activated @nachklang.art account could administer vouchers * parameters - headers only (see DEFERRED_SECURITY.md item 1).
* (see docs/plan-ticket-shop.md, which called a roles model out of scope for
* v1). It is in scope now, and lives in the admin module rather than here.
*
* Explicitly forbidden: accepting session credentials from query parameters -
* see DEFERRED_SECURITY.md item 1.
*/ */
export interface AdminIdentity { export interface AdminIdentity {
@@ -26,6 +23,41 @@ export interface AdminIdentity {
displayName: string; displayName: string;
} }
// On failure: 401 when not signed in, 403 when signed in without the tickets export type AdminAuthenticator = (req: express.Request) => Promise<AdminIdentity | null>;
// permission.
export const requireAdminAuth = requireAppAccess('tickets'); export const sessionHeaderAuthenticator: AdminAuthenticator = async (req) => {
const sessionId = req.header('X-Session-Id');
const sessionKey = req.header('X-Session-Key');
if (!sessionId || !sessionKey) {
return null;
}
const ip = req.ip || '';
const user = await UserService.checkSession(sessionId, sessionKey, ip);
if (!user || !user.isActive) {
return null;
}
return {
id: String(user.userId),
email: user.email,
displayName: user.fullName
};
};
export const activeAuthenticator: AdminAuthenticator = sessionHeaderAuthenticator;
export const requireAdminAuth: express.RequestHandler = async (req, res, next) => {
try {
const identity = await activeAuthenticator(req);
if (!identity) {
res.status(401).send({status: 'UNAUTHORIZED', message: 'Anmeldung erforderlich.'});
return;
}
res.locals.admin = identity;
next();
} catch (e: any) {
sendServerError(res, e);
}
};
+1 -6
View File
@@ -79,12 +79,7 @@ describe('bootstrapAdmin', () => {
await bootstrapAdmin(); await bootstrapAdmin();
expect(createInvitation).toHaveBeenCalledWith( expect(createInvitation).toHaveBeenCalledWith('boss@nachklang.art', 'Nachklang Admin', ['admin'], null);
'boss@nachklang.art',
'Nachklang Admin',
[{app: 'admin', role: 'access'}],
null
);
expect(mockMail).toHaveBeenCalled(); expect(mockMail).toHaveBeenCalled();
}); });
-143
View File
@@ -1,143 +0,0 @@
import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest';
// admin.config calls dotenv.config(), which would read the repo's own .env and
// quietly reintroduce NODE_ENV=development - the exact value several of these
// cases exist to remove. Stub it so the tests see only what they set.
vi.mock('dotenv', () => ({config: vi.fn()}));
/**
* admin.config reads the environment once at import, so every case here has to
* reset the module registry and re-import it. The two things worth pinning are
* the ones that are silent when wrong: which client-IP header is trusted, and
* whether an unset NODE_ENV counts as production.
*/
const ORIGINAL_ENV = {...process.env};
const loadConfig = async () => {
vi.resetModules();
return import('../../src/models/admin/admin.config.js');
};
beforeEach(() => {
process.env = {...ORIGINAL_ENV};
// dotenv.config() in admin.config does not overwrite what is already set,
// so setting these here is enough to keep the local .env out of the test.
process.env.NODE_ENV = 'test';
delete process.env.CLIENT_IP_HEADERS;
delete process.env.TRUSTED_PROXY_IPS;
});
afterEach(() => {
process.env = {...ORIGINAL_ENV};
});
describe('CLIENT_IP_HEADERS', () => {
it('defaults to the single header Plesk nginx sets', async () => {
const config = await loadConfig();
expect(config.CLIENT_IP_HEADERS).toEqual(['x-real-ip']);
expect(config.TRUST_NO_CLIENT_IP_HEADER).toBe(false);
});
it('reads a comma-separated list', async () => {
process.env.CLIENT_IP_HEADERS = 'x-real-ip, cf-connecting-ip';
const config = await loadConfig();
expect(config.CLIENT_IP_HEADERS).toEqual(['x-real-ip', 'cf-connecting-ip']);
});
it('trusts nothing when set to "none"', async () => {
// The escape hatch. An empty list is what better-auth reads as "no
// headers" - it only falls back to its own default when the option is
// absent - so this really does stop any header being believed.
process.env.CLIENT_IP_HEADERS = 'none';
const config = await loadConfig();
expect(config.CLIENT_IP_HEADERS).toEqual([]);
expect(config.TRUST_NO_CLIENT_IP_HEADER).toBe(true);
});
it('accepts the hatch case-insensitively and with stray whitespace', async () => {
process.env.CLIENT_IP_HEADERS = ' NONE ';
const config = await loadConfig();
expect(config.CLIENT_IP_HEADERS).toEqual([]);
});
it('treats an empty value as "use the default", not as the hatch', async () => {
// A blank line in a .env must not silently change how requests are
// bucketed - only the explicit word does that.
process.env.CLIENT_IP_HEADERS = '';
const config = await loadConfig();
expect(config.CLIENT_IP_HEADERS).toEqual(['x-real-ip']);
expect(config.TRUST_NO_CLIENT_IP_HEADER).toBe(false);
});
it('does not mistake a header actually named none-ish for the hatch', async () => {
process.env.CLIENT_IP_HEADERS = 'x-none';
const config = await loadConfig();
expect(config.CLIENT_IP_HEADERS).toEqual(['x-none']);
expect(config.TRUST_NO_CLIENT_IP_HEADER).toBe(false);
});
});
describe('APP_ORIGINS', () => {
beforeEach(() => {
delete process.env.APP_ORIGINS;
});
// These reach better-auth's trustedOrigins, and the step 4 cutover made the
// tickets and feedback origins load-bearing: without them their sign-out
// call is rejected while everything else still works.
it('defaults to the two production frontends', async () => {
const config = await loadConfig();
expect(config.APP_ORIGINS).toEqual([
'https://tickets.nachklang.art',
'https://feedback.nachklang.art'
]);
});
it('is overridden wholesale by the environment, for a staging host', async () => {
process.env.APP_ORIGINS = 'https://tickets.staging.example, https://feedback.staging.example/';
const config = await loadConfig();
expect(config.APP_ORIGINS).toEqual([
'https://tickets.staging.example',
// Trailing slash stripped: an origin with one never matches.
'https://feedback.staging.example'
]);
});
it('always includes the admin app itself in ADMIN_ALLOWED_ORIGINS', async () => {
process.env.ADMIN_APP_URL = 'https://admin.nachklang.art';
const config = await loadConfig();
expect(config.ADMIN_ALLOWED_ORIGINS).toContain('https://admin.nachklang.art');
expect(config.ADMIN_ALLOWED_ORIGINS).toContain('https://tickets.nachklang.art');
});
});
describe('isProd', () => {
it('is false only for the explicit relaxed environments', async () => {
process.env.NODE_ENV = 'development';
expect((await loadConfig()).isProd).toBe(false);
process.env.NODE_ENV = 'test';
expect((await loadConfig()).isProd).toBe(false);
});
it('treats an unset NODE_ENV as production, which is what a bare vhost gives', async () => {
delete process.env.NODE_ENV;
// Strict mode refuses to boot without these; supply them so the import
// gets far enough to answer the question being asked.
process.env.BETTER_AUTH_SECRET = 'x'.repeat(48);
process.env.API_BASE_URL = 'https://api.nachklang.art';
process.env.ADMIN_APP_URL = 'https://admin.nachklang.art';
expect((await loadConfig()).isProd).toBe(true);
});
it('refuses to start without a signing key outside development', async () => {
delete process.env.NODE_ENV;
delete process.env.BETTER_AUTH_SECRET;
process.env.API_BASE_URL = 'https://api.nachklang.art';
process.env.ADMIN_APP_URL = 'https://admin.nachklang.art';
await expect(loadConfig()).rejects.toThrow(/BETTER_AUTH_SECRET/);
});
});
+1 -49
View File
@@ -30,10 +30,6 @@ const activeUser = {
email: 'a@nachklang.art', email: 'a@nachklang.art',
displayName: 'A', displayName: 'A',
disabled: false, disabled: false,
permissions: [
{app: 'feedback', role: 'access'},
{app: 'admin', role: 'access'}
],
apps: ['feedback', 'admin'] apps: ['feedback', 'admin']
}; };
@@ -64,10 +60,6 @@ describe('resolveAccess', () => {
email: 'a@nachklang.art', email: 'a@nachklang.art',
displayName: 'A', displayName: 'A',
disabled: false, disabled: false,
permissions: [
{app: 'feedback', role: 'access'},
{app: 'admin', role: 'access'}
],
apps: ['feedback', 'admin'] apps: ['feedback', 'admin']
}); });
// No cookieCache: exactly one lookup per request, never zero. // No cookieCache: exactly one lookup per request, never zero.
@@ -135,11 +127,7 @@ describe('requireAppAccess', () => {
it('403s a signed-in user without that app permission', async () => { it('403s a signed-in user without that app permission', async () => {
mockGetSession.mockResolvedValue({user: {id: 'u1'}}); mockGetSession.mockResolvedValue({user: {id: 'u1'}});
mockLoadAccess.mockResolvedValue({ mockLoadAccess.mockResolvedValue({...activeUser, apps: ['feedback']});
...activeUser,
permissions: [{app: 'feedback', role: 'access'}],
apps: ['feedback']
});
const res = makeRes(); const res = makeRes();
const next = vi.fn(); const next = vi.fn();
@@ -183,40 +171,4 @@ describe('requireAppAccess', () => {
expect(res.status).toHaveBeenCalledWith(500); expect(res.status).toHaveBeenCalledWith(500);
expect(next).not.toHaveBeenCalled(); expect(next).not.toHaveBeenCalled();
}); });
// The seam a finer per-app permission arrives through. Nothing passes a role
// today, so these two pin the behaviour before there is anything to break.
it('403s when a specific role is required and the user only holds another', async () => {
mockGetSession.mockResolvedValue({user: {id: 'u1'}});
mockLoadAccess.mockResolvedValue({
...activeUser,
permissions: [{app: 'tickets', role: 'access'}],
apps: ['tickets']
});
const res = makeRes();
const next = vi.fn();
await requireAppAccess('tickets', 'refund')(makeReq(), res, next);
expect(res.status).toHaveBeenCalledWith(403);
expect(next).not.toHaveBeenCalled();
});
it('passes when the user holds exactly the required role', async () => {
mockGetSession.mockResolvedValue({user: {id: 'u1'}});
mockLoadAccess.mockResolvedValue({
...activeUser,
permissions: [
{app: 'tickets', role: 'access'},
{app: 'tickets', role: 'refund'}
],
apps: ['tickets']
});
const res = makeRes();
const next = vi.fn();
await requireAppAccess('tickets', 'refund')(makeReq(), res, next);
expect(next).toHaveBeenCalled();
});
}); });
-99
View File
@@ -1,99 +0,0 @@
import {describe, expect, it} from 'vitest';
import {
ACCESS_ROLE,
appsOf,
isAppPermission,
isAppRole,
toPermissions
} from '../../src/models/admin/admin.schema.js';
/**
* The permission model is (app, role). These tests pin the two properties the
* rest of the module leans on: that the older `['tickets']` shape still means
* "tickets at the access role", and that nothing outside APP_ROLES gets in.
*/
describe('toPermissions', () => {
it('reads the full (app, role) form', () => {
expect(toPermissions([{app: 'tickets', role: 'access'}])).toEqual([
{app: 'tickets', role: 'access'}
]);
});
it('reads a plain app list as that app at the access role', () => {
expect(toPermissions(['feedback', 'admin'])).toEqual([
{app: 'feedback', role: ACCESS_ROLE},
{app: 'admin', role: ACCESS_ROLE}
]);
});
it('accepts the two forms mixed, which is what a half-migrated caller sends', () => {
expect(toPermissions(['feedback', {app: 'tickets', role: 'access'}])).toEqual([
{app: 'feedback', role: ACCESS_ROLE},
{app: 'tickets', role: ACCESS_ROLE}
]);
});
it('drops duplicates of the same (app, role)', () => {
expect(toPermissions(['tickets', {app: 'tickets', role: 'access'}])).toEqual([
{app: 'tickets', role: ACCESS_ROLE}
]);
});
it('rejects rather than silently dropping an unknown app', () => {
// Silently ignoring it would let "grant calendar + nonsense" look like a
// success while granting less than the caller asked for.
expect(toPermissions(['calendar', 'nonsense'])).toBeNull();
});
it('rejects an unknown role', () => {
expect(toPermissions([{app: 'tickets', role: 'refund'}])).toBeNull();
});
it('rejects anything that is not a list', () => {
expect(toPermissions('admin')).toBeNull();
expect(toPermissions(null)).toBeNull();
expect(toPermissions({app: 'admin', role: 'access'})).toBeNull();
});
it('reads an empty list as "no permissions", not as invalid', () => {
expect(toPermissions([])).toEqual([]);
});
});
describe('isAppRole', () => {
it('accepts the access role for every app', () => {
expect(isAppRole('admin', ACCESS_ROLE)).toBe(true);
expect(isAppRole('calendar', ACCESS_ROLE)).toBe(true);
});
it('rejects a role that does not exist yet', () => {
expect(isAppRole('tickets', 'refund')).toBe(false);
});
});
describe('isAppPermission', () => {
it('needs both halves to be valid', () => {
expect(isAppPermission({app: 'tickets', role: ACCESS_ROLE})).toBe(true);
expect(isAppPermission({app: 'tickets'})).toBe(false);
expect(isAppPermission({role: ACCESS_ROLE})).toBe(false);
expect(isAppPermission(null)).toBe(false);
});
});
describe('appsOf', () => {
it('collapses several roles on one app to a single entry', () => {
// The point of the derived list: a user with two roles on tickets has
// access to tickets once, not twice.
const apps = appsOf([
{app: 'tickets', role: ACCESS_ROLE},
{app: 'tickets', role: 'future-role'},
{app: 'admin', role: ACCESS_ROLE}
]);
expect(apps).toEqual(['tickets', 'admin']);
});
it('is empty for no permissions', () => {
expect(appsOf([])).toEqual([]);
});
});
-118
View File
@@ -1,118 +0,0 @@
import {vi, describe, it, expect, beforeEach, type Mock} from 'vitest';
import express, {Request, Response} from 'express';
/**
* Shared body for the two cutover tests (2026-09-06). feedback.auth.ts and
* tickets.auth.ts used to carry their own header-session authenticator against
* the calendar users table; both are now one binding to the shared admin gate.
*
* What is worth asserting is not how that gate works - admin.middleware.test.ts
* owns that - but that each module is bound to *its own* app, and that neither
* consults the calendar users service any more. The mocks live in the calling
* file because vi.mock is per-module-graph; only the assertions are shared.
*/
export interface BindingMocks {
/** auth.api.getSession from the mocked admin.auth.js */
getSession: Mock;
/** loadAccess from the mocked users.admin.service.js */
loadAccess: Mock;
/** checkSession from the mocked calendar users.service.js */
checkSession: Mock;
}
const makeReq = (): Request => ({headers: {cookie: 'nachklang.session_token=abc'}} as unknown as Request);
const makeRes = (): Response => {
const res: any = {};
res.status = vi.fn().mockReturnValue(res);
res.send = vi.fn().mockReturnValue(res);
res.locals = {};
return res as Response;
};
const userWith = (...apps: string[]) => ({
id: 'u1',
email: 'a@nachklang.art',
displayName: 'Anna Admin',
disabled: false,
permissions: apps.map(app => ({app, role: 'access'})),
apps
});
export const describeAdminBinding = (
app: string,
otherApp: string,
middleware: express.RequestHandler,
mocks: () => BindingMocks
): void => {
describe(`${app} requireAdminAuth`, () => {
let m: BindingMocks;
const run = async () => {
const res = makeRes();
const next = vi.fn();
await middleware(makeReq(), res, next);
return {res, next};
};
beforeEach(() => {
m = mocks();
m.getSession.mockReset();
m.loadAccess.mockReset();
m.checkSession.mockReset();
});
it('responds 401 and does not call next() without a session', async () => {
m.getSession.mockResolvedValue(null);
const {res, next} = await run();
expect(res.status).toHaveBeenCalledWith(401);
expect(next).not.toHaveBeenCalled();
});
it(`responds 403 for a signed-in user who only has ${otherApp}`, async () => {
m.getSession.mockResolvedValue({user: {id: 'u1'}});
m.loadAccess.mockResolvedValue(userWith(otherApp));
const {res, next} = await run();
expect(res.status).toHaveBeenCalledWith(403);
expect(next).not.toHaveBeenCalled();
});
it('responds 403 for a disabled user who still holds the permission', async () => {
m.getSession.mockResolvedValue({user: {id: 'u1'}});
m.loadAccess.mockResolvedValue({...userWith(app), disabled: true});
const {res, next} = await run();
expect(res.status).toHaveBeenCalledWith(403);
expect(next).not.toHaveBeenCalled();
});
it('sets res.locals.admin and calls next() with the permission', async () => {
m.getSession.mockResolvedValue({user: {id: 'u1'}});
m.loadAccess.mockResolvedValue(userWith(app));
const {res, next} = await run();
expect(next).toHaveBeenCalled();
expect(res.locals.admin).toMatchObject({id: 'u1', email: 'a@nachklang.art', displayName: 'Anna Admin'});
});
// Weaker than it looks and kept deliberately: neither module imports
// checkSession any more, so this cannot fail today. It is a tripwire for
// the change that would matter - someone reintroducing a header-session
// fallback "just for the calendar users who have not been invited yet",
// which is exactly the shortcut the cutover exists to close.
it('never falls back to a calendar header session', async () => {
m.getSession.mockResolvedValue(null);
await run();
expect(m.checkSession).not.toHaveBeenCalled();
});
});
};
+2 -36
View File
@@ -97,37 +97,7 @@ describe('PUT /admin/users/:id/permissions', () => {
const res = await request(makeApp('me')).put('/admin/users/other/permissions').send({apps: ['tickets']}); const res = await request(makeApp('me')).put('/admin/users/other/permissions').send({apps: ['tickets']});
expect(res.status).toBe(200); expect(res.status).toBe(200);
expect(service.setPermissionsGuarded).toHaveBeenCalledWith( expect(service.setPermissionsGuarded).toHaveBeenCalledWith('other', ['tickets'], 'me');
'other',
[{app: 'tickets', role: 'access'}],
'me'
);
});
it('accepts the richer {permissions} body', async () => {
service.loadAccess.mockResolvedValue({id: 'other', disabled: false, apps: []});
const res = await request(makeApp('me'))
.put('/admin/users/other/permissions')
.send({permissions: [{app: 'tickets', role: 'access'}]});
expect(res.status).toBe(200);
expect(service.setPermissionsGuarded).toHaveBeenCalledWith(
'other',
[{app: 'tickets', role: 'access'}],
'me'
);
});
it('rejects a role that does not exist', async () => {
service.loadAccess.mockResolvedValue({id: 'other', disabled: false, apps: []});
const res = await request(makeApp('me'))
.put('/admin/users/other/permissions')
.send({permissions: [{app: 'tickets', role: 'refund'}]});
expect(res.status).toBe(400);
expect(service.setPermissionsGuarded).not.toHaveBeenCalled();
}); });
it('allows granting permissions to someone who has none', async () => { it('allows granting permissions to someone who has none', async () => {
@@ -136,11 +106,7 @@ describe('PUT /admin/users/:id/permissions', () => {
const res = await request(makeApp('me')).put('/admin/users/other/permissions').send({apps: ['feedback', 'tickets']}); const res = await request(makeApp('me')).put('/admin/users/other/permissions').send({apps: ['feedback', 'tickets']});
expect(res.status).toBe(200); expect(res.status).toBe(200);
expect(service.setPermissionsGuarded).toHaveBeenCalledWith( expect(service.setPermissionsGuarded).toHaveBeenCalledWith('other', ['feedback', 'tickets'], 'me');
'other',
[{app: 'feedback', role: 'access'}, {app: 'tickets', role: 'access'}],
'me'
);
}); });
}); });
+81 -16
View File
@@ -1,23 +1,88 @@
import {vi, type Mock} from 'vitest'; import {vi, describe, it, expect, beforeEach, type Mock} from 'vitest';
import {Request, Response} from 'express';
vi.mock('../../src/models/calendar/users/users.service.js', () => ({ vi.mock('../../src/models/calendar/users/users.service.js', () => ({
checkSession: vi.fn() checkSession: vi.fn()
})); }));
vi.mock('../../src/models/admin/admin.auth.js', () => ({
auth: {api: {getSession: vi.fn()}}
}));
vi.mock('../../src/models/admin/users/users.admin.service.js', () => ({
loadAccess: vi.fn()
}));
import * as UserService from '../../src/models/calendar/users/users.service.js'; import * as UserService from '../../src/models/calendar/users/users.service.js';
import {auth} from '../../src/models/admin/admin.auth.js'; import {requireAdminAuth, sessionHeaderAuthenticator} from '../../src/models/feedback/feedback.auth.js';
import * as UsersService from '../../src/models/admin/users/users.admin.service.js';
import {requireAdminAuth} from '../../src/models/feedback/feedback.auth.js';
import {describeAdminBinding} from '../admin/auth-binding.js';
describeAdminBinding('feedback', 'tickets', requireAdminAuth, () => ({ const mockCheckSession = UserService.checkSession as Mock;
getSession: auth.api.getSession as unknown as Mock,
loadAccess: UsersService.loadAccess as Mock, const makeReq = (headers: Record<string, string>): Request => {
checkSession: UserService.checkSession as Mock return {
})); header: (name: string) => headers[name],
ip: '203.0.113.42'
} as unknown as Request;
};
const makeRes = (): Response => {
const res: any = {};
res.status = vi.fn().mockReturnValue(res);
res.send = vi.fn().mockReturnValue(res);
res.locals = {};
return res as Response;
};
describe('sessionHeaderAuthenticator', () => {
beforeEach(() => mockCheckSession.mockReset());
it('returns null when headers are missing', async () => {
const identity = await sessionHeaderAuthenticator(makeReq({}));
expect(identity).toBeNull();
expect(mockCheckSession).not.toHaveBeenCalled();
});
it('returns null when checkSession finds no user', async () => {
mockCheckSession.mockResolvedValue(null);
const identity = await sessionHeaderAuthenticator(makeReq({'X-Session-Id': '1', 'X-Session-Key': 'k'}));
expect(identity).toBeNull();
});
it('returns null for a valid session on an inactive account', async () => {
mockCheckSession.mockResolvedValue({userId: 1, email: 'a@nachklang.art', fullName: 'A', isActive: false});
const identity = await sessionHeaderAuthenticator(makeReq({'X-Session-Id': '1', 'X-Session-Key': 'k'}));
expect(identity).toBeNull();
});
it('returns the identity for a valid session on an active account', async () => {
mockCheckSession.mockResolvedValue({userId: 1, email: 'a@nachklang.art', fullName: 'Anna Admin', isActive: true});
const identity = await sessionHeaderAuthenticator(makeReq({'X-Session-Id': '1', 'X-Session-Key': 'k'}));
expect(identity).toEqual({id: '1', email: 'a@nachklang.art', displayName: 'Anna Admin'});
});
it('passes the session id and key from headers through to checkSession, never from query params', async () => {
mockCheckSession.mockResolvedValue({userId: 1, email: 'a@nachklang.art', fullName: 'A', isActive: true});
await sessionHeaderAuthenticator(makeReq({'X-Session-Id': '42', 'X-Session-Key': 'sekret'}));
expect(mockCheckSession).toHaveBeenCalledWith('42', 'sekret', '203.0.113.42');
});
});
describe('requireAdminAuth', () => {
beforeEach(() => mockCheckSession.mockReset());
it('responds 401 and does not call next() when unauthenticated', async () => {
mockCheckSession.mockResolvedValue(null);
const req = makeReq({});
const res = makeRes();
const next = vi.fn();
await requireAdminAuth(req, res, next);
expect(res.status).toHaveBeenCalledWith(401);
expect(next).not.toHaveBeenCalled();
});
it('sets res.locals.admin and calls next() when authenticated', async () => {
mockCheckSession.mockResolvedValue({userId: 1, email: 'a@nachklang.art', fullName: 'Anna Admin', isActive: true});
const req = makeReq({'X-Session-Id': '1', 'X-Session-Key': 'k'});
const res = makeRes();
const next = vi.fn();
await requireAdminAuth(req, res, next);
expect(next).toHaveBeenCalled();
expect(res.locals.admin).toEqual({id: '1', email: 'a@nachklang.art', displayName: 'Anna Admin'});
});
});
+16 -33
View File
@@ -9,8 +9,7 @@ import {
createAndAcceptInvitation, createAndAcceptInvitation,
resetDatabase, resetDatabase,
sessionCookieFrom, sessionCookieFrom,
SESSION_COOKIE, SESSION_COOKIE
accessTo
} from './helpers.js'; } from './helpers.js';
/** /**
@@ -67,7 +66,7 @@ describe('invitation acceptance', () => {
}); });
it('sets the session cookie under the configured prefix', async () => { it('sets the session cookie under the configured prefix', async () => {
const invitation = await InvitationsService.createInvitation('b@nachklang.art', 'B', accessTo('feedback'), null); const invitation = await InvitationsService.createInvitation('b@nachklang.art', 'B', ['feedback'], null);
const res = await request(app) const res = await request(app)
.post('/admin/auth/invitations/accept') .post('/admin/auth/invitations/accept')
@@ -90,7 +89,7 @@ describe('invitation acceptance', () => {
}); });
it('previews an invitation without revealing the granted apps', async () => { it('previews an invitation without revealing the granted apps', async () => {
const invitation = await InvitationsService.createInvitation('d@nachklang.art', 'D', accessTo('admin'), null); const invitation = await InvitationsService.createInvitation('d@nachklang.art', 'D', ['admin'], null);
const res = await request(app) const res = await request(app)
.post('/admin/auth/invitations/preview') .post('/admin/auth/invitations/preview')
@@ -101,7 +100,7 @@ describe('invitation acceptance', () => {
}); });
it('answers an unknown token exactly like an expired one', async () => { it('answers an unknown token exactly like an expired one', async () => {
const invitation = await InvitationsService.createInvitation('e@nachklang.art', 'E', accessTo('feedback'), null); const invitation = await InvitationsService.createInvitation('e@nachklang.art', 'E', ['feedback'], null);
await InvitationsService.revokeInvitation(invitation.id); await InvitationsService.revokeInvitation(invitation.id);
const unknown = await request(app).post('/admin/auth/invitations/preview').send({token: 'no-such-token'}); const unknown = await request(app).post('/admin/auth/invitations/preview').send({token: 'no-such-token'});
@@ -112,7 +111,7 @@ describe('invitation acceptance', () => {
}); });
it('cannot be redeemed twice', async () => { it('cannot be redeemed twice', async () => {
const invitation = await InvitationsService.createInvitation('f@nachklang.art', 'F', accessTo('feedback'), null); const invitation = await InvitationsService.createInvitation('f@nachklang.art', 'F', ['feedback'], null);
const first = await request(app) const first = await request(app)
.post('/admin/auth/invitations/accept') .post('/admin/auth/invitations/accept')
@@ -126,7 +125,7 @@ describe('invitation acceptance', () => {
}); });
it('rejects an expired invitation', async () => { it('rejects an expired invitation', async () => {
const invitation = await InvitationsService.createInvitation('g@nachklang.art', 'G', accessTo('feedback'), null); const invitation = await InvitationsService.createInvitation('g@nachklang.art', 'G', ['feedback'], null);
// Reach past the service to age it: there is deliberately no API for this. // Reach past the service to age it: there is deliberately no API for this.
const {NachklangAdminDB} = await import('../../src/models/admin/Admin.db.js'); const {NachklangAdminDB} = await import('../../src/models/admin/Admin.db.js');
await NachklangAdminDB.db await NachklangAdminDB.db
@@ -143,7 +142,7 @@ describe('invitation acceptance', () => {
}); });
it('rejects a password below the minimum length', async () => { it('rejects a password below the minimum length', async () => {
const invitation = await InvitationsService.createInvitation('h@nachklang.art', 'H', accessTo('feedback'), null); const invitation = await InvitationsService.createInvitation('h@nachklang.art', 'H', ['feedback'], null);
const res = await request(app) const res = await request(app)
.post('/admin/auth/invitations/accept') .post('/admin/auth/invitations/accept')
@@ -221,32 +220,16 @@ describe('requireAppAccess', () => {
expect(Array.isArray(res.body)).toBe(true); expect(Array.isArray(res.body)).toBe(true);
}); });
// The step 4 cutover (2026-09-06): the feedback and tickets admin areas now // Step 2 deliberately does NOT swap the feedback and tickets authenticators:
// sit behind this same gate, so one sign-in reaches every app the user has a // they still authenticate against the legacy calendar sessions, so an admin
// permission for - and reaches no further. Until step 4 these two returned // cookie means nothing to them yet. This asserts that boundary rather than
// 401 for an admin cookie, because each module still ran its own header // the end state - when step 4 lands, these two expectations become 200/403
// session against the calendar users table. // and this comment goes away.
it('lets an admin cookie into the feedback and tickets admin areas', async () => { it('leaves the feedback and tickets admin areas on their legacy authenticator', async () => {
const user = await createAndAcceptInvitation(app, 'o@nachklang.art', 'O', ['feedback', 'tickets']); const user = await createAndAcceptInvitation(app, 'o@nachklang.art', 'O', ['feedback', 'tickets']);
expect((await user.agent.get('/feedback/admin/me')).status).toBe(200); expect((await user.agent.get('/feedback/admin/me')).status).toBe(401);
expect((await user.agent.get('/tickets/admin/me')).status).toBe(200); expect((await user.agent.get('/tickets/admin/me')).status).toBe(401);
});
it('403s each app separately for a user who only holds the other one', async () => {
const user = await createAndAcceptInvitation(app, 'q@nachklang.art', 'Q', ['feedback']);
expect((await user.agent.get('/feedback/admin/me')).status).toBe(200);
expect((await user.agent.get('/tickets/admin/me')).status).toBe(403);
});
it('401s the feedback and tickets admin areas for a legacy header session', async () => {
const res = await request(app)
.get('/feedback/admin/me')
.set('X-Session-Id', '1')
.set('X-Session-Key', 'whatever');
expect(res.status).toBe(401);
}); });
}); });
@@ -276,7 +259,7 @@ describe('origin checks', () => {
describe('the session cookie is not readable by scripts', () => { describe('the session cookie is not readable by scripts', () => {
it('is HttpOnly and SameSite=Lax', async () => { it('is HttpOnly and SameSite=Lax', async () => {
const invitation = await InvitationsService.createInvitation('r@nachklang.art', 'R', accessTo('feedback'), null); const invitation = await InvitationsService.createInvitation('r@nachklang.art', 'R', ['feedback'], null);
const res = await request(app) const res = await request(app)
.post('/admin/auth/invitations/accept') .post('/admin/auth/invitations/accept')
.send({token: invitation.token, password: 'devpassword123'}); .send({token: invitation.token, password: 'devpassword123'});
+3 -3
View File
@@ -5,7 +5,7 @@ import {createApp} from '../../src/app.factory.js';
import * as InvitationsService from '../../src/models/admin/invitations/invitations.service.js'; import * as InvitationsService from '../../src/models/admin/invitations/invitations.service.js';
import * as UsersService from '../../src/models/admin/users/users.admin.service.js'; import * as UsersService from '../../src/models/admin/users/users.admin.service.js';
import {bootstrapAdmin} from '../../src/models/admin/admin.bootstrap.js'; import {bootstrapAdmin} from '../../src/models/admin/admin.bootstrap.js';
import {accessTo, closeDatabase, createAndAcceptInvitation, resetDatabase} from './helpers.js'; import {closeDatabase, createAndAcceptInvitation, resetDatabase} from './helpers.js';
let app: Application; let app: Application;
@@ -185,7 +185,7 @@ describe('invitations', () => {
it('invalidates the previous link on resend', async () => { it('invalidates the previous link on resend', async () => {
const {agent} = await signedInAdmin(); const {agent} = await signedInAdmin();
const original = await InvitationsService.createInvitation('new@nachklang.art', 'New', accessTo('feedback'), null); const original = await InvitationsService.createInvitation('new@nachklang.art', 'New', ['feedback'], null);
const resent = await agent.post(`/admin/invitations/${original.id}/resend`); const resent = await agent.post(`/admin/invitations/${original.id}/resend`);
expect(resent.status).toBe(200); expect(resent.status).toBe(200);
@@ -198,7 +198,7 @@ describe('invitations', () => {
it('revokes an invitation', async () => { it('revokes an invitation', async () => {
const {agent} = await signedInAdmin(); const {agent} = await signedInAdmin();
const invitation = await InvitationsService.createInvitation('new@nachklang.art', 'New', accessTo('feedback'), null); const invitation = await InvitationsService.createInvitation('new@nachklang.art', 'New', ['feedback'], null);
expect((await agent.delete(`/admin/invitations/${invitation.id}`)).status).toBe(204); expect((await agent.delete(`/admin/invitations/${invitation.id}`)).status).toBe(204);
expect((await agent.delete(`/admin/invitations/${invitation.id}`)).status).toBe(404); expect((await agent.delete(`/admin/invitations/${invitation.id}`)).status).toBe(404);
+3 -12
View File
@@ -3,7 +3,7 @@ import type {Application} from 'express';
import request from 'supertest'; import request from 'supertest';
import {NachklangAdminDB} from '../../src/models/admin/Admin.db.js'; import {NachklangAdminDB} from '../../src/models/admin/Admin.db.js';
import * as InvitationsService from '../../src/models/admin/invitations/invitations.service.js'; import * as InvitationsService from '../../src/models/admin/invitations/invitations.service.js';
import {ACCESS_ROLE, AppName, AppPermission, toPermissions} from '../../src/models/admin/admin.schema.js'; import {AppName} from '../../src/models/admin/admin.schema.js';
const db = NachklangAdminDB.db; const db = NachklangAdminDB.db;
@@ -44,13 +44,10 @@ export const createAndAcceptInvitation = async (
app: Application, app: Application,
email: string, email: string,
name: string, name: string,
// Takes the shorthand as well as the full form: most tests only care that apps: AppName[],
// someone can open an app, and `['tickets']` says that with less noise.
grants: (AppName | AppPermission)[],
password = 'devpassword123' password = 'devpassword123'
) => { ) => {
const permissions = toPermissions(grants) ?? []; const invitation = await InvitationsService.createInvitation(email, name, apps, null);
const invitation = await InvitationsService.createInvitation(email, name, permissions, null);
const agent = request.agent(app); const agent = request.agent(app);
const res = await agent const res = await agent
@@ -69,9 +66,3 @@ export const cookieHeader = (res: request.Response): string[] => {
export const sessionCookieFrom = (res: request.Response): string | undefined => { export const sessionCookieFrom = (res: request.Response): string | undefined => {
return cookieHeader(res).find(cookie => cookie.startsWith(SESSION_COOKIE)); return cookieHeader(res).find(cookie => cookie.startsWith(SESSION_COOKIE));
}; };
/** `accessTo('feedback')` reads better than the (app, role) literal in tests
* that only care that someone can open an app. */
export const accessTo = (...apps: AppName[]): AppPermission[] => {
return apps.map(app => ({app, role: ACCESS_ROLE}));
};
-23
View File
@@ -1,23 +0,0 @@
import {vi, type Mock} from 'vitest';
vi.mock('../../src/models/calendar/users/users.service.js', () => ({
checkSession: vi.fn()
}));
vi.mock('../../src/models/admin/admin.auth.js', () => ({
auth: {api: {getSession: vi.fn()}}
}));
vi.mock('../../src/models/admin/users/users.admin.service.js', () => ({
loadAccess: vi.fn()
}));
import * as UserService from '../../src/models/calendar/users/users.service.js';
import {auth} from '../../src/models/admin/admin.auth.js';
import * as UsersService from '../../src/models/admin/users/users.admin.service.js';
import {requireAdminAuth} from '../../src/models/tickets/tickets.auth.js';
import {describeAdminBinding} from '../admin/auth-binding.js';
describeAdminBinding('tickets', 'feedback', requireAdminAuth, () => ({
getSession: auth.api.getSession as unknown as Mock,
loadAccess: UsersService.loadAccess as Mock,
checkSession: UserService.checkSession as Mock
}));