Closed Bug 1787379 Opened 2 years ago Closed 8 months ago

Support MessageSender.origin for extension messages

Categories

(WebExtensions :: General, enhancement, P3)

Firefox 104
enhancement

Tracking

(firefox126 fixed)

RESOLVED FIXED
126 Branch
Tracking Status
firefox126 --- fixed

People

(Reporter: y, Assigned: willdurand, Mentored)

References

Details

(Keywords: dev-doc-complete, good-first-bug, Whiteboard: [addons-jira])

Attachments

(1 file)

Steps to reproduce:

  1. Install an extension with a background page and content script.
  2. Register a browser.runtime.onMessage listener on the background page, e.g., browser.runtime.onMessage.addListener(console.log).
  3. Send a message from the content script, e.g., browser.runtime.sendMessage({}).
  4. Check the received second argument (MessageSender).

Actual results:

The MessageSender does not include the origin property.

Expected results:

The MessageSender should include the origin property.

From https://crbug.com/987014:

Identification of the sender of an extension message is useful in scenarios where content scripts should be restricted in what they can ask for. For example - if an extension stores per-origin passwords or other per-origin secrets, then a content script running in foo.com page should not be able to ask for passwords/secrets of bar.com.

Extension background page that handles messages from content scripts (see chrome.runtime.onMessage) is able to identify some properties of the webpage that the message was sent from: url, frame id, tab id (these are the properties of MessageSender - see https://developers.chrome.com/extensions/runtime#type-MessageSender).

The current properties of MessageSender are insufficient to identify the origin of an about:blank or about:srcdoc page (in theory the frame id could be used to lookup the frame, but this is very indirect and moreover is subject to races).

Summary: Support MessageSender.origin → Support MessageSender.origin for extension messages
Component: Untriaged → General
Product: Firefox → WebExtensions
Mentor: rob
Severity: -- → N/A
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Whiteboard: [addons-jira]

I see this is labeled as a good first bug. I've used the Web Extension API for a school project so this seems like a good place to start with open source.

I've read the onramp docs and I'm working on getting the development environment. Is there anything else to know as I get started?

Hi Shane, comment 1 provides pointers. Don't hesitate asking questions if you have them.

I am stuck on this, source doesn't appear to have a property for origin to add to the sender.

Flags: needinfo?(rob)

(In reply to Shane Panchot from comment #4)

I am stuck on this, source doesn't appear to have a property for origin to add to the sender.

source holds many properties. I suggest to attach the debugger, put a breakpoint at the line of interest, and then call the function (e.g. browser.runtime.sendMessage("x") via an extension, whether via a separate devtools console or code within the extension). By doing so, the breakpoint will be triggered and you can inspect the available variables to get an idea of what you can use.

To learn more about using the Browser Toolbox, see https://firefox-source-docs.mozilla.org/devtools-user/browser_toolbox/index.html

The "origin" concept is often similar to the URL of a document, but may sometimes differ in case of documents whose origin is inherit rather than derived from the URL (e.g. about:blank, about:srcdoc, blob:). In Firefox's code, the "origin" is often recorded in the "principal".
In the code that I linked in comment 1, you see a way to get browsingContext at some point. This object indirectly offers the principal, e.g. through browsingContext.currentWindowContext.documentPrincipal . The nsIPrincipal interface describing principals specifies the originNoSuffix attribute, so you should be able to get the information you need in this way.

Flags: needinfo?(rob)

Hi Rob,

I was able to pick this back up again, sorry for the long period of silence. Also, thanks for the tip on how to get to the Browser Toolbox, the first attempt I made I couldn't figure out the debugger and was just trying to log the source from a test case which provided nowhere near the same level of detail.

I'm now setting origin as a sender property with source.actor.browsingContext.currentWindowContext.documentPrincipal.origin but I can't seem to create a scenario where the values are significantly different:

Clicking the browser action from about:blank or web page

origin: "moz-extension://c40a2d2f-69b4-42e8-8354-6767bd77ebbc"
url: "moz-extension://c40a2d2f-69b4-42e8-8354-6767bd77ebbc/popup/pointfinder-popup.html"

Triggering from a content script:

origin: "https://www.adidas.com"
url: "https://www.adidas.com/us"

From what I'm reading on content scripts, it doesn't seem like you would ever be able to have a content script run from something like about:blank because it doesn't qualify for a match? Content Script Doc I reviewed

I think I'm ready to start working on the test cases, but first I wanted to check in and confirm the origin is what is expected. I also want to confirm that no test case would be needed for about:blank and that a message could never be sent from there?

Flags: needinfo?(rob)

(In reply to Shane Panchot from comment #6)

I'm now setting origin as a sender property with source.actor.browsingContext.currentWindowContext.documentPrincipal.origin but I can't seem to create a scenario where the values are significantly different:

Use originNoSuffix as I suggested before. Test case with different output: open a private browsing window or a container tab.

From what I'm reading on content scripts, it doesn't seem like you would ever be able to have a content script run from something like about:blank because it doesn't qualify for a match? Content Script Doc I reviewed

Content scripts with "match_about_blank": true (https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/content_scripts#match_about_blank) can run on about:blank (alternative: tabs.executeScript API with matchAboutBlank: true). Even in these cases, the origin is the origin of the document that created the about:blank frame/window.

Flags: needinfo?(rob)

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

Thank you for getting back to me so quickly.

Use originNoSuffix as I suggested before. Test case with different output: open a private browsing window or a container tab.

I saw that link and the comment on the property said it was only for legacy situations and referenced the full origin having some important in-variants so that is partly why I wanted confirmation I had the right one.

Content scripts with "match_about_blank": true (https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/content_scripts#match_about_blank) can run on about:blank (alternative: tabs.executeScript API with matchAboutBlank: true). Even in these cases, the origin is the origin of the document that created the about:blank frame/window.

Got it, can't believe I missed that. Between this and the private browsing example, I feel like I'm starting to get a much better idea of what I'm working with here and I've observed the situation where they would be different. This is my first introduction to a bunch of new concepts. I've never looked this deep into the inner-workings of a browser or even a code base of this size. Thank you for your patience and providing the links.

For the tests, I'm looking at the on-ramp document and see three categories of test: xpcshell, mochitest, and browser-chrome. From the links you provided for existing test cases, I don't see any tests in the xpcshell directory. Can you confirm that I don't need to create any xpcshell tests? From the test you did send, I think I'm getting a decent idea of how the browser and mochitest ones are used and will start there. Hopefully I'll have a patch for you to review soon.

(In reply to Shane Panchot from comment #8)

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

Thank you for getting back to me so quickly.

Use originNoSuffix as I suggested before. Test case with different output: open a private browsing window or a container tab.

I saw that link and the comment on the property said it was only for legacy situations and referenced the full origin having some important in-variants so that is partly why I wanted confirmation I had the right one.

The invariant you're referring is "principalA.equals(principalB) if and only if principalA.origin == principalB.origin". This matters if the comparison is used for security- or privacy-sensitive decisions. For example, in the context of reading cookies, even if the "originNoSuffix" is identical (e.g. https://example.com), the implementation should take care to not inadvertently allow that website in private browsing mode (origin attributes privateBrowsingId=1) to read from non-private browsing mode.

This is not relevant in the extension API. In the API, origin is meant to be the URL.
There are other fields on MessageSender that allows the extension to isolate contexts:

  • sender.tab.incognito - detect private browsing vs regular browsing mode.
  • sender.tab.cookieStoreId - to discern container tabs (can also be used to detect private browsing mode).

For the tests, I'm looking at the on-ramp document and see three categories of test: xpcshell, mochitest, and browser-chrome. From the links you provided for existing test cases, I don't see any tests in the xpcshell directory. Can you confirm that I don't need to create any xpcshell tests? From the test you did send, I think I'm getting a decent idea of how the browser and mochitest ones are used and will start there. Hopefully I'll have a patch for you to review soon.

The concept of a "tab" requires a browser environment, and is not implemented in xpcshell tests. I would recommend creating a mochitest.

See Also: → 1628178
Assignee: nobody → wdurand
Status: NEW → ASSIGNED
Pushed by wdurand@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/44d2d919f8ef Add `origin` to `runtime.MessageSender`. r=robwu,zombie,geckoview-reviewers
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → 126 Branch

Documentation updates are ready for review in

Blocks: 1891693
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: