Fix security and correctness issues found in full-codebase audit
- Restrict meetingPassword field so it isn't leaked via the public REST/GraphQL API - Fix booking slot computation to use Europe/Berlin time instead of the server's UTC clock - Reset email verification when a customer changes their email address - Fix ICS calendar injection via unescaped carriage returns in event descriptions - Bind WebRTC meeting kicks to a persistent client id so they survive a rejoin - Move the meeting signaling token out of the WebSocket query string into the first message - Prevent duplicate signaling connections from leaking when the same participant reconnects - Use the rightmost X-Forwarded-For entry for rate limiting (single trusted reverse proxy) - Use constant-time comparison for the cron endpoint's bearer token - Remove SVG from allowed Media upload types (stored-XSS risk, unused anyway) - Add baseline security headers and a CSP to next.config.ts - Fix CI/CD script injection via unsanitized git ref names in release.yml
This commit is contained in:
+29
-14
@@ -1,24 +1,40 @@
|
||||
import type { CollectionAfterChangeHook, CollectionBeforeChangeHook, CollectionConfig } from "payload";
|
||||
import { isAdminFieldLevel } from "@/access";
|
||||
import { isAdmin, isAdminFieldLevel } from "@/access";
|
||||
import { EMAIL_VERIFICATION_TTL_MS, generateVerificationToken, hashVerificationToken, verificationExpiryISO } from "@/lib/auth/verification";
|
||||
import { verificationEmail } from "@/lib/email/authTemplates";
|
||||
import { sendEmail } from "@/lib/email/sendBookingEmails";
|
||||
|
||||
const isSelfOrAdmin = ({ req }: { req: { user?: { collection?: string; id?: unknown } | null } }) => {
|
||||
const user = req.user;
|
||||
// Routes through the shared isAdmin check (role === "admin", not just
|
||||
// collection === "users") so this stays correct if a non-admin role is ever
|
||||
// added to Users — see the comment on isAdmin in access/index.ts.
|
||||
const isSelfOrAdmin: typeof isAdmin = (args) => {
|
||||
const user = args.req.user;
|
||||
if (!user) return false;
|
||||
if (user.collection === "users") return true;
|
||||
if (user.collection === "users") return isAdmin(args);
|
||||
if (user.collection === "customers") return { id: { equals: user.id } };
|
||||
return false;
|
||||
};
|
||||
|
||||
// Runs on every new customer account: issues a random, single-use,
|
||||
// time-limited email-verification token. Only its hash is ever persisted
|
||||
// (see the emailVerificationTokenHash field below) — the plaintext exists
|
||||
// only in req.context for the afterChange hook below to email out, and is
|
||||
// never written to the database or logged.
|
||||
const issueVerificationToken: CollectionBeforeChangeHook = ({ data, operation, req }) => {
|
||||
if (operation !== "create") return data;
|
||||
// Runs on every new customer account, and again whenever an existing
|
||||
// customer changes their email address: issues a random, single-use,
|
||||
// time-limited email-verification token and resets emailVerified to false.
|
||||
// Only the token's hash is ever persisted (see emailVerificationTokenHash
|
||||
// below) — the plaintext exists only in req.context for the afterChange
|
||||
// hook below to email out, and is never written to the database or logged.
|
||||
//
|
||||
// The email-change case matters because `update` access on this collection
|
||||
// is isSelfOrAdmin and the auto-added `email` field carries no field-level
|
||||
// access of its own — without this, a customer could verify a throwaway
|
||||
// address once and then repoint `email` at an address they don't own while
|
||||
// keeping emailVerified: true (which gates booking-confirmation emails).
|
||||
const issueVerificationToken: CollectionBeforeChangeHook = ({ data, originalDoc, operation, req }) => {
|
||||
const emailChanged =
|
||||
operation === "update" &&
|
||||
typeof data.email === "string" &&
|
||||
typeof originalDoc?.email === "string" &&
|
||||
data.email !== originalDoc.email;
|
||||
if (operation !== "create" && !emailChanged) return data;
|
||||
|
||||
const token = generateVerificationToken();
|
||||
data.emailVerified = false;
|
||||
data.emailVerificationTokenHash = hashVerificationToken(token);
|
||||
@@ -27,8 +43,7 @@ const issueVerificationToken: CollectionBeforeChangeHook = ({ data, operation, r
|
||||
return data;
|
||||
};
|
||||
|
||||
const sendVerificationEmail: CollectionAfterChangeHook = async ({ doc, operation, req }) => {
|
||||
if (operation !== "create") return doc;
|
||||
const sendVerificationEmail: CollectionAfterChangeHook = async ({ doc, req }) => {
|
||||
const token = req.context.pendingVerificationToken;
|
||||
if (typeof token !== "string") return doc;
|
||||
try {
|
||||
@@ -69,7 +84,7 @@ export const Customers: CollectionConfig = {
|
||||
create: () => true,
|
||||
read: isSelfOrAdmin,
|
||||
update: isSelfOrAdmin,
|
||||
delete: ({ req }) => req.user?.collection === "users",
|
||||
delete: isAdmin,
|
||||
},
|
||||
hooks: {
|
||||
beforeChange: [issueVerificationToken],
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import type { CollectionBeforeChangeHook, CollectionConfig } from "payload";
|
||||
import { isAdmin, publishedOrAdmin } from "@/access";
|
||||
import { isAdmin, isAdminFieldLevel, publishedOrAdmin } from "@/access";
|
||||
import { slugField } from "@/fields/slug";
|
||||
import { seoFields } from "@/fields/seo";
|
||||
import { generateMeetingPassword } from "@/lib/meeting/password";
|
||||
@@ -162,6 +162,13 @@ export const Events: CollectionConfig = {
|
||||
name: "meetingPassword",
|
||||
type: "text",
|
||||
label: "Meeting-Passwort",
|
||||
// Events are publicly readable (publishedOrAdmin) via the REST/
|
||||
// GraphQL API, so without this the plaintext password would be
|
||||
// returned to any visitor fetching the event — defeating the
|
||||
// password check in app/(payload)/api/meetings/[slug]/join. Only
|
||||
// exposed to admins there; the join route reads it via the Local
|
||||
// API, which bypasses field access by default.
|
||||
access: { read: isAdminFieldLevel },
|
||||
admin: {
|
||||
readOnly: true,
|
||||
condition: (data) => Boolean(data?.isOnline),
|
||||
|
||||
@@ -24,7 +24,10 @@ export const Media: CollectionConfig = {
|
||||
},
|
||||
upload: {
|
||||
staticDir: path.resolve(dirname, "..", "media"),
|
||||
mimeTypes: ["image/jpeg", "image/png", "image/webp", "image/svg+xml"],
|
||||
// SVG deliberately excluded: it can carry inline <script>/event-handler
|
||||
// payloads that execute if the raw file is ever opened directly (not via
|
||||
// <img>) — not currently needed by any part of the site.
|
||||
mimeTypes: ["image/jpeg", "image/png", "image/webp"],
|
||||
imageSizes: [
|
||||
{ name: "thumbnail", width: 400, position: "centre" },
|
||||
{ name: "card", width: 800, position: "centre" },
|
||||
|
||||
@@ -2,7 +2,11 @@ import type { CollectionConfig } from "payload";
|
||||
import { isAdmin } from "@/access";
|
||||
|
||||
function normalizePath(value: string): string {
|
||||
const trimmed = value.trim();
|
||||
// Lowercased to match the slugs it points at/from (fields/slug.ts always
|
||||
// lowercases via slugify) — without this, "/Angebote/Doula" and
|
||||
// "/angebote/doula" count as different values to the `unique` constraint
|
||||
// on fromPath, letting near-duplicate redirects coexist.
|
||||
const trimmed = value.trim().toLowerCase();
|
||||
const withSlash = trimmed.startsWith("/") ? trimmed : `/${trimmed}`;
|
||||
return withSlash.length > 1 ? withSlash.replace(/\/+$/, "") : withSlash;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user