Parse Config with ACL

[mtrezza]

Is your feature request related to a problem? Please describe.
Parse Config is a convenient way to remotely configure client parameters. It can also be used in Cloud Code to conveniently configure global parameters like this:

const config = await Parse.Config.get();
const myParam = config.get("myParam");

However, using Parse Config for both poses a problem, as all config parameters are exposed to the client. Cloud Code parameters are of internal nature and it may not be wanted to expose these to clients. In that case Parse Config cannot be used for Cloud Code.

Describe the solution you'd like
Implement ACL for config parameters, on a per-parameter basis.

• The default being Public so it would not pose a breaking change.
• At least the ACL row for "Public" is required to make some parameters only accessible via useMasterKey in cloud code. A per-user / per-role ACL configuration could be omitted if that reduces implementation efforts.

Describe alternatives you've considered
As @davimacedo proposed, the alternative would be to create a class and configure the parameters there. This is a working but not elegant solution since we already have Parse Config for that sort of functionality.

[Moumouls]

Hi @mtrezza
What kind of configuration do you use in your cloud code functions ?

[mtrezza]

@Moumouls config parameters in cloud code can be useful if they

• need to be adjusted manually
• need easy access for manipulation via dashboard rather then hard coding them in cloud code
• need to be adjusted frequently where is it not an option to restart the environment for every adjustment
• the adjustment needs to have an instant effect rather than slowly propagate through individual instances in a load balanced environment

[Moumouls]

I think like @davimacedo that you need to use a class why:

• To have up-to-date settings in your cloud code, you will need to call Parse.Config.get() every time, but the main behavior of Parse.Config.get() is to retrieve all configuration objects, it's not really an optimized solution.
• Parse.Config is designed to be an easy, fast and lightweight solution for obtaining a configuration for clients (public data). If you need to store a sensitive configuration, it is not a good idea to store public and sensitive data in the same place.

A suggestion for your use case

// Not optimized: The full config is pulled from DB
const config = await Parse.Config.get()

// Not tested: Optimized and secure, (name field should be indexed), InternalConfig need to be protected by CLP
const getInternalConfig = async (...args) => {
    const config = {}
   (await (new Parse.Query('InternalConfig'))
        .containedIn('name', args)
        .find({useMasterKey: true}))
        .forEach(result => config[result.get("name")] = config[result.get("value")]
    return config
}

const optimizedConfig = await getInternalConfig("parameter1", "parameter4" )

[mtrezza]

@Moumouls I like it, I’ll go with a class. Mainly because its cheaper to query only the parameters needed.

[mtrezza]

Just to mention some advantages of Parse Config compared to a custom class:

• parameters in Parse Config can easily be of different types, whereas in a class a column has one fixed type. I had to go with a column of type Object which requires some parsing when retrieving the value. So there is some convenience lost.
• parameters that are used by client and cloud code have to be created in Parse Config and the custom class. That means they have to be updated twice and there is a risk of inconsistency.
• a config parameter is safer for frequent manipulation than fields of a custom class; for a config parameter to be saved you have to click the save button but for a field in a custom class it's enough to cloud outside the edit window to save its value

To @Moumouls' arguments I would respond:

• If you need to store a sensitive configuration, it is not a good idea to store public and sensitive data in the same place. -> that would be the purpose of ACLs
• the main behavior of Parse.Config.get() is to retrieve all configuration objects, it's not really an optimized solution. -> making a DB query for a custom class is essentially the same as for Parse Config. The cost difference comes from the data transfer between the DB instance and the Parse Server instance, which can be addressed by adding an optional interface, e.g. const config = ParseConfig.get({parameters: ["p1", "p2"]});

Because of that I hereby reopen for further discussion :)

[mtrezza]

Actually the PR seems fairly simple:

1. Add a Bool switch to the parameter create/edit popup in dashboard Read with master key only with default value false.
   

2. Add logic when changing the switch value add the parameter name to a readWithMasterKeyOnly array in the global config. When deleting a parameter remove its name from that array. Or flag the parameter somehow.

3. Change GlobalConfigRouter.js to not return parameters that are readable with master key only when the request is not done with master key:

getGlobalConfig(req) {
    return req.config.database
      .find('_GlobalConfig', { objectId: '1' }, { limit: 1 })
      .then(results => {
        if (results.length != 1) {
          // If there is no config in the database - return empty config.
          return { response: { params: {} } };
        }
        const globalConfig = results[0];

         // Begin PR
         if (!req.auth.isMaster) {
           for (const param of globalConfig.params) {
            if (result.readWithMasterKeyOnly.includes(param) {
              delete globalConfig.params[param];
            }
          }
        }
        // End PR

        return { response: { params: globalConfig.params } };
      });
  }

Did I forget anything?

[Moumouls]

Seems good to me !
Would you like to start these PRs (one for dashboard, and one for parse-server) ?
@davimacedo Do you see some drawbacks ?

[davimacedo]

I don't see any problem and I'd be happy to review the PRs.

[mtrezza]

I’ll give it a shot.

[mtrezza]

PRs are ready for review:

• Get ParseConfig parameters with Master Key #5954
• Add masterkey parameters parse-dashboard#1233

[Moumouls]

Coud be interesting to implement the feature on Parse.Config.save() in the JS SDK: https://docs.parseplatform.org/js/guide/#save--update-config

I guess it's a little PR 😉

@mtrezza Do you think you can take a look at it?

[Moumouls]

Suggestion

In addition of the current input:

Parse.Config.save({
	welcomeMesssage : "Welcome to Parse",
	ageOfParse : 3,
	tags : ["parse","sdk","js"]
})

We can support this type of input:

Parse.Config.save({
	welcomeMesssage : "Welcome to Parse",
	ageOfParse : 3,
	tags : ["parse","sdk","js"]
        aPrivateConfigField: {
            value: "ImPrivate",
            masterKeyOnly: true
        }
})

[mtrezza]

@Moumouls We have a working feature, my suggestion is to add that in another PR.

[mtrezza]

PR waiting to be merged:

• added section about master key config docs#667

PRs merged:

• Get ParseConfig parameters with Master Key #5954
• Add masterkey parameters parse-dashboard#1233

[TomWFox]

I’ll merge the docs pr once new releases have been made for the server and dashboard.

[mtrezza]

@TomWFox Please reopen.

I just realized that some changes need to be made in the JS SDK as well. The tests passed because they use REST requests, but the JS SDK does not regard the useMasterKey option yet:

https://github.com/parse-community/Parse-SDK-JS/blob/074f65135be751cc987eadfd570b1409bf00110b/src/ParseConfig.js#L80

So const config = Parse.Config.get({useMaserKey: true}); does not work.

[mtrezza]

@Moumouls Eventually the functionality you requested has been added in parse-community/Parse-SDK-JS#910.

[davimacedo]

@mtrezza I think we can now close this one, right?

[mtrezza]

We can! Thanks everyone for their support! 🚀

[mtrezza]

Reopened, because Parse Server 3.8.0 still points to Parse SDK 2.6.0 instead of 2.7.1.
So the feature is not usable.

@davimacedo can we please create a Parse Server release to point to the current JS SDK?

I will open a PR for the dashboard, which currently requires Parse Server 3.8.0 for the feature, but it should actually be the new release version.

[davimacedo]

@mtrezza I think we can.

@dplewis what do you think? Is there something else that you want to include in the next release yet?

[dplewis]

@davimacedo None that I can think of. Go for it!