Refactor canEditAs functions and fix hacks not editable as admin

This commit is contained in:
jschoeny 2026-01-01 20:14:38 -10:00
parent 155b2c3241
commit 51ea9594c3
7 changed files with 140 additions and 41 deletions

View File

@ -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<HackI
const { data: admin } = await supa.rpc("is_admin");
if (admin) {
// Admin can access any hack
} else if (await canEditAsAdminOrArchiver(hack, user.id, supa)) {
// Archiver can access archive hacks (function already checks if it's an archive)
} else if (await canEditAsArchiver(hack, user.id, supa)) {
// Archiver can access archive hacks (function already checks if it's an archive and includes admins)
} else {
throw new Error("Forbidden");
}

View File

@ -2,7 +2,7 @@
import { createClient, createServiceClient } from "@/utils/supabase/server";
import { getMinioClient, PATCHES_BUCKET } from "@/utils/minio/server";
import { isInformationalArchiveHack, canEditAsCreator } from "@/utils/hack";
import { isInformationalArchiveHack, canEditAsCreator, canEditAsAdmin } from "@/utils/hack";
import { sendDiscordMessageEmbed } from "@/utils/discord";
import { headers } from "next/headers";
import { validateEmail } from "@/utils/auth";
@ -450,10 +450,16 @@ export async function archivePatchVersion(slug: string, patchId: number): Promis
.maybeSingle();
if (hErr || !hack) return { ok: false, error: "Hack not found" };
// Check permissions: creator first (optimization), then admin
if (!canEditAsCreator(hack, user.id)) {
return { ok: false, error: "Forbidden" };
}
const editableAsAdmin = await canEditAsAdmin(hack, user.id, supabase);
if (!editableAsAdmin) {
return { ok: false, error: "Forbidden" };
}
// Cannot archive current_patch
if (hack.current_patch === patchId) {
return { ok: false, error: "Cannot archive the current patch version" };
@ -495,10 +501,16 @@ export async function restorePatchVersion(slug: string, patchId: number): Promis
.maybeSingle();
if (hErr || !hack) return { ok: false, error: "Hack not found" };
// Check permissions: creator first (optimization), then admin
if (!canEditAsCreator(hack, user.id)) {
return { ok: false, error: "Forbidden" };
}
const editableAsAdmin = await canEditAsAdmin(hack, user.id, supabase);
if (!editableAsAdmin) {
return { ok: false, error: "Forbidden" };
}
// Verify patch belongs to this hack
const { data: patch, error: pErr } = await supabase
.from("patches")
@ -535,10 +547,16 @@ export async function rollbackToVersion(slug: string, patchId: number): Promise<
.maybeSingle();
if (hErr || !hack) return { ok: false, error: "Hack not found" };
// Check permissions: creator first (optimization), then admin
if (!canEditAsCreator(hack, user.id)) {
return { ok: false, error: "Forbidden" };
}
const editableAsAdmin = await canEditAsAdmin(hack, user.id, supabase);
if (!editableAsAdmin) {
return { ok: false, error: "Forbidden" };
}
// Verify patch belongs to this hack and get its created_at
const { data: rollbackPatch, error: pErr } = await supabase
.from("patches")
@ -585,10 +603,16 @@ export async function updatePatchChangelog(slug: string, patchId: number, change
.maybeSingle();
if (hErr || !hack) return { ok: false, error: "Hack not found" };
// Check permissions: creator first (optimization), then admin
if (!canEditAsCreator(hack, user.id)) {
return { ok: false, error: "Forbidden" };
}
const editableAsAdmin = await canEditAsAdmin(hack, user.id, supabase);
if (!editableAsAdmin) {
return { ok: false, error: "Forbidden" };
}
// Verify patch belongs to this hack
const { data: patch, error: pErr } = await supabase
.from("patches")
@ -627,10 +651,16 @@ export async function updatePatchVersion(slug: string, patchId: number, version:
.maybeSingle();
if (hErr || !hack) return { ok: false, error: "Hack not found" };
// Check permissions: creator first (optimization), then admin
if (!canEditAsCreator(hack, user.id)) {
return { ok: false, error: "Forbidden" };
}
const editableAsAdmin = await canEditAsAdmin(hack, user.id, supabase);
if (!editableAsAdmin) {
return { ok: false, error: "Forbidden" };
}
// Verify patch belongs to this hack
const { data: patch, error: pErr } = await supabase
.from("patches")
@ -691,10 +721,16 @@ export async function publishPatchVersion(slug: string, patchId: number): Promis
.maybeSingle();
if (hErr || !hack) return { ok: false, error: "Hack not found" };
// Check permissions: creator first (optimization), then admin
if (!canEditAsCreator(hack, user.id)) {
return { ok: false, error: "Forbidden" };
}
const editableAsAdmin = await canEditAsAdmin(hack, user.id, supabase);
if (!editableAsAdmin) {
return { ok: false, error: "Forbidden" };
}
// Verify patch belongs to this hack and get its created_at
const { data: patch, error: pErr } = await supabase
.from("patches")
@ -760,10 +796,16 @@ export async function reuploadPatchVersion(
.maybeSingle();
if (hErr || !hack) return { ok: false, error: "Hack not found" };
// Check permissions: creator first (optimization), then admin
if (!canEditAsCreator(hack, user.id)) {
return { ok: false, error: "Forbidden" };
}
const editableAsAdmin = await canEditAsAdmin(hack, user.id, supabase);
if (!editableAsAdmin) {
return { ok: false, error: "Forbidden" };
}
// Verify patch belongs to this hack
const { data: patch, error: pErr } = await supabase
.from("patches")
@ -799,10 +841,16 @@ export async function confirmReuploadPatchVersion(
.maybeSingle();
if (hErr || !hack) return { ok: false, error: "Hack not found" };
// Check permissions: creator first (optimization), then admin
if (!canEditAsCreator(hack, user.id)) {
return { ok: false, error: "Forbidden" };
}
const editableAsAdmin = await canEditAsAdmin(hack, user.id, supabase);
if (!editableAsAdmin) {
return { ok: false, error: "Forbidden" };
}
// Verify patch belongs to this hack
const { data: patch, error: pErr } = await supabase
.from("patches")

View File

@ -3,7 +3,7 @@ import { createClient } from "@/utils/supabase/server";
import HackPatchForm from "@/components/Hack/HackPatchForm";
import Link from "next/link";
import { FaChevronLeft } from "react-icons/fa6";
import { isInformationalArchiveHack, isDownloadableArchiveHack, canEditAsCreator, canEditAsArchiver } from "@/utils/hack";
import { isInformationalArchiveHack, isDownloadableArchiveHack, canEditAsCreator, canEditAsAdmin, canEditAsArchiver } from "@/utils/hack";
interface EditPatchPageProps {
params: Promise<{ slug: string }>;
@ -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

View File

@ -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();
}

View File

@ -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();
}

View File

@ -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

View File

@ -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<any>,
options?: {
roles?: {
isAdmin: boolean;
isArchiver: boolean;
}
},
): Promise<boolean> {
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<any>,
options?: {
roles?: {
isArchiver: boolean;
isAdmin?: boolean;
isArchiver?: boolean;
}
},
): Promise<boolean> {
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,