Compare commits

...
Author SHA1 Message Date
PaddyandClaude Opus 5.5 bfb8f2ff9c Add a read-only role per app for calendar, feedback and tickets
A grant can now be `read` instead of `access`: requireAppAccess lets it
through for GET/HEAD/OPTIONS and answers 403 to every other method, so
new write routes are covered without being marked. `read` and `access`
on the same app are rejected together; user management has no read role.
/tickets/admin/me and /feedback/admin/me report `readOnly`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-29 23:24:28 +01:00
Paddy bc865813cb Merge pull request 'Fix issue where email would still be required for personalized vouchers in the backend' (#19) from feature/optional-personalized-voucher-email into master
Jenkins Production Deployment
Reviewed-on: #19
2026-09-21 21:01:40 +00:00
Paddy e8f971b54e Fix issue where email would still be required for personalized vouchers in the backend 2026-09-21 23:01:22 +02:00
11 changed files with 383 additions and 15 deletions

No files matched your search

+3 -2
View File
@@ -47,8 +47,9 @@ 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
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
`(user_id, app, role)` so one user can hold several roles per app. `access` is the only role
today and means "may use this app at all"; `APP_ROLES` in `admin.schema.ts` is the contract,
`(user_id, app, role)` so one user can hold several roles per app. Two roles exist: `access`
(full access) and `read` (read-only, not for `admin`) - see `docs/admin-roles.md`, including
how `requireAppAccess` enforces `read` by HTTP method; `APP_ROLES` in `admin.schema.ts` is the contract,
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
+73
View File
@@ -0,0 +1,73 @@
# Admin roles: full access and read-only
Added 2026-10-29. A user's permission is a list of **(app, role)** grants in
`user_app_permissions` (see `src/models/admin/admin.schema.ts`). Each app can now be granted at
one of two levels:
| Role | UI label (admin app) | Apps | Meaning |
|----------|----------------------|-------------------------------|--------------------------------------------|
| `access` | Vollzugriff | calendar, feedback, tickets, admin | Everything in that app, as before. |
| `read` | Nur lesen | calendar, feedback, tickets | See everything, change nothing. |
Rules:
- **`access` and `read` on the same app are mutually exclusive.** `toPermissions` returns `null`
for a list holding both, so `PUT /admin/users/:id/permissions` and `POST /admin/invitations`
answer 400. There is no "higher one wins".
- **`admin` (user management) has no `read` role.** Viewing it means viewing other people's
email addresses, IPs and sessions; that is a different trust level from viewing concerts.
- Existing grants were not migrated: every pre-existing row is `access` and still means full
access.
## Enforcement
`requireAppAccess(app)` (no role argument - which is how every app router uses it) does, after
the existing session/disabled/has-any-grant checks:
- `GET`, `HEAD`, `OPTIONS` - allowed for any grant on the app.
- anything else - allowed only with `access` (`canWrite`). A `read`-only user gets
`403 {status: 'FORBIDDEN', message: 'Du hast für diesen Bereich nur Lesezugriff.'}`.
This is by method on purpose: a write route added later is covered without being marked. It
relies on one convention that **must stay true**: admin routes read with GET and change with
POST/PUT/PATCH/DELETE. When this was built, every GET handler in tickets, feedback and calendar
was checked to call only read functions (no SQL writes, no mail). A GET with a side effect, or a
POST that only reads, breaks the read-only role in one direction or the other.
Consequences worth knowing:
- "Resend confirmation email" (tickets) is a POST and therefore not available read-only - by
decision: it has an effect outside the system.
- CSV exports and reports are GETs and therefore available read-only - also by decision.
Read-only means all data, including personal data. A restricted variant would be a new role.
- `requireAppAccess(app, role)` with an explicit role does **not** apply the method rule: the
caller has already named exactly the grant it needs.
## Telling the frontend
- `GET /tickets/admin/me` and `GET /feedback/admin/me` return `readOnly: boolean` alongside
`email` and `fullName`. Frontends hide write controls when it is `true`; the API refuses the
writes regardless.
- `GET /admin/me` returns the full `permissions` list; `isReadOnly(permissions, app)` in
`admin.schema.ts` is the same rule.
## Calendar
The API side is done: calendar writes sit behind `requireAppAccess('calendar')` and so refuse a
`read`-only user, and the optional-cookie reads (`/:calendar/json`, `/ical`) check
`access.apps.includes('calendar')`, which a `read` grant satisfies - read-only users see the
restricted calendars.
The current Angular frontend (`Calendar_Frontend`) was deliberately **not** changed, because it
is being rewritten in Next.js. Until then a read-only calendar user sees the edit buttons and
gets the 403 above when using them. Contract for the rewrite: there is no `/calendar/.../me`
with `readOnly`; read `GET /admin/me` and compute `isReadOnly(permissions, 'calendar')` - or add
a calendar `/me` mirroring tickets/feedback.
## Adding a finer role later
`canWrite` checks for `access` specifically, so a new role (e.g. `tickets:redeem`) grants no
writes through the method rule. Watch out: tickets and feedback apply the role-less
`requireAdminAuth` once at the top of their admin router, so that gate would already refuse the
new role's POST before any route-level `requireAppAccess(app, 'redeem')` runs. A finer role
therefore needs `canWrite` (or the top-level gate) taught about it, not just a route-level check.
+15 -1
View File
@@ -2,9 +2,14 @@ import express from 'express';
import {fromNodeHeaders} from 'better-auth/node';
import {auth} from './admin.auth.js';
import * as UsersService from './users/users.admin.service.js';
import {AppName, AppPermission, AppRole} from './admin.schema.js';
import {AppName, AppPermission, AppRole, canWrite} from './admin.schema.js';
import {sendServerError} from './admin.errors.js';
// What a read-only grant may do. Every admin route reads with GET and changes
// with POST/PUT/PATCH/DELETE - keep it that way, or a read-only user either
// loses a view or gains a write.
const READ_METHODS = new Set(['GET', 'HEAD', 'OPTIONS']);
/**
* The one authenticator for every admin area in this API. It replaces
* feedback.auth.ts and tickets.auth.ts, which each re-implemented the same
@@ -131,6 +136,15 @@ export const requireAppAccess = (app: AppName, role?: AppRole): express.RequestH
return;
}
// The read-only level, enforced by method rather than per route so a
// write route added later is covered without being marked. Only the
// role-less form applies it: a caller asking for a specific role has
// already said exactly what is required.
if (role === undefined && !READ_METHODS.has(req.method) && !canWrite(access.permissions, app)) {
forbidden(res, 'Du hast für diesen Bereich nur Lesezugriff.');
return;
}
res.locals.admin = access;
next();
} catch (e: any) {
+36 -4
View File
@@ -35,13 +35,26 @@ export const isAppName = (value: unknown): value is AppName => {
*/
export const ACCESS_ROLE = 'access';
/**
* Read-only access to an app: everything `access` may see, nothing it may
* change. requireAppAccess enforces it by HTTP method - a user holding only
* this role gets GET/HEAD/OPTIONS and a 403 for anything else - so a new write
* route is covered without anyone remembering to mark it.
*
* It and `access` are mutually exclusive on one app (toPermissions rejects a
* list holding both), because "full access and also read-only" has no sensible
* reading. The `admin` app deliberately has no read role: viewing user
* management means viewing other people's addresses, IPs and sessions.
*/
export const READ_ROLE = 'read';
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],
calendar: [ACCESS_ROLE, READ_ROLE],
feedback: [ACCESS_ROLE, READ_ROLE],
tickets: [ACCESS_ROLE, READ_ROLE],
admin: [ACCESS_ROLE]
};
@@ -85,7 +98,7 @@ export const toPermissions = (value: unknown): AppPermission[] | null => {
}
const seen = new Set<string>();
return permissions.filter(permission => {
const unique = permissions.filter(permission => {
const key = `${permission.app}:${permission.role}`;
if (seen.has(key)) {
return false;
@@ -93,6 +106,11 @@ export const toPermissions = (value: unknown): AppPermission[] | null => {
seen.add(key);
return true;
});
// Rejected rather than resolved to either level: whichever one we picked,
// the caller asked for something else.
const contradictory = APP_NAMES.some(app => seen.has(`${app}:${ACCESS_ROLE}`) && seen.has(`${app}:${READ_ROLE}`));
return contradictory ? null : unique;
};
/** The distinct apps a permission list grants any access to. */
@@ -100,6 +118,20 @@ export const appsOf = (permissions: AppPermission[]): AppName[] => {
return APP_NAMES.filter(app => permissions.some(permission => permission.app === app));
};
/**
* Whether these grants allow changing anything in `app`. Only `access` does:
* `read` is the read-only level, and a finer role added later grants exactly
* what its own check says, not writes in general.
*/
export const canWrite = (permissions: AppPermission[], app: AppName): boolean => {
return permissions.some(permission => permission.app === app && permission.role === ACCESS_ROLE);
};
/** Holds some grant on `app`, but none that allows changes. */
export const isReadOnly = (permissions: AppPermission[], app: AppName): boolean => {
return appsOf(permissions).includes(app) && !canWrite(permissions, app);
};
export interface UserTable {
id: string;
name: string;
+10 -1
View File
@@ -3,6 +3,7 @@
*/
import express, {Request, Response} from 'express';
import {requireAdminAuth} from '../feedback.auth.js';
import {isReadOnly} from '../../admin/admin.schema.js';
import {sendServerError} from '../feedback.errors.js';
import {eventsAdminRouter} from './events.admin.router.js';
import {songsAdminRouter} from './songs.admin.router.js';
@@ -40,13 +41,21 @@ adminRouter.use(requireAdminAuth);
* type: string
* fullName:
* type: string
* readOnly:
* type: boolean
* description: Holds only the read role for feedback - every write answers 403.
* 401:
* description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/
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,
// Lets the frontend hide what the API would refuse anyway.
readOnly: isReadOnly(res.locals.admin.permissions, 'feedback')
});
});
/**
+19 -1
View File
@@ -1,5 +1,6 @@
import express, {Request, Response} from 'express';
import {requireAdminAuth} from '../tickets.auth.js';
import {isReadOnly} from '../../admin/admin.schema.js';
import {vouchersAdminRouter} from './vouchers.admin.router.js';
import {redemptionsAdminRouter, voucherHistoryRouter} from './redemptions.admin.router.js';
import {eventsAdminRouter} from './events.admin.router.js';
@@ -21,13 +22,30 @@ adminRouter.use(requireAdminAuth);
* responses:
* 200:
* description: Success
* content:
* application/json:
* schema:
* type: object
* properties:
* email:
* type: string
* fullName:
* type: string
* readOnly:
* type: boolean
* description: Holds only the read role for tickets - every write answers 403.
* 401:
* description: Unauthorized
* 403:
* description: Signed in without the permission for this app, or account disabled
*/
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,
// Lets the frontend hide what the API would refuse anyway.
readOnly: isReadOnly(res.locals.admin.permissions, 'tickets')
});
});
adminRouter.use('/vouchers', vouchersAdminRouter);
@@ -129,12 +129,13 @@ vouchersAdminRouter.post('/wildcard', async (req: Request, res: Response) => {
* type: array
* items:
* type: object
* required: [name, email, eventIds]
* required: [name, eventIds]
* properties:
* name:
* type: string
* email:
* type: string
* description: Optional - a guest can supply their own email when redeeming.
* eventIds:
* type: array
* items:
@@ -123,10 +123,10 @@ export const generatePersonalizedBatch = async (rows: PersonalizedRowInput[], cr
throw new Error('at least one row is required');
}
for (const row of rows) {
if (!row.name || !row.email || row.eventIds.length === 0) {
throw new Error('each row requires a name, email, and at least one eligible event');
if (!row.name || row.eventIds.length === 0) {
throw new Error('each row requires a name and at least one eligible event');
}
if (!isValidEmail(row.email)) {
if (row.email && !isValidEmail(row.email)) {
throw new Error(`"${row.email}" does not look like a valid email address`);
}
}
@@ -147,7 +147,7 @@ export const generatePersonalizedBatch = async (rows: PersonalizedRowInput[], cr
codes.push(code);
await conn.query(
'INSERT INTO voucher_codes (code, status, max_guests, prefill_name, prefill_email, batch_id, created_by_email) VALUES (?,?,?,?,?,?,?)',
[code, 'UNUSED', row.maxGuests, row.name, row.email, batchId, createdByEmail]
[code, 'UNUSED', row.maxGuests, row.name, row.email || null, batchId, createdByEmail]
);
for (const eventId of row.eventIds) {
await conn.query('INSERT INTO voucher_code_events (code, event_id) VALUES (?,?)', [code, eventId]);
+59 -1
View File
@@ -15,7 +15,7 @@ import {requireAppAccess, requireSignedIn, resolveAccess} from '../../src/models
const mockGetSession = auth.api.getSession as unknown as Mock;
const mockLoadAccess = UsersService.loadAccess as Mock;
const makeReq = (): Request => ({headers: {cookie: 'nachklang.session_token=abc'}} as unknown as Request);
const makeReq = (method = 'GET'): Request => ({method, headers: {cookie: 'nachklang.session_token=abc'}} as unknown as Request);
const makeRes = (): Response => {
const res: any = {};
@@ -219,4 +219,62 @@ describe('requireAppAccess', () => {
expect(next).toHaveBeenCalled();
});
describe('read-only grant', () => {
const readOnlyUser = {
...activeUser,
permissions: [{app: 'tickets', role: 'read'}],
apps: ['tickets']
};
it.each(['GET', 'HEAD', 'OPTIONS'])('lets %s through', async method => {
mockGetSession.mockResolvedValue({user: {id: 'u1'}});
mockLoadAccess.mockResolvedValue(readOnlyUser);
const res = makeRes();
const next = vi.fn();
await requireAppAccess('tickets')(makeReq(method), res, next);
expect(next).toHaveBeenCalled();
});
it.each(['POST', 'PUT', 'PATCH', 'DELETE'])('403s %s', async method => {
mockGetSession.mockResolvedValue({user: {id: 'u1'}});
mockLoadAccess.mockResolvedValue(readOnlyUser);
const res = makeRes();
const next = vi.fn();
await requireAppAccess('tickets')(makeReq(method), res, next);
expect(res.status).toHaveBeenCalledWith(403);
expect(res.send).toHaveBeenCalledWith(expect.objectContaining({message: 'Du hast für diesen Bereich nur Lesezugriff.'}));
expect(next).not.toHaveBeenCalled();
});
it('does not let full access to another app write here', async () => {
mockGetSession.mockResolvedValue({user: {id: 'u1'}});
mockLoadAccess.mockResolvedValue({
...activeUser,
permissions: [{app: 'tickets', role: 'read'}, {app: 'feedback', role: 'access'}],
apps: ['feedback', 'tickets']
});
const res = makeRes();
const next = vi.fn();
await requireAppAccess('tickets')(makeReq('POST'), res, next);
expect(res.status).toHaveBeenCalledWith(403);
});
it('lets full access write', async () => {
mockGetSession.mockResolvedValue({user: {id: 'u1'}});
mockLoadAccess.mockResolvedValue(activeUser);
const res = makeRes();
const next = vi.fn();
await requireAppAccess('feedback')(makeReq('DELETE'), res, next);
expect(next).toHaveBeenCalled();
});
});
});
+41
View File
@@ -1,9 +1,12 @@
import {describe, expect, it} from 'vitest';
import {
ACCESS_ROLE,
READ_ROLE,
appsOf,
canWrite,
isAppPermission,
isAppRole,
isReadOnly,
toPermissions
} from '../../src/models/admin/admin.schema.js';
@@ -59,6 +62,44 @@ describe('toPermissions', () => {
it('reads an empty list as "no permissions", not as invalid', () => {
expect(toPermissions([])).toEqual([]);
});
it('reads the read-only role', () => {
expect(toPermissions([{app: 'tickets', role: READ_ROLE}])).toEqual([
{app: 'tickets', role: READ_ROLE}
]);
});
it('rejects full and read-only access to the same app together', () => {
expect(toPermissions(['tickets', {app: 'tickets', role: READ_ROLE}])).toBeNull();
});
it('allows full access to one app next to read-only access to another', () => {
expect(toPermissions(['tickets', {app: 'feedback', role: READ_ROLE}])).toEqual([
{app: 'tickets', role: ACCESS_ROLE},
{app: 'feedback', role: READ_ROLE}
]);
});
it('rejects read-only access to user management', () => {
expect(toPermissions([{app: 'admin', role: READ_ROLE}])).toBeNull();
});
});
describe('canWrite / isReadOnly', () => {
it('lets only the access role write', () => {
const permissions = [
{app: 'tickets' as const, role: ACCESS_ROLE},
{app: 'feedback' as const, role: READ_ROLE}
];
expect(canWrite(permissions, 'tickets')).toBe(true);
expect(canWrite(permissions, 'feedback')).toBe(false);
expect(isReadOnly(permissions, 'tickets')).toBe(false);
expect(isReadOnly(permissions, 'feedback')).toBe(true);
});
it('is neither for an app without any grant', () => {
expect(canWrite([], 'calendar')).toBe(false);
expect(isReadOnly([], 'calendar')).toBe(false);
});
});
describe('isAppRole', () => {
+121
View File
@@ -0,0 +1,121 @@
import {describe, it, expect, beforeAll, beforeEach, afterAll} from 'vitest';
import type {Application} from 'express';
import {createApp} from '../../src/app.factory.js';
import {READ_ROLE} from '../../src/models/admin/admin.schema.js';
import {closeDatabase, createAndAcceptInvitation, resetDatabase} from './helpers.js';
/**
* The read-only role end to end: a real session cookie through the real
* middleware into each app's routers. The writes are rejected before any
* handler runs, which is why this suite needs no tickets/feedback/calendar
* database - a 403 here proves the gate held, not that a query failed.
*/
let app: Application;
beforeAll(() => {
app = createApp();
});
beforeEach(async () => {
await resetDatabase();
});
afterAll(async () => {
await closeDatabase();
});
const readOnlyEverywhere = async () => {
return createAndAcceptInvitation(app, 'reader@nachklang.art', 'Reader', [
{app: 'tickets', role: READ_ROLE},
{app: 'feedback', role: READ_ROLE},
{app: 'calendar', role: READ_ROLE}
]);
};
describe('read-only access', () => {
it('reports readOnly on each app\'s /me', async () => {
const {agent} = await readOnlyEverywhere();
const tickets = await agent.get('/tickets/admin/me');
const feedback = await agent.get('/feedback/admin/me');
expect(tickets.status).toBe(200);
expect(tickets.body.readOnly).toBe(true);
expect(feedback.status).toBe(200);
expect(feedback.body.readOnly).toBe(true);
});
it('reports readOnly false for full access', async () => {
const {agent} = await createAndAcceptInvitation(app, 'full@nachklang.art', 'Full', ['tickets']);
const res = await agent.get('/tickets/admin/me');
expect(res.status).toBe(200);
expect(res.body.readOnly).toBe(false);
});
it.each([
['post', '/tickets/admin/vouchers/wildcard'],
['post', '/tickets/admin/redemptions/1/resend-confirmation'],
['patch', '/tickets/admin/redemptions/1'],
['put', '/tickets/admin/events/1/settings'],
['delete', '/feedback/admin/submissions/1'],
['post', '/feedback/admin/events'],
['post', '/calendar/events'],
['delete', '/calendar/events/1']
] as const)('refuses %s %s', async (method, path) => {
const {agent} = await readOnlyEverywhere();
const res = await agent[method](path).send({});
expect(res.status).toBe(403);
expect(res.body.message).toBe('Du hast für diesen Bereich nur Lesezugriff.');
});
it('does not open user management', async () => {
const {agent} = await readOnlyEverywhere();
expect((await agent.get('/admin/users')).status).toBe(403);
});
});
describe('granting read-only access', () => {
const signedInAdmin = () => createAndAcceptInvitation(app, 'admin@nachklang.art', 'Admin', ['admin']);
it('stores the read role per app', async () => {
const {agent} = await signedInAdmin();
const other = await createAndAcceptInvitation(app, 'user@nachklang.art', 'User', ['feedback']);
const res = await agent
.put(`/admin/users/${other.userId}/permissions`)
.send({permissions: [{app: 'tickets', role: READ_ROLE}, {app: 'feedback', role: 'access'}]});
expect(res.status).toBe(200);
expect(res.body.permissions).toEqual(expect.arrayContaining([
{app: 'tickets', role: READ_ROLE},
{app: 'feedback', role: 'access'}
]));
});
it('rejects full and read-only access to the same app', async () => {
const {agent} = await signedInAdmin();
const other = await createAndAcceptInvitation(app, 'user@nachklang.art', 'User', ['feedback']);
const res = await agent
.put(`/admin/users/${other.userId}/permissions`)
.send({permissions: [{app: 'tickets', role: READ_ROLE}, {app: 'tickets', role: 'access'}]});
expect(res.status).toBe(400);
});
it('rejects inviting someone with read-only user management', async () => {
const {agent} = await signedInAdmin();
const res = await agent
.post('/admin/invitations')
.send({email: 'new@nachklang.art', name: 'New', permissions: [{app: 'admin', role: READ_ROLE}]});
expect(res.status).toBe(400);
});
});