zestedesavoir / django-cors-middleware

django-cors-middleware is DEPRECIATED, please use https://github.com/adamchainz/django-cors-headers instead.
Other
135 stars 34 forks source link

Per Site CORS #13

Closed blaklites closed 6 years ago

blaklites commented 8 years ago

We now have CORS that is global to our Django project. But for Django projects running multiple sites, one may want to configure CORS per site. I have faced this at my current employer. While working for a multi tenant e-commerce platform, we had to modify django-cors-headers to support dynamic CORS management per site(customer). What about a configuration where, alongside the current settings for django-cors-middlewere, we detect if settings.py has the configuration for the current http request's site and apply those above the global ones?

Something like

'SITE_CORS': { 'example.com': { 'CORS_ALLOW_METHODS': ( 'GET', 'POST', ) }

apart from CORS_ALLOW_METHODS, all other configurations for example.com will be same the global ones.

Or something cleaner and easier to configure can be used for the settings. In the middleware

gustavi commented 8 years ago

Very interesting ! I'll try to work on this as soon. Do you use django.contrib.sites ?

blaklites commented 8 years ago

Well, not really, but we use RequestSite object as we are not using the database backed sites framework.

May I work on this item and create a pull request maybe? :).

gustavi commented 8 years ago

It would be nice :)

blaklites commented 8 years ago

Thanks :).

blaklites commented 8 years ago

@gustavi while analyzing solution for this, I came across this observation: The settings for cors-middleware is not within a self contained dictionary like the ones in django-rest-framework (https://github.com/tomchristie/django-rest-framework/blob/master/rest_framework/settings.py) or other supporting Django libraries for example https://github.com/GetBlimp/django-rest-framework-jwt/blob/master/rest_framework_jwt/settings.py. As such, not a problem, but I think having a self contained dictionary to manage settings for third party apps gives much more flexibility and dynamics to the management of the settings. What about (not for immediate release) we update our settings to the pattern followed by the above mentioned libraries and alongside create the new feature for per site CORS?

And our users will have to get used to the new settings too, thus we can start off with deprecation warning and later remove them...

Pomax commented 7 years ago

quick bump - using Mezzanine, which has django sites enabled by default, the lack of cors-per-site makes a multi-tenancy setup quite interesting (or, problematic =D). If there had been more thinking around adding multi-site support to django-cors-middleware, it might still be worth getting an implementation out the door.

Pomax commented 7 years ago

Never mind, It appears to be hidden in plain si(gh)t(e) in the docs over on https://docs.djangoproject.com/en/1.10/ref/contrib/sites/#enabling-the-sites-framework,

In order to serve different sites in production, you'd create a separate settings file with each SITE_ID (perhaps importing from a common settings file to avoid duplicating shared settings) and then specify the appropriate DJANGO_SETTINGS_MODULE for each site

so I guess no work on the cors-middleware side should be necessary for our purposes =)

blaklites commented 6 years ago

Wow, I seem to have missed the last message. So we do not need to resolve this issue then, let's close it.