Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
show grey insecure icon for passive mixed content
Browse files Browse the repository at this point in the history
fix #2168

Auditors: @darkdh

Test Plan:
1. automated tests related to lockIcon should pass
2. go to mixed.badssl.com; the icon should be a grey unlocked icon
3. clicking on the icon should tell you that the page is partially insecure
  • Loading branch information
diracdeltas committed Mar 8, 2017
1 parent 433ff00 commit 4a2d9c3
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 60 deletions.
3 changes: 2 additions & 1 deletion app/extensions/brave/locales/en-US/app.properties
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ applyAndRestart=Apply and restart
fullscreenNotice={{origin}} is now fullscreen. Press ESC to exit.
secureConnection=Secure connection
secureConnectionInfo=This page was loaded securely over HTTPS.
mixedConnection=Partially insecure connection
partiallySecureConnection=Partially insecure connection
partiallySecureConnectionInfo=This page was loaded over HTTPS, but some elements were not loaded securely.
insecureConnection=Using an insecure connection
insecureConnectionInfo=Your connection to this site is not private. An eavesdropper may be able to tamper with this page and read your data.
blockedTrackingElements={{blockedTrackingElementsSize}} Blocked tracking elements
Expand Down
56 changes: 23 additions & 33 deletions app/renderer/components/urlBarIcon.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,40 +11,28 @@ const dndData = require('../../../js/dndData')
const {isSourceAboutUrl} = require('../../../js/lib/appUrlUtil')
const searchIconSize = 16

const getIconCssClass = (ctx) => {
if (ctx.isSearch) {
return 'fa-search'
} else if (ctx.isAboutPage && !ctx.props.titleMode) {
return 'fa-list'
} else if (ctx.isSecure) {
// NOTE: EV style not approved yet; see discussion at https://github.com/brave/browser-laptop/issues/791
return 'fa-lock'
} else if (ctx.isInsecure) {
return 'fa-unlock'
}
}

class UrlBarIcon extends ImmutableComponent {
constructor () {
super()
this.onClick = this.onClick.bind(this)
this.onDragStart = this.onDragStart.bind(this)
}
get isSecure () {
return this.props.isHTTPPage &&
this.props.isSecure &&
!this.props.active
}
/**
* insecure icon does not show when:
* - loading
* - in title mode
* - urlbar is active (ex: you can type)
*/
get isInsecure () {
return this.props.isHTTPPage &&
this.props.isSecure === false &&
!this.props.active
get iconCssClasses () {
if (this.isSearch) {
return ['fa-search']
} else if (this.isAboutPage && !this.props.titleMode) {
return ['fa-list']
} else if (this.props.isHTTPPage && !this.props.active) {
// NOTE: EV style not approved yet; see discussion at https://github.com/brave/browser-laptop/issues/791
if (this.props.isSecure === true) {
return ['fa-lock']
} else if (this.props.isSecure === false) {
return ['fa-unlock', 'insecure-color']
} else if (this.props.isSecure === 1) {
return ['fa-unlock']
}
}
return []
}
/**
* search icon:
Expand All @@ -55,7 +43,7 @@ class UrlBarIcon extends ImmutableComponent {
get isSearch () {
const showSearch = this.props.isSearching && !this.props.titleMode

const defaultToSearch = (!this.isSecure && !this.isInsecure && !showSearch) &&
const defaultToSearch = (!this.props.isHTTPPage || this.props.active) &&
!this.props.titleMode &&
!this.isAboutPage

Expand All @@ -69,12 +57,14 @@ class UrlBarIcon extends ImmutableComponent {
if (this.props.activateSearchEngine) {
return cx({urlbarIcon: true})
}

return cx({
const iconClasses = {
urlbarIcon: true,
'fa': true,
[ getIconCssClass(this) ]: true
fa: true
}
this.iconCssClasses.forEach((iconClass) => {
iconClasses[iconClass] = true
})
return cx(iconClasses)
}
get iconStyles () {
if (!this.props.activateSearchEngine) {
Expand Down
4 changes: 2 additions & 2 deletions docs/state.md
Original file line number Diff line number Diff line change
Expand Up @@ -456,8 +456,8 @@ WindowStore
showFullScreenWarning: boolean, // true if a warning should be shown about full screen
security: {
blockedRunInsecureContent: Array<string>, // sources of blocked active mixed content
isSecure: boolean, // is using https
isExtendedValidation: boolean, // is using https ev
isSecure: (boolean|number), // true = fully secure, false = fully insecure, 1 = partially secure
isExtendedValidation: boolean, // is using https ev. not currently used.
loginRequiredDetail: {
isProxy: boolean,
host: string,
Expand Down
16 changes: 8 additions & 8 deletions js/components/frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -892,19 +892,19 @@ class Frame extends ImmutableComponent {
}
let isSecure = null
let runInsecureContent = this.runInsecureContent()
// 'warning' and 'passive mixed content' should never upgrade the
// security state from insecure to secure
if (e.securityState === 'secure' ||
(this.props.isSecure !== false &&
runInsecureContent !== true &&
['warning', 'passive-mixed-content'].includes(e.securityState))) {
if (e.securityState === 'secure') {
isSecure = true
} else if (['broken', 'insecure'].includes(e.securityState)) {
isSecure = false
} else if (this.props.isSecure !== false &&
['warning', 'passive-mixed-content'].includes(e.securityState)) {
// Passive mixed content should not upgrade an insecure connection to a
// partially-secure connection. It can only downgrade a secure
// connection.
isSecure = 1
}
// TODO: show intermediate UI for 'warning' and 'passive-mixed-content'
windowActions.setSecurityState(this.frame, {
secure: isSecure,
secure: runInsecureContent ? false : isSecure,
runInsecureContent
})
if (isSecure) {
Expand Down
3 changes: 1 addition & 2 deletions js/components/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -998,8 +998,7 @@ class Main extends ImmutableComponent {
partitionNumber={activeFrame && activeFrame.get('partitionNumber') || 0}
history={activeFrame && activeFrame.get('history') || emptyList}
suggestionIndex={activeFrame && activeFrame.getIn(['navbar', 'urlbar', 'suggestions', 'selectedIndex']) || 0}
isSecure={activeFrame && activeFrame.getIn(['security', 'isSecure']) &&
!activeFrame.getIn(['security', 'runInsecureContent'])}
isSecure={activeFrame ? activeFrame.getIn(['security', 'isSecure']) : null}
hasLocationValueSuffix={activeFrame && activeFrame.getIn(['navbar', 'urlbar', 'suggestions', 'urlSuffix'])}
startLoadTime={activeFrame && activeFrame.get('startLoadTime') || undefined}
endLoadTime={activeFrame && activeFrame.get('endLoadTime') || undefined}
Expand Down
16 changes: 11 additions & 5 deletions js/components/siteInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,20 @@ class SiteInfo extends ImmutableComponent {
partitionNumber: this.partitionNumber
}
let secureIcon
if (this.isSecure && !this.runInsecureContent) {
if (this.isSecure === true && !this.runInsecureContent) {
// fully secure
secureIcon = <li><span
className={cx({
fa: true,
'fa-lock': true,
extendedValidation: this.isExtendedValidation
})} /><span data-l10n-id='secureConnection' /></li>
} else if (this.isSecure && this.runInsecureContent) {
secureIcon = <li><span className='fa fa-unlock' /><span data-l10n-id='mixedConnection' /></li>
} else if (this.isSecure === 1 && !this.runInsecureContent) {
// passive mixed content loaded
secureIcon = <li><span className='fa fa-unlock' /><span data-l10n-id='partiallySecureConnection' /></li>
} else {
secureIcon = <li><span className='fa fa-unlock' /><span data-l10n-id='insecureConnection' data-l10n-args={JSON.stringify(l10nArgs)} /></li>
// insecure
secureIcon = <li><span className='fa fa-unlock' /><span data-l10n-id='insecureConnection' /></li>
}

let partitionInfo
Expand Down Expand Up @@ -99,9 +102,12 @@ class SiteInfo extends ImmutableComponent {
</li>
</ul>
</li>
} else if (this.isSecure) {
} else if (this.isSecure === true) {
connectionInfo =
<div className='connectionInfo' data-l10n-id='secureConnectionInfo' />
} else if (this.isSecure === 1) {
connectionInfo =
<div className='connectionInfo' data-l10n-id='partiallySecureConnectionInfo' />
} else {
connectionInfo =
<div className='connectionInfo' data-l10n-id='insecureConnectionInfo' />
Expand Down
4 changes: 2 additions & 2 deletions less/navigationBar.less
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,7 @@
left: 2px;
min-width: 0;

&.fa-unlock {
&.insecure-color {
color: @siteInsecureColor;
opacity: 1.0;
}
Expand Down Expand Up @@ -911,7 +911,7 @@
font-size: 16px;
}

&.fa-unlock {
&.insecure-color {
color: @siteInsecureColor;
}

Expand Down
16 changes: 9 additions & 7 deletions test/components/navigationBarTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ describe('navigationBar tests', function () {
.activateURLMode()
.waitForExist(urlbarIcon)
.getAttribute(urlbarIcon, 'class').then((classes) =>
classes.includes('fa-unlock')
classes.includes('fa-unlock') && classes.includes('insecure-color')
))
.windowByUrl(Brave.browserWindowUrl)
.click(urlbarIcon)
Expand All @@ -426,7 +426,7 @@ describe('navigationBar tests', function () {
.waitForExist(titleBar)
.isExisting(urlbarIcon).then((isExisting) => assert(isExisting))
.getAttribute(urlbarIcon, 'class').then((classes) =>
classes.includes('fa-unlock')
classes.includes('fa-unlock') && classes.includes('insecure-color')
)
})
it('Shows secure URL icon', function * () {
Expand Down Expand Up @@ -470,17 +470,19 @@ describe('navigationBar tests', function () {
assert(!classes.includes('fa-lock'))
)
})
it('shows secure icon on a site with passive mixed content', function * () {
it('shows partially-secure icon on a site with passive mixed content', function * () {
const page1Url = 'https://mixed.badssl.com/'
yield this.app.client.tabByUrl(Brave.newTabUrl).url(page1Url).waitForUrl(page1Url).windowParentByUrl(page1Url)
yield this.app.client
.activateURLMode()
.waitForExist(urlbarIcon)
.waitUntil(() =>
this.app.client.getAttribute(urlbarIcon, 'class').then((classes) =>
classes.includes('fa-lock')
classes.includes('fa-unlock') && !classes.includes('insecure-color')
)
)
.click(urlbarIcon)
.waitForVisible('[data-l10n-id="partiallySecureConnection"]')
})
it('shows insecure icon on a site with a sha-1 cert', function * () {
const page1Url = 'https://sha1-2017.badssl.com/'
Expand All @@ -490,7 +492,7 @@ describe('navigationBar tests', function () {
.waitForExist(urlbarIcon)
.waitUntil(() =>
this.app.client.getAttribute(urlbarIcon, 'class').then((classes) =>
classes.includes('fa-unlock')
classes.includes('fa-unlock') && classes.includes('insecure-color')
)
)
})
Expand All @@ -514,7 +516,7 @@ describe('navigationBar tests', function () {
.waitForExist(urlbarIcon)
.waitUntil(() =>
this.app.client.getAttribute(urlbarIcon, 'class').then((classes) =>
classes.includes('fa-unlock')
classes.includes('fa-unlock') && classes.includes('insecure-color')
)
)
})
Expand Down Expand Up @@ -598,7 +600,7 @@ describe('navigationBar tests', function () {
.windowByUrl(Brave.browserWindowUrl)
.waitForExist(urlbarIcon)
.click(urlbarIcon)
.waitForVisible('[data-l10n-id="mixedConnection"]')
.waitForVisible('[data-l10n-id="insecureConnection"]')
.waitForVisible('.denyRunInsecureContentWarning')
.waitForVisible(dismissDenyRunInsecureContentButton)
.waitForVisible(denyRunInsecureContentButton)
Expand Down

0 comments on commit 4a2d9c3

Please sign in to comment.