diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index ca58314..7f1b71e 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -23,6 +23,7 @@ jobs: DATABASE_URI: postgresql://ci:ci@localhost:5432/ci PAYLOAD_SECRET: ci-only-not-a-real-secret MEETING_SESSION_SECRET: ci-only-not-a-real-secret + ANALYTICS_SALT_SECRET: ci-only-not-a-real-secret CRON_SECRET: ci-only-not-a-real-secret NEXT_PUBLIC_SERVER_URL: http://localhost:3000 steps: diff --git a/.gitea/workflows/release.yml b/.gitea/workflows/release.yml index 67ee0ee..e4782ae 100644 --- a/.gitea/workflows/release.yml +++ b/.gitea/workflows/release.yml @@ -19,6 +19,7 @@ jobs: DATABASE_URI: postgresql://ci:ci@localhost:5432/ci PAYLOAD_SECRET: ci-only-not-a-real-secret MEETING_SESSION_SECRET: ci-only-not-a-real-secret + ANALYTICS_SALT_SECRET: ci-only-not-a-real-secret CRON_SECRET: ci-only-not-a-real-secret NEXT_PUBLIC_SERVER_URL: http://localhost:3000 steps: @@ -67,27 +68,43 @@ jobs: # so step commands use the same $GITHUB_OUTPUT / $GITHUB_ENV # mechanism (the `gitea.*` vs `github.*` expression contexts are # just aliases of each other for `${{ }}` templating). + # + # The tag name (github.ref_name / $GITHUB_REF_NAME) is + # attacker-influenced: anyone with tag-push rights controls it, and + # git allows shell metacharacters in tag names. It's read here via + # the automatic $GITHUB_REF_NAME env var rather than `${{ github.ref_name }}` + # so its value reaches bash as data (argv/envp), never spliced into + # the script text — `${{ }}` expressions are substituted into the + # YAML before bash ever sees the script, which is what makes + # interpolating attacker-controlled values that way a script-injection + # vector (arbitrary code execution in a job that holds the registry + # and production deploy credentials). run: | echo "registry=${REGISTRY:-git.maro.run}" >> "$GITHUB_OUTPUT" echo "repo=${REGISTRY_REPO:-maro/anouma}" >> "$GITHUB_OUTPUT" - echo "version=${{ github.ref_name }}" >> "$GITHUB_OUTPUT" + echo "version=$GITHUB_REF_NAME" >> "$GITHUB_OUTPUT" env: REGISTRY: git.maro.run REGISTRY_REPO: maro/anouma - name: Log in to registry - run: echo "${{ secrets.REGISTRY_PASSWORD }}" | docker login "${{ steps.image.outputs.registry }}" -u "${{ secrets.REGISTRY_USERNAME }}" --password-stdin + env: + REGISTRY_IMAGE: ${{ steps.image.outputs.registry }} + REGISTRY_USERNAME: ${{ secrets.REGISTRY_USERNAME }} + REGISTRY_PASSWORD: ${{ secrets.REGISTRY_PASSWORD }} + run: echo "$REGISTRY_PASSWORD" | docker login "$REGISTRY_IMAGE" -u "$REGISTRY_USERNAME" --password-stdin - name: Build image - run: | - IMAGE="${{ steps.image.outputs.registry }}/${{ steps.image.outputs.repo }}" - VERSION="${{ steps.image.outputs.version }}" - docker build -t "$IMAGE:$VERSION" -t "$IMAGE:latest" . + env: + IMAGE: ${{ steps.image.outputs.registry }}/${{ steps.image.outputs.repo }} + VERSION: ${{ steps.image.outputs.version }} + run: docker build -t "$IMAGE:$VERSION" -t "$IMAGE:latest" . - name: Push image + env: + IMAGE: ${{ steps.image.outputs.registry }}/${{ steps.image.outputs.repo }} + VERSION: ${{ steps.image.outputs.version }} run: | - IMAGE="${{ steps.image.outputs.registry }}/${{ steps.image.outputs.repo }}" - VERSION="${{ steps.image.outputs.version }}" docker push "$IMAGE:$VERSION" docker push "$IMAGE:latest" diff --git a/access/index.ts b/access/index.ts index bc9c522..b274c8a 100644 --- a/access/index.ts +++ b/access/index.ts @@ -13,6 +13,17 @@ export const isAdmin: Access = ({ req: { user } }) => export const isAdminFieldLevel: FieldAccess = ({ req: { user } }) => user?.collection === "users" && user.role === "admin"; +/** + * Admins always see Analytics; any other "users" account additionally needs + * its own `canViewAnalytics` flag (see collections/Users.ts) — this is the + * granular permission the single "admin" role doesn't otherwise provide. + * Used both as a Payload Access/FieldAccess function and called directly + * (with a minimal `{ req: { user } }` shape) from the custom admin nav-link + * and view components, which Payload does not gate via collection access. + */ +export const canViewAnalytics: Access = ({ req: { user } }) => + user?.collection === "users" && (user.role === "admin" || user.canViewAnalytics === true); + /** * Admins can read every document (including drafts); everyone else may only * read documents whose Payload draft/publish status is "published". diff --git a/app/(payload)/api/cron/event-reminders/route.ts b/app/(payload)/api/cron/event-reminders/route.ts index b804b09..a1f1559 100644 --- a/app/(payload)/api/cron/event-reminders/route.ts +++ b/app/(payload)/api/cron/event-reminders/route.ts @@ -1,8 +1,16 @@ +import { timingSafeEqual } from "node:crypto"; import { NextResponse } from "next/server"; import { runEventReminders } from "@/lib/meeting/reminders"; export const dynamic = "force-dynamic"; +function safeEqual(a: string, b: string): boolean { + const bufA = Buffer.from(a); + const bufB = Buffer.from(b); + if (bufA.length !== bufB.length) return false; + return timingSafeEqual(bufA, bufB); +} + /** * Intended to be called by an external cron job every few minutes, e.g.: * curl -H "Authorization: Bearer $CRON_SECRET" https://anouma.org/api/cron/event-reminders @@ -14,7 +22,7 @@ export async function GET(request: Request) { } const authHeader = request.headers.get("authorization"); - if (authHeader !== `Bearer ${secret}`) { + if (!authHeader || !safeEqual(authHeader, `Bearer ${secret}`)) { return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); } diff --git a/app/(payload)/api/meetings/[slug]/join/route.ts b/app/(payload)/api/meetings/[slug]/join/route.ts index 6f0ceee..4f6d0aa 100644 --- a/app/(payload)/api/meetings/[slug]/join/route.ts +++ b/app/(payload)/api/meetings/[slug]/join/route.ts @@ -20,7 +20,7 @@ type Args = { params: Promise<{ slug: string }> }; export async function POST(request: Request, { params }: Args) { const { slug } = await params; - let body: { name?: unknown; password?: unknown }; + let body: { name?: unknown; password?: unknown; clientId?: unknown }; try { body = await request.json(); } catch { @@ -29,6 +29,7 @@ export async function POST(request: Request, { params }: Args) { const name = typeof body.name === "string" ? body.name.trim() : ""; const password = typeof body.password === "string" ? body.password.trim() : ""; + const clientId = typeof body.clientId === "string" ? body.clientId.trim() : undefined; if (!name) { return NextResponse.json({ error: "Bitte gib deinen Namen ein." }, { status: 400 }); } @@ -85,6 +86,10 @@ export async function POST(request: Request, { params }: Args) { eventSlug: slug, name, role: isHost ? "host" : "participant", + // Only meaningful for participants — a host authenticates via a real + // session already, so there's no need to anchor a kick to it (and + // hosts can't be kicked in the first place, see signalingServer.ts). + clientId: isHost ? undefined : clientId, }); return NextResponse.json({ diff --git a/collections/Customers.ts b/collections/Customers.ts index dbccd71..f5e9686 100644 --- a/collections/Customers.ts +++ b/collections/Customers.ts @@ -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], diff --git a/collections/Events.ts b/collections/Events.ts index 3389b82..bdc114f 100644 --- a/collections/Events.ts +++ b/collections/Events.ts @@ -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), diff --git a/collections/Media.ts b/collections/Media.ts index 3454ee4..a58b563 100644 --- a/collections/Media.ts +++ b/collections/Media.ts @@ -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