Skip to content

beforeSave / afterSave triggers for Parse.File #4672

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tolgaatam opened this issue Mar 23, 2018 · 7 comments
Closed

beforeSave / afterSave triggers for Parse.File #4672

tolgaatam opened this issue Mar 23, 2018 · 7 comments

Comments

@tolgaatam
Copy link

tolgaatam commented Mar 23, 2018

Hello,

this is a request and an issue for some i guess.

In Parse Server, anybody (even a non-user client) can upload files to the server and this is quite uncontrolled.

My story is: I myself wrote a cloud function for uploading photos, it takes base64 photo, saves it and its 40x40 thumbnail and create a Photo object which holds these two files and a pointer to the user. This way, I can use the Photo object anywhere I like and also I know which user has made which upload. This also lets me be able to track every files in my system and whenever some of the Photo objects are not pointed anymore, I can delete those files they include, from my s3 bucket. This method opts to give me full control over the file uploads.

Although I tried very hard and made this system, still any clients who create a Parse.File themselves can upload it to the system :(

Long story short, any developer who might need some control over the file upload or at least wish to bookkeep the uploads, might find handy to use beforeSave and afterSave triggers for Parse.File class. How do you think of this suggestion? Is there any other mechanism or workaround to solve this "uncontrolled upload" problem?

@tolgaatam
Copy link
Author

tolgaatam commented Mar 23, 2018

quick and dirty workaround could be enforcing master key to file create route,

parse-server/src/Routers/FilesRouter.js : line 20

Original:

    router.post('/files/:filename',
      Middlewares.allowCrossDomain,
      BodyParser.raw({type: () => { return true; }, limit: maxUploadSize }), // Allow uploads without Content-Type, or with any Content-Type.
      Middlewares.handleParseHeaders,
      this.createHandler
    );

with dirty one-liner added:

    router.post('/files/:filename',
      Middlewares.allowCrossDomain,
      BodyParser.raw({type: () => { return true; }, limit: maxUploadSize }), // Allow uploads without Content-Type, or with any Content-Type.
      Middlewares.handleParseHeaders,
      Middlewares.enforceMasterKeyAccess,
      this.createHandler
    );

still this is very hacky and only is doable if you wish to save your files through cloud functions like me.
triggers for Parse.File class would be the most robust solution for the case, i think.
or a config could be set for restricting file uploads to users/roles/master etc. . waiting for your opininons on the case

@dplewis
Copy link
Member

dplewis commented Mar 23, 2018

I do like the idea of having an uploadedBy / owner on Parse.File personally. Its better than having a separate class like I did.

So your main goal here is to have better control over Parse.Files outside of Storing, Deleting, and Retrieving.

There have been a few discussion on a few ways to add functionality to Parse.File. May or may not be related to your Feature Request

PR: Adds authentication to file retrieval
HIPPA Compliance

I'm down to help if we can come up with some plan.

@JacobJT
Copy link

JacobJT commented Mar 23, 2018

While it may not be exactly what you're looking for, are you using S3 for storage? Maybe you could use S3 / Lambda to remove these files you don't want: https://docs.aws.amazon.com/lambda/latest/dg/with-s3.html

I'm not sure if their object created trigger has a before like feature, so if not and someone is creating these files and then attaching them to objects themselves, they'll end up with dead file links, but depending on your circumstances that may be preferable to files you didn't want to allow in the first place being stored.

@tolgaatam
Copy link
Author

@JacobJT deleting the unused files indeed is not a big part of the problem. For any class which use a file field, one can implement an afterSave trigger and detect if the file object is replaced with a new one. with the help of aws sdk for node, the replaced file could be deleted easily, i assume.

a bigger problem is if a user uploads a file and do not attach it to any parse object. then the file is stored in s3 but god knows who uploaded it for what reason :/

so i assume the developer might want to reject some uploads depending on the user or the file extension or maybe restrict it to master alltogether. for this purpose a beforesave seems handy. for keeping user information, @dplewis ‘s uploadedBy field suggestion seems a good fit for the purpose but i fear it might require updates to the client sdk’s. correct me if i’m wrong in this case. an easier-to-implement shortcut could be an aftersave trigger where the developers can do whatever they want with the file information. by not changing the current parse server structure much.

@tolgaatam
Copy link
Author

addition: indeed there is a delete route for files in parse server and it is protected by master key, so deleting files which are being replaced is not so much of an hassle if you make afterSave’s for file-containing classes. So the problem boils down to the allowance of the upload from the first place.

@johnnydimas
Copy link
Contributor

Not sure if this helps you out, but you could try adding some middleware to your parse-server config, and do the stuff you are looking for:

{
  "middleware": "disableFilesMiddleware",
}

And then for your middleware module disableFilesMiddleware.js:

module.exports = function( req , res , next ){

  if( req.path.substring( 0 , 12 ) === '/parse/files' ) {
    res.status(400).send({ code: 119 , message: 'files endpoints are disabled' }); 
    return;
  }

  next();
};

@stale
Copy link

stale bot commented Sep 18, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants