veliovgroup / Meteor-Files

🚀 Upload files via DDP or HTTP to ☄️ Meteor server FS, AWS, GridFS, DropBox or Google Drive. Fast, secure and robust.
https://packosphere.com/ostrio/files
BSD 3-Clause "New" or "Revised" License
1.11k stars 166 forks source link

allowedOrigins error at server.js line 277 #850

Open djlogan2 opened 2 years ago

djlogan2 commented 2 years ago

Hi guys

I believe your code for allowedOrigins has a bug. In server.js, line 277, the if statement does not actually allow for allowedOrigins to contains a regex expression as your documentation states.

The workaround is to add a line after the "new FileCollection" with: obj.allowedOrigins = /xyz/ but obviously this is not ideal, particularly since your documentation states otherwise.

dr-dimitru commented 2 years ago

@djlogan2 thank you for reporting this.

As I remember @menelike and @s-ol authored this feature. Guys, can you check this one? Related piece of code;

dr-dimitru commented 1 year ago

Friendly ping. @djlogan2 have you managed to solve this one? How? Just with the workaround you mention? @menelike @s-ol any thoughts?

djlogan2 commented 1 year ago

Yessir, I implemented the workaround just as I said above. Here is a real world example working in production:

export const ImportedPgnFiles = new FilesCollection({
  collectionName: "importedPgnFiles",
  allowClientCode: false, // Disallow remove files from Client
  allowedOrigins: /(.*chessclub.com.*)|(.*localhost.*)/,
  allowQueryStringCookies: true,
  onBeforeUpload(file) {
    // Allow upload files under 10MB, and only in png/jpg/jpeg formats
    if (/*file.size <= 10485760 && */ /pgn/i.test(file.extension)) {
      return true;
    }
    return "Please upload PGN file";
  },
});
ImportedPgnFiles.allowedOrigins = /(.*chessclub.com.*)|(.*localhost.*)/;
dr-dimitru commented 1 year ago

@djlogan2 great, thank you. I feel like we can close it, unless you wish to update documentation? PRs are welcome

P.S. you're checking pgn instead of png extension

djlogan2 commented 1 year ago

Yes, "pgn" is "Portable Game Notation." I didn't fix the comment. I'll do that now :)

Re closing, I suppose it depends upon whether you want your users to have to add another line or use the configuration object. That's not my call. Your doc says it works, and in the code it actually sets allowedOrigin from the configuration object, just incorrectly. In my opinion, if you want users to write an extra line, changing the doc is required, but so is removing the erroneous code.

dr-dimitru commented 1 year ago

Lol, yeah, In looked at the comment, then looked at name of collection importedPgnFiles and started thinking it might be on purpose 😅

I'm open to continue discussing this one, but I need to know why you are the first one to report this. Since it wasn't reported by other developers I assume it's isolated to you. If so, we need to know what's unique about your project, and add this exception to the docs.

djlogan2 commented 1 year ago

I assumed that I was the only one that needed CORS support. It works fine if your meteor project is deployed to "www.production.com" and your users log on to "www.production.com."

What I assume is unique in my case is that I am using multiple domain names. So mup/meteor gets deployed with "www.production.com", but my users logon to "mycustomprefix.production.com". Because meteor was deployed to "www.production.com", the file uploader code was built with a hard coded url for "www.production.com", which incurs a CORS issue. Thus, I had to put in an allowedOrigin of "*.production.com".

Then of course, as I stated initially, the file uploader code on the indicated line incorrectly evaluates true/false and discards my setting.

dr-dimitru commented 1 year ago

@djlogan2 thank you for reporting this and additional details you have provided. We have escalated this one to the bug level and its fix is available in v2.3.1

Feel free to close it in case if the issue is solved on your end.

dr-dimitru commented 1 year ago

@djlogan2 have you had a chance to test v2.3.1?