-
Notifications
You must be signed in to change notification settings - Fork 9
Blocked domain telemetry: Add referrer and original URL for redirects #307
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
base: enterprise-main
Are you sure you want to change the base?
Conversation
Record referrer (when available) in the enterprise blocklist_domain_browsed Glean event and update the WebsiteFilter mochitest to flush FOG and use a referrer-producing navigation.
When referrerInfo is missing/empty for blocked navigations, fall back to the associated window document URI to populate the enterprise blocklist_domain_browsed referrer extra.
browser/components/enterprisepolicies/tests/browser/browser_policy_websitefilter_telemetry.js
Outdated
Show resolved
Hide resolved
| if (!events?.length) { | ||
| return; | ||
| } | ||
| Assert.greaterOrEqual(events.length, 1, "Should record at least one event"); // TODO this should eventually be exactly 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah this refers to the debouncing/deduplication which this is still missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I planned to implement the debouncing/deduplication in a separate PR. I don't know if we have a policy around landing TODOs into the code base and/or tagging them with links to bugs to track, etc.?
| type: string | ||
| original_url: | ||
| description: > | ||
| The original url, prior to redirects, that was requested resulting in a block, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, for myself I set a block on morbo.be, then navigate to https://sjeng.org/referrer-hop.html, which is:
document.getElementById("go").addEventListener("click", () => {
location.href = "http://morbo.org/";
});
morbo.org has a redirect to www.morbo.be
This records
"events": [
{
"category": "content_policy",
"extra": {
"glean_timestamp": "1766755855522",
"original_url": "http://morbo.org/",
"referrer": "",
"url": "http://www.morbo.be/"
},
"name": "blocklist_domain_browsed",
"timestamp": 0
}
],
a previous version of this patch was still getting the original referrer right:
"events": [
{
"category": "content_policy",
"extra": {
"glean_timestamp": "1766060860297",
"referrer": "https://sjeng.org/referrer-hop.html",
"url": "http://www.morbo.be/"
},
"name": "blocklist_domain_browsed",
"timestamp": 0
}
],
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added https://sjeng.org/referrer-hop-direct.html, which is
document.getElementById("go").addEventListener("click", () => {
location.href = "http://www.morbo.be/";
});
this records
"events": [
{
"category": "content_policy",
"extra": {
"glean_timestamp": "1766756680769",
"referrer": "",
"url": "http://www.morbo.be/"
},
"name": "blocklist_domain_browsed",
"timestamp": 0
}
],
So the referrer isn't detected correctly (...any more).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In an offline discussion we clarified that the lack of referrer in this case is a result of the choice to use the "computed referrer", i.e. matching what is in the "Referer" header after applying the referrer-policy. In these scenarios, the block is happening while executing an http request from an https referrer, so the policy has stripped the referrer.
It is technically easy to instead use the original referrer (pre-policy), so we're left with a judgment call of which is the correct data to include. I'll pursue that question and update here accordingly.
| if (!events?.length) { | ||
| return; | ||
| } | ||
| Assert.greaterOrEqual(events.length, 1, "Should record at least one event"); // TODO this should eventually be exactly 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah this refers to the debouncing/deduplication which this is still missing.
shouldLoadto the caller in C++ that has access to the IHttpChannel with referrer informationobservecallback when it blocks a redirected URL.urltoblocked_urland in the case of a redirect, include anoriginal_urlfield.