fix: auth + double sending etc
CI / build-and-test (push) Canceled after 0s

This commit is contained in:
2026-10-30 00:41:58 +01:00
parent 314717ba3a
commit 3caa6d6f0d
15 changed files with 129 additions and 32 deletions
+1 -1
View File
@@ -152,7 +152,7 @@ export class AdminController {
@Post('images/cleanup') @Post('images/cleanup')
@HttpCode(HttpStatus.OK) @HttpCode(HttpStatus.OK)
cleanupImages(@Body(zod(dryRunSchema)) dto: Body<typeof dryRunSchema>, @CurrentUser() user: AuthUser) { cleanupImages(@Body(zod(dryRunSchema)) dto: Body<typeof dryRunSchema>, @CurrentUser() user: AuthUser) {
return this.admin.cleanupImages(dto.dryRun, user); return this.admin.cleanupImages(dto.dryRun, { type: 'user', id: user.id });
} }
@Post('email/test') @Post('email/test')
+2 -1
View File
@@ -4,10 +4,11 @@ import { RemindersModule } from '../reminders/reminders.module';
import { VenuesModule } from '../venues/venues.module'; import { VenuesModule } from '../venues/venues.module';
import { AdminController } from './admin.controller'; import { AdminController } from './admin.controller';
import { AdminService } from './admin.service'; import { AdminService } from './admin.service';
import { HousekeepingService } from './housekeeping.service';
@Module({ @Module({
imports: [ImagesModule, RemindersModule, VenuesModule], imports: [ImagesModule, RemindersModule, VenuesModule],
controllers: [AdminController], controllers: [AdminController],
providers: [AdminService], providers: [AdminService, HousekeepingService],
}) })
export class AdminModule {} export class AdminModule {}
+12 -3
View File
@@ -7,7 +7,7 @@ import {
SystemHealth, SystemHealth,
} from '@kooltap/shared'; } from '@kooltap/shared';
import { Prisma, PrismaService } from '../prisma/prisma.service'; import { Prisma, PrismaService } from '../prisma/prisma.service';
import { AuditService } from '../audit/audit.service'; import { AuditEntry as AuditRecord, AuditService } from '../audit/audit.service';
import { RealtimeService } from '../realtime/realtime.service'; import { RealtimeService } from '../realtime/realtime.service';
import { RemindersService } from '../reminders/reminders.service'; import { RemindersService } from '../reminders/reminders.service';
import { VenuesService } from '../venues/venues.service'; import { VenuesService } from '../venues/venues.service';
@@ -38,6 +38,11 @@ const ORPHAN_IMAGE_GRACE_MS = 7 * 24 * 60 * 60 * 1000;
const DAY_MS = 24 * 60 * 60 * 1000; const DAY_MS = 24 * 60 * 60 * 1000;
/** Dashboard sessions that expired or were signed out before [before]; they can never be used again. */
export const endedSessions = (before: Date): Prisma.RefreshTokenWhereInput => ({
OR: [{ expiresAt: { lt: before } }, { revokedAt: { lt: before } }],
});
/** Everything the superadmin page does. */ /** Everything the superadmin page does. */
@Injectable() @Injectable()
export class AdminService { export class AdminService {
@@ -387,9 +392,13 @@ export class AdminService {
const venue = venueId ? { venueId } : {}; const venue = venueId ? { venueId } : {};
const tabEvents = { ...venue, receivedAt: { lt: before } }; const tabEvents = { ...venue, receivedAt: { lt: before } };
const auditLog = { ...venue, createdAt: { lt: before } }; const auditLog = { ...venue, createdAt: { lt: before } };
const refreshTokens = { ...endedSessions(before), ...(venueId ? { user: { venueId } } : {}) };
const run: Record<PurgeTarget, () => Promise<number>> = { const run: Record<PurgeTarget, () => Promise<number>> = {
tabEvents: () => (dryRun ? this.prisma.tabEvent.count({ where: tabEvents }) : this.prisma.tabEvent.deleteMany({ where: tabEvents }).then((r) => r.count)), tabEvents: () => (dryRun ? this.prisma.tabEvent.count({ where: tabEvents }) : this.prisma.tabEvent.deleteMany({ where: tabEvents }).then((r) => r.count)),
auditLog: () => (dryRun ? this.prisma.auditLog.count({ where: auditLog }) : this.prisma.auditLog.deleteMany({ where: auditLog }).then((r) => r.count)), auditLog: () => (dryRun ? this.prisma.auditLog.count({ where: auditLog }) : this.prisma.auditLog.deleteMany({ where: auditLog }).then((r) => r.count)),
refreshTokens: () => (dryRun
? this.prisma.refreshToken.count({ where: refreshTokens })
: this.prisma.refreshToken.deleteMany({ where: refreshTokens }).then((r) => r.count)),
}; };
const counts: Partial<Record<PurgeTarget, number>> = {}; const counts: Partial<Record<PurgeTarget, number>> = {};
@@ -401,7 +410,7 @@ export class AdminService {
} }
/** Deletes pictures no product uses, after a grace period for terminals that upload before they link. */ /** Deletes pictures no product uses, after a grace period for terminals that upload before they link. */
async cleanupImages(dryRun: boolean, actor: AuthUser) { async cleanupImages(dryRun: boolean, actor: AuditRecord['actor']) {
const images = await this.prisma.productImage.findMany({ const images = await this.prisma.productImage.findMany({
where: { products: { none: {} }, createdAt: { lt: new Date(Date.now() - ORPHAN_IMAGE_GRACE_MS) } }, where: { products: { none: {} }, createdAt: { lt: new Date(Date.now() - ORPHAN_IMAGE_GRACE_MS) } },
select: { id: true, bytes: true }, select: { id: true, bytes: true },
@@ -409,7 +418,7 @@ export class AdminService {
const bytes = images.reduce((sum, image) => sum + image.bytes, 0); const bytes = images.reduce((sum, image) => sum + image.bytes, 0);
if (!dryRun && images.length > 0) { if (!dryRun && images.length > 0) {
await this.images.remove(images.map((image) => image.id)); await this.images.remove(images.map((image) => image.id));
await this.audit.log({ venueId: null, actor: { type: 'user', id: actor.id }, action: 'admin_images_cleaned', details: { count: images.length } }); await this.audit.log({ venueId: null, actor, action: 'admin_images_cleaned', details: { count: images.length } });
} }
return { dryRun, count: images.length, bytes }; return { dryRun, count: images.length, bytes };
} }
@@ -0,0 +1,53 @@
import { Injectable, Logger, OnApplicationBootstrap, OnApplicationShutdown } from '@nestjs/common';
import { PrismaService } from '../prisma/prisma.service';
import { AdminService, endedSessions } from './admin.service';
const DAY_MS = 24 * 60 * 60 * 1000;
// Out of the way of startup and the migrations, then once a day
const FIRST_RUN_DELAY_MS = 5 * 60 * 1000;
const INTERVAL_MS = DAY_MS;
// Ended sessions are kept a while, so a superadmin can still see recent sign-outs while looking into something
const ENDED_SESSION_RETENTION_MS = 7 * DAY_MS;
/**
* Daily cleanup of what can never be used again: ended sessions, used and expired registration codes, and pictures
* no product uses. History (terminal events, audit log) is a choice, so it stays with the purge on the admin page.
* Every instance runs it; the deletes don't mind running twice.
*/
@Injectable()
export class HousekeepingService implements OnApplicationBootstrap, OnApplicationShutdown {
private readonly logger = new Logger(HousekeepingService.name);
private timer: NodeJS.Timeout | null = null;
constructor(
private prisma: PrismaService,
private admin: AdminService,
) {}
onApplicationBootstrap() {
// Tests run against a database they set up themselves
if (process.env.NODE_ENV === 'test') return;
this.timer = setTimeout(() => {
void this.run();
this.timer = setInterval(() => void this.run(), INTERVAL_MS).unref();
}, FIRST_RUN_DELAY_MS).unref();
}
onApplicationShutdown() {
if (this.timer) clearTimeout(this.timer);
}
/** Never throws: a failure is logged and the next run tries again. */
async run() {
try {
const sessions = await this.prisma.refreshToken.deleteMany({ where: endedSessions(new Date(Date.now() - ENDED_SESSION_RETENTION_MS)) });
const codes = await this.admin.cleanupRegistrationCodes();
const images = await this.admin.cleanupImages(false, { type: 'system', id: null });
this.logger.log(`Deleted ${sessions.count} ended session(s), ${codes.deleted} registration code(s), ${images.count} unused picture(s)`);
} catch (error) {
this.logger.error(`Housekeeping failed: ${error instanceof Error ? error.stack : error}`);
}
}
}
+2
View File
@@ -21,8 +21,10 @@ export class AuthController {
return me; return me;
} }
// A dashboard refreshes about every 15 minutes; this still leaves room for many staff behind a venue's shared IP
@Post('refresh') @Post('refresh')
@HttpCode(HttpStatus.OK) @HttpCode(HttpStatus.OK)
@Throttle({ default: { limit: 30, ttl: 60_000 } })
async refresh(@Req() req: Request, @Res({ passthrough: true }) res: Response): Promise<Me> { async refresh(@Req() req: Request, @Res({ passthrough: true }) res: Response): Promise<Me> {
try { try {
const { me, tokens } = await this.auth.refresh(req.cookies?.[REFRESH_COOKIE]); const { me, tokens } = await this.auth.refresh(req.cookies?.[REFRESH_COOKIE]);
+5 -2
View File
@@ -78,9 +78,12 @@ export class AuthService {
throw new BadRequestException({ code: 'wrong_password', message: 'The current password is not correct' }); throw new BadRequestException({ code: 'wrong_password', message: 'The current password is not correct' });
} }
// Revoking the sessions also ends the other sessions' access tokens at once; the new tokens are issued after it,
// in the same second or later, so they stay valid
const now = new Date();
await this.prisma.$transaction([ await this.prisma.$transaction([
this.prisma.user.update({ where: { id: userId }, data: { passwordHash: await hashPassword(newPassword) } }), this.prisma.user.update({ where: { id: userId }, data: { passwordHash: await hashPassword(newPassword), sessionsRevokedAt: now } }),
this.prisma.refreshToken.updateMany({ where: { userId, revokedAt: null }, data: { revokedAt: new Date() } }), this.prisma.refreshToken.updateMany({ where: { userId, revokedAt: null }, data: { revokedAt: now } }),
]); ]);
await this.audit.log({ venueId: user.venueId, actor: { type: 'user', id: userId }, action: 'password_changed' }); await this.audit.log({ venueId: user.venueId, actor: { type: 'user', id: userId }, action: 'password_changed' });
return { me: toMe(user), tokens: await this.issueTokens(userId) }; return { me: toMe(user), tokens: await this.issueTokens(userId) };
+15 -5
View File
@@ -1,18 +1,28 @@
import { CallHandler, ExecutionContext, Injectable, Logger, NestInterceptor } from '@nestjs/common'; import { CallHandler, ExecutionContext, HttpException, Injectable, Logger, NestInterceptor } from '@nestjs/common';
import { Observable, tap } from 'rxjs'; import { Observable, tap } from 'rxjs';
/** One line per request, failed ones included: 4xx as a warning, 5xx as an error. */
@Injectable() @Injectable()
export class LoggingInterceptor implements NestInterceptor { export class LoggingInterceptor implements NestInterceptor {
private readonly logger = new Logger('HTTP'); private readonly logger = new Logger('HTTP');
intercept(context: ExecutionContext, next: CallHandler): Observable<unknown> { intercept(context: ExecutionContext, next: CallHandler): Observable<unknown> {
const request = context.switchToHttp().getRequest(); const http = context.switchToHttp();
const request = http.getRequest();
const start = Date.now(); const start = Date.now();
return next.handle().pipe( const line = (status: number) => {
tap(() => {
const actor = request.user?.id ?? request.device?.id ?? 'anonymous'; const actor = request.user?.id ?? request.device?.id ?? 'anonymous';
const venueId = request.user?.venueId ?? request.device?.venueId ?? '-'; const venueId = request.user?.venueId ?? request.device?.venueId ?? '-';
this.logger.log(`${request.method} ${request.url} | actor=${actor} venue=${venueId} | ${Date.now() - start}ms`); return `${request.method} ${request.url} ${status} | actor=${actor} venue=${venueId} | ${Date.now() - start}ms`;
};
return next.handle().pipe(
tap({
next: () => this.logger.log(line(http.getResponse().statusCode)),
error: (error: unknown) => {
const status = error instanceof HttpException ? error.getStatus() : 500;
if (status >= 500) this.logger.error(line(status));
else this.logger.warn(line(status));
},
}), }),
); );
} }
+10 -1
View File
@@ -3,6 +3,8 @@ import { RenderedEmail } from './email-templates.service';
export type OutgoingEmail = RenderedEmail & { to: string }; export type OutgoingEmail = RenderedEmail & { to: string };
const SEND_TIMEOUT_MS = 15_000;
/** Sends email through Resend (RESEND_API_KEY, RESEND_FROM_EMAIL). */ /** Sends email through Resend (RESEND_API_KEY, RESEND_FROM_EMAIL). */
@Injectable() @Injectable()
export class Resend { export class Resend {
@@ -23,9 +25,11 @@ export class Resend {
/** /**
* Sends up to 100 emails in one request. Resend ignores a request with the same [idempotencyKey] for 24 hours, so a * Sends up to 100 emails in one request. Resend ignores a request with the same [idempotencyKey] for 24 hours, so a
* retry never emails twice. Returns Resend's error text, or null when sent. * retry never emails twice. Returns Resend's error text, or null when sent. Gives up after [SEND_TIMEOUT_MS], so a
* slow Resend never holds up the request that sends.
*/ */
async sendBatch(emails: OutgoingEmail[], idempotencyKey?: string): Promise<string | null> { async sendBatch(emails: OutgoingEmail[], idempotencyKey?: string): Promise<string | null> {
try {
const res = await fetch('https://api.resend.com/emails/batch', { const res = await fetch('https://api.resend.com/emails/batch', {
method: 'POST', method: 'POST',
headers: { headers: {
@@ -34,8 +38,13 @@ export class Resend {
...(idempotencyKey ? { 'Idempotency-Key': idempotencyKey } : {}), ...(idempotencyKey ? { 'Idempotency-Key': idempotencyKey } : {}),
}, },
body: JSON.stringify(emails.map((email) => ({ from: this.from, ...email }))), body: JSON.stringify(emails.map((email) => ({ from: this.from, ...email }))),
signal: AbortSignal.timeout(SEND_TIMEOUT_MS),
}); });
return res.ok ? null : await res.text(); return res.ok ? null : await res.text();
} catch (error) {
if (error instanceof Error && error.name === 'TimeoutError') return `Resend did not answer within ${SEND_TIMEOUT_MS / 1000} seconds`;
return `Could not reach Resend: ${error instanceof Error ? error.message : error}`;
}
} }
} }
+4 -2
View File
@@ -1,4 +1,5 @@
import { BadRequestException, Injectable, NotFoundException } from '@nestjs/common'; import { BadRequestException, Injectable, NotFoundException } from '@nestjs/common';
import { randomBytes } from 'crypto';
import { promises as fs } from 'fs'; import { promises as fs } from 'fs';
import * as path from 'path'; import * as path from 'path';
import { PrismaService } from '../prisma/prisma.service'; import { PrismaService } from '../prisma/prisma.service';
@@ -32,8 +33,9 @@ export class ImagesService {
const file = this.file(image.id); const file = this.file(image.id);
await fs.mkdir(path.dirname(file), { recursive: true }); await fs.mkdir(path.dirname(file), { recursive: true });
// Written next to it and renamed, so a picture is never read half-written // Written next to it and renamed, so a picture is never read half-written. Unique, so two uploads of the same
const temp = `${file}.${process.pid}.tmp`; // picture at once don't share a temp file
const temp = `${file}.${randomBytes(8).toString('hex')}.tmp`;
await fs.writeFile(temp, image.bytes); await fs.writeFile(temp, image.bytes);
await fs.rename(temp, file); await fs.rename(temp, file);
+4 -1
View File
@@ -47,10 +47,13 @@ async function request(endpoint: string, init: RequestInit): Promise<Response> {
} }
} }
// A 401 from these is the answer itself (wrong password, session over), not an expired access token
const NO_REFRESH_ENDPOINTS = ['/auth/login', '/auth/refresh', '/auth/logout'];
export async function api<T = unknown>(endpoint: string, init: RequestInit = {}): Promise<T> { export async function api<T = unknown>(endpoint: string, init: RequestInit = {}): Promise<T> {
let res = await request(endpoint, init); let res = await request(endpoint, init);
if (res.status === 401 && !endpoint.startsWith('/auth/')) { if (res.status === 401 && !NO_REFRESH_ENDPOINTS.includes(endpoint)) {
if (await refreshSession()) { if (await refreshSession()) {
res = await request(endpoint, init); res = await request(endpoint, init);
} else { } else {
+2
View File
@@ -507,6 +507,8 @@
"admin.purge.target.tabEvents_hint": "What the terminals reported, drink by drink", "admin.purge.target.tabEvents_hint": "What the terminals reported, drink by drink",
"admin.purge.target.auditLog": "Audit log", "admin.purge.target.auditLog": "Audit log",
"admin.purge.target.auditLog_hint": "Who did what on the dashboard", "admin.purge.target.auditLog_hint": "Who did what on the dashboard",
"admin.purge.target.refreshTokens": "Ended sessions",
"admin.purge.target.refreshTokens_hint": "Dashboard sign-ins that expired or were signed out; active sessions stay",
"admin.purge.days": "Older than (days)", "admin.purge.days": "Older than (days)",
"admin.purge.preview": "Preview…", "admin.purge.preview": "Preview…",
"admin.purge.confirm_desc": "Everything older than {days} days:", "admin.purge.confirm_desc": "Everything older than {days} days:",
+2
View File
@@ -507,6 +507,8 @@
"admin.purge.target.tabEvents_hint": "Wat de kassa's doorgaven, drankje per drankje", "admin.purge.target.tabEvents_hint": "Wat de kassa's doorgaven, drankje per drankje",
"admin.purge.target.auditLog": "Logboek", "admin.purge.target.auditLog": "Logboek",
"admin.purge.target.auditLog_hint": "Wie wat deed op het dashboard", "admin.purge.target.auditLog_hint": "Wie wat deed op het dashboard",
"admin.purge.target.refreshTokens": "Beëindigde sessies",
"admin.purge.target.refreshTokens_hint": "Aanmeldingen op het dashboard die verlopen of afgemeld zijn; actieve sessies blijven",
"admin.purge.days": "Ouder dan (dagen)", "admin.purge.days": "Ouder dan (dagen)",
"admin.purge.preview": "Voorbeeld…", "admin.purge.preview": "Voorbeeld…",
"admin.purge.confirm_desc": "Alles ouder dan {days} dagen:", "admin.purge.confirm_desc": "Alles ouder dan {days} dagen:",
+1 -1
View File
@@ -25,7 +25,7 @@ products); terminals see that on their next sync. See [terminal-protocol.md](ter
| `terminal/` | Pairing, sync and picture uploads for terminals; device token auth; the minimum app version | | `terminal/` | Pairing, sync and picture uploads for terminals; device token auth; the minimum app version |
| `auth/` | Dashboard sign-in with httpOnly cookies, refresh token rotation, `GET /auth/me` | | `auth/` | Dashboard sign-in with httpOnly cookies, refresh token rotation, `GET /auth/me` |
| `tabs/`, `customers/`, `stock/`, `reminders/`, `venues/` | The dashboard API, one module per page | | `tabs/`, `customers/`, `stock/`, `reminders/`, `venues/` | The dashboard API, one module per page |
| `admin/` | The superadmin page: venues, terminals, users, resets, consistency checks, purges | | `admin/` | The superadmin page: venues, terminals, users, resets, consistency checks, purges; daily housekeeping |
| `realtime/` | Socket.IO: tells dashboards which data changed (topics only, never data) | | `realtime/` | Socket.IO: tells dashboards which data changed (topics only, never data) |
| `email/` | Templates in `templates/emails` and Resend | | `email/` | Templates in `templates/emails` and Resend |
| `images/` | Product pictures, processed with sharp and stored on disk by content hash | | `images/` | Product pictures, processed with sharp and stored on disk by content hash |
+4 -3
View File
@@ -5,7 +5,8 @@
- Passwords hashed with bcrypt (cost 12), at least 8 characters; emails stored lowercased. - Passwords hashed with bcrypt (cost 12), at least 8 characters; emails stored lowercased.
- Sign-in sets two httpOnly, SameSite=Lax cookies (Secure in production): a JWT access token for 15 minutes, and a - Sign-in sets two httpOnly, SameSite=Lax cookies (Secure in production): a JWT access token for 15 minutes, and a
refresh token for 30 days, sent only to `/api/v1/auth`. No script can read either. refresh token for 30 days, sent only to `/api/v1/auth`. No script can read either.
- A refresh token works once: refreshing revokes it and issues a new pair. Signing out revokes it. - A refresh token works once: refreshing revokes it and issues a new pair. Signing out revokes it. Changing the
password signs out every other session at once. Ended sessions are deleted daily after a week.
- Every request looks the user up: a deleted user, a changed role or a forced sign-out (`sessionsRevokedAt`) applies at - Every request looks the user up: a deleted user, a changed role or a forced sign-out (`sessionsRevokedAt`) applies at
once instead of when the access token expires. once instead of when the access token expires.
- Sign-in answers the same, in the same time, for an unknown email and a wrong password. - Sign-in answers the same, in the same time, for an unknown email and a wrong password.
@@ -32,7 +33,7 @@ public by id, a SHA-256 hash that can't be guessed.
- Every body and query is validated with a zod schema; unknown fields are dropped. - Every body and query is validated with a zod schema; unknown fields are dropped.
- Helmet headers; CORS only for the dashboard's origin. - Helmet headers; CORS only for the dashboard's origin.
- Rate limits: 10 sign-ins or pairings per minute per client, 100 requests per minute otherwise. Terminal syncs are - Rate limits: 10 sign-ins or pairings and 30 refreshes per minute per client, 100 requests per minute otherwise.
not limited (they are authenticated by the device token). Terminal syncs are not limited (they are authenticated by the device token).
- Errors are `{ error: { code, message } }`; internal errors are logged and never shown. - Errors are `{ error: { code, message } }`; internal errors are logged and never shown.
- The realtime socket authenticates with the session cookie and only carries topic names, never data. - The realtime socket authenticates with the session cookie and only carries topic names, never data.
+1 -1
View File
@@ -37,5 +37,5 @@ export const RESET_SCOPES = ['tabs', 'settlements', 'tabEvents', 'customers', 's
export type ResetScope = (typeof RESET_SCOPES)[number]; export type ResetScope = (typeof RESET_SCOPES)[number];
/** Old history a superadmin can purge. */ /** Old history a superadmin can purge. */
export const PURGE_TARGETS = ['tabEvents', 'auditLog'] as const; export const PURGE_TARGETS = ['tabEvents', 'auditLog', 'refreshTokens'] as const;
export type PurgeTarget = (typeof PURGE_TARGETS)[number]; export type PurgeTarget = (typeof PURGE_TARGETS)[number];