diff --git a/app/src/components/mainWindow.ts b/app/src/components/mainWindow.ts index 85219c1..10c6bb6 100644 --- a/app/src/components/mainWindow.ts +++ b/app/src/components/mainWindow.ts @@ -225,7 +225,14 @@ export function createMainWindow( }; const onWillNavigate = (event: Event, urlToGo: string): void => { - if (!linkIsInternal(options.targetUrl, urlToGo, options.internalUrls)) { + if ( + !linkIsInternal( + options.targetUrl, + urlToGo, + options.internalLoginPages, + options.internalUrls, + ) + ) { event.preventDefault(); if (options.blockExternalUrls) { onBlockedExternalUrl(urlToGo); @@ -294,6 +301,7 @@ export function createMainWindow( urlToGo, disposition, options.targetUrl, + options.internalLoginPages, options.internalUrls, preventDefault, shell.openExternal.bind(this), diff --git a/app/src/components/mainWindowHelpers.test.ts b/app/src/components/mainWindowHelpers.test.ts index 667aff6..9d2d18b 100644 --- a/app/src/components/mainWindowHelpers.test.ts +++ b/app/src/components/mainWindowHelpers.test.ts @@ -21,6 +21,7 @@ test('internal urls should not be handled', () => { internalUrl, undefined, originalUrl, + true, undefined, preventDefault, openExternal, @@ -49,6 +50,7 @@ test('external urls should be opened externally', () => { externalUrl, undefined, originalUrl, + true, undefined, preventDefault, openExternal, @@ -78,6 +80,7 @@ test('external urls should be ignored if blockExternal is true', () => { externalUrl, undefined, originalUrl, + true, undefined, preventDefault, openExternal, @@ -106,6 +109,7 @@ test('tab disposition should be ignored if tabs are not enabled', () => { internalUrl, foregroundDisposition, originalUrl, + true, undefined, preventDefault, openExternal, @@ -134,6 +138,7 @@ test('tab disposition should be ignored if url is external', () => { externalUrl, foregroundDisposition, originalUrl, + true, undefined, preventDefault, openExternal, @@ -162,6 +167,7 @@ test('foreground tabs with internal urls should be opened in the foreground', () internalUrl, foregroundDisposition, originalUrl, + true, undefined, preventDefault, openExternal, @@ -191,6 +197,7 @@ test('background tabs with internal urls should be opened in background tabs', ( internalUrl, backgroundDisposition, originalUrl, + true, undefined, preventDefault, openExternal, @@ -220,6 +227,7 @@ test('about:blank urls should be handled', () => { 'about:blank', undefined, originalUrl, + true, undefined, preventDefault, openExternal, diff --git a/app/src/components/mainWindowHelpers.ts b/app/src/components/mainWindowHelpers.ts index 10fff02..448dafc 100644 --- a/app/src/components/mainWindowHelpers.ts +++ b/app/src/components/mainWindowHelpers.ts @@ -2,9 +2,10 @@ import { linkIsInternal } from '../helpers/helpers'; export function onNewWindowHelper( urlToGo: string, - disposition, + disposition: string, targetUrl: string, - internalUrls, + internalLoginPages: boolean, + internalUrls: string | RegExp, preventDefault, openExternal, createAboutBlankWindow, @@ -13,7 +14,7 @@ export function onNewWindowHelper( blockExternal: boolean, onBlockedExternalUrl: (url: string) => void, ): void { - if (!linkIsInternal(targetUrl, urlToGo, internalUrls)) { + if (!linkIsInternal(targetUrl, urlToGo, internalLoginPages, internalUrls)) { preventDefault(); if (blockExternal) { onBlockedExternalUrl(urlToGo); diff --git a/app/src/helpers/helpers.test.ts b/app/src/helpers/helpers.test.ts index 3b86ec7..b12ca32 100644 --- a/app/src/helpers/helpers.test.ts +++ b/app/src/helpers/helpers.test.ts @@ -10,49 +10,111 @@ const externalUrl = 'https://www.wikipedia.org/wiki/Electron'; const wildcardRegex = /.*/; test('the original url should be internal', () => { - expect(linkIsInternal(internalUrl, internalUrl, undefined)).toEqual(true); + expect(linkIsInternal(internalUrl, internalUrl, false, undefined)).toEqual( + true, + ); }); test('sub-paths of the original url should be internal', () => { expect( - linkIsInternal(internalUrl, internalUrl + internalUrlSubPath, undefined), + linkIsInternal( + internalUrl, + internalUrl + internalUrlSubPath, + false, + undefined, + ), ).toEqual(true); }); test("'about:blank' should be internal", () => { - expect(linkIsInternal(internalUrl, 'about:blank', undefined)).toEqual(true); + expect(linkIsInternal(internalUrl, 'about:blank', false, undefined)).toEqual( + true, + ); }); test('urls from different sites should not be internal', () => { - expect(linkIsInternal(internalUrl, externalUrl, undefined)).toEqual(false); + expect(linkIsInternal(internalUrl, externalUrl, false, undefined)).toEqual( + false, + ); }); test('all urls should be internal with wildcard regex', () => { - expect(linkIsInternal(internalUrl, externalUrl, wildcardRegex)).toEqual(true); + expect( + linkIsInternal(internalUrl, externalUrl, false, wildcardRegex), + ).toEqual(true); }); test('a "www." of a domain should be considered internal', () => { - expect(linkIsInternal(internalUrl, internalUrlWww, undefined)).toEqual(true); + expect(linkIsInternal(internalUrl, internalUrlWww, false, undefined)).toEqual( + true, + ); }); test('urls on the same "base domain" should be considered internal', () => { - expect(linkIsInternal(internalUrl, sameBaseDomainUrl, undefined)).toEqual( - true, - ); + expect( + linkIsInternal(internalUrl, sameBaseDomainUrl, false, undefined), + ).toEqual(true); }); test('urls on the same "base domain" should be considered internal, even with a www', () => { - expect(linkIsInternal(internalUrlWww, sameBaseDomainUrl, undefined)).toEqual( - true, - ); + expect( + linkIsInternal(internalUrlWww, sameBaseDomainUrl, false, undefined), + ).toEqual(true); }); test('urls on the same "base domain" should be considered internal, long SLD', () => { expect( - linkIsInternal(internalUrlCoUk, sameBaseDomainUrlCoUk, undefined), + linkIsInternal(internalUrlCoUk, sameBaseDomainUrlCoUk, false, undefined), ).toEqual(true); }); +const testLoginPages = [ + 'https://amazon.co.uk/signin', + 'https://amazon.com/signin', + 'https://amazon.de/signin', + 'https://amazon.com/ap/signin', + 'https://facebook.co.uk/login', + 'https://facebook.com/login', + 'https://facebook.de/login', + 'https://github.co.uk/login', + 'https://github.com/login', + 'https://github.de/login', + 'https://accounts.google.co.uk', + 'https://accounts.google.com', + 'https://accounts.google.de', + 'https://linkedin.co.uk/uas/login', + 'https://linkedin.com/uas/login', + 'https://linkedin.de/uas/login', + 'https://login.live.co.uk', + 'https://login.live.com', + 'https://login.live.de', + 'https://okta.co.uk', + 'https://okta.com', + 'https://subdomain.okta.com', + 'https://okta.de', + 'https://twitter.co.uk/oauth/authenticate', + 'https://twitter.com/oauth/authenticate', + 'https://twitter.de/oauth/authenticate', +]; + +test.each(testLoginPages)( + '%s login page should be internal if internalLoginPages is enabled', + (loginUrl: string) => { + expect(linkIsInternal(internalUrl, loginUrl, true, undefined)).toEqual( + true, + ); + }, +); + +test.each(testLoginPages)( + '%s login page should not be internal if internalLoginPages is disabled', + (loginUrl: string) => { + expect(linkIsInternal(internalUrl, loginUrl, false, undefined)).toEqual( + false, + ); + }, +); + const smallCounterTitle = 'Inbox (11) - nobody@example.com - Gmail'; const largeCounterTitle = 'Inbox (8,756) - nobody@example.com - Gmail'; const noCounterTitle = 'Inbox - nobody@example.com - Gmail'; diff --git a/app/src/helpers/helpers.ts b/app/src/helpers/helpers.ts index 4658f99..62df2ec 100644 --- a/app/src/helpers/helpers.ts +++ b/app/src/helpers/helpers.ts @@ -18,15 +18,35 @@ export function isWindows(): boolean { return os.platform() === 'win32'; } +function isInternalLoginPage(url: string): boolean { + const internalLoginPagesArray = [ + 'amazon\\.[a-zA-Z\\.]*/[a-zA-Z\\/]*signin', // Amazon + `facebook\\.[a-zA-Z\\.]*\\/login`, // Facebook + 'github\\.[a-zA-Z\\.]*\\/login', // GitHub + 'accounts\\.google\\.[a-zA-Z\\.]*', // Google + 'linkedin\\.[a-zA-Z\\.]*/uas/login', // LinkedIn + 'login\\.live\\.[a-zA-Z\\.]*', // Microsoft + 'okta\\.[a-zA-Z\\.]*', // Okta + 'twitter\\.[a-zA-Z\\.]*/oauth/authenticate', // Twitter + ]; + const regex = RegExp(internalLoginPagesArray.join('|')); + return regex.test(url); +} + export function linkIsInternal( currentUrl: string, newUrl: string, + internalLoginPages: boolean, internalUrlRegex: string | RegExp, ): boolean { if (newUrl === 'about:blank') { return true; } + if (internalLoginPages && isInternalLoginPage(newUrl)) { + return true; + } + if (internalUrlRegex) { const regex = RegExp(internalUrlRegex); return regex.test(newUrl); @@ -50,6 +70,7 @@ export function linkIsInternal( currentUrl, 'To:', newUrl, + err, ); return false; } diff --git a/docs/api.md b/docs/api.md index a1449e0..5cf8d0e 100644 --- a/docs/api.md +++ b/docs/api.md @@ -43,6 +43,7 @@ - [[ignore-gpu-blacklist]](#ignore-gpu-blacklist) - [[enable-es3-apis]](#enable-es3-apis) - [[insecure]](#insecure) + - [[internal-login-pages]](#internal-login-pages) - [[internal-urls]](#internal-urls) - [[block-external-urls]](#block-external-urls) - [[proxy-rules]](#proxy-rules) @@ -394,6 +395,30 @@ Passes the enable-es3-apis flag to the Chrome engine, to force the activation of Forces the packaged app to ignore web security errors, such as [Mixed Content](https://developer.mozilla.org/en-US/docs/Security/Mixed_content) errors when receiving HTTP content on a HTTPS site. +#### [internal-login-pages] + +``` +--internal-login-pages +``` + +Whether to consider known login providers to be internal, so as to avoid login urls being opened in a browser without being able to login to the application. Default: true + +Known login providers: +- Amazon +- Facebook +- GitHub +- Google +- LinkedIn +- Microsoft +- Okta +- Twitter + +Example: + +```bash +nativefier https://google.com --internal-login-pages false +``` + #### [internal-urls] ``` @@ -408,13 +433,23 @@ once stripped of `www.`. For example, by default, - URLs from/to `foo.com`, `app.foo.com`, `www.foo.com` are considered internal. - URLs from/to `abc.com` and `xyz.com` are considered external. -Example: +*[Breaking change in Nativefier 43.0.0]* Finally, URLs for known login pages +(e.g. `accounts.google.com` or `login.live.com`) are considered internal. +This does not replace `internal-urls`, it complements it, and happens *before* +your `internal-urls` rule is applied. So, if you already set the flag to let such +auth pages open internally, you don't need to change it but it might be unnecessary. + +We think this is desirable behavior and are so far unaware of cases where users +might not want this. If you disagree, please chime in at +[PR #1124: App: Automatically consider known login pages as internal](https://github.com/nativefier/nativefier/pull/1124) + +Example of `--internal-urls` causing all links to Google to be considered internal: ```bash nativefier https://google.com --internal-urls ".*?\.google\.*?" ``` -Or, if you want to allow all domains for example for external auths, +Or, if you never expect Nativefier to open an "external" page in your OS browser, ```bash nativefier https://google.com --internal-urls ".*?" diff --git a/src/build/prepareElectronApp.ts b/src/build/prepareElectronApp.ts index 7632886..d7193a7 100644 --- a/src/build/prepareElectronApp.ts +++ b/src/build/prepareElectronApp.ts @@ -44,6 +44,7 @@ function pickElectronAppArgs(options: AppOptions): any { ignoreCertificate: options.nativefier.ignoreCertificate, ignoreGpuBlacklist: options.nativefier.ignoreGpuBlacklist, insecure: options.nativefier.insecure, + internalLoginPages: options.nativefier.internalLoginPages, internalUrls: options.nativefier.internalUrls, blockExternalUrls: options.nativefier.blockExternalUrls, maxHeight: options.nativefier.maxHeight, diff --git a/src/cli.ts b/src/cli.ts index 21b6213..513d9f8 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -6,9 +6,14 @@ import * as dns from 'dns'; import * as commander from 'commander'; import * as log from 'loglevel'; -import { isArgFormatInvalid, isWindows } from './helpers/helpers'; +import { isArgFormatInvalid } from './helpers/helpers'; import { supportedArchs, supportedPlatforms } from './infer/inferOs'; import { buildNativefierApp } from './main'; +import { + parseBoolean, + parseBooleanOrString, + parseJson, +} from './utils/parseUtils'; // package.json is `require`d to let tsc strip the `src` folder by determining // baseUrl=src. A static import would prevent that and cause an ugly extra "src" folder @@ -19,37 +24,6 @@ function collect(val: any, memo: any[]): any[] { return memo; } -function parseBooleanOrString(val: string): boolean | string { - switch (val) { - case 'true': - return true; - case 'false': - return false; - default: - return val; - } -} - -function parseJson(val: string): any { - if (!val) return {}; - try { - return JSON.parse(val); - } catch (err) { - const windowsShellHint = isWindows() - ? `\n In particular, Windows cmd doesn't have single quotes, so you have to use only double-quotes plus escaping: "{\\"someKey\\": \\"someValue\\"}"` - : ''; - - log.error( - `Unable to parse JSON value: ${val}\n` + - `JSON should look like {"someString": "someValue", "someBoolean": true, "someArray": [1,2,3]}.\n` + - ` - Only double quotes are allowed, single quotes are not.\n` + - ` - Learn how your shell behaves and escapes characters.${windowsShellHint}\n` + - ` - If unsure, validate your JSON using an online service.`, - ); - throw err; - } -} - function getProcessEnvs(val: string): any { if (!val) { return {}; @@ -238,6 +212,11 @@ if (require.main === module) { 'default zoom factor to use when the app is opened; defaults to 1.0', parseFloat, ) + .option( + '--internal-login-pages', + "Force known login pages (Amazon, Facebook, GitHub, Google, LinkedIn, Microsoft, Okta, Twitter) to be internal urls so they don't have to be individually specified. Default: true", + (value) => parseBoolean(value, true), + ) .option( '--internal-urls ', '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', diff --git a/src/integration-test.ts b/src/integration-test.ts index 25eb164..81200e7 100644 --- a/src/integration-test.ts +++ b/src/integration-test.ts @@ -36,13 +36,21 @@ function checkApp(appRoot: string, inputOptions: any): void { const iconPath = path.join(appPath, iconFile); expect(fs.existsSync(iconPath)).toBe(true); expect(fs.statSync(iconPath).size).toBeGreaterThan(1000); + + // Internal Login Pages enabled by default + expect(nativefierConfig.internalLoginPages).toBe( + inputOptions.internalLoginPages === undefined + ? true + : inputOptions.internalLoginPages, + ); } describe('Nativefier', () => { jest.setTimeout(300000); - test('builds a Nativefier app for several platforms', async () => { - for (const platform of ['darwin', 'linux']) { + test.each(['darwin', 'linux'])( + 'builds a Nativefier app for platform %s', + async (platform) => { const tempDirectory = getTempDir('integtest'); const options = { targetUrl: 'https://google.com/', @@ -52,6 +60,19 @@ describe('Nativefier', () => { }; const appPath = await buildNativefierApp(options); checkApp(appPath, options); - } + }, + ); + + test('can disable internalLoginPages', async () => { + const tempDirectory = getTempDir('integtest'); + const options = { + targetUrl: 'https://google.com/', + out: tempDirectory, + overwrite: true, + platform: 'linux', + internalLoginPages: false, + }; + const appPath = await buildNativefierApp(options); + checkApp(appPath, options); }); }); diff --git a/src/options/model.ts b/src/options/model.ts index 7c29f47..7fd4dd6 100644 --- a/src/options/model.ts +++ b/src/options/model.ts @@ -34,6 +34,7 @@ export interface AppOptions { ignoreGpuBlacklist: boolean; inject: string[]; insecure: boolean; + internalLoginPages: boolean; internalUrls: string; blockExternalUrls: boolean; maximize: boolean; diff --git a/src/options/optionsMain.ts b/src/options/optionsMain.ts index 8942eba..5240e75 100644 --- a/src/options/optionsMain.ts +++ b/src/options/optionsMain.ts @@ -74,6 +74,11 @@ export async function getOptions(rawOptions: any): Promise { ignoreGpuBlacklist: rawOptions.ignoreGpuBlacklist || false, inject: rawOptions.inject || [], insecure: rawOptions.insecure || false, + internalLoginPages: + rawOptions.internalLoginPages === undefined || + typeof rawOptions.internalLoginPages !== 'boolean' + ? true + : rawOptions.internalLoginPages, internalUrls: rawOptions.internalUrls || null, blockExternalUrls: rawOptions.blockExternalUrls || false, maximize: rawOptions.maximize || false, diff --git a/src/utils/parseUtils.test.ts b/src/utils/parseUtils.test.ts new file mode 100644 index 0000000..4f80383 --- /dev/null +++ b/src/utils/parseUtils.test.ts @@ -0,0 +1,21 @@ +import { parseBoolean } from './parseUtils'; + +test.each([ + ['true', true, true], + ['1', true, true], + ['yes', true, true], + [1, true, true], + [true, true, true], + ['false', false, true], + ['0', false, true], + ['no', false, true], + [0, false, true], + [false, false, true], + [undefined, true, true], + [undefined, false, false], +])( + 'parseBoolean("%s") === %s', + (testString: string, expectedResult: boolean, _default: boolean) => { + expect(parseBoolean(testString, _default)).toBe(expectedResult); + }, +); diff --git a/src/utils/parseUtils.ts b/src/utils/parseUtils.ts new file mode 100644 index 0000000..390c9f7 --- /dev/null +++ b/src/utils/parseUtils.ts @@ -0,0 +1,60 @@ +import * as log from 'loglevel'; + +import { isWindows } from '../helpers/helpers'; + +export function parseBoolean( + val: boolean | string | number, + _default: boolean, +): boolean { + try { + if (typeof val === 'boolean') { + return val; + } + val = String(val); + switch (val.toLocaleLowerCase()) { + case 'true': + case '1': + case 'yes': + return true; + case 'false': + case '0': + case 'no': + return false; + default: + return _default; + } + } catch (_) { + return _default; + } +} + +export function parseBooleanOrString(val: string): boolean | string { + switch (val) { + case 'true': + return true; + case 'false': + return false; + default: + return val; + } +} + +export function parseJson(val: string): any { + if (!val) return {}; + try { + return JSON.parse(val); + } catch (err) { + const windowsShellHint = isWindows() + ? `\n In particular, Windows cmd doesn't have single quotes, so you have to use only double-quotes plus escaping: "{\\"someKey\\": \\"someValue\\"}"` + : ''; + + log.error( + `Unable to parse JSON value: ${val}\n` + + `JSON should look like {"someString": "someValue", "someBoolean": true, "someArray": [1,2,3]}.\n` + + ` - Only double quotes are allowed, single quotes are not.\n` + + ` - Learn how your shell behaves and escapes characters.${windowsShellHint}\n` + + ` - If unsure, validate your JSON using an online service.`, + ); + throw err; + } +}