Fix keys aren't enforced

[Marco129]

#1733 will happen when only some client keys are defined, like defined clientKey, javascriptKey and restAPIKey but missing the dotNetKey.

Added some test cases to demonstrate the fix.

[codecov-io]

Current coverage is 92.16%

Merging #1789 into master will decrease coverage by -<.01%

1. File src/rest.js (not in diff) was modified. more
   • Misses +1
   • Partials 0
   • Hits -1

@@             master      #1789   diff @@
==========================================
  Files            87         87          
  Lines          6206       6210     +4   
  Methods        1069       1070     +1   
  Messages          0          0          
  Branches       1292       1293     +1   
==========================================
+ Hits           5720       5723     +3   
- Misses          486        487     +1   
  Partials          0          0          

Powered by Codecov. Last updated by 1854928...40dcb35

[drew-gross]

Personally I think we should be removing client keys entirely, they aren't necessary anymore in Parse Server. So I'd rather change the documentation to reflect the current behaviours. If people have specific use cases for client keys, though, I'd happily listen to them.

Also, this is a breaking change for people who currently have weird key setups so we would need to bump to 2.3.0 if we do this.

[natanrolnik]

I understand
So I think we have two options:

1. fix the docs, as they are currently misleading and don't reflect the behavior.
2. if people want to use the keys, and declare them on the server initialization, then enforce. Otherwise, if no keys are set, don't require (except master, of course).

Technically, in Parse.com the applicationId alone would be enough also, right? Considering that both are meant to be public as well, what the keys really added there that they don't add here?

[natanrolnik]

Also, if we remove them completely, people will need to change the server initialization to remove those unneeded parameters - although I don't think it would be breaking as much as enforcing if defined.

[drew-gross]

Yep the client keys are pretty much unnecessary. In JS you can provide extra, unused parameters, so people wouldn't need to remove them from the config.

[natanrolnik]

So should I open a PR fixing the docs?

[drew-gross]

I am in favor of that solution but I haven't considered all use cases so if someone has a good reason to keep client keys around, we should merge this instead. I think @lacker had a reason, but I never heard it.

[steven-supersolid]

Can we spell out why we do or don't need the keys?

All I can think of for keeping:
Don't the client keys provide a weak layer of security? E.g. if someone wants to use a malicious client then they have a bit more work to do to extract the keys from an existing client.

[natanrolnik]

I agree with @steven-supersolid . Even though it's a weak layer, it's another hurdle (small, I agree) to be passed.

[Marco129]

weird key...even Parse blog said it is useless?
May I know what is the initial purpose of introducing client key in Parse.com?

The first thing you need to understand is that your client key is not a security mechanism.
http://blog.parse.com/learn/engineering/parse-security-i-are-you-the-key-master/

[drew-gross]

Why would extracting the client key be any harder than extracting the application ID?

[natanrolnik]

That's right, I thought about it.
So taking it to the other extreme: if parse server isn't multi-app, why do we need an Application Id?

[drew-gross]

Actually we probably don't. But, if/when we approach running multiple Parse apps on a single server, the application ID might be necessary depending on the approach we use. Personally I think one app per mount point is the way to go but I don't want to close the door on multiple apps per mount point.

[natanrolnik]

Fair enough! That's what I imagined.

[steven-supersolid]

I misunderstood that we were talking about removing the application ID as well. If not then the client keys are not so important.

I'm fine with removing the client keys.

[pan83]

Please do not remove client keys. Unfortunately I strongly believe that there are many misconfigured apps out there with messed up db rights that heavily rely on this mechanism. It is not a strong one for sure, but at least a key other than the appid is needed.