Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-chhr-gxrg-64x7
Security/cors configuration
  • Loading branch information
sergei-maertens committed Dec 17, 2020
2 parents 1bd1ca9 + 673726a commit 9522692
Show file tree
Hide file tree
Showing 11 changed files with 231 additions and 19 deletions.
39 changes: 39 additions & 0 deletions docs/client-development/cors.rst
@@ -0,0 +1,39 @@
.. _client-development-cors:

Cross-Origin Resource Sharing (CORS)
====================================

Some clients develop against Open Zaak using single-page-application technology that
runs completely in the browser, such as React, Angular or other frameworks.

Open Zaak must be deployed with an appropriate CORS-configuration for this.

.. note:: We always recommend using an API gateway/own backend to communicate with Open
Zaak. It's simpler because you don't have to deal with CORS, and there's less risk
of credentials/secrets leaking. You should **never** store client ID/secret in your
dist bundle(s).

Production-grade settings
-------------------------

In production-like environments, we recommend using an explicit allow-list for the
trusted origins. This requires deploying Open Zaak with
``CORS_ALLOWED_ORIGINS=https://my-app.example.com``, where ``https://my-app.example.com``
is the domain where the application is deployed.

Development/experimental configuration
--------------------------------------

If you're running Open Zaak locally or on an environment with dummy data for
development purposes, you can grant CORS access to every possible client using
``CORS_ALLOW_ALL_ORIGINS=True`` in the Open Zaak deployment.

Separation of administrative interface and API
----------------------------------------------

The administrative interface authenticates using session cookies, while the APIs use
the ``Authorization`` header with bearer tokens.

The session cookies are never sent on cross-domain requests, and the CORS configuration
is configured to not allow credentials (which are typically session cookies). The API
with the ``Authorization`` header is not affected by this policy.
1 change: 1 addition & 0 deletions docs/client-development/index.rst
Expand Up @@ -12,4 +12,5 @@ Please select your relevant topic
:maxdepth: 2

authentication
cors
recipes
16 changes: 16 additions & 0 deletions docs/installation/config/env_config.md
Expand Up @@ -108,6 +108,22 @@ on Docker, since `localhost` is contained within the container:
Zaak to other services such as the [Selectie Lijst](https://selectielijst.openzaak.nl/) will be disabled, so this
variable should be used with care to prevent unwanted side-effects.

### Cross-Origin-Resource-Sharing

The following parameters control the CORS policy.

* `CORS_ALLOW_ALL_ORIGINS`: allow cross-domain access from any client. Defaults to `False`.

* `CORS_ALLOWED_ORIGINS`: explicitly list the allowed origins for cross-domain requests.
Defaults to an empty list. Example: `http://localhost:3000,https://some-app.gemeente.nl`.

* `CORS_ALLOWED_ORIGIN_REGEXES`: same as `CORS_ALLOWED_ORIGINS`, but supports regular
expressions.

* `CORS_EXTRA_ALLOW_HEADERS`: headers that are allowed to be sent as part of the cross-domain
request. By default, `Authorization`, `Accept-Crs` and `Content-Crs` are already
included. The value of this variable is added to these already included headers.
Defaults to an empty list.

## Specifying the environment variables

Expand Down
2 changes: 1 addition & 1 deletion requirements/base.in
Expand Up @@ -14,7 +14,7 @@ django
django-auth-adfs-db
django-axes
django-choices
django-cors-middleware
django-cors-headers
django-db-logger
django-extra-views
django-loose-fk
Expand Down
4 changes: 2 additions & 2 deletions requirements/base.txt
Expand Up @@ -18,7 +18,7 @@ django-auth-adfs-db==0.2.0 # via -r requirements/base.in
django-auth-adfs==1.3.1 # via django-auth-adfs-db
django-axes==4.4.0 # via -r requirements/base.in
django-choices==1.7.0 # via -r requirements/base.in, drc-cmis, vng-api-common, zgw-consumers
django-cors-middleware==1.3.1 # via -r requirements/base.in
django-cors-headers==3.5.0 # via -r requirements/base.in
django-db-logger==0.1.7 # via -r requirements/base.in
django-extra-fields==1.2.4 # via -r requirements/base.in
django-extra-views==0.13.0 # via -r requirements/base.in
Expand All @@ -33,7 +33,7 @@ django-relativedelta==1.0.5 # via -r requirements/base.in, zgw-consumers
django-sendfile2==0.6.0 # via django-privates
django-sniplates==0.7.0 # via -r requirements/base.in
django-solo==1.1.3 # via django-auth-adfs-db, drc-cmis, vng-api-common, zgw-consumers
django==2.2.10 # via -r requirements/base.in, django-auth-adfs, django-auth-adfs-db, django-choices, django-db-logger, django-extra-fields, django-extra-views, django-filter, django-loose-fk, django-markup, django-privates, django-redis, django-relativedelta, django-sendfile2, django-sniplates, drc-cmis, drf-nested-routers, drf-yasg, nlx-url-rewriter, vng-api-common, zgw-consumers
django==2.2.10 # via -r requirements/base.in, django-auth-adfs, django-auth-adfs-db, django-choices, django-cors-headers, django-db-logger, django-extra-fields, django-extra-views, django-filter, django-loose-fk, django-markup, django-privates, django-redis, django-relativedelta, django-sendfile2, django-sniplates, drc-cmis, drf-nested-routers, drf-yasg, nlx-url-rewriter, vng-api-common, zgw-consumers
djangorestframework-camel-case==0.2.0 # via -r requirements/base.in, vng-api-common
djangorestframework-gis==0.14 # via -r requirements/base.in
djangorestframework==3.9.4 # via -r requirements/base.in, django-extra-fields, django-loose-fk, djangorestframework-gis, drf-nested-routers, drf-yasg, vng-api-common
Expand Down
4 changes: 2 additions & 2 deletions requirements/ci.txt
Expand Up @@ -24,7 +24,7 @@ django-auth-adfs-db==0.2.0 # via -r requirements/base.txt
django-auth-adfs==1.3.1 # via -r requirements/base.txt, django-auth-adfs-db
django-axes==4.4.0 # via -r requirements/base.txt
django-choices==1.7.0 # via -r requirements/base.txt, drc-cmis, vng-api-common, zgw-consumers
django-cors-middleware==1.3.1 # via -r requirements/base.txt
django-cors-headers==3.5.0 # via -r requirements/base.txt
django-db-logger==0.1.7 # via -r requirements/base.txt
django-extra-fields==1.2.4 # via -r requirements/base.txt
django-extra-views==0.13.0 # via -r requirements/base.txt
Expand All @@ -40,7 +40,7 @@ django-sendfile2==0.6.0 # via -r requirements/base.txt, django-privates
django-sniplates==0.7.0 # via -r requirements/base.txt
django-solo==1.1.3 # via -r requirements/base.txt, django-auth-adfs-db, drc-cmis, vng-api-common, zgw-consumers
django-webtest==1.9.7 # via -r requirements/test-tools.in
django==2.2.10 # via -r requirements/base.txt, django-auth-adfs, django-auth-adfs-db, django-choices, django-db-logger, django-extra-fields, django-extra-views, django-filter, django-loose-fk, django-markup, django-privates, django-redis, django-relativedelta, django-sendfile2, django-sniplates, drc-cmis, drf-nested-routers, drf-yasg, nlx-url-rewriter, vng-api-common, zgw-consumers
django==2.2.10 # via -r requirements/base.txt, django-auth-adfs, django-auth-adfs-db, django-choices, django-cors-headers, django-db-logger, django-extra-fields, django-extra-views, django-filter, django-loose-fk, django-markup, django-privates, django-redis, django-relativedelta, django-sendfile2, django-sniplates, drc-cmis, drf-nested-routers, drf-yasg, nlx-url-rewriter, vng-api-common, zgw-consumers
djangorestframework-camel-case==0.2.0 # via -r requirements/base.txt, vng-api-common
djangorestframework-gis==0.14 # via -r requirements/base.txt
djangorestframework==3.9.4 # via -r requirements/base.txt, django-extra-fields, django-loose-fk, djangorestframework-gis, drf-nested-routers, drf-yasg, vng-api-common
Expand Down
4 changes: 2 additions & 2 deletions requirements/dev.txt
Expand Up @@ -31,7 +31,7 @@ django-auth-adfs-db==0.2.0 # via -r requirements/ci.txt
django-auth-adfs==1.3.1 # via -r requirements/ci.txt, django-auth-adfs-db
django-axes==4.4.0 # via -r requirements/ci.txt
django-choices==1.7.0 # via -r requirements/ci.txt, drc-cmis, vng-api-common, zgw-consumers
django-cors-middleware==1.3.1 # via -r requirements/ci.txt
django-cors-headers==3.5.0 # via -r requirements/ci.txt
django-db-logger==0.1.7 # via -r requirements/ci.txt
django-debug-toolbar==2.0 # via -r requirements/dev.in
django-extensions==2.2.1 # via -r requirements/dev.in
Expand All @@ -50,7 +50,7 @@ django-silk==4.0.1 # via -r requirements/dev.in
django-sniplates==0.7.0 # via -r requirements/ci.txt
django-solo==1.1.3 # via -r requirements/ci.txt, django-auth-adfs-db, drc-cmis, vng-api-common, zgw-consumers
django-webtest==1.9.7 # via -r requirements/ci.txt
django==2.2.10 # via -r requirements/ci.txt, django-auth-adfs, django-auth-adfs-db, django-choices, django-db-logger, django-debug-toolbar, django-extra-fields, django-extra-views, django-filter, django-loose-fk, django-markup, django-privates, django-redis, django-relativedelta, django-sendfile2, django-silk, django-sniplates, drc-cmis, drf-nested-routers, drf-yasg, nlx-url-rewriter, vng-api-common, zgw-consumers
django==2.2.10 # via -r requirements/ci.txt, django-auth-adfs, django-auth-adfs-db, django-choices, django-cors-headers, django-db-logger, django-debug-toolbar, django-extra-fields, django-extra-views, django-filter, django-loose-fk, django-markup, django-privates, django-redis, django-relativedelta, django-sendfile2, django-silk, django-sniplates, drc-cmis, drf-nested-routers, drf-yasg, nlx-url-rewriter, vng-api-common, zgw-consumers
djangorestframework-camel-case==0.2.0 # via -r requirements/ci.txt, vng-api-common
djangorestframework-gis==0.14 # via -r requirements/ci.txt
djangorestframework==3.9.4 # via -r requirements/ci.txt, django-extra-fields, django-loose-fk, djangorestframework-gis, drf-nested-routers, drf-yasg, vng-api-common
Expand Down
1 change: 1 addition & 0 deletions src/openzaak/accounts/tests/factories.py
Expand Up @@ -5,6 +5,7 @@

class UserFactory(factory.django.DjangoModelFactory):
username = factory.Sequence(lambda n: f"user-{n}")
password = factory.PostGenerationMethodCall("set_password")

class Meta:
model = "accounts.User"
Expand Down
29 changes: 17 additions & 12 deletions src/openzaak/conf/includes/base.py
Expand Up @@ -7,6 +7,7 @@

import git
import sentry_sdk
from corsheaders.defaults import default_headers as default_cors_headers
from sentry_sdk.integrations import django, redis

# NLX directory urls
Expand Down Expand Up @@ -152,13 +153,13 @@
"openzaak.utils.middleware.LogHeadersMiddleware",
"django.contrib.sessions.middleware.SessionMiddleware",
# 'django.middleware.locale.LocaleMiddleware',
"corsheaders.middleware.CorsMiddleware",
"django.middleware.common.CommonMiddleware",
"django.middleware.csrf.CsrfViewMiddleware",
"django.contrib.auth.middleware.AuthenticationMiddleware",
"openzaak.components.autorisaties.middleware.AuthMiddleware",
"django.contrib.messages.middleware.MessageMiddleware",
"django.middleware.clickjacking.XFrameOptionsMiddleware",
"corsheaders.middleware.CorsMiddleware",
"openzaak.utils.middleware.APIVersionHeaderMiddleware",
"openzaak.utils.middleware.EnabledMiddleware",
]
Expand Down Expand Up @@ -471,19 +472,23 @@
#
# DJANGO-CORS-MIDDLEWARE
#
CORS_ORIGIN_ALLOW_ALL = True
CORS_ALLOW_ALL_ORIGINS = config("CORS_ALLOW_ALL_ORIGINS", default=False)
CORS_ALLOWED_ORIGINS = config("CORS_ALLOWED_ORIGINS", split=True, default=[])
CORS_ALLOWED_ORIGIN_REGEXES = config(
"CORS_ALLOWED_ORIGIN_REGEXES", split=True, default=[]
)
# Authorization is included in default_cors_headers
CORS_ALLOW_HEADERS = (
"x-requested-with",
"content-type",
"accept",
"origin",
"authorization",
"x-csrftoken",
"user-agent",
"accept-encoding",
"accept-crs",
"content-crs",
list(default_cors_headers)
+ ["accept-crs", "content-crs",]
+ config("CORS_EXTRA_ALLOW_HEADERS", split=True, default=[])
)
CORS_EXPOSE_HEADERS = [
"content-crs",
]
# Django's SESSION_COOKIE_SAMESITE = "Lax" prevents session cookies from being sent
# cross-domain. There is no need for these cookies to be sent, since the API itself
# uses Bearer Authentication.

#
# DJANGO-PRIVATES -- safely serve files after authorization
Expand Down
2 changes: 2 additions & 0 deletions src/openzaak/conf/includes/environ.py
Expand Up @@ -7,6 +7,8 @@ def config(option: str, default=undefined, *args, **kwargs):
if "split" in kwargs:
kwargs.pop("split")
kwargs["cast"] = Csv()
if default == []:
default = ""

if default is not undefined and default is not None:
kwargs.setdefault("cast", type(default))
Expand Down
148 changes: 148 additions & 0 deletions src/openzaak/tests/test_cors_configuration.py
@@ -0,0 +1,148 @@
# SPDX-License-Identifier: EUPL-1.2
# Copyright (C) 2019 - 2020 Dimpact
#
# Documentation on CORS spec, see MDN
# https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request
from unittest.mock import patch

from django.test import override_settings
from django.urls import path

from rest_framework import views
from rest_framework.response import Response
from rest_framework.test import APITestCase

from openzaak.accounts.tests.factories import SuperUserFactory


class View(views.APIView):
def get(self, request, *args, **kwargs):
return Response({"ok": True})

post = get


urlpatterns = [path("cors", View.as_view())]


class CorsMixin:
def setUp(self):
super().setUp()
mocker = patch(
"openzaak.utils.middleware.get_version_mapping", return_value={"/": "1.0.0"}
)
mocker.start()
self.addCleanup(mocker.stop)


@override_settings(ROOT_URLCONF="openzaak.tests.test_cors_configuration")
class DefaultCORSConfigurationTests(CorsMixin, APITestCase):
"""
Test the default CORS settings.
"""

def test_preflight_request(self):
"""
Test the most basic preflight request.
"""
response = self.client.options(
"/cors",
HTTP_ACCESS_CONTROL_REQUEST_METHOD="POST",
HTTP_ACCESS_CONTROL_REQUEST_HEADERS="origin, x-requested-with",
HTTP_ORIGIN="https://evil.com",
)

self.assertNotIn("Access-Control-Allow-Origin", response)
self.assertNotIn("Access-Control-Allow-Credentials", response)

def test_credentialed_request(self):
user = SuperUserFactory.create(password="secret")
self.assertTrue(self.client.login(username=user.username, password="secret"))

response = self.client.get("/cors", HTTP_ORIGIN="https://evil.com",)

self.assertNotIn("Access-Control-Allow-Origin", response)
self.assertNotIn("Access-Control-Allow-Credentials", response)


@override_settings(
ROOT_URLCONF="openzaak.tests.test_cors_configuration",
CORS_ALLOW_ALL_ORIGINS=True,
CORS_ALLOW_CREDENTIALS=False,
)
class CORSEnabledWithoutCredentialsTests(CorsMixin, APITestCase):
"""
Test the default CORS settings.
"""

def test_preflight_request(self):
"""
Test the most basic preflight request.
"""
response = self.client.options(
"/cors",
HTTP_ACCESS_CONTROL_REQUEST_METHOD="POST",
HTTP_ACCESS_CONTROL_REQUEST_HEADERS="origin, x-requested-with",
HTTP_ORIGIN="https://evil.com",
)

# wildcard "*" prevents browsers from sending credentials - this is good
self.assertNotEqual(response["Access-Control-Allow-Origin"], "https://evil.com")
self.assertEqual(response["Access-Control-Allow-Origin"], "*")
self.assertNotIn("Access-Control-Allow-Credentials", response)

def test_simple_request(self):
response = self.client.get("/cors", HTTP_ORIGIN="https://evil.com",)

# wildcard "*" prevents browsers from sending credentials - this is good
self.assertNotEqual(response["Access-Control-Allow-Origin"], "https://evil.com")
self.assertEqual(response["Access-Control-Allow-Origin"], "*")
self.assertNotIn("Access-Control-Allow-Credentials", response)

def test_credentialed_request(self):
user = SuperUserFactory.create(password="secret")
self.assertTrue(self.client.login(username=user.username, password="secret"))

response = self.client.get("/cors", HTTP_ORIGIN="https://evil.com",)

# wildcard "*" prevents browsers from sending credentials - this is good
self.assertNotEqual(response["Access-Control-Allow-Origin"], "https://evil.com")
self.assertEqual(response["Access-Control-Allow-Origin"], "*")
self.assertNotIn("Access-Control-Allow-Credentials", response)


@override_settings(
ROOT_URLCONF="openzaak.tests.test_cors_configuration",
CORS_ALLOW_ALL_ORIGINS=True,
CORS_ALLOW_CREDENTIALS=False,
)
class CORSEnabledWithAuthHeaderTests(CorsMixin, APITestCase):
def test_preflight_request(self):
"""
Test a pre-flight request for requests including the HTTP Authorization header.
The inclusion of htis header makes it a not-simple request, see
https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#Simple_requests
"""
response = self.client.options(
"/cors",
HTTP_ACCESS_CONTROL_REQUEST_METHOD="GET",
HTTP_ACCESS_CONTROL_REQUEST_HEADERS="origin, x-requested-with, authorization",
HTTP_ORIGIN="https://evil.com",
)

self.assertEqual(response["Access-Control-Allow-Origin"], "*")
self.assertIn(
"authorization", response["Access-Control-Allow-Headers"].lower(),
)
self.assertNotIn("Access-Control-Allow-Credentials", response)

def test_credentialed_request(self):
response = self.client.get(
"/cors",
HTTP_ORIGIN="http://localhost:3000",
HTTP_AUTHORIZATION="Bearer foobarbaz",
)

self.assertEqual(response["Access-Control-Allow-Origin"], "*")
self.assertNotIn("Access-Control-Allow-Credentials", response)

0 comments on commit 9522692

Please sign in to comment.