1398833 - chrome.permissions.request needs to be called directly from input handler, making it impossible to check for permissions first

Status: NEW

(WebExtensions :: Compatibility, defect, P3)

Description

[Aleks Dimitrov]

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170901071738

Steps to reproduce:

Called from the options UI of a webextension, the following code works in Chrome, but not in Firefox:

document.querySelector('#some-element').addEventListener('click', () => {
      const url = 'https://example.com';
      chrome.permissions.contains(
        { origins: [url] },
        (result) => {
          if (!result) {
            chrome.permissions.request(
              { origins: [url] },
              (granted) => {
                if (granted) {
                  console.log('granted permission');
                  );
                }
              },
            );
          }
        });
   });

It should be enough to stick this code in an HTML file, which has a <a id="some-element">click me</a>, and point the manifest to the file:

"options_ui": {
  "page": "options.html"
}


Actual results:

Clicking on #some-element yields the error message:

Unchecked lastError value: Error: May only request permissions from a user input handler

Calling chrome.permissions.request(...) directly from the handler and not putting it in a callback first works.


Expected results:

I'm unsure about the security implications, but the current model disallows requesting permissions if the decision to request them depends on a callback or promise. Chrome seems to have taken a different approach.

Comment 1

[Andrew Swan [:aswan]]

In this particular case, you don't actually need to check for the permission before requesting it, request() will just quietly return true if you request a permission you already have.
From your description, it sounds as if Chrome is propagating the "isHandlingUserInput" state across some asynchronous operations. Rob, do you know exactly what Chrome does here and do you think its worth doing the same?  Note that we have a handful of API methods that check this bit:
http://searchfox.org/mozilla-central/search?q=requireUserInput&case=false&regexp=false&path=.json%24

Comment 2

[Rob Wu [:robwu]]

(In reply to Andrew Swan [:aswan] from comment #1)
> Rob, do you
> know exactly what Chrome does here and do you think its worth doing the
> same?  Note that we have a handful of API methods that check this bit:
> http://searchfox.org/mozilla-central/
> search?q=requireUserInput&case=false&regexp=false&path=.json%24

Chrome propagates the user input flag across callbacks, ever since the patch for https://crbug.com/361116 landed.
It can make sense for us to do so too. I'm not sure whether it is technically feasible to implement this for our promise-based API though.

With callbacks, the desired lifetime of user gestures is obvious (from the start of the callback until return, consumed when an API requiring user input is called).

Is it possible to do something similar for promises (and simultaneously making it impossible to store the promise for later)? (perhaps by forcing the user gesture to expire one second after the promise is resolved?)

bug 1392624 is also relevant (because it also involves transferring the isHandlingUserInput flag across async functions).

Comment 3

[Aleks Dimitrov]

(In reply to Andrew Swan [:aswan] from comment #1)
> In this particular case, you don't actually need to check for the permission
> before requesting it, request() will just quietly return true if you request
> a permission you already have.

Not in my experience, I just tested it again. It re-requests permission for the same domain that the user has already added permissions for. In my addon I'm using promises (the code sample above uses callbacks for simplicity,) but the call to request() is the same.

If you're interested, the code I'm using is here:

https://gitlab.com/adimit/gitlab-time-tracking-button/blob/master/src/ChromeAdapter.js#L13

Note the workaround I'm using for FF at the moment. The permission requesting code is a bit lower:

    return new Promise((resolve, reject) => {
      this.chrome.permissions.request({
        origins: [url],
      }, (granted) => {
        if (granted) {
          resolve(granted);
        } else {
          reject(Error(`Permission for ${url} not granted`));
        }
      });
    });

I don't think this should in principle behave any differently than a plain callback-call to request(). If you think this is a bug, I could file a new one.

Comment 6

[Tian, Sheng]

Since bug 1438465 is duplicated here for "handling user input cross callback". I would prefer comment here instead of file another bug.

bug 1352598 implemented a new API for searching. Extensions may call `search.search(name, searchTerms, tabId)` to perform a searching. While, it is common to pass a newly created tab's id as `tabId` parameter. But `tabs.create` is async function, which means user input status is lost, and `search.search` won't work after it.

Comment 7

[tom]

i can confirm aleks' comments from three years ago while working with 74.0b9:

1. checking whether url(!) permissions are granted via chrome.permissions.contains beforehand cause the infamuous
   'permissions.request may only be called from a user input handler'

2. calling chrome.permissions.request straight for urls works as expected... in google chrome - chrome recognizes that the permission has already been granted when declared in manifest.json#permissions. at its current stage – 74.0b9 – firefox makes me declare the same permission in optional_permissions or it won't grant it; in some situations this requires the use of <all_urls> – something we certainly try to avoid – since we cannot foresee which sites the user may want to access.

may i ask to put this on higher priority?

Comment 8

[Rob Wu [:robwu]]

(In reply to tom from comment #7)

i can confirm aleks' comments from three years ago while working with 74.0b9:

1. checking whether url(!) permissions are granted via chrome.permissions.contains beforehand cause the infamuous
   'permissions.request may only be called from a user input handler'

Not true. The contains method can unconditionally be called.

2. calling chrome.permissions.request straight for urls works as expected... in google chrome - chrome recognizes that the permission has already been granted when declared in manifest.json#permissions. at its current stage – 74.0b9 – firefox makes me declare the same permission in optional_permissions or it won't grant it; in some situations this requires the use of <all_urls> – something we certainly try to avoid – since we cannot foresee which sites the user may want to access.

Requiring a permission to be matched by a value in optional_permissions before it can be requested as an optional permission looks like intended behavior.
The fact that an optional permission is not prompted for if there is already a matching mandatory permission is a bug that tracked at bug 1502987.

may i ask to put this on higher priority?

The current priority looks about right. There is no need to call contains() before request, and removing that call resolves the issue.
Extending the lifetime of the user interaction is somewhat risky, because it can be abused to extend the user interaction for (much) longer than desired.

Comment 9

[tom]

(In reply to Rob Wu [:robwu] from comment #8)

1. checking whether url(!) permissions are granted via chrome.permissions.contains beforehand cause the infamuous
   'permissions.request may only be called from a user input handler'

Not true. The contains method can unconditionally be called.

sorry, i wasn't clear enough: a subsequent call to #request causes the error above, not the call to #contains itself

2. calling chrome.permissions.request straight for urls works as expected... in google chrome - chrome recognizes that the permission has already been granted when declared in manifest.json#permissions. at its current stage – 74.0b9 – firefox makes me declare the same permission in optional_permissions or it won't grant it; in some situations this requires the use of <all_urls> – something we certainly try to avoid – since we cannot foresee which sites the user may want to access.

Requiring a permission to be matched by a value in optional_permissions before it can be requested as an optional permission looks like intended behavior.

even if we have to maintain it in both permissions and optional_permissions in manifest.json?

The fact that an optional permission is not prompted for if there is already a matching mandatory permission is a bug that tracked at bug 1502987.

ah, thank you. so i guess that one needs higher prio ;)

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S4. However, the bug has 3 duplicates.
:rpl, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 12

[Luca Greco [:rpl] [:luca] [:lgreco]]

We discussed about this bug in a pretty recent triage and agreed on setting it as an S4, the duplicates were already known at that time, and so we will keep the S4 set for now.

(But there is still works ongoing around the changes to host permissions behaviors in manifest_version 3 and so I added it as a SeeAlso to the related tracking bug, as something to keep it in mind and consider to bump in priority).

Comment 13

[max]

Inability to call any async function before permission request sounds like a serious obstacle on the route to manifest v3 and service worker.

Consider an add-on that declares some optional permissions and specific permission required to complete user actions depends on configuration. Non-persistent background page or service worker may be unloaded any time. Since browser.storage.local.get() is an async method, it is impossible to conditionally request optional permission from browser.action.onClicked handler. With persistent background page it is not an issue since configuration may be fetched from storage on extension load, so it is almost certainly ready in user action handlers.

I just confirmed the issue with "manifest_version": 3 in Firefox-109. I can post a tiny demo add-on.

Comment 14

[max]

(In reply to Rob Wu [:robwu] from comment #8)

The current priority looks about right. There is no need to call contains() before request, and removing that call resolves the issue.

Since other bugs have been marked as duplicates of this one, I suggest to drop permissions.contains from the title. Please, consider: "No API calls allowed before permissions.request".

Chromium allows the following before permissions.request(), however it requires a trick. API calls must be fired in parallel:

• Load extension settings from storage.local to determine if some permission is necessary.
• Check if all selected tab titles and URLs are available and request "tabs" otherwise.
• Request permissions on demand.

addon-swsp-bg.js

"use strict";
var gConfig = undefined;
var gTabs = undefined;
var gPending = 0;
const DEFAULT_CONFIG = {
	permissions: [ /* "clipboardWrite", */ "nativeMessaging"],
	permissionsOnDemand: true,
};

function swspGetParams(tab) {
	console.log("swspGetParams", tab);
	++gPending;
	chrome.storage.local.get(
		DEFAULT_CONFIG,
		function swspConfigLoaded(config) {
			console.log("swspConfigLoaded", JSON.stringify(config));
			gConfig = config;
			swspCallAction();
		});
	++gPending;
	chrome.tabs.query(
		{ highlighted: true, lastFocusedWindow: true },
		function swspTabsQueryCb(tabs) {
			console.log("swspTabsQueryCb", tabs?.length);
			gTabs = tabs;
			swspCallAction();
		});
}

function swspCallAction() {
	if (--gPending != 0) {
		console.log("swspCallAction pending", gPending);
		return;
	}
	swspAction();
}

async function swspAction() {
	const hasUnavailableTab = gTabs?.some(t => t?.url == null);
	if (gConfig.permissionsOnDemand) {
		const tabsPermission = hasUnavailableTab
			? chrome.permissions.request({ permissions: ["tabs"] }) : null;
		const actionPermission =
			chrome.permissions.request({ permissions: gConfig.permissions });
		const [ tabsGranted, actionGranted ]
			= await Promise.allSettled([ tabsPermission, actionPermission ]);
		console.log("granted", "tabs", tabsGranted, "action", actionGranted);
		if (tabsGranted.value) {
			gTabs = await chrome.tabs.query(
				{ highlighted: true, lastFocusedWindow: true });
		}
	}
	console.table(gTabs.map(t => ({ id: t?.id, url: t?.url, title: t?.title})));
	// Either copy to clipboard or pass tabs to a native host application.
}

chrome.action.onClicked.addListener(tab => void swspGetParams(tab));

manifest.json

{
	"manifest_version": 3,
	"name": "perm in cb",
	"version": "0.1",
	"action": { "default_title": "Permissions request after API calls" },
	"background": { "scripts": [ "addon-swsp-bg.js" ], "service_worker": "addon-swsp-bg.js" },
	"permissions": [ "storage", "activeTab" ],
	"optional_permissions": [ "clipboardWrite", "nativeMessaging", "tabs" ],
	"description": "FF Bug #1398833: no async/callback API method allowed before permissions.request"
}

storage is not a blocker for mv2 add-ons with persistent background page since settings may be loaded with background scripts, not in response to an event.

Comment 16

[Rob Wu [:robwu]]

Copying relevant discussion from bug 1800401:

(Comment by Simeon Vincent [:dotproto] from comment #6 at bug 1800401)

The same underlying issue is still present in other async event handlers, such as situations where a value needs to be retrieved from storage before calling permissions.request() or interacting with the system clipboard.

To my knowledge the primary concern with passing a user input flag down a promise chain is that this could enable abuse scenarios where an extension holds onto a promise with a valid user interaction and consume it later without the user's knowledge. I think the most obvious way to mitigate this kind of abuse is to make a user interaction invalid a short amount of time after the input. For example, a user input flag might be consumable for up to 200 millisecond after the user initiates the interaction.

The main drawback to this approach is that a fixed time limit may feel arbitrary and unpredictable for extension developers. For example, some browser.storage operations will complete within this time period while others will not. External factors may also impact whether or not a promise chain resolves in time, such as lower powered devices or resource contention on higher power systems.

A fixed lifetime isn't ideal, but at the moment it strikes me as the most practical solution for a material problem facing developers.

(Comment by Rob Wu [:robwu] from comment #7 at bug 1800401)

Nowadays the web platform defines the concept of "transient user activation", which is a short-lived period of time where an API gated by user activation can be used. Some APIs consume the transient user activation upon invocation.

Documentation: https://developer.mozilla.org/en-US/docs/Web/Security/User_activation#transient_activation

The recommended value for the timeout is specified to be "at most a few seconds", and in practice Firefox uses 5 seconds in Firefox: https://searchfox.org/mozilla-central/rev/5756c5a3dea4f2896cdb3c8bb15d0ced5e2bf690/modules/libpref/init/StaticPrefList.yaml#4356-4360

I think that we can consider integrating the concept of user activation in the extension framework to support this use case more broadly.

Comment 18

[max]

One more observation on Chromium vs. Firefox difference. Consider

chrome.storage.local.get({config: 1}, v => console.log("storage", v));
chrome.action.onClicked.addListener(tab => console.log("action", tab));

User clicks add-on action button when service worker/background page is stopped. In Chromium-126 storage callback is executed before action one. In Firefox-115.13 action callback is invoked before storage one. As a result, in Chromium it is possible to make decisions based on stored user settings before permissions.request (or other API methods requiring user interaction) while in Firefox it does not work.

Comment 19

[Rob Wu [:robwu]]

We are going to look into the feasibility of extending the duration of validity for user interaction.

Next step is to raise this topic in the WECG to align and specify the desired behavior.

Comment 20

[Rob Wu [:robwu]]

(In reply to max from comment #18)

One more observation on Chromium vs. Firefox difference. Consider

chrome.storage.local.get({config: 1}, v => console.log("storage", v));
chrome.action.onClicked.addListener(tab => console.log("action", tab));

User clicks add-on action button when service worker/background page is stopped. In Chromium-126 storage callback is executed before action one. In Firefox-115.13 action callback is invoked before storage one. As a result, in Chromium it is possible to make decisions based on stored user settings before permissions.request (or other API methods requiring user interaction) while in Firefox it does not work.

This is off-topic, but I'd like to clarify this misconception. Chrome does not guarantee the order as you described. The behavior you observed is accidental and not consistently reproducible. If the physical storage is slow, it is very likely for the database operation (to look up the stored information) delays the callback until after the action.onClicked listener is invoked.

Let's not add additional off-topic comments here, to make it easier for people to check the status of this bug.

Comment 21

[max]

(In reply to Rob Wu [:robwu] from comment #20)

This is off-topic

Sorry, I failed to express the idea. If there were guarantee that some API calls, fired during loading of event page/service worker, are completed before invocation of event handlers then the issue would be significantly alleviated. I mean purely local API methods like storage.local.get that do not rely on message exchanging with other add-on components. Loading extension preferences is more close to initialization than to reaction on some event. This approach is not the best one to allow tabs.query and similar functions that are part of event handling. Perhaps reliable initialization is harder to implement. On the other hand timeout has some shortcoming as well (Bug #1838845).

Indeed you are right that callback of storage.local.get fired from top-level script may be invoked later than action.onClicked listener and without user action gesture, but my attempt to break example from comment 14 in a similar way has failed (in Chromium).