Add admin identity module: better-auth, per-app permissions, invitations #12

Merged
Paddy merged 5 commits from feature/admin-auth-module into master 2026-09-06 10:41:55 +00:00
20 changed files with 579 additions and 121 deletions
Showing only changes of commit 33489585a0 - Show all commits
+11 -3
View File
@@ -46,9 +46,17 @@ 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.
Permissions are per app in `user_app_permissions`; `requireAppAccess(app)` in A permission is **(app, role)** in `user_app_permissions`, keyed on
`admin.middleware.ts` is the single authenticator, and it queries the database on every `(user_id, app, role)` so one user can hold several roles per app. `access` is the only role
request (no cookie cache) so disabling a user takes effect at once. `ADMIN_BOOTSTRAP_EMAIL` today and means "may use this app at all"; `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
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
+15 -9
View File
@@ -103,15 +103,21 @@ 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. `role` is reserved for -- what lets someone manage users and invitations. A permission is (app, role);
-- per-app roles later and is 'admin' for every row today. -- `access` is the only role today, and the key admits several per app so finer
-- 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,
`role` VARCHAR(32) NOT NULL DEFAULT 'admin', -- One row per (user, app, role). `access` means "may use this app at all"
-- 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,
PRIMARY KEY (`user_id`, `app`), -- (user_id, app) is the leftmost prefix of this key, so the per-request
-- 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;
@@ -122,7 +128,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,
`apps` JSON NOT NULL, `permissions` 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,
@@ -158,7 +164,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', 'admin'), ('dev-user-0000-0000-0000-000000000001', 'calendar', 'access'),
('dev-user-0000-0000-0000-000000000001', 'feedback', 'admin'), ('dev-user-0000-0000-0000-000000000001', 'feedback', 'access'),
('dev-user-0000-0000-0000-000000000001', 'tickets', 'admin'), ('dev-user-0000-0000-0000-000000000001', 'tickets', 'access'),
('dev-user-0000-0000-0000-000000000001', 'admin', 'admin'); ('dev-user-0000-0000-0000-000000000001', 'admin', 'access');
+11 -5
View File
@@ -114,15 +114,21 @@ 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. `role` is reserved for -- what lets someone manage users and invitations. A permission is (app, role);
-- per-app roles later and is 'admin' for every row today. -- `access` is the only role today, and the key admits several per app so finer
-- 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,
`role` VARCHAR(32) NOT NULL DEFAULT 'admin', -- One row per (user, app, role). `access` means "may use this app at all"
-- 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,
PRIMARY KEY (`user_id`, `app`), -- (user_id, app) is the leftmost prefix of this key, so the per-request
-- 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;
@@ -133,7 +139,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,
`apps` JSON NOT NULL, `permissions` 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
View File
@@ -35,6 +35,11 @@ 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
}); });
}); });
+19 -2
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} from '@better-auth/passkey'; import {passkey, getAuthenticatorName} 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,7 +116,24 @@ 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()
], ],
+2 -1
View File
@@ -1,6 +1,7 @@
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';
@@ -48,7 +49,7 @@ export const bootstrapAdmin = async (): Promise<void> => {
const invitation = await InvitationsService.createInvitation( const invitation = await InvitationsService.createInvitation(
email, email,
'Nachklang Admin', 'Nachklang Admin',
['admin'], [{app: 'admin', role: ACCESS_ROLE}],
null null
); );
+19 -3
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} from './admin.schema.js'; import {AppName, AppPermission, AppRole} from './admin.schema.js';
import {sendServerError} from './admin.errors.js'; import {sendServerError} from './admin.errors.js';
/** /**
@@ -28,6 +28,9 @@ 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[];
} }
@@ -59,6 +62,7 @@ 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
}; };
}; };
@@ -97,8 +101,12 @@ 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): express.RequestHandler => { export const requireAppAccess = (app: AppName, role?: AppRole): express.RequestHandler => {
return async (req, res, next) => { return async (req, res, next) => {
try { try {
const access = await resolveAccess(req); const access = await resolveAccess(req);
@@ -110,7 +118,15 @@ export const requireAppAccess = (app: AppName): express.RequestHandler => {
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;
} }
+85 -3
View File
@@ -19,6 +19,87 @@ 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;
@@ -52,7 +133,7 @@ export interface PasskeyTable {
export interface UserAppPermissionTable { export interface UserAppPermissionTable {
user_id: string; user_id: string;
app: AppName; app: AppName;
role: string; role: AppRole;
granted_by: string | null; granted_by: string | null;
granted_at: Generated<Date>; granted_at: Generated<Date>;
} }
@@ -63,8 +144,9 @@ export interface InvitationTable {
email: string; email: string;
name: string; name: string;
token_hash: string; token_hash: string;
// JSON column holding an AppName[]. // JSON column holding an AppPermission[]. Older rows may hold a plain
apps: string; // AppName[]; `parsePermissions` reads both.
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.apps, null); await UsersService.setPermissions(user.id, invitation.permissions, 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 {isAppName, AppName} from '../admin.schema.js'; import {toPermissions} 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,15 +64,23 @@ invitationsRouter.get('/', async (req: Request, res: Response) => {
* application/json: * application/json:
* schema: * schema:
* type: object * type: object
* required: [email, name, apps] * required: [email, name, permissions]
* properties: * properties:
* email: * email:
* type: string * type: string
* name: * name:
* type: string * type: string
* apps: * permissions:
* 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
* properties:
* app:
* type: string
* role:
* type: string * type: string
* responses: * responses:
* 201: * 201:
@@ -86,10 +94,15 @@ 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;
if (!EMAIL_PATTERN.test(email) || name.length === 0 || !Array.isArray(apps) || !apps.every(isAppName)) { // Same two accepted shapes as PUT /admin/users/:id/permissions.
res.status(400).send({status: 'BAD_REQUEST', message: 'E-Mail, Name und App-Liste sind erforderlich.'}); const permissions = toPermissions(req.body?.permissions ?? req.body?.apps);
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;
} }
@@ -107,7 +120,7 @@ invitationsRouter.post('/', async (req: Request, res: Response) => {
const invitation = await InvitationsService.createInvitation( const invitation = await InvitationsService.createInvitation(
email, email,
name, name,
apps as AppName[], permissions,
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 {AppName, APP_NAMES, isAppName} from '../admin.schema.js'; import {AppPermission, isAppName, isAppPermission, ACCESS_ROLE} 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;
apps: AppName[]; permissions: AppPermission[];
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;
apps: AppName[]; permissions: AppPermission[];
} }
const hashToken = (token: string): string => { const hashToken = (token: string): string => {
@@ -41,11 +41,28 @@ const generateToken = (): string => {
return crypto.randomBytes(32).toString('base64url'); return crypto.randomBytes(32).toString('base64url');
}; };
const parseApps = (value: unknown): AppName[] => { /**
// mysql2 hands back a JSON column already parsed; a driver or column-type * Reads the stored permission list. Two shapes are accepted: the current
// change that turns it into a string must not break the read path. * `[{app, role}]`, and a bare `['tickets', ...]` from before roles existed,
* 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;
return Array.isArray(raw) ? raw.filter(isAppName) : []; if (!Array.isArray(raw)) {
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 => {
@@ -61,12 +78,12 @@ const expiryFromNow = (): Date => {
export const createInvitation = async ( export const createInvitation = async (
email: string, email: string,
name: string, name: string,
apps: AppName[], permissions: AppPermission[],
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 validApps = Array.from(new Set(apps)).filter(app => APP_NAMES.includes(app)); const valid = permissions.filter(isAppPermission);
const id = await db.transaction().execute(async trx => { const id = await db.transaction().execute(async trx => {
await trx await trx
@@ -83,7 +100,7 @@ export const createInvitation = async (
email, email,
name, name,
token_hash: hashToken(token), token_hash: hashToken(token),
apps: JSON.stringify(validApps), permissions: JSON.stringify(valid),
invited_by: invitedBy, invited_by: invitedBy,
created_at: new Date(), created_at: new Date(),
expires_at: expiresAt expires_at: expiresAt
@@ -105,7 +122,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', 'apps']) .select(['id', 'email', 'name', 'permissions'])
.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)
@@ -116,7 +133,7 @@ export const findByToken = async (token: string): Promise<AcceptableInvitation |
return null; return null;
} }
return {id: row.id, email: row.email, name: row.name, apps: parseApps(row.apps)}; return {id: row.id, email: row.email, name: row.name, permissions: parsePermissions(row.permissions)};
}; };
/** Marks the invitation accepted. Conditional on it still being open so two /** Marks the invitation accepted. Conditional on it still being open so two
@@ -149,7 +166,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', 'apps', 'invited_by', 'created_at', 'expires_at']) .select(['id', 'email', 'name', 'permissions', '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())
@@ -160,7 +177,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,
apps: parseApps(row.apps), permissions: parsePermissions(row.permissions),
invitedBy: row.invited_by, invitedBy: row.invited_by,
createdAt: row.created_at, createdAt: row.created_at,
expiresAt: row.expires_at expiresAt: row.expires_at
+23 -8
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 {AppName, isAppName} from '../admin.schema.js'; import {toPermissions} 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,26 +90,40 @@ usersAdminRouter.get('/:userId', async (req: Request, res: Response) => {
* schema: * schema:
* type: object * type: object
* properties: * properties:
* apps: * permissions:
* 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
* properties:
* app:
* type: string * type: string
* enum: [calendar, feedback, tickets, admin] * enum: [calendar, feedback, tickets, admin]
* role:
* type: string
* enum: [access]
* responses: * responses:
* 200: * 200:
* description: Success * description: Success
* 400: * 400:
* description: Invalid app name * description: Invalid app or role
* 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;
if (!Array.isArray(apps) || !apps.every(isAppName)) { // `permissions: [{app, role}]` is the real shape; `apps: ['tickets']` is
res.status(400).send({status: 'BAD_REQUEST', message: 'Ungültige App-Liste.'}); // accepted as shorthand for the same thing at the `access` role, so a
// 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;
} }
@@ -119,7 +133,8 @@ usersAdminRouter.put('/:userId/permissions', async (req: Request, res: Response)
} }
const target = await UsersService.loadAccess(userId); const target = await UsersService.loadAccess(userId);
const losesAdmin = Boolean(target?.apps.includes('admin')) && !(apps as AppName[]).includes('admin'); const keepsAdmin = permissions.some(permission => permission.app === '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
@@ -130,7 +145,7 @@ usersAdminRouter.put('/:userId/permissions', async (req: Request, res: Response)
return; return;
} }
const result = await UsersService.setPermissionsGuarded(userId, apps as AppName[], res.locals.admin.id); const result = await UsersService.setPermissionsGuarded(userId, permissions, 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;
+115 -39
View File
@@ -1,6 +1,15 @@
import {Transaction} from 'kysely'; import {Transaction} from 'kysely';
import {NachklangAdminDB} from '../Admin.db.js'; import {NachklangAdminDB} from '../Admin.db.js';
import {AdminDatabase, AppName, APP_NAMES} from '../admin.schema.js'; import {
AdminDatabase,
AppName,
AppPermission,
AppRole,
ACCESS_ROLE,
appsOf,
isAppName,
isAppRole
} from '../admin.schema.js';
const db = NachklangAdminDB.db; const db = NachklangAdminDB.db;
@@ -18,6 +27,10 @@ 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[];
} }
@@ -27,6 +40,7 @@ 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;
@@ -61,7 +75,8 @@ 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();
@@ -69,16 +84,32 @@ 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),
apps: rows.map(row => row.app).filter((app): app is AppName => app !== null) permissions,
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')
@@ -88,7 +119,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']) .select(['user_id', 'app', 'role'])
.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
@@ -107,26 +138,33 @@ export const listUsers = async (): Promise<UserListEntry[]> => {
.groupBy('userId') .groupBy('userId')
.execute(); .execute();
const appsByUser = new Map<string, AppName[]>(); const permissionsByUser = new Map<string, AppPermission[]>();
for (const row of permissions) { for (const row of permissions) {
const apps = appsByUser.get(row.user_id) || []; if (!isAppRole(row.app, row.role)) {
apps.push(row.app); continue;
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) || [];
return {
id: user.id, id: user.id,
email: user.email, email: user.email,
name: user.name, name: user.name,
apps: appsByUser.get(user.id) || [], permissions: held,
status: user.disabled ? 'deaktiviert' : 'aktiv', apps: appsOf(held),
status: user.disabled ? ('deaktiviert' as const) : ('aktiv' as const),
createdAt: user.createdAt, createdAt: user.createdAt,
lastSignInAt: lastSignInByUser.get(user.id) ?? null lastSignInAt: lastSignInByUser.get(user.id) ?? null
})); };
});
}; };
export const getUserDetail = async (userId: string): Promise<UserDetail | null> => { export const getUserDetail = async (userId: string): Promise<UserDetail | null> => {
@@ -141,7 +179,11 @@ export const getUserDetail = async (userId: string): Promise<UserDetail | null>
} }
const [permissions, sessions, passkeys] = await Promise.all([ const [permissions, sessions, passkeys] = await Promise.all([
db.selectFrom('user_app_permissions').select('app').where('user_id', '=', userId).execute(), db
.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'])
@@ -156,11 +198,14 @@ 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,
apps: permissions.map(row => row.app), permissions: held,
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,
@@ -174,25 +219,51 @@ 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,
apps: AppName[], permissions: AppPermission[],
grantedBy: string | null grantedBy: string | null
): Promise<void> => { ): Promise<void> => {
const unique = Array.from(new Set(apps)).filter(app => APP_NAMES.includes(app)); const valid = validPermissions(permissions);
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 (unique.length > 0) { if (valid.length > 0) {
await trx await trx
.insertInto('user_app_permissions') .insertInto('user_app_permissions')
.values(unique.map(app => ({ .values(permissionRows(userId, valid, grantedBy))
user_id: userId,
app,
role: 'admin',
granted_by: grantedBy,
granted_at: new Date()
})))
.execute(); .execute();
} }
}); });
@@ -217,7 +288,11 @@ 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)
.select(({fn}) => fn.countAll<number>().as('count')) // countDistinct, not countAll: with (user_id, app, role) as the key one
// 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();
@@ -230,10 +305,11 @@ const countActiveAdminsForUpdate = async (trx: Transaction<AdminDatabase>): Prom
*/ */
export const setPermissionsGuarded = async ( export const setPermissionsGuarded = async (
userId: string, userId: string,
apps: AppName[], permissions: AppPermission[],
grantedBy: string | null grantedBy: string | null
): Promise<LastAdminGuardResult> => { ): Promise<LastAdminGuardResult> => {
const unique = Array.from(new Set(apps)).filter(app => APP_NAMES.includes(app)); const valid = validPermissions(permissions);
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
@@ -242,25 +318,20 @@ 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) && !unique.includes('admin'); const losesAdmin = Boolean(target) && !keepsAdmin;
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 (unique.length > 0) { if (valid.length > 0) {
await trx await trx
.insertInto('user_app_permissions') .insertInto('user_app_permissions')
.values(unique.map(app => ({ .values(permissionRows(userId, valid, grantedBy))
user_id: userId,
app,
role: 'admin',
granted_by: grantedBy,
granted_at: new Date()
})))
.execute(); .execute();
} }
@@ -281,6 +352,7 @@ 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();
@@ -298,12 +370,16 @@ 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: 'admin', granted_by: grantedBy, granted_at: new Date()}) .values({user_id: userId, app, role, granted_by: grantedBy, granted_at: new Date()})
.onDuplicateKeyUpdate({role: 'admin'}) // The row already existing is the success case - this is "make sure they
// hold it", not "re-grant it" - so nothing is overwritten and granted_by
// keeps naming whoever granted it first.
.onDuplicateKeyUpdate({role})
.execute(); .execute();
}; };
+6 -1
View File
@@ -79,7 +79,12 @@ describe('bootstrapAdmin', () => {
await bootstrapAdmin(); await bootstrapAdmin();
expect(createInvitation).toHaveBeenCalledWith('boss@nachklang.art', 'Nachklang Admin', ['admin'], null); expect(createInvitation).toHaveBeenCalledWith(
'boss@nachklang.art',
'Nachklang Admin',
[{app: 'admin', role: 'access'}],
null
);
expect(mockMail).toHaveBeenCalled(); expect(mockMail).toHaveBeenCalled();
}); });
+49 -1
View File
@@ -30,6 +30,10 @@ 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']
}; };
@@ -60,6 +64,10 @@ 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.
@@ -127,7 +135,11 @@ 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({...activeUser, apps: ['feedback']}); mockLoadAccess.mockResolvedValue({
...activeUser,
permissions: [{app: 'feedback', role: 'access'}],
apps: ['feedback']
});
const res = makeRes(); const res = makeRes();
const next = vi.fn(); const next = vi.fn();
@@ -171,4 +183,40 @@ 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
@@ -0,0 +1,99 @@
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([]);
});
});
+36 -2
View File
@@ -97,7 +97,37 @@ 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('other', ['tickets'], 'me'); expect(service.setPermissionsGuarded).toHaveBeenCalledWith(
'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 () => {
@@ -106,7 +136,11 @@ 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('other', ['feedback', 'tickets'], 'me'); expect(service.setPermissionsGuarded).toHaveBeenCalledWith(
'other',
[{app: 'feedback', role: 'access'}, {app: 'tickets', role: 'access'}],
'me'
);
}); });
}); });
+9 -8
View File
@@ -9,7 +9,8 @@ import {
createAndAcceptInvitation, createAndAcceptInvitation,
resetDatabase, resetDatabase,
sessionCookieFrom, sessionCookieFrom,
SESSION_COOKIE SESSION_COOKIE,
accessTo
} from './helpers.js'; } from './helpers.js';
/** /**
@@ -66,7 +67,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', ['feedback'], null); const invitation = await InvitationsService.createInvitation('b@nachklang.art', 'B', accessTo('feedback'), null);
const res = await request(app) const res = await request(app)
.post('/admin/auth/invitations/accept') .post('/admin/auth/invitations/accept')
@@ -89,7 +90,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', ['admin'], null); const invitation = await InvitationsService.createInvitation('d@nachklang.art', 'D', accessTo('admin'), null);
const res = await request(app) const res = await request(app)
.post('/admin/auth/invitations/preview') .post('/admin/auth/invitations/preview')
@@ -100,7 +101,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', ['feedback'], null); const invitation = await InvitationsService.createInvitation('e@nachklang.art', 'E', accessTo('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'});
@@ -111,7 +112,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', ['feedback'], null); const invitation = await InvitationsService.createInvitation('f@nachklang.art', 'F', accessTo('feedback'), null);
const first = await request(app) const first = await request(app)
.post('/admin/auth/invitations/accept') .post('/admin/auth/invitations/accept')
@@ -125,7 +126,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', ['feedback'], null); const invitation = await InvitationsService.createInvitation('g@nachklang.art', 'G', accessTo('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
@@ -142,7 +143,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', ['feedback'], null); const invitation = await InvitationsService.createInvitation('h@nachklang.art', 'H', accessTo('feedback'), null);
const res = await request(app) const res = await request(app)
.post('/admin/auth/invitations/accept') .post('/admin/auth/invitations/accept')
@@ -259,7 +260,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', ['feedback'], null); const invitation = await InvitationsService.createInvitation('r@nachklang.art', 'R', accessTo('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 {closeDatabase, createAndAcceptInvitation, resetDatabase} from './helpers.js'; import {accessTo, 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', ['feedback'], null); const original = await InvitationsService.createInvitation('new@nachklang.art', 'New', accessTo('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', ['feedback'], null); const invitation = await InvitationsService.createInvitation('new@nachklang.art', 'New', accessTo('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);
+12 -3
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 {AppName} from '../../src/models/admin/admin.schema.js'; import {ACCESS_ROLE, AppName, AppPermission, toPermissions} from '../../src/models/admin/admin.schema.js';
const db = NachklangAdminDB.db; const db = NachklangAdminDB.db;
@@ -44,10 +44,13 @@ export const createAndAcceptInvitation = async (
app: Application, app: Application,
email: string, email: string,
name: string, name: string,
apps: AppName[], // Takes the shorthand as well as the full form: most tests only care that
// someone can open an app, and `['tickets']` says that with less noise.
grants: (AppName | AppPermission)[],
password = 'devpassword123' password = 'devpassword123'
) => { ) => {
const invitation = await InvitationsService.createInvitation(email, name, apps, null); const permissions = toPermissions(grants) ?? [];
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
@@ -66,3 +69,9 @@ 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}));
};