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

Commit ec065b9

Browse files
committed
restore noscript allow once & allow temp when noscript is globally on
if noscript is on for a site only instead of globally, this feature is not available because it is more complicated to implement and because it is less useful for privacy (since changing shield settings is not possible in private tabs anyway). fix #2745 fix #3835 Auditors: @ayumi Test Plan: 1. turn on 'block scripts' in preferences 2. go to apple.com, click noscript icon, choose 'allow'. verify that scripts are allowed. 3. open private tab, go to bing.com, click noscript icon, verify that 'allow this time'is the only option. click it and verify that scripts are allowed. 4. navigate the private tab to google.com and then back to bing.com. verify that scripts are blocked on both sites. 5. go to google.com in a regular tab and choose 'allow until restart'. verify that scripts are allowed. 6. restart browser and go to about:preferences#shields. verify that only apple.com is listed as an exception.
1 parent 61f0021 commit ec065b9

File tree

9 files changed

+63
-34
lines changed

9 files changed

+63
-34
lines changed

app/extensions/brave/locales/en-US/preferences.properties

+2-1
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,8 @@ protocolRegistrationPermission=Protocol registration
192192
shieldsUp=All Brave Shields
193193
ledgerPaymentsShown=Brave Payments
194194
flash=Run Adobe Flash Player
195-
flashAllowOnce=Allow once
195+
allowOnce=Allow once
196+
allowUntilRestart=Allow until restart
196197
flashAllowAlways=Allow until {{time}}
197198
alwaysAllow=Always allow
198199
alwaysDeny=Always deny

app/sessionStore.js

+9-1
Original file line numberDiff line numberDiff line change
@@ -248,15 +248,23 @@ module.exports.cleanAppData = (data, isShutdown) => {
248248
data.perWindowState.forEach((perWindowState) =>
249249
module.exports.cleanPerWindowData(perWindowState, isShutdown))
250250
}
251-
// Delete expired Flash approvals
251+
// Delete expired Flash and NoScript allow-once approvals
252252
let now = Date.now()
253253
for (var host in data.siteSettings) {
254254
let expireTime = data.siteSettings[host].flash
255255
if (typeof expireTime === 'number' && expireTime < now) {
256256
delete data.siteSettings[host].flash
257257
}
258+
let noScript = data.siteSettings[host].noScript
259+
if (typeof noScript === 'number') {
260+
delete data.siteSettings[host].noScript
261+
}
258262
// Don't write runInsecureContent to session
259263
delete data.siteSettings[host].runInsecureContent
264+
// If the site setting is empty, delete it for privacy
265+
if (Object.keys(data.siteSettings[host]).length === 0) {
266+
delete data.siteSettings[host]
267+
}
260268
}
261269
if (data.sites) {
262270
const clearHistory = isShutdown && getSetting(settings.SHUTDOWN_CLEAR_HISTORY) === true

docs/state.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ AppStore
6868
adControl: string, // (showBraveAds | blockAds | allowAdsAndTracking)
6969
cookieControl: string, // (block3rdPartyCookie | allowAllCookies)
7070
safeBrowsing: boolean,
71-
noScript: boolean,
71+
noScript: (number|boolean), // true = block scripts, false = allow, 0 = allow once, 1 = allow until restart
7272
httpsEverywhere: boolean,
7373
fingerprintingProtection: boolean,
7474
flash: (number|boolean), // approval expiration time if allowed, false if never allow
@@ -79,6 +79,7 @@ AppStore
7979
},
8080
temporarySiteSettings: {
8181
// Same as above but never gets written to disk
82+
// XXX: This was intended for Private Browsing but is currently unused.
8283
},
8384
visits: [{
8485
location: string,

js/about/preferences.js

+8-2
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ const braveryPermissionNames = {
6868
'safeBrowsing': ['boolean'],
6969
'httpsEverywhere': ['boolean'],
7070
'fingerprintingProtection': ['boolean'],
71-
'noScript': ['boolean']
71+
'noScript': ['boolean', 'number']
7272
}
7373

7474
const changeSetting = (cb, key, e) => {
@@ -1049,7 +1049,7 @@ class SitePermissionsPage extends React.Component {
10491049
if (name === 'flash') {
10501050
if (granted === 1) {
10511051
// Flash is allowed just one time
1052-
statusText = 'flashAllowOnce'
1052+
statusText = 'allowOnce'
10531053
} else if (granted === false) {
10541054
// Flash installer is never intercepted
10551055
statusText = 'alwaysDeny'
@@ -1060,6 +1060,12 @@ class SitePermissionsPage extends React.Component {
10601060
time: new Date(granted).toLocaleString()
10611061
}
10621062
}
1063+
} else if (name === 'noScript' && typeof granted === 'number') {
1064+
if (granted === 1) {
1065+
statusText = 'allowUntilRestart'
1066+
} else if (granted === 0) {
1067+
statusText = 'allowOnce'
1068+
}
10631069
} else if (typeof granted === 'string') {
10641070
statusText = granted
10651071
} else if (!this.props.defaults) {

js/components/frame.js

+12-7
Original file line numberDiff line numberDiff line change
@@ -200,21 +200,26 @@ class Frame extends ImmutableComponent {
200200
return false
201201
}
202202

203-
expireFlash (origin) {
203+
expireContentSettings (origin) {
204204
// Expired Flash settings should be deleted when the webview is
205-
// navigated or closed.
205+
// navigated or closed. Same for NoScript's allow-once option.
206206
const activeSiteSettings = getSiteSettingsForHostPattern(this.props.allSiteSettings,
207207
origin)
208-
if (activeSiteSettings && typeof activeSiteSettings.get('flash') === 'number') {
208+
if (!activeSiteSettings) {
209+
return
210+
}
211+
if (typeof activeSiteSettings.get('flash') === 'number') {
209212
if (activeSiteSettings.get('flash') < Date.now()) {
210-
// Expired entry. Remove it.
211213
appActions.removeSiteSetting(origin, 'flash')
212214
}
213215
}
216+
if (activeSiteSettings.get('noScript') === 0) {
217+
appActions.removeSiteSetting(origin, 'noScript')
218+
}
214219
}
215220

216221
componentWillUnmount () {
217-
this.expireFlash(this.origin)
222+
this.expireContentSettings(this.origin)
218223
}
219224

220225
updateWebview (cb, newSrc) {
@@ -381,10 +386,10 @@ class Frame extends ImmutableComponent {
381386
this.updateAboutDetails()
382387
}
383388

384-
// For cross-origin navigation, clear temp Flash approvals
389+
// For cross-origin navigation, clear temp approvals
385390
const prevOrigin = siteUtil.getOrigin(prevProps.location)
386391
if (this.origin !== prevOrigin) {
387-
this.expireFlash(prevOrigin)
392+
this.expireContentSettings(prevOrigin)
388393
}
389394

390395
if (this.props.src !== prevProps.src) {

js/components/main.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,7 @@ class Main extends ImmutableComponent {
620620
}
621621

622622
enableNoScript (settings) {
623-
return siteSettings.activeSettings(settings, this.props.appState, appConfig).noScript
623+
return siteSettings.activeSettings(settings, this.props.appState, appConfig).noScript === true
624624
}
625625

626626
onCloseFrame (activeFrameProps) {
@@ -1008,6 +1008,7 @@ class Main extends ImmutableComponent {
10081008
{
10091009
noScriptIsVisible
10101010
? <NoScriptInfo frameProps={activeFrame}
1011+
noScriptGlobalEnabled={this.props.appState.getIn(['noScript', 'enabled'])}
10111012
onHide={this.onHideNoScript} />
10121013
: null
10131014
}

js/components/noScriptInfo.js

+26-16
Original file line numberDiff line numberDiff line change
@@ -21,24 +21,41 @@ class NoScriptInfo extends ImmutableComponent {
2121
return siteUtil.getOrigin(this.props.frameProps.get('location'))
2222
}
2323

24+
get isPrivate () {
25+
return this.props.frameProps.get('isPrivate')
26+
}
27+
2428
reload () {
2529
ipc.emit(messages.SHORTCUT_ACTIVE_FRAME_CLEAN_RELOAD)
2630
}
2731

28-
onAllowOnce () {
32+
onAllow (setting) {
2933
if (!this.origin) {
3034
return
3135
}
32-
ipc.send(messages.TEMPORARY_ALLOW_SCRIPTS, this.origin)
36+
appActions.changeSiteSetting(this.origin, 'noScript', setting)
3337
this.reload()
3438
}
3539

36-
onAllow (temp) {
37-
if (!this.origin) {
38-
return
40+
get buttons () {
41+
if (!this.props.noScriptGlobalEnabled) {
42+
// NoScript is not turned on globally
43+
return <div><Button l10nId='allow' className='actionButton'
44+
onClick={this.onAllow.bind(this, false)} /></div>
45+
} else {
46+
return <div>
47+
<Button l10nId='allowScriptsOnce' className='actionButton'
48+
onClick={this.onAllow.bind(this, 0)} />
49+
{this.isPrivate
50+
? null
51+
: <div>
52+
<div><Button l10nId='allowScriptsTemp' className='subtleButton'
53+
onClick={this.onAllow.bind(this, 1)} /></div>
54+
<div><Button l10nId='allow' className='subtleButton'
55+
onClick={this.onAllow.bind(this, false)} /></div>
56+
</div>}
57+
</div>
3958
}
40-
appActions.changeSiteSetting(this.origin, 'noScript', false, temp)
41-
this.reload()
4259
}
4360

4461
render () {
@@ -50,21 +67,14 @@ class NoScriptInfo extends ImmutableComponent {
5067
<div>
5168
<div className='truncate' data-l10n-args={JSON.stringify(l10nArgs)}
5269
data-l10n-id={this.numberBlocked === 1 ? 'scriptBlocked' : 'scriptsBlocked'} />
53-
<div>
54-
{
55-
// TODO: restore the allow-once button
56-
// TODO: If this is a private tab, this should only allow scripts
57-
// temporarily. Depends on #1824
58-
<Button l10nId='allow' className='actionButton'
59-
onClick={this.onAllow.bind(this, false)} />
60-
}
61-
</div>
70+
{this.buttons}
6271
</div>
6372
</Dialog>
6473
}
6574
}
6675

6776
NoScriptInfo.propTypes = {
77+
noScriptGlobalEnabled: React.PropTypes.bool,
6878
frameProps: React.PropTypes.object,
6979
onHide: React.PropTypes.func
7080
}

js/constants/messages.js

-2
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,6 @@ const messages = {
137137
HTTPSE_RULE_APPLIED: _, /** @arg {string} name of ruleset file, @arg {Object} details of rewritten request */
138138
// Extensions
139139
NEW_POPUP_WINDOW: _,
140-
// NoScript
141-
TEMPORARY_ALLOW_SCRIPTS: _, /** @arg {string} origin to allow scripts on */
142140
// Localization
143141
LANGUAGE: _, /** @arg {string} langCode, @arg {Array} availableLanguages */
144142
REQUEST_LANGUAGE: _,

js/state/contentSettings.js

+2-3
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,9 @@ const getContentSettingsFromSiteSettings = (appState) => {
130130
// We do 2 passes for setting content settings. On the first pass we consider all shield types.
131131
for (let hostPattern in hostSettings) {
132132
let hostSetting = hostSettings[hostPattern]
133-
if (typeof hostSetting.noScript === 'boolean') {
134-
// TODO: support temporary override
133+
if (['number', 'boolean'].includes(typeof hostSetting.noScript)) {
135134
addContentSettings(contentSettings.javascript, hostPattern, '*',
136-
hostSetting.noScript ? 'block' : 'allow')
135+
hostSetting.noScript === true ? 'block' : 'allow')
137136
}
138137
if (typeof hostSetting.runInsecureContent === 'boolean') {
139138
addContentSettings(contentSettings.runInsecureContent, hostPattern, '*',

0 commit comments

Comments
 (0)