1612979 - Enable locking of auto update setting

Status: VERIFIED FIXED

(Toolkit :: Application Update, enhancement)

Description

[Robin Steuber (she/her) [:bytesized]]

The automatic update setting has a bit of a complicated implementation that no longer strictly uses the pref system. Instead, the correct interface to interact with the setting is UpdateUtils.(get|set)AppUpdateAutoEnabled(). This interface currently does not allow the setting to be locked. I would like to add this functionality to enable an enterprise policy forcing automatic updates to be enabled.

Comment 1

[Robin Steuber (she/her) [:bytesized]]

I was going to do this by adding a locking mechanism for this setting that the policy could be hooked up to, but I have now realized that, on Windows, you could just open the file and edit the value, which isn't a great enforcement mechanism for an enterprise policy. So I am changing my plan to instead just query the policy engine directly.

Comment 2

[Robin Steuber (she/her) [:bytesized]]

Attachment: auto_update_setting.png

While changing the code that controls the update settings on the preferences page, I noticed a bug that I intend to include a fix for in this patch (screenshot of bug attached). I had thought that we locked this setting when update is disabled by policy, but it turns out that we hide it. But we don't hide the Windows-specific notification about how the setting works. So the notification is shown, referring to a preference that is not.

Given that this is a one-line change in an area of code that I'm already changing with this patch, I think it is acceptable to skip filing a separate bug for this.

Comment 3

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1612979 - Add AppAutoUpdate enterprise policy r=mkaply

This adds the policy to the policy engine, but does not add the functionality, which will be done in another patch in this stack.

Comment 4

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1612979 - Remove app.update.auto from the list of preferences that can be manipulated by policy r=mkaply

This functionality is being replaced by the policy: AppAutoUpdate

Depends on D63978

Comment 5

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1612979 - Add functionality to UpdateUtils for automatic app update to be locked by policy r=mhowell,mkaply

Depends on D63979

Comment 6

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1612979 - Have the about:preferences UI respect locked automatic application update r=jaws

Depends on D63980

Comment 7

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1612979 - Fix bug where message about a prefrence is shown when the preference itself is hidden r=jaws

Depends on D63981

Comment 8

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1612979 - Fix "auto-update-config-change" notification bugs r=mhowell

Includes fixes for the following bugs:

• "auto-update-config-change" notification is never sent because it never updates its cached value so it always thinks the value is uninitialized, at state in which it does not send the notification.
• maybeUpdateAutoConfigChanged doesn't send the first notification because the value hasn't changed yet, just been set. But this is only true on Windows. On other OS's, the function is only called when the value changes.
• If a non-Windows user changes the app.update.auto pref value, it effectively changes the value of the auto update setting but doesn't generate a "auto-update-config-change" notification.
• Minor cleanup of an unnecessary bind call.

Depends on D63982

Comment 9

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1612979 - Add tests for the AppAutoUpdate policy r=mkaply

Depends on D63983

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69cd81740888
Add AppAutoUpdate enterprise policy r=mkaply,fluent-reviewers,flod
https://hg.mozilla.org/integration/autoland/rev/67b182d75120
Remove app.update.auto from the list of preferences that can be manipulated by policy r=mkaply
https://hg.mozilla.org/integration/autoland/rev/935ee9393b16
Add functionality to UpdateUtils for automatic app update to be locked by policy r=mhowell,mkaply
https://hg.mozilla.org/integration/autoland/rev/c5223f21be3d
Have the about:preferences UI respect locked automatic application update r=jaws
https://hg.mozilla.org/integration/autoland/rev/975a2ced2917
Fix bug where message about a prefrence is shown when the preference itself is hidden r=jaws
https://hg.mozilla.org/integration/autoland/rev/8162219895ea
Fix "auto-update-config-change" notification bugs r=mhowell
https://hg.mozilla.org/integration/autoland/rev/b65ab84f5403
Add tests for the AppAutoUpdate policy r=mkaply

Comment 11

[Daniel Varga [:dvarga]]

Backed out 7 changesets (Bug 1612979) for causing browser chrome failure at browser/disable_app_update/browser_policy_disable_app_update.js

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=291467147&searchStr=linux%2C18.04%2Cx64%2Cdebug%2Cmochitests%2Cwith%2Cfission%2Cenabled%2Ctest-linux1804-64%2Fdebug-mochitest-browser-chrome-fis-e10s-11%2Cm-fis%28bc11%29&revision=b65ab84f5403f122a4c5a42fbab60391cec055f0

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=291467147&repo=autoland&lineNumber=16547

Backout link: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=291467147&searchStr=linux%2C18.04%2Cx64%2Cdebug%2Cmochitests%2Cwith%2Cfission%2Cenabled%2Ctest-linux1804-64%2Fdebug-mochitest-browser-chrome-fis-e10s-11%2Cm-fis%28bc11%29&revision=634719f9e5db1cce3bcb0da8f4251277ca3044f3

[task 2020-03-03T17:17:24.161Z] 17:17:24     INFO - TEST-PASS | browser/components/enterprisepolicies/tests/browser/disable_app_update/browser_policy_disable_app_update.js | Should not be able to check for updates with DisableAppUpdate enabled. - 
[task 2020-03-03T17:17:24.163Z] 17:17:24     INFO - Leaving test bound test_updates_post_policy
[task 2020-03-03T17:17:24.163Z] 17:17:24     INFO - Entering test bound test_update_preferences_ui
[task 2020-03-03T17:17:24.166Z] 17:17:24     INFO - Buffered messages finished
[task 2020-03-03T17:17:24.168Z] 17:17:24     INFO - TEST-UNEXPECTED-FAIL | browser/components/enterprisepolicies/tests/browser/disable_app_update/browser_policy_disable_app_update.js | Update choices should be diabled when app update is locked by policy - false == true - got false, expected true (operator ==)
[task 2020-03-03T17:17:24.168Z] 17:17:24     INFO - Stack trace:
[task 2020-03-03T17:17:24.169Z] 17:17:24     INFO - is@resource://specialpowers/SpecialPowersSandbox.jsm:89:21
[task 2020-03-03T17:17:24.169Z] 17:17:24     INFO - @chrome://mochitests/content/browser/browser/components/enterprisepolicies/tests/browser/disable_app_update/browser_policy_disable_app_update.js:32:7
[task 2020-03-03T17:17:24.170Z] 17:17:24     INFO - execute@resource://specialpowers/SpecialPowersSandbox.jsm:140:12
[task 2020-03-03T17:17:24.170Z] 17:17:24     INFO - _spawnTask@resource://specialpowers/SpecialPowersChild.jsm:1727:15
[task 2020-03-03T17:17:24.170Z] 17:17:24     INFO - receiveMessage@resource://specialpowers/SpecialPowersChild.jsm:285:21
[task 2020-03-03T17:17:24.171Z] 17:17:24     INFO - JSWindowActor query*receiveMessage@resource://specialpowers/SpecialPowersParent.jsm:1055:12
[task 2020-03-03T17:17:24.171Z] 17:17:24     INFO - JSWindowActor query*spawn@resource://specialpowers/SpecialPowersChild.jsm:1682:17
[task 2020-03-03T17:17:24.171Z] 17:17:24     INFO - test_update_preferences_ui@chrome://mochitests/content/browser/browser/components/enterprisepolicies/tests/browser/disable_app_update/browser_policy_disable_app_update.js:29:23
[task 2020-03-03T17:17:24.172Z] 17:17:24     INFO - Async*Tester_execTest/<@chrome://mochikit/content/browser-test.js:1062:34
[task 2020-03-03T17:17:24.172Z] 17:17:24     INFO - async*Tester_execTest@chrome://mochikit/content/browser-test.js:1097:11
[task 2020-03-03T17:17:24.173Z] 17:17:24     INFO - nextTest/<@chrome://mochikit/content/browser-test.js:925:14
[task 2020-03-03T17:17:24.173Z] 17:17:24     INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:914:23
[task 2020-03-03T17:17:24.174Z] 17:17:24     INFO - GECKO(4206) | [Parent 4206, Main Thread] WARNING: '!inner', file /builds/worker/workspace/build/src/dom/ipc/JSWindowActorService.cpp, line 182
[task 2020-03-03T17:17:24.178Z] 17:17:24     INFO - Leaving test bound test_update_preferences_ui
[task 2020-03-03T17:17:24.178Z] 17:17:24     INFO - Entering test bound test_update_about_ui
[task 2020-03-03T17:17:24.178Z] 17:17:24     INFO - GECKO(4206) | [Parent 4206: Main Thread]: I/DocShellAndDOMWindowLeak ++DOCSHELL 0x7fd006997000 == 9 [pid = 4206] [id = {ed1363b6-b428-41eb-bf63-cbd71e9b94d2}]
[task 2020-03-03T17:17:24.178Z] 17:17:24     INFO - GECKO(4206) | [Parent 4206: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 19 (0x7fd01136bde0) [pid = 4206] [serial = 19] [outer = (nil)]
[task 2020-03-03T17:17:24.178Z] 17:17:24     INFO - GECKO(4206) | [Parent 4206: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 20 (0x7fd007b5cc00) [pid = 4206] [serial = 20] [outer = 0x7fd01136bde0]
[task 2020-03-03T17:17:24.178Z] 17:17:24     INFO - GECKO(4206) | [Parent 4206, Main Thread] WARNING: '!aUseRemoteTabs && mUseRemoteSubframes', file /builds/worker/workspace/build/src/docshell/base/nsDocShell.cpp, line 1581
[task 2020-03-03T17:17:24.179Z] 17:17:24     INFO - GECKO(4206) | [Parent 4206, Main Thread] WARNING: Attempting to get a displayport from a content with no primary frame!: file /builds/worker/workspace/build/src/layout/base/nsLayoutUtils.cpp, line 791
[task 2020-03-03T17:17:24.180Z] 17:17:24     INFO - GECKO(4206) | [Parent 4206, Main Thread] WARNING: NS_ENSURE_TRUE(root) failed: file /builds/worker/workspace/build/src/layout/base/nsDocumentViewer.cpp, line 3137
[task 2020-03-03T17:17:24.180Z] 17:17:24     INFO - Console message: [JavaScript Warning: "XUL box for hbox element contained an inline #text child, forcing all its children to be wrapped in a block." {file: "chrome://browser/content/aboutDialog.xhtml" line: 0}]
[task 2020-03-03T17:17:24.183Z] 17:17:24     INFO - Console message: [JavaScript Warning: "XUL box for hbox element contained an inline #text child, forcing all its children to be wrapped in a block." {file: "chrome://browser/content/aboutDialog.xhtml" line: 0}]
[task 2020-03-03T17:17:24.183Z] 17:17:24     INFO - Console message: [JavaScript Warning: "XUL box for hbox element contained an inline #text child, forcing all its children to be wrapped in a block." {file: "chrome://browser/content/aboutDialog.xhtml" line: 0}]
[task 2020-03-03T17:17:24.184Z] 17:17:24     INFO - Console message: [JavaScript Warning: "XUL box for hbox element contained an inline #text child, forcing all its children to be wrapped in a block." {file: "chrome://browser/content/aboutDialog.xhtml" line: 0}]
[task 2020-03-03T17:17:24.184Z] 17:17:24     INFO - TEST-PASS | browser/components/enterprisepolicies/tests/browser/disable_app_update/browser_policy_disable_app_update.js | About dialog appeared - 
[task 2020-03-03T17:17:24.264Z] 17:17:24     INFO - TEST-PASS | browser/components/enterprisepolicies/tests/browser/disable_app_update/browser_policy_disable_app_update.js | The About Dialog panel Id should equal policyDisabled - 

Comment 12

[Robin Steuber (she/her) [:bytesized]]

Should have a fix up pretty quickly

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ae0edb10b4b6
Add AppAutoUpdate enterprise policy r=mkaply,fluent-reviewers,flod
https://hg.mozilla.org/integration/autoland/rev/7ff3e1eca482
Remove app.update.auto from the list of preferences that can be manipulated by policy r=mkaply
https://hg.mozilla.org/integration/autoland/rev/92a849b58a96
Add functionality to UpdateUtils for automatic app update to be locked by policy r=mhowell,mkaply
https://hg.mozilla.org/integration/autoland/rev/b97e098a8590
Have the about:preferences UI respect locked automatic application update r=jaws
https://hg.mozilla.org/integration/autoland/rev/73880324e493
Fix bug where message about a prefrence is shown when the preference itself is hidden r=jaws
https://hg.mozilla.org/integration/autoland/rev/9140dcb8abf8
Fix "auto-update-config-change" notification bugs r=mhowell
https://hg.mozilla.org/integration/autoland/rev/abeadcea1d5d
Add tests for the AppAutoUpdate policy r=mkaply

Comment 14

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/ae0edb10b4b6
https://hg.mozilla.org/mozilla-central/rev/7ff3e1eca482
https://hg.mozilla.org/mozilla-central/rev/92a849b58a96
https://hg.mozilla.org/mozilla-central/rev/b97e098a8590
https://hg.mozilla.org/mozilla-central/rev/73880324e493
https://hg.mozilla.org/mozilla-central/rev/9140dcb8abf8
https://hg.mozilla.org/mozilla-central/rev/abeadcea1d5d

Comment 15

[Mike Kaply [:mkaply]]

Is this upliftable to ESR?

Comment 16

[Robin Steuber (she/her) [:bytesized]]

Attachment: ESR68 Rollup.patch

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
This patch replaces a non-functional policy mechanism with a new policy. Since ESR is widely used by enterprise, I think it makes sense to uplift an enterprise policy fix to ESR.

User impact if declined:
Attempts to control automatic update by setting app.update.auto with the preference manipulation enterprise policy will have inconsistent results.

Fix Landed on Version:
75

Risk to taking this patch (and alternatives if risky):
Risk seems pretty low. The code being changed hasn't been touched since 68. I rebased the patch onto ESR68 and did some manual inspection and testing of the patch, which all looks fine.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

---

I know that we generally land code through Lando now, but I wanted to post a rollup patch for uplift, since the rebase did require some merging. Let me know if there is a different way that I ought to have done this.

Comment 17

[Julien Cristau [:jcristau]]

flod, there's a new string in the esr68 patch here, how much of a pain is it from a l10n pov?

Comment 18

[Francesco Lodolo [:flod] (OOO July 12–20)]

(In reply to Julien Cristau [:jcristau] from comment #17)

flod, there's a new string in the esr68 patch here, how much of a pain is it from a l10n pov?

No sides effect on sign-offs, etc. but it will never be translated at this point. From a user point of view, it doesn't seem like a huge problem, since it's a policy description with a limited target.

Comment 19

[Mike Kaply [:mkaply]]

Yeah, we've decided we're OK if policy descriptions don't get translated. More important to get the changes into ESR.

Comment 20

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Hello,

I have tried verifying this with 76.0a1(20200317093640), on Windows 10x64 and Ubuntu 18.04 and as far as I can tell, the UI of about:preferences is affected by this policy(in the sense that "Allow Nightly:" subsection is removed).

However the aformentioned section is removed with both AppAutoUpdate to false&true.

Could you please provide QA with STR to properly test this policy implementation?

Comment 21

[Robin Steuber (she/her) [:bytesized]]

Yes, that behavior is expected. This is how I would verify this patch:

Set the AppAutoUpdate policy to true. This forces automatic updates.

• Check about:preferences to see that the Auto Update setting is hidden.
• Ensure that the policy has taken effect by checking for this console output:
  • await UpdateUtils.getAppUpdateAutoEnabled() should return true
  • UpdateUtils.appUpdateAutoSettingIsLocked() should return true
  • await UpdateUtils.setAppUpdateAutoEnabled(false) should throw an exception

Now set the AppAutoUpdate policy to false. This prevents automatic updates.

• Check about:preferences to see that the Auto Update setting is hidden.
• Ensure that the policy has taken effect by checking for this console output:
  • await UpdateUtils.getAppUpdateAutoEnabled() should return false
  • UpdateUtils.appUpdateAutoSettingIsLocked() should return true
  • await UpdateUtils.setAppUpdateAutoEnabled(true) should throw an exception

Now unset the AppAutoUpdate policy.

• Check about:preferences to see that the Auto Update setting is not hidden.
• Ensure that the setting is unaffected by policy by checking for this console output (make sure to run these in order):
  • await UpdateUtils.setAppUpdateAutoEnabled(true) should return true
  • await UpdateUtils.getAppUpdateAutoEnabled() should return true
  • await UpdateUtils.setAppUpdateAutoEnabled(false) should return false
  • await UpdateUtils.getAppUpdateAutoEnabled() should return false
  • UpdateUtils.appUpdateAutoSettingIsLocked() should return false

Comment 22

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Hello all,

Thank you Kirk for the clarification in regards to the patch content.

Confirming this issue as verified fixed on 75.0b7(ID:20200322132212) and 76.0a1(ID:20200323213856) with all the above enlisted elements.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9132093 [details] [diff] [review]
ESR68 Rollup.patch

Policy engine change needed for release parity. Approved for 68.7esr.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/4cbbc30e1ebc

Comment 25

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Hello,

Confirming this issue as verified fixed. I have verified 68.7.0esr(ID:20200326192020) (Treeherder build) on Windows 10x64, Ubuntu 18.04x64, macOS 10.15 .