diff --git a/e2e/scenarios/policy-remove-confirm.test.ts b/e2e/scenarios/policy-remove-confirm.test.ts new file mode 100644 index 000000000..8e6f10382 --- /dev/null +++ b/e2e/scenarios/policy-remove-confirm.test.ts @@ -0,0 +1,85 @@ +// Cross-target (browser): removing a tool-access policy asks for confirmation +// first. Remove fires from a row's dropdown menu and is irreversible โ€” the +// rule is gone and every tool it governed silently falls back to the default +// policy โ€” so the menu item must open a confirm dialog rather than firing the +// mutation directly. Cancel keeps the rule; confirming removes it for real +// (asserted through the API, not just the rendered list). +import { randomBytes } from "node:crypto"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; + +import { scenario } from "../src/scenario"; +import { Api, Browser, Target } from "../src/services"; +import { visit } from "../src/surfaces/browser"; + +const coreApi = composePluginApi([] as const); + +scenario( + "Policies (UI) ยท Remove asks for confirmation; cancel keeps, confirm removes", + {}, + Effect.gen(function* () { + const target = yield* Target; + const browser = yield* Browser; + const { client: makeClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* makeClient(coreApi, identity); + + // Selfhost scenarios share one workspace, so the pattern is unique to this + // run and the row lookup below cannot match another scenario's rule. + const pattern = `rmconfirm-${randomBytes(4).toString("hex")}.*`; + + const created = yield* client.policies.create({ + payload: { owner: "org", pattern, action: "block" }, + }); + + yield* Effect.ensuring( + Effect.gen(function* () { + yield* browser.session(identity, async ({ page, step }) => { + const row = page.locator("[data-slot='card-stack-entry']").filter({ hasText: pattern }); + const menuTrigger = row.locator('button[aria-haspopup="menu"]'); + const confirm = page.getByRole("alertdialog"); + + await step("Open the policies page", async () => { + await visit(page, "/policies"); + await row.waitFor(); + }); + + await step("Remove asks for confirmation instead of firing", async () => { + await menuTrigger.click(); + await page.getByRole("menuitem", { name: "Remove" }).click(); + await confirm.getByText("Remove policy?").waitFor(); + // The dialog names the exact rule being destroyed. + await confirm.getByText(pattern, { exact: false }).waitFor(); + }); + + await step("Cancel keeps the policy", async () => { + await confirm.getByRole("button", { name: "Cancel" }).click(); + await confirm.waitFor({ state: "detached" }); + await row.waitFor(); + }); + + await step("Confirming actually removes it", async () => { + await menuTrigger.click(); + await page.getByRole("menuitem", { name: "Remove" }).click(); + await confirm.getByRole("button", { name: "Remove policy" }).click(); + await confirm.waitFor({ state: "detached" }); + await row.waitFor({ state: "detached" }); + }); + }); + + // Cancel left the removal un-fired and confirm fired it for real: the + // rule is gone from the API, not merely from the rendered list. + const remaining = yield* client.policies.list(); + expect( + remaining.map((policy) => policy.pattern), + "the removed policy is gone from the API", + ).not.toContain(pattern); + }), + client.policies + .remove({ params: { policyId: created.id }, payload: { owner: "org" } }) + .pipe(Effect.ignore), + ); + }), +); diff --git a/packages/react/src/pages/api-keys.tsx b/packages/react/src/pages/api-keys.tsx index eb15084cb..b48e04856 100644 --- a/packages/react/src/pages/api-keys.tsx +++ b/packages/react/src/pages/api-keys.tsx @@ -233,6 +233,7 @@ export function ApiKeysPage(props: { readonly orgKeysSection?: ReactNode }) { // stays mounted for Radix's exit animation (see CreateKeyDialogBody). const [openCount, setOpenCount] = useState(0); const [revokingId, setRevokingId] = useState(null); + const [confirmRevoke, setConfirmRevoke] = useState(null); const handleCreate = async (name: string): Promise => { const exit = await doCreate({ payload: { name }, reactivityKeys: apiKeyWriteKeys }); @@ -312,7 +313,11 @@ export function ApiKeysPage(props: { readonly orgKeysSection?: ReactNode }) {

) : ( - + setConfirmRevoke(key)} + /> ), }) )} @@ -333,6 +338,42 @@ export function ApiKeysPage(props: { readonly orgKeysSection?: ReactNode }) { /> + + {/* A personal key revoke breaks any script or tool using it, so it asks + first โ€” same as the org-key revoke. */} + { + if (!open) setConfirmRevoke(null); + }} + > + + + Revoke API key + + {confirmRevoke + ? `Revoke ${confirmRevoke.name}? Any script or tool authenticating with it loses access immediately. This cannot be undone.` + : ""} + + + + + + + + + + ); } @@ -492,8 +533,7 @@ function OrgApiKeysSectionBody() { - {/* Revoking an org key breaks every backend using it, so it is the one - revoke on this page that asks first. */} + {/* Revoking an org key breaks every backend using it, so it asks first. */} { diff --git a/packages/react/src/pages/org.tsx b/packages/react/src/pages/org.tsx index 8797533af..670c0845f 100644 --- a/packages/react/src/pages/org.tsx +++ b/packages/react/src/pages/org.tsx @@ -14,6 +14,16 @@ import { DialogFooter, DialogClose, } from "../components/dialog"; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "../components/alert-dialog"; import { Button } from "../components/button"; import { PageContainer, PageHeader } from "../components/page"; import { Badge } from "../components/badge"; @@ -153,6 +163,7 @@ export function OrgPage(props: { const refreshMembers = useAtomRefresh(orgMembersAtom); const rolesResult = useAtomValue(orgRolesAtom); const doRemove = useAtomSet(removeMember, { mode: "promiseExit" }); + const [removingMember, setRemovingMember] = useState<{ id: string; name: string } | null>(null); const doUpdateRole = useAtomSet(updateMemberRole, { mode: "promiseExit" }); const doUpdateOrgName = useAtomSet(updateOrgName, { mode: "promiseExit" }); const [inviteOpen, setInviteOpen] = useState(false); @@ -179,6 +190,7 @@ export function OrgPage(props: { const showUpgradeOnInvite = atSeatLimit && !!props.upgradeAction; const handleRemove = async (membershipId: string, name: string) => { + setRemovingMember(null); const exit = await doRemove({ params: { membershipId }, reactivityKeys: orgMemberWriteKeys, @@ -406,7 +418,12 @@ export function OrgPage(props: { )} handleRemove(member.id, member.name ?? member.email)} + onClick={() => + setRemovingMember({ + id: member.id, + name: member.name ?? member.email, + }) + } > Remove member @@ -426,6 +443,38 @@ export function OrgPage(props: { {props.dangerZoneSection} + { + if (!open) setRemovingMember(null); + }} + > + + + + {removingMember ? `Remove ${removingMember.name}?` : "Remove member?"} + + + They lose access to this organization immediately. This cannot be undone; you would + need to invite them again. + + + + Cancel + { + if (removingMember !== null) { + void handleRemove(removingMember.id, removingMember.name); + } + }} + > + Remove member + + + + + (null); const ownerDisplay = useOwnerDisplay(); // Policies default to org/workspace. On local this is the hidden Local owner // that v1 local data migrates into. @@ -336,6 +351,7 @@ export function PoliciesPage() { }; const handleRemove = async (policy: { id: string; owner: Owner }) => { + setRemovingPolicy(null); const exit = await doRemove({ params: { policyId: PolicyId.make(policy.id) }, payload: { owner: policy.owner }, @@ -462,7 +478,9 @@ export function PoliciesPage() { isFirst={!reorderable || j === 0} isLast={!reorderable || j === committed.length - 1} showOwnerLabel={ownerDisplay.showOwnerLabels} - onRemove={() => handleRemove({ id: p.id, owner: p.owner })} + onRemove={() => + setRemovingPolicy({ id: p.id, owner: p.owner, pattern: p.pattern }) + } onChangeAction={(action) => handleUpdate({ id: p.id, owner: p.owner }, action) } @@ -490,6 +508,37 @@ export function PoliciesPage() { }, }) )} + + { + if (!open) setRemovingPolicy(null); + }} + > + + + Remove policy? + + {removingPolicy + ? `The rule for "${removingPolicy.pattern}" will be deleted. Tools it governs fall back to the default policy. This cannot be undone.` + : ""} + + + + Cancel + { + if (removingPolicy !== null) { + void handleRemove({ id: removingPolicy.id, owner: removingPolicy.owner }); + } + }} + > + Remove policy + + + + ); }