Skip to content

Conversation

@finish06
Copy link
Member

Explanation

Adds CORS headers default allow all. Initially needed for building UI.

Rationale

UI request from testing partners were being blocked due to cross origin.

Tests

  1. What testing did you do?
    Rebuilt docker image with these changes. Confirmed with @jrlegrand UI is not acccessible.

@yevgenybulochnik
Copy link
Member

Looks good to me for now to get this working with the UI repo. @finish06 how do you feel about using a LB and maybe docker to remove the necessity of CORS altogether, just serve everything same-site. Tho I guess if we want others to build UIs based off the api we would still need CORS.

Only other comment is per the django-cors-header doc the position of the middleware is confusing.

MIDDLEWARE = [
    ...
    'corsheaders.middleware.CorsMiddleware',
    'django.middleware.common.CommonMiddleware',
    ...
]

Does this mean it should always be above CommonMiddleware? I know there is some stuff in the middlewares array that is higher up, Django SessionMiddleware and SecurityMiddleware. I couldnt tell from the docs if they mean place Cors at the very top or just place it above commonmiddleware. Also not really sure if this matters.

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

Successfully merging this pull request may close these issues.

3 participants