Closed Bug 1469943 Opened 6 years ago Closed 6 years ago

Policy: set a custom app update URL

Categories

(Firefox :: Enterprise Policies, enhancement, P1)

Product:
Firefox
Component:
Enterprise Policies
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 63
Tracking Flags:
Tracking Status
firefox-esr60 --- fixed
firefox62 --- fixed
firefox63 --- fixed

People

(Reporter: molly, Assigned: kanika16047)

References

Details

Attachments

(1 file)

Bug 1469943 - Policy: Custom App Update URL
59 bytes, text/x-review-board-request
Felipe
: review+
RyanVM
: approval-mozilla-beta+
RyanVM
: approval-mozilla-esr60+
Details 
In bug 1468948, the reporter is trying to run their own app update server and ran into difficulty customizing the update URL, because we only read the app.update.url pref from the default branch. I replied with instructions on how to override a pref default, but I think running a custom update server is a reasonable thing to want to do, and we could support it much better by adding a (possibly ESR-only) policy for setting that URL, instead of making people go through that process.
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Priority: -- → P1
:mkaply Can I assign this to myself? Thanks!
> :mkaply Can I assign this to myself? Thanks!

Absolutely. If that doesn't work, let me know.
Assignee: mozilla → ksaini
Comment on attachment 8991683 [details]
Bug 1469943 - Policy: Custom App Update URL

https://reviewboard.mozilla.org/r/256614/#review263508

This looks good. We'll need a test for this. If you can, use this API in nsUpdateService to query the update URL:

https://searchfox.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#2933

Then we can make sure it's not just the pref that gets set, but the actual update URL.

::: browser/components/enterprisepolicies/schemas/policies-schema.json:7
(Diff revision 1)
>    "$schema": "http://json-schema.org/draft-04/schema#",
>    "type": "object",
>    "properties": {
> +    "AppUpdateURL": {
> +      "description": "Sets custom app update server URL",
> +      "enterprise_only": true,

We're not using enterprise_only anymore. I would use machine_only.
Attachment #8991683 - Flags: review?(mozilla) → review-
Comment on attachment 8991683 [details]
Bug 1469943 - Policy: Custom App Update URL

https://reviewboard.mozilla.org/r/256614/#review264176

::: browser/components/enterprisepolicies/Policies.jsm:66
(Diff revision 1)
>   * The callbacks will be bound to their parent policy object.
>   */
>  var Policies = {
> +  "AppUpdateURL": {
> +    onBeforeAddons(manager, param) {
> +      Services.prefs.getDefaultBranch(null).setCharPref("app.update.url", param);

Policies.jsm has a helper function to be used here: setDefaultPref

It should be used because the policies tests replace that function to keep track of changes to auto-clean any changes at the end of each test.
(In reply to :Felipe Gomes (needinfo me!) from comment #5)
> Comment on attachment 8991683 [details]
> Bug 1469943 - Policy: Custom App Update URL
> 
> https://reviewboard.mozilla.org/r/256614/#review264176
> 
> ::: browser/components/enterprisepolicies/Policies.jsm:66
> (Diff revision 1)
> >   * The callbacks will be bound to their parent policy object.
> >   */
> >  var Policies = {
> > +  "AppUpdateURL": {
> > +    onBeforeAddons(manager, param) {
> > +      Services.prefs.getDefaultBranch(null).setCharPref("app.update.url", param);
> 
> Policies.jsm has a helper function to be used here: setDefaultPref
> 
> It should be used because the policies tests replace that function to keep
> track of changes to auto-clean any changes at the end of each test.


The switch case in setDefaultPref doesn't catch the typeof(prefValue) in this case as typeof(prefValue) returns "object". 
Should we pass the URL after stringifying it or make type of param a string in schema(doesn't seem a good idea)?
(In reply to Kanika Saini from comment #6)
> The switch case in setDefaultPref doesn't catch the typeof(prefValue) in
> this case as typeof(prefValue) returns "object". 
> Should we pass the URL after stringifying it or make type of param a string
> in schema(doesn't seem a good idea)?

You should stringify the param before passing it to setDefaultPref. Since it's an URL object, all you need is to use param.href
Comment on attachment 8991683 [details]
Bug 1469943 - Policy: Custom App Update URL

https://reviewboard.mozilla.org/r/256614/#review264774

::: browser/components/enterprisepolicies/schemas/policies-schema.json:6
(Diff revision 3)
>  {
>    "$schema": "http://json-schema.org/draft-04/schema#",
>    "type": "object",
>    "properties": {
> +    "AppUpdateURL": {
> +      "description": "Sets custom app update server URL",

nit: please add a period to the end of the phrase.

::: browser/components/enterprisepolicies/tests/browser/browser_policy_app_update_URL.js:16
(Diff revision 3)
> +add_task(async function test_app_update_URL() {
> +  await setupPolicyEngineWithJson({
> +    "policies": {
> +    	"AppUpdateURL": "https://www.example.com/"
> +    }
> +  });

no need to set this up again. You can do the check for status and the expected value in the same test function.

::: browser/components/enterprisepolicies/tests/browser/browser_policy_app_update_URL.js:25
(Diff revision 3)
> +    }
> +  });
> +
> +  let expected = Services.prefs.getStringPref("app.update.url", undefined);
> +
> +  Assert.deepEqual("https://www.example.com/", expected, "Correct app update URL");

since this is comparing strings, just use `is` instead of `Assert.deepEqual`
Attachment #8991683 - Flags: review?(felipc)
Comment on attachment 8991683 [details]
Bug 1469943 - Policy: Custom App Update URL

https://reviewboard.mozilla.org/r/256614/#review264794
Attachment #8991683 - Flags: review?(felipc) → review+
Pushed by felipc@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9723a216fe4a
Enterprise policy to set a custom app update URL. r=felipe
https://hg.mozilla.org/mozilla-central/rev/9723a216fe4a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Comment on attachment 8991683 [details]
Bug 1469943 - Policy: Custom App Update URL

I missed this one in my work to find policies to uplift. If it's too late, I'm totally ok with that, but thought I would try.


Approval Request Comment
[Feature/Bug causing the regression]: Policy to add customizable update URL.
[User impact if declined]:
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: No.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Only adds new policy.
[String changes made/needed]:

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Policy related.
User impact if declined: 
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): Very low risk, only adds new policy.
String or UUID changes made by this patch: 

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




It's possible these patches won't apply on beta/ESR because they were early. I'll create new patches if accepted.
Attachment #8991683 - Flags: approval-mozilla-esr60?
Attachment #8991683 - Flags: approval-mozilla-beta?
Comment on attachment 8991683 [details]
Bug 1469943 - Policy: Custom App Update URL

Adds a policy to allow customization of the app update URL with an automated test included. Approved for 62.0b20 and ESR 60.2.
Attachment #8991683 - Flags: approval-mozilla-esr60?
Attachment #8991683 - Flags: approval-mozilla-esr60+
Attachment #8991683 - Flags: approval-mozilla-beta?
Attachment #8991683 - Flags: approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: