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

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

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

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.

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.<number>,
       openExternalPermission: boolean,
       pointerLockPermission: boolean,

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.<string, (boolean|number)>} 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.<number>} objectId

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 () {

js/components/noScriptInfo.js

@@ -3,18 +3,45 @@
  * 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')
 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 <div className='noScriptCheckbox' id={this.id}>
+      <input type='checkbox' onClick={(e) => { e.stopPropagation() }}
+        ref={(node) => { this.checkbox = node }} defaultChecked
+        origin={this.props.origin} />
+      <label htmlFor={this.id}
+        onClick={this.toggleCheckbox.bind(this)}>{this.props.origin}</label>
+    </div>
+  }
+}
 
 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,45 +56,65 @@ 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 <div><Button l10nId='allow' className='actionButton'
+      return <div><Button l10nId='allowScripts' className='actionButton'
         onClick={this.onAllow.bind(this, false)} /></div>
     } else {
       return <div>
         <Button l10nId='allowScriptsOnce' className='actionButton'
           onClick={this.onAllow.bind(this, 0)} />
         {this.isPrivate
           ? null
-          : <div>
-            <div><Button l10nId='allowScriptsTemp' className='subtleButton'
-              onClick={this.onAllow.bind(this, 1)} /></div>
-            <div><Button l10nId='allow' className='subtleButton'
-              onClick={this.onAllow.bind(this, false)} /></div>
-          </div>}
+          : <span><Button l10nId='allowScriptsTemp' className='subtleButton'
+            onClick={this.onAllow.bind(this, 1)} /></span>
+        }
       </div>
     }
   }
 
   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 <Dialog onHide={this.props.onHide} className='noScriptInfo' isClickDismiss>
       <div className='dialogInner'>
         <div className='truncate' data-l10n-args={JSON.stringify(l10nArgs)}
-          data-l10n-id={this.numberBlocked === 1 ? 'scriptBlocked' : 'scriptsBlocked'} />
-        {this.buttons}
+          data-l10n-id={'scriptsBlocked'} />
+        {this.blockedOrigins.size
+          ? <div>
+            <div ref={(node) => { this.checkboxes = node }} className='blockedOriginsList'>
+              {this.blockedOrigins.map((origin) => <NoScriptCheckbox origin={origin} />)}
+            </div>
+            {this.buttons}
+          </div>
+          : null}
       </div>
     </Dialog>
   }

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)

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)

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

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;
+    }
   }
 }
 

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')
+  })
+})

test/fixtures/noscript.html

@@ -0,0 +1,10 @@
+<head>
+  <script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.1.1/jquery.min.js"></script>
+</head>
+<body>
+    <div id='result'>0</div>
+    <script>
+        document.querySelector('#result').innerText = 1
+        $('#result').text(2)
+    </script>
+</body>