Merge pull request #29 from keepass-web/issue-1-warn-before-unload

Avoid accidental data loss

Author: bishopb

pages/0x67/page.ts

@@ -1434,11 +1434,13 @@ function openMoveToDialog(
 // it a vault to open and receive the edited vault back, without the app
 // reimplementing any of its own file handling.
 //
-// The protocol is four same-origin postMessage types:
+// The protocol is six same-origin postMessage types:
 //   app  → host : { type: 'kw-ready' }                     app booted, send a vault
 //   host → app  : { type: 'kw-open', filename, bytes }     open this vault (bytes: ArrayBuffer)
 //   app  → host : { type: 'kw-save', filename, bytes }     user saved; please persist (bytes: ArrayBuffer)
 //   host → app  : { type: 'kw-saved', ok, error? }         result of that persist
+//   host → app  : { type: 'kw-close-request' }             host wants to remove this iframe; may I?
+//   app  → host : { type: 'kw-close-ack' }                 yes — nothing unsaved, or the user chose to discard
 //
 // Every inbound message is checked to come from the parent frame at this
 // page's own origin; anything else is ignored. Nothing here runs unless the
@@ -1473,6 +1475,8 @@ function handleHostMessage(event: MessageEvent): void {
     showUnlock();
   } else if (data.type === 'kw-saved') {
     notifyHostSaveResult(data.ok === true, typeof data.error === 'string' ? data.error : undefined);
+  } else if (data.type === 'kw-close-request') {
+    confirmDiscardIfDirty(() => postToHost({ type: 'kw-close-ack' }));
   }
 }
 
@@ -1514,4 +1518,13 @@ if (isEmbedded()) {
   postToHost({ type: 'kw-ready' });
 }
 
+// Closing the tab, reloading, or navigating away with unsaved edits would
+// otherwise discard them with no warning — there's no autosave to fall back
+// on. This is the browser's own native prompt, not a custom dialog.
+window.addEventListener('beforeunload', (e) => {
+  if (!app.dirty) return;
+  e.preventDefault();
+  e.returnValue = true;
+});
+
 showUpload();

pages/cloud-google-drive/bundle-iife.json

@@ -7,6 +7,7 @@
     "buildDriveDownloadUrl",
     "buildDriveUpdateUrl",
     "isReadyMessage",
-    "isSaveMessage"
+    "isSaveMessage",
+    "isCloseAckMessage"
   ]
 }

pages/cloud-google-drive/globals.d.ts

@@ -27,6 +27,7 @@ declare function buildDriveDownloadUrl(apiBase: string, id: string): string;
 declare function buildDriveUpdateUrl(uploadBase: string, id: string): string;
 declare function isReadyMessage(data: unknown): boolean;
 declare function isSaveMessage(data: unknown): data is SaveMessage;
+declare function isCloseAckMessage(data: unknown): boolean;
 
 // --- Google SDKs (loaded at runtime from Google) ---
 // Declared loosely on purpose — these are foreign, remotely-loaded APIs, not

pages/cloud-google-drive/logic.ts

@@ -72,3 +72,14 @@ export function isSaveMessage(data: unknown): data is SaveMessage {
     rec.type === 'kw-save' && typeof rec.filename === 'string' && rec.bytes instanceof ArrayBuffer
   );
 }
+
+/** True if `data` is the embedded app's "safe to remove me now" reply to a
+ * `kw-close-request` — either nothing was unsaved, or the user chose to
+ * discard it. */
+export function isCloseAckMessage(data: unknown): boolean {
+  return (
+    data !== null &&
+    typeof data === 'object' &&
+    (data as Record<string, unknown>).type === 'kw-close-ack'
+  );
+}

pages/cloud-google-drive/page.ts

@@ -76,6 +76,9 @@ let currentFile: DriveFile | null = null;
 let pendingOpen: { filename: string; bytes: ArrayBuffer } | null = null;
 let pickerApiLoaded = false;
 let tokenClient: TokenClient | null = null;
+// Set while waiting for the embedded app to ack a kw-close-request, so
+// handleFrameMessage knows what to run once it's safe to tear the iframe down.
+let pendingClose: (() => void) | null = null;
 // Cached so the GIS script loads at most once, and concurrent callers share it.
 let gisReady: Promise<void> | null = null;
 
@@ -264,17 +267,33 @@ function showHost(file: DriveFile, bytes: ArrayBuffer): void {
   setRoot(cloneTemplate('tpl-host'));
   qs('#host-filename').textContent = file.name;
   qs('[data-action="back-to-drive"]').addEventListener('click', () => {
-    window.removeEventListener('message', handleFrameMessage);
-    currentFile = null;
-    pendingOpen = null;
-    showChooser();
+    requestCloseIframe(() => {
+      window.removeEventListener('message', handleFrameMessage);
+      currentFile = null;
+      pendingOpen = null;
+      showChooser();
+    });
   });
   window.addEventListener('message', handleFrameMessage);
   // Setting src last means the iframe's script (and its kw-ready handshake)
   // can't fire before the listener above is attached.
   qs<HTMLIFrameElement>('#app-frame').src = '0x67.html';
 }
 
+/** Ask the embedded app whether it's safe to remove the iframe — it may have
+ * unsaved edits, in which case it shows its own discard-confirmation dialog
+ * and only acks if the user agrees. `afterClose` runs once that ack arrives
+ * (see handleFrameMessage's isCloseAckMessage branch); if the user cancels,
+ * no ack ever comes and nothing happens, exactly like cancelling the same
+ * dialog standalone. */
+function requestCloseIframe(afterClose: () => void): void {
+  pendingClose = afterClose;
+  must(qs<HTMLIFrameElement>('#app-frame').contentWindow).postMessage(
+    { type: 'kw-close-request' },
+    APP_ORIGIN,
+  );
+}
+
 function handleFrameMessage(event: MessageEvent): void {
   if (event.origin !== APP_ORIGIN) return;
   const iframe = document.getElementById('app-frame') as HTMLIFrameElement | null;
@@ -286,6 +305,10 @@ function handleFrameMessage(event: MessageEvent): void {
     source.postMessage({ type: 'kw-open', filename: open.filename, bytes: open.bytes }, APP_ORIGIN);
   } else if (isSaveMessage(event.data)) {
     void saveToDrive(event.data.bytes, source);
+  } else if (isCloseAckMessage(event.data)) {
+    const afterClose = pendingClose;
+    pendingClose = null;
+    afterClose?.();
   }
 }
 

pages/tests/0x67-host.test.ts

@@ -295,4 +295,35 @@ test('0x67 embedded in a host frame', async (t) => {
       assert.equal(dq<HTMLElement>('[data-role="save-status"]').textContent, 'Save failed.');
     },
   );
+
+  await t.test('kw-close-request acks immediately when nothing is dirty', () => {
+    // The last write-back above failed, but it was a retry of an edit already
+    // saved successfully earlier in this walkthrough — nothing new since then.
+    const before = hostInbox.length;
+    sendFromHost({ type: 'kw-close-request' });
+    assert.equal(hostInbox.length, before + 1);
+    assert.deepEqual(lastHostMessage(), { type: 'kw-close-ack' });
+    assert.equal(dq<HTMLDialogElement>('#dlg-confirm-discard').open, false);
+  });
+
+  await t.test(
+    'kw-close-request with unsaved changes opens the confirm dialog; confirming acks the host',
+    () => {
+      // The walkthrough above left us on the entry-detail screen (commitEdits
+      // returns there); back to the list, where a fresh edit can be made.
+      click(q('[data-action="back"]'));
+      click(q('[data-action="add-entry"]'));
+
+      const before = hostInbox.length;
+      sendFromHost({ type: 'kw-close-request' });
+      assert.equal(hostInbox.length, before, 'no ack until the user decides');
+      const dlg = dq<HTMLDialogElement>('#dlg-confirm-discard');
+      assert.equal(dlg.open, true);
+
+      click(dq('#dlg-confirm-discard [data-action="confirm-discard"]'));
+      assert.equal(dlg.open, false);
+      assert.equal(hostInbox.length, before + 1);
+      assert.deepEqual(lastHostMessage(), { type: 'kw-close-ack' });
+    },
+  );
 });

pages/tests/0x67-page.test.ts

@@ -1482,6 +1482,48 @@ test('closing with unsaved changes prompts to discard, and confirming discards t
   assert.ok(q('#drop-zone'), 'confirming discard returns to the upload screen');
 });
 
+test('beforeunload is only blocked while there are unsaved edits', async () => {
+  const fileInput = q<HTMLInputElement>('#file-input');
+  setFiles(fileInput, [makeFile('beforeunload-test.kdbx', dbBytes)]);
+  dispatch(fileInput, 'change');
+  await waitFor(() => q('#master-password') !== null);
+
+  q<HTMLInputElement>('#master-password').value = PASSWORD;
+  const keyfileInput = q<HTMLInputElement>('#keyfile-input');
+  setFiles(keyfileInput, [makeFile('keyfile.bin', KEYFILE)]);
+  dispatch(keyfileInput, 'change');
+  await waitFor(() => q<HTMLElement>('#keyfile-label').textContent === 'keyfile.bin');
+  dispatch(q('#unlock-form'), 'submit');
+  await waitFor(() => dom.window.document.body.classList.contains('app-mode'));
+
+  const fireBeforeUnload = (): Event => dispatch(dom.window, 'beforeunload');
+
+  assert.equal(
+    fireBeforeUnload().defaultPrevented,
+    false,
+    'nothing unsaved yet, so the tab may close freely',
+  );
+
+  q('[data-action="add-entry"]').dispatchEvent(new dom.window.Event('click', { bubbles: true }));
+
+  assert.equal(fireBeforeUnload().defaultPrevented, true, 'an unsaved edit blocks the unload');
+
+  // Return to the entry list (the close button lives in its header, not the
+  // entry-edit screen add-entry leaves us on — same recovery as the test
+  // above), then discard, leaving the app back on the upload screen as every
+  // other test here expects.
+  q('[data-action="save"]').dispatchEvent(new dom.window.Event('click', { bubbles: true }));
+  dq('#dlg-save [data-action="close"]').dispatchEvent(
+    new dom.window.Event('click', { bubbles: true }),
+  );
+  q('[data-action="back"]').dispatchEvent(new dom.window.Event('click', { bubbles: true }));
+  q('[data-action="close"]').dispatchEvent(new dom.window.Event('click', { bubbles: true }));
+  dq('#dlg-confirm-discard [data-action="confirm-discard"]').dispatchEvent(
+    new dom.window.Event('click', { bubbles: true }),
+  );
+  assert.ok(q('#drop-zone'));
+});
+
 // ============================================================
 // Edge cases: closing genuinely reachable defensive branches
 // ============================================================

pages/tests/cloud-google-drive-logic.test.ts

@@ -11,6 +11,7 @@ import { test } from 'node:test';
 import {
   buildDriveDownloadUrl,
   buildDriveUpdateUrl,
+  isCloseAckMessage,
   isReadyMessage,
   isSaveMessage,
   must,
@@ -51,3 +52,10 @@ test('isSaveMessage requires a filename string and ArrayBuffer bytes', () => {
   assert.equal(isSaveMessage({ type: 'kw-save', filename: 1, bytes: new ArrayBuffer(2) }), false);
   assert.equal(isSaveMessage({ type: 'kw-save', filename: 'a', bytes: 'no' }), false);
 });
+
+test('isCloseAckMessage recognises the close acknowledgement', () => {
+  assert.equal(isCloseAckMessage({ type: 'kw-close-ack' }), true);
+  assert.equal(isCloseAckMessage(null), false);
+  assert.equal(isCloseAckMessage(42), false);
+  assert.equal(isCloseAckMessage({ type: 'nope' }), false);
+});

pages/tests/cloud-google-drive-page.test.ts

@@ -343,16 +343,30 @@ test('Google Drive connector', async (t) => {
     });
   });
 
-  await t.test('a frame message after the iframe is gone is ignored', () => {
-    q<HTMLIFrameElement>('#app-frame').remove();
-    sendMessage({ type: 'kw-ready' }, { source: frameWin });
-    assert.equal(frameInbox.length, 4, 'nothing more posted');
+  await t.test('a stray kw-close-ack with nothing pending is a harmless no-op', () => {
+    const before = frameInbox.length;
+    sendMessage({ type: 'kw-close-ack' }, { source: frameWin });
+    assert.equal(frameInbox.length, before, 'nothing posted back');
+    assert.ok(q('#app-frame'), 'still showing the host screen');
   });
 
-  await t.test('back to Drive returns to the chooser, and sign-out returns to sign-in', () => {
+  await t.test('back to Drive asks the app first, and only leaves once it acks', () => {
     click(q('[data-action="back-to-drive"]'));
-    assert.ok(q('[data-action="pick"]'));
+    assert.ok(q('#app-frame'), 'still on the host screen — waiting for the app to confirm');
+    const req = frameInbox.at(-1)?.message;
+    assert.equal(req?.type, 'kw-close-request');
+
+    sendMessage({ type: 'kw-close-ack' }, { source: frameWin });
+    assert.ok(q('[data-action="pick"]'), 'now back at the chooser');
+  });
+
+  await t.test('a frame message after back to Drive completed is ignored', () => {
+    const before = frameInbox.length;
+    sendMessage({ type: 'kw-ready' }, { source: frameWin });
+    assert.equal(frameInbox.length, before, 'nothing more posted — the iframe is gone');
+  });
 
+  await t.test('sign-out returns to sign-in', () => {
     click(q('[data-action="signout"]'));
     assert.ok(q('[data-action="signin"]'));
   });