Skip to content

Commit

Permalink
Bug 1792138: Show the extension's name in permission prompts for open…
Browse files Browse the repository at this point in the history
…ing external links. r=ckerschb,robwu,fluent-reviewers,pbz,flod

In order to handle the content script case correctly we must expose the
contentScriptAddonPolicy to JavaScript. With that we can always see what
extension is trying to perform an action and use its name rather than internal
ID in the dialog.

Differential Revision: https://phabricator.services.mozilla.com/D161282
  • Loading branch information
Mossop committed Dec 13, 2022
1 parent ac2d5ca commit e021cf6
Show file tree
Hide file tree
Showing 9 changed files with 513 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,34 @@ 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"],
Expand All @@ -170,4 +198,13 @@ 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();
});
7 changes: 7 additions & 0 deletions caps/BasePrincipal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1133,6 +1133,13 @@ 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();
Expand Down
2 changes: 2 additions & 0 deletions caps/BasePrincipal.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ 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;
Expand Down
1 change: 1 addition & 0 deletions caps/nsIPrincipal.idl
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,7 @@ 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
Expand Down
13 changes: 13 additions & 0 deletions toolkit/components/extensions/Extension.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,9 @@ 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).
//
Expand Down Expand Up @@ -574,6 +577,16 @@ 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);
Expand Down
10 changes: 10 additions & 0 deletions toolkit/locales/en-US/toolkit/global/handlerDialog.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
## $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?
Expand All @@ -17,6 +18,9 @@ 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 }?
Expand All @@ -26,6 +30,9 @@ 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.
Expand All @@ -36,6 +43,9 @@ 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 =
Expand Down
37 changes: 26 additions & 11 deletions toolkit/mozapps/handling/ContentDispatchChooser.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -293,10 +293,6 @@ class nsContentDispatchChooser {
return false;
}

if (aPrincipal.isAddonOrExpandedAddonPrincipal) {
return true;
}

let key = this._getSkipProtoDialogPermissionKey(scheme);
return (
Services.perms.testPermissionFromPrincipal(aPrincipal, key) ===
Expand Down Expand Up @@ -396,16 +392,28 @@ 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(
aPrincipal,
principal,
permKey,
Services.perms.ALLOW_ACTION,
Services.perms.EXPIRE_NEVER
);
} else {
Services.perms.removeFromPrincipal(aPrincipal, permKey);
Services.perms.removeFromPrincipal(principal, permKey);
}
}

Expand All @@ -415,11 +423,18 @@ class nsContentDispatchChooser {
* @returns {boolean} - true if we can store permissions, false otherwise.
*/
_isSupportedPrincipal(aPrincipal) {
return (
aPrincipal &&
["http", "https", "moz-extension", "file"].some(scheme =>
aPrincipal.schemeIs(scheme)
)
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)
);
}
}
Expand Down
13 changes: 13 additions & 0 deletions toolkit/mozapps/handling/content/permissionDialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ 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;
Expand Down Expand Up @@ -82,6 +84,13 @@ 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";
Expand Down Expand Up @@ -116,6 +125,9 @@ let dialog = {
return null;
}

if (this._addonPolicy) {
return "permission-dialog-remember-extension";
}
if (this._principal.schemeIs("file")) {
return "permission-dialog-remember-file";
}
Expand Down Expand Up @@ -168,6 +180,7 @@ let dialog = {
document.l10n.setAttributes(description, this.l10nDescriptionId, {
host,
scheme,
extension: this._addonPolicy?.name,
appName: this._preferredHandlerName,
});

Expand Down
Loading

0 comments on commit e021cf6

Please sign in to comment.