silverwind / droppy

**ARCHIVED** Self-hosted file storage
BSD 2-Clause "Simplified" License
1.62k stars 195 forks source link

Allow to set Access-Control-Allow-Origin header #381

Closed mpromonet closed 5 years ago

mpromonet commented 5 years ago

Hi,

Please find this pull request to allow to set Access-Control-Allow-Origin header. It allow to disable CORS setting configuration parameter "accessCrossOrigin":"*".

Best Regards,

Michel.

silverwind commented 5 years ago

Also need to document it in the example config in the Configuration section in the README.

mpromonet commented 5 years ago

Hi silverwind,

You like to prefer a boolean to disable CORS (setting Access-Control-Allow-Origin to *), the idea was that giving an argument if more open in order to allow access from some domain.

Re-reading the pull request I see that I should write :

if (config.accessCrossOrigin) {
   res.setHeader("Access-Control-Allow-Origin", config.accessCrossOrigin);
} 

My use case is clearly to disable CORS, so if you like I can update the pull request in this way. Does allowCrossOrigin good for you ?

Best Regards,

Michel.

silverwind commented 5 years ago

My use case is clearly to disable CORS

I think you got your terminology wrong. If you set * you actually allow/enable CORS.

You like to prefer a boolean

I prefer a string/boolean to allow the user to set their own value of the header. They can for example put a domain name in it to allow only a single source origin, which is something I do want to offer.

Ideally, I would like to see something like https://github.com/expressjs/cors for CORS handling. CORS is a rather complicated topic in itself once preflight requests are introduced (currently not needed for droppy as all request are "simple" right now), but it will probably be a hack job to integrate that module.

mpromonet commented 5 years ago

Hi silverwind,

I was a little confusing my use case is not disabling CORS, to allow access from everywhere.

So please find an update using a property 'origin' inpiring from https://github.com/expressjs/cors.

Best Regards,

Michel.

silverwind commented 5 years ago

Actually, I think I will add a different option altogether which allows to set arbitrary HTTP headers using a key-value mapping object.

mpromonet commented 5 years ago

Hi silverwind,

Thanks. This is clearly more open.

Best Regards, Michel.