Closed
Bug 1429162
Opened 7 years ago
Closed 7 years ago
Policy: Disable Heartbeat and Shield
Categories
(Firefox :: Enterprise Policies, enhancement, P1)
Firefox
Enterprise Policies
Tracking
()
VERIFIED
FIXED
Firefox 60
Tracking | Status | |
---|---|---|
firefox60 | --- | verified |
People
(Reporter: Felipe, Assigned: Felipe)
References
Details
Attachments
(1 file)
There's a request to disable Heartbeat but it should probably disable Shield entirely.
extensions.shield-recipe-client.enabled will disable all of Shield (which includes Heartbeat).
If we want more fine-grained control, we will need more extensive changes.
Comment 1•7 years ago
|
||
> extensions.shield-recipe-client.enabled
I think this certainly _seems_ inverted, and seems non-intuitive
are you saying:
> pref("extensions.shield-recipe-client.enabled", true);
DISABLES all of Shield ???
Comment 2•7 years ago
|
||
Hi. I'm the tech lead for this feature, and I can help out with any questions you have in this area. I'd also be happy to review the code implementing this feature (though as I'm not a Firefox peer, it will need sign off from a peer as well).
extensions.shield-recipe-client.enabled is the correct preference to disable Shield (including heartbeat). Setting it to `true`, which is the default, enables Shield. To disable shield, the value should be `false`.
ie:
> pref("extensions.shield-recipe-client.enabled", false);
will disable all of Shield.
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to Mike Cooper [:mythmon] from comment #2)
> Hi. I'm the tech lead for this feature, and I can help out with any
> questions you have in this area. I'd also be happy to review the code
> implementing this feature (though as I'm not a Firefox peer, it will need
> sign off from a peer as well).
>
> extensions.shield-recipe-client.enabled is the correct preference to disable
> Shield (including heartbeat). Setting it to `true`, which is the default,
> enables Shield. To disable shield, the value should be `false`.
>
> ie:
>
> > pref("extensions.shield-recipe-client.enabled", false);
>
> will disable all of Shield.
Ok cool, thanks! Some more detailed questions:
- does that include Heartbeat?
- does that correctly disable or hide any UI related to Shield?
- what would be a simple way to test that shield was disabled through it (in a browser-chrome test)?
Flags: needinfo?(mcooper)
Comment 4•7 years ago
|
||
That setting will prevent Shield from executing anything (it won't contact the server).
It does not disable `about:studies`, which shows a list of all studies that Firefox has ever participated in. It also won't remove (or uncheck) the "allow firefox to runs studies" box from about:preferences (setting extensions.shield-recipe-client.enabled to false will however prevent studies from running anyways). There is currently no way to remove about:studies or the changes to about:preferences.
To test this in a browser-chrome test, I would stub out the `run` method of RecipeRunner (in resource://shield-recipe-client/lib/RecipeRunner.jsm), and then call the `init` method of RecipeRunner. If the feature is correctly disabled, `run` will not be called at all.
Flags: needinfo?(mcooper)
Comment 5•7 years ago
|
||
(In reply to Mike Cooper [:mythmon] from comment #2)
I may have led you down to make a wrong statement by my using: 'prefs' vs 'user_prefs' ... is there a difference here?
> see also: https://bugzilla.mozilla.org/show_bug.cgi?id=1425663
Flags: needinfo?(mcooper)
Comment 6•7 years ago
|
||
Ah. After reviewing the code, I believe `user_pref` will be needed. I suppose it depends on how the policy engine works. `pref(...)` in prefs.js won't work, but `user_pref(...)` should
I'm not sure what bug 1425663 is caused by, but I don't think it is related to this.
Flags: needinfo?(mcooper)
Comment 7•7 years ago
|
||
Thank you
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → felipc
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Assignee | ||
Comment 9•7 years ago
|
||
Since the checkbox in about:preferences is not tied to extensions.shield-recipe-client.enabled, but to a different pref (the opt-out pref), I decided to use the policies API instead of prefs to disable shield via policy. That made it more clear in the code what is going on, and also to disable the checkbox in about:preferences.
I'd also love to see if about:studies could show a message saying that shield is disabled (for whatever reason, not just policy), but that's out of scope for me and this bug.
I named the policy DisableFirefoxStudies, because my understanding is that this is the external name of the feature, while Shield means the internal one.
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8948494 [details]
Bug 1429162 - Policy: Disable Shield.
https://reviewboard.mozilla.org/r/217924/#review223734
I didn't know that this kind of approach was possible. I think having an extra check specifically for policy manager is a nice way to do this.
Attachment #8948494 -
Flags: review?(mcooper) → review+
Assignee | ||
Comment 11•7 years ago
|
||
Thanks!
(In reply to :Felipe Gomes (needinfo me!) from comment #9)
> I'd also love to see if about:studies could show a message saying that
> shield is disabled (for whatever reason, not just policy), but that's out of
> scope for me and this bug.
I filed bug 1435838 about this. It'd be nice if that bug got fixed in time for the release of the policy engine (Firefox 60)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 13•7 years ago
|
||
I had forgotten to `hg add` the test file that I wrote for this. I updated the request here: https://reviewboard.mozilla.org/r/217924/diff/1-2/
It's very straightforward so I don't think a new review is necessary, and I'm sending it to try before landing. But feel free to take a look if you have a moment.
Comment 14•7 years ago
|
||
The test file looks good to me. Pretty simple, but it should test the needed things.
Comment 15•7 years ago
|
||
Pushed by felipc@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/05b29220fc4d
Policy: Disable Shield. r=mythmon
Comment 16•7 years ago
|
||
Backed out 2 changesets (bug 1432890, bug 1429162) for failing in dom/canvas/test/webgl-conf/generated/ on a CLOSED TREE
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=05b29220fc4de84e676cf4bfe7a673ae640dd929&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success&filter-resultStatus=pending&filter-resultStatus=running
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=160546226&repo=mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/c48084ca4348e0488537038fbc3ac1b482ad1d4b
Flags: needinfo?(felipc)
Comment 17•7 years ago
|
||
Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b88e2d957837
Policy: Disable Shield. r=mythmon
Comment 18•7 years ago
|
||
Relanded bug 1432890 and bug 1429162 because the failures the backouts attempted to fix had been caused by a different push.
Flags: needinfo?(felipc)
Assignee | ||
Comment 19•7 years ago
|
||
Thanks! I saw that the test failed on test-verify, and I know why.. It's a test problem that is easy to trigger on these policies test, so I'm gonna follow-up on a separate bug to improve something on head.js to avoid this problem in the future
Assignee | ||
Comment 20•7 years ago
|
||
(In reply to :Felipe Gomes (needinfo me!) from comment #19)
> It's a test problem that is easy to trigger on these policies test..
clarifying: not an intermittent problem, it's triggered when the same test runs twice in a row
Comment 21•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Updated•7 years ago
|
status-firefox59:
affected → ---
Comment 22•7 years ago
|
||
We tested "Disable Heartbeat and Shield” policy using JSON file. We verified this manually as fixed.
This bug will also be retested with ADMX files when it is ready for testing.
Test steps and runs are available here: https://testrail.stage.mozaws.net/index.php?/plans/view/7734
Comment 23•7 years ago
|
||
We retested this on beta builds[FX60] with ADM and JSON policy formats and it is verified as fixed.
Test cases and runs are here- https://testrail.stage.mozaws.net/index.php?/plans/view/8760
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•