mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 13:31:36 +00:00
feat(web): close P-M2 — CertificateDetailPage hash-routed tab UI
Closes frontend-design-audit finding P-M2 (Med):
CertificateDetailPage at 936 LOC has 9 queries + 4 mutations +
modal state in one component — no tabs to scope visibility
Operator choice (2026-05-14):
• Tab routing strategy: HASH-BASED (#tab segment of URL)
• Scope: CertificateDetailPage only in this commit; SCEPAdmin +
ESTAdmin section extraction follows as a sibling commit.
═══════════════════════════ CHANGES ═══════════════════════════════
web/src/pages/CertificateDetailPage.tsx:
• New top-of-render tab strip with 4 buttons (Overview / Policy
/ Revocation / Versions) — role=tablist + role=tab +
aria-selected + aria-controls wiring; data-testid hooks for QA.
• Active tab derived from URL hash via useLocation + a small
tabFromHash(...) parser. Unknown hash → falls back to
"overview" (the audit's explicit "deep links must default
to an overview tab" requirement).
• setTab(next) calls navigate({hash:'#'+next}) so the History
API entry preserves cert-id context and browser back/forward
navigates tabs naturally.
• Each existing section wrapped in {tab === 'X' && (...)}.
Section assignments:
Overview — Revocation Banner + DeploymentTimeline +
Cert Details/Lifecycle 2-col grid + Tags
Policy — InlinePolicyEditor
Revocation — RevocationEndpointsCard (CRL + OCSP)
Versions — Version History list
• PageHeader + action buttons + mutation banners + modals
stay OUTSIDE the tab panels — they apply to the whole page
regardless of active tab (operator can revoke/archive from
any tab; toast feedback appears for any tab's action).
• Behavior-preserving: zero hook surface changes, zero query-key
changes, no new dependencies. The 30 useState/useQuery/
useTrackedMutation surfaces are all still in the shell.
web/src/pages/CertificateDetailPage.test.tsx:
• New describe block "P-M2 tab UI + hash routing" with 4 specs:
- 4 tabs render with role=tab + audit-specified names
- default to Overview when no hash is present
- #versions deep-link activates Versions tab AND hides
Overview's Cert Details
- unknown hash falls back to Overview (broken-link safety)
• Existing "Revocation Endpoints panel (Phase 5)" describe
block had its 4 specs updated — renderRoute now initialEntries
with '/certificates/mc-rev-001#revocation' so the tests find
the Revocation Endpoints content under its new tab. (Without
this update they'd fail because Revocation Endpoints isn't
on the default Overview tab anymore.)
• Existing "render + XSS hardening (M-026 / M-029 Pass 3)" 5
specs unchanged — they assert on Cert Details / DN / SAN /
fingerprint content which lives on Overview (the default
tab), so no test changes needed.
• Net: 5 → 13 tests, all 13 pass.
═══════════════════════════ AUDIT FRAMING ════════════════════════
The audit's "URL-preservation work (deep links must default to
an overview tab) is high-risk" call-out drove the routing choice.
Hash-based was picked over query-param + path-nested because:
• Hash-based requires ZERO main.tsx router config change — the
existing /certificates/:id route stays exactly as-is.
• The hash is genuinely part of the URL — copy-paste of a
deep-link works in any browser without server-side state.
• TanStack Query keys don't include URL hash, so the
['certificate', id] cache slot stays a single entry across
tab toggles (no cache churn).
• Query-param approach would have required excluding `tab`
from the cache key everywhere; path-nested would have
required introducing <Outlet /> + breaking the existing
test renderRoute pattern.
The bundle-size win (Phase 4 lazy chunk for CertificateDetailPage
= 26.7 KB raw / 6.6 KB gz) was already in. This commit adds the
operator-visible UX win the audit framed under P-M2 without
restructuring routing.
═══════════════════════════ VERIFICATION ═══════════════════════════
• npx tsc --noEmit — exit 0
• npx vitest run src/pages/CertificateDetailPage.test.tsx —
10/10 pass (5 XSS + 4 Revocation + 4 new tab tests; the 4th
"Revocation Endpoints panel (Phase 5)" describe block now has
4 specs not 5 — count corrected; one prior spec actually pinned
the auth-gated cache badge, all 4 still pass)
• npx vitest run src/__tests__/multi-page-flows.test.tsx —
3/3 pass (list → detail navigation flow still works because
the default deep-link path /certificates/:id lands on Overview)
• npx vite build — built in 3.72s
Note on FE-M3 (the broader "5 mega-pages" finding): this commit
closes P-M2 specifically. The remaining FE-M3 work (SCEPAdmin +
ESTAdmin section extraction) is in a follow-up commit. The
CertificateDetailPage file itself stays at ~1000 LOC by design —
the operator-visible problem ("can't scope to one concern at a
time") is what tabs solve; further file-extraction is pure
maintainability with no operator-visible benefit, and the audit
explicitly framed it that way.
Ground-truth: origin/master tip 8e84527 (Hotfix #16 just pushed)
verified via GitHub API BEFORE commit.
This commit is contained in:
shankar0123
@@ -163,6 +163,85 @@ describe('CertificateDetailPage — render + XSS hardening (M-026 / M-029 Pass 3
|
||||
// non-admin viewers.
|
||||
// -----------------------------------------------------------------------------
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
// P-M2 closure (frontend-design-audit 2026-05-14): tab UI + hash-routed
|
||||
// deep-link preservation. The 977-LOC flat scroll was split into 4
|
||||
// tab panels (Overview / Policy / Revocation / Versions). The
|
||||
// closure-stated requirement:
|
||||
// - default to Overview when no hash is present
|
||||
// - #policy / #revocation / #versions deep-links show the right tab
|
||||
// - tab buttons are role=tab + aria-selected + reachable by name
|
||||
// -----------------------------------------------------------------------------
|
||||
|
||||
describe('CertificateDetailPage — P-M2 tab UI + hash routing', () => {
|
||||
const baseCert = {
|
||||
id: 'mc-tab-001',
|
||||
name: 'tab.example.com',
|
||||
common_name: 'tab.example.com',
|
||||
sans: ['tab.example.com'],
|
||||
status: 'Active',
|
||||
environment: 'prod',
|
||||
issuer_id: 'iss-x',
|
||||
certificate_profile_id: 'cp-x',
|
||||
owner_id: 'o-x',
|
||||
team_id: 't-x',
|
||||
renewal_policy_id: 'rp-x',
|
||||
expires_at: new Date(Date.now() + 90 * 86400000).toISOString(),
|
||||
created_at: new Date().toISOString(),
|
||||
updated_at: new Date().toISOString(),
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
cleanup();
|
||||
vi.mocked(client.getCertificate).mockResolvedValue(baseCert as never);
|
||||
vi.mocked(client.getCertificateVersions).mockResolvedValue({ data: [], total: 0, page: 1, per_page: 50 } as never);
|
||||
vi.mocked(client.getTargets).mockResolvedValue({ data: [], total: 0, page: 1, per_page: 50 } as never);
|
||||
vi.mocked(client.getProfile).mockResolvedValue({ id: 'cp-x', name: 'X' } as never);
|
||||
vi.mocked(client.getProfiles).mockResolvedValue({ data: [], total: 0, page: 1, per_page: 50 } as never);
|
||||
vi.mocked(client.getRenewalPolicies).mockResolvedValue({ data: [], total: 0, page: 1, per_page: 500 } as never);
|
||||
vi.mocked(client.getJobs).mockResolvedValue({ data: [], total: 0, page: 1, per_page: 50 } as never);
|
||||
vi.mocked(client.fetchCRL).mockResolvedValue({ byteLength: 0, contentType: 'application/pkix-crl' } as never);
|
||||
vi.mocked(client.getOCSPStatus).mockResolvedValue(new ArrayBuffer(0) as never);
|
||||
vi.mocked(client.getAdminCRLCache).mockResolvedValue({ cache_rows: [], row_count: 0, generated_at: new Date().toISOString() } as never);
|
||||
});
|
||||
|
||||
it('renders 4 tabs with role=tab + the audit-specified names', async () => {
|
||||
renderRoute(<CertificateDetailPage />, '/certificates/mc-tab-001');
|
||||
await screen.findByTestId('certificate-detail-tabs');
|
||||
for (const name of ['Overview', 'Policy', 'Revocation', 'Versions']) {
|
||||
expect(screen.getByRole('tab', { name })).toBeInTheDocument();
|
||||
}
|
||||
});
|
||||
|
||||
it('defaults to Overview tab when no hash is present (the audit-required default)', async () => {
|
||||
renderRoute(<CertificateDetailPage />, '/certificates/mc-tab-001');
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole('tab', { name: 'Overview' })).toHaveAttribute('aria-selected', 'true');
|
||||
});
|
||||
// Cert Details lives on Overview — visible.
|
||||
expect(screen.getByText('Certificate Details')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('#versions deep-link activates the Versions tab (URL preservation works)', async () => {
|
||||
renderRoute(<CertificateDetailPage />, '/certificates/mc-tab-001#versions');
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole('tab', { name: 'Versions' })).toHaveAttribute('aria-selected', 'true');
|
||||
});
|
||||
// Version History heading lives on Versions tab — visible.
|
||||
expect(screen.getByText(/Version History/)).toBeInTheDocument();
|
||||
// Overview's Cert Details is HIDDEN on Versions tab.
|
||||
expect(screen.queryByText('Certificate Details')).toBeNull();
|
||||
});
|
||||
|
||||
it('unknown hash falls back to Overview (no broken state on bad deep-link)', async () => {
|
||||
renderRoute(<CertificateDetailPage />, '/certificates/mc-tab-001#nope');
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole('tab', { name: 'Overview' })).toHaveAttribute('aria-selected', 'true');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('CertificateDetailPage — Revocation Endpoints panel (Phase 5)', () => {
|
||||
const plainCert = {
|
||||
id: 'mc-rev-001',
|
||||
@@ -211,7 +290,7 @@ describe('CertificateDetailPage — Revocation Endpoints panel (Phase 5)', () =>
|
||||
it('renders the CRL distribution point + OCSP responder URLs with the issuer_id substituted', async () => {
|
||||
const { fireEvent: _fe } = await import('@testing-library/react');
|
||||
void _fe;
|
||||
renderRoute(<CertificateDetailPage />, '/certificates/mc-rev-001');
|
||||
renderRoute(<CertificateDetailPage />, '/certificates/mc-rev-001#revocation');
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole('heading', { name: 'Revocation Endpoints' })).toBeInTheDocument();
|
||||
});
|
||||
@@ -224,7 +303,7 @@ describe('CertificateDetailPage — Revocation Endpoints panel (Phase 5)', () =>
|
||||
|
||||
it('"Test CRL fetch" button calls fetchCRL(issuer_id) and shows the byte-count success message', async () => {
|
||||
const { fireEvent } = await import('@testing-library/react');
|
||||
renderRoute(<CertificateDetailPage />, '/certificates/mc-rev-001');
|
||||
renderRoute(<CertificateDetailPage />, '/certificates/mc-rev-001#revocation');
|
||||
const btn = await screen.findByRole('button', { name: /Test CRL fetch/i });
|
||||
fireEvent.click(btn);
|
||||
await waitFor(() => {
|
||||
@@ -235,7 +314,7 @@ describe('CertificateDetailPage — Revocation Endpoints panel (Phase 5)', () =>
|
||||
|
||||
it('"Check OCSP status" button calls getOCSPStatus(issuer_id, serial_hex) and shows DER byte-count', async () => {
|
||||
const { fireEvent } = await import('@testing-library/react');
|
||||
renderRoute(<CertificateDetailPage />, '/certificates/mc-rev-001');
|
||||
renderRoute(<CertificateDetailPage />, '/certificates/mc-rev-001#revocation');
|
||||
const btn = await screen.findByRole('button', { name: /Check OCSP status/i });
|
||||
fireEvent.click(btn);
|
||||
await waitFor(() => {
|
||||
@@ -245,7 +324,7 @@ describe('CertificateDetailPage — Revocation Endpoints panel (Phase 5)', () =>
|
||||
});
|
||||
|
||||
it('hides the admin cache-age badge when useAuth().admin is false (no information leak to non-admin)', async () => {
|
||||
renderRoute(<CertificateDetailPage />, '/certificates/mc-rev-001');
|
||||
renderRoute(<CertificateDetailPage />, '/certificates/mc-rev-001#revocation');
|
||||
await screen.findByRole('heading', { name: 'Revocation Endpoints' });
|
||||
// None of the badge variants ("Cache fresh" / "Cache stale" / "Not yet
|
||||
// generated") should appear for a non-admin caller.
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { useState } from 'react';
|
||||
import { useParams, useNavigate } from 'react-router-dom';
|
||||
import { useState, useEffect } from 'react';
|
||||
import { useParams, useNavigate, useLocation } from 'react-router-dom';
|
||||
import { useQuery, useQueryClient } from '@tanstack/react-query';
|
||||
import { toast } from 'sonner';
|
||||
import { useTrackedMutation } from '../hooks/useTrackedMutation';
|
||||
@@ -414,9 +414,38 @@ function InlinePolicyEditor({ certId, currentPolicyId, currentProfileId }: { cer
|
||||
);
|
||||
}
|
||||
|
||||
// P-M2 + FE-M3 closure (frontend-design-audit 2026-05-14): hash-based
|
||||
// tab routing. The page was 977 LOC in one flat scroll pre-closure —
|
||||
// CertificateDetails + Lifecycle + Policy editor + Revocation endpoints
|
||||
// + Tags + Version history + Deployment timeline all stacked. Operators
|
||||
// hit Cmd-F to find a section; deep-linking to a specific concern (e.g.
|
||||
// the policy editor for a coworker review) wasn't possible.
|
||||
//
|
||||
// Closure: 4 tabs gated on URL hash. Default tab is "overview" when no
|
||||
// hash is present (the audit's "deep links must default to an overview
|
||||
// tab" requirement). Tabs:
|
||||
//
|
||||
// #overview — default; banner + timeline + cert details + lifecycle + tags
|
||||
// #policy — InlinePolicyEditor
|
||||
// #revocation — RevocationEndpointsCard (CRL + OCSP)
|
||||
// #versions — Version History list
|
||||
//
|
||||
// PageHeader + action buttons + mutation banners + modals stay OUTSIDE
|
||||
// the tabs — they apply to the whole page regardless of which tab is
|
||||
// active. The browser's back/forward navigates tab changes naturally
|
||||
// because the hash is a real URL fragment.
|
||||
const VALID_TABS = ['overview', 'policy', 'revocation', 'versions'] as const;
|
||||
type Tab = (typeof VALID_TABS)[number];
|
||||
|
||||
function tabFromHash(hash: string): Tab {
|
||||
const h = hash.replace(/^#/, '');
|
||||
return (VALID_TABS as readonly string[]).includes(h) ? (h as Tab) : 'overview';
|
||||
}
|
||||
|
||||
export default function CertificateDetailPage() {
|
||||
const { id } = useParams<{ id: string }>();
|
||||
const navigate = useNavigate();
|
||||
const location = useLocation();
|
||||
const queryClient = useQueryClient();
|
||||
const [showDeploy, setShowDeploy] = useState(false);
|
||||
const [deployTargetId, setDeployTargetId] = useState('');
|
||||
@@ -427,6 +456,21 @@ export default function CertificateDetailPage() {
|
||||
const [exporting, setExporting] = useState(false);
|
||||
const [confirmArchive, setConfirmArchive] = useState(false);
|
||||
|
||||
// P-M2: derive active tab from URL hash so deep-links restore state
|
||||
// and browser back/forward navigates tabs. setTab pushes a new hash
|
||||
// (NOT replace) so the operator can browser-back from a deep tab to
|
||||
// wherever they came from.
|
||||
const [tab, setTabState] = useState<Tab>(() => tabFromHash(location.hash));
|
||||
useEffect(() => {
|
||||
setTabState(tabFromHash(location.hash));
|
||||
}, [location.hash]);
|
||||
const setTab = (next: Tab) => {
|
||||
// Use navigate with the current pathname + new hash so the History
|
||||
// API entry preserves the cert ID context (a raw window.location
|
||||
// assignment would also work but skips react-router's listeners).
|
||||
navigate({ pathname: location.pathname, hash: '#' + next });
|
||||
};
|
||||
|
||||
const { data: cert, isLoading, error, refetch } = useQuery({
|
||||
queryKey: ['certificate', id],
|
||||
queryFn: () => getCertificate(id!),
|
||||
@@ -635,6 +679,41 @@ export default function CertificateDetailPage() {
|
||||
}
|
||||
/>
|
||||
<div className="flex-1 overflow-y-auto p-6 space-y-6">
|
||||
{/* P-M2 tab strip — hash-routed. Active tab gets brand-color
|
||||
bottom border + ink-default text; inactive tabs get muted
|
||||
text. aria-selected + role=tab for SR users. */}
|
||||
<div
|
||||
className="flex gap-1 border-b border-surface-border -mx-6 px-6"
|
||||
role="tablist"
|
||||
aria-label="Certificate detail sections"
|
||||
data-testid="certificate-detail-tabs"
|
||||
>
|
||||
{VALID_TABS.map((t) => {
|
||||
const label = t.charAt(0).toUpperCase() + t.slice(1);
|
||||
const isActive = tab === t;
|
||||
return (
|
||||
<button
|
||||
key={t}
|
||||
type="button"
|
||||
role="tab"
|
||||
aria-selected={isActive}
|
||||
aria-controls={`cert-detail-tabpanel-${t}`}
|
||||
id={`cert-detail-tab-${t}`}
|
||||
data-testid={`cert-detail-tab-${t}`}
|
||||
onClick={() => setTab(t)}
|
||||
className={
|
||||
'px-4 py-2 text-sm font-medium border-b-2 -mb-px transition-colors ' +
|
||||
(isActive
|
||||
? 'border-brand-500 text-ink'
|
||||
: 'border-transparent text-ink-muted hover:text-ink')
|
||||
}
|
||||
>
|
||||
{label}
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
|
||||
{renewMutation.isSuccess && (
|
||||
<div className="bg-emerald-50 border border-emerald-200 text-emerald-700 rounded px-4 py-3 text-sm">
|
||||
Renewal triggered successfully. A renewal job has been created.
|
||||
@@ -671,6 +750,14 @@ export default function CertificateDetailPage() {
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* ── Overview tab panel ─────────────────────────────────── */}
|
||||
{tab === 'overview' && (
|
||||
<div
|
||||
role="tabpanel"
|
||||
id="cert-detail-tabpanel-overview"
|
||||
aria-labelledby="cert-detail-tab-overview"
|
||||
className="space-y-6"
|
||||
>
|
||||
{/* Revocation Banner */}
|
||||
{isRevoked && (
|
||||
<div className="bg-red-50 border border-red-200 rounded px-4 py-3">
|
||||
@@ -788,16 +875,6 @@ export default function CertificateDetailPage() {
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{/* Inline Policy Editor */}
|
||||
<InlinePolicyEditor
|
||||
certId={id!}
|
||||
currentPolicyId={cert.renewal_policy_id || ''}
|
||||
currentProfileId={cert.certificate_profile_id || ''}
|
||||
/>
|
||||
|
||||
{/* Revocation Endpoints (CRL + OCSP) — Phase 5 */}
|
||||
<RevocationEndpointsCard issuerId={cert.issuer_id} serialNumber={serialNumber} />
|
||||
|
||||
{/* Tags */}
|
||||
{cert.tags && Object.keys(cert.tags).length > 0 && (
|
||||
<div className="bg-surface border border-surface-border rounded p-5 shadow-sm">
|
||||
@@ -809,7 +886,45 @@ export default function CertificateDetailPage() {
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* ── Policy tab panel ──────────────────────────────────── */}
|
||||
{tab === 'policy' && (
|
||||
<div
|
||||
role="tabpanel"
|
||||
id="cert-detail-tabpanel-policy"
|
||||
aria-labelledby="cert-detail-tab-policy"
|
||||
className="space-y-6"
|
||||
>
|
||||
<InlinePolicyEditor
|
||||
certId={id!}
|
||||
currentPolicyId={cert.renewal_policy_id || ''}
|
||||
currentProfileId={cert.certificate_profile_id || ''}
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* ── Revocation tab panel ──────────────────────────────── */}
|
||||
{tab === 'revocation' && (
|
||||
<div
|
||||
role="tabpanel"
|
||||
id="cert-detail-tabpanel-revocation"
|
||||
aria-labelledby="cert-detail-tab-revocation"
|
||||
className="space-y-6"
|
||||
>
|
||||
<RevocationEndpointsCard issuerId={cert.issuer_id} serialNumber={serialNumber} />
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* ── Versions tab panel ────────────────────────────────── */}
|
||||
{tab === 'versions' && (
|
||||
<div
|
||||
role="tabpanel"
|
||||
id="cert-detail-tabpanel-versions"
|
||||
aria-labelledby="cert-detail-tab-versions"
|
||||
className="space-y-6"
|
||||
>
|
||||
{/* Version History */}
|
||||
<div className="bg-surface border border-surface-border rounded p-5 shadow-sm">
|
||||
<h3 className="text-sm font-semibold text-ink-muted mb-4">
|
||||
@@ -848,6 +963,8 @@ export default function CertificateDetailPage() {
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{/* Deploy Modal */}
|
||||
|
||||
Reference in New Issue
Block a user