Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Details for the webRequestAuthProvider permission #30188

Merged
merged 10 commits into from
Dec 11, 2023

Conversation

rebloor
Copy link
Contributor

@rebloor rebloor commented Nov 10, 2023

Description

  • Adds the webRequestAuthProvider permission to the permissions manifest key.
  • Add a note about support for webRequestAuthProvider permission in Chrome to webRequest.onAuthRequired
  • various edits to webRequest.onAuthRequired

Motivation

Provide documentation for the Chrome permission.

Additional details

BCD provided in mdn/browser-compat-data#21216.

Related issues and pull requests

Fixes #21586

@rebloor rebloor added the Content:WebExt WebExtensions docs label Nov 10, 2023
@rebloor rebloor self-assigned this Nov 10, 2023
@rebloor rebloor requested a review from a team as a code owner November 10, 2023 17:39
@rebloor rebloor requested review from jpmedley and removed request for a team November 10, 2023 17:39
Copy link
Contributor

github-actions bot commented Nov 10, 2023

Preview URLs

External URLs (3)

URL: /en-US/docs/Mozilla/Add-ons/WebExtensions/API/webRequest/onAuthRequired
Title: webRequest.onAuthRequired

(comment last updated: 2023-12-11 22:01:19)

Copy link
Collaborator

@jpmedley jpmedley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a nice bit of editing. You made some fixes most don't even know to do. I have one nit, but it's kind of an important one. We put compatibility notes in the data itself so that it is available to client applications.


See [Examples](#examples).

If you use `"blocking"` you must have the ["webRequestBlocking" API permission](/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/permissions#api_permissions) in your `manifest.json`.

If your extension provides bad credentials, then the listener will be called again. For this reason, take care not to enter an infinite loop by repeatedly providing bad credentials.
> **Note:** In Chrome, as of Manifest V3, the "webRequestBlocking" permission is no longer available for most extensions (the exception being policy-installed extensions). However, as of Chrome 108, using the "webRequest" and "webRequestAuthProvider" permissions enables you to supply credentials asynchronously.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This information needs to go in the browser compatibility data. Here's an example of how to do it. Let me know if you need help.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the complex requirements and non-obvious usage, I'm in favor of including a mention of how/when these can be used. See my larger review comment. The version can be dropped from the text (and kept in the BCD), as 108 is multiple releases ago and not relevant to most. Just "MV2" vs "MV3" should be clear enough.

Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much more is needed before this article is Chrome-compatible. This is mainly due to incompatibilities. Below I am only mentioning Firefox and Chrome because Safari doesn't support the webRequest API at all.

On the permission requirement:

  • Firefox: webRequest + webRequestBlocking. webRequestAuthProvider is not supported, but we have a bug tracking that at https://bugzilla.mozilla.org/show_bug.cgi?id=1820569
  • Chrome: webRequest + webRequestBlocking (MV2 only) OR webRequest + webRequestAuthProvider (MV2 and MV3)

On registering the event, to handle the event synchronously - this is the same across browsers (unfortunately, only viable for the simplest of extensions).

  • Firefox and Chrome: extraInfoSpec array must include "blocking", and the event handler function must return the BlockingResponse directly. Note that it does not count if async function is used instead of a function, because that automatically turns the function into a function that returns a Promise. That works in Firefox (see below), but not in Chrome.

Now the more difficult part happens when the event should be handled asynchronously. Support is inconsistent and Chrome's documentation is very lacking there (seems that there is an open bug at crbug.com/693286 to improve that?).

  • Firefox: extraInfoSpec array must include "blocking", and the event handler function can return a Promise that resolves to a BlockingResponse object. Note that the usage is basically the same as the synchronous event handler case.
  • Chrome: extraInfoSpec array must include "asyncBlocking" (without "blocking"!). The event handler function is passed a second parameter (called asyncCallback) that should be invoked with the BlockingResponse result.

EDIT: FYI I filed w3c/webextensions#490 to seek alignment between Firefox and Chrome.


See [Examples](#examples).

If you use `"blocking"` you must have the ["webRequestBlocking" API permission](/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/permissions#api_permissions) in your `manifest.json`.

If your extension provides bad credentials, then the listener will be called again. For this reason, take care not to enter an infinite loop by repeatedly providing bad credentials.
> **Note:** In Chrome, as of Manifest V3, the "webRequestBlocking" permission is no longer available for most extensions (the exception being policy-installed extensions). However, as of Chrome 108, using the "webRequest" and "webRequestAuthProvider" permissions enables you to supply credentials asynchronously.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the complex requirements and non-obvious usage, I'm in favor of including a mention of how/when these can be used. See my larger review comment. The version can be dropped from the text (and kept in the BCD), as 108 is multiple releases ago and not relevant to most. Just "MV2" vs "MV3" should be clear enough.

rebloor and others added 2 commits November 16, 2023 14:33
Co-authored-by: Joe Medley <[email protected]>
Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: the thread on the urlclassier was resolved. One of my remarks was that it should be consistent across all webRequest events. Are you going to create a separate PR for that?

@rebloor
Copy link
Contributor Author

rebloor commented Nov 30, 2023

Note: the thread on the urlclassier was resolved. One of my remarks was that it should be consistent across all webRequest events. Are you going to create a separate PR for that?

@Rob--W as it's tangential I've created Clarify fingerprinting and fingerprinting_content #30644 – I have an unwound the grammatical improvements, I'll apply those to the other events when I address the issue.

@rebloor rebloor requested a review from Rob--W November 30, 2023 02:51
Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+ with 2 minor changes.


- in addListener, pass `"blocking"` in the `extraInfoSpec` parameter
- in the listener, return a `Promise` that is resolved with an object containing an `authCredentials` property, set to the credentials to supply
- in the listener, return a `Promise` that resolves with an object containing an `authCredentials` property, set to the credentials to supply

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[mdn-linter] reported by reviewdog 🐶

Suggested change

@rebloor rebloor merged commit 789f5df into mdn:main Dec 11, 2023
7 checks passed
@rebloor rebloor deleted the webRequestAuthProvider-permission branch December 11, 2023 22:03
dipikabh pushed a commit to dipikabh/content that referenced this pull request Jan 17, 2024
* Details for the webRequestAuthProvider permission

* Feedback update

* Review feedback

Co-authored-by: Joe Medley <[email protected]>

* Apply suggestions from review

Co-authored-by: Joe Medley <[email protected]>

* Feedback from review

Co-authored-by: Rob Wu <[email protected]>

* Apply suggestions from review

Co-authored-by: Rob Wu <[email protected]>

* Adjust formatting

---------

Co-authored-by: Joe Medley <[email protected]>
Co-authored-by: Rob Wu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebExt WebExtensions docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update webRequest.onAuthRequired extraInfoSpec with Chrome webRequestAuthProvider
3 participants