From 51ea9594c3be8b2b8a169ff477dc97dac2d61bb7 Mon Sep 17 00:00:00 2001 From: jschoeny Date: Thu, 1 Jan 2026 20:14:38 -1000 Subject: [PATCH] Refactor `canEditAs` functions and fix hacks not editable as admin --- src/app/dashboard/actions.ts | 11 ++-- src/app/hack/[slug]/actions.ts | 50 +++++++++++++++- src/app/hack/[slug]/edit/patch/page.tsx | 26 ++++++-- src/app/hack/[slug]/page.tsx | 4 +- src/app/hack/[slug]/stats/page.tsx | 4 +- src/app/hack/[slug]/versions/page.tsx | 6 +- src/utils/hack.ts | 80 ++++++++++++++++++------- 7 files changed, 140 insertions(+), 41 deletions(-) diff --git a/src/app/dashboard/actions.ts b/src/app/dashboard/actions.ts index 392149f..ff6d548 100644 --- a/src/app/dashboard/actions.ts +++ b/src/app/dashboard/actions.ts @@ -2,7 +2,7 @@ import { unstable_cache as cache } from "next/cache"; import { createClient, createServiceClient } from "@/utils/supabase/server"; -import { canEditAsCreator, canEditAsAdminOrArchiver } from "@/utils/hack"; +import { canEditAsCreator, canEditAsArchiver } from "@/utils/hack"; import { checkUserRoles } from "@/utils/user"; interface SeriesDataset { @@ -77,8 +77,9 @@ export const getDownloadsSeriesAll = async ({ days = 30 }: { days?: number }): P // Skip if already owned if (canEditAsCreator(hack, user.id)) continue; - // Check if user can edit as archiver (function already checks if it's an archive) - if (await canEditAsAdminOrArchiver(hack, user.id, supa, { roles: { isAdmin, isArchiver } })) { + // Check if user can edit as archiver (function already checks if it's an archive and includes admins) + // Pass both isAdmin and isArchiver since checkUserRoles distinguishes them, but admins should be included + if (await canEditAsArchiver(hack, user.id, supa, { roles: { isAdmin, isArchiver } })) { accessibleArchiveSlugs.push(hack.slug); } } @@ -179,8 +180,8 @@ export const getHackInsights = async ({ slug }: { slug: string }): Promise; @@ -23,13 +23,29 @@ export default async function EditPatchPage({ params }: EditPatchPageProps) { .eq("slug", slug) .maybeSingle(); if (!hack) return notFound(); - if (!canEditAsCreator(hack, user!.id)) { + + // Check permissions: creator first (optimization), then admin, then archiver + const isCreator = canEditAsCreator(hack, user!.id); + if (!isCreator) { const isInformationalArchive = isInformationalArchiveHack(hack); const isDownloadableArchive = isDownloadableArchiveHack(hack); - const isEditableByArchiver = await canEditAsArchiver(hack, user!.id, supabase); - + + // Informational archives cannot have patches if (isInformationalArchive) redirect(`/hack/${slug}`); - if (isDownloadableArchive && !isEditableByArchiver) redirect(`/hack/${slug}`); + + // Check admin (works for any hack) + const isAdmin = await canEditAsAdmin(hack, user!.id, supabase); + if (isAdmin) { + // Admin can edit patches for any hack except informational archives + // (already checked above) + } else if (isDownloadableArchive) { + // Check archiver (only for downloadable archives) + const isEditableByArchiver = await canEditAsArchiver(hack, user!.id, supabase); + if (!isEditableByArchiver) redirect(`/hack/${slug}`); + } else { + // Not creator, not admin, not downloadable archive + redirect(`/hack/${slug}`); + } } const { data: patchRows } = await supabase diff --git a/src/app/hack/[slug]/page.tsx b/src/app/hack/[slug]/page.tsx index d01e4f4..6d6c9b5 100644 --- a/src/app/hack/[slug]/page.tsx +++ b/src/app/hack/[slug]/page.tsx @@ -125,7 +125,7 @@ export default async function HackDetail({ params }: HackDetailProps) { } = await supabase.auth.getUser(); const { canEdit, - canEditAsAdminOrArchiver, + canEditAsArchiver, isInformationalArchive, isDownloadableArchive, isArchive, @@ -142,7 +142,7 @@ export default async function HackDetail({ params }: HackDetailProps) { } } - if (isArchive && !isAdmin && !canEditAsAdminOrArchiver) { + if (isArchive && !isAdmin && !canEditAsArchiver) { return notFound(); } diff --git a/src/app/hack/[slug]/stats/page.tsx b/src/app/hack/[slug]/stats/page.tsx index 37c8917..3e4fda5 100644 --- a/src/app/hack/[slug]/stats/page.tsx +++ b/src/app/hack/[slug]/stats/page.tsx @@ -2,7 +2,7 @@ import { createClient } from "@/utils/supabase/server"; import { notFound, redirect } from "next/navigation"; import HackStatsClient from "@/components/Hack/Stats/HackStatsClient"; import { getDownloadsSeriesAll, getHackInsights } from "@/app/dashboard/actions"; -import { isArchiveHack, canEditAsAdminOrArchiver } from "@/utils/hack"; +import { isArchiveHack, canEditAsArchiver } from "@/utils/hack"; export default async function HackStatsPage({ params: { slug } }: { params: { slug: string } }) { const supa = await createClient(); @@ -20,7 +20,7 @@ export default async function HackStatsPage({ params: { slug } }: { params: { sl let isOwner = hack.created_by === user.id; if (!isOwner) { const isArchive = isArchiveHack(hack); - const isEditableByArchiver = await canEditAsAdminOrArchiver(hack, user.id, supa); + const isEditableByArchiver = await canEditAsArchiver(hack, user.id, supa); if (!isOwner && !isArchive && !isEditableByArchiver) notFound(); } diff --git a/src/app/hack/[slug]/versions/page.tsx b/src/app/hack/[slug]/versions/page.tsx index c889605..57fbaf3 100644 --- a/src/app/hack/[slug]/versions/page.tsx +++ b/src/app/hack/[slug]/versions/page.tsx @@ -1,6 +1,6 @@ import { notFound } from "next/navigation"; import { createClient } from "@/utils/supabase/server"; -import { canEditAsCreator } from "@/utils/hack"; +import { canEditAsCreator, canEditAsAdmin } from "@/utils/hack"; import VersionList from "@/components/Hack/VersionList"; import CollapsibleCard from "@/components/Primitives/CollapsibleCard"; import Link from "next/link"; @@ -24,8 +24,8 @@ export default async function VersionsPage({ params }: VersionsPageProps) { if (!hack) return notFound(); - // Check if user can edit (creator only for version management) - const canEdit = user ? canEditAsCreator(hack, user.id) : false; + // Check if user can edit (creator or admin for version management) + const canEdit = user ? (canEditAsCreator(hack, user.id) || await canEditAsAdmin(hack, user.id, supabase)) : false; // Fetch all published, non-archived patches const { data: patches } = await supabase diff --git a/src/utils/hack.ts b/src/utils/hack.ts index 4002010..c255da1 100644 --- a/src/utils/hack.ts +++ b/src/utils/hack.ts @@ -1,5 +1,4 @@ import type { SupabaseClient } from "@supabase/supabase-js"; -import { checkUserRoles } from "@/utils/user"; type HackWithArchiveFields = { is_archive: boolean; @@ -38,34 +37,35 @@ export function canEditAsCreator(hack: HackWithArchiveFields, userId: string): b } /** - * Check if a user can edit a hack as admin or archiver (for archive hacks only) + * Check if a user can edit a hack as admin (works for any hack, archive or not) * Requires a Supabase client to check RPC functions */ -export async function canEditAsAdminOrArchiver( +export async function canEditAsAdmin( hack: HackWithArchiveFields, userId: string, supabase: SupabaseClient, options?: { roles?: { isAdmin: boolean; - isArchiver: boolean; } }, ): Promise { - if (!isArchiveHack(hack) || canEditAsCreator(hack, userId)) { + // Optimization: check creator first (no DB call) + if (canEditAsCreator(hack, userId)) { return false; } if (options?.roles) { - return options.roles.isAdmin || options.roles.isArchiver; + return options.roles.isAdmin; } - const { isAdmin, isArchiver } = await checkUserRoles(supabase); - return isAdmin || isArchiver; + const { data: isAdmin } = await supabase.rpc("is_admin"); + return isAdmin ?? false; } /** - * Check if a user can edit a hack as archiver (for downloadable archives only) + * Check if a user can edit a hack as archiver (for archive hacks only) + * Uses the SQL is_archiver() RPC which includes admins * Requires a Supabase client to check RPC functions */ export async function canEditAsArchiver( @@ -74,20 +74,29 @@ export async function canEditAsArchiver( supabase: SupabaseClient, options?: { roles?: { - isArchiver: boolean; + isAdmin?: boolean; + isArchiver?: boolean; } }, ): Promise { - if (!isDownloadableArchiveHack(hack) || canEditAsCreator(hack, userId)) { + // Optimization: check creator first (no DB call) + if (canEditAsCreator(hack, userId)) { + return false; + } + + // Only works for archive hacks + if (!isArchiveHack(hack)) { return false; } if (options?.roles) { - return options.roles.isArchiver; + // If roles are provided, check both admin and archiver (admins are considered archivers) + return (options.roles.isAdmin ?? false) || (options.roles.isArchiver ?? false); } - const { isArchiver } = await checkUserRoles(supabase); - return isArchiver; + // Use is_archiver() RPC directly (includes admins per SQL function) + const { data: isArchiver } = await supabase.rpc("is_archiver"); + return isArchiver ?? false; } /** @@ -101,25 +110,37 @@ export async function checkEditPermission( ): Promise<{ canEdit: boolean; canEditAsCreator: boolean; - canEditAsAdminOrArchiver: boolean; + canEditAsAdmin: boolean; + canEditAsArchiver: boolean; isInformationalArchive: boolean; isDownloadableArchive: boolean; isArchive: boolean; }> { + // Optimization: check creator first (no DB call) const canEditAsCreatorValue = canEditAsCreator(hack, userId); const isInformationalArchiveValue = isInformationalArchiveHack(hack); const isDownloadableArchiveValue = isDownloadableArchiveHack(hack); const isArchiveValue = isArchiveHack(hack); - let canEditAsAdminOrArchiverValue = false; - if (isArchiveValue && !canEditAsCreatorValue) { - canEditAsAdminOrArchiverValue = await canEditAsAdminOrArchiver(hack, userId, supabase); + let canEditAsAdminValue = false; + let canEditAsArchiverValue = false; + + // Only check admin/archiver if not creator + if (!canEditAsCreatorValue) { + // Check admin (works for any hack) + canEditAsAdminValue = await canEditAsAdmin(hack, userId, supabase); + + // Check archiver (only for archive hacks, and only if not admin) + if (isArchiveValue && !canEditAsAdminValue) { + canEditAsArchiverValue = await canEditAsArchiver(hack, userId, supabase); + } } return { - canEdit: canEditAsCreatorValue || canEditAsAdminOrArchiverValue, + canEdit: canEditAsCreatorValue || canEditAsAdminValue || canEditAsArchiverValue, canEditAsCreator: canEditAsCreatorValue, - canEditAsAdminOrArchiver: canEditAsAdminOrArchiverValue, + canEditAsAdmin: canEditAsAdminValue, + canEditAsArchiver: canEditAsArchiverValue, isInformationalArchive: isInformationalArchiveValue, isDownloadableArchive: isDownloadableArchiveValue, isArchive: isArchiveValue, @@ -137,11 +158,13 @@ export async function checkPatchEditPermission( ): Promise<{ canEdit: boolean; canEditAsCreator: boolean; + canEditAsAdmin: boolean; canEditAsArchiver: boolean; isInformationalArchive: boolean; isDownloadableArchive: boolean; error?: string; }> { + // Optimization: check creator first (no DB call) const canEditAsCreatorValue = canEditAsCreator(hack, userId); const isInformationalArchiveValue = isInformationalArchiveHack(hack); const isDownloadableArchiveValue = isDownloadableArchiveHack(hack); @@ -151,6 +174,7 @@ export async function checkPatchEditPermission( return { canEdit: false, canEditAsCreator: canEditAsCreatorValue, + canEditAsAdmin: false, canEditAsArchiver: false, isInformationalArchive: isInformationalArchiveValue, isDownloadableArchive: isDownloadableArchiveValue, @@ -158,14 +182,24 @@ export async function checkPatchEditPermission( }; } + let canEditAsAdminValue = false; let canEditAsArchiverValue = false; - if (isDownloadableArchiveValue && !canEditAsCreatorValue) { - canEditAsArchiverValue = await canEditAsArchiver(hack, userId, supabase); + + // Only check admin/archiver if not creator + if (!canEditAsCreatorValue) { + // Check admin (works for any hack) + canEditAsAdminValue = await canEditAsAdmin(hack, userId, supabase); + + // Check archiver (only for downloadable archives, and only if not admin) + if (isDownloadableArchiveValue && !canEditAsAdminValue) { + canEditAsArchiverValue = await canEditAsArchiver(hack, userId, supabase); + } } return { - canEdit: canEditAsCreatorValue || canEditAsArchiverValue, + canEdit: canEditAsCreatorValue || canEditAsAdminValue || canEditAsArchiverValue, canEditAsCreator: canEditAsCreatorValue, + canEditAsAdmin: canEditAsAdminValue, canEditAsArchiver: canEditAsArchiverValue, isInformationalArchive: isInformationalArchiveValue, isDownloadableArchive: isDownloadableArchiveValue,