Fortify modals stay open if client vanishes

[Ottunger]

The Fortify PIN modal stays active even if a user leaves the webpage that triggered it.
If the user actually did a reload, he may end up with two PIN modals, only one being linked to the webpage active.

Could the modals be closed if client closes websocket connection?

[Ottunger]

I attach a diff of how we did a stub of this, but it lacks port checking => if a user has two tabs and reloads one only, it kills all modals.

diff --git a/scripts/postinstall.ts b/scripts/postinstall.ts
index e585dbb5486d0cb755c51ddaa63c9ca203bf373e..6e010ba3f965984219f837345768edaf6ece9421 100644
--- a/scripts/postinstall.ts
+++ b/scripts/postinstall.ts
@@ -91,6 +91,7 @@ async function win32() {
                         this.emit("error", {
                             code: -100,
                             type: "ok",
+                            origin: session.origin,
                             resolve,
                             reject,
                         });
@@ -99,7 +100,16 @@ async function win32() {
                     crypto.session.lib.initial_authentication_last(can);
                     keys = await crypto.keyStorage.keys();
                 }
-                // ---- CAN`);
+                // ---- CAN`).replace('.on("disconnect", (e) => {', `// line // .on("disconnect", (e) => {
+            // ---- TRANSFER DISCONNECT
+            .on("disconnect", (e) => {
+            // @ts-ignore
+            this.emit("error", {
+                code: -101,
+                origin: e.remoteAddress,
+                type: "ok"
+            });
+            // ---- TRANSFER DISCONNECT`);
   fs.writeFileSync(__dirname + '/../node_modules/@webcrypto-local/server/build/index.es.js', result);
 
   if (!fs.existsSync(pvpkcs11File)) {
diff --git a/src/main/server.ts b/src/main/server.ts
index b21d3efb46af65c547c931b4869f835144d922d4..26b0176fd74a866a098648b0e07966f3963ff412 100644
--- a/src/main/server.ts
+++ b/src/main/server.ts
@@ -13,8 +13,6 @@ import * as constants from './constants';
 import logger from './logger';
 import { windowsController } from './windows';
 import { l10n } from './l10n';
-import * as jws from './jws';
-import { request } from './utils';
 import { getConfig } from './config';
 import './crypto';
 
@@ -176,7 +174,7 @@ export class Server {
               const p11CanWindowResult = await windowsController.showP11CanWindow({
                 ...error as any,
                 can: '',
-              });
+              }, (error as any).origin);
   
               if (p11CanWindowResult.can) {
                 (error as any).resolve(p11CanWindowResult.can);
@@ -184,6 +182,10 @@ export class Server {
                 (error as any).reject(new wsServer.WebCryptoLocalError(10001, 'Incorrect CAN value. It cannot be empty.'));
               }
               break;
+            case -101:
+              logger.info('server', 'Closing open disposable windows', {origin: (error as any).origin});
+              windowsController.destroyDisposableWindows((error as any).origin);
+              break;
             case CODE.PCSC_CANNOT_START:
               windowsController.showWarningWindow(
                 {
@@ -232,7 +234,7 @@ export class Server {
             const keyPinWindowResult = await windowsController.showKeyPinWindow({
               ...params,
               accept: false,
-            });
+            }, params.origin);
 
             params.resolve(keyPinWindowResult.accept);
 
@@ -243,7 +245,7 @@ export class Server {
             const p11PinWindowResult = await windowsController.showP11PinWindow({
               ...params,
               pin: '',
-            });
+            }, params.origin);
 
             if (p11PinWindowResult.pin) {
               params.resolve(p11PinWindowResult.pin);
diff --git a/src/main/tray/development_template.ts b/src/main/tray/development_template.ts
index a34e0704f15ef22427683b49a48330629ce88505..3939e2ae1e3f6ff4dee934c8cfca0ca417c61e92 100644
--- a/src/main/tray/development_template.ts
+++ b/src/main/tray/development_template.ts
@@ -114,7 +114,7 @@ export const developmentTemplate = (): MenuItemConstructorOptions[] => ([
               pin: '123456',
               origin: 'https://TEST.com/',
               accept: true,
-            },
+            }, 'test'
           );
         },
       },
@@ -125,7 +125,7 @@ export const developmentTemplate = (): MenuItemConstructorOptions[] => ([
             {
               pin: '',
               origin: 'https://TEST.com/',
-            },
+            }, 'test'
           );
         },
       },
diff --git a/src/main/windows/browser_window.ts b/src/main/windows/browser_window.ts
index 86ad843e422a51a7aedbfff8009c2c46bbbbb758..f71f0745d9cef867382a61b2c8e123726992fb04 100644
--- a/src/main/windows/browser_window.ts
+++ b/src/main/windows/browser_window.ts
@@ -145,7 +145,11 @@ export class BrowserWindow {
   }
 
   public getParams() {
-    return this.window.params || {};
+    try {
+      return this.window.params || {};
+    } catch {
+      return {};
+    } 
   }
 
   public focus() {
diff --git a/src/main/windows/windows_controller.ts b/src/main/windows/windows_controller.ts
index f77e44f46610c6671c87345fd4487f4dcd79f162..53208be77ed7b981974ad4a5e14589baada3ce93 100644
--- a/src/main/windows/windows_controller.ts
+++ b/src/main/windows/windows_controller.ts
@@ -51,6 +51,7 @@ interface IWarningWindowParams {
 
 class WindowsController {
   windows: Assoc<BrowserWindow> = {};
+  private disposableWindowsBySocketId: Assoc<BrowserWindow[]> = {};
 
   static getScreenSize() {
     return screen.getPrimaryDisplay().bounds;
@@ -94,8 +95,14 @@ class WindowsController {
     });
   }
 
+  destroyDisposableWindows(socketId: string) {
+    const arr = [...this.disposableWindowsBySocketId[socketId] || []];
+    this.disposableWindowsBySocketId[socketId] = [];
+    arr.forEach(w => w.window.destroy());
+  }
+
   // eslint-disable-next-line class-methods-use-this
-  showP11PinWindow(params: IP11PinWindowParams): Promise<IP11PinWindowParams> {
+  showP11PinWindow(params: IP11PinWindowParams, socketId: string): Promise<IP11PinWindowParams> {
     return new Promise((resolve) => {
       const browserWindow = new BrowserWindow({
         params: {
@@ -108,14 +115,23 @@ class WindowsController {
         },
         title: params.label || l10n.get('p11-pin'),
         onClosed: () => {
+          this.disposableWindowsBySocketId[socketId] = this.disposableWindowsBySocketId[socketId].filter(w => {
+            try {
+              return w.window.id !== browserWindow.window.id;
+            } catch {
+              return false;
+            }
+          });
           resolve(browserWindow.getParams() as IP11PinWindowParams);
         },
       });
+      this.disposableWindowsBySocketId[socketId] = this.disposableWindowsBySocketId[socketId] || [];
+      this.disposableWindowsBySocketId[socketId].push(browserWindow);
     });
   }
 
   // eslint-disable-next-line class-methods-use-this
-  showP11CanWindow(params: IP11CanWindowParams): Promise<IP11CanWindowParams> {
+  showP11CanWindow(params: IP11CanWindowParams, socketId: string): Promise<IP11CanWindowParams> {
     return new Promise((resolve) => {
       const browserWindow = new BrowserWindow({
         params: {
@@ -128,14 +144,23 @@ class WindowsController {
         },
         title: params.label || l10n.get('p11-can'),
         onClosed: () => {
+          this.disposableWindowsBySocketId[socketId] = this.disposableWindowsBySocketId[socketId].filter(w => {
+            try {
+              return w.window.id !== browserWindow.window.id;
+            } catch {
+              return false;
+            }
+          });
           resolve(browserWindow.getParams() as IP11CanWindowParams);
         },
       });
+      this.disposableWindowsBySocketId[socketId] = this.disposableWindowsBySocketId[socketId] || [];
+      this.disposableWindowsBySocketId[socketId].push(browserWindow);
     });
   }
 
   // eslint-disable-next-line class-methods-use-this
-  showKeyPinWindow(params: IKeyPinWindowParams): Promise<IKeyPinWindowParams> {
+  showKeyPinWindow(params: IKeyPinWindowParams, socketId: string): Promise<IKeyPinWindowParams> {
     return new Promise((resolve) => {
       const { width, height } = WindowsController.getScreenSize();
 
@@ -154,9 +179,18 @@ class WindowsController {
           y: height - windowSizes.default.height,
         },
         onClosed: () => {
+          this.disposableWindowsBySocketId[socketId] = this.disposableWindowsBySocketId[socketId].filter(w => {
+            try {
+              return w.window.id !== browserWindow.window.id;
+            } catch {
+              return false;
+            }
+          });
           resolve(browserWindow.getParams() as IKeyPinWindowParams);
         },
       });
+      this.disposableWindowsBySocketId[socketId] = this.disposableWindowsBySocketId[socketId] || [];
+      this.disposableWindowsBySocketId[socketId].push(browserWindow);
     });
   }

[Ottunger]

Probably this code to change?

function getOrigin(request) {
    let origin = request.headers.origin;
    if (!origin || !/^https:\/\//.test(origin)) {
        origin = request.connection.remoteAddress || "UNKNOWN";
    }
    return origin;
}

[Ottunger]

FIY, changing getOrigin is not the way to go it kills the 2key ratchet SSL check.
We kept the port, request.connetion.remotePort in the session next to origin and check for matching pair.

Have a good one,

[rmhrisk]

Any chance to see the PRs for the changes you have been making?

[Ottunger]

Sure, I'll try to ASAP

[Ottunger]

First, needs PeculiarVentures/webcrypto-local#269 and publish result to npm.
Then #487

[donskov]

@microshine Please publish the new version of @webcrypto-local/server.

[donskov]

@Ottunger Please try https://github.com/PeculiarVentures/fortify/releases/tag/1.8.4.