1445943 - Enterprise Policy support for macOS via `defaults write` and configuration profiles

Status: VERIFIED FIXED

(Firefox :: Enterprise Policies, defect, P1)

Description

[prodigalson80]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.162 Safari/537.36

Steps to reproduce:

Currently the only way to manage policies in the Firefox engine on macOS is to utilize CCK2. Since there was a recent implementation for GPO support on the Policy Engine in Firefox (Bug #1433136), I would like to request similar support for macOS which utilizes Configuration Profiles to manage application preferences. The way to do this is to add support for Firefox to make use of Core Foundation so that when a configuration profile is installed, the application can read the preference values that should be getting enforced. More on Core Foundation here: https://developer.apple.com/documentation/corefoundation/preferences_utilities

Commonly, preferences are written to a plist in macOS which contain the preference values that Core Foundation will read from and then set. This is not required but is the common way in which applications tend to write preferences out which could then be managed by a configuration profile.

Ideally, all the current settings that are manageable with CCK2 would be manageable with a Configuration Profile. However, even just having feature parity with GPO support on Firefox on Windows would be great.


Actual results:

Firefox currently does not store preferences in a plist and does not support Configuration Profiles to enforce preferences.


Expected results:

Firefox currently does not store preferences in a plist and does not support Configuration Profiles to enforce preferences.

Comment 1

[Per Olofsson]

This would be an immense help when deploying Firefox in an organization. Currently Firefox needs special case management via the CCK2, whereas other applications can be managed through standard macOS configuration profiles. As an example, see how Chrome allows management here: https://support.google.com/chrome/a/answer/7550274?hl=en

I'd like to point out though that the API that best suits Firefox' use case is NSUserDefaults, https://developer.apple.com/documentation/foundation/nsuserdefaults?language=objc, rather than the lower level CFPreferences API.

Comment 2

[Mike Kaply [:mkaply]]

I'm definitely planning to investigate Mac management once I get the Windows stuff working.

I've looked a little at what Chrome is doing.

I'm really hoping there's some way to read the configuration profile via Javascript.

Where is the profile typically placed? Would it be possible to just read the plist directly?

Comment 3

[Per Olofsson]

Profiles are stored in a system database and cannot be accessed directly. You have to use the NSUserDefaults (or CFPreferences) API to interact with it - that's where the magic happens.

Comment 4

[Mike Lynn]

More explicitly - the tendency in macOS is towards configuration being changed/queried by services and that the on-disk representation is a.) used for preserving state between reboots and b.) not authoritative compared to the services themselves - that the files on disk are effectively an implementation detail and subject to change.

An example of Apple documenting this is at https://developer.apple.com/library/content/releasenotes/DataManagement/RN-CoreFoundationOlderNotes/index.html under the section "CFPreferences Plist Files".

In the case of profiles, "cannot be accessed directly" is now a literal truth in 10.13 and later. The on-disk database is now stored in a path location protected by SIP (System Integrity Protection) and not even the root account can read the files located there. It is 100% locked down to access only by an Apple signed entitled system service and all configuration is accessed via the API as mentioned above.

While this may sound like an overly complex system - please believe us when we say all major macOS configuration management tools (both open source and commercial) have added extensive support for configuration profiles and most enterprise environments expect mature products to support configuration by profile.

In fact, in some managed environments they are attempting to do management purely by the native MDM service that's part of the OS - a situation where configuration profiles are pretty much the *only* option and products that don't offer management by this mechanism may be avoided or blocked altogether.

Comment 5

[Mike Kaply [:mkaply]]

It sounds like we would have to build a mac specific service that allowed us to acces these via Javascript.

Comment 6

[Mike Kaply [:mkaply]]

Email exchange with more info.

You don't explicitly read MCX or preferences managed via configuration profiles, instead you use the Foundation NSUserDefaults methods or the lower-level CoreFoundation CFPreferences methods. If a preference is managed via MCX or configuration profiles, the managed value is returned.

NSUserDefaults are easier to call from Objective-C or Swift; CFPreferences can be easily called from C and C++ as well as Objective-C and Swift.

NSUserDefaults: https://developer.apple.com/documentation/foundation/nsuserdefaults?language=objc

CFPreferences: https://developer.apple.com/documentation/corefoundation/preferences_utilities?language=objc
CFPreferences is a lower-level API. Stick to the CFPreferencesCopyApp* fucnctions; CFPreferencesCopyAppValue and similar. The CFPreferencesCopyKeyList, CFPreferencesCopyMultiple and CFPreferencesCopyValue functions bypass managed preferences.

----------

We already access NSUserDefaults in a few places. I'll just have to figure out how to make it available to the policy manager. 

Is there some way to know an application has managed preferences so I don't incur the overhead of checking all the policies?

Or would I just create a setting in my plist that is always set?

----------

I'm not sure if there's any way to tell if there are _any_ managed preferences. You can tell if a _specific_ preference is managed:
https://developer.apple.com/documentation/foundation/nsuserdefaults/1408635-objectisforcedforkey?language=objc

This is usually used so an application developer can provide UI feedback as to why a certain preference cannot be modified; for example, by graying-out the relevant controls.

----------

I was just trying to figure out some way to ask generically "are policies set" versus hitting the Mac policy engine for everything.

The way it works with JSON, is we have a list of the applied policies so we enumerate them and query the values.

I'll do some more research

----------

On macOS, you just ask for the value for a key. For example, I can ask for the value for "HomePageURL" for "com.mozilla.firefox".

The macOS preferences system searches all the places this could be defined; managed preferences is one place (and the place with the highest precedence).

I can define that value in a plist file in ~/Library/Preferences/com.mozilla.firefox.plist or /Library/Preferences/com.mozilla.firefox.plist or via MCX (deprecated) or in a configuration profile.

Comment 7

[Stephen A Pohl [:spohl]]

Attachment: 1. Main functionality and support for policies of type "string", "URL", "URLorEmpty" and "boolean"

Dao, not quite sure if you might be able to review the front-end (JS) pieces. Please feel free to forward the review.

The main logic currently lives under xpcom/base, but I'm happy to move this to a more appropriate place.

This currently implements support for enterprise policy types "string", "URL", "URLorEmpty" and "boolean". I will be posting a separate patch to add support for more complex object types. This will be to support the following policies:

Authentication
Bookmarks
Certificates
Cookies
DisableSecurityBypass
EnableTrackingProtection
Extensions
FlashPlugin
Homepage
InstallAddonsPermission
Permissions
PopupBlocking
Proxy
SearchEngines
WebsiteFilter

To enable Enterprise Policies, a key of EnterprisePoliciesEnabled has to be set to "YES" as follows:

defaults write org.mozilla.nightly EnterprisePoliciesEnabled -bool YES

The above assumes that we're testing Nightly. The App ID would need to be changed to org.mozilla.firefox for release.

Here are a few examples of policies and how they can be set once EnterprisePoliciesEnabled is set to YES:

defaults write org.mozilla.nightly BlockAboutAddons -bool YES
defaults write org.mozilla.nightly AppUpdateURL https://www.mozilla.org/test/
defaults write org.mozilla.nightly SearchBar separate
defaults write org.mozilla.nightly DisableSetDesktopBackground -bool YES

These preferences are reflected in /Users/spohl/Library/Preferences/org.mozilla.nightly.plist once set.

Comment 8

[Stephen A Pohl [:spohl]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=10ac3e9a20cb7621feeeb44538519f90f329473a

Comment 9

[Stephen A Pohl [:spohl]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f3a1d430514d12c8285dd80811ebad1a0c586c59

Comment 10

[Stephen A Pohl [:spohl]]

Attachment: 1. Main functionality and support for policies of type "string", "URL", "URLorEmpty" and "boolean"

This fixes build bustages on try by excluding Mac-only files on other platforms and by building off a changeset that has support for nsISimpleEnumerator. I will address the macOS test failures in a separate patch.

Comment 11

[:Felipe Gomes (needinfo for replies!)]

Attachment: org.mozilla.felipe.plist

This is exciting to see! Here are a few drive-by comments:

In general, the reader code shouldn't need to look on the policies' schema. It should read the .plist file, caring about the string/number/boolean values, and output a json that will then be validated by the enterprise policies engine.

For example, it should not need to know that a policy is of type URLorEmpty. It just needs to read that value, which will come as a string from the plist. Then that string will be validated as a URL at a later step that is independent from the macOSPoliciesParser.

The Array types in the plist will be converted to a JS array, and the Dictionary types to an object (so this needs to be a recursive function).


It's ok if the plist has wrong values (for example: an invalid policy name, or a policy that should be a boolean but is a string), because the JsonSchemaValidator will take care of that later.


I'm attaching an example .plist file with what I think would be the ideal format for it, as it directly translates from the same .json format and GPO format that we use for policies


(fwiw, I used sample policies.json from https://searchfox.org/mozilla-central/source/browser/components/enterprisepolicies/helpers to build this plist manually)

Comment 12

[:Felipe Gomes (needinfo for replies!)]

Attachment: plain text plist file

That plist file was generated from XCode and was in some optimized binary format that needs xcode to view. Here it is converted to xml

Comment 13

[Stephen A Pohl [:spohl]]

(In reply to :Felipe Gomes (needinfo me!) from comment #11)
> In general, the reader code shouldn't need to look on the policies' schema.
> It should read the .plist file, caring about the string/number/boolean
> values, and output a json that will then be validated by the enterprise
> policies engine.
> 
> For example, it should not need to know that a policy is of type URLorEmpty.
> It just needs to read that value, which will come as a string from the
> plist. Then that string will be validated as a URL at a later step that is
> independent from the macOSPoliciesParser.

This is certainly an option. We would need to be smart about parsing the plist because we wouldn't want to reject a boolean value just because the type in the plist is set to "Boolean" instead of "String". I see that your plist uses the appropriate types and I'll use that to test against. Fyi, there will be entries in NSUserDefaults that are getting set by the OS and since we are no longer querying keys by name (and instead will be requesting all defaults as a dictionary), we will be passing these preferences to the schema validator. I agree with what you said below about the plist having wrong values and I don't expect this to cause any problems either.

> The Array types in the plist will be converted to a JS array, and the
> Dictionary types to an object (so this needs to be a recursive function).

I was about to complete a patch with a recursive solution for types "object" and "array", just as you suggested. I'll fold everything into the same patch to address your comments here.

> It's ok if the plist has wrong values (for example: an invalid policy name,
> or a policy that should be a boolean but is a string), because the
> JsonSchemaValidator will take care of that later.

Yes, I've been able to confirm that this is indeed the case. This makes things a lot easier on the native side.

> I'm attaching an example .plist file with what I think would be the ideal
> format for it, as it directly translates from the same .json format and GPO
> format that we use for policies

Thank you!

Comment 14

[:Felipe Gomes (needinfo for replies!)]

Attachment: policies.json

And this is the json that I imagine would come out of reading that plist file

(In reply to Stephen A Pohl [:spohl] from comment #13)
> there will be entries in NSUserDefaults that are getting set by the OS and
> since we are no longer querying keys by name (and instead will be requesting
> all defaults as a dictionary), we will be passing these preferences to the
> schema validator.

For this, could we put all the policies-related keys under a "Policies" dict root, so that other things in the file can be skipped?

Comment 15

[Stephen A Pohl [:spohl]]

(In reply to :Felipe Gomes (needinfo me!) from comment #14)
> Created attachment 9010386 [details]
> policies.json
> 
> And this is the json that I imagine would come out of reading that plist file
> 
> (In reply to Stephen A Pohl [:spohl] from comment #13)
> > there will be entries in NSUserDefaults that are getting set by the OS and
> > since we are no longer querying keys by name (and instead will be requesting
> > all defaults as a dictionary), we will be passing these preferences to the
> > schema validator.
> 
> For this, could we put all the policies-related keys under a "Policies" dict
> root, so that other things in the file can be skipped?

This was my original idea. However, :mkaply directed me to a number of Mac admins and they strongly discouraged me/us from going that route. There is a strong preference for keeping all prefs at the root level to make it easier to set them via `defaults write`.

Comment 16

[Stephen A Pohl [:spohl]]

Comment on attachment 9010289 [details] [diff] [review]
1. Main functionality and support for policies of type "string", "URL", "URLorEmpty" and "boolean"

Clearing review requests until I've addressed :felipe's feedback.

Comment 17

[:Felipe Gomes (needinfo for replies!)]

(In reply to Stephen A Pohl [:spohl] from comment #15)
> (In reply to :Felipe Gomes (needinfo me!) from comment #14)
> > Created attachment 9010386 [details]
> > policies.json
> > 
> > And this is the json that I imagine would come out of reading that plist file
> > 
> > (In reply to Stephen A Pohl [:spohl] from comment #13)
> > > there will be entries in NSUserDefaults that are getting set by the OS and
> > > since we are no longer querying keys by name (and instead will be requesting
> > > all defaults as a dictionary), we will be passing these preferences to the
> > > schema validator.
> > 
> > For this, could we put all the policies-related keys under a "Policies" dict
> > root, so that other things in the file can be skipped?
> 
> This was my original idea. However, :mkaply directed me to a number of Mac
> admins and they strongly discouraged me/us from going that route. There is a
> strong preference for keeping all prefs at the root level to make it easier
> to set them via `defaults write`.

Ok, good to know. It will still be hard for them to specify complex policies through `defaults write`, but if that's what they want, sure.

If it makes it easier for you, it's fine to use the schema to get all the key names to be requested. My main objection was to using the type information in the schema by the reader, since that's meant for the validator that runs later.

(FWIW, don't know if you know about it since it's recent, but there's an about:policies page that should help verify that things are being correctly read)

Comment 18

[Stephen A Pohl [:spohl]]

(In reply to :Felipe Gomes (needinfo me!) from comment #17)
> (In reply to Stephen A Pohl [:spohl] from comment #15)
> > (In reply to :Felipe Gomes (needinfo me!) from comment #14)
> > > Created attachment 9010386 [details]
> > > policies.json
> > > 
> > > And this is the json that I imagine would come out of reading that plist file
> > > 
> > > (In reply to Stephen A Pohl [:spohl] from comment #13)
> > > > there will be entries in NSUserDefaults that are getting set by the OS and
> > > > since we are no longer querying keys by name (and instead will be requesting
> > > > all defaults as a dictionary), we will be passing these preferences to the
> > > > schema validator.
> > > 
> > > For this, could we put all the policies-related keys under a "Policies" dict
> > > root, so that other things in the file can be skipped?
> > 
> > This was my original idea. However, :mkaply directed me to a number of Mac
> > admins and they strongly discouraged me/us from going that route. There is a
> > strong preference for keeping all prefs at the root level to make it easier
> > to set them via `defaults write`.
> 
> Ok, good to know. It will still be hard for them to specify complex policies
> through `defaults write`, but if that's what they want, sure.

The schema was mostly designed for GPO. Over time, it would be ideal to flatten the schema. I considered flattening the schema on the native side for macOS by expanding it with underscore separators (for example: "Homepage_URL", "Homepage_Locked" etc.). However, I'm now thinking that this is probably just as hard to get right as writing the more complex policies via `defaults write` directly.

> (FWIW, don't know if you know about it since it's recent, but there's an
> about:policies page that should help verify that things are being correctly
> read)

I didn't know about it. Thanks for pointing this out!

Comment 19

[Per Olofsson]

Just to clarify, the NSUserDefaults and CFPreferences APIs only support reading and writing at the root level — it's not just the `defaults` command. Reading nested values isn't terribly hard if you're comfortable with e.g. Objective-C or Python, but to update a nested value you have to read out the whole structure to a mutable format, perform the change, and then write it all back from the root down. Doing it from bash requires some fairly advanced acrobatics.

So please keep the preferences as flat as possible, it'll make adoption easier for admins that aren't advanced scripters.

Comment 20

[Stephen A Pohl [:spohl]]

(In reply to Per Olofsson from comment #19)
> Just to clarify, the NSUserDefaults and CFPreferences APIs only support
> reading and writing at the root level — it's not just the `defaults`
> command. Reading nested values isn't terribly hard if you're comfortable
> with e.g. Objective-C or Python, but to update a nested value you have to
> read out the whole structure to a mutable format, perform the change, and
> then write it all back from the root down. Doing it from bash requires some
> fairly advanced acrobatics.
> 
> So please keep the preferences as flat as possible, it'll make adoption
> easier for admins that aren't advanced scripters.

I may be misunderstanding your comment, but by requesting objects from the root level, it isn't particularly difficult to drill down into nested values in Objective-C. Configuration profiles also make it easy to deploy nested structures. I have just completed a patch that does just that, and I will be posting a sample .plist and configuration profile to illustrate this as well.

It is true however that `defaults write` is not very amenable to nested structures and it would indeed be preferable to keep preferences as flat as possible in the future to make it easy for admins to test/deploy preferences via `defaults write`.

Comment 21

[Stephen A Pohl [:spohl]]

Attachment: Patch

As far as I can tell, this patch supports all types in our schema. There are two additional types that can be specified in a macOS .plist file: `Date` and `Data`. Since there doesn't seem to be a use for them in our current schema, I have not attempted to add support for them.

:mstange, I would appreciate it if you could take an extra moment to check that my ref counting of XPCOM objects in nsMacPreferencesReader.mm is done properly.

Comment 22

[Stephen A Pohl [:spohl]]

Attachment: org.mozilla.nightly.plist

This is a sample .plist file with a number of preferences. It sets the homepage to about:policies to make it easy to check which policies are enabled.

Flat policies can be set via `defaults write`. Nested structures (a dictionary with arrays, for examples) will have to be configured using a configuration profile. A configuration profile can be created from this .plist file using mcxToProfile[1]. Note that the .plist file has to be named according to the application that we want to configure in reverse DNS form. For nightly, this would be org.mozilla.nightly.plist.

To create the configuration profile, run the following command:

./mcxToProfile.py --plist org.mozilla.nightly.plist --identifier SampleEnterprisePolicies

I will be posting the configuration profile for this .plist file in a minute.

[1] https://github.com/timsutton/mcxToProfile

Comment 23

[Stephen A Pohl [:spohl]]

Attachment: SampleEnterprisePolicies.mobileconfig

Configuration profile based on attachment 9011081 [details].

Simply double-click this file on your macOS system to configure your system.

Comment 24

[Stephen A Pohl [:spohl]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b43ca20064335f54397577e6b70d740d272d4f2

Comment 25

[Markus Stange [:mstange]]

Comment on attachment 9011070 [details] [diff] [review]
Patch

Review of attachment 9011070 [details] [diff] [review]:
-----------------------------------------------------------------

Hey Stephen, I don't want to make you rewrite the whole patch, but it seems to me that this code would get a lot simpler if you used JSONWriter and JS_ParseJSON similar to how nsProfiler::GetProfileData uses it.

Comment 26

[Stephen A Pohl [:spohl]]

(In reply to Markus Stange [:mstange] from comment #25)
> Comment on attachment 9011070 [details] [diff] [review]
> Patch
> 
> Review of attachment 9011070 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hey Stephen, I don't want to make you rewrite the whole patch,

This would only be the third time writing this patch, so why not. ;-) I'll take a look. Thanks!

Comment 27

[Stephen A Pohl [:spohl]]

Attachment: Patch

New patch using JSONWriter.

Still need to look into the test failures on macOS and will address in a separate patch.

One thing I'd like to point out: Even though the JSON parser properly rejects policies that are invalid (of which there are many, since NSUserDefaults returns a whole bunch of settings that are unrelated to enterprise policies), we print an error for every rejected policy in the browser console. These errors are also displayed in the Error section of about:policies. This is the downside of not passing the schema down to native code to request policies by name. I'm not sure how big of a concern this should be. We could change EnterprisePoliciesManager._activatePolicies to not report this as an error by switching to log.debug.

Comment 28

[Stephen A Pohl [:spohl]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac0e09d94d712310388b63b0c89102d2deec3e8c

Comment 29

[Stephen A Pohl [:spohl]]

Attachment: Patch

This should fix the macOS test failures. The previous patch accidentally disabled JSON policies even if no native policies had been set. Try build coming up.

Comment 30

[Stephen A Pohl [:spohl]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d268f7bd326be7893c311e6a14183c82cee78702

Comment 31

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 9011577 [details] [diff] [review]
Patch

Review of attachment 9011577 [details] [diff] [review]:
-----------------------------------------------------------------

Note: I only reviewed the JS parts. I'll leave the backend for mstange


(In reply to Stephen A Pohl [:spohl] from comment #27)
> One thing I'd like to point out: Even though the JSON parser properly
> rejects policies that are invalid (of which there are many, since
> NSUserDefaults returns a whole bunch of settings that are unrelated to
> enterprise policies), we print an error for every rejected policy in the
> browser console. These errors are also displayed in the Error section of
> about:policies. This is the downside of not passing the schema down to
> native code to request policies by name. I'm not sure how big of a concern
> this should be. We could change EnterprisePoliciesManager._activatePolicies
> to not report this as an error by switching to log.debug.

Hmm yeah, this is a bit unfortunate.. I think I prefer the trade-off of not displaying "unknown policy" errors only on this case, by making either the macOSPoliciesProvider or the macOSPoliciesParser.jsm filter out invalid policies names from the JSON object created.

::: browser/components/enterprisepolicies/EnterprisePolicies.js
@@ +435,5 @@
>  
> +class macOSPoliciesProvider {
> +  constructor() {
> +    let wrk = Cc["@mozilla.org/mac-preferences-reader;1"]
> +                .createInstance(Ci.nsIMacPreferencesReader);

The variable name `wrk` is specific to Windows as it's just the short for Windows Registry Key, so feel free to use a different one here.


The policies engine was designed to do very little work if policies aren't enabled, and this patch respects that, but it's hard to see. (in the fact that it returns null and doesn't do any json parsing if the master "EnterprisePoliciesEnabled" key is not present)

What I suggest is to expose a separate function in the preferences reader to check if policies are enabled. Or, even better, expose a function that just checks a simple boolean key, so that the preferences reader gets completely dissociated from policies.

Then you would run that check here and not even call the full readPreferences() function in that case.

::: browser/components/enterprisepolicies/macOSPoliciesParser.jsm
@@ +10,5 @@
> +
> +XPCOMUtils.defineLazyGetter(this, "log", () => {
> +  let { ConsoleAPI } = ChromeUtils.import("resource://gre/modules/Console.jsm", {});
> +  return new ConsoleAPI({
> +    prefix: "macOSPoliciesParser.jsm",

can you include this prefix in https://searchfox.org/mozilla-central/source/browser/components/enterprisepolicies/content/aboutPolicies.js#200 ?

::: xpcom/base/moz.build
@@ +215,5 @@
>  
>  LOCAL_INCLUDES += [
>      '../build',
>      '/dom/base',
> +    '/mfbt',

this file will require a review of a build peer

Comment 32

[Stephen A Pohl [:spohl]]

Attachment: Patch

Addressed Felipe's feedback.

Added r?=glandium for xpcom/base/moz.build.

Comment 33

[Stephen A Pohl [:spohl]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=197d0f5a65ca194b5aa8d67fe58b1eace11afd10

Comment 34

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 9011669 [details] [diff] [review]
Patch

Review of attachment 9011669 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome! Thanks :D

::: browser/components/enterprisepolicies/EnterprisePolicies.js
@@ +435,5 @@
>  
> +class macOSPoliciesProvider {
> +  constructor() {
> +    let prefReader = Cc["@mozilla.org/mac-preferences-reader;1"]
> +                       .createInstance(Ci.nsIMacPreferencesReader);

nit: could you do the reader.policiesEnabled() check directly here so that macOSPoliciesParser.jsm is not even loaded in the common case?

@@ +447,5 @@
> +  _removeUnknownPolicies() {
> +    let { schema } = ChromeUtils.import("resource:///modules/policies/schema.jsm", {});
> +
> +    for (let policyName of Object.keys(this._policies)) {
> +      let isValid = schema.properties[policyName];

nit: use schema.properties.hasOwnProperty(policyName)

Comment 35

[Markus Stange [:mstange]]

Comment on attachment 9011669 [details] [diff] [review]
Patch

Review of attachment 9011669 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/base/nsMacPreferencesReader.mm
@@ +85,5 @@
> +  EvaluateDict(&w, [[NSUserDefaults standardUserDefaults]
> +                     dictionaryRepresentation]);
> +  w.End();
> +
> +  nsCOMPtr<nsISupportsCString> supportsStr =

Oh so this returns it as a JSON string. I'd prefer returning the actual parsed JS object, the way GetProfileData does it:
https://searchfox.org/mozilla-central/rev/881a3c5664ede5e08ee986d76433bc5c4b5680e6/tools/profiler/gecko/nsProfiler.cpp#207,217-220

Then the caller doesn't need to call JSON.parse.

Comment 36

[Stephen A Pohl [:spohl]]

Attachment: Patch

Addressed felipe's and mstange's feedback. Carrying over r+=felipe,glandium.

Comment 37

[Stephen A Pohl [:spohl]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6bd97a82657f2f1d10dd84b20d6b38c08d344962

Comment 38

[Markus Stange [:mstange]]

Comment on attachment 9011944 [details] [diff] [review]
Patch

Review of attachment 9011944 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.

Comment 39

[Stephen A Pohl [:spohl]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/816e01983b58fcf1cc3b02802a73dc9a41e88f84
Bug 1445943: Add Enterprise Policy support for macOS. r=mstange,felipe,glandium

Comment 40

[Stephen A Pohl [:spohl]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/359c5286645408a2f07461a97566dd9448fb46c0
Bug 1445943: Fix ES lint error on inbound due to a missing comma and macOS test failures. r=me CLOSED TREE

Comment 41

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/816e01983b58
https://hg.mozilla.org/mozilla-central/rev/359c52866454

Comment 42

[Stephen A Pohl [:spohl]]

Attachment: Full config plist

This plist lists all supported policies to make it easier to put together a configuration profile.

Not listed are the following policies:
1. Certificates: This policy is Windows-only
2. SearchEngines: This policy is ESR-only.

Comment 43

[Stephen A Pohl [:spohl]]

Attachment: FullEnterprisePolicies.mobileconfig

This is the configuration profile created from the plist file that lists all policies in attachment 9013264 [details]. Using mcxToProfile.py[1], the configuration profile can be generated with the following command in Terminal:

./mcxToProfile.py --plist org.mozilla.nightly.plist --identifier FullEnterprisePolicies

[1] https://github.com/timsutton/mcxToProfile

Comment 44

[jeffreywmcclain]

Does the "DisplayMenuBar" policy do anything on MacOS? I tried setting the policy to both enabled and disabled, but I didn't really notice a difference. I also didn't see the "DisplayBookmarksToolbar" policy doing anything, not sure if it's actually enforced (overriding user-preference) or just the default when the browser is first opened.

Otherwise, all the other policies I've tested seem to work as intended. My only other suggestion would be for the "about:policies" page to list the enabled policies in alphabetical order.

Comment 45

[Mike Kaply [:mkaply]]

Good catch. I'll mark DisplayMenubar as Windows/Linux only.

DisplayBookmarksToolbar should work though. I'll verify.

Thanks for looking at this!

I'll check on the about:policies thing.

Comment 46

[Stephen A Pohl [:spohl]]

Fixing title to clarify that this bug is to add support for enterprise policies using `defaults write` and configuration profiles on macOS. Enterprise policies via JSON policies have been supported for some time.

Comment 47

[Mike Kaply [:mkaply]]

Stephen:

What is your opinion on if this could go on the ESR safely (assuming relman is OK with it)?

Comment 48

[Stephen A Pohl [:spohl]]

(In reply to Mike Kaply [:mkaply] from comment #47)
> Stephen:
> 
> What is your opinion on if this could go on the ESR safely (assuming relman
> is OK with it)?

I have taken a look at this and we should be able to uplift this code with only minor changes. I will be posting a patch shortly that includes all patches for bug 1445943, bug 1497408, bug 1497948 and bug 1498032, updated to make them apply to ESR60. I've combined these patches to make it easier to land. If it is preferred to land them individually, I can go back and post individual ESR patches in the individual bugs.

Comment 49

[Mike Kaply [:mkaply]]

I think one patch for ESR is fine. Thanks!

Comment 50

[Stephen A Pohl [:spohl]]

Attachment: Combined patch for ESR60 (includes patches in bug 1445943, bug 1497408, bug 1497948, bug 1498032)

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: This enables Enterprise Policies via `defaults write` and configuration profiles on macOS, which are preferred over setting policies via JSON policies.

User impact if declined: Admins will not be able to configure Enterprise Policies via `defaults write` or configuration profiles on macOS, which are the preferred ways of setting policies on macOS.

Fix Landed on Version: 64

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): The code is very isolated. In case of any issues, only users with Enterprise Policies set via `defaults write` or configuration profiles should be impacted.

String or UUID changes made by this patch: none

Comment 51

[Liz Henry (:lizzard) (relman/hg->git project)]

Emil, is this something you could verified in ESR60 once it lands? Mike or Stephen should be able to help come up with testing if it isn't already covered by your plan for nightly testing.

Comment 52

[Liz Henry (:lizzard) (relman/hg->git project)]

Here I am just a bit worried that without real world testing we don't have a good idea of the risk. 
Would this only affect macOS?

Comment 53

[Stephen A Pohl [:spohl]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #52)
> Here I am just a bit worried that without real world testing we don't have a
> good idea of the risk. 
> Would this only affect macOS?

Yes, this only affects macOS. And from comment 50:

> The code is very isolated. In case of any issues, only users with Enterprise
> Policies set via `defaults write` or configuration profiles should be impacted.

Comment 54

[Mike Kaply [:mkaply]]

Here I am just a bit worried that without real world testing we don't have a good idea of the risk. 

Also, we have been getting real world testing on this from Mac Admins. All feedback positive and they haven't reported any issues.

Comment 55

[Emil Ghitta, QA [:emilghitta]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #51)
> Emil, is this something you could verified in ESR60 once it lands? Mike or
> Stephen should be able to help come up with testing if it isn't already
> covered by your plan for nightly testing.

This is part of the “Enterprise policy support for MacOS” feature and we have all the necessary information documented. 

This feature is currently under test and since this is the meta bug for this feature,  we will update the 64 flag after our pre-release QA sign-off.

We will definitely verify if this feature works as expected in ESR60 as well (once it lands) and a follow up test report will be sent.

Leaving the ni? on myself until this is done.

Comment 56

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9024839 [details] [diff] [review]
Combined patch for ESR60 (includes patches in bug 1445943, bug 1497408, bug 1497948, bug 1498032)

OK, let's give this new enterprise policy a try.

Comment 57

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/4503802587c8

Comment 58

[Ryan VanderMeulen [:RyanVM]]

Backed out for various OSX Policy Engine test failures.
https://hg.mozilla.org/releases/mozilla-esr60/rev/af3d0194519b

https://treeherder.mozilla.org/logviewer.html#?job_id=212616416&repo=mozilla-esr60
https://treeherder.mozilla.org/logviewer.html#?job_id=212616420&repo=mozilla-esr60
https://treeherder.mozilla.org/logviewer.html#?job_id=212616396&repo=mozilla-esr60
etc.

Comment 59

[Stephen A Pohl [:spohl]]

Attachment: Combined patch for ESR60 (includes patches in bug 1445943, bug 1497408, bug 1497948, bug 1498032)

This was missing a fix that was added while the original patch was still on inbound. I have verified that the tests in question pass successfully now.

[1] https://hg.mozilla.org/mozilla-central/rev/359c52866454

Comment 60

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/b1ece767c018

Comment 61

[Emil Ghitta, QA [:emilghitta]]

This was covered by the overall testing efforts invested in the Enterprise policy support for macOS feature.

Marking this as verified fixed for Fx 64.

Leaving the qe-verify+ flag until this is verified fixed in esr as well.

Comment 62

[Emil Ghitta, QA [:emilghitta]]

This is verified fixed using Firefox 60.3.1 esr (provided in comment 60) on macOS 10.14 and macOS 10.13.

Comment 63

[aimnfires]

I installed esr 60.6.1 and downloaded the plist file from comment 43 and in terminal exectued
./mcxToProfile.py --plist org.mozilla.nightly.plist --identifier FullEnterprisePolicies
then executed FullEnterprisePolicies.mobileconfig
This did not work. In fact about:policies when typed into address bar does not work.

Comment 64

[Mike Kaply [:mkaply]]

about:policies does not work on Firefox 60.

What do you mean by "did not work"? Did a configuration profile apply on the Mac?

That sample plist doesn't really do anything interesting. You'll need to customize your own plist file or use something like Profile Creator.

https://github.com/erikberglund/ProfileCreator

Comment 65

[aimnfires]

I have esr 60.6.1 installed. I go to the address bar and type about:policies and the page displays Hmm. that address doesn't look right. Please check the URL is correct and try again.
Yes, it created a configuration profile. I have 2 profiles at the present time, one for chrome (which works) and one I just created for firefox.

Comment 66

[Mike Kaply [:mkaply]]

After installing the configuration profile for Firefox, can you to to about:config? Or is it blocked.

Please continue this conversation by opening an issue here.

https://github.com/mozilla/policy-templates/

This bug is closed.

Comment 67

[jeffreywmcclain]

(In reply to aimnfires from comment #65)

I have esr 60.6.1 installed. I go to the address bar and type about:policies and the page displays Hmm. that address doesn't look right. Please check the URL is correct and try again.
Yes, it created a configuration profile. I have 2 profiles at the present time, one for chrome (which works) and one I just created for firefox.

"about:policies" isn't supported in the current ESR version (60.6.1), it was only added in Firefox 63 (stable).

Comment 68

[Mike Kaply [:mkaply]]

FYI, the problem was that the plist has to be named org.mozilla.firefox