Skip to content

Commit 05a00d1

Browse files
committed
Bug 1720458: Do not isolate https-only-load-insecure by origin attributes r=fluent-reviewers,settings-reviewers,flod,ckerschb,Gijs
Do not isolate `https-only-load-insecure` by origin attributes. This way the HTTPS-Only exceptions will behave similar to the `cookie` permission. This means that exceptions set in the system settings will also apply to private windows, but exceptions set in private windows via the identity pane will be reset after closing the browser. Depends on D182761 Differential Revision: https://phabricator.services.mozilla.com/D183745
1 parent 31665cb commit 05a00d1

File tree

9 files changed

+43
-39
lines changed

9 files changed

+43
-39
lines changed

browser/base/content/browser-siteIdentity.js

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -234,10 +234,10 @@ var gIdentityHandler = {
234234
"identity-popup-security-httpsonlymode-menulist"
235235
));
236236
},
237-
get _identityPopupHttpsOnlyModeMenuListTempItem() {
238-
delete this._identityPopupHttpsOnlyModeMenuListTempItem;
239-
return (this._identityPopupHttpsOnlyModeMenuListTempItem =
240-
document.getElementById("identity-popup-security-menulist-tempitem"));
237+
get _identityPopupHttpsOnlyModeMenuListOffItem() {
238+
delete this._identityPopupHttpsOnlyModeMenuListOffItem;
239+
return (this._identityPopupHttpsOnlyModeMenuListOffItem =
240+
document.getElementById("identity-popup-security-menulist-off-item"));
241241
},
242242
get _identityPopupSecurityEVContentOwner() {
243243
delete this._identityPopupSecurityEVContentOwner;
@@ -551,12 +551,6 @@ var gIdentityHandler = {
551551
return;
552552
}
553553

554-
// Permissions set in PMB get deleted anyway, but to make sure, let's make
555-
// the permission session-only.
556-
if (newValue === 1 && PrivateBrowsingUtils.isWindowPrivate(window)) {
557-
newValue = 2;
558-
}
559-
560554
// We always want to set the exception for the HTTP version of the current URI,
561555
// since when we check wether we should upgrade a request, we are checking permissons
562556
// for the HTTP principal (Bug 1757297).
@@ -1054,16 +1048,8 @@ var gIdentityHandler = {
10541048
// in _getHttpsOnlyPermission
10551049
let value = this._getHttpsOnlyPermission();
10561050

1057-
// Because everything in PBM is temporary anyway, we don't need to make the distinction
1058-
if (privateBrowsingWindow) {
1059-
if (value === 2) {
1060-
value = 1;
1061-
}
1062-
// Hide "off temporarily" option
1063-
this._identityPopupHttpsOnlyModeMenuListTempItem.style.display = "none";
1064-
} else {
1065-
this._identityPopupHttpsOnlyModeMenuListTempItem.style.display = "";
1066-
}
1051+
this._identityPopupHttpsOnlyModeMenuListOffItem.hidden =
1052+
privateBrowsingWindow && value != 1;
10671053

10681054
this._identityPopupHttpsOnlyModeMenuList.value = value;
10691055

browser/components/controlcenter/content/identityPanel.inc.xhtml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,9 @@
6868
oncommand="gIdentityHandler.changeHttpsOnlyPermission();" sizetopopup="none">
6969
<menupopup>
7070
<menuitem value="0" data-l10n-id="identity-https-only-dropdown-on" />
71-
<menuitem value="1" data-l10n-id="identity-https-only-dropdown-off" />
72-
<menuitem value="2" id="identity-popup-security-menulist-tempitem"
73-
data-l10n-id="identity-https-only-dropdown-off-temporarily" />
71+
<menuitem value="1" data-l10n-id="identity-https-only-dropdown-off"
72+
id="identity-popup-security-menulist-off-item" />
73+
<menuitem value="2" data-l10n-id="identity-https-only-dropdown-off-temporarily" />
7474
</menupopup>
7575
</menulist>
7676
<vbox id="identity-popup-security-httpsonlymode-info">

browser/components/preferences/dialogs/permissions.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ const permissionExceptionsL10n = {
3838
},
3939
"https-only-load-insecure": {
4040
window: "permissions-exceptions-https-only-window2",
41-
description: "permissions-exceptions-https-only-desc",
41+
description: "permissions-exceptions-https-only-desc2",
4242
},
4343
install: {
4444
window: "permissions-exceptions-addons-window2",

browser/components/preferences/privacy.inc.xhtml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1177,7 +1177,7 @@
11771177
permissions-allow.label,
11781178
permissions-remove.label,
11791179
permissions-remove-all.label,
1180-
permissions-exceptions-https-only-desc,
1180+
permissions-exceptions-https-only-desc2,
11811181
" />
11821182
</vbox>
11831183
</hbox>

browser/components/preferences/privacy.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ Preferences.addAll([
204204
{ id: "dom.security.https_only_mode", type: "bool" },
205205
{ id: "dom.security.https_only_mode_pbm", type: "bool" },
206206
{ id: "dom.security.https_first", type: "bool" },
207+
{ id: "dom.security.https_first_pbm", type: "bool" },
207208

208209
// Windows SSO
209210
{ id: "network.http.windows-sso.enabled", type: "bool" },
@@ -449,6 +450,9 @@ var gPrivacyPane = {
449450
let httpsFirstOnPref = Services.prefs.getBoolPref(
450451
"dom.security.https_first"
451452
);
453+
let httpsFirstOnPBMPref = Services.prefs.getBoolPref(
454+
"dom.security.https_first_pbm"
455+
);
452456
let httpsOnlyRadioGroup = document.getElementById("httpsOnlyRadioGroup");
453457
let httpsOnlyExceptionButton = document.getElementById(
454458
"httpsOnlyExceptionButton"
@@ -462,7 +466,11 @@ var gPrivacyPane = {
462466
httpsOnlyRadioGroup.value = "disabled";
463467
}
464468

465-
httpsOnlyExceptionButton.disabled = !httpsOnlyOnPref && !httpsFirstOnPref;
469+
httpsOnlyExceptionButton.disabled =
470+
!httpsOnlyOnPref &&
471+
!httpsFirstOnPref &&
472+
!httpsOnlyOnPBMPref &&
473+
!httpsFirstOnPBMPref;
466474

467475
if (
468476
Services.prefs.prefIsLocked("dom.security.https_only_mode") ||
@@ -508,6 +516,9 @@ var gPrivacyPane = {
508516
Preferences.get("dom.security.https_first").on("change", () =>
509517
this.syncFromHttpsOnlyPref()
510518
);
519+
Preferences.get("dom.security.https_first_pbm").on("change", () =>
520+
this.syncFromHttpsOnlyPref()
521+
);
511522
},
512523

513524
get dnsOverHttpsResolvers() {

browser/components/preferences/tests/browser_https_only_exceptions.js

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@
66
* Checks if buttons are disabled/enabled and visible/hidden correctly.
77
*/
88
add_task(async function testButtons() {
9-
// Let's make sure HTTPS-Only Mode is off.
9+
// Let's make sure HTTPS-Only and HTTPS-First Mode is off.
1010
await setHttpsOnlyPref("off");
11+
await setHttpsFirstPref("off");
1112

1213
// Open the privacy-pane in about:preferences
1314
await openPreferencesViaOpenPreferencesAPI("panePrivacy", {
@@ -28,8 +29,8 @@ add_task(async function testButtons() {
2829
await setHttpsOnlyPref("private");
2930
is(
3031
exceptionButton.disabled,
31-
true,
32-
"HTTPS-Only exception button should be disabled when HTTPS-Only Mode is only enabled in private browsing."
32+
false,
33+
"HTTPS-Only exception button should be enabled when HTTPS-Only Mode is only enabled in private browsing."
3334
);
3435

3536
await setHttpsOnlyPref("everywhere");
@@ -40,11 +41,17 @@ add_task(async function testButtons() {
4041
);
4142

4243
await setHttpsOnlyPref("off");
43-
await setHttpsFirstPref("private");
4444
is(
4545
exceptionButton.disabled,
4646
true,
47-
"HTTPS-Only exception button should be disabled when HTTPS-Only Mode is disabled and HTTPS-First Mode is only enabled in private browsing."
47+
"Turning off HTTPS-Only should disable the exception button again."
48+
);
49+
50+
await setHttpsFirstPref("private");
51+
is(
52+
exceptionButton.disabled,
53+
false,
54+
"HTTPS-Only exception button should be enabled when HTTPS-Only Mode is disabled and HTTPS-First Mode is only enabled in private browsing."
4855
);
4956

5057
await setHttpsFirstPref("everywhere");

browser/locales/en-US/browser/preferences/permissions.ftl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ permissions-exceptions-cookie-desc = You can specify which websites are always o
109109
permissions-exceptions-https-only-window2 =
110110
.title = Exceptions - HTTPS-Only Mode
111111
.style = { permissions-window2.style }
112-
permissions-exceptions-https-only-desc = You can turn off HTTPS-Only Mode for specific websites. { -brand-short-name } won’t attempt to upgrade the connection to secure HTTPS for those sites. Exceptions do not apply to private windows.
112+
permissions-exceptions-https-only-desc2 = You can turn off HTTPS-Only Mode for specific websites. { -brand-short-name } won’t attempt to upgrade the connection to secure HTTPS for those sites.
113113
114114
## Exceptions - Pop-ups
115115

extensions/permissions/PermissionManager.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ static const nsLiteralCString kPreloadPermissions[] = {
132132
// interception when a user has disabled storage for a specific site. Once
133133
// service worker interception moves to the parent process this should be
134134
// removed. See bug 1428130.
135-
"cookie"_ns};
135+
"cookie"_ns, "https-only-load-insecure"_ns};
136136

137137
// NOTE: nullptr can be passed as aType - if it is this function will return
138138
// "false" unconditionally.
@@ -156,8 +156,8 @@ bool IsPreloadPermission(const nsACString& aType) {
156156
// This is because perms are sent to the content process in bulk by perm key.
157157
// Non-preloaded, but OA stripped permissions would not be accessible by sites
158158
// in private browsing / non-default user context.
159-
static constexpr std::array<nsLiteralCString, 1> kStripOAPermissions = {
160-
{"cookie"_ns}};
159+
static constexpr std::array<nsLiteralCString, 2> kStripOAPermissions = {
160+
{"cookie"_ns, "https-only-load-insecure"_ns}};
161161

162162
bool IsOAForceStripPermission(const nsACString& aType) {
163163
if (aType.IsEmpty()) {

extensions/permissions/test/unit/test_permmanager_oa_strip.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const TEST_PERMISSION3 = "test/oastrip3";
88

99
// List of permissions which are not isolated by private browsing or user context
1010
// as per array kStripOAPermissions in PermissionManager.cpp
11-
const STRIPPED_PERMS = ["cookie"];
11+
const STRIPPED_PERMS = ["cookie", "https-only-load-insecure"];
1212

1313
let principal = Services.scriptSecurityManager.createContentPrincipal(
1414
TEST_URI,
@@ -204,10 +204,10 @@ function testOAIsolation(permIsolateUserContext, permIsolatePrivateBrowsing) {
204204
)
205205
);
206206
}
207-
});
208207

209-
// Cleanup
210-
pm.removeAll();
208+
// Cleanup
209+
pm.removeAll();
210+
});
211211
}
212212

213213
add_task(async function do_test() {

0 commit comments

Comments
 (0)