diff --git a/app/admin/members/page.tsx b/app/admin/members/page.tsx index 1fa5cf6..567950f 100644 --- a/app/admin/members/page.tsx +++ b/app/admin/members/page.tsx @@ -24,6 +24,7 @@ import { applyOptimisticRole, applyOptimisticRemoveRole, } from "@/lib/api/optimistic"; +import { roleRemovalConfirmationMessage } from "@/lib/api/role-removal"; import { AddressText } from "@/components/wallet/address-text"; import { isWalletAddress, normalizeAddress } from "@/lib/wallet/address"; @@ -202,6 +203,21 @@ export default function MembersPage() { }, }); + const requestRoleRemoval = (member: MemberRow, roleToRemove: Role) => { + const confirmationMessage = roleRemovalConfirmationMessage( + member.address, + roleToRemove, + member.roles, + ); + + if (confirmationMessage && !window.confirm(confirmationMessage)) return; + + removeRoleMutation.mutate({ + address: member.address, + role: roleToRemove, + }); + }; + return (
@@ -281,7 +297,7 @@ export default function MembersPage() { className="text-sm text-green-700 dark:text-green-400" role="status" > - Role "{successAssignment.role}" saved for{" "} + Role "{successAssignment.role}" saved for{" "} - removeRoleMutation.mutate({ - address: m.address, - role: r, - }) - } + onClick={() => requestRoleRemoval(m, r)} aria-label={`Remove ${r} role from ${m.address}`} title={`Remove ${r} role`} > diff --git a/lib/api/role-removal.ts b/lib/api/role-removal.ts new file mode 100644 index 0000000..989cf0f --- /dev/null +++ b/lib/api/role-removal.ts @@ -0,0 +1,29 @@ +import type { Role } from './types' + +/** + * Returns true when removing a role should require an explicit confirmation. + * + * Admin removal is sensitive because it changes governance access. Removing a + * member's last role is also sensitive because it can leave the member without + * any role-based access path. + */ +export function roleRemovalNeedsConfirmation( + role: Role, + currentRoles: readonly Role[], +): boolean { + return role === 'admin' || currentRoles.length <= 1 +} + +export function roleRemovalConfirmationMessage( + address: string, + role: Role, + currentRoles: readonly Role[], +): string | null { + if (!roleRemovalNeedsConfirmation(role, currentRoles)) return null + + const reasons = [] + if (role === 'admin') reasons.push('the admin role') + if (currentRoles.length <= 1) reasons.push("the member's last remaining role") + + return `Remove ${role} role from ${address}? This removes ${reasons.join(' and ')}.` +} diff --git a/test/role-removal-confirmation.test.ts b/test/role-removal-confirmation.test.ts new file mode 100644 index 0000000..bb94b8f --- /dev/null +++ b/test/role-removal-confirmation.test.ts @@ -0,0 +1,37 @@ +import { test } from 'node:test' +import * as assert from 'node:assert/strict' +import { + roleRemovalConfirmationMessage, + roleRemovalNeedsConfirmation, +} from '../lib/api/role-removal' + +test('role removal confirmation is not required for non-sensitive extra roles', () => { + assert.equal(roleRemovalNeedsConfirmation('moderator', ['member', 'moderator']), false) + assert.equal( + roleRemovalConfirmationMessage('0xabc', 'moderator', ['member', 'moderator']), + null, + ) +}) + +test('role removal confirmation is required for admin roles', () => { + assert.equal(roleRemovalNeedsConfirmation('admin', ['member', 'admin']), true) + assert.equal( + roleRemovalConfirmationMessage('0xabc', 'admin', ['member', 'admin']), + 'Remove admin role from 0xabc? This removes the admin role.', + ) +}) + +test('role removal confirmation is required for a member last role', () => { + assert.equal(roleRemovalNeedsConfirmation('member', ['member']), true) + assert.equal( + roleRemovalConfirmationMessage('0xabc', 'member', ['member']), + "Remove member role from 0xabc? This removes the member's last remaining role.", + ) +}) + +test('role removal confirmation explains when admin is also the last role', () => { + assert.equal( + roleRemovalConfirmationMessage('0xabc', 'admin', ['admin']), + "Remove admin role from 0xabc? This removes the admin role and the member's last remaining role.", + ) +})