From 6db8a15c22905ac5ab02b24fdd064277a1479b8f Mon Sep 17 00:00:00 2001 From: Connor Yoh Date: Wed, 2 Sep 2026 13:33:00 +0100 Subject: [PATCH] fix(portal): show each action's error on its own card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One shared actionError string was passed only to the migration card, so a failed revoke or re-wrap appeared under "Encrypt existing files" — and the rotation card's own error banner was unreachable code. Each action area now carries its own error, including the fingerprint copy fallback, which was reported on a card that had nothing to do with it. Passing the shared string to all three would have rendered the same banner three times, so the split is the fix rather than the extra props. Two related corrections in the same handlers: - A failed revoke, restore or re-wrap now re-reads status. A 409 means the row is not what is drawn, and a rotation can fail partway, leaving the pending count too high. - The S3 streaming note followed the write flag, but the backend suppresses presigned URLs once encryption is on or any key row exists. A decrypt-only install on S3 was streaming every download through the app with nothing on screen saying why. Both bugs now have stories that fail without the fix. --- .../infrastructure/EncryptionKeyTable.tsx | 8 +++ .../EncryptionPanel.stories.tsx | 64 +++++++++++++++++++ .../infrastructure/EncryptionPanel.tsx | 39 +++++++---- 3 files changed, 99 insertions(+), 12 deletions(-) diff --git a/frontend/editor/src/portal/components/infrastructure/EncryptionKeyTable.tsx b/frontend/editor/src/portal/components/infrastructure/EncryptionKeyTable.tsx index 0bc83dbac1..79d4136769 100644 --- a/frontend/editor/src/portal/components/infrastructure/EncryptionKeyTable.tsx +++ b/frontend/editor/src/portal/components/infrastructure/EncryptionKeyTable.tsx @@ -1,6 +1,7 @@ import { useState } from "react"; import { useTranslation } from "react-i18next"; import { + Banner, Button, Card, column, @@ -23,6 +24,8 @@ const STATUS_TONE: Record = { }; export interface EncryptionKeyTableProps { + /** Why the last revoke or restore failed. Shown here, beside the rows it concerns. */ + actionError?: string | null; keys: EncryptionKeyInfo[]; /** Shows the cross-node propagation delay in the revoke dialog. */ clusterEnabled?: boolean; @@ -39,6 +42,7 @@ export interface EncryptionKeyTableProps { * dialog exists at all. */ export function EncryptionKeyTable({ + actionError = null, keys, clusterEnabled = false, busyKeyId = null, @@ -133,6 +137,10 @@ export function EncryptionKeyTable({ hint={t("portal.infrastructure.encryption.keys.subheading")} hintLabel={t("portal.infrastructure.encryption.hintLabel")} /> + {actionError ? ( + + ) : null} + {/* Column headers over an empty body are chrome around nothing. */} {keys.length === 0 ? ( + HttpResponse.json({ detail: "boom" }, { status: 500 }), + ), + ...handlers(ACTIVE_STATUS), + ], + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const rowButtons = await canvas.findAllByRole("button", { + name: /^revoke$/i, + }); + await userEvent.click(rowButtons[0]); + await userEvent.click( + await within(document.body).findByRole("button", { + name: /revoke access/i, + }), + ); + + const message = await canvas.findByText(/could not be applied/i); + const keysCard = canvas.getByText(/scope keys/i).closest(".sui-card"); + await expect(keysCard).not.toBeNull(); + await expect(keysCard?.contains(message)).toBe(true); + }, +}; + +/** + * Decrypt-only on S3. The backend still streams downloads through the app once + * any key exists, so the bandwidth note has to show here too — it used to be + * tied to the write flag and vanished in exactly this state. + */ +export const DecryptOnlyOnS3: Story = { + globals: { tier: "enterprise" }, + parameters: { + msw: { + handlers: handlers({ + ...ACTIVE_STATUS, + writeEnabled: false, + active: true, + provider: "s3", + encryptedFiles: 4128, + plaintextFiles: 12, + }), + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect( + await canvas.findByText(/streamed through this server/i), + ).toBeInTheDocument(); + }, +}; + /** 09. Mid-rotation: rows still on the old key, so the old key must be kept. */ export const RotationPending: Story = { globals: { tier: "enterprise" }, diff --git a/frontend/editor/src/portal/components/infrastructure/EncryptionPanel.tsx b/frontend/editor/src/portal/components/infrastructure/EncryptionPanel.tsx index 1c5072e60d..b0a33d1ab3 100644 --- a/frontend/editor/src/portal/components/infrastructure/EncryptionPanel.tsx +++ b/frontend/editor/src/portal/components/infrastructure/EncryptionPanel.tsx @@ -71,7 +71,12 @@ export function EncryptionPanel({ const [starting, setStarting] = useState(false); const [rotating, setRotating] = useState(false); const [lastRewrapped, setLastRewrapped] = useState(null); - const [actionError, setActionError] = useState(null); + // One per action area: a single shared string renders wherever it is passed, + // so a failed revoke surfaced under "Encrypt existing files". + const [keyActionError, setKeyActionError] = useState(null); + const [rotateError, setRotateError] = useState(null); + const [migrationError, setMigrationError] = useState(null); + const [copyError, setCopyError] = useState(null); const [copied, setCopied] = useState(false); const mounted = useRef(true); const fingerprintRef = useRef(null); @@ -129,18 +134,19 @@ export function EncryptionPanel({ action: (keyId: string) => Promise, ) => { setBusyKeyId(key.keyId); - setActionError(null); + setKeyActionError(null); try { await action(key.keyId); await load(); } catch (error) { - setActionError( + setKeyActionError( isConflict(error) ? errorMessage(error) : isMissing(error) ? t("portal.infrastructure.encryption.error.keyMissing") : t("portal.infrastructure.encryption.error.action"), ); + await load(); } finally { if (mounted.current) setBusyKeyId(null); } @@ -148,11 +154,11 @@ export function EncryptionPanel({ const onStartMigration = async () => { setStarting(true); - setActionError(null); + setMigrationError(null); try { setMigration(await startEncryptionMigration()); } catch (error) { - setActionError( + setMigrationError( isConflict(error) ? errorMessage(error) : t("portal.infrastructure.encryption.error.migrationStart"), @@ -172,6 +178,7 @@ export function EncryptionPanel({ try { await navigator.clipboard.writeText(fingerprint); setCopied(true); + setCopyError(null); } catch { const node = fingerprintRef.current; if (node) { @@ -182,25 +189,25 @@ export function EncryptionPanel({ selection?.addRange(range); } setCopied(false); - setActionError( - t("portal.infrastructure.encryption.masterKey.copyFailed"), - ); + setCopyError(t("portal.infrastructure.encryption.masterKey.copyFailed")); } }; const onRotate = async () => { setRotating(true); - setActionError(null); + setRotateError(null); try { const result = await rotateMasterKey(); setLastRewrapped(result.rewrapped); await load(); } catch (error) { - setActionError( + setRotateError( isConflict(error) ? errorMessage(error) : t("portal.infrastructure.encryption.error.rotate"), ); + // A rotation can fail partway, so the pending count on screen is stale. + await load(); } finally { if (mounted.current) setRotating(false); } @@ -432,13 +439,19 @@ export function EncryptionPanel({ {/* Presigned URLs are suppressed once anything is encrypted, so object-store downloads start streaming through the app. */} - {status.provider === "s3" && status.writeEnabled ? ( + {status.provider === "s3" && + (status.writeEnabled || + status.active || + status.keys.length > 0) ? (

{t( "portal.infrastructure.encryption.masterKey.s3StreamingNote", )}

) : null} + {copyError ? ( + + ) : null} ) : (

@@ -450,6 +463,7 @@ export function EncryptionPanel({ void onStartMigration()} />