Skip to content

Commit

Permalink
modify allow_no_origin to default True and also pass Origin: null
Browse files Browse the repository at this point in the history
  • Loading branch information
mmerickel committed Oct 1, 2019
1 parent ada0a97 commit b00fc2b
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 10 deletions.
11 changes: 10 additions & 1 deletion CHANGES.rst
Expand Up @@ -43,7 +43,8 @@ Features
``pyramid.csrf.check_csrf_origin``. This option controls whether a
request is rejected if it has no ``Origin`` or ``Referer`` header -
often the result of a user configuring their browser not to send a
``Referer`` header for privacy reasons.
``Referer`` header for privacy reasons. The default is to allow requests
without a known origin.
See https://github.com/Pylons/pyramid/pull/3512

Deprecations
Expand Down Expand Up @@ -103,6 +104,14 @@ Backward Incompatibilities
by the default execution policy.
See https://github.com/Pylons/pyramid/pull/3496

- ``pyramid.csrf.check_csrf_origin``, used automatically by the CSRF system,
has changed to treat requests without a decipherable origin (no ``Origin``
or ``Referrer`` header) as permissible. The previous behavior can be
turned on using ``allow_no_origin=False``. The special value of ``null``
in the ``Origin`` header is also considering permissible and is not
configurable.
See https://github.com/Pylons/pyramid/pull/3518

Documentation Changes
---------------------

Expand Down
2 changes: 1 addition & 1 deletion docs/narr/security.rst
Expand Up @@ -946,7 +946,7 @@ is the current host, however additional origins may be configured by setting
are non-standard). If a host in the list of domains starts with a ``.`` then
that will allow all subdomains as well as the domain without the ``.``. If no
``Referer`` or ``Origin`` header is present in an HTTPS request, the CSRF check
will fail unless ``allow_no_origin`` is set.
will pass unless ``allow_no_origin`` is disabled.

If CSRF checks fail then a :class:`pyramid.exceptions.BadCSRFToken` or
:class:`pyramid.exceptions.BadCSRFOrigin` exception will be raised. This
Expand Down
6 changes: 3 additions & 3 deletions src/pyramid/config/security.py
Expand Up @@ -197,7 +197,7 @@ def set_default_csrf_options(
token='csrf_token',
header='X-CSRF-Token',
safe_methods=('GET', 'HEAD', 'OPTIONS', 'TRACE'),
allow_no_origin=False,
allow_no_origin=True,
callback=None,
):
"""
Expand All @@ -222,8 +222,8 @@ def set_default_csrf_options(
never be automatically checked for CSRF tokens.
Default: ``('GET', 'HEAD', 'OPTIONS', TRACE')``.
``allow_no_origin`` is a boolean. If false, a request lacking both an
``Origin`` and ``Referer`` header will fail the CSRF check.
``allow_no_origin`` is a boolean. If ``False``, a request lacking both
an ``Origin`` and ``Referer`` header will fail the CSRF check.
If ``callback`` is set, it must be a callable accepting ``(request)``
and returning ``True`` if the request should be checked for a valid
Expand Down
15 changes: 14 additions & 1 deletion src/pyramid/csrf.py
Expand Up @@ -248,7 +248,7 @@ def check_csrf_token(


def check_csrf_origin(
request, trusted_origins=None, allow_no_origin=False, raises=True
request, trusted_origins=None, allow_no_origin=True, raises=True
):
"""
Check the ``Origin`` of the request to see if it is a cross site request or
Expand Down Expand Up @@ -301,8 +301,16 @@ def _fail(reason):

# Determine the origin of this request
origin = request.headers.get("Origin")
origin_is_referrer = False
if origin is None:
origin = request.referrer
origin_is_referrer = True

else:
# use the last origin in the list under the assumption that the
# server generally appends values and we want the origin closest
# to us
origin = origin.split(' ')[-1]

# If we can't find an origin, fail or pass immediately depending on
# ``allow_no_origin``
Expand All @@ -312,6 +320,11 @@ def _fail(reason):
else:
return _fail("Origin checking failed - no Origin or Referer.")

# the client has explicitly hidden their origin so this check is
# not useful and we'll pass it
if not origin_is_referrer and origin == 'null':
return True

# Parse our origin so we we can extract the required information from
# it.
originp = urlparse(origin)
Expand Down
2 changes: 1 addition & 1 deletion src/pyramid/viewderivers.py
Expand Up @@ -488,7 +488,7 @@ def csrf_view(view, info):
token = 'csrf_token'
header = 'X-CSRF-Token'
safe_methods = frozenset(["GET", "HEAD", "OPTIONS", "TRACE"])
allow_no_origin = False
allow_no_origin = True
callback = None
else:
default_val = defaults.require_csrf
Expand Down
2 changes: 1 addition & 1 deletion tests/test_config/test_security.py
Expand Up @@ -126,7 +126,7 @@ def test_set_default_csrf_options(self):
list(sorted(result.safe_methods)),
['GET', 'HEAD', 'OPTIONS', 'TRACE'],
)
self.assertFalse(result.allow_no_origin)
self.assertTrue(result.allow_no_origin)
self.assertTrue(result.callback is None)

def test_changing_set_default_csrf_options(self):
Expand Down
23 changes: 21 additions & 2 deletions tests/test_csrf.py
Expand Up @@ -387,8 +387,27 @@ def test_fails_with_no_origin(self):
request = testing.DummyRequest()
request.scheme = "https"
request.referrer = None
self.assertRaises(BadCSRFOrigin, self._callFUT, request)
self.assertFalse(self._callFUT(request, raises=False))
self.assertRaises(BadCSRFOrigin, self._callFUT, request, allow_no_origin=False)
self.assertFalse(self._callFUT(request, raises=False, allow_no_origin=False))

def test_success_with_null_origin(self):
request = testing.DummyRequest()
request.scheme = "https"
request.referrer = None
request.headers = {'Origin': 'null'}
self.assertTrue(self._callFUT(request, raises=False))
self.assertTrue(self._callFUT(request, raises=False, allow_no_origin=False))

def test_success_with_multiple_origins(self):
request = testing.DummyRequest()
request.scheme = "https"
request.host = "example.com"
request.host_port = "443"
request.headers = {'Origin': 'https://google.com https://not-example.com'}
request.registry.settings = {
"pyramid.csrf_trusted_origins": ["not-example.com"]
}
self.assertTrue(self._callFUT(request, raises=False))

def test_fails_when_http_to_https(self):
from pyramid.exceptions import BadCSRFOrigin
Expand Down

0 comments on commit b00fc2b

Please sign in to comment.