From 707868014f5d8a330c300cf29bb81dc0aff7c3d8 Mon Sep 17 00:00:00 2001 From: Anthony Tseng Date: Fri, 26 Aug 2016 17:59:31 +0800 Subject: [PATCH] 1. Add option to allow mixed content 2. Change icon of http and mixed content to fa-unlock 3. Remove twitch out of site hack 4. Add test fix #3443 reuire https://github.com/brave/electron/pull/47 Auditors: @bridiver Test Plan: Visit https://mixed-script.badssl.com/ and click the urlbar lock to temporarily allow run insecure content and the background color will be red. --- .../brave/locales/en-US/app.properties | 3 + app/sessionStore.js | 2 + docs/state.md | 4 +- docs/windowActions.md | 13 ++++ js/actions/windowActions.js | 14 ++++ js/components/frame.js | 24 ++++--- js/components/siteInfo.js | 44 ++++++++++-- js/components/urlBar.js | 2 +- js/constants/windowConstants.js | 3 +- js/data/siteHacks.js | 4 -- js/state/contentSettings.js | 8 +++ js/stores/windowStore.js | 15 ++++ less/navigationBar.less | 4 +- test/components/navigationBarTest.js | 70 ++++++++++++++++--- test/lib/selectors.js | 4 +- 15 files changed, 179 insertions(+), 35 deletions(-) diff --git a/app/extensions/brave/locales/en-US/app.properties b/app/extensions/brave/locales/en-US/app.properties index 4345bcc4c57..75d37407f24 100644 --- a/app/extensions/brave/locales/en-US/app.properties +++ b/app/extensions/brave/locales/en-US/app.properties @@ -190,3 +190,6 @@ phone=Phone email=Email editAddress=Edit Address editCreditCard=Edit Credit Card +denyRunInsecureContent=Stay Secure +allowRunInsecureContent=Load Unsafe Scripts +runInsecureContentWarning=This page is trying to load scripts from insecure sources. If you allow this content to run it will not be encrypted and it may transmit unencrypted data to other sites. diff --git a/app/sessionStore.js b/app/sessionStore.js index 848d126ae74..002163dfc3b 100644 --- a/app/sessionStore.js +++ b/app/sessionStore.js @@ -248,6 +248,8 @@ module.exports.cleanAppData = (data, isShutdown) => { if (typeof expireTime === 'number' && expireTime < now) { delete data.siteSettings[host].flash } + // Don't write runInsecureContent to session + delete data.siteSettings[host].runInsecureContent } if (data.sites) { const clearHistory = isShutdown && getSetting(settings.SHUTDOWN_CLEAR_HISTORY) === true diff --git a/docs/state.md b/docs/state.md index ab57234a2d7..9702c02818e 100644 --- a/docs/state.md +++ b/docs/state.md @@ -267,8 +267,8 @@ WindowStore realm: string }, isExtendedValidation: boolean, // is using https ev - activeMixedContent: boolean, // has active mixed content - passiveMixedContent: boolean, // has passive mixed content + runInsecureContent: boolean, // has active mixed content + blockedRunInsecureContent: string, // first domain of blocked active mixed content }, parentFrameKey: number, // the key of the frame this frame was opened from modalPromptDetail: {...}, diff --git a/docs/windowActions.md b/docs/windowActions.md index 86c8aeba492..cb921022b9e 100644 --- a/docs/windowActions.md +++ b/docs/windowActions.md @@ -753,6 +753,19 @@ Sets the manage autofill credit card popup detail +### setBlockedRunInsecureContent(frameProps, source) + +Sets page url with blocked active mixed content. + +**Parameters** + +**frameProps**: `Object`, The frame to set source of +blocked active mixed content on + +**source**: `string`, Source of blocked active mixed content + + + * * * diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index 8d818c6bf98..e0a21f7dbb0 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -971,6 +971,20 @@ const windowActions = { currentDetail, originalDetail }) + }, + + /** + * Sets page url with blocked active mixed content. + * @param {Object} frameProps - The frame to set source of + * blocked active mixed content on + * @param {string} source - Source of blocked active mixed content + */ + setBlockedRunInsecureContent: function (frameProps, source) { + dispatch({ + actionType: WindowConstants.WINDOW_SET_BLOCKED_RUN_INSECURE_CONTENT, + frameProps, + source + }) } } diff --git a/js/components/frame.js b/js/components/frame.js index cedcece7c9a..c7c017cb587 100644 --- a/js/components/frame.js +++ b/js/components/frame.js @@ -166,13 +166,13 @@ class Frame extends ImmutableComponent { } shouldCreateWebview () { - return !this.webview || this.webview.allowRunningInsecureContent !== this.allowRunningInsecureContent() || - !!this.webview.allowRunningPlugins !== this.allowRunningPlugins() + return !this.webview || !!this.webview.allowRunningPlugins !== this.allowRunningPlugins() } - allowRunningInsecureContent () { - let hack = siteHacks[urlParse(this.props.location).hostname] - return !!(hack && hack.allowRunningInsecureContent) + runInsecureContent () { + const activeSiteSettings = getSiteSettingsForHostPattern(this.props.allSiteSettings, this.origin) + return activeSiteSettings === undefined + ? false : activeSiteSettings.get('runInsecureContent') } allowRunningPlugins (url) { @@ -263,10 +263,6 @@ class Frame extends ImmutableComponent { if (hack && hack.userAgent) { this.webview.setAttribute('useragent', hack.userAgent) } - if (this.allowRunningInsecureContent()) { - this.webview.setAttribute('allowRunningInsecureContent', true) - this.webview.allowRunningInsecureContent = true - } if (this.allowRunningPlugins()) { this.webview.setAttribute('plugins', true) this.webview.allowRunningPlugins = true @@ -584,6 +580,9 @@ class Frame extends ImmutableComponent { windowActions.setBlockedBy(this.frame, 'noScript', e.details[1]) } }) + this.webview.addEventListener('did-block-run-insecure-content', (e) => { + windowActions.setBlockedRunInsecureContent(this.frame, this.props.location) + }) this.webview.addEventListener('context-menu', (e) => { contextMenus.onMainContextMenu(e.params, this.frame) e.preventDefault() @@ -758,9 +757,12 @@ class Frame extends ImmutableComponent { interceptFlash(true, e.url) } windowActions.onWebviewLoadStart(this.frame, e.url) - const isSecure = parsedUrl.protocol === 'https:' && !this.allowRunningInsecureContent() + windowActions.setBlockedRunInsecureContent(this.frame) + const isSecure = parsedUrl.protocol === 'https:' && !this.runInsecureContent() + const runInsecureContent = parsedUrl.protocol === 'https:' && this.runInsecureContent() windowActions.setSecurityState(this.frame, { - secure: isSecure + secure: isSecure, + runInsecureContent: runInsecureContent }) if (isSecure) { // Check that there isn't a cert error. diff --git a/js/components/siteInfo.js b/js/components/siteInfo.js index 3425820fd75..e89b8491d62 100644 --- a/js/components/siteInfo.js +++ b/js/components/siteInfo.js @@ -3,34 +3,51 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ const React = require('react') +const ipc = require('electron').ipcRenderer const ImmutableComponent = require('./immutableComponent') const cx = require('../lib/classSet.js') const Dialog = require('./dialog') +const Button = require('./button') +const appActions = require('../actions/appActions') +const messages = require('../constants/messages') +const siteUtil = require('../state/siteUtil') class SiteInfo extends ImmutableComponent { + constructor () { + super() + this.onAllowRunInsecureContent = this.onAllowRunInsecureContent.bind(this) + } + onAllowRunInsecureContent () { + appActions.changeSiteSetting(siteUtil.getOrigin(this.isBlockedRunInsecureContent), 'runInsecureContent', true) + ipc.emit(messages.SHORTCUT_ACTIVE_FRAME_LOAD_URL, {}, this.isBlockedRunInsecureContent) + this.props.onHide() + } get isExtendedValidation () { return this.props.frameProps.getIn(['security', 'isExtendedValidation']) } get isSecure () { return this.props.frameProps.getIn(['security', 'isSecure']) } - get isMixedContent () { - return this.props.frameProps.getIn(['security', 'isMixedContent']) + get runInsecureContent () { + return this.props.frameProps.getIn(['security', 'runInsecureContent']) + } + get isBlockedRunInsecureContent () { + return this.props.frameProps.getIn(['security', 'blockedRunInsecureContent']) } get partitionNumber () { return this.props.frameProps.getIn(['partitionNumber']) } render () { let secureIcon - if (this.isSecure && !this.isMixedContent) { + if (this.isSecure && !this.runInsecureContent) { secureIcon =
    • - } else if (this.isMixedContent) { - secureIcon =
    • + } else if (this.runInsecureContent) { + secureIcon =
    • } else { secureIcon =
    • } @@ -46,6 +63,20 @@ class SiteInfo extends ImmutableComponent { } + let runInsecureContentWarning = null + if (this.isBlockedRunInsecureContent) { + runInsecureContentWarning = +
  • +
      +
      • +
    • +
      • +
    +
    • + } + return
      e.stopPropagation()}> { @@ -54,6 +85,9 @@ class SiteInfo extends ImmutableComponent { { partitionInfo } + { + runInsecureContentWarning + }
    } diff --git a/js/components/urlBar.js b/js/components/urlBar.js index a6b2d3d1443..0e4d14f11f5 100644 --- a/js/components/urlBar.js +++ b/js/components/urlBar.js @@ -383,7 +383,7 @@ class UrlBar extends ImmutableComponent { urlbarIcon: true, 'fa': !this.activateSearchEngine, 'fa-lock': !this.activateSearchEngine && this.isHTTPPage && this.props.isSecure && !this.props.urlbar.get('active'), - 'fa-unlock-alt': !this.activateSearchEngine && this.isHTTPPage && !this.props.isSecure && !this.props.urlbar.get('active') && !this.props.titleMode, + 'fa-unlock': !this.activateSearchEngine && this.isHTTPPage && !this.props.isSecure && !this.props.urlbar.get('active') && !this.props.titleMode, 'fa fa-file': !this.activateSearchEngine && this.props.urlbar.get('active') && this.props.loading === false, extendedValidation: this.extendedValidationSSL })} diff --git a/js/constants/windowConstants.js b/js/constants/windowConstants.js index fd537128644..a24bfa04c7a 100644 --- a/js/constants/windowConstants.js +++ b/js/constants/windowConstants.js @@ -66,7 +66,8 @@ const windowConstants = { WINDOW_SET_LAST_ZOOM_PERCENTAGE: _, WINDOW_SET_CLEAR_BROWSING_DATA_DETAIL: _, WINDOW_SET_AUTOFILL_ADDRESS_DETAIL: _, - WINDOW_SET_AUTOFILL_CREDIT_CARD_DETAIL: _ + WINDOW_SET_AUTOFILL_CREDIT_CARD_DETAIL: _, + WINDOW_SET_BLOCKED_RUN_INSECURE_CONTENT: _ } module.exports = mapValuesByKeys(windowConstants) diff --git a/js/data/siteHacks.js b/js/data/siteHacks.js index d9c5dd99445..cd1f83b0536 100644 --- a/js/data/siteHacks.js +++ b/js/data/siteHacks.js @@ -35,7 +35,6 @@ module.exports = { }, // For links like: https://player.twitch.tv/?channel=iwilldominate 'player.twitch.tv': { - allowRunningInsecureContent: true, enableForAll: true }, 'www.wired.com': { @@ -61,9 +60,6 @@ module.exports = { }; })();` }, - 'www.twitch.tv': { - allowRunningInsecureContent: true - }, 'imasdk.googleapis.com': { enableForAdblock: true, onBeforeRequest: function (details) { diff --git a/js/state/contentSettings.js b/js/state/contentSettings.js index 8ecc2bae4d5..7f08c597bda 100644 --- a/js/state/contentSettings.js +++ b/js/state/contentSettings.js @@ -112,6 +112,10 @@ const getContentSettingsFromSiteSettings = (appState) => { flashActive: [{ setting: 'block', primaryPattern: '*' + }], + runInsecureContent: [{ + setting: 'block', + primaryPattern: '*' }] } @@ -124,6 +128,10 @@ const getContentSettingsFromSiteSettings = (appState) => { addContentSettings(contentSettings.javascript, hostPattern, '*', hostSetting.noScript ? 'block' : 'allow') } + if (typeof hostSetting.runInsecureContent === 'boolean') { + addContentSettings(contentSettings.runInsecureContent, hostPattern, '*', + hostSetting.runInsecureContent ? 'allow' : 'block') + } if (hostSetting.cookieControl) { if (hostSetting.cookieControl === 'block3rdPartyCookie') { addContentSettings(contentSettings.cookies, hostPattern, '*', 'block') diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index 3ed515c1929..302e0c4cedb 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -742,6 +742,10 @@ const doAction = (action) => { windowState = windowState.setIn(path.concat(['security', 'isSecure']), action.securityState.secure) } + if (action.securityState.runInsecureContent !== undefined) { + windowState = windowState.setIn(path.concat(['security', 'runInsecureContent']), + action.securityState.runInsecureContent) + } if (action.securityState.certDetails) { windowState = windowState.setIn(path.concat(['security', 'certDetails']), action.securityState.certDetails) @@ -763,6 +767,17 @@ const doAction = (action) => { history: addToHistory(action.frameProps) }) break + case WindowConstants.WINDOW_SET_BLOCKED_RUN_INSECURE_CONTENT: + const blockedRunInsecureContentPath = + ['frames', FrameStateUtil.getFramePropsIndex(windowState.get('frames'), action.frameProps)] + if (action.source) { + windowState = + windowState.setIn(blockedRunInsecureContentPath.concat(['security', 'blockedRunInsecureContent']), action.source) + } else { + windowState = + windowState.deleteIn(blockedRunInsecureContentPath.concat(['security', 'blockedRunInsecureContent'])) + } + break default: } diff --git a/less/navigationBar.less b/less/navigationBar.less index d8ea09a862b..8f2a6f52949 100644 --- a/less/navigationBar.less +++ b/less/navigationBar.less @@ -384,14 +384,14 @@ min-width: 16px; &.fa-lock, - &.fa-unlock-alt { + &.fa-unlock { margin-top: 4px; font-size: 16px; min-height: 10px; min-width: 16px; } - &.fa-unlock-alt { + &.fa-unlock { color: @gray; } diff --git a/test/components/navigationBarTest.js b/test/components/navigationBarTest.js index 5bcd1f17847..89d3ce77092 100644 --- a/test/components/navigationBarTest.js +++ b/test/components/navigationBarTest.js @@ -2,7 +2,9 @@ const Brave = require('../lib/brave') const config = require('../../js/constants/config') -const {urlBarSuggestions, urlInput, activeWebview, activeTabFavicon, activeTab, navigatorLoadTime, navigator, titleBar, urlbarIcon, bookmarksToolbar, navigatorNotBookmarked, navigatorBookmarked, saveButton} = require('../lib/selectors') +const {urlBarSuggestions, urlInput, activeWebview, activeTabFavicon, activeTab, navigatorLoadTime, + navigator, titleBar, urlbarIcon, bookmarksToolbar, navigatorNotBookmarked, navigatorBookmarked, + saveButton, allowRunInsecureContentButton, denyRunInsecureContentButton} = require('../lib/selectors') const urlParse = require('url').parse const assert = require('assert') const settings = require('../../js/constants/settings') @@ -231,9 +233,9 @@ describe('navigationBar', function () { }) describe('lockIcon', function () { - Brave.beforeAll(this) + Brave.beforeEach(this) - before(function * () { + beforeEach(function * () { yield setup(this.app.client) }) @@ -247,6 +249,10 @@ describe('navigationBar', function () { .getAttribute(urlbarIcon, 'class').then((classes) => classes.includes('fa-unlock') )) + .windowByUrl(Brave.browserWindowUrl) + .click(urlbarIcon) + .waitForVisible('[data-l10n-id="insecureConnection"]') + .keys('\uE00C') }) it('Shows secure URL icon', function * () { const page1Url = 'https://badssl.com/' @@ -258,20 +264,68 @@ describe('navigationBar', function () { this.app.client.getAttribute(urlbarIcon, 'class').then((classes) => classes.includes('fa-lock') )) + .windowByUrl(Brave.browserWindowUrl) + .click(urlbarIcon) + .waitForVisible('[data-l10n-id="secureConnection"]') + .keys('\uE00C') }) - it('Blocks active mixed content', function * () { + it('Blocks running insecure content', function * () { const page1Url = 'https://mixed-script.badssl.com/' yield this.app.client.tabByUrl(Brave.newTabUrl) - .url(page1Url) + .loadUrl(page1Url) + // background color changes when insecure content runs .waitUntil(() => { - return this.app.client.execute(() => document.readyState).then((ret) => - ret.value === 'complete' + return this.app.client.getCssProperty('body', 'background-color').then((color) => + color.value === 'rgba(128,128,128,1)' + ) + }) + .windowByUrl(Brave.browserWindowUrl) + .waitForExist(urlbarIcon + '.fa-lock') + .click(urlbarIcon) + .waitForVisible('.runInsecureContentWarning') + .waitForVisible(denyRunInsecureContentButton) + .waitForVisible(allowRunInsecureContentButton) + .waitForVisible('[data-l10n-id="secureConnection"]') + .click(denyRunInsecureContentButton) + // TODO(bridiver) there is a race condition here because we are waiting for a non-change + // and we need some way to verify that the page does not reload and allow insecure content + .tabByUrl(page1Url).waitUntil(() => { + return this.app.client.getCssProperty('body', 'background-color').then((color) => + color.value === 'rgba(128,128,128,1)' ) - }).waitUntil(() => { + }) + .windowByUrl(Brave.browserWindowUrl) + .click(urlbarIcon) + .waitForExist(urlbarIcon + '.fa-lock') + }) + it('Temporarily allow running insecure content', function * () { + const page1Url = 'https://mixed-script.badssl.com/' + yield this.app.client.tabByUrl(Brave.newTabUrl) + .loadUrl(page1Url) + // background color changes when insecure content runs + .waitUntil(() => { return this.app.client.getCssProperty('body', 'background-color').then((color) => color.value === 'rgba(128,128,128,1)' ) }) + .windowByUrl(Brave.browserWindowUrl) + .waitForExist(urlbarIcon + '.fa-lock') + .click(urlbarIcon) + .waitForVisible('.runInsecureContentWarning') + .waitForVisible(denyRunInsecureContentButton) + .waitForVisible(allowRunInsecureContentButton) + .click(allowRunInsecureContentButton) + .tabByUrl(this.page1Url) + .waitUntil(() => { + return this.app.client.getCssProperty('body', 'background-color').then((color) => + color.value === 'rgba(255,0,0,1)' + ) + }) + .windowByUrl(Brave.browserWindowUrl) + .click(urlbarIcon) + .waitForExist(urlbarIcon + '.fa-unlock') + .waitForVisible('[data-l10n-id="mixedConnection"]') + .keys('\uE00C') }) }) diff --git a/test/lib/selectors.js b/test/lib/selectors.js index f82ce4bbb23..eb8c71de6eb 100644 --- a/test/lib/selectors.js +++ b/test/lib/selectors.js @@ -60,5 +60,7 @@ module.exports = { paymentHistoryButton: '.paymentHistoryButton', paymentsWelcomePage: '.paymentsMessage', autofillAddressPanel: '.autofillAddressPanel', - autofillCreditCardPanel: '.autofillCreditCardPanel' + autofillCreditCardPanel: '.autofillCreditCardPanel', + allowRunInsecureContentButton: '.allowRunInsecureContentButton', + denyRunInsecureContentButton: '.denyRunInsecureContentButton' }