fix(frontend): harden owner action confirmation
This commit is contained in:
@@ -49,3 +49,17 @@
|
|||||||
- E2E remains environment-blocked until the Playwright Chromium browser is installed.
|
- E2E remains environment-blocked until the Playwright Chromium browser is installed.
|
||||||
- Existing Vitest runs emit pre-existing MSW unmatched-request and dialog-ref warnings; all
|
- Existing Vitest runs emit pre-existing MSW unmatched-request and dialog-ref warnings; all
|
||||||
assertions pass and this task does not modify those shared test/UI primitives.
|
assertions pass and this task does not modify those shared test/UI primitives.
|
||||||
|
|
||||||
|
## Review remediation
|
||||||
|
|
||||||
|
- A post-commit review correctly identified that matching `displayName` must never establish
|
||||||
|
ownership. The predicate now skips confirmation only when `session.author` exactly equals
|
||||||
|
`principal.subject`; all display-name matches and missing authors are conservative
|
||||||
|
cross-owner actions.
|
||||||
|
- Added RED/GREEN regressions where two principals share display name `Alice` but have distinct
|
||||||
|
subjects: both delete (with another session present, so select-all cannot mask the guard) and
|
||||||
|
archive require confirmation.
|
||||||
|
- Added `aria-pressed` to the My sessions / All sessions controls and asserts their selected state
|
||||||
|
before and after switching.
|
||||||
|
- Remediation verification: focused regressions passed; full frontend Vitest (44 files / 305
|
||||||
|
tests), `npx tsc -b`, `npm run build`, and `git diff --check` all passed.
|
||||||
|
|||||||
@@ -82,7 +82,7 @@ test("administrators can explicitly switch to all sessions and see owners", asyn
|
|||||||
let scope = "";
|
let scope = "";
|
||||||
server.use(
|
server.use(
|
||||||
http.get("http://localhost:8787/me", () =>
|
http.get("http://localhost:8787/me", () =>
|
||||||
HttpResponse.json({ issuer: "portal", subject: "alice", displayName: "Alice", isAdmin: true }),
|
HttpResponse.json({ issuer: "portal", subject: "alice-id", displayName: "Alice", isAdmin: true }),
|
||||||
),
|
),
|
||||||
http.get("http://localhost:8787/sessions", ({ request }) => {
|
http.get("http://localhost:8787/sessions", ({ request }) => {
|
||||||
scope = new URL(request.url).searchParams.get("scope") ?? "";
|
scope = new URL(request.url).searchParams.get("scope") ?? "";
|
||||||
@@ -94,20 +94,25 @@ test("administrators can explicitly switch to all sessions and see owners", asyn
|
|||||||
);
|
);
|
||||||
wrap();
|
wrap();
|
||||||
await screen.findByRole("button", { name: "All sessions" });
|
await screen.findByRole("button", { name: "All sessions" });
|
||||||
|
expect(screen.getByRole("button", { name: "My sessions" })).toHaveAttribute("aria-pressed", "true");
|
||||||
|
expect(screen.getByRole("button", { name: "All sessions" })).toHaveAttribute("aria-pressed", "false");
|
||||||
await userEvent.click(screen.getByRole("button", { name: "All sessions" }));
|
await userEvent.click(screen.getByRole("button", { name: "All sessions" }));
|
||||||
await waitFor(() => expect(scope).toBe("all"));
|
await waitFor(() => expect(scope).toBe("all"));
|
||||||
|
expect(screen.getByRole("button", { name: "My sessions" })).toHaveAttribute("aria-pressed", "false");
|
||||||
|
expect(screen.getByRole("button", { name: "All sessions" })).toHaveAttribute("aria-pressed", "true");
|
||||||
expect(await screen.findByText("Administrator view: all sessions")).toBeInTheDocument();
|
expect(await screen.findByText("Administrator view: all sessions")).toBeInTheDocument();
|
||||||
expect(screen.getByText("Owner: Bob")).toBeInTheDocument();
|
expect(screen.getByText("Owner: Bob")).toBeInTheDocument();
|
||||||
});
|
});
|
||||||
|
|
||||||
test("administrator confirms before deleting another owner's session", async () => {
|
test("administrator confirms before deleting a same-named user's session", async () => {
|
||||||
let deletes = 0;
|
let deletes = 0;
|
||||||
server.use(
|
server.use(
|
||||||
http.get("http://localhost:8787/me", () =>
|
http.get("http://localhost:8787/me", () =>
|
||||||
HttpResponse.json({ issuer: "portal", subject: "alice", displayName: "Alice", isAdmin: true }),
|
HttpResponse.json({ issuer: "portal", subject: "alice-id", displayName: "Alice", isAdmin: true }),
|
||||||
),
|
),
|
||||||
http.get("http://localhost:8787/sessions", () => HttpResponse.json([
|
http.get("http://localhost:8787/sessions", () => HttpResponse.json([
|
||||||
{ ...LIST[0], author: "Bob" },
|
{ ...LIST[0], author: "Alice" },
|
||||||
|
{ ...LIST[1], id: "s3", question: "Second session", archived: false, author: "Bob" },
|
||||||
])),
|
])),
|
||||||
http.delete("http://localhost:8787/sessions/:id", () => {
|
http.delete("http://localhost:8787/sessions/:id", () => {
|
||||||
deletes += 1;
|
deletes += 1;
|
||||||
@@ -116,7 +121,7 @@ test("administrator confirms before deleting another owner's session", async ()
|
|||||||
);
|
);
|
||||||
wrap();
|
wrap();
|
||||||
await userEvent.click(await screen.findByRole("button", { name: "All sessions" }));
|
await userEvent.click(await screen.findByRole("button", { name: "All sessions" }));
|
||||||
await screen.findByText("Owner: Bob");
|
await screen.findByText("Owner: Alice");
|
||||||
await userEvent.click(screen.getByRole("checkbox", { name: "Select Attiva uno" }));
|
await userEvent.click(screen.getByRole("checkbox", { name: "Select Attiva uno" }));
|
||||||
await userEvent.click(screen.getByRole("button", { name: "Delete 1 selected sessions" }));
|
await userEvent.click(screen.getByRole("button", { name: "Delete 1 selected sessions" }));
|
||||||
expect(deletes).toBe(0);
|
expect(deletes).toBe(0);
|
||||||
@@ -125,15 +130,15 @@ test("administrator confirms before deleting another owner's session", async ()
|
|||||||
await waitFor(() => expect(deletes).toBe(1));
|
await waitFor(() => expect(deletes).toBe(1));
|
||||||
});
|
});
|
||||||
|
|
||||||
test("administrator confirms before archiving another owner's session", async () => {
|
test("administrator confirms before archiving a same-named user's session", async () => {
|
||||||
let archives = 0;
|
let archives = 0;
|
||||||
const confirm = vi.spyOn(window, "confirm").mockReturnValue(false);
|
const confirm = vi.spyOn(window, "confirm").mockReturnValue(false);
|
||||||
server.use(
|
server.use(
|
||||||
http.get("http://localhost:8787/me", () =>
|
http.get("http://localhost:8787/me", () =>
|
||||||
HttpResponse.json({ issuer: "portal", subject: "alice", displayName: "Alice", isAdmin: true }),
|
HttpResponse.json({ issuer: "portal", subject: "alice-id", displayName: "Alice", isAdmin: true }),
|
||||||
),
|
),
|
||||||
http.get("http://localhost:8787/sessions", () => HttpResponse.json([
|
http.get("http://localhost:8787/sessions", () => HttpResponse.json([
|
||||||
{ ...LIST[0], author: "Bob" },
|
{ ...LIST[0], author: "Alice" },
|
||||||
])),
|
])),
|
||||||
http.post("http://localhost:8787/sessions/:id/archive", () => {
|
http.post("http://localhost:8787/sessions/:id/archive", () => {
|
||||||
archives += 1;
|
archives += 1;
|
||||||
@@ -142,10 +147,10 @@ test("administrator confirms before archiving another owner's session", async ()
|
|||||||
);
|
);
|
||||||
wrap();
|
wrap();
|
||||||
await userEvent.click(await screen.findByRole("button", { name: "All sessions" }));
|
await userEvent.click(await screen.findByRole("button", { name: "All sessions" }));
|
||||||
await screen.findByText("Owner: Bob");
|
await screen.findByText("Owner: Alice");
|
||||||
await userEvent.click(screen.getByRole("button", { name: "Session actions" }));
|
await userEvent.click(screen.getByRole("button", { name: "Session actions" }));
|
||||||
await userEvent.click(await screen.findByText("Archive"));
|
await userEvent.click(await screen.findByText("Archive"));
|
||||||
expect(confirm).toHaveBeenCalledWith("Archive Bob's session?");
|
expect(confirm).toHaveBeenCalledWith("Archive Alice's session?");
|
||||||
expect(archives).toBe(0);
|
expect(archives).toBe(0);
|
||||||
confirm.mockRestore();
|
confirm.mockRestore();
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -83,7 +83,7 @@ export function AppShell() {
|
|||||||
const isForeignSession = (session: SessionSummary) => {
|
const isForeignSession = (session: SessionSummary) => {
|
||||||
if (!showingAllSessions || !principal) return false;
|
if (!showingAllSessions || !principal) return false;
|
||||||
if (!session.author) return true;
|
if (!session.author) return true;
|
||||||
return session.author !== principal.subject && session.author !== principal.displayName;
|
return session.author !== principal.subject;
|
||||||
};
|
};
|
||||||
|
|
||||||
function selectActiveSession(id: string | null) {
|
function selectActiveSession(id: string | null) {
|
||||||
@@ -526,6 +526,7 @@ export function AppShell() {
|
|||||||
<Button
|
<Button
|
||||||
variant={sessionScope === "mine" ? "secondary" : "ghost"}
|
variant={sessionScope === "mine" ? "secondary" : "ghost"}
|
||||||
size="xs"
|
size="xs"
|
||||||
|
aria-pressed={sessionScope === "mine"}
|
||||||
onClick={() => setSessionScope("mine")}
|
onClick={() => setSessionScope("mine")}
|
||||||
>
|
>
|
||||||
My sessions
|
My sessions
|
||||||
@@ -533,6 +534,7 @@ export function AppShell() {
|
|||||||
<Button
|
<Button
|
||||||
variant={showingAllSessions ? "secondary" : "ghost"}
|
variant={showingAllSessions ? "secondary" : "ghost"}
|
||||||
size="xs"
|
size="xs"
|
||||||
|
aria-pressed={showingAllSessions}
|
||||||
onClick={() => setSessionScope("all")}
|
onClick={() => setSessionScope("all")}
|
||||||
>
|
>
|
||||||
All sessions
|
All sessions
|
||||||
|
|||||||
Reference in New Issue
Block a user