diff --git a/extension/src/popup/components/org-add-form.ts b/extension/src/popup/components/org-add-form.ts index e7d486a..d9ee5a2 100644 --- a/extension/src/popup/components/org-add-form.ts +++ b/extension/src/popup/components/org-add-form.ts @@ -117,6 +117,11 @@ export function openOrgAddForm(onAdded?: (orgId: string) => void): void { memberId: (data.get('memberId') as string).trim(), }; + if (!config.orgId) { + errorSlot.textContent = 'Org ID is required'; + return; + } + const resp = await sendMessage({ type: 'org_add_config', config }); if (resp.ok) { close(); diff --git a/extension/src/popup/components/org-switcher.ts b/extension/src/popup/components/org-switcher.ts index 1598aaa..ccba402 100644 --- a/extension/src/popup/components/org-switcher.ts +++ b/extension/src/popup/components/org-switcher.ts @@ -7,7 +7,7 @@ // disable writes. import type { OrgConfigSummary, OrgManifestEntry, ManifestEntry, ItemId, Collection } from '../../shared/types'; -import { sendMessage, setState, navigate, getState } from '../../shared/state'; +import { sendMessage, setState, navigate, getState, escapeHtml } from '../../shared/state'; import { messageForList, normalizeOrgEntries } from '../../shared/org-context'; import { openOrgAddForm } from './org-add-form'; @@ -33,7 +33,7 @@ export async function renderOrgSwitcher(host: HTMLElement): Promise { ``; for (const org of orgConfigs) { const sel = currentContext === org.orgId ? ' selected' : ''; - optionsHtml += ``; + optionsHtml += ``; } host.innerHTML = `` + @@ -62,7 +62,13 @@ export async function renderOrgSwitcher(host: HTMLElement): Promise { existing?.remove(); } - // Load the new context's entries into shared state before navigating. + // Fire the collection fetch immediately so it overlaps with the list fetch + // (both are independent after org_switch above). We await them sequentially + // below so setState({ entries }) still happens before setState({ orgCollections }). + const colPromise = data.context !== 'personal' + ? sendMessage({ type: 'org_list_collections' }) + : null; + // messageForList() reads the just-set orgContext so it picks the right SW // message (list_items for personal, org_list_items for org). Both popup's // renderItemList and vault's renderListPane read state.entries, so updating @@ -81,8 +87,8 @@ export async function renderOrgSwitcher(host: HTMLElement): Promise { if (data.context === 'personal') { setState({ orgCollections: [], collectionFilter: undefined }); } else { - const colResp = await sendMessage({ type: 'org_list_collections' }); - if (colResp.ok) setState({ orgCollections: colResp.data as Collection[] }); + const colResp = colPromise ? await colPromise : null; + if (colResp?.ok) setState({ orgCollections: colResp.data as Collection[] }); } navigate('list', {}); diff --git a/extension/src/popup/components/types/__tests__/detail-read-only.test.ts b/extension/src/popup/components/types/__tests__/detail-read-only.test.ts new file mode 100644 index 0000000..723655f --- /dev/null +++ b/extension/src/popup/components/types/__tests__/detail-read-only.test.ts @@ -0,0 +1,101 @@ +// Regression test: in org context the detail view must NOT render edit or +// trash buttons (Finding #1 from the whole-branch code review). + +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +// Mock shared/org-context so currentContext() returns a non-personal org. +vi.mock('../../../../shared/org-context', () => ({ + currentContext: vi.fn(() => 'acme-corp'), + messageForGet: vi.fn((id: string) => ({ type: 'org_get_item', id })), + messageForList: vi.fn(() => ({ type: 'org_list_items' })), + normalizeOrgEntries: vi.fn((e: unknown[]) => e), +})); + +vi.mock('../../../../shared/state', async () => { + const navigate = vi.fn(); + const setState = vi.fn(); + const sendMessage = vi.fn(); + const getState = vi.fn(() => ({ + view: 'detail', entries: [], selectedId: 'abc123', selectedItem: null, + selectedIndex: 0, searchQuery: '', activeGroup: null, error: null, + loading: false, capturedTabId: null, capturedUrl: '', newType: null, + orgContext: 'acme-corp', orgOffline: false, + })); + const escapeHtml = (s: string) => + s.replace(/&/g, '&').replace(//g, '>') + .replace(/"/g, '"').replace(/'/g, '''); + return { + navigate, setState, sendMessage, getState, escapeHtml, + popOutToTab: vi.fn(), isInTab: vi.fn(() => false), openVaultTab: vi.fn(), + }; +}); + +vi.mock('../../../../setup/setup-helpers', () => ({ + scheduleRate: vi.fn(), + STRENGTH_LABELS: {}, + entropyText: vi.fn(() => ''), +})); + +import { renderDetail } from '../login'; +import type { Item } from '../../../../shared/types'; + +const ORG_LOGIN_ITEM: Item = { + id: 'aabbccdd00000001', + title: 'Acme Intranet', + type: 'login', + tags: [], + favorite: false, + created: 0, + modified: 0, + trashed_at: undefined, + notes: undefined, + group: undefined, + core: { + type: 'login', + username: 'alice', + password: 's3cr3t', + url: 'https://intranet.acme.com', + totp: undefined, + }, + sections: [], + attachments: [], + field_history: {}, +}; + +describe('login renderDetail — org context read-only gate', () => { + beforeEach(() => { + document.body.innerHTML = '
'; + // Minimal chrome stub for wireAttachmentsDisclosure + wireFieldHandlers + (globalThis as Record).chrome = { + storage: { + local: { + get: vi.fn((_k: unknown, cb: (v: Record) => void) => cb({})), + set: vi.fn((_o: unknown, cb?: () => void) => cb?.()), + }, + }, + runtime: { sendMessage: vi.fn() }, + }; + }); + + it('renders no edit or trash buttons in org context', async () => { + const app = document.getElementById('app')!; + await renderDetail(app, ORG_LOGIN_ITEM); + + expect(document.getElementById('edit-btn')).toBeNull(); + expect(document.getElementById('trash-btn')).toBeNull(); + }); + + it('keeps the back button (read affordance) in org context', async () => { + const app = document.getElementById('app')!; + await renderDetail(app, ORG_LOGIN_ITEM); + + expect(document.getElementById('back-btn')).not.toBeNull(); + }); + + it('keeps the fill/autofill button (read affordance) in org context', async () => { + const app = document.getElementById('app')!; + await renderDetail(app, ORG_LOGIN_ITEM); + + expect(document.getElementById('fill-btn')).not.toBeNull(); + }); +}); diff --git a/extension/src/popup/components/types/card.ts b/extension/src/popup/components/types/card.ts index 9413011..127ef0b 100644 --- a/extension/src/popup/components/types/card.ts +++ b/extension/src/popup/components/types/card.ts @@ -2,6 +2,7 @@ /// Detail view has a styled card-silhouette signature block. import { getState, setState, sendMessage, navigate, escapeHtml, popOutToTab } from '../../../shared/state'; +import { currentContext } from '../../../shared/org-context'; import { renderFormHeader } from '../form-header'; import { REQUIRED_PILL_HTML } from '../../../shared/glyphs'; import type { Item, ItemId, ManifestEntry, CardKind, Section, AttachmentRef } from '../../../shared/types'; @@ -58,6 +59,7 @@ function formatExpiry(e: { month: number; year: number } | undefined): string { export async function renderDetail(app: HTMLElement, item: Item): Promise { if (item.core.type !== 'card') return; + const isPersonal = currentContext() === 'personal'; const c = item.core; const number = c.number ?? ''; const brand = brandFromNumber(number); @@ -94,8 +96,8 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise ${renderAttachmentsDisclosure({ itemId: item.id, attachments: item.attachments, mode: 'view' })}
- - + ${isPersonal ? '' : ''} + ${isPersonal ? '' : ''}
`; @@ -127,8 +129,11 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise } switch (e.key) { case 'Escape': teardown(); navigate('list'); break; - case 'e': teardown(); navigate('edit'); break; + case 'e': + if (!isPersonal) break; + teardown(); navigate('edit'); break; case 'd': + if (!isPersonal) break; e.preventDefault(); if (confirm(`Move "${item.title}" to trash?`)) { teardown(); diff --git a/extension/src/popup/components/types/document.ts b/extension/src/popup/components/types/document.ts index a60cf5e..4f05e72 100644 --- a/extension/src/popup/components/types/document.ts +++ b/extension/src/popup/components/types/document.ts @@ -3,6 +3,7 @@ /// Primary attachment is referenced by ID from the item's attachments array. import { getState, setState, sendMessage, navigate, escapeHtml, popOutToTab } from '../../../shared/state'; +import { currentContext } from '../../../shared/org-context'; import { renderFormHeader } from '../form-header'; import { REQUIRED_PILL_HTML, GLYPH_TYPE_DOCUMENT, GLYPH_PREVIEW } from '../../../shared/glyphs'; import type { Item, ItemId, ManifestEntry, Section, AttachmentRef } from '../../../shared/types'; @@ -264,6 +265,7 @@ async function saveDocument( export async function renderDetail(app: HTMLElement, item: Item): Promise { teardown(); if (item.core.type !== 'document') return; + const isPersonal = currentContext() === 'personal'; const c = item.core; const primaryRef = item.attachments.find((a) => a.id === c.primary_attachment); if (!primaryRef) { @@ -301,8 +303,8 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise
- - + ${isPersonal ? '' : ''} + ${isPersonal ? '' : ''}
`; @@ -381,8 +383,11 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise } switch (e.key) { case 'Escape': teardown(); navigate('list'); break; - case 'e': teardown(); navigate('edit'); break; + case 'e': + if (!isPersonal) break; + teardown(); navigate('edit'); break; case 'd': + if (!isPersonal) break; e.preventDefault(); if (confirm(`Move "${item.title}" to trash?`)) { teardown(); diff --git a/extension/src/popup/components/types/identity.ts b/extension/src/popup/components/types/identity.ts index f9776c1..6bc8a33 100644 --- a/extension/src/popup/components/types/identity.ts +++ b/extension/src/popup/components/types/identity.ts @@ -2,6 +2,7 @@ /// Detail view shows a "profile card" signature block + plain rows. import { getState, setState, sendMessage, navigate, escapeHtml, popOutToTab } from '../../../shared/state'; +import { currentContext } from '../../../shared/org-context'; import { renderFormHeader } from '../form-header'; import { REQUIRED_PILL_HTML } from '../../../shared/glyphs'; import type { Item, ItemId, ManifestEntry, Section, AttachmentRef } from '../../../shared/types'; @@ -48,6 +49,7 @@ function formatDate(iso: string | undefined): string { export async function renderDetail(app: HTMLElement, item: Item): Promise { if (item.core.type !== 'identity') return; + const isPersonal = currentContext() === 'personal'; const c = item.core; const sigInner = `
@@ -72,8 +74,8 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise ${renderAttachmentsDisclosure({ itemId: item.id, attachments: item.attachments, mode: 'view' })}
- - + ${isPersonal ? '' : ''} + ${isPersonal ? '' : ''}
`; @@ -103,8 +105,11 @@ export async function renderDetail(app: HTMLElement, item: Item): Promise } switch (e.key) { case 'Escape': teardown(); navigate('list'); break; - case 'e': teardown(); navigate('edit'); break; + case 'e': + if (!isPersonal) break; + teardown(); navigate('edit'); break; case 'd': + if (!isPersonal) break; e.preventDefault(); if (confirm(`Move "${item.title}" to trash?`)) { teardown(); diff --git a/extension/src/popup/components/types/key.ts b/extension/src/popup/components/types/key.ts index 36cb3ba..d62b2b6 100644 --- a/extension/src/popup/components/types/key.ts +++ b/extension/src/popup/components/types/key.ts @@ -3,6 +3,7 @@ /// since