Closed Bug 1429162 Opened 7 years ago Closed 7 years ago

Policy: Disable Heartbeat and Shield

Categories

(Firefox :: Enterprise Policies, enhancement, P1)

enhancement

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.
> 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 ???
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.
(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)
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)
(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)
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)
Thank you
Assignee: nobody → felipc
Status: NEW → ASSIGNED
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 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+
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)
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.
The test file looks good to me. Pretty simple, but it should test the needed things.
Relanded bug 1432890 and bug 1429162 because the failures the backouts attempted to fix had been caused by a different push.
Flags: needinfo?(felipc)
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
(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
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
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
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.

Attachment

General

Created:
Updated:
Size: