gsadmin/certctl
1
0
Fork 0
mirror of https://github.com/shankar0123/certctl.git synced 2026-06-14 12:19:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(web): close P-M2 — CertificateDetailPage hash-routed tab UI

Browse Source 
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
2026-05-14 20:14:26 +00:00
parent 8e84527ba2
commit 9ba5ee41be
2 changed files with 212 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web/src/pages/CertificateDetailPage.test.tsx
+83 -4
View File
@@ -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.