Use the webob CookieProfile in the Cookie implementation, rename some implemenations based on feedback, split CSRF implementation and option configuration and make the csrf token function exposed as a system default rather than a renderer event.

10 files changed, 84 insertions, 152 deletions

diff --git a/docs/api/config.rst b/docs/api/config.rst
index c76d3d5ff..a785b64ad 100644
--- a/docs/api/config.rst
+++ b/docs/api/config.rst
@@ -37,6 +37,7 @@
      .. automethod:: set_authentication_policy
      .. automethod:: set_authorization_policy
      .. automethod:: set_default_csrf_options
+     .. automethod:: set_csrf_storage_policy
      .. automethod:: set_default_permission
      .. automethod:: add_permission
 
diff --git a/docs/api/csrf.rst b/docs/api/csrf.rst
index 89fb0c4b2..f890ee660 100644
--- a/docs/api/csrf.rst
+++ b/docs/api/csrf.rst
@@ -5,10 +5,10 @@
 
 .. automodule:: pyramid.csrf
 
-  .. autoclass:: SessionCSRF
+  .. autoclass:: SessionCSRFStoragePolicy
      :members:
 
-  .. autoclass:: CookieCSRF
+  .. autoclass:: CookieCSRFStoragePolicy
      :members:
 
   .. autofunction:: get_csrf_token
diff --git a/docs/narr/extconfig.rst b/docs/narr/extconfig.rst
index 4009ec1dc..c20685cbf 100644
--- a/docs/narr/extconfig.rst
+++ b/docs/narr/extconfig.rst
@@ -263,6 +263,7 @@ Pre-defined Phases
 - :meth:`pyramid.config.Configurator.override_asset`
 - :meth:`pyramid.config.Configurator.set_authorization_policy`
 - :meth:`pyramid.config.Configurator.set_default_csrf_options`
+- :meth:`pyramid.config.Configurator.set_csrf_storage_policy`
 - :meth:`pyramid.config.Configurator.set_default_permission`
 - :meth:`pyramid.config.Configurator.set_view_mapper`
 
diff --git a/docs/narr/security.rst b/docs/narr/security.rst
index 04c236e0b..e67f7b98c 100644
--- a/docs/narr/security.rst
+++ b/docs/narr/security.rst
@@ -780,15 +780,15 @@ and then requiring that it be present in all potentially unsafe requests.
 :app:`Pyramid` provides facilities to create and check CSRF tokens.
 
 By default :app:`Pyramid` comes with a session-based CSRF implementation
-:class:`pyramid.csrf.SessionCSRF`. To use it, you must first enable
+:class:`pyramid.csrf.SessionCSRFStoragePolicy`. To use it, you must first enable
 a :term:`session factory` as described in
 :ref:`using_the_default_session_factory` or
 :ref:`using_alternate_session_factories`. Alternatively, you can use
-a cookie-based implementation :class:`pyramid.csrf.CookieCSRF` which gives
+a cookie-based implementation :class:`pyramid.csrf.CookieCSRFStoragePolicy` which gives
 some additional flexibility as it does not require a session for each user.
 You can also define your own implementation of
 :class:`pyramid.interfaces.ICSRFStoragePolicy` and register it with the
-:meth:`pyramid.config.Configurator.set_default_csrf_options` directive.
+:meth:`pyramid.config.Configurator.set_csrf_storage_policy` directive.
 
 For example:
 
@@ -797,7 +797,7 @@ For example:
    from pyramid.config import Configurator
 
    config = Configurator()
-   config.set_default_csrf_options(implementation=MyCustomCSRFPolicy())
+   config.set_csrf_storage_policy(MyCustomCSRFPolicy())
 
 .. index::
    single: csrf.get_csrf_token
diff --git a/pyramid/config/__init__.py b/pyramid/config/__init__.py
index 6c661aa59..b05effbde 100644
--- a/pyramid/config/__init__.py
+++ b/pyramid/config/__init__.py
@@ -380,6 +380,7 @@ class Configurator(
         self.add_default_view_derivers()
         self.add_default_route_predicates()
         self.add_default_tweens()
+        self.add_default_security()
 
         if exceptionresponse_view is not None:
             exceptionresponse_view = self.maybe_dotted(exceptionresponse_view)
diff --git a/pyramid/config/security.py b/pyramid/config/security.py
index c8becce1f..0b565e322 100644
--- a/pyramid/config/security.py
+++ b/pyramid/config/security.py
@@ -10,15 +10,17 @@ from pyramid.interfaces import (
     PHASE2_CONFIG,
     )
 
-from pyramid.csrf import csrf_token_template_global
-from pyramid.csrf import SessionCSRF
-from pyramid.events import BeforeRender
+from pyramid.csrf import SessionCSRFStoragePolicy
 from pyramid.exceptions import ConfigurationError
 from pyramid.util import action_method
 from pyramid.util import as_sorted_tuple
 
 
 class SecurityConfiguratorMixin(object):
+
+    def add_default_security(self):
+        self.set_csrf_storage_policy(SessionCSRFStoragePolicy())
+
     @action_method
     def set_authentication_policy(self, policy):
         """ Override the :app:`Pyramid` :term:`authentication policy` in the
@@ -170,7 +172,6 @@ class SecurityConfiguratorMixin(object):
     @action_method
     def set_default_csrf_options(
         self,
-        implementation=None,
         require_csrf=True,
         token='csrf_token',
         header='X-CSRF-Token',
@@ -180,10 +181,6 @@ class SecurityConfiguratorMixin(object):
         """
         Set the default CSRF options used by subsequent view registrations.
 
-        ``implementation`` is a class that implements the
-        :meth:`pyramid.interfaces.ICSRFStoragePolicy` interface that will be used for all
-        CSRF functionality. Default: :class:`pyramid.csrf.SessionCSRF`.
-
         ``require_csrf`` controls whether CSRF checks will be automatically
         enabled on each view in the application. This value is used as the
         fallback when ``require_csrf`` is left at the default of ``None`` on
@@ -217,10 +214,7 @@ class SecurityConfiguratorMixin(object):
         options = DefaultCSRFOptions(
             require_csrf, token, header, safe_methods, callback,
         )
-        if implementation is None:
-            implementation = SessionCSRF()
         def register():
-            self.registry.registerUtility(implementation, ICSRFStoragePolicy)
             self.registry.registerUtility(options, IDefaultCSRFOptions)
         intr = self.introspectable('default csrf view options',
                                    None,
@@ -232,10 +226,23 @@ class SecurityConfiguratorMixin(object):
         intr['safe_methods'] = as_sorted_tuple(safe_methods)
         intr['callback'] = callback
 
-        self.add_subscriber(csrf_token_template_global, [BeforeRender])
         self.action(IDefaultCSRFOptions, register, order=PHASE1_CONFIG,
                     introspectables=(intr,))
 
+    @action_method
+    def set_csrf_storage_policy(self, policy):
+        """
+        Set the CSRF storage policy used by subsequent view registrations.
+
+        ``policy`` is a class that implements the
+        :meth:`pyramid.interfaces.ICSRFStoragePolicy` interface that will be used for all
+        CSRF functionality.
+        """
+        def register():
+            self.registry.registerUtility(policy, ICSRFStoragePolicy)
+
+        self.action(ICSRFStoragePolicy, register, order=PHASE1_CONFIG)
+
 
 @implementer(IDefaultCSRFOptions)
 class DefaultCSRFOptions(object):
diff --git a/pyramid/csrf.py b/pyramid/csrf.py
index f282eb569..4c5a73940 100644
--- a/pyramid/csrf.py
+++ b/pyramid/csrf.py
@@ -1,8 +1,11 @@
-from functools import partial
 import uuid
 
+from webob.cookies import CookieProfile
 from zope.interface import implementer
 
+
+from pyramid.authentication import _SimpleSerializer
+
 from pyramid.compat import (
     urlparse,
     bytes_
@@ -20,7 +23,7 @@ from pyramid.util import (
 
 
 @implementer(ICSRFStoragePolicy)
-class SessionCSRF(object):
+class SessionCSRFStoragePolicy(object):
     """ The default CSRF implementation, which mimics the behavior from older
     versions of Pyramid. The ``new_csrf_token`` and ``get_csrf_token`` methods
     are indirected to the underlying session implementation.
@@ -49,7 +52,7 @@ class SessionCSRF(object):
         )
 
 @implementer(ICSRFStoragePolicy)
-class CookieCSRF(object):
+class CookieCSRFStoragePolicy(object):
     """ An alternative CSRF implementation that stores its information in
     unauthenticated cookies, known as the 'Double Submit Cookie' method in the
     OWASP CSRF guidelines. This gives some additional flexibility with regards
@@ -60,25 +63,25 @@ class CookieCSRF(object):
     """
 
     def __init__(self, cookie_name='csrf_token', secure=False, httponly=False,
-                 domain=None, path='/'):
-        self.cookie_name = cookie_name
-        self.secure = secure
-        self.httponly = httponly
+                 domain=None, max_age=None, path='/'):
+        serializer = _SimpleSerializer()
+        self.cookie_profile = CookieProfile(
+            cookie_name=cookie_name,
+            secure=secure,
+            max_age=max_age,
+            httponly=httponly,
+            path=path,
+            serializer=serializer
+        )
         self.domain = domain
-        self.path = path
 
     def new_csrf_token(self, request):
         """ Sets a new CSRF token into the request and returns it. """
         token = uuid.uuid4().hex
         def set_cookie(request, response):
-            response.set_cookie(
-                self.cookie_name,
+            self.cookie_profile.set_cookies(
+                response,
                 token,
-                httponly=self.httponly,
-                secure=self.secure,
-                domain=self.domain,
-                path=self.path,
-                overwrite=True,
             )
         request.add_response_callback(set_cookie)
         return token
@@ -86,7 +89,8 @@ class CookieCSRF(object):
     def get_csrf_token(self, request):
         """ Returns the currently active CSRF token by checking the cookies
         sent with the current request."""
-        token = request.cookies.get(self.cookie_name)
+        bound_cookies = self.cookie_profile.bind(request)
+        token = bound_cookies.get_value()
         if not token:
             token = self.new_csrf_token(request)
         return token
@@ -100,18 +104,6 @@ class CookieCSRF(object):
             bytes_(supplied_token, 'ascii'),
         )
 
-
-def csrf_token_template_global(event):
-    request = event.get('request', None)
-    try:
-        registry = request.registry
-    except AttributeError:
-        return
-    else:
-        csrf = registry.getUtility(ICSRFStoragePolicy)
-        event['get_csrf_token'] = partial(csrf.get_csrf_token, request)
-
-
 def get_csrf_token(request):
     """ Get the currently active CSRF token for the request passed, generating
     a new one using ``new_csrf_token(request)`` if one does not exist. This
@@ -188,9 +180,9 @@ def check_csrf_token(request,
     if policy is None:
         # There is no policy set, but we are trying to validate a CSRF token
         # This means explicit validation has been asked for without configuring
-        # the CSRF implementation. Fall back to SessionCSRF as that is the
+        # the CSRF implementation. Fall back to SessionCSRFStoragePolicy as that is the
         # default
-        policy = SessionCSRF()
+        policy = SessionCSRFStoragePolicy()
     if not policy.check_csrf_token(request, supplied_token):
         if raises:
             raise BadCSRFToken('check_csrf_token(): Invalid token')
diff --git a/pyramid/renderers.py b/pyramid/renderers.py
index 7d667ba7b..6019f50fb 100644
--- a/pyramid/renderers.py
+++ b/pyramid/renderers.py
@@ -1,3 +1,4 @@
+from functools import partial
 import json
 import os
 import re
@@ -19,6 +20,7 @@ from pyramid.compat import (
     text_type,
     )
 
+from pyramid.csrf import get_csrf_token
 from pyramid.decorator import reify
 
 from pyramid.events import BeforeRender
@@ -428,6 +430,7 @@ class RendererHelper(object):
                   'context':context,
                   'request':request,
                   'req':request,
+                  'get_csrf_token':partial(get_csrf_token, request),
                   }
         return self.render_to_response(response, system, request=request)
 
@@ -441,6 +444,7 @@ class RendererHelper(object):
                 'context':getattr(request, 'context', None),
                 'request':request,
                 'req':request,
+                'get_csrf_token':partial(get_csrf_token, request),
                 }
 
         system_values = BeforeRender(system_values, value)
diff --git a/pyramid/tests/test_csrf.py b/pyramid/tests/test_csrf.py
index 3994a31d4..e6ae05eec 100644
--- a/pyramid/tests/test_csrf.py
+++ b/pyramid/tests/test_csrf.py
@@ -22,7 +22,7 @@ class Test_get_csrf_token(unittest.TestCase):
             self._callFUT(request)
 
     def test_success(self):
-        self.config.set_default_csrf_options(implementation=DummyCSRF())
+        self.config.set_csrf_storage_policy(DummyCSRF())
         request = testing.DummyRequest()
 
         csrf_token = self._callFUT(request)
@@ -45,7 +45,7 @@ class Test_new_csrf_token(unittest.TestCase):
             self._callFUT(request)
 
     def test_success(self):
-        self.config.set_default_csrf_options(implementation=DummyCSRF())
+        self.config.set_csrf_storage_policy(DummyCSRF())
         request = testing.DummyRequest()
 
         csrf_token = self._callFUT(request)
@@ -53,57 +53,7 @@ class Test_new_csrf_token(unittest.TestCase):
         self.assertEquals(csrf_token, 'e5e9e30a08b34ff9842ff7d2b958c14b')
 
 
-class Test_csrf_token_template_global(unittest.TestCase):
-    def setUp(self):
-        self.config = testing.setUp()
-
-    def _callFUT(self, *args, **kwargs):
-        from pyramid.csrf import csrf_token_template_global
-        return csrf_token_template_global(*args, **kwargs)
-
-    def test_event_is_missing_request(self):
-        event = BeforeRender({}, {})
-
-        self._callFUT(event)
-
-        self.assertNotIn('get_csrf_token', event)
-
-    def test_request_is_missing_registry(self):
-        request = DummyRequest(registry=None)
-        del request.registry
-        del request.__class__.registry
-        event = BeforeRender({'request': request}, {})
-
-        self._callFUT(event)
-
-        self.assertNotIn('get_csrf_token', event)
-
-    def test_csrf_utility_not_registered(self):
-        request = testing.DummyRequest()
-        event = BeforeRender({'request': request}, {})
-
-        with self.assertRaises(ComponentLookupError):
-            self._callFUT(event)
-
-    def test_csrf_token_passed_to_template(self):
-        config = Configurator()
-        config.set_default_csrf_options(implementation=DummyCSRF())
-        config.commit()
-
-        request = testing.DummyRequest()
-        request.registry = config.registry
-
-        before = BeforeRender({'request': request}, {})
-        config.registry.notify(before)
-
-        self.assertIn('get_csrf_token', before)
-        self.assertEqual(
-            before['get_csrf_token'](),
-            '02821185e4c94269bdc38e6eeae0a2f8'
-        )
-
-
-class TestSessionCSRF(unittest.TestCase):
+class TestSessionCSRFStoragePolicy(unittest.TestCase):
     class MockSession(object):
         def new_csrf_token(self):
             return 'e5e9e30a08b34ff9842ff7d2b958c14b'
@@ -112,20 +62,20 @@ class TestSessionCSRF(unittest.TestCase):
             return '02821185e4c94269bdc38e6eeae0a2f8'
 
     def _makeOne(self):
-        from pyramid.csrf import SessionCSRF
-        return SessionCSRF()
+        from pyramid.csrf import SessionCSRFStoragePolicy
+        return SessionCSRFStoragePolicy()
 
     def test_register_session_csrf_policy(self):
-        from pyramid.csrf import SessionCSRF
+        from pyramid.csrf import SessionCSRFStoragePolicy
         from pyramid.interfaces import ICSRFStoragePolicy
 
         config = Configurator()
-        config.set_default_csrf_options(implementation=self._makeOne())
+        config.set_csrf_storage_policy(self._makeOne())
         config.commit()
 
         policy = config.registry.queryUtility(ICSRFStoragePolicy)
 
-        self.assertTrue(isinstance(policy, SessionCSRF))
+        self.assertTrue(isinstance(policy, SessionCSRFStoragePolicy))
 
     def test_session_csrf_implementation_delegates_to_session(self):
         policy = self._makeOne()
@@ -156,22 +106,22 @@ class TestSessionCSRF(unittest.TestCase):
         self.assertTrue(result)
 
 
-class TestCookieCSRF(unittest.TestCase):
+class TestCookieCSRFStoragePolicy(unittest.TestCase):
     def _makeOne(self):
-        from pyramid.csrf import CookieCSRF
-        return CookieCSRF()
+        from pyramid.csrf import CookieCSRFStoragePolicy
+        return CookieCSRFStoragePolicy()
 
     def test_register_cookie_csrf_policy(self):
-        from pyramid.csrf import CookieCSRF
+        from pyramid.csrf import CookieCSRFStoragePolicy
         from pyramid.interfaces import ICSRFStoragePolicy
 
         config = Configurator()
-        config.set_default_csrf_options(implementation=self._makeOne())
+        config.set_csrf_storage_policy(self._makeOne())
         config.commit()
 
         policy = config.registry.queryUtility(ICSRFStoragePolicy)
 
-        self.assertTrue(isinstance(policy, CookieCSRF))
+        self.assertTrue(isinstance(policy, CookieCSRFStoragePolicy))
 
     def test_get_cookie_csrf_with_no_existing_cookie_sets_cookies(self):
         response = MockResponse()
@@ -179,20 +129,9 @@ class TestCookieCSRF(unittest.TestCase):
 
         policy = self._makeOne()
         token = policy.get_csrf_token(request)
-
         self.assertEqual(
-            response.called_args,
-            ('csrf_token', token),
-        )
-        self.assertEqual(
-            response.called_kwargs,
-            {
-                'secure': False,
-                'httponly': False,
-                'domain': None,
-                'path': '/',
-                'overwrite': True
-            }
+            response.headerlist,
+            [('Set-Cookie', 'csrf_token={}; Path=/'.format(token))]
         )
 
     def test_existing_cookie_csrf_does_not_set_cookie(self):
@@ -208,12 +147,8 @@ class TestCookieCSRF(unittest.TestCase):
             'e6f325fee5974f3da4315a8ccf4513d2'
         )
         self.assertEqual(
-            response.called_args,
-            (),
-        )
-        self.assertEqual(
-            response.called_kwargs,
-            {}
+            response.headerlist,
+            [],
         )
 
     def test_new_cookie_csrf_with_existing_cookie_sets_cookies(self):
@@ -223,20 +158,9 @@ class TestCookieCSRF(unittest.TestCase):
 
         policy = self._makeOne()
         token = policy.new_csrf_token(request)
-
-        self.assertEqual(
-            response.called_args,
-            ('csrf_token', token),
-        )
         self.assertEqual(
-            response.called_kwargs,
-            {
-                'secure': False,
-                'httponly': False,
-                'domain': None,
-                'path': '/',
-                'overwrite': True
-            }
+            response.headerlist,
+            [('Set-Cookie', 'csrf_token={}; Path=/'.format(token))]
         )
 
     def test_verifying_token_invalid_token(self):
@@ -264,7 +188,7 @@ class Test_check_csrf_token(unittest.TestCase):
     def setUp(self):
         self.config = testing.setUp()
 
-        # set up CSRF (this will also register SessionCSRF policy)
+        # set up CSRF (this will also register SessionCSRFStoragePolicy policy)
         self.config.set_default_csrf_options(require_csrf=False)
 
     def _callFUT(self, *args, **kwargs):
@@ -446,13 +370,7 @@ class DummyRequest(object):
 
 class MockResponse(object):
     def __init__(self):
-        self.called_args = ()
-        self.called_kwargs = {}
-
-    def set_cookie(self, *args, **kwargs):
-        self.called_args = args
-        self.called_kwargs = kwargs
-        return
+        self.headerlist = []
 
 
 class DummyCSRF(object):
diff --git a/pyramid/tests/test_renderers.py b/pyramid/tests/test_renderers.py
index 65bfa5582..86d8b582a 100644
--- a/pyramid/tests/test_renderers.py
+++ b/pyramid/tests/test_renderers.py
@@ -203,6 +203,7 @@ class TestRendererHelper(unittest.TestCase):
         self.assertEqual(helper.get_renderer(), factory.respond)
 
     def test_render_view(self):
+        import pyramid.csrf
         self._registerRendererFactory()
         self._registerResponseFactory()
         request = Dummy()
@@ -212,6 +213,9 @@ class TestRendererHelper(unittest.TestCase):
         request = testing.DummyRequest()
         response = 'response'
         response = helper.render_view(request, response, view, context)
+        get_csrf = response.app_iter[1].pop('get_csrf_token')
+        self.assertEqual(get_csrf.args, (request, ))
+        self.assertEqual(get_csrf.func, pyramid.csrf.get_csrf_token)
         self.assertEqual(response.app_iter[0], 'response')
         self.assertEqual(response.app_iter[1],
                           {'renderer_info': helper,
@@ -242,12 +246,16 @@ class TestRendererHelper(unittest.TestCase):
         self.assertEqual(reg.event.__class__.__name__, 'BeforeRender')
 
     def test_render_system_values_is_None(self):
+        import pyramid.csrf
         self._registerRendererFactory()
         request = Dummy()
         context = Dummy()
         request.context = context
         helper = self._makeOne('loo.foo')
         result = helper.render('values', None, request=request)
+        get_csrf = result[1].pop('get_csrf_token')
+        self.assertEqual(get_csrf.args, (request, ))
+        self.assertEqual(get_csrf.func, pyramid.csrf.get_csrf_token)
         system = {'request':request,
                   'context':context,
                   'renderer_name':'loo.foo',