-
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?
Changes from all commits
d655bac
ab1acbf
51ba54a
6858855
a7de5e8
f6d0e3b
08cbdcd
937c8f2
788d2a1
0f234bf
b1a4836
447f2cb
2e89006
6c73829
1035d4f
49aec8c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| /* Any copyright is dedicated to the Public Domain. | ||
| * http://creativecommons.org/publicdomain/zero/1.0/ */ | ||
| "use strict"; | ||
|
|
||
| const SUPPORT_FILES_PATH = | ||
| "http://mochi.test:8888/browser/browser/components/enterprisepolicies/tests/browser/"; | ||
| const BLOCKED_PAGE = "policy_websitefilter_block.html"; | ||
| const SAVELINKAS_PAGE = "policy_websitefilter_savelink.html"; | ||
|
|
||
| async function clearWebsiteFilter() { | ||
| await setupPolicyEngineWithJson({ | ||
| policies: { | ||
| WebsiteFilter: { | ||
| Block: [], | ||
| Exceptions: [], | ||
| }, | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| add_task(async function test_policy_enterprise_telemetry() { | ||
| await setupPolicyEngineWithJson({ | ||
| policies: { | ||
| WebsiteFilter: `{ | ||
| "Block": ["*://mochi.test/*policy_websitefilter_block*"] | ||
| }`, | ||
| }, | ||
| }); | ||
| await SpecialPowers.pushPrefEnv({ | ||
| set: [ | ||
| ["browser.policies.enterprise.telemetry.testing.disableSubmit", true], | ||
| [ | ||
| "browser.policies.enterprise.telemetry.blocklistDomainBrowsed.enabled", | ||
| true, | ||
| ], | ||
| [ | ||
| "browser.policies.enterprise.telemetry.blocklistDomainBrowsed.urlLogging", | ||
| "full", | ||
| ], | ||
| ], | ||
| }); | ||
|
|
||
| const referrerURL = SUPPORT_FILES_PATH + SAVELINKAS_PAGE; | ||
| const resolvedURL = SUPPORT_FILES_PATH + BLOCKED_PAGE; | ||
| await checkBlockedPageTelemetry(SUPPORT_FILES_PATH + BLOCKED_PAGE); | ||
| await checkBlockedPageTelemetry(SUPPORT_FILES_PATH + BLOCKED_PAGE, { | ||
| referrerURL, | ||
| }); | ||
| await checkBlockedPageTelemetry( | ||
| "view-source:" + SUPPORT_FILES_PATH + BLOCKED_PAGE | ||
| ); | ||
| await checkBlockedPageTelemetry( | ||
| "about:reader?url=" + SUPPORT_FILES_PATH + BLOCKED_PAGE | ||
| ); | ||
|
|
||
| await checkBlockedPageTelemetry(SUPPORT_FILES_PATH + "301.sjs", { | ||
| resolvedURL, | ||
| }); | ||
| await checkBlockedPageTelemetry(SUPPORT_FILES_PATH + "301.sjs", { | ||
| resolvedURL, | ||
| referrerURL, | ||
| }); | ||
|
|
||
| await checkBlockedPageTelemetry(SUPPORT_FILES_PATH + "302.sjs", { | ||
| resolvedURL, | ||
| }); | ||
| await checkBlockedPageTelemetry(SUPPORT_FILES_PATH + "302.sjs", { | ||
| resolvedURL, | ||
| referrerURL, | ||
| }); | ||
|
|
||
| await clearWebsiteFilter(); | ||
| }); | ||
|
|
||
| // Checks that a page was blocked by seeing if it was replaced with about:neterror | ||
| async function checkBlockedPageTelemetry( | ||
| url, | ||
| { resolvedURL, referrerURL } = {} | ||
| ) { | ||
| const expectedBlockedUrl = resolvedURL ?? url; | ||
|
|
||
| let newTab; | ||
| try { | ||
| if (referrerURL) { | ||
| newTab = await BrowserTestUtils.openNewForegroundTab( | ||
| gBrowser, | ||
| referrerURL | ||
| ); | ||
|
|
||
| await SpecialPowers.spawn(newTab.linkedBrowser, [url], async href => { | ||
| let link = content.document.getElementById("savelink_blocked"); | ||
| link.href = href; | ||
| }); | ||
| } else { | ||
| newTab = BrowserTestUtils.addTab(gBrowser); | ||
| gBrowser.selectedTab = newTab; | ||
| } | ||
| let browser = newTab.linkedBrowser; | ||
|
|
||
| let promise = BrowserTestUtils.waitForErrorPage(browser); | ||
| if (referrerURL) { | ||
| await BrowserTestUtils.synthesizeMouseAtCenter( | ||
| "#savelink_blocked", | ||
| {}, | ||
| browser | ||
| ); | ||
| } else { | ||
| BrowserTestUtils.startLoadingURIString(browser, url); | ||
| } | ||
| await promise; | ||
|
|
||
| let events = | ||
| Glean.contentPolicy.blocklistDomainBrowsed.testGetValue("enterprise"); | ||
| Assert.ok(events?.length, "Should have recorded events"); | ||
| if (!events?.length) { | ||
| return; | ||
| } | ||
| Assert.greaterOrEqual(events.length, 1, "Should record at least one event"); // TODO this should eventually be exactly 1 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah this refers to the debouncing/deduplication which this is still missing.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.? |
||
| const event = events.at(-1); | ||
| Assert.ok(event.extra, "Event should have extra data"); | ||
| Assert.equal( | ||
| event.extra.url, | ||
| expectedBlockedUrl, | ||
| "Telemetry should include blocked URL" | ||
| ); | ||
| if (resolvedURL) { | ||
| Assert.equal( | ||
| event.extra.original_url, | ||
| url, | ||
| "Telemetry should include original requested URL" | ||
| ); | ||
| } | ||
| if (referrerURL) { | ||
| Assert.equal( | ||
| event.extra.referrer, | ||
| referrerURL, | ||
| "Telemetry should include referrer URL" | ||
| ); | ||
| } | ||
| } finally { | ||
| if (newTab) { | ||
| BrowserTestUtils.removeTab(newTab); | ||
| } | ||
| Services.fog.testResetFOG(); | ||
| } | ||
| } | ||
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 tohttps://sjeng.org/referrer-hop.html, which is:morbo.orghas a redirect to www.morbo.beThis records
a previous version of this patch was still getting the original referrer right:
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
this records
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.