From 4a395a77a5531e5a716ad5e453f1e580926b7dcd Mon Sep 17 00:00:00 2001 From: yan Date: Wed, 15 Feb 2017 02:13:28 +0000 Subject: [PATCH] add noScriptInfo checkboxes to allow scripts by origin fix #6431. adjusts the noScriptInfo dialog so that it can be used to allow scripts by origin, instead of all scripts on the page, when NoScript is globally enabled. removes the option to allow scripts persistently (since this can be done via the Bravery panel anyway) auditors: @bbondy --- .../brave/locales/en-US/app.properties | 7 +- app/sessionStore.js | 2 + docs/appActions.md | 12 +++ docs/state.md | 1 + js/actions/appActions.js | 13 +++ js/components/frame.js | 4 + js/components/noScriptInfo.js | 79 +++++++++++++++---- js/constants/appConstants.js | 3 +- js/state/contentSettings.js | 21 +++++ js/stores/appStore.js | 12 +++ less/forms.less | 14 +++- test/components/noScriptInfoTest.js | 77 ++++++++++++++++++ test/fixtures/noscript.html | 10 +++ test/lib/selectors.js | 4 + 14 files changed, 236 insertions(+), 23 deletions(-) create mode 100644 test/components/noScriptInfoTest.js create mode 100644 test/fixtures/noscript.html diff --git a/app/extensions/brave/locales/en-US/app.properties b/app/extensions/brave/locales/en-US/app.properties index a5b55115f9d..c81e1657348 100644 --- a/app/extensions/brave/locales/en-US/app.properties +++ b/app/extensions/brave/locales/en-US/app.properties @@ -117,11 +117,10 @@ updateHide=Hide sessionInfoCommon=This tab uses session {{partitionNumber}} sessionInfo={{sessionInfoCommon}} sessionInfoTab.title={{sessionInfoCommon}} -allowScripts=Allow on this site always -allowScriptsTemp=Allow on this site until restart +allowScripts=Allow always +allowScriptsTemp=Allow until restart allowScriptsOnce=Allow this time -scriptsBlocked={{numberBlocked}} scripts blocked on {{site}} -scriptBlocked={{numberBlocked}} script blocked on {{site}} +scriptsBlocked=Do you want to allow scripts on {{site}} from these sources? findResults={{activeMatchOrdinal}} of {{numberOfMatches}} findResultMatches={[plural(numberOfMatches)]} findResultMatches[one]={{numberOfMatches}} match diff --git a/app/sessionStore.js b/app/sessionStore.js index 3da9ddc19b0..ea7337b92c6 100644 --- a/app/sessionStore.js +++ b/app/sessionStore.js @@ -293,6 +293,8 @@ module.exports.cleanAppData = (data, isShutdown) => { if (typeof noScript === 'number') { delete data.siteSettings[host].noScript } + // Don't persist any noScript exceptions + delete data.siteSettings[host].noScriptExceptions // Don't write runInsecureContent to session delete data.siteSettings[host].runInsecureContent // If the site setting is empty, delete it for privacy diff --git a/docs/appActions.md b/docs/appActions.md index ab09972995b..de73e77056a 100644 --- a/docs/appActions.md +++ b/docs/appActions.md @@ -660,6 +660,18 @@ Dispatches a message when a tab is being cloned +### setNoScriptExceptions(hostPattern, origins) + +Dispatches a message when noscript exceptions are added for an origin + +**Parameters** + +**hostPattern**: `string`, Dispatches a message when noscript exceptions are added for an origin + +**origins**: `Object.<string, (boolean|number)>`, Dispatches a message when noscript exceptions are added for an origin + + + ### setObjectId(objectId, objectPath) Dispatches a message to set objectId for a syncable object. diff --git a/docs/state.md b/docs/state.md index c434dca9521..bbe6b2c356c 100644 --- a/docs/state.md +++ b/docs/state.md @@ -246,6 +246,7 @@ AppStore midiSysexPermission: boolean, notificationsPermission: boolean, noScript: (number|boolean), // true = block scripts, false = allow, 0 = allow once, 1 = allow until restart + noScriptExceptions: {[hostPattern]: (number|boolean)}, // hosts where scripts are allowed once (0) or until restart (1). false = block objectId: Array., openExternalPermission: boolean, pointerLockPermission: boolean, diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 32e2867c153..68fd9862797 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -810,6 +810,19 @@ const appActions = { }) }, + /** + * Dispatches a message when noscript exceptions are added for an origin + * @param {string} hostPattern + * @param {Object.} origins + */ + setNoScriptExceptions: function (hostPattern, origins) { + AppDispatcher.dispatch({ + actionType: appConstants.APP_SET_NOSCRIPT_EXCEPTIONS, + hostPattern, + origins + }) + }, + /** * Dispatches a message to set objectId for a syncable object. * @param {Array.} objectId diff --git a/js/components/frame.js b/js/components/frame.js index bc19f2c4987..17d7cbd514c 100644 --- a/js/components/frame.js +++ b/js/components/frame.js @@ -283,6 +283,10 @@ class Frame extends ImmutableComponent { if (activeSiteSettings.get('noScript') === 0) { appActions.removeSiteSetting(origin, 'noScript', this.props.isPrivate) } + const noScriptExceptions = activeSiteSettings.get('noScriptExceptions') + if (noScriptExceptions) { + appActions.setNoScriptExceptions(origin, noScriptExceptions.filter((value, host) => value !== 0)) + } } componentWillUnmount () { diff --git a/js/components/noScriptInfo.js b/js/components/noScriptInfo.js index 213ca82ea63..02c3eee6283 100644 --- a/js/components/noScriptInfo.js +++ b/js/components/noScriptInfo.js @@ -3,6 +3,7 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ const React = require('react') +const Immutable = require('immutable') const ImmutableComponent = require('./immutableComponent') const Dialog = require('./dialog') const Button = require('./button') @@ -10,11 +11,37 @@ const appActions = require('../actions/appActions') const siteUtil = require('../state/siteUtil') const ipc = require('electron').ipcRenderer const messages = require('../constants/messages') +const urlParse = require('url').parse + +class NoScriptCheckbox extends ImmutableComponent { + toggleCheckbox (e) { + this.checkbox.checked = !this.checkbox.checked + e.stopPropagation() + } + + get id () { + return `checkbox-for-${this.props.origin}` + } + + render () { + return
+ { e.stopPropagation() }} + ref={(node) => { this.checkbox = node }} defaultChecked + origin={this.props.origin} /> + +
+ } +} class NoScriptInfo extends ImmutableComponent { - get numberBlocked () { + get blockedOrigins () { const blocked = this.props.frameProps.getIn(['noScript', 'blocked']) - return blocked ? blocked.size : 0 + if (blocked && blocked.size) { + return new Immutable.Set(blocked.map(siteUtil.getOrigin)) + } else { + return new Immutable.Set() + } } get origin () { @@ -29,18 +56,32 @@ class NoScriptInfo extends ImmutableComponent { ipc.emit(messages.SHORTCUT_ACTIVE_FRAME_CLEAN_RELOAD) } - onAllow (setting) { + onAllow (setting, e) { if (!this.origin) { return } - appActions.changeSiteSetting(this.origin, 'noScript', setting) - this.reload() + if (setting === false) { + appActions.changeSiteSetting(this.origin, 'noScript', setting) + this.reload() + } else { + let checkedOrigins = new Immutable.Map() + this.checkboxes.querySelectorAll('input').forEach((box) => { + const origin = box.getAttribute('origin') + if (origin) { + checkedOrigins = checkedOrigins.set(origin, box.checked ? setting : false) + } + }) + if (checkedOrigins.size) { + appActions.setNoScriptExceptions(this.origin, checkedOrigins) + this.reload() + } + } } get buttons () { if (!this.props.noScriptGlobalEnabled) { // NoScript is not turned on globally - return
} else { return
@@ -48,26 +89,32 @@ class NoScriptInfo extends ImmutableComponent { onClick={this.onAllow.bind(this, 0)} /> {this.isPrivate ? null - :
-
-
-
} + :
} } render () { + if (!this.origin) { + return null + } const l10nArgs = { - numberBlocked: this.numberBlocked, - site: this.props.frameProps.get('location') || 'this page' + site: urlParse(this.props.frameProps.get('location')).host } return
- {this.buttons} + data-l10n-id={'scriptsBlocked'} /> + {this.blockedOrigins.size + ?
+
{ this.checkboxes = node }} className='blockedOriginsList'> + {this.blockedOrigins.map((origin) => )} +
+ {this.buttons} +
+ : null}
} diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index 406da26d2c8..015006ca4a5 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -86,7 +86,8 @@ const appConstants = { APP_TAB_TOGGLE_DEV_TOOLS: _, APP_TAB_CLONED: _, APP_SET_OBJECT_ID: _, - APP_SAVE_SYNC_INIT_DATA: _ + APP_SAVE_SYNC_INIT_DATA: _, + APP_SET_NOSCRIPT_EXCEPTIONS: _ } module.exports = mapValuesByKeys(appConstants) diff --git a/js/state/contentSettings.js b/js/state/contentSettings.js index 7e01933a5da..de8f5eda8f0 100644 --- a/js/state/contentSettings.js +++ b/js/state/contentSettings.js @@ -200,6 +200,26 @@ const siteSettingsToContentSettings = (currentSiteSettings, defaultContentSettin if (['number', 'boolean'].includes(typeof siteSetting.get('noScript'))) { contentSettings = addContentSettings(contentSettings, 'javascript', primaryPattern, '*', siteSetting.get('noScript') === true ? 'block' : 'allow') } + const noScriptExceptions = siteSetting.get('noScriptExceptions') + if (noScriptExceptions && noScriptExceptions.has(hostPattern)) { + // Allow all is needed for inline scripts to run. XXX: this seems like + // a muon bug. + contentSettings = addContentSettings(contentSettings, 'javascript', primaryPattern, '*', 'allow') + // Re-block the origins that aren't excluded + noScriptExceptions.forEach((value, origin) => { + if (value === false) { + contentSettings = addContentSettings(contentSettings, 'javascript', + primaryPattern, origin, 'block') + } + }) + } else if (noScriptExceptions && noScriptExceptions.size) { + noScriptExceptions.forEach((value, origin) => { + if (typeof value === 'number') { + contentSettings = addContentSettings(contentSettings, 'javascript', + primaryPattern, origin === hostPattern ? '*' : origin, 'allow') + } + }) + } if (typeof siteSetting.get('runInsecureContent') === 'boolean') { contentSettings = addContentSettings(contentSettings, 'runInsecureContent', primaryPattern, '*', siteSetting.get('runInsecureContent') ? 'allow' : 'block') @@ -279,6 +299,7 @@ const doAction = (action) => { switch (action.actionType) { case appConstants.APP_REMOVE_SITE_SETTING: case appConstants.APP_CHANGE_SITE_SETTING: + case appConstants.APP_SET_NOSCRIPT_EXCEPTIONS: AppDispatcher.waitFor([AppStore.dispatchToken], () => { userPrefsUpdateTrigger(action.temporary) contentSettingsUpdateTrigger(action.temporary) diff --git a/js/stores/appStore.js b/js/stores/appStore.js index 5a02410cee2..631150b1e27 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -650,6 +650,18 @@ const handleAppAction = (action) => { appState = appState.set(propertyName, newSiteSettings) break } + case appConstants.APP_SET_NOSCRIPT_EXCEPTIONS: + // Note that this is always cleared on restart or reload, so should not + // be synced or persisted. + let key = 'noScriptExceptions' + if (!action.origins || !action.origins.size) { + // Clear the exceptions + appState = appState.setIn(['siteSettings', action.hostPattern, key], new Immutable.Map()) + } else { + const currentExceptions = appState.getIn(['siteSettings', action.hostPattern, key]) || new Immutable.Map() + appState = appState.setIn(['siteSettings', action.hostPattern, key], currentExceptions.merge(action.origins)) + } + break case appConstants.APP_UPDATE_LEDGER_INFO: appState = appState.set('ledgerInfo', Immutable.fromJS(action.ledgerInfo)) break diff --git a/less/forms.less b/less/forms.less index 551516956ae..fdc29de9327 100644 --- a/less/forms.less +++ b/less/forms.less @@ -494,14 +494,24 @@ select { .flyoutDialog; right: 20px; width: auto; - max-width: 350px; - text-align: center; + max-width: 400px; font-size: 15px; cursor: default; + text-align: center; .truncate { margin-bottom: 5px; } + .noScriptCheckbox { + text-align: left; + } + button { + margin: 2px; + margin-top: 10px; + } + label { + margin-left: 2px; + } } } diff --git a/test/components/noScriptInfoTest.js b/test/components/noScriptInfoTest.js new file mode 100644 index 00000000000..7d9f2b54b29 --- /dev/null +++ b/test/components/noScriptInfoTest.js @@ -0,0 +1,77 @@ +/* global describe, it, beforeEach */ + +const Brave = require('../lib/brave') +const appConfig = require('../../js/constants/appConfig') +const messages = require('../../js/constants/messages') +const assert = require('assert') +const {urlInput, noScriptNavButton, noScriptInfo, noScriptAllowTempButton, noScriptAllowOnceButton} = require('../lib/selectors') + +describe('noscript info', function () { + function * setup (client) { + yield client + .waitForUrl(Brave.newTabUrl) + .waitForBrowserWindow() + .waitForVisible(urlInput) + .setResourceEnabled(appConfig.resourceNames.NOSCRIPT, true) + } + + const result = '#result' + + Brave.beforeEach(this) + beforeEach(function * () { + this.url = Brave.server.url('noscript.html') + yield setup(this.app.client) + }) + + it('can selectively allow scripts', function * () { + yield this.app.client + .tabByIndex(0) + .loadUrl(this.url) + .waitForTextValue(result, '0') + .windowByUrl(Brave.browserWindowUrl) + .waitForVisible(noScriptNavButton) + .click(noScriptNavButton) + .waitForVisible(noScriptInfo) + .waitUntil(function () { + return this.getText('.blockedOriginsList') + .then((text) => { + return text.includes('https://cdnjs.cloudflare.com') && text.includes('http://localhost:') + }) + }) + .click('[for="checkbox-for-https://cdnjs.cloudflare.com"]') // keep blocking cloudflare + .waitForVisible(noScriptAllowTempButton) + .click(noScriptAllowTempButton) + .tabByIndex(0) + .loadUrl(this.url) + .waitForTextValue(result, '1') + .windowByUrl(Brave.browserWindowUrl) + .waitForVisible(noScriptNavButton) + .click(noScriptNavButton) + .waitForVisible(noScriptInfo) + .waitUntil(function () { + return this.getText('.blockedOriginsList') + .then((text) => { + return text.includes('https://cdnjs.cloudflare.com') && !text.includes('http://localhost:') + }) + }) + .click(noScriptAllowOnceButton) // unblock cloudflare + .tabByIndex(0) + .loadUrl(this.url) + .waitForTextValue(result, '2') + }) + + it('only shows allow once in private tab', function * () { + yield this.app.client + .ipcSend(messages.SHORTCUT_NEW_FRAME, this.url, { isPrivate: true }) + .waitForTabCount(2) + .windowByUrl(Brave.browserWindowUrl) + .waitForVisible(noScriptNavButton) + .click(noScriptNavButton) + .waitForVisible(noScriptAllowOnceButton) + .isExisting(noScriptAllowTempButton).then((isExisting) => assert(!isExisting)) + .click(noScriptAllowOnceButton) + .tabByIndex(1) + .loadUrl(this.url) + .waitForTextValue(result, '2') + }) +}) diff --git a/test/fixtures/noscript.html b/test/fixtures/noscript.html new file mode 100644 index 00000000000..afcb590e719 --- /dev/null +++ b/test/fixtures/noscript.html @@ -0,0 +1,10 @@ + + + + +
0
+ + diff --git a/test/lib/selectors.js b/test/lib/selectors.js index 1117fa925b9..475ebc0ab77 100644 --- a/test/lib/selectors.js +++ b/test/lib/selectors.js @@ -48,6 +48,10 @@ module.exports = { fpSwitch: '.fingerprintingProtectionSwitch .switchMiddle', noScriptSwitch: '.noScriptSwitch .switchMiddle', noScriptNavButton: '#navigator .noScript', + noScriptInfo: '.noScriptInfo', + noScriptAllowTempButton: '[data-l10n-id="allowScriptsTemp"]', + noScriptAllowOnceButton: '[data-l10n-id="allowScriptsOnce"]', + noScriptAllowButton: '[data-l10n-id="allowScripts"]', safeBrowsingSwitch: '.safeBrowsingSwitch .switchMiddle', backButton: '.backforward .backButton', forwardButton: '.backforward .forwardButton',