Put the feedback and tickets admin areas behind the shared identity (#13)

Reviewed-on: #13
Co-authored-by: Patrick Müller <mail@pmueller.me>
Co-committed-by: Patrick Müller <mail@pmueller.me>

test/admin/admin.config.test.ts

@@ -78,6 +78,40 @@ describe('CLIENT_IP_HEADERS', () => {
 	});
 });
 
+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';

test/admin/auth-binding.ts

@@ -0,0 +1,118 @@
+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();
+		});
+	});
+};

test/feedback/feedback.auth.test.ts

@@ -1,88 +1,23 @@
-import {vi, describe, it, expect, beforeEach, type Mock} from 'vitest';
-import {Request, Response} from 'express';
+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 {requireAdminAuth, sessionHeaderAuthenticator} from '../../src/models/feedback/feedback.auth.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/feedback/feedback.auth.js';
+import {describeAdminBinding} from '../admin/auth-binding.js';
 
-const mockCheckSession = UserService.checkSession as Mock;
-
-const makeReq = (headers: Record<string, string>): Request => {
-	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'});
-	});
-});
+describeAdminBinding('feedback', 'tickets', requireAdminAuth, () => ({
+	getSession: auth.api.getSession as unknown as Mock,
+	loadAccess: UsersService.loadAccess as Mock,
+	checkSession: UserService.checkSession as Mock
+}));

test/integration/admin.auth.test.ts

@@ -221,16 +221,32 @@ describe('requireAppAccess', () => {
 		expect(Array.isArray(res.body)).toBe(true);
 	});
 
-	// Step 2 deliberately does NOT swap the feedback and tickets authenticators:
-	// they still authenticate against the legacy calendar sessions, so an admin
-	// cookie means nothing to them yet. This asserts that boundary rather than
-	// the end state - when step 4 lands, these two expectations become 200/403
-	// and this comment goes away.
-	it('leaves the feedback and tickets admin areas on their legacy authenticator', async () => {
+	// The step 4 cutover (2026-09-06): the feedback and tickets admin areas now
+	// sit behind this same gate, so one sign-in reaches every app the user has a
+	// permission for - and reaches no further. Until step 4 these two returned
+	// 401 for an admin cookie, because each module still ran its own header
+	// session against the calendar users table.
+	it('lets an admin cookie into the feedback and tickets admin areas', async () => {
 		const user = await createAndAcceptInvitation(app, 'o@nachklang.art', 'O', ['feedback', 'tickets']);
 
-		expect((await user.agent.get('/feedback/admin/me')).status).toBe(401);
-		expect((await user.agent.get('/tickets/admin/me')).status).toBe(401);
+		expect((await user.agent.get('/feedback/admin/me')).status).toBe(200);
+		expect((await user.agent.get('/tickets/admin/me')).status).toBe(200);
+	});
+
+	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);
 	});
 });
 

test/tickets/tickets.auth.test.ts

@@ -0,0 +1,23 @@
+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
+}));