Put the feedback and tickets admin areas behind the shared identity
feedback.auth.ts and tickets.auth.ts each become one binding to requireAppAccess. Everything downstream was already written against requireAdminAuth and res.locals.admin, and both still mean what they meant, so no router or service changed. What changed is the policy: an activated @nachklang.art account is no longer sufficient, an explicit per-app permission is. Three things followed from that and are not obvious from the diff: - APP_ORIGINS gets a production default. It feeds better-auth's trustedOrigins, and this is the first time the tickets and feedback origins matter there - before, the only browser origin that ever reached /admin/auth was the admin app itself. An origin missing from that list fails in a way that is easy to misread: sign-in works, the app works, and only sign-out returns an origin error. - Nothing reads X-Session-* any more; these two files were the last readers, and the calendar module passes its session in query parameters. The headers stay in the CORS allowedHeaders only so a browser still running a pre-cutover bundle gets a clean 401 rather than a preflight failure, and can come out once both frontends are deployed. - 40 admin operations documented a required X-Session-Id/X-Session-Key in swagger. They now declare the AdminSessionCookie scheme the admin module already defined, and each documents a 403 next to its 401. The integration assertions flip as their own comment predicted: one admin cookie opens both /feedback/admin/me and /tickets/admin/me, a user holding only feedback gets 200 and 403 respectively, and a legacy header session gets 401. The unit test that covered the old header authenticator is replaced by one asserting each module is bound to its own app and that neither consults the calendar users service. 163 unit tests and 43 integration tests green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,78 +1,38 @@
|
||||
import express from 'express';
|
||||
import * as UserService from '../calendar/users/users.service.js';
|
||||
import {sendServerError} from './feedback.errors.js';
|
||||
import {requireAppAccess} from '../admin/admin.middleware.js';
|
||||
|
||||
/**
|
||||
* This file is the ONLY place in the feedback module that knows how admin
|
||||
* authentication works today. No route handler and no service outside this
|
||||
* file may import users.service, read session headers, or touch bcrypt.
|
||||
* authentication works. No route handler and no service outside this file may
|
||||
* read session headers or resolve a user itself.
|
||||
*
|
||||
* Today: reuses the existing Calendar users/sessions mechanism. Any
|
||||
* activated @nachklang.art account may administer feedback — no roles.
|
||||
* Migrating to Keycloak later means writing a keycloakJwtAuthenticator
|
||||
* below and changing the one `activeAuthenticator` binding (plus the
|
||||
* frontend's login route handler) — nothing else in the feedback module
|
||||
* needs to change.
|
||||
* Today: the shared admin identity in `src/models/admin/`. A session cookie
|
||||
* set by /admin/auth on admin.nachklang.art, plus a `feedback` permission on
|
||||
* the account. Both are re-checked on every request, so disabling a user or
|
||||
* taking their feedback permission away takes effect immediately.
|
||||
*
|
||||
* Explicitly forbidden: accepting sessionId/sessionKey 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.
|
||||
* Headers only.
|
||||
* Before 2026-09-06 this was a header session against the calendar users
|
||||
* table, and any activated @nachklang.art account could administer feedback.
|
||||
* That is why the swap is a one-line binding: everything downstream only ever
|
||||
* saw `requireAdminAuth` and `res.locals.admin`, and both still mean what
|
||||
* they meant. What changed is that access is now granted per user rather than
|
||||
* 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 only thing the rest of the feedback module knows about an admin. The
|
||||
// shared middleware puts a superset of this on res.locals.admin.
|
||||
export interface AdminIdentity {
|
||||
id: string;
|
||||
email: 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:
|
||||
// res.locals.admin = AdminIdentity, calls next(). On failure: 401.
|
||||
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);
|
||||
}
|
||||
};
|
||||
// res.locals.admin = AdminAccess (an AdminIdentity plus permissions), calls
|
||||
// next(). On failure: 401 when not signed in, 403 when signed in without the
|
||||
// feedback permission.
|
||||
export const requireAdminAuth = requireAppAccess('feedback');
|
||||
|
||||
Reference in New Issue
Block a user