From 8c0cc4492b4390a3119c58f0e771e9eb0caa8d7a Mon Sep 17 00:00:00 2001 From: gunshiz Date: Thu, 9 Jul 2026 23:53:40 +0700 Subject: [PATCH] Harden form auth and submission handling --- SKILL.md | 6 +- app/actions/discord.ts | 146 +---------------- app/actions/questions.ts | 44 +++-- app/actions/submissions.ts | 152 ++++++++---------- app/admin/backend/actions.ts | 11 +- app/admin/form/[id]/client.tsx | 28 ++-- app/admin/form/[id]/extra/client.tsx | 29 ++-- app/admin/form/[id]/layout.tsx | 7 +- .../[id]/result/[submissionId]/client.tsx | 5 +- app/admin/form/actions.ts | 56 ++++--- app/admin/form/page.tsx | 12 +- app/admin/layout.tsx | 9 +- app/api/auth/[...nextauth]/route.ts | 5 +- app/api/upload/route.ts | 26 +-- app/form/[id]/not-found.tsx | 27 ++-- app/form/[id]/page.tsx | 32 +--- app/form/client.tsx | 45 +++--- app/form/layout.tsx | 5 +- app/form/page.tsx | 43 +---- app/not-found.tsx | 27 ++-- components/admin-shell-server.tsx | 5 +- components/admin-shell.tsx | 3 +- components/copybutton.tsx | 7 +- components/delete-buttons.tsx | 13 +- components/dropdown-menu-avatar.tsx | 29 ++-- components/html-display.tsx | 3 +- components/page-progress.tsx | 19 ++- components/rich-text-editor.tsx | 17 +- components/theme-provider.tsx | 7 +- components/theme-toggle.tsx | 5 +- db/schema/form.ts | 38 +++-- drizzle/0011_unique_submission_user.sql | 1 + drizzle/meta/_journal.json | 9 +- hooks/use-mobile.ts | 7 +- lib/auth.ts | 86 ++++++++++ lib/discord.ts | 151 +++++++++++++++++ lib/errors.ts | 3 + lib/redis.ts | 2 +- lib/sanitize-html.ts | 79 +++++++++ next.config.ts | 1 - proxy.ts | 3 +- scripts/apply-0011-migration.ts | 65 ++++++++ skill/erika/SKILL.md | 13 +- 43 files changed, 747 insertions(+), 534 deletions(-) create mode 100644 drizzle/0011_unique_submission_user.sql create mode 100644 lib/auth.ts create mode 100644 lib/discord.ts create mode 100644 lib/errors.ts create mode 100644 lib/sanitize-html.ts create mode 100644 scripts/apply-0011-migration.ts diff --git a/SKILL.md b/SKILL.md index d08410e..cc1e65e 100644 --- a/SKILL.md +++ b/SKILL.md @@ -31,9 +31,9 @@ This successfully provides a fully responsive, easily configurable, and containe ## 5. Form Module Structure - **Public Form Viewer:** Located at `app/form/page.tsx` and `app/form/client.tsx`. This is where users see and submit the form. It renders questions and handles user input with validation. -- **Admin Form Builder:** Located at `app/form/admin/page.tsx` and `app/form/admin/client.tsx`. This interface allows administrators to create and edit questions, change form settings, and specify input types (Text, Textarea, Radio, Checkboxes). +- **Admin Form Builder:** Located under `app/admin/form/`. This interface allows administrators to create and edit questions, change form settings, and specify input types (Text, Textarea, Radio, Checkboxes). - **Database Schema:** Defined in `db/schema/form.ts`, which contains definitions for `forms`, `questions`, `submissions`, and `answers`. -- **Server Actions:** Backend logic for managing the form (such as creating/updating questions and handling submissions) are located in `app/actions/form.ts`, `app/actions/questions.ts`, and `app/actions/submissions.ts`. +- **Server Actions:** Backend logic for managing forms and submissions is located in `app/admin/form/actions.ts`, `app/actions/questions.ts`, and `app/actions/submissions.ts`. Shared authorization lives in `lib/auth.ts`. Behavioral guidelines to reduce common LLM coding mistakes. Merge with project-specific instructions as needed. @@ -97,4 +97,4 @@ Strong success criteria let you loop independently. Weak criteria ("make it work --- -**These guidelines are working if:** fewer unnecessary changes in diffs, fewer rewrites due to overcomplication, and clarifying questions come before implementation rather than after mistakes. \ No newline at end of file +**These guidelines are working if:** fewer unnecessary changes in diffs, fewer rewrites due to overcomplication, and clarifying questions come before implementation rather than after mistakes. diff --git a/app/actions/discord.ts b/app/actions/discord.ts index 46d503d..05d41fa 100644 --- a/app/actions/discord.ts +++ b/app/actions/discord.ts @@ -1,149 +1,9 @@ "use server"; -import { getServerSession } from "next-auth"; -import { authOptions } from "@/app/api/auth/[...nextauth]/route"; +import { requireAdmin } from "@/lib/auth"; +import { getGuildRolesInternal, type DiscordRole } from "@/lib/discord"; -export interface DiscordRole { - id: string; - name: string; - color: number; - position: number; -} - -// --- In-memory caches with TTL --- -interface CacheEntry { data: T; expiresAt: number } -const rolesCache: { entry: CacheEntry | null } = { entry: null }; -const profileCache = new Map>(); -const CACHE_TTL = 60_000; // 60 seconds - -function getCached(entry: CacheEntry | null | undefined): T | null { - if (entry && Date.now() < entry.expiresAt) return entry.data; - return null; -} - -/** Internal: fetch guild roles without auth check (for use by other server functions). */ -async function getGuildRolesInternal(): Promise { - const cached = getCached(rolesCache.entry); - if (cached) return cached; - - const token = process.env.DISCORD_BOT_TOKEN; - const guildId = process.env.DISCORD_GUILD_ID; - if (!token || !guildId) return []; - - try { - const res = await fetch(`https://discord.com/api/v10/guilds/${guildId}/roles`, { - headers: { Authorization: `Bot ${token}` }, - signal: AbortSignal.timeout(5000), - }); - - if (!res.ok) return []; - - const roles: any[] = await res.json(); - const result = roles.map(r => ({ - id: r.id, - name: r.name, - color: r.color, - position: r.position, - })).sort((a, b) => b.position - a.position); - - rolesCache.entry = { data: result, expiresAt: Date.now() + CACHE_TTL }; - return result; - } catch { - return []; - } -} - -/** Public: fetch guild roles with admin auth check. */ export async function getGuildRoles(): Promise { - const session = await getServerSession(authOptions); - const discordId = (session?.user as any)?.discordId; - const admins = (process.env.ADMIN_DISCORD_IDS || "").split(","); - if (!discordId || !admins.includes(discordId)) { - throw new Error("Unauthorized"); - } - + await requireAdmin(); return getGuildRolesInternal(); } - -export async function getGuildMemberRoles(discordId: string): Promise { - const token = process.env.DISCORD_BOT_TOKEN; - const guildId = process.env.DISCORD_GUILD_ID; - if (!token || !guildId || !discordId) return []; - - try { - const res = await fetch(`https://discord.com/api/v10/guilds/${guildId}/members/${discordId}`, { - headers: { Authorization: `Bot ${token}` }, - cache: "no-store", // Always fresh for access control - signal: AbortSignal.timeout(5000), - }); - - if (!res.ok) return []; - - const member = await res.json(); - return member.roles || []; - } catch { - return []; - } -} - -export async function getDiscordMemberProfile(discordId: string) { - const cached = getCached(profileCache.get(discordId)); - if (cached) return cached; - - const token = process.env.DISCORD_BOT_TOKEN; - const guildId = process.env.DISCORD_GUILD_ID; - if (!token || !guildId || !discordId) return null; - - try { - const [memberRes, allRoles] = await Promise.all([ - fetch(`https://discord.com/api/v10/guilds/${guildId}/members/${discordId}`, { - headers: { Authorization: `Bot ${token}` }, - cache: "no-store", - signal: AbortSignal.timeout(5000), - }), - getGuildRolesInternal(), - ]); - - if (!memberRes.ok) return null; - const member = await memberRes.json(); - - // Map member role IDs to actual role objects - const memberRoleIds = member.roles || []; - const roles = memberRoleIds - .map((id: string) => allRoles.find(r => r.id === id)) - .filter(Boolean); - - // Sort roles by position (descending) - roles.sort((a: any, b: any) => (b?.position || 0) - (a?.position || 0)); - - const user = member.user; - - let avatarUrl = null; - if (user.avatar) { - const ext = user.avatar.startsWith("a_") ? "gif" : "png"; - avatarUrl = `https://cdn.discordapp.com/avatars/${user.id}/${user.avatar}.${ext}?size=128`; - } - - let bannerUrl = null; - if (user.banner) { - const ext = user.banner.startsWith("a_") ? "gif" : "png"; - bannerUrl = `https://cdn.discordapp.com/banners/${user.id}/${user.banner}.${ext}?size=512`; - } - - const result = { - id: user.id, - username: user.username, - globalName: user.global_name || null, - avatarUrl, - bannerUrl, - accentColor: user.accent_color || null, - roles, - }; - - profileCache.set(discordId, { data: result, expiresAt: Date.now() + CACHE_TTL }); - return result; - } catch (e) { - console.error("Failed to fetch member profile", e); - return null; - } -} diff --git a/app/actions/questions.ts b/app/actions/questions.ts index 39e7f3f..b78e042 100644 --- a/app/actions/questions.ts +++ b/app/actions/questions.ts @@ -7,17 +7,8 @@ import { questions } from "@/db/schema"; import type { QuestionType } from "@/db/schema"; import { eq } from "drizzle-orm"; import { revalidatePath } from "next/cache"; -import { getServerSession } from "next-auth"; -import { authOptions } from "@/app/api/auth/[...nextauth]/route"; - -async function assertAdmin() { - const session = await getServerSession(authOptions); - const adminIds = (process.env.ADMIN_DISCORD_IDS ?? "").split(",").map((s) => s.trim()).filter(Boolean); - const discordId = (session?.user as { discordId?: string } | undefined)?.discordId; - if (!discordId || !adminIds.includes(discordId)) { - throw new Error("Unauthorized"); - } -} +import { requireAdmin } from "@/lib/auth"; +import { sanitizeHtml } from "@/lib/sanitize-html"; export async function createQuestion( formId: string, @@ -29,18 +20,18 @@ export async function createQuestion( options?: string[]; } ) { - await assertAdmin(); + await requireAdmin(); await db.insert(questions).values({ formId, type: data.type, - label: data.label, + label: sanitizeHtml(data.label), required: data.required, displayOrder: data.displayOrder, options: data.options ?? [], allowOther: false, }); revalidatePath("/form"); - revalidatePath("/form/admin"); + revalidatePath("/admin/form"); } export async function updateQuestion( @@ -56,7 +47,7 @@ export async function updateQuestion( allowOther: boolean; }> ) { - await assertAdmin(); + await requireAdmin(); if (data.imageUrl !== undefined) { const q = await db.query.questions.findFirst({ where: (q, { eq }) => eq(q.id, id), @@ -65,9 +56,14 @@ export async function updateQuestion( await deleteImageFile(q.imageUrl); } } - await db.update(questions).set(data).where(eq(questions.id, id)); + const nextData = { + ...data, + label: data.label === undefined ? undefined : sanitizeHtml(data.label), + }; + + await db.update(questions).set(nextData).where(eq(questions.id, id)); revalidatePath("/form"); - revalidatePath("/form/admin"); + revalidatePath("/admin/form"); } export async function bulkUpdateQuestions( @@ -82,7 +78,7 @@ export async function bulkUpdateQuestions( allowOther: boolean; }[] ) { - await assertAdmin(); + await requireAdmin(); // Use Promise.all to update all questions concurrently await Promise.all( @@ -90,7 +86,7 @@ export async function bulkUpdateQuestions( db .update(questions) .set({ - label: u.label, + label: sanitizeHtml(u.label), type: u.type, required: u.required, displayOrder: u.displayOrder, @@ -102,7 +98,7 @@ export async function bulkUpdateQuestions( ); revalidatePath("/form"); - revalidatePath("/form/admin"); + revalidatePath("/admin/form"); } async function deleteImageFile(imageUrl: string | null) { @@ -119,7 +115,7 @@ async function deleteImageFile(imageUrl: string | null) { } export async function deleteQuestion(id: string, formId: string) { - await assertAdmin(); + await requireAdmin(); const q = await db.query.questions.findFirst({ where: (q, { eq }) => eq(q.id, id), }); @@ -128,14 +124,14 @@ export async function deleteQuestion(id: string, formId: string) { } await db.delete(questions).where(eq(questions.id, id)); revalidatePath("/form"); - revalidatePath("/form/admin"); + revalidatePath(`/admin/form/${formId}`); } export async function reorderQuestions( formId: string, orderedIds: string[] ) { - await assertAdmin(); + await requireAdmin(); await Promise.all( orderedIds.map((id, index) => db @@ -145,5 +141,5 @@ export async function reorderQuestions( ) ); revalidatePath("/form"); - revalidatePath("/form/admin"); + revalidatePath(`/admin/form/${formId}`); } diff --git a/app/actions/submissions.ts b/app/actions/submissions.ts index 38846ee..44c883d 100644 --- a/app/actions/submissions.ts +++ b/app/actions/submissions.ts @@ -6,6 +6,7 @@ import { eq } from "drizzle-orm"; import { revalidatePath } from "next/cache"; import { getServerSession } from "next-auth"; import { authOptions } from "@/app/api/auth/[...nextauth]/route"; +import { requireAdmin, requireDiscordId, requireFormAccess } from "@/lib/auth"; export async function submitForm( formId: string, @@ -14,85 +15,76 @@ export async function submitForm( const session = await getServerSession(authOptions); if (!session?.user) throw new Error("Not authenticated"); - const discordId = (session.user as { discordId?: string }).discordId; - if (!discordId) throw new Error("No Discord ID found"); - - const form = await db.query.forms.findFirst({ - where: (f, { eq }) => eq(f.id, formId), - }); - if (!form) throw new Error("Form not found"); + const discordId = await requireDiscordId(); + const userName = session.user.name ?? null; + const form = await requireFormAccess(formId, discordId); if (!form.isOpen) throw new Error("This form is disabled for now"); - const existingSubmission = await db.query.submissions.findFirst({ - where: (s, { eq, and }) => - and(eq(s.formId, formId), eq(s.userDiscordId, discordId)), - }); - - let submissionId = ""; - const isUpdate = !!existingSubmission; - let oldAnswersDict: Record = {}; - - if (existingSubmission) { - submissionId = existingSubmission.id; - - // Fetch old answers - const oldAnswersList = await db.query.answers.findMany({ - where: (a, { eq }) => eq(a.submissionId, submissionId), + const result = await db.transaction(async (tx) => { + const existingSubmission = await tx.query.submissions.findFirst({ + where: (submission, { eq, and }) => + and(eq(submission.formId, formId), eq(submission.userDiscordId, discordId)), }); - oldAnswersDict = oldAnswersList.reduce((acc, curr) => { - acc[curr.questionId] = curr.value; - return acc; - }, {} as Record); + let submissionId = ""; + const isUpdate = Boolean(existingSubmission); + let oldAnswersDict: Record = {}; - // Ensure editHistory is an array - let history = existingSubmission.editHistory; - if (!Array.isArray(history)) { - history = []; + if (existingSubmission) { + submissionId = existingSubmission.id; + + const oldAnswersList = await tx.query.answers.findMany({ + where: (answer, { eq }) => eq(answer.submissionId, submissionId), + }); + + oldAnswersDict = oldAnswersList.reduce>((acc, curr) => { + acc[curr.questionId] = curr.value; + return acc; + }, {}); + + const history = Array.isArray(existingSubmission.editHistory) + ? [...existingSubmission.editHistory] + : []; + history.push({ + editedAt: new Date().toISOString(), + oldAnswers: oldAnswersDict, + }); + + await tx + .update(submissions) + .set({ editHistory: history }) + .where(eq(submissions.id, submissionId)); + + await tx.delete(answers).where(eq(answers.submissionId, submissionId)); + } else { + const [inserted] = await tx + .insert(submissions) + .values({ + formId, + userDiscordId: discordId, + userName, + }) + .returning(); + submissionId = inserted.id; } - // Push the old answers to history - history.push({ - editedAt: new Date().toISOString(), - oldAnswers: oldAnswersDict, - }); + if (answersList.length > 0) { + await tx.insert(answers).values( + answersList.map((answer) => ({ + submissionId, + questionId: answer.questionId, + value: answer.value, + })) + ); + } - // Update submission record - await db - .update(submissions) - .set({ - editHistory: history, - }) - .where(eq(submissions.id, submissionId)); - - // Delete old answers - await db.delete(answers).where(eq(answers.submissionId, submissionId)); - } else { - // Insert new submission - const [inserted] = await db - .insert(submissions) - .values({ - formId, - userDiscordId: discordId, - userName: session.user.name ?? null, - }) - .returning(); - submissionId = inserted.id; - } - - // Insert new/updated answers - if (answersList.length > 0) { - await db.insert(answers).values( - answersList.map((a) => ({ - submissionId, - questionId: a.questionId, - value: a.value, - })) - ); - } + return { submissionId, isUpdate, oldAnswersDict }; + }); // Send Discord Webhook - const templateToUse = isUpdate ? (form as any).discordWebhookUpdateTemplate : form.discordWebhookTemplate; + const templateToUse = result.isUpdate + ? form.discordWebhookUpdateTemplate + : form.discordWebhookTemplate; if (form.discordWebhookUrl && templateToUse) { try { @@ -114,10 +106,10 @@ export async function submitForm( }, {} as Record); let updatesText = ""; - if (isUpdate) { + if (result.isUpdate) { const changed: string[] = []; for (const ans of answersList) { - const oldVal = oldAnswersDict[ans.questionId]; + const oldVal = result.oldAnswersDict[ans.questionId]; if (oldVal !== ans.value) { const qTitle = questionsList.find(q => q.id === ans.questionId)?.label || `Question`; changed.push(`**${qTitle}**: \`${oldVal || "Empty"}\` ➡️ \`${ans.value || "Empty"}\``); @@ -156,22 +148,20 @@ export async function submitForm( } } - revalidatePath("/form/admin/result"); + revalidatePath(`/admin/form/${formId}/result`); return { ok: true }; } export async function deleteSubmission(id: string) { - const session = await getServerSession(authOptions); - if (!session?.user) throw new Error("Not authenticated"); + await requireAdmin(); await db.delete(submissions).where(eq(submissions.id, id)); - revalidatePath("/form/admin/result"); + revalidatePath("/admin/form"); return { ok: true }; } export async function deleteAllSubmissions(formId: string) { - const session = await getServerSession(authOptions); - if (!session?.user) throw new Error("Not authenticated"); + await requireAdmin(); await db.delete(submissions).where(eq(submissions.formId, formId)); revalidatePath(`/admin/form/${formId}/result`); @@ -179,11 +169,7 @@ export async function deleteAllSubmissions(formId: string) { } export async function deleteOwnSubmission(formId: string) { - const session = await getServerSession(authOptions); - if (!session?.user) throw new Error("Not authenticated"); - - const discordId = (session.user as { discordId?: string }).discordId; - if (!discordId) throw new Error("No Discord ID found"); + const discordId = await requireDiscordId(); const { and } = await import("drizzle-orm"); @@ -194,6 +180,6 @@ export async function deleteOwnSubmission(formId: string) { ) ); revalidatePath(`/form`); - revalidatePath("/form/admin/result"); + revalidatePath(`/admin/form/${formId}/result`); return { ok: true }; } diff --git a/app/admin/backend/actions.ts b/app/admin/backend/actions.ts index d8b7284..a299127 100644 --- a/app/admin/backend/actions.ts +++ b/app/admin/backend/actions.ts @@ -3,6 +3,8 @@ import { revalidateTag } from "next/cache"; const validPlatforms = ["youtube", "roblox", "discord", "tiktok"]; +type FollowerFetcher = () => Promise; + export async function refetchPlatform( platform: string ): Promise<{ count: number | null }> { @@ -15,10 +17,10 @@ export async function refetchPlatform( let tiktokProfile = null; try { - const mod = await import(`@/lib/followers/${platform}`); + const mod = (await import(`@/lib/followers/${platform}`)) as Record; const fns = Object.entries(mod).filter( - ([, v]) => typeof v === "function" - ) as [string, Function][]; + (entry): entry is [string, FollowerFetcher] => typeof entry[1] === "function" + ); // Prefer function with "Count" or "Member" in name const countFn = fns.find(([name]) => /count|member/i.test(name)); @@ -33,7 +35,8 @@ export async function refetchPlatform( } else if ( result && typeof result === "object" && - "followerCount" in result + "followerCount" in result && + (typeof result.followerCount === "number" || result.followerCount === null) ) { count = result.followerCount; tiktokProfile = result; diff --git a/app/admin/form/[id]/client.tsx b/app/admin/form/[id]/client.tsx index cd6dbec..9e90c6a 100644 --- a/app/admin/form/[id]/client.tsx +++ b/app/admin/form/[id]/client.tsx @@ -39,6 +39,7 @@ import { RichTextEditor } from "@/components/rich-text-editor"; import { FormattedText } from "@/components/formatted-text"; import { ThemeToggle } from "@/components/theme-toggle"; import { DropdownMenuAvatar, DiscordMemberProfile } from "@/components/dropdown-menu-avatar"; +import { getErrorMessage } from "@/lib/errors"; interface Question { id: string; @@ -84,10 +85,12 @@ export default function FormEditorClient({ form, initialQuestions }: FormEditorC const formIsOpenRef = useRef(formIsOpen); const questionsRef = useRef(questions); - formTitleRef.current = formTitle; - formDescriptionRef.current = formDescription; - formIsOpenRef.current = formIsOpen; - questionsRef.current = questions; + useEffect(() => { + formTitleRef.current = formTitle; + formDescriptionRef.current = formDescription; + formIsOpenRef.current = formIsOpen; + questionsRef.current = questions; + }, [formDescription, formIsOpen, formTitle, questions]); const autoSaveForm = useCallback(() => { if (formTimerRef.current) clearTimeout(formTimerRef.current); @@ -170,7 +173,7 @@ export default function FormEditorClient({ form, initialQuestions }: FormEditorC options: [], }); router.refresh(); - } catch (err: any) { + } catch (err: unknown) { console.error("Failed to add question:", err); } }; @@ -186,7 +189,7 @@ export default function FormEditorClient({ form, initialQuestions }: FormEditorC try { await deleteQuestion(id, form.id); router.refresh(); - } catch (err: any) { + } catch (err: unknown) { console.error("Failed to delete question:", err); } }; @@ -243,7 +246,7 @@ export default function FormEditorClient({ form, initialQuestions }: FormEditorC { loading: "Uploading image...", success: "Image uploaded!", - error: (err: any) => err.message || "Failed to upload image.", + error: (err: unknown) => getErrorMessage(err, "Failed to upload image."), } ); }; @@ -259,7 +262,7 @@ export default function FormEditorClient({ form, initialQuestions }: FormEditorC { loading: "Removing image...", success: "Image removed.", - error: (err: any) => err.message || "Failed to remove image.", + error: (err: unknown) => getErrorMessage(err, "Failed to remove image."), } ); }; @@ -283,8 +286,9 @@ export default function FormEditorClient({ form, initialQuestions }: FormEditorC handleUpdateQuestion(question.id, { options }); }; - React.useEffect(() => { - setQuestions(initialQuestions); + useEffect(() => { + const timeout = setTimeout(() => setQuestions(initialQuestions), 0); + return () => clearTimeout(timeout); }, [initialQuestions]); return ( @@ -360,7 +364,7 @@ export default function FormEditorClient({ form, initialQuestions }: FormEditorC