Improving default File Security

[dblythy]

Is your feature request related to a problem? Please describe.
This FR is in relation to the default security around files, specifically around deployments that never intend to save files.

As discussed in this PR, personally I think it feels bit clunky to recommend a fileTrigger for users that never intend to have any files uploaded to their servers. Also, considering anonymous users are disabled by default, I would expect that file uploads require a logged in user.

Describe the solution you'd like
I was thinking of possibly baking these into the server config:

-allowAnonymousFileUploads: would be a breaking change as would prevent file uploads for no logged in users (unless set to true)
-allowFileUploads: defaults to true. Can set to false for deployments that never want file uploads.

Describe alternatives you've considered
As said, you can achieve this functionality with a file trigger. I just think you should be able to build a secure server without requiring cloud code. If you're building a Parse Server and only reading the Object docs, you won't ever realise that your deployment can accept files, which could be problematic.

Additional context
Made this an issue instead of a PR for @mtrezza :)

[mtrezza]

Thanks for suggesting (and for opening an issue to discuss this 😉).

This seems to be an elegant solution to the file upload issue. The breaking change seems acceptable because it is a security improvement (the PR requires an warning note in the Changelog).

Note that the file upload issue describes that not even an anonymous user is required to upload a file, just the endpoint and appId. Since "Anonymous User" in Parse Server is a feature that describes a "provisional, internal user", this is different from the file upload endpoint being open to the "public". So we need a 3rd configuration option for that case.

Grouping the parameters could make for better overview in the configuration, what do you think about something like:

fileUpload: {
  enabled: true, // Is true if files can be uploaded with Parse Server. Default is true.
  enabledForAnonymousUser: false, // Is true if file upload is enabled for Anonymous Users. Default is false.
  enabledForPublic: false // If true if file upload is enabled for anyone with access to the Parse Server file upload endpoint, regardless of user authentication. Default is false.
}

Please feel free to refine the wording. Maybe you can look into how the Parse docs refer to "public" vs. "Anonymous User", so that we use the same terminology here.

[dblythy]

That's a good point. I completely forgot about anonymous users, I was referring to "public" users, not "anonymous". I like your suggestion. Wording looks simple enough - I'll look into it.

I'll work on a PR, this should be a really simple addition so should have something shortly.

[mtrezza]

Now that I think about it again, isn't the fileUpload.enabled parameter implicitly a fileUpload.enabledForAuthenticatedUser parameter? And if so, should we change that logic, to allow for more granular control?

E.g. with the previous suggestion, it is not possible to allow file uploads only for anonymous users, but not for authenticated users or public. I wouldn't have a use case for that at hand, but it would give more granular control.

What is your opinion on that?

[dblythy]

It's definitely an interesting use case. I see the fileUpload.enabled as a master kill switch. But I agree it should be as granular as possible

I've added enabledForAuthenticatedUser to the latest commit.

[mtrezza]

I don't think fileUpload.enabled is still needed when one can disable the 3 other options and achieve the same result.

[dblythy]

Yeah, good point.

[dblythy]

Closed via #7071

[tehsunnliu]

Hi, does this prevent useMasterKey to save files too? Because from CloudCode when I try to save the file with useMasterKey set to true, I get "File upload by public is disabled" error message. Even from the dashboard I get the same error.
Setting enableForPublic works fine.

[mtrezza]

@tehsunnliu If you need help or have a question please post in the community forum. If you discovered a bug, please open a new issue in this repo. This PR is closed.