Skip to content

Commit

Permalink
MM-14446: consider subpath when evaluating if url is internal (#946)
Browse files Browse the repository at this point in the history 
* MM-14446: consider subpath when evaluating if url is internal

When clicking on an URL with `target=_blank`, the webview decides if it should launch an external browser or a new window within the Electron application. Update this logic to consider the application's configured subpath so as to treat links outside the subpath but on the same domain as external.

* fix licensing on new file

* fix .eslintrc.json indentation

* tweak header eslint rules for specific files
  • Loading branch information
@lieut-data @wget
lieut-data authored and wget committed Mar 15, 2019
1 parent 6e2b3d7 commit 79e020b
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 30 deletions.
138 changes: 111 additions & 27 deletions .eslintrc.json
View file
Original file line number Diff line number Diff line change
@@ -1,31 +1,115 @@
{
"extends": [
"./.eslintrc-webapp.json",
"plugin:eslint-comments/recommended"
],
"parserOptions": {
"ecmaVersion": 2017
},
"settings": {
"import/resolver": "node"
},
"rules": {
"eslint-comments/no-unused-disable": "error",
"extends": [
"./.eslintrc-webapp.json",
"plugin:eslint-comments/recommended"
],
"parserOptions": {
"ecmaVersion": 2017
},
"settings": {
"import/resolver": "node"
},
"rules": {
"header/header": [2, "line", [
" Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.",
" See LICENSE.txt for license information."
]],
"import/no-commonjs": 2,
"indent": [2, 2, {"SwitchCase": 0}],
"no-console": 0,
"no-process-env": 0,
"no-underscore-dangle": 1,
"no-var": 2,
"react/jsx-indent": [2, 2],
"react/jsx-indent-props": [2, 2],
"react/no-find-dom-node": 2,
"react/no-set-state": 1,
"react/require-optimization": 0
},
"overrides": [
{
"files": [
"webpack.config.renderer.js",
"test/specs/settings_test.js",
"test/specs/spellchecker_test.js",
"test/specs/app_test.js",
"test/specs/security_test.js",
"test/specs/permisson_test.js",
"test/specs/browser/index_test.js",
"test/specs/browser/settings_test.js",
"test/modules/utils.js",
"test/modules/environment.js",
"webpack.config.main.js",
"CHANGELOG.md",
"webpack.config.base.js",
"babel.config.js",
"README.md",
"scripts/watch_main_and_preload.js",
"scripts/extract_dict.js",
"scripts/manipulate_windows_zip.js",
"scripts/check_build_config.js",
"LICENSE.txt",
"src/utils/util.js",
"src/main.js",
"src/browser/config/AppConfig.js",
"src/browser/js/contextMenu.js",
"src/browser/updater.jsx",
"src/browser/js/notification.js",
"src/browser/js/badge.js",
"src/browser/webview/mattermost.js",
"src/browser/components/RemoveServerModal.jsx",
"src/browser/components/MainPage.jsx",
"src/browser/components/HoveringURL.jsx",
"src/browser/components/AutoSaveIndicator.jsx",
"src/browser/components/MattermostView.jsx",
"src/browser/components/TabBar.jsx",
"src/browser/components/DestructiveConfirmModal.jsx",
"src/browser/components/ErrorView.jsx",
"src/browser/components/UpdaterPage.jsx",
"src/browser/components/PermissionRequestDialog.jsx",
"src/browser/components/Finder.jsx",
"src/browser/components/SettingsPage.jsx",
"src/browser/components/TeamListItem.jsx",
"src/browser/components/UpdaterPage/UpdaterPage.stories.jsx",
"src/browser/components/Button/Button.stories.jsx",
"src/browser/components/TeamList.jsx",
"src/browser/components/LoginModal.jsx",
"src/browser/components/NewTeamModal.jsx",
"src/browser/settings.jsx",
"src/browser/index.jsx",
"src/common/deepmerge.js",
"src/common/config/buildConfig.js",
"src/common/config/pastDefaultPreferences.js",
"src/common/config/upgradePreferences.js",
"src/common/osVersion.js",
"src/common/config/defaultPreferences.js",
"src/common/JsonFileManager.js",
"src/common/settings.js",
"src/main/certificateStore.js",
"src/main/mainWindow.js",
"src/main/allowProtocolDialog.js",
"src/main/permissionRequestHandler.js",
"src/main/squirrelStartup.js",
"src/main/autoLaunch.js",
"src/main/PermissionManager.js",
"src/main/AutoLauncher.js",
"src/main/AppStateManager.js",
"src/main/menus/tray.js",
"src/main/CriticalErrorHandler.js",
"src/main/cookieManager.js",
"src/main/utils.js",
"src/main/downloadURL.js",
"src/main/autoUpdater.js",
"src/main/SpellChecker.js",
"src/main/menus/app.js"
],
"rules": {
"header/header": [2, "line", [
" Copyright (c) 2015-2016 Yuya Ochiai",
" Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.",
" See LICENSE.txt for license information."
]],
"import/no-commonjs": 2,
"indent": [2, 2, {"SwitchCase": 0}],
"no-console": 0,
"no-process-env": 0,
"no-underscore-dangle": 1,
"no-var": 2,
"react/jsx-indent": [2, 2],
"react/jsx-indent-props": [2, 2],
"react/no-find-dom-node": 2,
"react/no-set-state": 1,
"react/require-optimization": 0
" Copyright (c) 2015-2016 Yuya Ochiai",
" Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.",
" See LICENSE.txt for license information."
]]
}
}
]
}
5 changes: 4 additions & 1 deletion src/browser/components/MattermostView.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import PropTypes from 'prop-types';
import {ipcRenderer, remote, shell} from 'electron';

import contextMenu from '../js/contextMenu';
import Utils from '../../utils/util';
import {protocols} from '../../../electron-builder.json';
const scheme = protocols[0].schemes[0];

Expand All @@ -31,6 +32,7 @@ export default class MattermostView extends React.Component {
isContextMenuAdded: false,
reloadTimeoutID: null,
isLoaded: false,
basename: '/',
};

this.handleUnreadCountChange = this.handleUnreadCountChange.bind(this);
Expand Down Expand Up @@ -94,7 +96,7 @@ export default class MattermostView extends React.Component {
return;
}

if (currentURL.host === destURL.host) {
if (Utils.isInternalURL(destURL, currentURL, this.state.basename)) {
if (destURL.path.match(/^\/api\/v[3-4]\/public\/files\//)) {
ipcRenderer.send('download-url', e.url);
} else {
Expand Down Expand Up @@ -137,6 +139,7 @@ export default class MattermostView extends React.Component {
case 'onGuestInitialized':
self.setState({
isLoaded: true,
basename: event.args[0] || '/',
});
break;
case 'onBadgeChange': {
Expand Down
2 changes: 1 addition & 1 deletion src/browser/webview/mattermost.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ window.addEventListener('load', () => {
return;
}
watchReactAppUntilInitialized(() => {
ipcRenderer.sendToHost('onGuestInitialized');
ipcRenderer.sendToHost('onGuestInitialized', window.basename);
});
});

Expand Down
20 changes: 19 additions & 1 deletion src/utils/util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,22 @@ function getDomain(inputURL) {
return `${parsedURL.protocol}//${parsedURL.host}`;
}

export default {getDomain};
// isInternalURL determines if the target url is internal to the application.
// - currentURL is the current url inside the webview
// - basename is the global export from the Mattermost application defining the subpath, if any
function isInternalURL(targetURL, currentURL, basename = '/') {
if (targetURL.host !== currentURL.host) {
return false;
}

if (!(targetURL.pathname || '/').startsWith(basename)) {
return false;
}

return true;
}

export default {
getDomain,
isInternalURL,
};
47 changes: 47 additions & 0 deletions test/specs/utils/util_test.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
'use strict';

import url from 'url';
import assert from 'assert';

import Utils from '../../../src/utils/util';

describe('Utils', () => {
describe('isInternalURL', () => {
it('should be false for different hosts', () => {
const currentURL = url.parse('http://localhost/team/channel1');
const targetURL = url.parse('http://example.com/team/channel2');
const basename = '/';
assert.equal(Utils.isInternalURL(targetURL, currentURL, basename), false);
});

it('should be false for same hosts, non-matching basename', () => {
const currentURL = url.parse('http://localhost/subpath/team/channel1');
const targetURL = url.parse('http://localhost/team/channel2');
const basename = '/subpath';
assert.equal(Utils.isInternalURL(targetURL, currentURL, basename), false);
});

it('should be true for same hosts, matching basename', () => {
const currentURL = url.parse('http://localhost/subpath/team/channel1');
const targetURL = url.parse('http://localhost/subpath/team/channel2');
const basename = '/subpath';
assert.equal(Utils.isInternalURL(targetURL, currentURL, basename), true);
});

it('should be true for same hosts, default basename', () => {
const currentURL = url.parse('http://localhost/team/channel1');
const targetURL = url.parse('http://localhost/team/channel2');
const basename = '/';
assert.equal(Utils.isInternalURL(targetURL, currentURL, basename), true);
});

it('should be true for same hosts, default basename, empty target path', () => {
const currentURL = url.parse('http://localhost/team/channel1');
const targetURL = url.parse('http://localhost/');
const basename = '/';
assert.equal(Utils.isInternalURL(targetURL, currentURL, basename), true);
});
});
});

0 comments on commit 79e020b

Please sign in to comment.