[Feature] Add file upload policy

[jaeggerr]

Related to this issue
#2484

This PR allows developers to specify permissions for file creation.

A new key fileCreationPolicy can be added in the Parse configuration.

• readonly to prevent anyone including master to create new files.
• master to only allow master to create files
• user to allow master and users to create files.

By default, anonymous users are allowed to create new files.

If the PR is approved, litttle changes will be required in the client library to pass the session token when peforming file.save().
Integration tests with authenticated users were tested with the REST API to pass the session token.

[codecov]

Codecov Report

Merging #4822 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4822      +/-   ##
==========================================
- Coverage   92.72%   92.71%   -0.02%     
==========================================
  Files         119      119              
  Lines        8688     8697       +9     
==========================================
+ Hits         8056     8063       +7     
- Misses        632      634       +2

Impacted Files	Coverage Δ
src/Routers/FilesRouter.js	98.98% <100%> (+0.1%)	⬆️
src/Adapters/Auth/meetup.js	84.21% <0%> (-5.27%)	⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js	97.16% <0%> (-0.09%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 260c466...83d95bd. Read the comment docs.

[flovilmart]

Thanks for the PR. Quick question, why would you want to prevent master key to upload files in the first place? I don't believe there is any limitations as such in the platform, so that's inconsistent with what one would be expecting.

If the PR is approved, litttle changes will be required in the client library to pass the session token when peforming file.save().

Which is really not ideal as iOS and android apps may not be updated and it is not acceptable to break the usage for existing users in the wild.

[jaeggerr]

For my personal use, I don't find especially useful to prevent master to upload files but I wanted to keep the possibility as there is already a readOnlyMasterKey property that gives readonly access to the whole database, so that might interest some people.

Concerning iOS, Android or any other client, it does not break anything since by default, if fileCreationPolicy is not specified, anonymous file uploads are allowed.
The only issue is that, as long as client libraries are not updated, developers will have to upload files with the REST API if they specify fileCreationPolicy: 'user' to pass the session token.

I think that it is critical to allow developers to disable anonymous file uploads.
Parse backends are easy to recognize and it would be pretty easy to anyone with bad intentions to use this vulnerability to fill the server with random files.

[flovilmart]

The only issue is that, as long as client libraries are not updated, developers will have to upload files with the REST API if they specify fileCreationPolicy: 'user' to pass the session token.

And that's quite a big issue.

For the readOnlyMasterKey, this should always be blocked if a readonly masterKey is provided. That's an overlook, as you mentioned, the upload is always feasible.

I don't find especially useful to prevent master to upload files

So let's not implement it, less code == better code

I think that it is critical to allow developers to disable anonymous file uploads.

It's not that critical as you've seen, this project has been out for a few years and not picked up, so it'S down below in the priorities, file uploads is not really a big deal compared to database DdoS which can be very easily achievable ;)

[flovilmart]

In summary, let's:

1. properly implement readOnlyMasterKey blocking of files upload.
2. Adds ability to disable parse files, perhaps by specifying a null files adapter OR enable masterKey only files upload.

[jaeggerr]

It's not that critical as you've seen, this project has been out for a few years and not picked up, so it'S down below in the priorities, file uploads is not really a big deal compared to database DdoS which can be very easily achievable ;)

It's not only about DDOS.
It's also about filling the disk, or having to pay for useless storage on S3 buckets.
Or just to prevent unwanted people to use Parse servers to store their files.

Another big issue is that kind of situation:
Let's say your website / app allows users to upload their avatar.

• The user sends a new file with a very large bmp picture of 15MB
• The beforeSave hook rejects the request because the file is too big

Now, what can we do with this file?

1. Delete the file. But what if the user referenced a file created by someone else?
2. Keep forever the useless file in the file storage.
3. Limit file size to 15MB... not a really good idea. Bigger files could be created for other purposes.

If I'm not wrong, the old Parse had a feature to delete files not referenced in any DB row, which was very useful.

A potential solution would be to use cloud functions to pass files in base64 when validation is required and let the server create files but I don't know about performance compared to sending files with the /files API, especially for bigger files.

1. properly implement readOnlyMasterKey blocking of files upload.

I agree

2. Adds ability to disable parse files, perhaps by specifying a null files adapter OR enable masterKey only files upload.

Also agree for both.

[flovilmart]

If I'm not wrong, the old Parse had a feature to delete files not referenced in any DB row, which was very useful.

Yes and if that’s the feature you want, it’s not those changes that will help.

It's also about filling the disk, or having to pay for useless storage on S3 buckets.
Or just to prevent unwanted people to use Parse servers to store their files.

That’s pretty much the case for any class that let users create the object.

For me this is a non issue as at best what will be attempted will give a false sense of security.

If the issue is the files cleanup, then writing a cleaner is the solution.

Requiring authentication is not the solution

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.