Skip to content

Commit bd70fef

Browse files
enejbkraftbj
enejb
and
kraftbj
authored
Protect Dashboard: Add a threat report for links that name threats (#53693)
* Protect Dashboard: Add a threat report for links that name threats * Protect Dashboard: Redirect top-level threats links into the dashboard route * Protect Dashboard: Test that threat ids in the URL show the report * Protect Dashboard: Drop the threat report's summary notice * Protect Dashboard: Show a notice while the threat report checks statuses * Protect Dashboard: Use the red alert shield on the report's Needs attention card * Protect Dashboard: Show the safe shield above a threat report with nothing to fix * Protect Dashboard: Tighten the threat report's status checks * A failed Scan report shows the alert's threats as "Status unavailable" with an error notice, instead of "No longer detected" under the safe shield. * A site without Scan sees "No known vulnerabilities found" rather than "Your site is safe right now", matching the Scan tab. * A history refresh requested while a fetch is running now fetches again after it, so a fix that lands mid-request still shows as Fixed. * Threats history has ignored stay in Ignored after the reader ignores another one before history loads. * The post-fix reload starts before paint, so a fixed threat no longer flashes as "No longer detected". * The redirect drops ids that aren't all digits, as the client does, instead of merging what's left of them. * Protect Dashboard: Hold the threat report's all-clear until Scan has a report * Before Scan's first report, the alert's threats show as "Status unavailable" with a notice to come back once it finishes, instead of "Your site is safe right now". * When other threats remain on the site, the report drops the green shield and says "Other threats on your site still need attention", which also fits sites without Scan. * "See them on the Overview" is a link that leaves the report. * Protect Dashboard: Read Scan fresh before the threat report calls a threat gone * The report asks WordPress.com for Scan's report when it opens, and keeps unmatched ids as "Checking" until it answers. The page's cached copy can predate the alert, which made a just-found threat read as "No longer detected" under "Your site is safe right now". * A history load adds the threats history ignored to the client's ignored list instead of skipping it, so ignoring one threat before history loads no longer hides the others from the Ignored group or the inspector. * The history refetch clears its in-flight request in the same callback that checks for a queued refresh. * Stage tells the report whether the plan has Scan history, from the same check the History tab uses. * The Scan card and the report share one fetch for Scan's report. * Prefix the package changelog entry. * Protect Dashboard: Wait for Scan's fresh report before the threat report's all-clear and actions * The safe shield waits for the fresh read, so ids already in Fixed or Ignored no longer get an all-clear drawn from the page's cached report. * Fix, ignore and unignore wait for the fresh read too, so its older response can't undo an edit made before it landed. * A rejected history request clears the in-flight request instead of blocking later loads. * Test the first-load ignored list, and reset the history store between tests. * Protect Dashboard: Keep the reader's threat edits when an older Scan read lands Replaces the button gating from the previous commit, which left gaps (Delete, Undo, the inspector during scan polls) and made the inspector's footer flicker. * The scan store logs the reader's edits (ignore, unignore, delete, a finished fix). A read's answer is applied, then the edits made since it started are replayed, so an older read never undoes one and fresh server data is still kept. * Reads of Scan's report, including a fix's final status, apply newest-started first; an older answer that lands later is dropped. A failed read shows its error but doesn't outrank a good answer. * The ignored-list load merges with ignores made while it was in flight. * Fix, Ignore and Unignore go back to depending on the plan alone; the threat report still waits for the fresh read before its all-clear. * Drop the unreachable catch on the history request. --------- Co-authored-by: Brandon Kraft <public@brandonkraft.com>
1 parent e8d04a7 commit bd70fef

16 files changed

Lines changed: 1237 additions & 37 deletions

File tree

‎projects/packages/protect/changelog/add-protect-dashboard-threat-links‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Significance: minor
2+
Type: added
3+
4+
Protect dashboard: Add a threat report page that shows the threats a link names, grouped into ones that need attention, fixed, ignored and no longer detected.

‎projects/packages/protect/routes/dashboard/sections/history/store.ts‎

Lines changed: 55 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import apiFetch from '@wordpress/api-fetch';
22
import { useSyncExternalStore } from '@wordpress/element';
33
import { __ } from '@wordpress/i18n';
44
import { createStore, setScan } from '../scan/store';
5-
import type { ScanThreat } from '../scan/types';
5+
import type { ScanState, ScanThreat } from '../scan/types';
66

77
/** The search param naming the history threat open in the inspector. */
88
export const HISTORY_THREAT_PARAM = 'historyThreat';
@@ -15,7 +15,8 @@ export type HistoryData = { threats: ScanThreat[] | null; error: string | null }
1515
// One object, not one per read: useSyncExternalStore re-renders forever on a new snapshot.
1616
const INITIAL: HistoryData = { threats: null, error: null };
1717
const historyStore = createStore< HistoryData >( () => INITIAL );
18-
let isLoading = false;
18+
let inFlight: Promise< void > | null = null;
19+
let refetch = false;
1920

2021
/**
2122
* Fetch the fixed and ignored threats; the stage and inspector share the result.
@@ -24,29 +25,69 @@ let isLoading = false;
2425
* @return Resolves once the list or the error is in the store.
2526
*/
2627
export function loadHistory( refresh = false ): Promise< void > {
27-
if ( isLoading || ( ! refresh && historyStore.get().threats ) ) {
28+
if ( inFlight ) {
29+
// The running request may predate the fix a refresh is for, so fetch once more after it.
30+
refetch = refetch || refresh;
31+
return inFlight;
32+
}
33+
if ( ! refresh && historyStore.get().threats ) {
2834
return Promise.resolve();
2935
}
30-
isLoading = true;
36+
// Check and clear in one callback, so no refresh can slip in between them.
37+
const run = (): Promise< void > =>
38+
fetchHistory().then( () => {
39+
if ( refetch ) {
40+
refetch = false;
41+
return run();
42+
}
43+
inFlight = null;
44+
} );
45+
inFlight = run();
46+
return inFlight;
47+
}
48+
49+
/** Forget the loaded history and any request, so tests start over; for tests. */
50+
export function resetHistory(): void {
51+
historyStore.reset();
52+
inFlight = null;
53+
refetch = false;
54+
}
55+
56+
/**
57+
* Fetch the history once and store the list or the error.
58+
*
59+
* @return Resolves once the store is updated; never rejects.
60+
*/
61+
function fetchHistory(): Promise< void > {
3162
return apiFetch< ScanThreat[] >( { path: '/jetpack/v4/protect-dashboard/history' } )
3263
.then( threats => {
3364
historyStore.set( () => ( { threats, error: null } ) );
34-
// The same response has the ignored threats; a list Scan already holds may have newer edits.
35-
setScan( scan =>
36-
scan.ignored
37-
? scan
38-
: { ...scan, ignored: threats.filter( item => item.status === 'ignored' ) }
39-
);
65+
setScan( scan => ( { ...scan, ignored: mergeIgnored( scan, threats ) } ) );
4066
} )
4167
.catch( ( e: { message?: string } ) =>
4268
historyStore.set( current => ( {
4369
threats: current.threats,
4470
error: e?.message || __( 'Scan history is unavailable right now.', 'jetpack-protect-pkg' ),
4571
} ) )
46-
)
47-
.finally( () => {
48-
isLoading = false;
49-
} );
72+
);
73+
}
74+
75+
/**
76+
* The ignored list after a history load: the client's own edits, plus threats history ignored that it doesn't know of.
77+
*
78+
* @param scan - The Scan state, whose lists may hold newer ignores and unignores.
79+
* @param history - The history response.
80+
* @return The ignored threats.
81+
*/
82+
function mergeIgnored( scan: ScanState, history: ScanThreat[] ): ScanThreat[] {
83+
const fromHistory = history.filter( item => item.status === 'ignored' );
84+
if ( ! scan.ignored ) {
85+
return fromHistory;
86+
}
87+
const known = new Set(
88+
[ ...scan.ignored, ...( scan.threats ?? [] ) ].map( item => String( item.id ) )
89+
);
90+
return [ ...scan.ignored, ...fromHistory.filter( item => ! known.has( String( item.id ) ) ) ];
5091
}
5192

5293
/**

‎projects/packages/protect/routes/dashboard/sections/history/test/store.test.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import apiFetch from '@wordpress/api-fetch';
2+
import { getScan, resetScan } from '../../scan/store';
3+
import { loadHistory, resetHistory } from '../store';
4+
5+
jest.mock( '@wordpress/api-fetch', () => ( { __esModule: true, default: jest.fn() } ) );
6+
7+
const mockApiFetch = apiFetch as unknown as jest.Mock;
8+
9+
const threat = ( id: number, status: string ) => ( { id, status, title: `Threat ${ id }` } );
10+
11+
describe( 'loadHistory', () => {
12+
afterEach( () => {
13+
delete window.jetpackProtectDashboard;
14+
resetScan();
15+
resetHistory();
16+
} );
17+
18+
it( 'fetches again after the running request when a refresh is asked for mid-request', async () => {
19+
const responses: ( ( threats: unknown[] ) => void )[] = [];
20+
mockApiFetch.mockImplementation( () => new Promise( resolve => responses.push( resolve ) ) );
21+
22+
const flush = () => new Promise( resolve => setTimeout( resolve, 0 ) );
23+
let refreshed = false;
24+
loadHistory();
25+
loadHistory( true ).then( () => ( refreshed = true ) );
26+
27+
responses[ 0 ]( [] );
28+
await flush();
29+
expect( mockApiFetch ).toHaveBeenCalledTimes( 2 );
30+
expect( refreshed ).toBe( false );
31+
32+
responses[ 1 ]( [] );
33+
await flush();
34+
expect( refreshed ).toBe( true );
35+
} );
36+
37+
it.each( [
38+
[ 'fills the ignored list on a first load', { threats: [] }, [ 4, 5, 6 ] ],
39+
[
40+
'adds threats history ignored to the client-held list, skipping ones the client moved',
41+
{ threats: [ threat( 6, 'current' ) ], ignored: [ threat( 5, 'ignored' ) ] },
42+
[ 5, 4 ],
43+
],
44+
] )( '%s', async ( _name, scan, expected ) => {
45+
window.jetpackProtectDashboard = { scan };
46+
mockApiFetch.mockResolvedValue( [
47+
...[ 4, 5, 6 ].map( id => threat( id, 'ignored' ) ),
48+
threat( 7, 'fixed' ),
49+
] );
50+
51+
await loadHistory( true );
52+
53+
expect( getScan()?.ignored?.map( item => item.id ) ).toEqual( expected );
54+
} );
55+
} );

‎projects/packages/protect/routes/dashboard/sections/scan/group-threats.ts‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import type { ScanThreat } from './types';
2+
3+
/** The search param listing the threats a report shows, such as the ones in a threat email. */
4+
export const REPORT_PARAM = 'threats';
5+
6+
const MAX_IDS = 50;
7+
8+
/**
9+
* The threat ids in the report param, in order and without duplicates.
10+
*
11+
* @param value - The param's value; the router parses a lone id as a number.
12+
* @return The ids.
13+
*/
14+
export function parseThreatIds( value: unknown ): string[] {
15+
if ( typeof value !== 'string' && typeof value !== 'number' ) {
16+
return [];
17+
}
18+
const ids = String( value )
19+
.split( ',' )
20+
.map( id => id.trim() )
21+
.filter( id => /^\d+$/.test( id ) );
22+
return [ ...new Set( ids ) ].slice( 0, MAX_IDS );
23+
}
24+
25+
export type ThreatGroups = {
26+
active: ScanThreat[];
27+
fixed: ScanThreat[];
28+
ignored: ScanThreat[];
29+
/** Ids Scan reports nowhere, such as a vulnerability gone after an update. */
30+
notFound: string[];
31+
/** Ids whose status is unknown while the active list or Scan history is. */
32+
pending: string[];
33+
};
34+
35+
type ThreatSources = {
36+
/** The active threats, or null when Scan's report failed. */
37+
active: ScanThreat[] | null;
38+
/** The ignored list the client holds, which a history load fills in; newer than history's. */
39+
ignored: ScanThreat[] | undefined;
40+
/** Scan history, or null while it loads or after it failed. */
41+
history: ScanThreat[] | null;
42+
};
43+
44+
/**
45+
* Sort a report's threats by where Scan has them now, keeping the requested order in each group.
46+
*
47+
* @param ids - The requested threat ids.
48+
* @param sources - The active threats, the ignored list and Scan history.
49+
* @return The groups.
50+
*/
51+
export function groupThreats( ids: string[], sources: ThreatSources ): ThreatGroups {
52+
const { active, ignored, history } = sources;
53+
const byId = ( threats: ScanThreat[] ) =>
54+
new Map( threats.map( item => [ String( item.id ), item ] ) );
55+
const activeById = byId( active ?? [] );
56+
const ignoredById = byId( ignored ?? history?.filter( item => item.status === 'ignored' ) ?? [] );
57+
const fixedById = byId( history?.filter( item => item.status === 'fixed' ) ?? [] );
58+
59+
const groups: ThreatGroups = { active: [], fixed: [], ignored: [], notFound: [], pending: [] };
60+
// Do not reorder: a threat fixed or ignored, then found again, is active.
61+
const lists = [
62+
[ activeById, groups.active ],
63+
[ ignoredById, groups.ignored ],
64+
[ fixedById, groups.fixed ],
65+
] as const;
66+
for ( const id of ids ) {
67+
const list = lists.find( ( [ map ] ) => map.has( id ) );
68+
if ( list ) {
69+
list[ 1 ].push( list[ 0 ].get( id ) as ScanThreat );
70+
} else {
71+
( history && active ? groups.notFound : groups.pending ).push( id );
72+
}
73+
}
74+
return groups;
75+
}

‎projects/packages/protect/routes/dashboard/sections/scan/safe-state.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { getCheckedCounts } from './labels';
55
import ScanButton, { getNextCheck, useHasPassed } from './scan-button';
66
import type { ScanState } from './types';
77

8-
const SafeShield = () => (
8+
export const SafeShield = () => (
99
<svg width="80" height="98" viewBox="0 0 123 150" aria-hidden="true" focusable="false">
1010
<path
1111
d="M61.365 0L0 27.273v40.909C0 106.023 26.182 141.409 61.365 150c35.183-8.591 61.365-43.977 61.365-81.818v-40.91L61.365 0z"

‎projects/packages/protect/routes/dashboard/sections/scan/scan-card.tsx‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { HISTORY_STATUS_PARAM } from '../history/store';
99
import SafeState from './safe-state';
1010
import ScanButton from './scan-button';
1111
import ScanningState from './scanning-state';
12-
import { SCAN_PATH, mergeScan, setScan, useScan } from './store';
12+
import { SCAN_PATH, mergeScan, refreshScan, useScan } from './store';
1313
import { loadIgnored } from './threat-actions';
1414
import threatsIcon from './threats-icon';
1515
import ThreatsList from './threats-list';
@@ -92,19 +92,11 @@ export default function ScanCard( { openTab }: Pick< DashboardContext, 'openTab'
9292
}
9393
}, [ hasPlan ] );
9494

95-
const fetchScan = useCallback(
96-
() =>
97-
apiFetch< ScanState >( { path: SCAN_PATH } )
98-
.then( mergeScan )
99-
.catch( () => setScan( current => ( { ...current, error: true, scanning: false } ) ) ),
100-
[]
101-
);
102-
10395
const refresh = useCallback( () => {
10496
setIsFetching( true );
10597
setPolls( 0 );
106-
fetchScan().finally( () => setIsFetching( false ) );
107-
}, [ fetchScan ] );
98+
refreshScan().finally( () => setIsFetching( false ) );
99+
}, [] );
108100

109101
const startScan = useCallback( () => {
110102
setIsStarting( true );
@@ -129,11 +121,11 @@ export default function ScanCard( { openTab }: Pick< DashboardContext, 'openTab'
129121
return;
130122
}
131123
const timer = setTimeout(
132-
() => fetchScan().finally( () => setPolls( count => count + 1 ) ),
124+
() => refreshScan().finally( () => setPolls( count => count + 1 ) ),
133125
pollInterval( polls )
134126
);
135127
return () => clearTimeout( timer );
136-
}, [ isScanning, isHidden, polls, fetchScan ] );
128+
}, [ isScanning, isHidden, polls ] );
137129

138130
// The running scan gets the whole card, without the header and footer.
139131
if ( isScanning ) {

0 commit comments

Comments
 (0)