Add --block-external-urls flag to forbid external navigation attemp…

…ts (Fix nativefier#978 - PR#1012) Fixes nativefier#978 Adds a `--block-external-urls` option (default: `false`) that prevents opening external links (as classified by the `--internal-urls` option). Documentation and tests updated. Example: ``` nativefier --internal-urls "classroom\.google\.com" --block-external-urls ``` ![image](https://user-images.githubusercontent.com/12286274/88739501-f12d5180-d0f7-11ea-9821-86f3e9bfa070.png) ![image](https://user-images.githubusercontent.com/12286274/88739512-fab6b980-d0f7-11ea-877c-7bd565352a93.png)
promagen · Aug 2, 2020 · 8e8cd24 · 8e8cd24
1 parent 5d9a7ae
commit 8e8cd24
Show file tree

Hide file tree

Showing 8 changed files with 120 additions and 2 deletions.
diff --git a/app/src/components/mainWindow.ts b/app/src/components/mainWindow.ts
@@ -210,10 +210,23 @@ export function createMainWindow(
   const getCurrentUrl = (): void =>
     withFocusedWindow((focusedWindow) => focusedWindow.webContents.getURL());
 
+  const onBlockedExternalUrl = (url: string) => {
+    // eslint-disable-next-line @typescript-eslint/no-floating-promises
+    dialog.showMessageBox(mainWindow, {
+      message: `Cannot navigate to external URL: ${url}`,
+      type: 'error',
+      title: 'Navigation blocked',
+    });
+  };
+
   const onWillNavigate = (event: Event, urlToGo: string): void => {
     if (!linkIsInternal(options.targetUrl, urlToGo, options.internalUrls)) {
       event.preventDefault();
-      shell.openExternal(urlToGo); // eslint-disable-line @typescript-eslint/no-floating-promises
+      if (options.blockExternalUrls) {
+        onBlockedExternalUrl(urlToGo);
+      } else {
+        shell.openExternal(urlToGo); // eslint-disable-line @typescript-eslint/no-floating-promises
+      }
     }
   };
 
@@ -282,6 +295,8 @@ export function createMainWindow(
       createAboutBlankWindow,
       nativeTabsSupported,
       createNewTab,
+      options.blockExternalUrls,
+      onBlockedExternalUrl,
     );
   };
 

diff --git a/app/src/components/mainWindowHelpers.test.ts b/app/src/components/mainWindowHelpers.test.ts
@@ -5,6 +5,7 @@ const internalUrl = 'https://medium.com/topics/technology';
 const externalUrl = 'https://www.wikipedia.org/wiki/Electron';
 const foregroundDisposition = 'foreground-tab';
 const backgroundDisposition = 'background-tab';
+const blockExternal = false;
 
 const nativeTabsSupported = () => true;
 const nativeTabsNotSupported = () => false;
@@ -14,6 +15,8 @@ test('internal urls should not be handled', () => {
   const openExternal = jest.fn();
   const createAboutBlankWindow = jest.fn();
   const createNewTab = jest.fn();
+  const onBlockedExternalUrl = jest.fn();
+
   onNewWindowHelper(
     internalUrl,
     undefined,
@@ -24,18 +27,24 @@ test('internal urls should not be handled', () => {
     createAboutBlankWindow,
     nativeTabsNotSupported,
     createNewTab,
+    blockExternal,
+    onBlockedExternalUrl,
   );
+
   expect(openExternal.mock.calls.length).toBe(0);
   expect(createAboutBlankWindow.mock.calls.length).toBe(0);
   expect(createNewTab.mock.calls.length).toBe(0);
   expect(preventDefault.mock.calls.length).toBe(0);
+  expect(onBlockedExternalUrl.mock.calls.length).toBe(0);
 });
 
 test('external urls should be opened externally', () => {
   const openExternal = jest.fn();
   const createAboutBlankWindow = jest.fn();
   const createNewTab = jest.fn();
   const preventDefault = jest.fn();
+  const onBlockedExternalUrl = jest.fn();
+
   onNewWindowHelper(
     externalUrl,
     undefined,
@@ -46,18 +55,53 @@ test('external urls should be opened externally', () => {
     createAboutBlankWindow,
     nativeTabsNotSupported,
     createNewTab,
+    blockExternal,
+    onBlockedExternalUrl,
   );
+
   expect(openExternal.mock.calls.length).toBe(1);
   expect(createAboutBlankWindow.mock.calls.length).toBe(0);
   expect(createNewTab.mock.calls.length).toBe(0);
   expect(preventDefault.mock.calls.length).toBe(1);
+  expect(onBlockedExternalUrl.mock.calls.length).toBe(0);
+});
+
+test('external urls should be ignored if blockExternal is true', () => {
+  const openExternal = jest.fn();
+  const createAboutBlankWindow = jest.fn();
+  const createNewTab = jest.fn();
+  const preventDefault = jest.fn();
+  const onBlockedExternalUrl = jest.fn();
+  const blockExternal = true;
+
+  onNewWindowHelper(
+    externalUrl,
+    undefined,
+    originalUrl,
+    undefined,
+    preventDefault,
+    openExternal,
+    createAboutBlankWindow,
+    nativeTabsNotSupported,
+    createNewTab,
+    blockExternal,
+    onBlockedExternalUrl,
+  );
+
+  expect(openExternal.mock.calls.length).toBe(0);
+  expect(createAboutBlankWindow.mock.calls.length).toBe(0);
+  expect(createNewTab.mock.calls.length).toBe(0);
+  expect(preventDefault.mock.calls.length).toBe(1);
+  expect(onBlockedExternalUrl.mock.calls.length).toBe(1);
 });
 
 test('tab disposition should be ignored if tabs are not enabled', () => {
   const preventDefault = jest.fn();
   const openExternal = jest.fn();
   const createAboutBlankWindow = jest.fn();
   const createNewTab = jest.fn();
+  const onBlockedExternalUrl = jest.fn();
+
   onNewWindowHelper(
     internalUrl,
     foregroundDisposition,
@@ -68,18 +112,24 @@ test('tab disposition should be ignored if tabs are not enabled', () => {
     createAboutBlankWindow,
     nativeTabsNotSupported,
     createNewTab,
+    blockExternal,
+    onBlockedExternalUrl,
   );
+
   expect(openExternal.mock.calls.length).toBe(0);
   expect(createAboutBlankWindow.mock.calls.length).toBe(0);
   expect(createNewTab.mock.calls.length).toBe(0);
   expect(preventDefault.mock.calls.length).toBe(0);
+  expect(onBlockedExternalUrl.mock.calls.length).toBe(0);
 });
 
 test('tab disposition should be ignored if url is external', () => {
   const openExternal = jest.fn();
   const createAboutBlankWindow = jest.fn();
   const createNewTab = jest.fn();
   const preventDefault = jest.fn();
+  const onBlockedExternalUrl = jest.fn();
+
   onNewWindowHelper(
     externalUrl,
     foregroundDisposition,
@@ -90,18 +140,24 @@ test('tab disposition should be ignored if url is external', () => {
     createAboutBlankWindow,
     nativeTabsSupported,
     createNewTab,
+    blockExternal,
+    onBlockedExternalUrl,
   );
+
   expect(openExternal.mock.calls.length).toBe(1);
   expect(createAboutBlankWindow.mock.calls.length).toBe(0);
   expect(createNewTab.mock.calls.length).toBe(0);
   expect(preventDefault.mock.calls.length).toBe(1);
+  expect(onBlockedExternalUrl.mock.calls.length).toBe(0);
 });
 
 test('foreground tabs with internal urls should be opened in the foreground', () => {
   const openExternal = jest.fn();
   const createAboutBlankWindow = jest.fn();
   const createNewTab = jest.fn();
   const preventDefault = jest.fn();
+  const onBlockedExternalUrl = jest.fn();
+
   onNewWindowHelper(
     internalUrl,
     foregroundDisposition,
@@ -112,19 +168,25 @@ test('foreground tabs with internal urls should be opened in the foreground', ()
     createAboutBlankWindow,
     nativeTabsSupported,
     createNewTab,
+    blockExternal,
+    onBlockedExternalUrl,
   );
+
   expect(openExternal.mock.calls.length).toBe(0);
   expect(createAboutBlankWindow.mock.calls.length).toBe(0);
   expect(createNewTab.mock.calls.length).toBe(1);
   expect(createNewTab.mock.calls[0][1]).toBe(true);
   expect(preventDefault.mock.calls.length).toBe(1);
+  expect(onBlockedExternalUrl.mock.calls.length).toBe(0);
 });
 
 test('background tabs with internal urls should be opened in background tabs', () => {
   const openExternal = jest.fn();
   const createAboutBlankWindow = jest.fn();
   const createNewTab = jest.fn();
   const preventDefault = jest.fn();
+  const onBlockedExternalUrl = jest.fn();
+
   onNewWindowHelper(
     internalUrl,
     backgroundDisposition,
@@ -135,19 +197,25 @@ test('background tabs with internal urls should be opened in background tabs', (
     createAboutBlankWindow,
     nativeTabsSupported,
     createNewTab,
+    blockExternal,
+    onBlockedExternalUrl,
   );
+
   expect(openExternal.mock.calls.length).toBe(0);
   expect(createAboutBlankWindow.mock.calls.length).toBe(0);
   expect(createNewTab.mock.calls.length).toBe(1);
   expect(createNewTab.mock.calls[0][1]).toBe(false);
   expect(preventDefault.mock.calls.length).toBe(1);
+  expect(onBlockedExternalUrl.mock.calls.length).toBe(0);
 });
 
 test('about:blank urls should be handled', () => {
   const preventDefault = jest.fn();
   const openExternal = jest.fn();
   const createAboutBlankWindow = jest.fn();
   const createNewTab = jest.fn();
+  const onBlockedExternalUrl = jest.fn();
+
   onNewWindowHelper(
     'about:blank',
     undefined,
@@ -158,9 +226,13 @@ test('about:blank urls should be handled', () => {
     createAboutBlankWindow,
     nativeTabsNotSupported,
     createNewTab,
+    blockExternal,
+    onBlockedExternalUrl,
   );
+
   expect(openExternal.mock.calls.length).toBe(0);
   expect(createAboutBlankWindow.mock.calls.length).toBe(1);
   expect(createNewTab.mock.calls.length).toBe(0);
   expect(preventDefault.mock.calls.length).toBe(1);
+  expect(onBlockedExternalUrl.mock.calls.length).toBe(0);
 });
diff --git a/app/src/components/mainWindowHelpers.ts b/app/src/components/mainWindowHelpers.ts
@@ -10,10 +10,16 @@ export function onNewWindowHelper(
   createAboutBlankWindow,
   nativeTabsSupported,
   createNewTab,
+  blockExternal: boolean,
+  onBlockedExternalUrl: (url: string) => void,
 ): void {
   if (!linkIsInternal(targetUrl, urlToGo, internalUrls)) {
-    openExternal(urlToGo);
     preventDefault();
+    if (blockExternal) {
+      onBlockedExternalUrl(urlToGo);
+    } else {
+      openExternal(urlToGo);
+    }
   } else if (urlToGo === 'about:blank') {
     const newWindow = createAboutBlankWindow();
     preventDefault(newWindow);

diff --git a/docs/api.md b/docs/api.md
@@ -43,6 +43,7 @@
     - [[enable-es3-apis]](#enable-es3-apis)
     - [[insecure]](#insecure)
     - [[internal-urls]](#internal-urls)
+    - [[block-external-urls]](#block-external-urls)
     - [[proxy-rules]](#proxy-rules)
     - [[flash]](#flash)
     - [[flash-path]](#flash-path)
@@ -379,6 +380,21 @@ Or, if you want to allow all domains for example for external auths,
 nativefier https://google.com --internal-urls ".*?"
 ```
 
+#### [block-external-urls]
+
+```
+--block-external-urls
+```
+
+Forbid navigation to URLs not considered "internal" (see '--internal-urls'). Instead of opening in an external browser, attempts to navigate to external URLs will be blocked, and an error message will be shown. Default: false
+
+Example:
+
+```bash
+nativefier https://google.com --internal-urls ".*?\.google\.*?" --block-external-urls
+```
+
+Blocks navigation to any URLs except Google and its subdomains.
 
 #### [proxy-rules]
 
@@ -785,6 +801,8 @@ var options = {
     ignoreCertificate: false,
     ignoreGpuBlacklist: false,
     enableEs3Apis: false,
+    internalUrls: '.*?', // defaults to URLs on same second-level domain as app
+    blockExternalUrls: false,
     insecure: false,
     honest: false,
     zoom: 1.0,

diff --git a/src/build/prepareElectronApp.ts b/src/build/prepareElectronApp.ts
@@ -45,6 +45,7 @@ function pickElectronAppArgs(options: AppOptions): any {
     ignoreGpuBlacklist: options.nativefier.ignoreGpuBlacklist,
     insecure: options.nativefier.insecure,
     internalUrls: options.nativefier.internalUrls,
+    blockExternalUrls: options.nativefier.blockExternalUrls,
     maxHeight: options.nativefier.maxHeight,
     maximize: options.nativefier.maximize,
     maxWidth: options.nativefier.maxWidth,

diff --git a/src/cli.ts b/src/cli.ts
@@ -224,6 +224,10 @@ if (require.main === module) {
       '--internal-urls <value>',
       'regex of URLs to consider "internal"; all other URLs will be opened in an external browser. Default: URLs on same second-level domain as app',
     )
+    .option(
+      '--block-external-urls',
+      `forbid navigation to URLs not considered "internal" (see '--internal-urls').  Instead of opening in an external browser, attempts to navigate to external URLs will be blocked. Default: false`,
+    )
     .option(
       '--proxy-rules <value>',
       'proxy rules; see https://www.electronjs.org/docs/api/session#sessetproxyconfig',

diff --git a/src/options/model.ts b/src/options/model.ts
@@ -33,6 +33,7 @@ export interface AppOptions {
     inject: string[];
     insecure: boolean;
     internalUrls: string;
+    blockExternalUrls: boolean;
     maximize: boolean;
     nativefierVersion: string;
     processEnvs: string;

diff --git a/src/options/optionsMain.ts b/src/options/optionsMain.ts
@@ -71,6 +71,7 @@ export async function getOptions(rawOptions: any): Promise<AppOptions> {
       inject: rawOptions.inject || [],
       insecure: rawOptions.insecure || false,
       internalUrls: rawOptions.internalUrls || null,
+      blockExternalUrls: rawOptions.blockExternalUrls || false,
       maximize: rawOptions.maximize || false,
       nativefierVersion: packageJson.version,
       processEnvs: rawOptions.processEnvs,