fix(clients): archiving is edit-level, not delete (unblocks Sales/Director)
Every client-archive route gated on clients:delete, which only super_admin holds, so Sales Agent/Manager and Director users (clients.delete=false) got 403 on archive — the button was shown but never worked. Archiving is reversible, so gate it on clients:edit instead; permanent deletion stays the separate admin:permanently_delete_clients permission. - New single source of truth CLIENT_ARCHIVE_ACTION='edit' (permissions.ts). - All 5 archive routes use it: [id] DELETE, archive, archive-dossier, bulk-archive-preflight, bulk POST. - UI affordances gate on clients:edit (detail-header archive/restore, bulk archive, per-row list + mobile-card archive) so view-only users don't see a button that 403s. - permission-matrix regression test locks the policy (Sales/Director/Agent can archive; Viewer cannot). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L2qc3xZTfif7N4Wq3QDa8X
This commit is contained in:
@@ -1,6 +1,7 @@
|
|||||||
import { NextResponse } from 'next/server';
|
import { NextResponse } from 'next/server';
|
||||||
|
|
||||||
import { withAuth, withPermission } from '@/lib/api/helpers';
|
import { withAuth, withPermission } from '@/lib/api/helpers';
|
||||||
|
import { CLIENT_ARCHIVE_ACTION } from '@/lib/auth/permissions';
|
||||||
import { getClientArchiveDossier } from '@/lib/services/client-archive-dossier.service';
|
import { getClientArchiveDossier } from '@/lib/services/client-archive-dossier.service';
|
||||||
import { errorResponse, NotFoundError } from '@/lib/errors';
|
import { errorResponse, NotFoundError } from '@/lib/errors';
|
||||||
|
|
||||||
@@ -10,7 +11,7 @@ import { errorResponse, NotFoundError } from '@/lib/errors';
|
|||||||
* decision points, and warnings.
|
* decision points, and warnings.
|
||||||
*/
|
*/
|
||||||
export const GET = withAuth(
|
export const GET = withAuth(
|
||||||
withPermission('clients', 'delete', async (_req, ctx, params) => {
|
withPermission('clients', CLIENT_ARCHIVE_ACTION, async (_req, ctx, params) => {
|
||||||
try {
|
try {
|
||||||
const id = params.id;
|
const id = params.id;
|
||||||
if (!id) throw new NotFoundError('client');
|
if (!id) throw new NotFoundError('client');
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import { NextResponse } from 'next/server';
|
|||||||
import { z } from 'zod';
|
import { z } from 'zod';
|
||||||
|
|
||||||
import { withAuth, withPermission } from '@/lib/api/helpers';
|
import { withAuth, withPermission } from '@/lib/api/helpers';
|
||||||
|
import { CLIENT_ARCHIVE_ACTION } from '@/lib/auth/permissions';
|
||||||
import { parseBody } from '@/lib/api/route-helpers';
|
import { parseBody } from '@/lib/api/route-helpers';
|
||||||
import {
|
import {
|
||||||
archiveClientWithDecisions,
|
archiveClientWithDecisions,
|
||||||
@@ -63,7 +64,7 @@ const decisionsSchema = z.object({
|
|||||||
});
|
});
|
||||||
|
|
||||||
export const POST = withAuth(
|
export const POST = withAuth(
|
||||||
withPermission('clients', 'delete', async (req, ctx, params) => {
|
withPermission('clients', CLIENT_ARCHIVE_ACTION, async (req, ctx, params) => {
|
||||||
try {
|
try {
|
||||||
const id = params.id;
|
const id = params.id;
|
||||||
if (!id) throw new NotFoundError('client');
|
if (!id) throw new NotFoundError('client');
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
import { NextResponse } from 'next/server';
|
import { NextResponse } from 'next/server';
|
||||||
|
|
||||||
import { withAuth, withPermission } from '@/lib/api/helpers';
|
import { withAuth, withPermission } from '@/lib/api/helpers';
|
||||||
|
import { CLIENT_ARCHIVE_ACTION } from '@/lib/auth/permissions';
|
||||||
import { parseBody } from '@/lib/api/route-helpers';
|
import { parseBody } from '@/lib/api/route-helpers';
|
||||||
import { errorResponse } from '@/lib/errors';
|
import { errorResponse } from '@/lib/errors';
|
||||||
import { getClientById, updateClient, archiveClient } from '@/lib/services/clients.service';
|
import { getClientById, updateClient, archiveClient } from '@/lib/services/clients.service';
|
||||||
@@ -35,7 +36,7 @@ export const PATCH = withAuth(
|
|||||||
);
|
);
|
||||||
|
|
||||||
export const DELETE = withAuth(
|
export const DELETE = withAuth(
|
||||||
withPermission('clients', 'delete', async (req, ctx, params) => {
|
withPermission('clients', CLIENT_ARCHIVE_ACTION, async (req, ctx, params) => {
|
||||||
try {
|
try {
|
||||||
await archiveClient(params.id!, ctx.portId, {
|
await archiveClient(params.id!, ctx.portId, {
|
||||||
userId: ctx.userId,
|
userId: ctx.userId,
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import { NextResponse } from 'next/server';
|
|||||||
import { z } from 'zod';
|
import { z } from 'zod';
|
||||||
|
|
||||||
import { withAuth, withPermission } from '@/lib/api/helpers';
|
import { withAuth, withPermission } from '@/lib/api/helpers';
|
||||||
|
import { CLIENT_ARCHIVE_ACTION } from '@/lib/auth/permissions';
|
||||||
import { parseBody } from '@/lib/api/route-helpers';
|
import { parseBody } from '@/lib/api/route-helpers';
|
||||||
import { getClientArchiveDossier } from '@/lib/services/client-archive-dossier.service';
|
import { getClientArchiveDossier } from '@/lib/services/client-archive-dossier.service';
|
||||||
import { errorResponse } from '@/lib/errors';
|
import { errorResponse } from '@/lib/errors';
|
||||||
@@ -27,7 +28,7 @@ interface PreflightItem {
|
|||||||
* - surface blockers (e.g. "has unpaid invoices") for the operator
|
* - surface blockers (e.g. "has unpaid invoices") for the operator
|
||||||
*/
|
*/
|
||||||
export const POST = withAuth(
|
export const POST = withAuth(
|
||||||
withPermission('clients', 'delete', async (req, ctx) => {
|
withPermission('clients', CLIENT_ARCHIVE_ACTION, async (req, ctx) => {
|
||||||
try {
|
try {
|
||||||
const { ids } = await parseBody(req, bodySchema);
|
const { ids } = await parseBody(req, bodySchema);
|
||||||
const items: PreflightItem[] = [];
|
const items: PreflightItem[] = [];
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ import { z } from 'zod';
|
|||||||
import { eq, and } from 'drizzle-orm';
|
import { eq, and } from 'drizzle-orm';
|
||||||
|
|
||||||
import { withAuth, withRateLimit } from '@/lib/api/helpers';
|
import { withAuth, withRateLimit } from '@/lib/api/helpers';
|
||||||
|
import { CLIENT_ARCHIVE_ACTION } from '@/lib/auth/permissions';
|
||||||
import { parseBody } from '@/lib/api/route-helpers';
|
import { parseBody } from '@/lib/api/route-helpers';
|
||||||
import { runBulk } from '@/lib/api/bulk-helpers';
|
import { runBulk } from '@/lib/api/bulk-helpers';
|
||||||
import { db } from '@/lib/db';
|
import { db } from '@/lib/db';
|
||||||
@@ -43,7 +44,9 @@ const bulkSchema = z.discriminatedUnion('action', [
|
|||||||
]);
|
]);
|
||||||
|
|
||||||
const PERMISSION_BY_ACTION = {
|
const PERMISSION_BY_ACTION = {
|
||||||
archive: 'delete' as const,
|
// Archiving is reversible -> edit-level (see CLIENT_ARCHIVE_ACTION). Not the
|
||||||
|
// destructive `delete`, which only super_admin holds.
|
||||||
|
archive: CLIENT_ARCHIVE_ACTION,
|
||||||
add_tag: 'edit' as const,
|
add_tag: 'edit' as const,
|
||||||
remove_tag: 'edit' as const,
|
remove_tag: 'edit' as const,
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -27,9 +27,17 @@ interface ClientCardProps {
|
|||||||
portSlug: string;
|
portSlug: string;
|
||||||
onEdit: (client: ClientRow) => void;
|
onEdit: (client: ClientRow) => void;
|
||||||
onArchive: (client: ClientRow) => void;
|
onArchive: (client: ClientRow) => void;
|
||||||
|
/** Hide the Archive action for users who can't archive (clients:edit). */
|
||||||
|
canArchive?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function ClientCard({ client, portSlug, onEdit, onArchive }: ClientCardProps) {
|
export function ClientCard({
|
||||||
|
client,
|
||||||
|
portSlug,
|
||||||
|
onEdit,
|
||||||
|
onArchive,
|
||||||
|
canArchive = true,
|
||||||
|
}: ClientCardProps) {
|
||||||
// Card display: prefer email, fall back to phone.
|
// Card display: prefer email, fall back to phone.
|
||||||
const primaryContactValue = client.primaryEmail ?? client.primaryPhone ?? null;
|
const primaryContactValue = client.primaryEmail ?? client.primaryPhone ?? null;
|
||||||
const nationality = client.nationalityIso ? getCountryName(client.nationalityIso, 'en') : null;
|
const nationality = client.nationalityIso ? getCountryName(client.nationalityIso, 'en') : null;
|
||||||
@@ -96,10 +104,12 @@ export function ClientCard({ client, portSlug, onEdit, onArchive }: ClientCardPr
|
|||||||
<Pencil className="mr-2 h-3.5 w-3.5" aria-hidden />
|
<Pencil className="mr-2 h-3.5 w-3.5" aria-hidden />
|
||||||
Edit
|
Edit
|
||||||
</DropdownMenuItem>
|
</DropdownMenuItem>
|
||||||
<DropdownMenuItem className="text-destructive" onClick={() => onArchive(client)}>
|
{canArchive && (
|
||||||
<Archive className="mr-2 h-3.5 w-3.5" aria-hidden />
|
<DropdownMenuItem className="text-destructive" onClick={() => onArchive(client)}>
|
||||||
Archive
|
<Archive className="mr-2 h-3.5 w-3.5" aria-hidden />
|
||||||
</DropdownMenuItem>
|
Archive
|
||||||
|
</DropdownMenuItem>
|
||||||
|
)}
|
||||||
</DropdownMenuContent>
|
</DropdownMenuContent>
|
||||||
</DropdownMenu>
|
</DropdownMenu>
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -87,12 +87,15 @@ interface GetColumnsOptions {
|
|||||||
portSlug: string;
|
portSlug: string;
|
||||||
onEdit: (client: ClientRow) => void;
|
onEdit: (client: ClientRow) => void;
|
||||||
onArchive: (client: ClientRow) => void;
|
onArchive: (client: ClientRow) => void;
|
||||||
|
/** Hide the row Archive action for users who can't archive (clients:edit). */
|
||||||
|
canArchive?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function getClientColumns({
|
export function getClientColumns({
|
||||||
portSlug,
|
portSlug,
|
||||||
onEdit,
|
onEdit,
|
||||||
onArchive,
|
onArchive,
|
||||||
|
canArchive = true,
|
||||||
}: GetColumnsOptions): ColumnDef<ClientRow, unknown>[] {
|
}: GetColumnsOptions): ColumnDef<ClientRow, unknown>[] {
|
||||||
return [
|
return [
|
||||||
{
|
{
|
||||||
@@ -318,10 +321,15 @@ export function getClientColumns({
|
|||||||
<Pencil className="mr-2 h-3.5 w-3.5" aria-hidden />
|
<Pencil className="mr-2 h-3.5 w-3.5" aria-hidden />
|
||||||
Edit
|
Edit
|
||||||
</DropdownMenuItem>
|
</DropdownMenuItem>
|
||||||
<DropdownMenuItem className="text-destructive" onClick={() => onArchive(row.original)}>
|
{canArchive && (
|
||||||
<Archive className="mr-2 h-3.5 w-3.5" aria-hidden />
|
<DropdownMenuItem
|
||||||
Archive
|
className="text-destructive"
|
||||||
</DropdownMenuItem>
|
onClick={() => onArchive(row.original)}
|
||||||
|
>
|
||||||
|
<Archive className="mr-2 h-3.5 w-3.5" aria-hidden />
|
||||||
|
Archive
|
||||||
|
</DropdownMenuItem>
|
||||||
|
)}
|
||||||
</DropdownMenuContent>
|
</DropdownMenuContent>
|
||||||
</DropdownMenu>
|
</DropdownMenu>
|
||||||
),
|
),
|
||||||
|
|||||||
@@ -146,23 +146,28 @@ export function ClientDetailHeader({ client }: ClientDetailHeaderProps) {
|
|||||||
>
|
>
|
||||||
<Bell className="size-4" aria-hidden />
|
<Bell className="size-4" aria-hidden />
|
||||||
</button>
|
</button>
|
||||||
<button
|
{/* Archive + Restore are reversible -> gate on clients:edit, the
|
||||||
type="button"
|
same action the archive routes now require. Hidden for view-only
|
||||||
onClick={() => setArchiveOpen(true)}
|
users so they never click a button that 403s. */}
|
||||||
aria-label={isArchived ? 'Restore client' : 'Archive client'}
|
<PermissionGate resource="clients" action="edit">
|
||||||
title={isArchived ? 'Restore client' : 'Archive client'}
|
<button
|
||||||
className={cn(
|
type="button"
|
||||||
'shrink-0 rounded-md p-1.5 text-muted-foreground/70 transition-colors',
|
onClick={() => setArchiveOpen(true)}
|
||||||
'hover:bg-foreground/5',
|
aria-label={isArchived ? 'Restore client' : 'Archive client'}
|
||||||
isArchived ? 'hover:text-emerald-600' : 'hover:text-destructive',
|
title={isArchived ? 'Restore client' : 'Archive client'}
|
||||||
)}
|
className={cn(
|
||||||
>
|
'shrink-0 rounded-md p-1.5 text-muted-foreground/70 transition-colors',
|
||||||
{isArchived ? (
|
'hover:bg-foreground/5',
|
||||||
<RotateCcw className="size-4" aria-hidden />
|
isArchived ? 'hover:text-emerald-600' : 'hover:text-destructive',
|
||||||
) : (
|
)}
|
||||||
<Archive className="size-4" aria-hidden />
|
>
|
||||||
)}
|
{isArchived ? (
|
||||||
</button>
|
<RotateCcw className="size-4" aria-hidden />
|
||||||
|
) : (
|
||||||
|
<Archive className="size-4" aria-hidden />
|
||||||
|
)}
|
||||||
|
</button>
|
||||||
|
</PermissionGate>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
</DetailHeaderStrip>
|
</DetailHeaderStrip>
|
||||||
|
|||||||
@@ -101,7 +101,9 @@ export function ClientList() {
|
|||||||
|
|
||||||
const { can } = usePermissions();
|
const { can } = usePermissions();
|
||||||
const canHardDelete = can('admin', 'permanently_delete_clients');
|
const canHardDelete = can('admin', 'permanently_delete_clients');
|
||||||
const canBulkArchive = can('clients', 'delete');
|
// Archiving is reversible -> edit-level (see CLIENT_ARCHIVE_ACTION), not delete.
|
||||||
|
const canArchive = can('clients', 'edit');
|
||||||
|
const canBulkArchive = canArchive;
|
||||||
const canBulkTag = can('clients', 'edit');
|
const canBulkTag = can('clients', 'edit');
|
||||||
|
|
||||||
const {
|
const {
|
||||||
@@ -167,6 +169,7 @@ export function ClientList() {
|
|||||||
portSlug,
|
portSlug,
|
||||||
onEdit: (client) => setEditClient(client),
|
onEdit: (client) => setEditClient(client),
|
||||||
onArchive: (client) => setArchiveClient(client),
|
onArchive: (client) => setArchiveClient(client),
|
||||||
|
canArchive,
|
||||||
});
|
});
|
||||||
|
|
||||||
// Per-user column visibility, persisted into user_profiles.preferences
|
// Per-user column visibility, persisted into user_profiles.preferences
|
||||||
@@ -296,6 +299,7 @@ export function ClientList() {
|
|||||||
portSlug={portSlug}
|
portSlug={portSlug}
|
||||||
onEdit={setEditClient}
|
onEdit={setEditClient}
|
||||||
onArchive={setArchiveClient}
|
onArchive={setArchiveClient}
|
||||||
|
canArchive={canArchive}
|
||||||
/>
|
/>
|
||||||
)}
|
)}
|
||||||
emptyState={
|
emptyState={
|
||||||
|
|||||||
@@ -75,6 +75,20 @@ export const PERMISSION_CATALOG = {
|
|||||||
[R in PermissionResource]: ReadonlyArray<PermissionAction<R> & string>;
|
[R in PermissionResource]: ReadonlyArray<PermissionAction<R> & string>;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Permission action that gates ARCHIVING a client. Archiving is reversible
|
||||||
|
* (sets `archivedAt`; restorable via the Restore flow), so it is an edit-level
|
||||||
|
* action — deliberately NOT the destructive `clients:delete` (which historically
|
||||||
|
* only super_admin held, locking every Sales/Director user out of archiving),
|
||||||
|
* and NOT `admin:permanently_delete_clients` (irreversible hard delete).
|
||||||
|
*
|
||||||
|
* Single source of truth: every client-archive route (single DELETE, archive
|
||||||
|
* with-decisions, archive-dossier, bulk-archive-preflight, bulk POST) gates on
|
||||||
|
* this, and the UI affordances gate on `clients:edit` to match. See the
|
||||||
|
* permission-matrix archive-policy test.
|
||||||
|
*/
|
||||||
|
export const CLIENT_ARCHIVE_ACTION = 'edit' as const;
|
||||||
|
|
||||||
/** Every valid resource key, in catalog order. */
|
/** Every valid resource key, in catalog order. */
|
||||||
export const PERMISSION_RESOURCES = Object.keys(PERMISSION_CATALOG) as PermissionResource[];
|
export const PERMISSION_RESOURCES = Object.keys(PERMISSION_CATALOG) as PermissionResource[];
|
||||||
|
|
||||||
|
|||||||
@@ -15,6 +15,7 @@
|
|||||||
import { describe, it, expect, vi } from 'vitest';
|
import { describe, it, expect, vi } from 'vitest';
|
||||||
|
|
||||||
import { withPermission, deepMerge, type AuthContext } from '@/lib/api/helpers';
|
import { withPermission, deepMerge, type AuthContext } from '@/lib/api/helpers';
|
||||||
|
import { CLIENT_ARCHIVE_ACTION } from '@/lib/auth/permissions';
|
||||||
import {
|
import {
|
||||||
makeViewerPermissions,
|
makeViewerPermissions,
|
||||||
makeSalesAgentPermissions,
|
makeSalesAgentPermissions,
|
||||||
@@ -64,6 +65,43 @@ async function checkPermission(
|
|||||||
return response.status;
|
return response.status;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ─── client archive policy ─────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Archiving a client is REVERSIBLE (sets archivedAt; restorable), so it is an
|
||||||
|
* edit-level action — NOT the destructive `clients:delete` (which only
|
||||||
|
* super_admin holds) and NOT `admin:permanently_delete_clients` (hard delete).
|
||||||
|
* Regression guard: prod showed Sales Managers/Directors getting 403 on archive
|
||||||
|
* because every archive route gated on `delete`. All archive routes now gate on
|
||||||
|
* CLIENT_ARCHIVE_ACTION; if anyone flips it back to a permission the sales roles
|
||||||
|
* lack, these fail.
|
||||||
|
*/
|
||||||
|
describe('Permission Matrix - client archive policy', () => {
|
||||||
|
it('archiving is an edit-level action, not delete', () => {
|
||||||
|
expect(CLIENT_ARCHIVE_ACTION).toBe('edit');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('a Sales Manager can archive a client', async () => {
|
||||||
|
const ctx = makeCtx({ permissions: makeSalesManagerPermissions() });
|
||||||
|
expect(await checkPermission(ctx, 'clients', CLIENT_ARCHIVE_ACTION)).toBe(200);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('a Director can archive a client', async () => {
|
||||||
|
const ctx = makeCtx({ permissions: makeDirectorPermissions() });
|
||||||
|
expect(await checkPermission(ctx, 'clients', CLIENT_ARCHIVE_ACTION)).toBe(200);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('a Sales Agent can archive a client', async () => {
|
||||||
|
const ctx = makeCtx({ permissions: makeSalesAgentPermissions() });
|
||||||
|
expect(await checkPermission(ctx, 'clients', CLIENT_ARCHIVE_ACTION)).toBe(200);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('a Viewer cannot archive a client', async () => {
|
||||||
|
const ctx = makeCtx({ permissions: makeViewerPermissions() });
|
||||||
|
expect(await checkPermission(ctx, 'clients', CLIENT_ARCHIVE_ACTION)).toBe(403);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
// ─── super_admin ──────────────────────────────────────────────────────────────
|
// ─── super_admin ──────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
describe('Permission Matrix - super_admin', () => {
|
describe('Permission Matrix - super_admin', () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user