From 8e7127a350780540685ea25edb240c0a65d215ca Mon Sep 17 00:00:00 2001 From: Lotus CI Date: Sat, 17 Oct 2026 20:26:54 -0400 Subject: [PATCH] fix(security): a dismissed status announcement no longer hides the security nudge (#307) SecurityBanner asked pickBanner whether another strip would show without the dismissed set, so a dismissed Kuma announcement (often pinned for weeks) still counted and suppressed the verify / backup nudge on every device that had dismissed it. The dismissed set is now a shared jotai atom (still persisted to localStorage) used by both banners. Adds a Playwright regression. Fixes #307 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017PCb59uY3UKqDqt7B6hzd5 --- e2e/security-nudge.spec.ts | 42 +++++++++++++++++++ .../security-nudge/SecurityBanner.tsx | 9 +++- .../server-status/ServerStatusBanner.tsx | 28 ++----------- src/app/state/serverStatus.ts | 32 ++++++++++++++ 4 files changed, 85 insertions(+), 26 deletions(-) diff --git a/e2e/security-nudge.spec.ts b/e2e/security-nudge.spec.ts index f9dcd73c9..e664eb46b 100644 --- a/e2e/security-nudge.spec.ts +++ b/e2e/security-nudge.spec.ts @@ -29,6 +29,32 @@ async function settled(page: Page) { await page.waitForTimeout(2000); } +/** [Gitea #307] Serve a Kuma status page for `localhost` with one posted announcement. */ +async function mockAnnouncement(page: Page) { + await page.route(/\/config\.json(\?.*)?$/, async (route) => { + const res = await route.fetch(); + const cfg = await res.json(); + cfg.statusPages = { localhost: { url: 'https://isitup.lotusguild.org', slug: 'matrix' } }; + await route.fulfill({ response: res, json: cfg }); + }); + await page.route(/isitup\.lotusguild\.org\/api\/status-page\//, async (route) => { + const cors = { 'access-control-allow-origin': '*' }; + if (route.request().url().includes('/heartbeat/')) { + await route.fulfill({ headers: cors, json: { heartbeatList: {}, uptimeList: {} } }); + return; + } + await route.fulfill({ + headers: cors, + json: { + config: { slug: 'matrix' }, + incidents: [{ id: 7, title: 'Community call on Friday', content: '', style: 'info' }], + maintenanceList: [], + publicGroupList: [], + }, + }); + }); +} + test.describe('security nudge (#110, #123)', () => { let user: TestUser; @@ -60,6 +86,22 @@ test.describe('security nudge (#110, #123)', () => { await expect(page.getByText('Device Verification').first()).toBeVisible(); }); + test('a dismissed announcement no longer hides the nudge (#307)', async ({ page }) => { + test.setTimeout(200_000); + await mockAnnouncement(page); + await loginUI(page, user); + await pastGrace(page); + await settled(page); + // The announcement wins the one strip slot until it's dismissed. + const announcement = page.getByRole('status').filter({ hasText: 'Community call on Friday' }); + await expect(announcement).toBeVisible(); + await expect(strip(page)).toHaveCount(0); + await announcement.getByRole('button', { name: 'Dismiss' }).click(); + await page.reload(); + await settled(page); + await expect(strip(page)).toContainText("Your encryption keys aren't backed up"); + }); + test('"Not now" snoozes it across a reload', async ({ page }) => { test.setTimeout(200_000); await loginUI(page, user); diff --git a/src/app/features/security-nudge/SecurityBanner.tsx b/src/app/features/security-nudge/SecurityBanner.tsx index bfe7f23d4..8ad82bb82 100644 --- a/src/app/features/security-nudge/SecurityBanner.tsx +++ b/src/app/features/security-nudge/SecurityBanner.tsx @@ -14,7 +14,7 @@ import { useKeyBackupInfo, useKeyBackupStatusChange } from '../../hooks/useKeyBa import { useAlive } from '../../hooks/useAlive'; import { useSyncState } from '../../hooks/useSyncState'; import { settingsRequestAtom } from '../../state/settingsRequest'; -import { serverStatusAtom } from '../../state/serverStatus'; +import { serverStatusAtom, statusDismissedAtom } from '../../state/serverStatus'; import { pickBanner } from '../../utils/kumaStatus'; import { dismissNudge, @@ -66,6 +66,7 @@ function SecurityBannerFor({ crypto }: { crypto: CryptoApi }) { const backupInfo = useKeyBackupInfo(crypto); const backupActive = useBackupActive(crypto); const { status: serverStatus, syncLost } = useAtomValue(serverStatusAtom); + const statusDismissed = useAtomValue(statusDismissedAtom); const requestSettings = useSetAtom(settingsRequestAtom); // Not urgent: wait until sync has settled (two SYNCING in a row), so it // never stacks under the "Connecting…" strip at startup. @@ -91,7 +92,11 @@ function SecurityBannerFor({ crypto }: { crypto: CryptoApi }) { }); // One strip at a time: outages, maintenance and a lost connection win. - const otherStrip = syncLost || !!pickBanner(serverStatus, { syncLost, where: 'client' }); + // [Gitea #307] Pass what the user dismissed: a dismissed announcement isn't + // on screen and must not keep the nudge hidden for as long as it's posted. + const otherStrip = + syncLost || + !!pickBanner(serverStatus, { syncLost, where: 'client', dismissed: new Set(statusDismissed) }); if (!settled || !deviceId || !kind || otherStrip || !shouldShowNudge(kind, record, Date.now())) return null; diff --git a/src/app/features/server-status/ServerStatusBanner.tsx b/src/app/features/server-status/ServerStatusBanner.tsx index b59e45fde..f33a2108e 100644 --- a/src/app/features/server-status/ServerStatusBanner.tsx +++ b/src/app/features/server-status/ServerStatusBanner.tsx @@ -1,5 +1,5 @@ import React, { useCallback, useEffect, useMemo, useState } from 'react'; -import { useAtomValue, useSetAtom } from 'jotai'; +import { useAtom, useAtomValue, useSetAtom } from 'jotai'; import { MatrixClient, SyncState } from 'matrix-js-sdk'; import { Box, Button, config, Icon, IconButton, Icons, IconSrc, Line, Text } from 'folds'; import { ContainerColor } from '../../styles/ContainerColor.css'; @@ -7,21 +7,9 @@ import { useClientConfig } from '../../hooks/useClientConfig'; import { useSyncState } from '../../hooks/useSyncState'; import { useServerStatus } from '../../hooks/useServerStatus'; import { ScreenSize, useScreenSizeContext } from '../../hooks/useScreenSize'; -import { serverStatusAtom } from '../../state/serverStatus'; +import { serverStatusAtom, statusDismissedAtom } from '../../state/serverStatus'; import { pickBanner, resolveStatusPage, StatusBanner } from '../../utils/kumaStatus'; -const DISMISS_KEY = 'lotus-status-banner-dismissed'; -const DISMISS_MAX = 50; - -const readDismissed = (): string[] => { - try { - const v = JSON.parse(localStorage.getItem(DISMISS_KEY) ?? '[]'); - return Array.isArray(v) ? v.filter((k): k is string => typeof k === 'string') : []; - } catch { - return []; - } -}; - const bannerIcon = (b: StripContent): IconSrc => { if (b.kind === 'maintenance') return Icons.Setting; if (b.tone === 'Primary') return Icons.Info; @@ -173,22 +161,14 @@ export function ServerStatusFeature({ mx }: { mx: MatrixClient }) { /** The logged-in app's status strip. */ export function ServerStatusBanner() { const { status, syncLost } = useAtomValue(serverStatusAtom); - const [dismissed, setDismissed] = useState(readDismissed); + const [dismissed, dismissKey] = useAtom(statusDismissedAtom); const banner = pickBanner(status, { syncLost, where: 'client', dismissed: new Set(dismissed), }); if (!banner) return null; - const dismiss = () => { - const next = [...dismissed.filter((k) => k !== banner.key), banner.key].slice(-DISMISS_MAX); - try { - localStorage.setItem(DISMISS_KEY, JSON.stringify(next)); - } catch { - // storage unavailable — dismissed for this session only - } - setDismissed(next); - }; + const dismiss = () => dismissKey(banner.key); return ; } diff --git a/src/app/state/serverStatus.ts b/src/app/state/serverStatus.ts index 9d4e94250..cd29358e2 100644 --- a/src/app/state/serverStatus.ts +++ b/src/app/state/serverStatus.ts @@ -16,3 +16,35 @@ export const serverStatusAtom = atom<{ syncLost: false, lostAt: null, }); + +const DISMISS_KEY = 'lotus-status-banner-dismissed'; +const DISMISS_MAX = 50; + +const readDismissed = (): string[] => { + try { + const v = JSON.parse(localStorage.getItem(DISMISS_KEY) ?? '[]'); + return Array.isArray(v) ? v.filter((k): k is string => typeof k === 'string') : []; + } catch { + return []; + } +}; + +const baseDismissedAtom = atom(readDismissed()); + +/** + * [Gitea #307] Status strips the user dismissed (by banner key), shared so + * the security nudge knows a dismissed announcement isn't on screen. + * Persisted to localStorage; session-only if storage is unavailable. + */ +export const statusDismissedAtom = atom( + (get) => get(baseDismissedAtom), + (get, set, key: string) => { + const next = [...get(baseDismissedAtom).filter((k) => k !== key), key].slice(-DISMISS_MAX); + try { + localStorage.setItem(DISMISS_KEY, JSON.stringify(next)); + } catch { + // storage unavailable — dismissed for this session only + } + set(baseDismissedAtom, next); + }, +);