diff --git a/src/collections/intelligence/Technologies.ts b/src/collections/intelligence/Technologies.ts index e07757a..00c77a1 100644 --- a/src/collections/intelligence/Technologies.ts +++ b/src/collections/intelligence/Technologies.ts @@ -1,14 +1,12 @@ import { CollectionConfig } from "payload"; import { requirePermission } from "@/utils/access-control/hasPermission"; -import { requireApprovalPermission, requireIntelligencePermission } from "@/utils/access-control/divisionAccess"; +import { + approvalStatusFieldAccess, + requireApprovalPermission, + requireIntelligencePermission, +} from "@/utils/access-control/divisionAccess"; -// Approval is a super-user decision: the field is stripped from create/update -// payloads of everyone below the Admin tier, so the schema default -// ("in_progress") applies and division members can never self-approve. -const approvalFieldAccess = { - create: requireApprovalPermission(), - update: requireApprovalPermission(), -}; +const approvalStatusFieldAccessConfig = approvalStatusFieldAccess("technologies"); export const Technologies: CollectionConfig = { slug: "technologies", @@ -32,7 +30,7 @@ export const Technologies: CollectionConfig = { name: "approvalStatus", type: "select", label: "Approval Status", - access: approvalFieldAccess, + access: approvalStatusFieldAccessConfig, options: [ { label: "In Progress", value: "in_progress" }, { label: "Ready for Review", value: "ready_for_review" }, diff --git a/src/collections/logistics/Assets.ts b/src/collections/logistics/Assets.ts index 9c92eb8..cf4f692 100644 --- a/src/collections/logistics/Assets.ts +++ b/src/collections/logistics/Assets.ts @@ -1,5 +1,6 @@ import { CollectionConfig } from "payload"; import { + approvalStatusFieldAccess, requireApprovalPermission, requireLogisticsPermission, } from "@/utils/access-control/divisionAccess"; @@ -9,6 +10,8 @@ const approvalFieldAccess = { update: requireApprovalPermission(), }; +const approvalStatusFieldAccessConfig = approvalStatusFieldAccess("assets"); + export const Assets: CollectionConfig = { slug: "assets", admin: { @@ -30,7 +33,7 @@ export const Assets: CollectionConfig = { name: "approvalStatus", type: "select", label: "Approval Status", - access: approvalFieldAccess, + access: approvalStatusFieldAccessConfig, options: [ { label: "In Progress", value: "in_progress" }, { label: "Ready for Review", value: "ready_for_review" }, diff --git a/src/collections/logistics/Resources.ts b/src/collections/logistics/Resources.ts index 04a9b04..678eb1d 100644 --- a/src/collections/logistics/Resources.ts +++ b/src/collections/logistics/Resources.ts @@ -2,6 +2,7 @@ import { AccessArgs, AccessResult, CollectionConfig, PayloadRequest } from "payl import type { Permission } from "@/permissions"; import { isDeveloper } from "@/utils/access-control/isRole"; import { + approvalStatusFieldAccess, requireApprovalPermission, requireLogisticsPermission, } from "@/utils/access-control/divisionAccess"; @@ -46,10 +47,7 @@ const logisticsWriteWithCurrencyGuard = (permission: Permission) => { }; }; -const approvalFieldAccess = { - create: requireApprovalPermission(), - update: requireApprovalPermission(), -}; +const approvalStatusFieldAccessConfig = approvalStatusFieldAccess("resources"); export const Resources: CollectionConfig = { slug: "resources", @@ -139,7 +137,7 @@ export const Resources: CollectionConfig = { name: "approvalStatus", type: "select", label: "Approval Status", - access: approvalFieldAccess, + access: approvalStatusFieldAccessConfig, options: [ { label: "In Progress", value: "in_progress" }, { label: "Ready for Review", value: "ready_for_review" }, diff --git a/src/collections/logistics/Vehicles.ts b/src/collections/logistics/Vehicles.ts index 23ae52c..facb37c 100644 --- a/src/collections/logistics/Vehicles.ts +++ b/src/collections/logistics/Vehicles.ts @@ -1,13 +1,11 @@ import { CollectionConfig } from "payload"; import { + approvalStatusFieldAccess, requireApprovalPermission, requireLogisticsPermission, } from "@/utils/access-control/divisionAccess"; -const approvalFieldAccess = { - create: requireApprovalPermission(), - update: requireApprovalPermission(), -}; +const approvalStatusFieldAccessConfig = approvalStatusFieldAccess("vehicles"); export const Vehicles: CollectionConfig = { slug: "vehicles", @@ -42,7 +40,7 @@ export const Vehicles: CollectionConfig = { name: "approvalStatus", type: "select", label: "Approval Status", - access: approvalFieldAccess, + access: approvalStatusFieldAccessConfig, options: [ { label: "In Progress", value: "in_progress" }, { label: "Ready for Review", value: "ready_for_review" }, diff --git a/src/collections/users/Users.ts b/src/collections/users/Users.ts index 994e7ed..a64b797 100644 --- a/src/collections/users/Users.ts +++ b/src/collections/users/Users.ts @@ -3,6 +3,7 @@ import { requirePermission, hasPermission, isSuperuser, + canAccessAdminPanel, } from "@/utils/access-control/hasPermission"; import { ensurePersonalAccount } from "@/lib/banking/index"; import { MUTEABLE_NOTIFICATION_TYPES } from "@/lib/notifications/notificationTypes"; @@ -88,7 +89,10 @@ export const Users: CollectionConfig = { }, slug: "users", access: { - admin: requirePermission("system:admin-access"), + // Admin panel entry: superusers, plus anyone holding a page-manage + // permission (division roles). Per-page visibility is filtered separately + // by requireAdminPageAccess (read + admin::manage per collection). + admin: async ({ req }) => canAccessAdminPanel(req.payload, req.user), unlock: requirePermission("users:unlock"), create: requirePermission("users:create"), update: async ({ req, id, data }) => { diff --git a/src/permissions/index.ts b/src/permissions/index.ts index c9de6eb..0c57ff3 100644 --- a/src/permissions/index.ts +++ b/src/permissions/index.ts @@ -34,7 +34,7 @@ export interface PermissionGroup { export const PERMISSION_GROUPS: PermissionGroup[] = [ { group: "System", - permissions: [{ value: "system:admin-access", label: "Access Admin Panel" }], + permissions: [{ value: "system:admin-access", label: "Superuser Tier (user management, notification oversight, final approval states)" }], }, // ---- Media --------------------------------------------------------------- diff --git a/src/utils/access-control/divisionAccess.ts b/src/utils/access-control/divisionAccess.ts index e0de91c..d0edee5 100644 --- a/src/utils/access-control/divisionAccess.ts +++ b/src/utils/access-control/divisionAccess.ts @@ -27,15 +27,20 @@ import { hasLogisticsQualification } from "@/utils/access-control/hasLogisticsQu * get full admin-panel access to exactly these collections (and nothing else, * because `requireAdminPageAccess` still gates every other collection's * admin-panel read behind `admin::manage` permissions). + * + * NOTE: `assets`, `resources`, and `vehicles` are intentionally NOT here. + * Admin-panel access to those collections is gated solely by the + * `admin::manage` "Manage Admin Page" permission (via + * `requireAdminPageAccess`), not by division membership. This prevents + * division members (e.g. an intelligence-officer role that also holds the + * logistics qualification) from viewing those backend pages without the + * explicit manage permission. */ const DIVISION_ADMIN_QUALIFICATIONS: Record< string, (payload: Payload, user: { id: number | string }) => Promise > = { technologies: hasIntelligenceQualification, - assets: hasLogisticsQualification, - resources: hasLogisticsQualification, - vehicles: hasLogisticsQualification, }; /** @@ -76,6 +81,34 @@ export function requireApprovalPermission() { hasPermission(req.payload, req.user, "system:admin-access"); } +/** + * Field-level access factory for approval-status select fields. + * + * Anyone with the collection's update permission may set the status to + * "In Progress" or "Ready for Review". Only superusers (holders of + * `system:admin-access`) may set it to final states ("Approved", + * "Rejected", "Revision Requested"). + */ +export function approvalStatusFieldAccess(collectionSlug: string) { + const fn = async ({ req, data }: { req: PayloadRequest; data?: Record }): Promise => { + const value = data?.approvalStatus as string | undefined; + const isSuperuser = await hasPermission(req.payload, req.user, "system:admin-access"); + if (isSuperuser) return true; + + const allowedValues = ["in_progress", "ready_for_review"]; + if (!value || allowedValues.includes(value)) { + return await hasPermission(req.payload, req.user, `${collectionSlug}:update` as Permission); + } + + return false; + }; + + return { + create: fn, + update: fn, + }; +} + /** * Same contract as `requireAdminPageAccess`, with one addition: for the four * division-scoped collections an admin-panel request also passes when the user diff --git a/tests/int/division-admin-access.int.spec.ts b/tests/int/division-admin-access.int.spec.ts index 40ea9f6..0a08613 100644 --- a/tests/int/division-admin-access.int.spec.ts +++ b/tests/int/division-admin-access.int.spec.ts @@ -6,6 +6,7 @@ import { afterAll, beforeAll, describe, expect, it } from "vitest"; import type { Role, User } from "@/payload-types"; import { + approvalStatusFieldAccess, canAccessAdminPanel, requireApprovalPermission, requireIntelligencePermission, @@ -32,6 +33,7 @@ describe("Division-scoped admin access (intelligence / logistics)", () => { let logiUser: User; let plainUser: User; let superUser: User; + let editorUser: User; const makeRole = async (label: string, extra: Partial = {}): Promise => { const role = (await payload.create({ @@ -119,8 +121,12 @@ describe("Division-scoped admin access (intelligence / logistics)", () => { ); }; - const accessFnDecision = async (fn: (args: AccessArgs) => Promise, user: User) => - Boolean(await fn({ req: { payload, user } } as unknown as AccessArgs)); + const accessFnDecision = async ( + fn: (args: AccessArgs) => Promise, + user: User, + data?: Record, + ) => + Boolean(await fn({ req: { payload, user }, data } as unknown as AccessArgs & { data?: Record })); const expectAccessDenied = async (fn: () => Promise) => { try { @@ -143,11 +149,13 @@ describe("Division-scoped admin access (intelligence / logistics)", () => { const bareRole = await makeRole("bare", { permissions: [] }); const superRole = await makeRole("super", { isSuperuser: true }); + const editorRole = await makeRole("editor", { permissions: ["assets:update"] }); intelUser = await makeUser("intel", bareRole.id); logiUser = await makeUser("logi", bareRole.id); plainUser = await makeUser("plain", bareRole.id); superUser = await makeUser("super", superRole.id); + editorUser = await makeUser("editor", editorRole.id); const intelligenceId = await findOrCreateQualification("Intelligence"); const logisticsId = await findOrCreateQualification("Logistics"); @@ -201,15 +209,31 @@ describe("Division-scoped admin access (intelligence / logistics)", () => { expect(await adminDecision("missions", intelUser)).toBe(false); }, TIMEOUT); - it("logistics qualification grants assets, resources, and vehicles only", async () => { - expect(await adminDecision("assets", logiUser)).toBe(true); - expect(await adminDecision("resources", logiUser)).toBe(true); - expect(await adminDecision("vehicles", logiUser)).toBe(true); + it("logistics qualification does not grant admin page access", async () => { + // Admin-panel access to assets/resources/vehicles is gated by the + // `admin::manage` "Manage Admin Page" permission, NOT by division + // membership. Logistics-qualified users must hold the explicit permission. + expect(await adminDecision("assets", logiUser)).toBe(false); + expect(await adminDecision("resources", logiUser)).toBe(false); + expect(await adminDecision("vehicles", logiUser)).toBe(false); expect(await adminDecision("technologies", logiUser)).toBe(false); expect(await adminDecision("missions", logiUser)).toBe(false); expect(await adminDecision("structures", logiUser)).toBe(false); }, TIMEOUT); + it("admin::manage + collection read permission grants the specific admin page", async () => { + // requireAdminPageAccess requires BOTH the collection read permission and + // the admin::manage permission. + const adminRole = await makeRole("admin-page", { + permissions: ["assets:read", "admin:assets:manage"], + }); + const adminUser = await makeUser("admin-page-user", adminRole.id); + expect(await adminDecision("assets", adminUser)).toBe(true); + expect(await adminDecision("resources", adminUser)).toBe(false); + expect(await adminDecision("vehicles", adminUser)).toBe(false); + expect(await adminDecision("technologies", adminUser)).toBe(false); + }, TIMEOUT); + it("plain users and anonymous users see none of the four", async () => { for (const slug of ["technologies", "assets", "resources", "vehicles"]) { expect(await adminDecision(slug, plainUser)).toBe(false); @@ -231,9 +255,13 @@ describe("Division-scoped admin access (intelligence / logistics)", () => { }); describe("admin panel gate (canAccessAdminPanel)", () => { - it("division members pass; plain users and anonymous users do not", async () => { + it("intelligence division members and superusers pass; logistics-only and plain users do not", async () => { + // With assets/resources/vehicles removed from DIVISION_ADMIN_QUALIFICATIONS, + // logistics-only members no longer pass the admin-panel gate. Only + // intelligence-qualified users (technologies), superusers, and holders of + // any `admin:*:manage` permission pass. expect(await canAccessAdminPanel(payload, intelUser)).toBe(true); - expect(await canAccessAdminPanel(payload, logiUser)).toBe(true); + expect(await canAccessAdminPanel(payload, logiUser)).toBe(false); expect(await canAccessAdminPanel(payload, superUser)).toBe(true); expect(await canAccessAdminPanel(payload, plainUser)).toBe(false); expect(await canAccessAdminPanel(payload, null)).toBe(false); @@ -447,5 +475,31 @@ describe("Division-scoped admin access (intelligence / logistics)", () => { expect(await accessFnDecision(approvalUpdate, intelUser)).toBe(false); expect(await accessFnDecision(approvalUpdate, plainUser)).toBe(false); }, TIMEOUT); + + it("approval-status fields allow non-superusers to set in-progress states, reserve final states for superusers", async () => { + const approvalStatusUpdate = approvalStatusFieldAccess("assets").update; + // Payload passes update data via `data` in AccessArgs, so check that path. + const data = (v: string) => ({ approvalStatus: v }); + // Superuser can set any value. + expect(await accessFnDecision(approvalStatusUpdate, superUser, data("approved"))).toBe(true); + expect(await accessFnDecision(approvalStatusUpdate, superUser, data("rejected"))).toBe(true); + expect( + await accessFnDecision(approvalStatusUpdate, superUser, data("revision_requested")), + ).toBe(true); + // Editor (has assets:update) can set the allowed in-progress states. + expect(await accessFnDecision(approvalStatusUpdate, editorUser, data("in_progress"))).toBe(true); + expect( + await accessFnDecision(approvalStatusUpdate, editorUser, data("ready_for_review")), + ).toBe(true); + // Editor cannot set final states. + expect(await accessFnDecision(approvalStatusUpdate, editorUser, data("approved"))).toBe(false); + expect(await accessFnDecision(approvalStatusUpdate, editorUser, data("rejected"))).toBe(false); + expect( + await accessFnDecision(approvalStatusUpdate, editorUser, data("revision_requested")), + ).toBe(false); + // Non-editor without superuser cannot set any value. + expect(await accessFnDecision(approvalStatusUpdate, plainUser, data("in_progress"))).toBe(false); + expect(await accessFnDecision(approvalStatusUpdate, plainUser, data("approved"))).toBe(false); + }, TIMEOUT); }); });