From bd29139bde8e7de7a31acb23fb71f88015fba93a Mon Sep 17 00:00:00 2001 From: Stanca Serban <sstanca@mozilla.com> Date: Tue, 13 Dec 2022 14:53:38 +0200 Subject: [PATCH] Backed out changeset 514642d76faa (bug 1792138) for causing mochitests failures in test_ext_protocolHandlers.html. CLOSED TREE --- .../browser/browser_ext_windows_create_url.js | 37 -- caps/BasePrincipal.cpp | 7 - caps/BasePrincipal.h | 2 - caps/nsIPrincipal.idl | 1 - toolkit/components/extensions/Extension.jsm | 13 - .../en-US/toolkit/global/handlerDialog.ftl | 10 - .../handling/ContentDispatchChooser.jsm | 37 +- .../handling/content/permissionDialog.js | 13 - .../browser_protocol_ask_dialog_permission.js | 406 +----------------- 9 files changed, 13 insertions(+), 513 deletions(-) diff --git a/browser/components/extensions/test/browser/browser_ext_windows_create_url.js b/browser/components/extensions/test/browser/browser_ext_windows_create_url.js index e5f40a7bf2783..285100cc09e30 100644 --- a/browser/components/extensions/test/browser/browser_ext_windows_create_url.js +++ b/browser/components/extensions/test/browser/browser_ext_windows_create_url.js @@ -147,34 +147,6 @@ add_task(async function testWindowCreate() { } } - // Watch for any permission prompts to show up and accept them. - let dialogCount = 0; - let windowObserver = window => { - // This listener will go away when the window is closed so there is no need - // to explicitely remove it. - // eslint-disable-next-line mozilla/balanced-listeners - window.addEventListener("dialogopen", event => { - dialogCount++; - let { dialog } = event.detail; - Assert.equal( - dialog?._openedURL, - "chrome://mozapps/content/handling/permissionDialog.xhtml", - "Should only ever see the permission dialog" - ); - let dialogEl = dialog._frame.contentDocument.querySelector("dialog"); - Assert.ok(dialogEl, "Dialog element should exist"); - dialogEl.setAttribute("buttondisabledaccept", false); - dialogEl.acceptDialog(); - }); - }; - Services.obs.addObserver(windowObserver, "browser-delayed-startup-finished"); - registerCleanupFunction(() => { - Services.obs.removeObserver( - windowObserver, - "browser-delayed-startup-finished" - ); - }); - let extension = ExtensionTestUtils.loadExtension({ manifest: { permissions: ["tabs"], @@ -198,13 +170,4 @@ add_task(async function testWindowCreate() { await extension.awaitFinish("window-create-url"); await extension.unload(); await pageExt.unload(); - - Assert.equal( - dialogCount, - 2, - "Expected to see the right number of permission prompts." - ); - - // Make sure windows have been released before finishing. - Cu.forceGC(); }); diff --git a/caps/BasePrincipal.cpp b/caps/BasePrincipal.cpp index 97f066255eeb8..1fa8dc0963ae7 100644 --- a/caps/BasePrincipal.cpp +++ b/caps/BasePrincipal.cpp @@ -1133,13 +1133,6 @@ nsresult BasePrincipal::GetAddonPolicy( return NS_OK; } -nsresult BasePrincipal::GetContentScriptAddonPolicy( - extensions::WebExtensionPolicy** aResult) { - RefPtr<extensions::WebExtensionPolicy> policy(ContentScriptAddonPolicy()); - policy.forget(aResult); - return NS_OK; -} - extensions::WebExtensionPolicy* BasePrincipal::AddonPolicy() { AssertIsOnMainThread(); RefPtr<extensions::WebExtensionPolicyCore> core = AddonPolicyCore(); diff --git a/caps/BasePrincipal.h b/caps/BasePrincipal.h index 3b845cb7b43d4..33644c3c87200 100644 --- a/caps/BasePrincipal.h +++ b/caps/BasePrincipal.h @@ -127,8 +127,6 @@ class BasePrincipal : public nsJSPrincipals { bool allowIfInheritsPrincipal, uint64_t innerWindowID) final; NS_IMETHOD GetAddonPolicy(extensions::WebExtensionPolicy** aResult) final; - NS_IMETHOD GetContentScriptAddonPolicy( - extensions::WebExtensionPolicy** aResult) final; NS_IMETHOD GetIsNullPrincipal(bool* aResult) override; NS_IMETHOD GetIsContentPrincipal(bool* aResult) override; NS_IMETHOD GetIsExpandedPrincipal(bool* aResult) override; diff --git a/caps/nsIPrincipal.idl b/caps/nsIPrincipal.idl index 7d42f49716258..e5bacca601aa9 100644 --- a/caps/nsIPrincipal.idl +++ b/caps/nsIPrincipal.idl @@ -551,7 +551,6 @@ interface nsIPrincipal : nsISupports * NOTE: Main-Thread Only. */ readonly attribute WebExtensionPolicy addonPolicy; - readonly attribute WebExtensionPolicy contentScriptAddonPolicy; /** * Gets the id of the user context this principal is inside. If this diff --git a/toolkit/components/extensions/Extension.jsm b/toolkit/components/extensions/Extension.jsm index f018cab5ddd3e..467e621ba72ff 100644 --- a/toolkit/components/extensions/Extension.jsm +++ b/toolkit/components/extensions/Extension.jsm @@ -233,9 +233,6 @@ const INSTALL_AND_UPDATE_STARTUP_REASONS = new Set([ "ADDON_DOWNGRADE", ]); -const PROTOCOL_HANDLER_OPEN_PERM_KEY = "open-protocol-handler"; -const PERMISSION_KEY_DELIMITER = "^"; - // Returns true if the extension is owned by Mozilla (is either privileged, // using one of the @mozilla.com/@mozilla.org protected addon id suffixes). // @@ -577,16 +574,6 @@ var ExtensionAddonObserver = { Services.perms.removeFromPrincipal(principal, "persistent-storage"); } - // Clear any protocol handler permissions granted to this add-on. - let permissions = Services.perms.getAllWithTypePrefix( - PROTOCOL_HANDLER_OPEN_PERM_KEY + PERMISSION_KEY_DELIMITER - ); - for (let perm of permissions) { - if (perm.principal.equalsURI(baseURI)) { - Services.perms.removePermission(perm); - } - } - if (!Services.prefs.getBoolPref(LEAVE_UUID_PREF, false)) { // Clear the entry in the UUID map UUIDMap.remove(addon.id); diff --git a/toolkit/locales/en-US/toolkit/global/handlerDialog.ftl b/toolkit/locales/en-US/toolkit/global/handlerDialog.ftl index 97fdfec188abe..20f0e72d7dc84 100644 --- a/toolkit/locales/en-US/toolkit/global/handlerDialog.ftl +++ b/toolkit/locales/en-US/toolkit/global/handlerDialog.ftl @@ -7,7 +7,6 @@ ## $host - the hostname that is initiating the request ## $scheme - the type of link that's being opened. ## $appName - Name of the application that will be opened. -## $extension - Name of extension that initiated the request permission-dialog-description = Allow this site to open the { $scheme } link? @@ -18,9 +17,6 @@ permission-dialog-description-file = permission-dialog-description-host = Allow { $host } to open the { $scheme } link? -permission-dialog-description-extension = - Allow the extension { $extension } to open the { $scheme } link? - permission-dialog-description-app = Allow this site to open the { $scheme } link with { $appName }? @@ -30,9 +26,6 @@ permission-dialog-description-host-app = permission-dialog-description-file-app = Allow this file to open the { $scheme } link with { $appName }? -permission-dialog-description-extension-app = - Allow the extension { $extension } to open the { $scheme } link with { $appName }? - ## Please keep the emphasis around the hostname and scheme (ie the ## `<strong>` HTML tags). Please also keep the hostname as close to the start ## of the sentence as your language's grammar allows. @@ -43,9 +36,6 @@ permission-dialog-remember = permission-dialog-remember-file = Always allow this file to open <strong>{ $scheme }</strong> links -permission-dialog-remember-extension = - Always allow this extension to open <strong>{ $scheme }</strong> links - ## permission-dialog-btn-open-link = diff --git a/toolkit/mozapps/handling/ContentDispatchChooser.jsm b/toolkit/mozapps/handling/ContentDispatchChooser.jsm index ef3c179fedfba..12ba75d0bef8f 100644 --- a/toolkit/mozapps/handling/ContentDispatchChooser.jsm +++ b/toolkit/mozapps/handling/ContentDispatchChooser.jsm @@ -293,6 +293,10 @@ class nsContentDispatchChooser { return false; } + if (aPrincipal.isAddonOrExpandedAddonPrincipal) { + return true; + } + let key = this._getSkipProtoDialogPermissionKey(scheme); return ( Services.perms.testPermissionFromPrincipal(aPrincipal, key) === @@ -392,28 +396,16 @@ class nsContentDispatchChooser { return; } - let principal = aPrincipal; - - // If this action was triggered by an extension content script then set the - // permission on the extension's principal. - let addonPolicy = aPrincipal.contentScriptAddonPolicy; - if (addonPolicy) { - principal = Services.scriptSecurityManager.principalWithOA( - addonPolicy.extension.principal, - principal.originAttributes - ); - } - let permKey = this._getSkipProtoDialogPermissionKey(aScheme); if (aAllow) { Services.perms.addFromPrincipal( - principal, + aPrincipal, permKey, Services.perms.ALLOW_ACTION, Services.perms.EXPIRE_NEVER ); } else { - Services.perms.removeFromPrincipal(principal, permKey); + Services.perms.removeFromPrincipal(aPrincipal, permKey); } } @@ -423,18 +415,11 @@ class nsContentDispatchChooser { * @returns {boolean} - true if we can store permissions, false otherwise. */ _isSupportedPrincipal(aPrincipal) { - if (!aPrincipal) { - return false; - } - - // If this is an add-on content script then we will be able to store - // permissions against the add-on's principal. - if (aPrincipal.contentScriptAddonPolicy) { - return true; - } - - return ["http", "https", "moz-extension", "file"].some(scheme => - aPrincipal.schemeIs(scheme) + return ( + aPrincipal && + ["http", "https", "moz-extension", "file"].some(scheme => + aPrincipal.schemeIs(scheme) + ) ); } } diff --git a/toolkit/mozapps/handling/content/permissionDialog.js b/toolkit/mozapps/handling/content/permissionDialog.js index 196d7e2411160..83164ff7bf9c1 100644 --- a/toolkit/mozapps/handling/content/permissionDialog.js +++ b/toolkit/mozapps/handling/content/permissionDialog.js @@ -23,8 +23,6 @@ let dialog = { this._handlerInfo = handler.QueryInterface(Ci.nsIHandlerInfo); this._principal = principal?.QueryInterface(Ci.nsIPrincipal); - this._addonPolicy = - this._principal?.addonPolicy ?? this._principal?.contentScriptAddonPolicy; this._browsingContext = browsingContext; this._outArgs = outArgs.QueryInterface(Ci.nsIWritablePropertyBag); this._preferredHandlerName = preferredHandlerName; @@ -84,13 +82,6 @@ let dialog = { * the triggering principal and the preferred application handler. */ get l10nDescriptionId() { - if (this._addonPolicy) { - if (this._preferredHandlerName) { - return "permission-dialog-description-extension-app"; - } - return "permission-dialog-description-extension"; - } - if (this._principal?.schemeIs("file")) { if (this._preferredHandlerName) { return "permission-dialog-description-file-app"; @@ -125,9 +116,6 @@ let dialog = { return null; } - if (this._addonPolicy) { - return "permission-dialog-remember-extension"; - } if (this._principal.schemeIs("file")) { return "permission-dialog-remember-file"; } @@ -180,7 +168,6 @@ let dialog = { document.l10n.setAttributes(description, this.l10nDescriptionId, { host, scheme, - extension: this._addonPolicy?.name, appName: this._preferredHandlerName, }); diff --git a/uriloader/exthandler/tests/mochitest/browser_protocol_ask_dialog_permission.js b/uriloader/exthandler/tests/mochitest/browser_protocol_ask_dialog_permission.js index 5afb85d30d0bc..8d1971e0da64d 100644 --- a/uriloader/exthandler/tests/mochitest/browser_protocol_ask_dialog_permission.js +++ b/uriloader/exthandler/tests/mochitest/browser_protocol_ask_dialog_permission.js @@ -7,10 +7,6 @@ var { AppConstants } = ChromeUtils.importESModule( "resource://gre/modules/AppConstants.sys.mjs" ); -const { ExtensionPermissions } = ChromeUtils.import( - "resource://gre/modules/ExtensionPermissions.jsm" -); - let gHandlerService = Cc["@mozilla.org/uriloader/handler-service;1"].getService( Ci.nsIHandlerService ); @@ -213,7 +209,6 @@ async function testOpenProto( actionCheckbox, actionConfirm, actionChangeApp, - checkContents, } = permDialogOptions; if (actionChangeApp) { @@ -261,10 +256,6 @@ async function testOpenProto( ); } - if (checkContents) { - checkContents(dialogEl); - } - if (actionChangeApp) { let dialogClosedPromise = waitForProtocolPermissionDialog(browser, false); changeAppLink.click(); @@ -789,254 +780,13 @@ add_task(async function test_non_standard_protocol() { }); /** - * Tests that we show the permission dialog for extension content scripts. - */ -add_task(async function test_extension_content_script_permission() { - let scheme = TEST_PROTOS[0]; - await BrowserTestUtils.withNewTab(ORIGIN1, async browser => { - let testExtension; - - await testOpenProto(browser, scheme, { - triggerLoad: async () => { - let uri = `${scheme}://test`; - - const EXTENSION_DATA = { - manifest: { - content_scripts: [ - { - matches: [browser.currentURI.spec], - js: ["navigate.js"], - }, - ], - browser_specific_settings: { - gecko: { id: "allowed@mochi.test" }, - }, - }, - files: { - "navigate.js": `window.location.href = "${uri}";`, - }, - useAddonManager: "permanent", - }; - - testExtension = ExtensionTestUtils.loadExtension(EXTENSION_DATA); - await testExtension.startup(); - }, - permDialogOptions: { - hasCheckbox: true, - chooserIsNext: true, - hasChangeApp: false, - actionCheckbox: true, - actionConfirm: true, - checkContents: dialogEl => { - let description = dialogEl.querySelector("#description"); - let { id, args } = description.ownerDocument.l10n.getAttributes( - description - ); - is( - id, - "permission-dialog-description-extension", - "Should be using the correct string." - ); - is( - args.extension, - "Generated extension", - "Should have the correct extension name." - ); - }, - }, - chooserDialogOptions: { - hasCheckbox: true, - actionConfirm: false, // Cancel dialog - }, - }); - - let extensionPrincipal = Services.scriptSecurityManager.createContentPrincipal( - Services.io.newURI(`moz-extension://${testExtension.uuid}/`), - {} - ); - let extensionPrivatePrincipal = Services.scriptSecurityManager.createContentPrincipal( - Services.io.newURI(`moz-extension://${testExtension.uuid}/`), - { privateBrowsingId: 1 } - ); - - let key = getSkipProtoDialogPermissionKey(scheme); - is( - Services.perms.testPermissionFromPrincipal(extensionPrincipal, key), - Services.perms.ALLOW_ACTION, - "Should have permanently allowed the extension" - ); - is( - Services.perms.testPermissionFromPrincipal( - extensionPrivatePrincipal, - key - ), - Services.perms.UNKNOWN_ACTION, - "Should not have changed the private principal permission" - ); - is( - Services.perms.testPermissionFromPrincipal(PRINCIPAL1, key), - Services.perms.UNKNOWN_ACTION, - "Should not have allowed the page" - ); - - await testExtension.unload(); - - is( - Services.perms.testPermissionFromPrincipal(extensionPrincipal, key), - Services.perms.UNKNOWN_ACTION, - "Should have cleared the extension's normal principal permission" - ); - is( - Services.perms.testPermissionFromPrincipal( - extensionPrivatePrincipal, - key - ), - Services.perms.UNKNOWN_ACTION, - "Should have cleared the private browsing principal" - ); - }); -}); - -/** - * Tests that we show the permission dialog for extension content scripts. - */ -add_task(async function test_extension_private_content_script_permission() { - let scheme = TEST_PROTOS[0]; - let win = await BrowserTestUtils.openNewBrowserWindow({ private: true }); - - await BrowserTestUtils.withNewTab( - { gBrowser: win.gBrowser, url: ORIGIN1 }, - async browser => { - let testExtension; - - await testOpenProto(browser, scheme, { - triggerLoad: async () => { - let uri = `${scheme}://test`; - - const EXTENSION_DATA = { - manifest: { - content_scripts: [ - { - matches: [browser.currentURI.spec], - js: ["navigate.js"], - }, - ], - browser_specific_settings: { - gecko: { id: "allowed@mochi.test" }, - }, - }, - files: { - "navigate.js": `window.location.href = "${uri}";`, - }, - useAddonManager: "permanent", - }; - - testExtension = ExtensionTestUtils.loadExtension(EXTENSION_DATA); - await testExtension.startup(); - let perms = { - permissions: ["internal:privateBrowsingAllowed"], - origins: [], - }; - await ExtensionPermissions.add("allowed@mochi.test", perms); - let addon = await AddonManager.getAddonByID("allowed@mochi.test"); - await addon.reload(); - }, - permDialogOptions: { - hasCheckbox: true, - chooserIsNext: true, - hasChangeApp: false, - actionCheckbox: true, - actionConfirm: true, - checkContents: dialogEl => { - let description = dialogEl.querySelector("#description"); - let { id, args } = description.ownerDocument.l10n.getAttributes( - description - ); - is( - id, - "permission-dialog-description-extension", - "Should be using the correct string." - ); - is( - args.extension, - "Generated extension", - "Should have the correct extension name." - ); - }, - }, - chooserDialogOptions: { - hasCheckbox: true, - actionConfirm: false, // Cancel dialog - }, - }); - - let extensionPrincipal = Services.scriptSecurityManager.createContentPrincipal( - Services.io.newURI(`moz-extension://${testExtension.uuid}/`), - {} - ); - let extensionPrivatePrincipal = Services.scriptSecurityManager.createContentPrincipal( - Services.io.newURI(`moz-extension://${testExtension.uuid}/`), - { privateBrowsingId: 1 } - ); - - let key = getSkipProtoDialogPermissionKey(scheme); - is( - Services.perms.testPermissionFromPrincipal(extensionPrincipal, key), - Services.perms.UNKNOWN_ACTION, - "Should not have changed the extension's normal principal permission" - ); - is( - Services.perms.testPermissionFromPrincipal( - extensionPrivatePrincipal, - key - ), - Services.perms.ALLOW_ACTION, - "Should have allowed the private browsing principal" - ); - is( - Services.perms.testPermissionFromPrincipal(PRINCIPAL1, key), - Services.perms.UNKNOWN_ACTION, - "Should not have allowed the page" - ); - - await testExtension.unload(); - - is( - Services.perms.testPermissionFromPrincipal(extensionPrincipal, key), - Services.perms.UNKNOWN_ACTION, - "Should have cleared the extension's normal principal permission" - ); - is( - Services.perms.testPermissionFromPrincipal( - extensionPrivatePrincipal, - key - ), - Services.perms.UNKNOWN_ACTION, - "Should have cleared the private browsing principal" - ); - } - ); - - await BrowserTestUtils.closeWindow(win); -}); - -/** - * Tests that we do not show the permission dialog for extension content scripts - * when the page already has permission. + * Tests that we skip the permission dialog for extension callers. */ -add_task(async function test_extension_allowed_content() { +add_task(async function test_extension_principal() { let scheme = TEST_PROTOS[0]; await BrowserTestUtils.withNewTab(ORIGIN1, async browser => { let testExtension; - let key = getSkipProtoDialogPermissionKey(scheme); - Services.perms.addFromPrincipal( - PRINCIPAL1, - key, - Services.perms.ALLOW_ACTION, - Services.perms.EXPIRE_NEVER - ); - await testOpenProto(browser, scheme, { triggerLoad: async () => { let uri = `${scheme}://test`; @@ -1064,158 +814,6 @@ add_task(async function test_extension_allowed_content() { }, }); - let extensionPrincipal = Services.scriptSecurityManager.createContentPrincipal( - Services.io.newURI(`moz-extension://${testExtension.uuid}/`), - {} - ); - - is( - Services.perms.testPermissionFromPrincipal(extensionPrincipal, key), - Services.perms.UNKNOWN_ACTION, - "Should not have permanently allowed the extension" - ); - - await testExtension.unload(); - Services.perms.removeFromPrincipal(PRINCIPAL1, key); - }); -}); - -/** - * Tests that we do not show the permission dialog for extension content scripts - * when the extension already has permission. - */ -add_task(async function test_extension_allowed_extension() { - let scheme = TEST_PROTOS[0]; - await BrowserTestUtils.withNewTab(ORIGIN1, async browser => { - let testExtension; - - let key = getSkipProtoDialogPermissionKey(scheme); - - await testOpenProto(browser, scheme, { - triggerLoad: async () => { - const EXTENSION_DATA = { - manifest: { - permissions: [`${ORIGIN1}/*`], - }, - background() { - browser.test.onMessage.addListener(async (msg, uri) => { - switch (msg) { - case "engage": - browser.tabs.executeScript({ - code: `window.location.href = "${uri}";`, - }); - break; - default: - browser.test.fail(`Unexpected message received: ${msg}`); - } - }); - }, - }; - - testExtension = ExtensionTestUtils.loadExtension(EXTENSION_DATA); - await testExtension.startup(); - - let extensionPrincipal = Services.scriptSecurityManager.createContentPrincipal( - Services.io.newURI(`moz-extension://${testExtension.uuid}/`), - {} - ); - Services.perms.addFromPrincipal( - extensionPrincipal, - key, - Services.perms.ALLOW_ACTION, - Services.perms.EXPIRE_NEVER - ); - - testExtension.sendMessage("engage", `${scheme}://test`); - }, - chooserDialogOptions: { - hasCheckbox: true, - actionConfirm: false, // Cancel dialog - }, - }); - - await testExtension.unload(); - Services.perms.removeFromPrincipal(PRINCIPAL1, key); - }); -}); - -/** - * Tests that we show the permission dialog for extensions directly opening a - * protocol. - */ -add_task(async function test_extension_principal() { - let scheme = TEST_PROTOS[0]; - await BrowserTestUtils.withNewTab(ORIGIN1, async browser => { - let testExtension; - - await testOpenProto(browser, scheme, { - triggerLoad: async () => { - const EXTENSION_DATA = { - background() { - browser.test.onMessage.addListener(async (msg, url) => { - switch (msg) { - case "engage": - browser.tabs.update({ - url, - }); - break; - default: - browser.test.fail(`Unexpected message received: ${msg}`); - } - }); - }, - }; - - testExtension = ExtensionTestUtils.loadExtension(EXTENSION_DATA); - await testExtension.startup(); - testExtension.sendMessage("engage", `${scheme}://test`); - }, - permDialogOptions: { - hasCheckbox: true, - chooserIsNext: true, - hasChangeApp: false, - actionCheckbox: true, - actionConfirm: true, - checkContents: dialogEl => { - let description = dialogEl.querySelector("#description"); - let { id, args } = description.ownerDocument.l10n.getAttributes( - description - ); - is( - id, - "permission-dialog-description-extension", - "Should be using the correct string." - ); - is( - args.extension, - "Generated extension", - "Should have the correct extension name." - ); - }, - }, - chooserDialogOptions: { - hasCheckbox: true, - actionConfirm: false, // Cancel dialog - }, - }); - - let extensionPrincipal = Services.scriptSecurityManager.createContentPrincipal( - Services.io.newURI(`moz-extension://${testExtension.uuid}/`), - {} - ); - - let key = getSkipProtoDialogPermissionKey(scheme); - is( - Services.perms.testPermissionFromPrincipal(extensionPrincipal, key), - Services.perms.ALLOW_ACTION, - "Should have permanently allowed the extension" - ); - is( - Services.perms.testPermissionFromPrincipal(PRINCIPAL1, key), - Services.perms.UNKNOWN_ACTION, - "Should not have allowed the page" - ); - await testExtension.unload(); }); }); -- GitLab