Skip to content
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

[Fixes #42] Dynamicully set CORS ALLOW ORIGINS #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

t-book
Copy link
Contributor

@t-book t-book commented May 20, 2024

@giohappy @ridoo

I'd suggest to add the header for now as geoserver does (with Tomcat). As said in the ticket django-cors-headers is enabled with django in case somebody uses some other fe server than nginx

My take on this is:

If CORS_ALLOW_ALL_ORIGINS = True

https://github.com/GeoNode/geonode/blob/684a1f3572a39b6ab8ba5a6ce7ea4d96fc739c8c/.env.sample#L134

we set the wildcard and all origins will be allowed

2024-05-20 11 13 23 AM

Else the domain name of SITEURL will be set
https://github.com/GeoNode/geonode/blob/684a1f3572a39b6ab8ba5a6ce7ea4d96fc739c8c/.env.sample#L44

2024-05-20 11 11 52 AM
2024-05-20 11 05 36 AM

@t-book t-book added the enhancement New feature or request label May 20, 2024
@ridoo
Copy link

ridoo commented May 22, 2024

@t-book while you are here, wouldn't it make sense to sync all CORS settings? https://pypi.org/project/django-cors-headers allows to configure more CORS settings which are configured in nginx also, e.g.

  • CORS_ALLOW_METHODS
  • CORS_ALLOW_HEADERS
  • CORS_PREFLIGHT_MAX_AGE
  • CORS_ALLOW_CREDENTIALS
  • and more

However, all these can be set from settings.py but they are not part of the .env file. Introducing all of them would blow up things, IMO. This only because we want to sync GeoNode CORS settings with nginx CORS settings? Maybe I did not understand why we would need to protect other (static) resources served by nginx rather than just let Django do its job.

@giohappy sorry for not (yet) being convinced here :)

@ridoo
Copy link

ridoo commented May 23, 2024

Discussion is still going on: #42

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

Successfully merging this pull request may close these issues.

2 participants