From 3bf674729e2d6772374b791299dbbc263f201776 Mon Sep 17 00:00:00 2001 From: Rendi Wijaya Date: Sat, 3 Oct 2026 01:05:15 +0700 Subject: [PATCH] feat(extension): save prompt for logins typed on a page After a trusted sign-in submit that follows a user gesture, the content script reads the form's username and password and the service worker keeps the pair per tab in chrome.storage.session, stamped with the submit's sender.url and a random id. The next page load in that tab, or the same page once the password has left its form, asks for the prompt: the new check-login action compares the pair with the submitted host's exact-host entries (new, changed, unchanged) and answers other-site when the asking page is not on the submit's site by the Domain rule. It never returns a password. The prompt is the inline picker in the viewport's top-right corner, never focused, with Add, Update (or a list of the host's entries) and Cancel. Add and Update pass the six guards and send save-login with via: "inline" and the pair's id, so they save only the pair the prompt shows; a new submit closes a prompt that is showing. No prompt when the pair is already stored, the vault is locked or the app is not running. A pending pair is dropped once answered, when its tab closes, after 3 page loads, or on the first page load after 5 minutes. Card codes and one-time codes in password fields are not captured; a password field toggled to text before the submit still counts. Docs: SECURITY.md (web pages), KNOWN-LIMITS.md, ARCHITECTURE.md flow 5, SUBCLAVE.md extension rules, CHANGELOG.md. Closes #9. --- ARCHITECTURE.md | 3 +- CHANGELOG.md | 2 +- KNOWN-LIMITS.md | 7 +- SECURITY.md | 25 +- SUBCLAVE.md | 14 +- extension/src/background.ts | 158 +++++++- extension/src/content/fill.ts | 6 +- extension/src/content/index.ts | 3 +- extension/src/content/inline.ts | 253 ++++++++++++- extension/src/lib/messages.ts | 23 +- extension/src/lib/protocol.ts | 6 + extension/test/fixtures/change-password.html | 31 ++ extension/test/fixtures/checkout.html | 14 + extension/test/fixtures/landing.html | 10 + extension/test/fixtures/planted-submit.html | 29 ++ extension/test/fixtures/save-login.html | 24 ++ extension/test/guard.spec.ts | 34 ++ extension/test/playwright.config.ts | 7 +- extension/test/save.spec.ts | 372 +++++++++++++++++++ extension/test/support/fake-app.ts | 22 ++ src-tauri/src/modules/browser/actions.rs | 171 ++++++++- src-tauri/src/modules/browser/protocol.rs | 1 + src-tauri/src/modules/browser/server.rs | 3 + 23 files changed, 1170 insertions(+), 48 deletions(-) create mode 100644 extension/test/fixtures/change-password.html create mode 100644 extension/test/fixtures/checkout.html create mode 100644 extension/test/fixtures/landing.html create mode 100644 extension/test/fixtures/planted-submit.html create mode 100644 extension/test/fixtures/save-login.html create mode 100644 extension/test/save.spec.ts diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 380ce6c..6898fe7 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -113,7 +113,8 @@ App data dir of the bundle id (`dev.rendy.subclave`; `pnpm tauri:dev` uses 3. **Lock**: manual, from the lock button, the status bar or the `vault.lock` shortcut (`useVaultStore.lock`, `vault_lock`, `lock_inner`); idle, when `spawn_vault_tick_thread` calls `VaultState::access` every 5 s past the deadline that `vault_touch` extends on activity (`drain_auto_lock`); minimize, in `on_main_window_event` when `lock_on_minimize` is set; tray, from the Lock item in `tray.rs`. Every path calls `Unlocked::wipe` and drops the key, then `emit_locked` closes the sync session (`session_closed`), drops every browser connection (`close_all`) and emits `subclave:vault-locked` with the `LockReason`. 4. **Sync pull and push**: `startSync` builds the scheduler (`createPolicy`). It pulls on unlock and on window focus (`onFocus`, at most once per `FOCUS_INTERVAL_MS`), and pushes `PUSH_DEBOUNCE_MS` after the first change of a burst (`markDirty`, on any `subclave:vault-changed` whose origin is not `sync`). Passes run one at a time (`serialize`) and re-read the switch each time (`readConfig`). Pull: `sync_pull` snapshots `locals_from_payload` and the etags, releases the lock, runs `pull` (list, fetch the objects whose etag moved, `open_envelope`, `merge`, prune this device's expired tombstones), lands the report with `apply_pull` (`merge_into_payload` per record) and `commit`s, then emits origin `sync`. Push: `sync_push` takes `take_dirty_envelopes`, `push` seals each one (`seal_envelope`) and writes it conditionally on its etag where the backend can, re-merging once on a conflict (`retry_merged`), then `finish_push` clears the marks that landed and `commit`s. 5. **Browser fill**: every request goes service worker, `createNativeTransport`, `subclave-proxy` (`read_frame`, `write_frame`), then the app's `accept_loop`, `handle_conn` and `dispatch` (status, locked check, `hello` and `auth` handshake, then the action). - - Inline: `startInline` and `createInline` draw an icon per field found by `scan`. Opening the picker sends `inline-logins`; the service worker answers it in `handleInline` for the sender's own URL at `scope: "host"` (`getState`, `get-logins`, `get_logins`, which returns no password). A pick passes `guard.check` (`createGuard`), then `inline-fill` runs `fillEntry` with `via: "inline"`: `get-credential` (`get_credential` re-runs `matching::matches` and requires `same_host`), then `subclave:fill` to the content script, where `fillTarget` refuses another origin and `fillCredential` fills. + - Inline: `startInline` and `createInline` draw an icon per field found by `scan`. Opening the picker sends `inline-logins`; the service worker answers it in `handleInline` at `scope: "host"`, and fills and the picker use the sender's own URL (`getState`, `get-logins`, `get_logins`, which returns no password). A pick passes `guard.check` (`createGuard`), then `inline-fill` runs `fillEntry` with `via: "inline"`: `get-credential` (`get_credential` re-runs `matching::matches` and requires `same_host`), then `subclave:fill` to the content script, where `fillTarget` refuses another origin and `fillCredential` fills. + - Save prompt: `pass` wires the form of each login field (`wireForm`). A trusted submit, or a click on its submit button, that follows a user gesture reads the pair with `submittedLogin` and sends `inline-submitted`; the service worker keeps it per tab in `chrome.storage.session` with the sender's URL and a fresh id. Each page load in that tab, and the submitting page itself once the password has left its form, sends `inline-pending-save`, which runs `check-login` with the asking page's URL as `pageUrl` (`check_login`: `other-site` off the submit's site by the Domain rule, else the submitted host's exact-host entries, no password returned); a new or changed login opens the picker as a prompt in the viewport's corner (`promptSave`). Add or Update pass `guard.check`, then `inline-save` carries the pair's id and sends `save-login` with `via: "inline"` (`save_login` requires `same_host` for an update, and the old password goes to history); Cancel sends `inline-save-cancel`, which drops the pair. A new submit closes a prompt that is showing. - Popup: `PopupApp` asks `get-state` for the active tab at `scope: "all"`, and a pick sends `fill-entry`, which runs `fillEntry` with `via: "popup"` (`ensureContentScript` injects `content.js` when it is missing). - Command: `fill-login` runs `runFillCommand`: `get-logins` for the active tab, then one match fills with `via: "command"`, and several open the popup where `chrome.action.openPopup` exists, or fill the most recently used one. - A released credential stamps `lastUsedAt` and emits `subclave:vault-changed` with origin `browser`. diff --git a/CHANGELOG.md b/CHANGELOG.md index e3b87e8..787dec1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,5 +13,5 @@ The latest release only. Every earlier version: - Entries and nested groups with tags, favourites, expiry dates, search, Trash and per-entry version history. - Copy with clipboard auto-clear, TOTP codes, and a password generator with a strength meter. - End-to-end encrypted sync to your own S3-compatible bucket or WebDAV share, with a merge that keeps both sides of a conflict. -- Browser extension for Chromium browsers and Firefox: pairing, an icon and picker inside login fields, popup fill, the fill command, and "Generate for this site". +- Browser extension for Chromium browsers and Firefox: pairing, an icon and picker inside login fields, popup fill, the fill command, "Generate for this site", and a save prompt that adds or updates a login typed on a page. - CSV import from KeePassXC, Bitwarden, Chrome and Firefox, CSV export, and encrypted `.subclave-backup` files. diff --git a/KNOWN-LIMITS.md b/KNOWN-LIMITS.md index 6c36f6f..e9b9ba6 100644 --- a/KNOWN-LIMITS.md +++ b/KNOWN-LIMITS.md @@ -28,10 +28,15 @@ lives, and what would change it. - **No fill in cross-origin iframes**: the content script runs in the top frame only and reaches same-origin iframes through `contentDocument`, which a cross-origin frame does not expose, so a login form inside one is neither detected nor filled. `extension/src/content/fill.ts` (`rootsOf`). Changes when: the content script runs in every frame with a per-frame origin check. - **The inline guards are JavaScript against JavaScript**: the six clickjacking guards run in the same page as the code they guard against, and Firefox has no `IntersectionObserver` v2, so the visibility guard (guard 6) runs on Chromium only. `extension/src/content/guard.ts` (`createGuard`). Changes when: browsers give extension UI a visibility guarantee. -- **No inline UI without the Popover API**: Firefox 115 to 124 (and Chrome before 114) get no icon in login fields; the popup and the fill command still work. `extension/src/content/inline.ts` (`startInline`). Changes when: the minimum Firefox version reaches 125. +- **No inline UI without the Popover API**: Firefox 115 to 124 (and Chrome before 114) get no icon in login fields and no save prompt; the popup and the fill command still work. The save prompt is part of the inline UI, so "Show in login fields" off turns it off too. `extension/src/content/inline.ts` (`startInline`). Changes when: the minimum Firefox version reaches 125. - **Very late login forms get no icon**: a page with no login field 5 seconds after it has loaded stops watching for good, so a single-page app that shows its login form later (or navigates to it without a page load) gets no icon. The popup and the fill command still fill it. `extension/src/content/inline.ts` (`SETTLE_AFTER_LOAD_MS`, `settle` in `createInline`). Changes when: detection keeps a cheap watch running on pages without a field. - **An unhinted first login step gets no icon**: a text field on its own (the first step of a two-step login) gets an icon only when it says `autocomplete="username"` or `"email"`, which keeps icons out of search boxes and newsletter fields. A first step without the hint (Yahoo and Zoom when last checked) gets no icon; the popup and the fill command still fill it. `extension/src/content/detect.ts` (`scan`). Changes when: detection gets a name or label heuristic that still leaves search and newsletter fields alone. - **"Show in login fields" applies from the next page load**: the content script reads the setting once when the page loads, so open tabs keep their current state. `extension/src/content/index.ts` (`askSettings`). Changes when: the service worker tells open tabs about the change. +- **No save prompt for password-only forms**: a submitted form is offered for saving only when it holds a username next to the filled password, so the second step of a two-step login, and a change-password form without a username field, get no prompt. An update from such a form would blank the stored username. `extension/src/content/inline.ts` (`submittedLogin`). Changes when: the first step's username is carried to the second. +- **No save prompt for formless or scripted sign-ins**: the submit is caught on the `<form>` itself, from its `submit` event or a click on its submit button, so a sign-in sent by script without a `<form>`, from a `type="button"` button or a non-button element, or by page script without a user gesture, is never offered. `extension/src/content/inline.ts` (`wireForm`, `SUBMIT_BUTTON`). Changes when: detection gets a formless submit rule. +- **A refused sign-in still waits to be saved**: a refused sign-in, or a click on a submit-type button that signs nothing in (a show-password toggle without `type="button"`), still leaves a pending sign-in. The page shows no prompt while the password is still in its form, but the tab's next page load offers it. `extension/src/content/inline.ts` (`wireForm`, `submitted`). Changes when: a sign-in can be told apart from a refused one. +- **A Domain-rule login still prompts on other hosts**: a login stored only under a Domain rule still gets a save prompt on another host of that domain, because the check, like every inline request, sees exact-host entries only. `src-tauri/src/modules/browser/actions.rs` (`check_login`). Changes when: wider matches get a browser-owned save path. +- **A typed password waits in session storage**: a pending sign-in is dropped after 3 page loads in its tab (loads on another site count, though the prompt shows only on the submit's site), and after 5 minutes once its tab loads another page. Until then, or until the tab closes or the browser exits, the typed password stays in `chrome.storage.session`, which the browser keeps in memory. `extension/src/background.ts` (`PENDING_SAVE_TTL_MS`, `PENDING_SAVE_PAGE_LOADS`). Changes when: the service worker can expire it on a timer that survives a restart. - **Sandboxed browsers**: for Snap and Flatpak installs the manifests are also written into the sandboxed profile path and Settings > Browser marks them, but native messaging from Snap Firefox is not tested yet. `src-tauri/src/modules/browser/manifests.rs` (`BrowserSlot`, `sandboxed`). Changes when: a run on Snap Firefox confirms it, or shows what it needs. ## Import and backup diff --git a/SECURITY.md b/SECURITY.md index 5ea4250..7d28951 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -22,7 +22,30 @@ Until `1.0.0`, only the latest minor gets security fixes. - **Web pages.** A malicious or compromised site must not trigger a fill, get a credential for another site, or read the vault. - The content script runs in the top frame only. It never reads extension - storage, and it holds a credential only after a guarded pick. + storage. It holds a credential only after a guarded pick, and reads the + login the user typed only on a submit that is trusted and follows a user + gesture. + - The service worker keeps that typed pair in `chrome.storage.session`, + for its tab only, until the save prompt is answered, the tab closes, the + tab's third page load passes, or the first page load after 5 minutes. + The save URL is the browser-stamped `MessageSender.url` of the submit. + The prompt shows only on a page of the submit's site: Rust compares the + asking page's `MessageSender.url` with the submit's URL by the Domain rule + (`check_login`). `check-login` sees the submitted host's exact-host + entries only and never returns a password. Add and Update pass the six + guards and save only the pair the prompt shows: the pair's id goes back + with the click, and a new submit closes a prompt that is showing. + Residual risk: + - After a user gesture, a page can plant values in its own form and + submit them. It can then learn whether a planted pair matches an entry + for its own exact host (whether the prompt shows), and offer the user an + Update the user must still click; the overwritten password stays in the + entry's history. + - When the tab moves to another host of the same site (`login.example.com` + to `www.example.com`), that host's page shows the prompt with the + submitted host's entry titles and usernames, inside the closed shadow + root. Its Add and Update can only save the pair the user typed to the + submitted host. - The inline picker offers only exact-host matches. The scheme and port rules still apply. Wider matches (subdomains, parent domain) are reachable only from diff --git a/SUBCLAVE.md b/SUBCLAVE.md index c2ea278..a3f591c 100644 --- a/SUBCLAVE.md +++ b/SUBCLAVE.md @@ -91,14 +91,16 @@ Agent memory and contributor reference for Subclave. Build and PR rules: verify script pins the agreement. - The service worker routes by sender (`senderKind` in `src/background.ts`): popup requests only from extension pages, inline requests only from the - top-frame content script, answered for the sender's own tab URL at - `scope: "host"` and `via: "inline"`. + top-frame content script, answered for the sender's own tab URL or, for the + save prompt, the URL the same tab's submit was stamped with (shown only on a + page of that site, which Rust decides), at `scope: "host"` and + `via: "inline"`. - The inline UI (`src/content/inline.ts`) lives in one closed shadow root under `<subclave-inline>`, built with `createElement`/`textContent` (no - `innerHTML`). Every fill, generate or update pick passes the six guards in - `src/content/guard.ts` first; `test/guard.spec.ts` has one attack fixture - per guard. `scripts/build.mjs` fails a build whose `content.js` is over - 30 KB. + `innerHTML`). Every fill, generate or update pick, and every save-prompt Add + or Update, passes the six guards in `src/content/guard.ts` first; + `test/guard.spec.ts` has one attack fixture per guard. `scripts/build.mjs` + fails a build whose `content.js` is over 30 KB. ### Import and backup (`src-tauri/src/modules/import/`, `src-tauri/src/modules/backup.rs`, `src/modules/backup/`) diff --git a/extension/src/background.ts b/extension/src/background.ts index d5f8700..0322c71 100644 --- a/extension/src/background.ts +++ b/extension/src/background.ts @@ -13,12 +13,14 @@ import type { InlineRequest, PairingCodeEvent, PopupRequest, + SavePrompt, SwResponse, SwState, } from "./lib/messages"; import { PAIRING_CODE_EVENT } from "./lib/messages"; import { PROTOCOL_VERSION } from "./lib/protocol"; import type { + CheckLoginResult, CredentialResult, GenerateResult, GetLoginsResult, @@ -35,6 +37,22 @@ import type { Transport } from "./lib/transport"; const CREDENTIALS_KEY = "subclave.credentials"; const SHOW_IN_LOGIN_FIELDS_KEY = "subclave.showInLoginFields"; +/** One pending sign-in per tab, in `chrome.storage.session` so it survives a + * service worker restart between the submit and the next page load. */ +const PENDING_SAVE_PREFIX = "subclave.pendingSave."; +/** How long a submitted login waits for its prompt to be answered. */ +const PENDING_SAVE_TTL_MS = 5 * 60_000; +/** How many page loads in its tab may show the prompt. */ +const PENDING_SAVE_PAGE_LOADS = 3; + +type PendingSave = { + id: string; + url: string; + username: string; + password: string; + at: number; + loads: number; +}; type Credentials = { clientId: string; secret: string }; type CallResult<T> = { ok: true; result: T } | { ok: false; code: NmErrorCode; message: string }; @@ -59,6 +77,10 @@ const INLINE_REQUEST_TYPES: Record<string, true> = { "inline-generate": true, "inline-pair": true, "inline-focus-app": true, + "inline-submitted": true, + "inline-pending-save": true, + "inline-save": true, + "inline-save-cancel": true, }; export type BackgroundOptions = { @@ -452,9 +474,112 @@ export function startBackground(options: BackgroundOptions = {}): void { return { type: "focus", ok: response.ok }; }; - /** Every inline request is answered for the sender's own tab and URL, at - * `scope: "host"` and `via: "inline"`, so the picker can never reach another - * page's logins or the popup's wider match. */ + // ponytail: one chain for all tabs; per-tab chains if it ever matters + let pendingChain: Promise<unknown> = Promise.resolve(); + /** Runs `step` after every earlier one. Storage has no transactions, so each + * read-modify-write of a pending pair is one step; native calls stay out. */ + const withPending = <T>(step: () => Promise<T>): Promise<T> => { + const result = pendingChain.then(step); + pendingChain = result.catch(() => undefined); + return result; + }; + + /** Call inside a `withPending` step. */ + const readPending = async (tabId: number): Promise<PendingSave | null> => { + const key = PENDING_SAVE_PREFIX + tabId; + const stored = await chrome.storage.session.get(key); + // `inline-submitted` is the only writer of this key. + return (stored[key] as PendingSave | undefined) ?? null; + }; + + /** Drops the tab's pair only if it is still the one named `id`, so a slow + * answer never deletes a newer sign-in. */ + const dropPending = (tabId: number, id: string): Promise<void> => + withPending(async () => { + if ((await readPending(tabId))?.id === id) { + await chrome.storage.session.remove(PENDING_SAVE_PREFIX + tabId); + } + }); + + /** The prompt for the tab's pending sign-in on the page at `url`, counted as + * one of its page loads. Nothing pending never connects to the app; a stored + * pair is dropped, and a failed check (locked, not running, unpaired) or a + * page on another site keeps it for later. Expiry is checked here only, so a + * prompt already showing can still be answered. */ + const pendingPrompt = async (tabId: number, url: string): Promise<SwResponse> => { + const none: SwResponse = { type: "save-prompt", prompt: null }; + const key = PENDING_SAVE_PREFIX + tabId; + const pending = await withPending(async () => { + const stored = await readPending(tabId); + if (!stored) return null; + if (stored.loads >= PENDING_SAVE_PAGE_LOADS || Date.now() - stored.at > PENDING_SAVE_TTL_MS) { + await chrome.storage.session.remove(key); + return null; + } + await chrome.storage.session.set({ [key]: { ...stored, loads: stored.loads + 1 } }); + return stored; + }); + if (!pending) return none; + const checked = await call<CheckLoginResult>("check-login", { + url: pending.url, + pageUrl: url, + username: pending.username, + password: pending.password, + }); + if (!checked.ok || checked.result.state === "other-site") return none; + if (checked.result.state === "unchanged") { + await dropPending(tabId, pending.id); + return none; + } + let host = pending.url; + try { + host = new URL(pending.url).hostname; + } catch { + // Keep the raw URL as the label. + } + const prompt: SavePrompt = { + id: pending.id, + host, + username: pending.username, + entries: checked.result.entries, + }; + return { type: "save-prompt", prompt }; + }; + + /** Adds the pending sign-in `id` (`entryId` null) or updates that entry with + * it; a newer sign-in in the tab answers `no-match`. */ + const savePending = async ( + tabId: number, + id: string, + entryId: string | null, + ): Promise<SwResponse> => { + const pending = await withPending(() => readPending(tabId)); + if (pending?.id !== id) { + return { + type: "save", + ok: false, + code: "no-match", + message: "This sign-in is no longer waiting to be saved.", + }; + } + const saved = await call<SaveLoginResult>("save-login", { + url: pending.url, + username: pending.username, + password: pending.password, + ...(entryId ? { entryId } : {}), + via: "inline", + }); + if (!saved.ok) return { type: "save", ok: false, code: saved.code, message: saved.message }; + await dropPending(tabId, id); + return { type: "save", ok: true }; + }; + + /** Every inline request is answered for the sender's own tab at + * `scope: "host"` and `via: "inline"`. Fills and the picker use the sender's + * URL, so they never reach another host's logins or the popup's wider match. + * The save prompt saves to the browser-stamped URL of the tab's own submitted + * sign-in, and shows only on a page of that sign-in's site (`check-login` + * decides, by the vault's Domain rule). */ const handleInline = async ( message: InlineRequest, tabId: number, @@ -474,6 +599,29 @@ export function startBackground(options: BackgroundOptions = {}): void { return pair((event) => sendToTab(tabId, event)); case "inline-focus-app": return focusApp(); + case "inline-submitted": { + // `url` is the browser-stamped `sender.url`; the content script never + // names one. A new submit replaces the tab's old pair. + const pending: PendingSave = { + id: crypto.randomUUID(), + url, + username: message.username, + password: message.password, + at: Date.now(), + loads: 0, + }; + await withPending(() => + chrome.storage.session.set({ [PENDING_SAVE_PREFIX + tabId]: pending }), + ); + return { type: "submitted" }; + } + case "inline-pending-save": + return pendingPrompt(tabId, url); + case "inline-save": + return savePending(tabId, message.id, message.entryId); + case "inline-save-cancel": + await dropPending(tabId, message.id); + return { type: "save", ok: true }; } }; @@ -485,6 +633,10 @@ export function startBackground(options: BackgroundOptions = {}): void { if (command === "fill-login") void runFillCommand(); }); + chrome.tabs.onRemoved.addListener((tabId) => { + void chrome.storage.session.remove(PENDING_SAVE_PREFIX + tabId); + }); + chrome.runtime.onMessage.addListener((message, sender, sendResponse) => { const type = runtimeMessageType(message); if (type === null) return false; diff --git a/extension/src/content/fill.ts b/extension/src/content/fill.ts index 6aea666..a1f9c82 100644 --- a/extension/src/content/fill.ts +++ b/extension/src/content/fill.ts @@ -94,8 +94,12 @@ export function scopeOf(input: HTMLInputElement): Node { } /** `autocomplete` is a token list (`"username webauthn"`), not one value. */ +export function autocompleteTokens(input: HTMLInputElement): string[] { + return (input.getAttribute("autocomplete") ?? "").toLowerCase().split(/\s+/); +} + export function hasAutocompleteToken(input: HTMLInputElement, token: string): boolean { - return (input.getAttribute("autocomplete") ?? "").toLowerCase().split(/\s+/).includes(token); + return autocompleteTokens(input).includes(token); } /** The first text-like input before the password field among `inputs`, which diff --git a/extension/src/content/index.ts b/extension/src/content/index.ts index 9cc14a0..04f3d7e 100644 --- a/extension/src/content/index.ts +++ b/extension/src/content/index.ts @@ -62,7 +62,8 @@ if (!scope.subclaveContent) { }); // A page load reads one setting from storage and draws icons; no native - // connection opens until the user opens the picker. + // connection opens until the user opens the picker, unless the tab has a + // sign-in waiting for the save prompt. const askSettings = (): void => { sendToBackground<SwResponse>({ type: "inline-settings" }).then( (response) => { diff --git a/extension/src/content/inline.ts b/extension/src/content/inline.ts index 3e4e73f..23d516d 100644 --- a/extension/src/content/inline.ts +++ b/extension/src/content/inline.ts @@ -1,7 +1,9 @@ // The inline icon and picker: an icon in each detected login field, and a -// top-layer picker of the page's exact-host logins. A fill, generate or update -// row runs only after every guard in `guard.ts` passes. Matching, credential -// release and every write stay in the service worker and Rust. +// top-layer picker of the page's exact-host logins. After a trusted sign-in +// submit, the same picker shows the save prompt (Add, Update, Cancel) in the +// viewport's corner. A fill, generate, update, Add or Update row runs only +// after every guard in `guard.ts` passes. Matching, credential release and +// every write stay in the service worker and Rust. // // Everything is built with `createElement`/`textContent` inside one closed // shadow root. Placement goes through `element.style` (CSSOM, which a page CSP @@ -9,9 +11,19 @@ // the shadow `<style>` with the colours and fonts is refused. import { sendToBackground } from "../lib/messages"; -import type { InlineRequest, SwResponse, SwState } from "../lib/messages"; -import type { NmErrorCode } from "../lib/protocol"; +import type { InlineRequest, SavePrompt, SwResponse, SwState } from "../lib/messages"; +import type { NmErrorCode, SaveCandidate } from "../lib/protocol"; import { scan, topRect } from "./detect"; +import { + autocompleteTokens, + findAllInputs, + hasAutocompleteToken, + inputType, + isRendered, + rootsOf, + scopeOf, + usernamePartner, +} from "./fill"; import { createGuard, otherTopLayer } from "./guard"; import type { GuardId } from "./guard"; @@ -25,6 +37,15 @@ const DETECT_MAX_WAIT_MS = 1000; * apps render their form after `load`: Discord, X and Stripe took 1.5 to 2.2 s * when this was measured. */ const SETTLE_AFTER_LOAD_MS = 5000; +/** How long a page that saw a sign-in waits before showing the prompt + * itself; most sign-ins navigate first, and the next page's script shows it. */ +const SUBMIT_PROMPT_DELAY_MS = 1000; +/** Longer page-supplied values are never captured: they would only fail the + * native frame cap and fill `chrome.storage.session`. */ +const MAX_SUBMITTED_LENGTH = 1024; +/** The controls whose click submits their form. */ +const SUBMIT_BUTTON = + 'button:not([type]), button[type="submit"], input[type="submit"], input[type="image"]'; const DETECT_ATTRIBUTES = ["type", "style", "class", "hidden", "open", "autocomplete"]; const SVG_NS = "http://www.w3.org/2000/svg"; const KEY_PATH = @@ -43,6 +64,7 @@ const CSS = ` .title, .detail { display: block; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } .detail, .notes p { color: #555; } .notes:empty { display: none; } +.heading { margin: 0; padding: 6px 10px 2px; font-weight: 600; } .notes p { margin: 0; padding: 6px 10px; } .notes .code { color: inherit; font: 600 20px/1.4 ui-monospace, monospace; letter-spacing: 0.1em; } @media (prefers-color-scheme: dark) { @@ -111,6 +133,55 @@ function keyGlyph(): SVGSVGElement { return svg; } +/** The form's rendered inputs in document order, reached the way detection + * reaches them (open shadow roots, same-origin iframes). */ +function formInputs(form: HTMLFormElement): HTMLInputElement[] { + return findAllInputs(rootsOf(document)).filter((i) => isRendered(i) && scopeOf(i) === form); +} + +/** The rendered username and password `form` holds, or `null` without a + * filled password or without a username (a password-only form is the second + * step of a two-step login, and an update from it would blank the stored + * username). A password is an input detection once saw as one (`seen`), so a + * password shown as text at submit still counts; card codes and one-time + * codes never do. A change-password form gives its new password: the first + * `new-password` field with a value, else the last filled password + * (current, new, repeat). */ +function submittedLogin( + form: HTMLFormElement, + seen: WeakSet<HTMLInputElement>, +): { username: string; password: string } | null { + const own = formInputs(form); + const passwords = own.filter( + (i) => + (inputType(i) === "password" || seen.has(i)) && + i.value !== "" && + !autocompleteTokens(i).some((token) => token.startsWith("cc-") || token === "one-time-code"), + ); + if (passwords.length === 0) return null; + const password = ( + passwords.find((i) => hasAutocompleteToken(i, "new-password")) ?? + passwords[passwords.length - 1] + ).value; + const username = usernamePartner(own, passwords[0])?.value.trim() ?? ""; + if ( + username === "" || + username.length > MAX_SUBMITTED_LENGTH || + password.length > MAX_SUBMITTED_LENGTH + ) { + return null; + } + return { username, password }; +} + +/** A real user gesture within the activation window. `navigator.userActivation` + * is Chrome 72+ and Firefox 120+, both below the Popover API floor + * `startInline` requires; it is read through a named interface. */ +function userActive(): boolean { + const nav: Navigator & { userActivation?: { isActive: boolean } } = navigator; + return nav.userActivation?.isActive === true; +} + function createInline(): Inline { let running = true; @@ -138,7 +209,10 @@ function createInline(): Inline { notes.className = "notes"; notes.setAttribute("role", "status"); notes.setAttribute("aria-live", "polite"); - picker.append(list, notes); + const heading = document.createElement("p"); + heading.className = "heading"; + heading.setAttribute("role", "status"); + picker.append(heading, list, notes); // The page field keeps focus while the pointer works the picker. picker.addEventListener("mousedown", (event) => event.preventDefault()); shadow.append(style, picker); @@ -149,6 +223,9 @@ function createInline(): Inline { /** Detected field -> is a new-password field. */ let fields = new Map<HTMLInputElement, boolean>(); const wired = new WeakSet<HTMLInputElement>(); + const wiredForms = new WeakSet<HTMLFormElement>(); + /** Every input detection ever saw as a password, for `submittedLogin`. */ + const seenPasswords = new WeakSet<HTMLInputElement>(); const watched = new WeakSet<Document>(); let observed = new WeakSet<Document | ShadowRoot>(); let generation = 0; @@ -164,6 +241,10 @@ function createInline(): Inline { let active = 0; let seq = 0; let pending: HTMLInputElement | null = null; + /** The save prompt on show; `listing` once the user asked to pick the entry + * to update. The prompt is the picker open with no anchor. */ + let saving: { prompt: SavePrompt; listing: boolean } | null = null; + let submitSeq = 0; const isOpen = (): boolean => picker.matches(":popover-open"); @@ -173,6 +254,7 @@ function createInline(): Inline { anchor = null; anchorRect = null; rows = []; + saving = null; seq += 1; }; @@ -192,7 +274,15 @@ function createInline(): Inline { }; const place = (): void => { - if (!anchor) return; + if (!anchor) { + // The save prompt: the viewport's top-right corner, placed again by + // `queueReflow` when the viewport resizes. + const width = Math.min(PICKER_MIN_WIDTH, innerWidth - 8); + picker.style.width = `${width}px`; + picker.style.left = `${Math.max(4, document.documentElement.clientWidth - width - 8)}px`; + picker.style.top = "8px"; + return; + } const rect = topRect(anchor); anchorRect = rect; const width = Math.min(Math.max(rect.width, PICKER_MIN_WIDTH), innerWidth - 8); @@ -212,8 +302,9 @@ function createInline(): Inline { }; /** Every render while open re-places the picker and restarts guard 1's - * timer, so the delay runs from when pickable rows appear. */ - const render = (next: Row[], lines: HTMLElement[]): void => { + * timer, so the delay runs from when pickable rows appear. Visibility of + * the heading goes through `hidden`: a page CSP can refuse the `<style>`. */ + const render = (next: Row[], lines: HTMLElement[], head = ""): void => { rows = next; active = 0; list.replaceChildren( @@ -238,6 +329,8 @@ function createInline(): Inline { ); if (next.length > 0) list.setAttribute("aria-activedescendant", "subclave-opt-0"); else list.removeAttribute("aria-activedescendant"); + heading.textContent = head; + heading.hidden = head === ""; notes.replaceChildren(...lines); if (isOpen()) { place(); @@ -266,9 +359,17 @@ function createInline(): Inline { }; const refuse = (id: GuardId): void => { - const message = note("Use the Subclave toolbar button to fill on this page.", "message"); + const message = note( + saving + ? "Subclave did not accept that click." + : "Use the Subclave toolbar button to fill on this page.", + "message", + ); message.dataset.guard = String(id); - render([], [message]); + // The prompt's rows come back (restarting guard 1's delay) so the user + // can try again. + if (saving) showPrompt([message]); + else render([], [message]); }; /** `null` once the extension was updated or reloaded under this page: this @@ -432,6 +533,98 @@ function createInline(): Inline { await loadLogins(mine); }; + /** Saves the pending sign-in `id`, the one the prompt shows. */ + const save = async (id: string, entryId: string | null): Promise<void> => { + const mine = seq; + const response = await ask({ type: "inline-save", id, entryId }); + if (!response || mine !== seq) return; + if (response.type === "save" && response.ok) close(); + else if (response.type === "save" || response.type === "error") { + showPrompt([note(response.message)]); + } + }; + + const cancelSave = async (id: string): Promise<void> => { + close(); + await ask({ type: "inline-save-cancel", id }); + }; + + /** Add and Update write to the vault, so they pass every guard; "Update an + * existing login" and Cancel write nothing and need a trusted event only. */ + const showPrompt = (lines: HTMLElement[] = []): void => { + if (!saving) return; + const { prompt, listing } = saving; + const update = (entry: SaveCandidate): Row => ({ + label: `Update ${entry.title}`, + detail: entry.username, + guarded: true, + run: () => save(prompt.id, entry.id), + }); + const add: Row = { + label: "Add", + detail: prompt.username, + guarded: true, + run: () => save(prompt.id, null), + }; + const cancel: Row = { label: "Cancel", guarded: false, run: () => cancelSave(prompt.id) }; + const choose: Row = { + label: "Update an existing login", + guarded: false, + run: async () => { + if (saving) { + saving.listing = true; + showPrompt(); + } + }, + }; + const same = prompt.entries.filter((entry) => entry.username === prompt.username); + let next: Row[]; + if (listing) next = [...prompt.entries.map(update), cancel]; + else if (same.length === 1) next = [update(same[0]), add, cancel]; + else if (prompt.entries.length > 0) next = [add, choose, cancel]; + else next = [add, cancel]; + render(next, lines, `Save login for ${prompt.host}?`); + }; + + /** Shows the tab's pending sign-in, if the service worker has one to offer. + * Never takes focus. */ + const promptSave = async (): Promise<void> => { + const response = await ask({ type: "inline-pending-save" }); + if (!running || response?.type !== "save-prompt" || !response.prompt) return; + // The user is working the picker, or guard 2 would refuse every Add and + // Update: the pair stays pending for a later page load. + if ((anchor && isOpen()) || otherTopLayer()) return; + close(); + // Before `arm`, so the tamper observer never sees this move. + if (document.documentElement.lastElementChild !== host) document.documentElement.append(host); + saving = { prompt: response.prompt, listing: false }; + showPrompt(); + picker.showPopover(); + place(); + guard.arm(); + }; + + const submitted = async ( + form: HTMLFormElement, + login: { username: string; password: string }, + ): Promise<void> => { + // A page can plant values and submit them from its own `mousedown` on a + // prompt row: closing in that same task leaves the click nothing to land + // on, and the pair id the rows carry backs it up. + if (saving) close(); + // The click-then-submit pair of one sign-in sends two identical messages; + // only the last one keeps its timer. + const mine = (submitSeq += 1); + const response = await ask({ type: "inline-submitted", ...login }); + if (!response || !running) return; + setTimeout(() => { + // Still in the form: not signed in yet (a show-password toggle, a refused + // password), so only a later page load shows the prompt. + const stillTyped = formInputs(form).some((input) => input.value === login.password); + if (mine === submitSeq && running && !stillTyped) void promptSave(); + }, SUBMIT_PROMPT_DELAY_MS); + }; + const pick = async ( index: number, event: Event, @@ -508,6 +701,13 @@ function createInline(): Inline { if (!running) return; positionIcons(); if (anchor && isOpen() && moved()) close(); + else if (!anchor && isOpen()) { + // The save prompt follows the viewport's corner; moved under the + // pointer, guard 1's delay starts over, as for the anchored picker. + const { left, width } = picker.style; + place(); + if (picker.style.left !== left || picker.style.width !== width) guard.markShown(); + } }); }; @@ -570,6 +770,29 @@ function createInline(): Inline { }); }; + // `isTrusted` alone is not enough: `requestSubmit()` and `button.click()` + // from page script fire a trusted `submit`, so a real user gesture must be + // active too. The click path covers sites that sign in by script from the + // button's click and never fire `submit`. Nothing here calls + // `preventDefault`. + const wireForm = (form: HTMLFormElement): void => { + const capture = (event: Event): void => { + if (!running || !event.isTrusted || !userActive()) return; + const login = submittedLogin(form, seenPasswords); + if (login) void submitted(form, login); + }; + form.addEventListener("submit", capture, true); + form.addEventListener( + "click", + (event) => { + // A click target is always an element. + const target = event.target as Element | null; + if (target?.closest(SUBMIT_BUTTON)) capture(event); + }, + true, + ); + }; + const watch = (doc: Document): void => { if (watched.has(doc)) return; watched.add(doc); @@ -612,6 +835,7 @@ function createInline(): Inline { icons.delete(input); } for (const { input } of detected) { + if (inputType(input) === "password") seenPasswords.add(input); if (!icons.has(input)) { const icon = makeIcon(input); shadow.append(icon); @@ -621,6 +845,11 @@ function createInline(): Inline { wired.add(input); wire(input); } + const form = input.form; + if (form && !wiredForms.has(form)) { + wiredForms.add(form); + wireForm(form); + } watch(input.ownerDocument); } fields = next; @@ -664,6 +893,8 @@ function createInline(): Inline { { once: true }, ); } + // A sign-in this tab submitted on an earlier page. + void promptSave(); return { stop() { diff --git a/extension/src/lib/messages.ts b/extension/src/lib/messages.ts index 7d9c3a3..6acc0c3 100644 --- a/extension/src/lib/messages.ts +++ b/extension/src/lib/messages.ts @@ -2,7 +2,7 @@ // worker, between the content script's inline picker and the service worker, // and between the service worker and the content script. -import type { LoginSummary, NmErrorCode } from "./protocol"; +import type { LoginSummary, NmErrorCode, SaveCandidate } from "./protocol"; /** Sent service worker -> popup (or the tab whose picker asked to pair) before * the pairing response arrives. */ @@ -21,14 +21,27 @@ export type PopupRequest = | { type: "fill-command" }; /** Sent by the top-frame content script only; the service worker answers these - * for the sender's own tab and URL, never for another page. */ + * for the sender's own tab and its `sender.url`, never a URL the page names. + * The save-prompt requests (`inline-pending-save`, `inline-save`, + * `inline-save-cancel`) act on the sign-in that same tab submitted: it saves + * to the `sender.url` stamped on its `inline-submitted`, and shows only on a + * page of that URL's site (`check-login` decides). `id` names the pending + * sign-in the prompt shows, so a newer one is never written in its place. */ export type InlineRequest = | { type: "inline-settings" } | { type: "inline-logins" } | { type: "inline-fill"; entryId: string } | { type: "inline-generate"; entryId: string | null } | { type: "inline-pair" } - | { type: "inline-focus-app" }; + | { type: "inline-focus-app" } + | { type: "inline-submitted"; username: string; password: string } + | { type: "inline-pending-save" } + | { type: "inline-save"; id: string; entryId: string | null } + | { type: "inline-save-cancel"; id: string }; + +/** A sign-in waiting for the save prompt. `id` is the pending sign-in's own; + * `host` is the submitted page's hostname. */ +export type SavePrompt = { id: string; host: string; username: string; entries: SaveCandidate[] }; export type SwState = | { state: "locked" } @@ -54,6 +67,10 @@ export type SwResponse = | { type: "generate"; ok: false; code: NmErrorCode; message: string } | { type: "settings"; showInLoginFields: boolean } | { type: "focus"; ok: boolean } + | { type: "submitted" } + | { type: "save-prompt"; prompt: SavePrompt | null } + | { type: "save"; ok: true } + | { type: "save"; ok: false; code: NmErrorCode; message: string } | { type: "error"; code: NmErrorCode; message: string }; /** `url` is the page the credential was released for, and `anchored` says the diff --git a/extension/src/lib/protocol.ts b/extension/src/lib/protocol.ts index eef096e..de244de 100644 --- a/extension/src/lib/protocol.ts +++ b/extension/src/lib/protocol.ts @@ -22,6 +22,7 @@ export type NmAction = | "hello" | "auth" | "get-logins" + | "check-login" | "get-credential" | "save-login" | "generate-password"; @@ -64,6 +65,11 @@ export type LoginSummary = { export type GetLoginsResult = { entries: LoginSummary[]; otherMatches: number; domain: string }; export type CredentialResult = { username: string; password: string }; export type SaveLoginResult = { id: string; created: boolean }; +export type SaveCandidate = { id: string; title: string; username: string }; +export type CheckLoginResult = { + state: "new" | "changed" | "unchanged" | "other-site"; + entries: SaveCandidate[]; +}; export type GenerateResult = { password: string }; export type PairResult = { clientId: string; secret: string }; export type HelloResult = { appNonce: string; appProof: string }; diff --git a/extension/test/fixtures/change-password.html b/extension/test/fixtures/change-password.html new file mode 100644 index 0000000..1468f64 --- /dev/null +++ b/extension/test/fixtures/change-password.html @@ -0,0 +1,31 @@ +<!doctype html> +<html lang="en"> + <head> + <meta charset="utf-8" /> + <title>Change password + + +
+ + + + + + +
+ + + diff --git a/extension/test/fixtures/checkout.html b/extension/test/fixtures/checkout.html new file mode 100644 index 0000000..0b4dead --- /dev/null +++ b/extension/test/fixtures/checkout.html @@ -0,0 +1,14 @@ + + + + + Checkout + + +
+ + + +
+ + diff --git a/extension/test/fixtures/landing.html b/extension/test/fixtures/landing.html new file mode 100644 index 0000000..98e281a --- /dev/null +++ b/extension/test/fixtures/landing.html @@ -0,0 +1,10 @@ + + + + + Landing + + +

Signed in

+ + diff --git a/extension/test/fixtures/planted-submit.html b/extension/test/fixtures/planted-submit.html new file mode 100644 index 0000000..7ee312a --- /dev/null +++ b/extension/test/fixtures/planted-submit.html @@ -0,0 +1,29 @@ + + + + + Planted submit + + +
+ + + +
+ + + diff --git a/extension/test/fixtures/save-login.html b/extension/test/fixtures/save-login.html new file mode 100644 index 0000000..1462bee --- /dev/null +++ b/extension/test/fixtures/save-login.html @@ -0,0 +1,24 @@ + + + + + Save login + + +
+ + + + + +
+ + + diff --git a/extension/test/guard.spec.ts b/extension/test/guard.spec.ts index 1feb475..7719c7e 100644 --- a/extension/test/guard.spec.ts +++ b/extension/test/guard.spec.ts @@ -169,3 +169,37 @@ test("generate rows pass the same guards", async () => { log.some((entry) => entry.action === "generate-password" || entry.action === "save-login"), ).toBe(false); }); + +test("save prompt rows pass the same guards", async () => { + await setMode(harness, "no-fields"); + const page = await openFixture(harness, "save-login.html"); + const ui = inlineOf(page); + await expect.poll(() => ui.count(".icon")).toBe(2); + await page.locator("#username").fill("newuser"); + await page.locator("#password").fill("typed-pw"); + await page.locator("#submit").click(); + await page.waitForURL("**/landing.html"); + await waitForOptions(page, ui); + await ui.dispatchUntrustedClick(OPTION, 0); + await expect.poll(() => ui.pickerText()).toContain("Subclave did not accept that click."); + expect(await ui.attr(".message", "data-guard")).toBe("1"); + expect(await ui.count(OPTION)).toBe(2); + expect((await readLog(harness)).some((entry) => entry.action === "save-login")).toBe(false); +}); + +test("save prompt Update rows pass the same guards", async () => { + const page = await openFixture(harness, "save-login.html"); + const ui = inlineOf(page); + await expect.poll(() => ui.count(".icon")).toBe(2); + await page.locator("#username").fill(FAKE_ENTRIES[1].username); + await page.locator("#password").fill("new-pw"); + await page.locator("#submit").click(); + await page.waitForURL("**/landing.html"); + await waitForOptions(page, ui); + expect(await ui.pickerText()).toContain(`Update ${FAKE_ENTRIES[1].title}`); + await ui.dispatchUntrustedClick(OPTION, 0); + await expect.poll(() => ui.pickerText()).toContain("Subclave did not accept that click."); + expect(await ui.attr(".message", "data-guard")).toBe("1"); + expect(await ui.count(OPTION)).toBe(3); + expect((await readLog(harness)).some((entry) => entry.action === "save-login")).toBe(false); +}); diff --git a/extension/test/playwright.config.ts b/extension/test/playwright.config.ts index 69c1027..bd3c0f0 100644 --- a/extension/test/playwright.config.ts +++ b/extension/test/playwright.config.ts @@ -11,8 +11,9 @@ import { FIXTURE_PORT } from "./support/harness"; // chrome.scripting.executeScript // fallback) // -// `inline.spec.ts` and `guard.spec.ts` run under `chrome` only: the inline UI -// lives in the declarative content script, which `chrome-inject` does not have. +// `inline.spec.ts`, `guard.spec.ts` and `save.spec.ts` run under `chrome` only: +// the inline UI lives in the declarative content script, which `chrome-inject` +// does not have. // // Both carry the same manifest `key`, so the extension id, and therefore the // native host name, are identical. @@ -59,7 +60,7 @@ export default defineConfig({ { name: "chrome", testIgnore: ["**/sites.spec.ts"] }, { name: "chrome-inject", - testIgnore: ["**/inline.spec.ts", "**/guard.spec.ts", "**/sites.spec.ts"], + testIgnore: ["**/inline.spec.ts", "**/guard.spec.ts", "**/save.spec.ts", "**/sites.spec.ts"], }, ], }); diff --git a/extension/test/save.spec.ts b/extension/test/save.spec.ts new file mode 100644 index 0000000..b3602ae --- /dev/null +++ b/extension/test/save.spec.ts @@ -0,0 +1,372 @@ +import { expect, test } from "@playwright/test"; +import type { Page } from "@playwright/test"; +import { FAKE_ENTRIES, FAKE_PASSWORD } from "./support/fake-app"; +import { + FIXTURE_ORIGIN, + FIXTURE_PORT, + openFixture, + openPopup, + readLog, + setMode, + startHarness, + type Harness, +} from "./support/harness"; +import { clickInline, inlineOf, waitForOptions, type Inline } from "./support/inline"; + +// The save prompt for a login typed and submitted on a page: the submit is +// captured in the content script, the pair waits in the service worker's +// `chrome.storage.session`, and the next page load (or the same page, when it +// stays) offers Add or Update. + +const OPTION = '[role="option"]'; +const LANDING = `${FIXTURE_ORIGIN}/landing.html`; +const SAVE_LOGIN = `${FIXTURE_ORIGIN}/save-login.html`; +/** Another site to the fake app, which compares origins. */ +const OTHER_SITE = `http://localhost:${FIXTURE_PORT}/landing.html`; +/** `PENDING_SAVE_PREFIX` in `background.ts`. */ +const PENDING_PREFIX = "subclave.pendingSave."; + +let harness: Harness; + +test.beforeEach(async ({}, testInfo) => { + harness = await startHarness(testInfo); +}); + +test.afterEach(async () => { + if (harness) await harness.context.close(); +}); + +/** The form is wired in the same pass that draws the icons, so a fill before + * that can be missed. */ +async function ready(ui: Inline, icons: number): Promise { + await expect.poll(() => ui.count(".icon")).toBe(icons); +} + +async function signIn(page: Page, ui: Inline, username: string, password: string): Promise { + await ready(ui, 2); + await page.locator("#username").fill(username); + await page.locator("#password").fill(password); + await page.locator("#submit").click(); +} + +async function changePassword( + page: Page, + ui: Inline, + username: string, + current: string, + next: string, +): Promise { + await ready(ui, 4); + await page.locator("#username").fill(username); + await page.locator("#current").fill(current); + await page.locator("#new").fill(next); + await page.locator("#repeat").fill(next); + await page.locator("#submit").click(); +} + +async function calls(action: string): Promise | null>> { + return (await readLog(harness)) + .filter((entry) => entry.action === action) + .map((entry) => entry.params); +} + +async function lastCall(action: string): Promise | null | undefined> { + return (await calls(action)).at(-1); +} + +async function pendingKeys(): Promise { + const stored = await harness.worker.evaluate(() => chrome.storage.session.get(null)); + return Object.keys(stored).filter((key) => key.startsWith(PENDING_PREFIX)); +} + +test("a sign-in with no stored login offers Add on the next page, and Add saves it", async () => { + await setMode(harness, "no-fields"); + const page = await openFixture(harness, "save-login.html"); + const ui = inlineOf(page); + await signIn(page, ui, "newuser", "typed-pw"); + await page.waitForURL("**/landing.html"); + await expect.poll(() => ui.pickerText()).toContain("Save login for 127.0.0.1?"); + const text = await ui.pickerText(); + for (const part of ["Add", "newuser", "Cancel"]) expect(text).toContain(part); + expect(text).not.toContain("Update"); + await waitForOptions(page, ui); + await clickInline(page, ui, OPTION, 0); + await expect + .poll(() => lastCall("save-login")) + .toEqual({ url: SAVE_LOGIN, username: "newuser", password: "typed-pw", via: "inline" }); + await expect.poll(() => ui.popoverOpen()).toBe(false); + const checked = await lastCall("check-login"); + expect(checked?.url).toBe(SAVE_LOGIN); + expect(checked?.pageUrl).toBe(LANDING); +}); + +test("a new password for a stored username offers Update for that entry", async () => { + const page = await openFixture(harness, "save-login.html"); + const ui = inlineOf(page); + await signIn(page, ui, FAKE_ENTRIES[1].username, "new-pw"); + await page.waitForURL("**/landing.html"); + await expect + .poll(() => ui.pickerText()) + .toMatch(new RegExp(`^Save login for 127\\.0\\.0\\.1\\?Update ${FAKE_ENTRIES[1].title}`)); + await waitForOptions(page, ui); + await clickInline(page, ui, OPTION, 0); + await expect.poll(() => lastCall("save-login")).toBeTruthy(); + const params = await lastCall("save-login"); + expect(params?.entryId).toBe(FAKE_ENTRIES[1].id); + expect(params?.password).toBe("new-pw"); + expect(params?.via).toBe("inline"); +}); + +test("an unknown username picks the entry to update from a list", async () => { + const page = await openFixture(harness, "save-login.html"); + const ui = inlineOf(page); + await signIn(page, ui, "carol", "pw"); + await page.waitForURL("**/landing.html"); + await expect + .poll(() => ui.pickerText()) + .toBe("Save login for 127.0.0.1?AddcarolUpdate an existing loginCancel"); + await waitForOptions(page, ui); + await clickInline(page, ui, OPTION, 1); + await expect.poll(() => ui.pickerText()).toContain(`Update ${FAKE_ENTRIES[0].title}`); + await waitForOptions(page, ui); + expect(await ui.pickerText()).toContain(`Update ${FAKE_ENTRIES[1].title}`); + await clickInline(page, ui, OPTION, 1); + await expect.poll(() => lastCall("save-login")).toBeTruthy(); + const params = await lastCall("save-login"); + expect(params?.entryId).toBe(FAKE_ENTRIES[1].id); + expect(params?.username).toBe("carol"); +}); + +test("a stored login gets no prompt and is not checked again", async () => { + const page = await openFixture(harness, "save-login.html"); + const ui = inlineOf(page); + await signIn(page, ui, FAKE_ENTRIES[0].username, FAKE_PASSWORD); + await expect + .poll(async () => + (await readLog(harness)).some( + (entry) => entry.action === "check-login" && entry.result?.state === "unchanged", + ), + ) + .toBe(true); + await page.waitForTimeout(600); + expect(await ui.popoverOpen()).toBe(false); + const checks = (await calls("check-login")).length; + await page.goto(LANDING); + await page.waitForTimeout(1000); + expect((await calls("check-login")).length).toBe(checks); +}); + +test("a locked vault keeps the sign-in for later and Cancel drops it", async () => { + await setMode(harness, "locked"); + const page = await openFixture(harness, "save-login.html"); + const ui = inlineOf(page); + await signIn(page, ui, "bob", "new-pw"); + await page.waitForURL("**/landing.html"); + await page.waitForTimeout(1000); + expect(await ui.popoverOpen()).toBe(false); + await setMode(harness, "ok"); + await page.goto(LANDING); + await expect.poll(() => ui.pickerText()).toContain(`Update ${FAKE_ENTRIES[1].title}`); + await waitForOptions(page, ui); + await clickInline(page, ui, OPTION, (await ui.count(OPTION)) - 1); + await expect.poll(() => ui.popoverOpen()).toBe(false); + await page.goto(LANDING); + await page.waitForTimeout(1000); + expect(await ui.popoverOpen()).toBe(false); + expect(await calls("save-login")).toEqual([]); +}); + +test("a pending sign-in shows on at most three page loads", async () => { + const page = await openFixture(harness, "change-password.html"); + const ui = inlineOf(page); + await changePassword(page, ui, "alice", FAKE_PASSWORD, "next-pw"); + await expect.poll(() => ui.popoverOpen()).toBe(true); + for (let load = 2; load <= 3; load += 1) { + await page.goto(LANDING); + await expect.poll(() => ui.popoverOpen()).toBe(true); + } + await page.goto(LANDING); + await page.waitForTimeout(1000); + expect(await ui.popoverOpen()).toBe(false); +}); + +test("a change-password form that stays on the page prompts in place with the new password", async () => { + const page = await openFixture(harness, "change-password.html"); + const ui = inlineOf(page); + await changePassword(page, ui, "alice", FAKE_PASSWORD, "next-pw"); + await expect + .poll(() => ui.pickerText()) + .toMatch(new RegExp(`^Save login for 127\\.0\\.0\\.1\\?Update ${FAKE_ENTRIES[0].title}`)); + expect(page.url()).toBe(`${FIXTURE_ORIGIN}/change-password.html`); + await waitForOptions(page, ui); + await clickInline(page, ui, OPTION, 0); + await expect.poll(() => lastCall("save-login")).toBeTruthy(); + const params = await lastCall("save-login"); + expect(params?.password).toBe("next-pw"); + expect(params?.entryId).toBe(FAKE_ENTRIES[0].id); +}); + +test("an untrusted submit is ignored", async () => { + const page = await openFixture(harness, "save-login.html"); + const ui = inlineOf(page); + await ready(ui, 2); + await page.locator("#username").fill("bob"); + await page.locator("#password").fill("new-pw"); + await page.evaluate("window.__untrustedSubmit()"); + await page.goto(LANDING); + await page.waitForTimeout(1500); + expect(await ui.popoverOpen()).toBe(false); + expect(await calls("check-login")).toEqual([]); +}); + +test("the sign-in survives a service worker restart", async () => { + const page = await openFixture(harness, "change-password.html"); + const ui = inlineOf(page); + // The marker lives only in this worker instance's global scope. + await harness.worker.evaluate(() => { + const scope: typeof globalThis & { subclaveMarker?: boolean } = globalThis; + scope.subclaveMarker = true; + }); + await changePassword(page, ui, "alice", FAKE_PASSWORD, "next-pw"); + await expect.poll(() => ui.popoverOpen()).toBe(true); + + // Playwright keeps one `Worker` handle across the restart (no second + // `serviceworker` event), so the marker is read back through it. + const popup = await openPopup(harness.context, harness.extensionId); + const cdp = await harness.context.newCDPSession(popup); + await cdp.send("ServiceWorker.enable"); + await cdp.send("ServiceWorker.stopAllWorkers"); + await popup.close(); + + await page.goto(LANDING); + await expect.poll(() => ui.pickerText()).toContain(`Update ${FAKE_ENTRIES[0].title}`); + expect( + await harness.worker.evaluate(() => { + const scope: typeof globalThis & { subclaveMarker?: boolean } = globalThis; + return scope.subclaveMarker; + }), + ).toBeUndefined(); +}); + +test("a submit run by page script without a user gesture is ignored", async () => { + // CDP reads only until the planted submit has run: Playwright's `evaluate`, + // `fill` and `click` would all grant the page a user activation. + const page = await openFixture(harness, "planted-submit.html"); + const ui = inlineOf(page); + await ready(ui, 2); + await page.waitForTimeout(3000); + // The pair would be stored about a second before its `check-login`, so this + // read is the one with margin. + expect(await pendingKeys()).toEqual([]); + expect(await ui.popoverOpen()).toBe(false); + expect(await calls("check-login")).toEqual([]); + expect(await page.title()).toBe("submitted"); +}); + +test("a click that leaves the password in the form shows no prompt in place", async () => { + const page = await openFixture(harness, "change-password.html"); + const ui = inlineOf(page); + await ready(ui, 4); + await page.locator("#username").fill("alice"); + await page.locator("#current").fill(FAKE_PASSWORD); + await page.locator("#new").fill("next-pw"); + await page.locator("#repeat").fill("next-pw"); + await page.locator("#toggle").click(); + await page.waitForTimeout(1500); + expect(await ui.popoverOpen()).toBe(false); +}); + +test("a prompt saves only the sign-in it shows", async () => { + const page = await openFixture(harness, "save-login.html"); + const ui = inlineOf(page); + await signIn(page, ui, FAKE_ENTRIES[1].username, "new-pw"); + await page.waitForURL("**/landing.html"); + await expect.poll(() => ui.pickerText()).toContain(`Update ${FAKE_ENTRIES[1].title}`); + // A newer sign-in in the same tab, as a later submit would store it. + const [key] = await pendingKeys(); + await harness.worker.evaluate(async (pendingKey) => { + const stored = await chrome.storage.session.get(pendingKey); + await chrome.storage.session.set({ + [pendingKey]: { ...(stored[pendingKey] as object), id: "newer" }, + }); + }, key); + await waitForOptions(page, ui); + await clickInline(page, ui, OPTION, 0); + await expect + .poll(() => ui.pickerText()) + .toContain("This sign-in is no longer waiting to be saved."); + expect(await calls("save-login")).toEqual([]); +}); + +test("a new submit closes a shown prompt", async () => { + const page = await openFixture(harness, "change-password.html"); + const ui = inlineOf(page); + await changePassword(page, ui, "alice", FAKE_PASSWORD, "next-pw"); + await expect.poll(() => ui.pickerText()).toContain(`Update ${FAKE_ENTRIES[0].title}`); + await changePassword(page, ui, FAKE_ENTRIES[1].username, FAKE_PASSWORD, "other-pw"); + await expect.poll(() => ui.popoverOpen()).toBe(false); + await expect + .poll(() => ui.pickerText()) + .toMatch(new RegExp(`^Save login for 127\\.0\\.0\\.1\\?Update ${FAKE_ENTRIES[1].title}`)); + expect(await ui.popoverOpen()).toBe(true); +}); + +test("a sign-in waits while the tab is on another site", async () => { + const page = await openFixture(harness, "save-login.html"); + const ui = inlineOf(page); + await signIn(page, ui, FAKE_ENTRIES[1].username, "new-pw"); + await page.waitForURL("**/landing.html"); + await expect.poll(() => ui.popoverOpen()).toBe(true); + await page.goto(OTHER_SITE); + await expect.poll(async () => (await lastCall("check-login"))?.pageUrl).toBe(OTHER_SITE); + await page.waitForTimeout(500); + expect(await ui.popoverOpen()).toBe(false); + await page.goto(LANDING); + await expect.poll(() => ui.pickerText()).toContain(`Update ${FAKE_ENTRIES[1].title}`); +}); + +test("a card code is not saved as a login", async () => { + const page = await openFixture(harness, "checkout.html"); + const ui = inlineOf(page); + await ready(ui, 2); + await page.locator("#email").fill("alice@example.com"); + await page.locator("#cvv").fill("123"); + await page.locator("#submit").click(); + await page.waitForURL("**/landing.html"); + await page.waitForTimeout(1500); + expect(await ui.popoverOpen()).toBe(false); + expect(await calls("check-login")).toEqual([]); +}); + +test("a password shown as text at submit still prompts", async () => { + await setMode(harness, "no-fields"); + const page = await openFixture(harness, "save-login.html"); + const ui = inlineOf(page); + await ready(ui, 2); + await page.locator("#username").fill("newuser"); + await page.locator("#password").fill("typed-pw"); + await page.locator("#reveal").click(); + await expect(page.locator("#password")).toHaveAttribute("type", "text"); + await page.locator("#submit").click(); + await page.waitForURL("**/landing.html"); + await expect.poll(() => ui.pickerText()).toBe("Save login for 127.0.0.1?AddnewuserCancel"); +}); + +test("the corner prompt stays in view when the window narrows", async () => { + await setMode(harness, "no-fields"); + const page = await openFixture(harness, "save-login.html"); + const ui = inlineOf(page); + await signIn(page, ui, "newuser", "typed-pw"); + await page.waitForURL("**/landing.html"); + await expect.poll(() => ui.popoverOpen()).toBe(true); + await page.setViewportSize({ width: 600, height: 600 }); + // `.picker` is `box-sizing: border-box`, so left + width is its right edge. + await expect + .poll( + async () => + parseFloat((await ui.style(".picker", "left")) ?? "NaN") + + parseFloat((await ui.style(".picker", "width")) ?? "NaN"), + ) + .toBeLessThanOrEqual(600); + expect(await ui.popoverOpen()).toBe(true); +}); diff --git a/extension/test/support/fake-app.ts b/extension/test/support/fake-app.ts index b3341c5..d490dc8 100644 --- a/extension/test/support/fake-app.ts +++ b/extension/test/support/fake-app.ts @@ -79,6 +79,7 @@ async function readNumber(key: string): Promise { /** The actions the app only serves on an authenticated connection. */ const AUTHED_ACTIONS: Record = { "get-logins": true, + "check-login": true, "get-credential": true, "save-login": true, "generate-password": true, @@ -184,6 +185,27 @@ export function createFakeTransport(): Transport { } return ok({ entries: FAKE_ENTRIES, otherMatches: hostScope ? 2 : 0, domain: FAKE_DOMAIN }); } + case "check-login": { + // Mirrors Rust's `required_str`; the site rule is reduced to the origin, + // which is enough for the fixtures (Rust unit-tests the real rule). + const url = stringParam(params, "url"); + const pageUrl = stringParam(params, "pageUrl"); + if (!url || !pageUrl) return error("bad-request", "Missing url or pageUrl"); + if (new URL(url).origin !== new URL(pageUrl).origin) { + return ok({ state: "other-site", entries: [] }); + } + if (mode === "no-fields") return ok({ state: "new", entries: [] }); + const stored = FAKE_ENTRIES.find( + (entry) => entry.username === stringParam(params, "username"), + ); + if (stored && stringParam(params, "password") === FAKE_PASSWORD) { + return ok({ state: "unchanged", entries: [] }); + } + return ok({ + state: stored ? "changed" : "new", + entries: FAKE_ENTRIES.map(({ id, title, username }) => ({ id, title, username })), + }); + } case "get-credential": { const delay = await readNumber(CREDENTIAL_DELAY_KEY); if (delay > 0) await new Promise((resolve) => setTimeout(resolve, delay)); diff --git a/src-tauri/src/modules/browser/actions.rs b/src-tauri/src/modules/browser/actions.rs index 430f78d..d586fdc 100644 --- a/src-tauri/src/modules/browser/actions.rs +++ b/src-tauri/src/modules/browser/actions.rs @@ -4,6 +4,7 @@ use std::path::Path; use serde_json::{json, Value}; +use url::Url; use crate::modules::browser::host::Host; use crate::modules::browser::matching; @@ -12,7 +13,7 @@ use crate::modules::generator; use crate::modules::prefs; use crate::modules::vault::entry_commands::vault_entry_upsert_inner; use crate::modules::vault::model::{ - DraftCustomField, EntryDraft, EntryUrl, MatchMode, VaultPayload, BROWSER_ID, + DraftCustomField, Entry, EntryDraft, EntryUrl, MatchMode, VaultPayload, BROWSER_ID, }; use crate::modules::vault::query::in_trash; use crate::modules::vault::state::{commit, now_ms, VaultState}; @@ -67,6 +68,21 @@ fn required_str<'a>(params: &'a Value, key: &str) -> Result<&'a str, Failure> { .ok_or_else(|| Failure::code(NmError::BadRequest)) } +/// `None` when no URL of `entry` matches `page`; else whether one of the +/// matching URLs is on the page's exact host (`matching::same_host`). +fn page_match(page: &Url, entry: &Entry) -> Option { + let mut found = None; + for url in &entry.urls { + let Some(entry_url) = matching::parse_entry(&url.url) else { + continue; + }; + if matching::matches(page, &entry_url, url.match_mode.clone()) { + found = Some(found == Some(true) || matching::same_host(page, &entry_url)); + } + } + found +} + /// `get-logins { url, scope }`: the entries whose URL rules match the page, /// plus `domain`, the page's registrable domain (or its host) for the picker /// footer. Never returns a password. @@ -87,22 +103,9 @@ pub(crate) fn get_logins(vault: &VaultState, params: &Value) -> Result Result Result { + let raw_url = required_str(params, "url")?; + let raw_showing = required_str(params, "pageUrl")?; + let username = required_str(params, "username")?; + let password = required_str(params, "password")?; + let page = matching::parse_page(raw_url).ok_or_else(|| Failure::code(NmError::BadRequest))?; + let showing = + matching::parse_page(raw_showing).ok_or_else(|| Failure::code(NmError::BadRequest))?; + if !matching::matches(&showing, &page, MatchMode::Domain) { + return Ok(json!({ "state": "other-site", "entries": [] })); + } + + let guard = vault.access().map_err(|_| locked_failure())?; + let unlocked = guard.as_ref().ok_or_else(locked_failure)?; + let payload = &unlocked.payload; + + let mut unchanged = false; + let mut changed = false; + let mut entries = Vec::new(); + for entry in &payload.entries { + // A Domain-only match is skipped entirely, so a page on another host + // of the domain cannot probe that entry through the prompt's presence. + if in_trash(payload, &entry.group_id) || page_match(&page, entry) != Some(true) { + continue; + } + if entry.username == username && entry.password == password { + unchanged = true; + } else if entry.username == username { + changed = true; + } + entries.push(json!({ "id": entry.id, "title": entry.title, "username": entry.username })); + } + let state = if unchanged { + "unchanged" + } else if changed { + "changed" + } else { + "new" + }; + Ok(json!({ "state": state, "entries": entries })) +} + /// `get-credential { id, url, via }`: release one credential, stamp its use, /// and learn the origin when it was released on a domain-only match. pub(crate) fn get_credential( @@ -787,6 +839,93 @@ mod tests { get_logins(vault, params) } + #[tokio::test] + async fn check_login_states() { + let (vault, dir, _host) = setup("checklogin"); + let page = "https://accounts.example.com/login"; + + let h = vault_entry_upsert_inner( + &vault, + &dir.0, + draft_with_url( + ROOT_ID, + "H", + "https://accounts.example.com", + MatchMode::Host, + ), + ) + .unwrap(); + let mut d = draft_with_url(ROOT_ID, "D", "https://example.com", MatchMode::Domain); + d.username = "dom".into(); + d.password = Some("dom-pw".into()); + vault_entry_upsert_inner(&vault, &dir.0, d).unwrap(); + let mut t = draft_with_url( + ROOT_ID, + "T", + "https://accounts.example.com", + MatchMode::Host, + ); + t.username = "gone".into(); + t.password = Some("gone-pw".into()); + let t = vault_entry_upsert_inner(&vault, &dir.0, t).unwrap(); + vault_entry_trash_inner(&vault, &dir.0, vec![t.id.clone()]).unwrap(); + + let check_on = |url: &str, showing: &str, username: &str, password: &str| { + check_login( + &vault, + &json!({ + "url": url, + "pageUrl": showing, + "username": username, + "password": password, + }), + ) + .unwrap() + }; + let check = + |url: &str, username: &str, password: &str| check_on(url, url, username, password); + let ids = |reply: &serde_json::Value| -> Vec { + reply["entries"] + .as_array() + .unwrap() + .iter() + .map(|e| e["id"].as_str().unwrap().to_string()) + .collect() + }; + + assert_eq!(check(page, "user", "pw-1")["state"], "unchanged"); + + let changed = check(page, "user", "new-pw"); + assert_eq!(changed["state"], "changed"); + assert_eq!(ids(&changed), vec![h.id.clone()]); + // A password never crosses the wire. + assert!(changed["entries"][0].get("password").is_none()); + + // A Domain-only match neither suppresses the prompt nor becomes a candidate. + let domain = check(page, "dom", "dom-pw"); + assert_eq!(domain["state"], "new"); + assert_eq!(ids(&domain), vec![h.id.clone()]); + + // A trashed entry is ignored. + assert_eq!(check(page, "gone", "gone-pw")["state"], "new"); + + let fresh = check("https://fresh.example/", "user", "pw-1"); + assert_eq!(fresh["state"], "new"); + assert!(fresh["entries"].as_array().unwrap().is_empty()); + + // Another host of the same site may show the prompt, with the + // submitted host's entries; another site never sees them. + let sibling = check_on(page, "https://www.example.com/home", "user", "new-pw"); + assert_eq!(sibling["state"], "changed"); + assert_eq!(ids(&sibling), vec![h.id.clone()]); + let elsewhere = check_on(page, "https://other.test/", "user", "new-pw"); + assert_eq!(elsewhere["state"], "other-site"); + assert!(elsewhere["entries"].as_array().unwrap().is_empty()); + + let missing = check_login(&vault, &json!({ "url": page, "username": "user" })); + assert_eq!(missing.unwrap_err().code, NmError::BadRequest); + } + #[tokio::test] async fn get_credential_inline_rule_and_origin_append() { let (vault, dir, host) = setup("cred"); diff --git a/src-tauri/src/modules/browser/protocol.rs b/src-tauri/src/modules/browser/protocol.rs index b7899c1..e474638 100644 --- a/src-tauri/src/modules/browser/protocol.rs +++ b/src-tauri/src/modules/browser/protocol.rs @@ -14,6 +14,7 @@ pub const ACTION_ASSOCIATE: &str = "associate"; pub const ACTION_HELLO: &str = "hello"; pub const ACTION_AUTH: &str = "auth"; pub const ACTION_GET_LOGINS: &str = "get-logins"; +pub const ACTION_CHECK_LOGIN: &str = "check-login"; pub const ACTION_GET_CREDENTIAL: &str = "get-credential"; pub const ACTION_SAVE_LOGIN: &str = "save-login"; pub const ACTION_GENERATE_PASSWORD: &str = "generate-password"; diff --git a/src-tauri/src/modules/browser/server.rs b/src-tauri/src/modules/browser/server.rs index ae76f89..4c300cf 100644 --- a/src-tauri/src/modules/browser/server.rs +++ b/src-tauri/src/modules/browser/server.rs @@ -454,6 +454,9 @@ pub(crate) async fn dispatch( protocol::ACTION_GET_LOGINS if authed => { to_reply(req, actions::get_logins(vault, &req.params)) } + protocol::ACTION_CHECK_LOGIN if authed => { + to_reply(req, actions::check_login(vault, &req.params)) + } protocol::ACTION_GET_CREDENTIAL if authed => { to_reply(req, actions::get_credential(vault, host, dir, &req.params)) }