WIP: add csrf token to every request

9 files changed, 55 insertions, 49 deletions

diff --git a/service/pixelated/resources/__init__.py b/service/pixelated/resources/__init__.py
index 97346a6f..023758de 100644
--- a/service/pixelated/resources/__init__.py
+++ b/service/pixelated/resources/__init__.py
@@ -66,7 +66,7 @@ class BaseResource(Resource):
         self._services_factory = services_factory
 
     def _add_csrf_cookie(self, request):
-        csrf_token = hashlib.sha256(os.urandom(CSRF_TOKEN_LENGTH)).hexdigest()
+        csrf_token = IPixelatedSession(request.getSession()).get_csrf_token()
         request.addCookie('XSRF-TOKEN', csrf_token)
         log.debug('XSRF-TOKEN added: %s' % csrf_token)
 
diff --git a/service/pixelated/resources/inbox_resource.py b/service/pixelated/resources/inbox_resource.py
index 47a3c072..f759dca9 100644
--- a/service/pixelated/resources/inbox_resource.py
+++ b/service/pixelated/resources/inbox_resource.py
@@ -53,7 +53,6 @@ class InboxResource(BaseResource):
 
     def render_GET(self, request):
         logger.debug('Inbox rendering GET. %s' % self)
-        self._add_csrf_cookie(request)
         if self._is_starting():
             logger.debug('Inbox rendering interstitial. %s' % self)
             return self.interstitial
diff --git a/service/pixelated/resources/root_resource.py b/service/pixelated/resources/root_resource.py
index 7d5b0b0a..1d32935b 100644
--- a/service/pixelated/resources/root_resource.py
+++ b/service/pixelated/resources/root_resource.py
@@ -65,6 +65,7 @@ class RootResource(BaseResource):
         logger.debug('Root in STARTUP mode. %s' % self)
 
     def getChildWithDefault(self, path, request):
+        self._add_csrf_cookie(request)
         if path == '':
             return self._redirect_to_login_resource if self._public else self._inbox_resource
         if self._mode == MODE_STARTUP:
@@ -81,7 +82,6 @@ class RootResource(BaseResource):
         xsrf_token = request.getCookie('XSRF-TOKEN')
         logger.debug('CSRF token: %s' % xsrf_token)
 
-        # TODO: how is comparing the cookie-csrf with the HTTP-header-csrf adding any csrf protection?
         ajax_request = (request.getHeader('x-requested-with') == 'XMLHttpRequest')
         if ajax_request:
             xsrf_header = request.getHeader('x-xsrf-token')
@@ -101,7 +101,7 @@ class RootResource(BaseResource):
         return self.putChildProtected(path, resource)  # to be on the safe side
 
     def initialize(self, provider=None, disclaimer_banner=None, authenticator=None):
-        self.putChildProtected('sandbox', SandboxResource(self._static_folder))
+        self.putChildPublic('sandbox', SandboxResource(self._static_folder))
         self.putChildProtected('keys', KeysResource(self._services_factory))
         self.putChildProtected(AttachmentsResource.BASE_URL, AttachmentsResource(self._services_factory))
         self.putChildProtected('contacts', ContactsResource(self._services_factory))
@@ -114,7 +114,7 @@ class RootResource(BaseResource):
         self.putChildProtected('users', UsersResource(self._services_factory))
         self.putChildPublic(LoginResource.BASE_URL,
                             LoginResource(self._services_factory, provider, disclaimer_banner=disclaimer_banner, authenticator=authenticator))
-        self.putChildProtected(LogoutResource.BASE_URL, LogoutResource(self._services_factory))
+        self.putChildPublic(LogoutResource.BASE_URL, LogoutResource(self._services_factory))
 
         self._inbox_resource.initialize()
         self._mode = MODE_RUNNING
diff --git a/service/test/integration/test_delete_mail.py b/service/test/integration/test_delete_mail.py
index a912f9f0..6cb9ceb6 100644
--- a/service/test/integration/test_delete_mail.py
+++ b/service/test/integration/test_delete_mail.py
@@ -15,6 +15,7 @@
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 from twisted.internet import defer
 from test.support.integration import SoledadTestBase, MailBuilder
+from pixelated.resources import IPixelatedSession
 
 
 class DeleteMailTest(SoledadTestBase):
@@ -27,7 +28,9 @@ class DeleteMailTest(SoledadTestBase):
         inbox_mails = yield self.app_test_client.get_mails_by_tag('inbox')
         self.assertEquals(1, len(inbox_mails))
 
-        yield self.app_test_client.delete_mail(mail.mail_id)
+        response, first_request = yield self.app_test_client.get('/', as_json=False)
+        csrftoken = IPixelatedSession(first_request.getSession()).get_csrf_token()
+        yield self.app_test_client.delete_mail(mail.mail_id, csrf=csrftoken)
 
         inbox_mails = yield self.app_test_client.get_mails_by_tag('inbox')
         self.assertEquals(0, len(inbox_mails))
diff --git a/service/test/integration/test_logout.py b/service/test/integration/test_logout.py
index c9d39d17..b4f8ebf3 100644
--- a/service/test/integration/test_logout.py
+++ b/service/test/integration/test_logout.py
@@ -29,7 +29,8 @@ class MultiUserLogoutTest(MultiUserSoledadTestBase):
 
     @defer.inlineCallbacks
     def test_logout_deletes_services_stop_background_reactor_tasks_and_closes_soledad(self):
-        response, login_request = yield self.app_test_client.login()
+        response, first_request = yield self.app_test_client.get('/login', as_json=False)
+        response, login_request = yield self.app_test_client.login(from_request=first_request)
         yield response
 
         yield self.wait_for_session_user_id_to_finish()
@@ -37,6 +38,7 @@ class MultiUserLogoutTest(MultiUserSoledadTestBase):
         response, request = self.app_test_client.post(
             "/logout",
             json.dumps({'csrftoken': [login_request.getCookie('XSRF-TOKEN')]}),
+            ajax=False,
             from_request=login_request,
             as_json=False)
         yield response
diff --git a/service/test/support/integration/app_test_client.py b/service/test/support/integration/app_test_client.py
index d52c85c0..ee5a1df2 100644
--- a/service/test/support/integration/app_test_client.py
+++ b/service/test/support/integration/app_test_client.py
@@ -387,8 +387,8 @@ class AppTestClient(object):
         return res
 
     # TODO: remove
-    def delete_mail(self, mail_ident):
-        res, req = self.delete("/mail/%s" % mail_ident)
+    def delete_mail(self, mail_ident, csrf='token'):
+        res, req = self.delete("/mail/%s" % mail_ident, csrf=csrf)
         return res
 
     def delete_mails(self, idents):
diff --git a/service/test/support/integration/multi_user_client.py b/service/test/support/integration/multi_user_client.py
index 82acb210..fe8595fb 100644
--- a/service/test/support/integration/multi_user_client.py
+++ b/service/test/support/integration/multi_user_client.py
@@ -24,6 +24,7 @@ from pixelated.config.services import ServicesFactory
 
 from pixelated.config.sessions import LeapSessionFactory
 import pixelated.config.services
+from pixelated.resources import IPixelatedSession
 from pixelated.resources.root_resource import RootResource
 from test.support.integration import AppTestClient
 from test.support.integration.app_test_client import AppTestAccount, StubSRPChecker
@@ -57,7 +58,7 @@ class MultiUserClient(AppTestClient):
         else:
             when(Authenticator)._bonafide_auth(username, password).thenRaise(SRPAuthError)
 
-    def login(self, username='username', password='password'):
+    def login(self, username='username', password='password', from_request=None):
         session = Authentication(username, 'some_user_token', 'some_user_uuid', 'session_id', {'is_admin': False})
         leap_session = self._test_account.leap_session
         leap_session.user_auth = session
@@ -76,7 +77,10 @@ class MultiUserClient(AppTestClient):
             when(leap_session).initial_sync().thenAnswer(lambda: defer.succeed(None))
         when(pixelated.config.services).Services(ANY()).thenReturn(self.services)
 
-        request = request_mock(path='/login', method="POST", body={'username': username, 'password': password})
+        session = from_request.getSession()
+        csrftoken = IPixelatedSession(session).get_csrf_token()
+        request = request_mock(path='/login', method="POST", body={'username': username, 'password': password, 'csrftoken': csrftoken}, ajax=False)
+        request.session = session
         return self._render(request, as_json=False)
 
     def get(self, path, get_args='', as_json=True, from_request=None):
diff --git a/service/test/unit/resources/test_inbox_resource.py b/service/test/unit/resources/test_inbox_resource.py
index 03fe6f1a..9af355ca 100644
--- a/service/test/unit/resources/test_inbox_resource.py
+++ b/service/test/unit/resources/test_inbox_resource.py
@@ -44,29 +44,3 @@ class TestInboxResource(unittest.TestCase):
 
         d.addCallback(assert_response)
         return d
-
-    def _test_should_renew_xsrf_cookie(self):
-        request = DummyRequest([''])
-        request.addCookie = MagicMock()
-        generated_csrf_token = 'csrf_token'
-        mock_sha = MagicMock()
-        mock_sha.hexdigest = MagicMock(return_value=generated_csrf_token)
-
-        with patch('hashlib.sha256', return_value=mock_sha):
-            d = self.web.get(request)
-
-        def assert_csrf_cookie(_):
-            request.addCookie.assert_called_once_with('XSRF-TOKEN', generated_csrf_token)
-
-        d.addCallback(assert_csrf_cookie)
-        return d
-
-    # TODO should this be here or just in the root resource test?
-    def test_should_renew_xsrf_cookie_on_startup_mode(self):
-        self.inbox_resource._mode = MODE_STARTUP
-        self._test_should_renew_xsrf_cookie()
-
-    # TODO should this be here or just in the root resource test?
-    def test_should_renew_xsrf_cookie_on_running_mode(self):
-        self.inbox_resource._mode = MODE_RUNNING
-        self._test_should_renew_xsrf_cookie()
diff --git a/service/test/unit/resources/test_root_resource.py b/service/test/unit/resources/test_root_resource.py
index b674103c..2dfe3e5a 100644
--- a/service/test/unit/resources/test_root_resource.py
+++ b/service/test/unit/resources/test_root_resource.py
@@ -6,7 +6,7 @@ from mockito import mock, when, any as ANY
 
 import pixelated
 from pixelated.application import UserAgentMode
-from pixelated.resources import UnAuthorizedResource
+from pixelated.resources import IPixelatedSession, UnAuthorizedResource
 from pixelated.resources.features_resource import FeaturesResource
 from pixelated.resources.login_resource import LoginResource
 from test.unit.resources import DummySite
@@ -30,7 +30,7 @@ class TestPublicRootResource(unittest.TestCase):
         url_fragment, resource_mock = 'some-url-fragment', mock()
         self.public_root_resource.putChildPublic(url_fragment, resource_mock)
         request = DummyRequest([url_fragment])
-        request.addCookie = lambda key, value: 'stubbed'
+        request.addCookie = MagicMock(return_value='stubbed')
         child_resource = getChildForRequest(self.public_root_resource, request)
         self.assertIs(child_resource, resource_mock)
 
@@ -39,7 +39,7 @@ class TestPublicRootResource(unittest.TestCase):
         url_fragment, resource_mock = 'some-url-fragment', mock()
         self.public_root_resource.putChildProtected(url_fragment, resource_mock)
         request = DummyRequest([url_fragment])
-        request.addCookie = lambda key, value: 'stubbed'
+        request.addCookie = MagicMock(return_value='stubbed')
         child_resource = getChildForRequest(self.public_root_resource, request)
         self.assertIsInstance(child_resource, UnAuthorizedResource)
 
@@ -48,14 +48,14 @@ class TestPublicRootResource(unittest.TestCase):
         url_fragment, resource_mock = 'some-url-fragment', mock()
         self.public_root_resource.putChild(url_fragment, resource_mock)
         request = DummyRequest([url_fragment])
-        request.addCookie = lambda key, value: 'stubbed'
+        request.addCookie = MagicMock(return_value='stubbed')
         child_resource = getChildForRequest(self.public_root_resource, request)
         self.assertIsInstance(child_resource, UnAuthorizedResource)
 
     def test_private_resource_returns_401(self):
         self.public_root_resource.initialize(provider=mock(), authenticator=mock())
         request = DummyRequest(['mails'])
-        request.addCookie = lambda key, value: 'stubbed'
+        request.addCookie = MagicMock(return_value='stubbed')
         d = self.web.get(request)
 
         def assert_unauthorized(request):
@@ -68,14 +68,14 @@ class TestPublicRootResource(unittest.TestCase):
     def test_login_url_should_delegate_to_login_resource(self):
         self.public_root_resource.initialize(provider=mock(), authenticator=mock())
         request = DummyRequest(['login'])
-        request.addCookie = lambda key, value: 'stubbed'
+        request.addCookie = MagicMock(return_value='stubbed')
         child_resource = getChildForRequest(self.public_root_resource, request)
         self.assertIsInstance(child_resource, LoginResource)
 
     def test_root_url_should_redirect_to_login_resource(self):
         self.public_root_resource.initialize(provider=mock(), authenticator=mock())
         request = DummyRequest([''])
-        request.addCookie = lambda key, value: 'stubbed'
+        request.addCookie = MagicMock(return_value='stubbed')
         d = self.web.get(request)
 
         def assert_redirect(request):
@@ -107,7 +107,7 @@ class TestRootResource(unittest.TestCase):
         url_fragment, resource_mock = 'some-url-fragment', mock()
         self.root_resource.putChildProtected(url_fragment, resource_mock)
         request = DummyRequest([url_fragment])
-        request.addCookie = lambda key, value: 'stubbed'
+        request.addCookie = MagicMock(return_value='stubbed')
         child_resource = getChildForRequest(self.root_resource, request)
         self.assertIs(child_resource, resource_mock)
 
@@ -116,13 +116,13 @@ class TestRootResource(unittest.TestCase):
         url_fragment, resource_mock = 'some-url-fragment', mock()
         self.root_resource.putChild(url_fragment, resource_mock)
         request = DummyRequest([url_fragment])
-        request.addCookie = lambda key, value: 'stubbed'
+        request.addCookie = MagicMock(return_value='stubbed')
         child_resource = getChildForRequest(self.root_resource, request)
         self.assertIs(child_resource, resource_mock)
 
     def test_root_url_should_delegate_to_inbox(self):
         request = DummyRequest([''])
-        request.addCookie = lambda key, value: 'stubbed'
+        request.addCookie = MagicMock(return_value='stubbed')
         child_resource = getChildForRequest(self.root_resource, request)
         self.assertIsInstance(child_resource, InboxResource)
 
@@ -130,13 +130,13 @@ class TestRootResource(unittest.TestCase):
     def test_login_url_should_delegate_to_login_resource(self, *mocks):
         self.root_resource.initialize(provider=mock(), authenticator=mock())
         request = DummyRequest(['login'])
-        request.addCookie = lambda key, value: 'stubbed'
+        request.addCookie = MagicMock(return_value='stubbed')
         child_resource = getChildForRequest(self.root_resource, request)
         self.assertIsInstance(child_resource, LoginResource)
 
     def _test_should_renew_xsrf_cookie(self):
         request = DummyRequest([''])
-        request.addCookie = MagicMock()
+        request.addCookie = MagicMock(return_value='stubbed')
         generated_csrf_token = 'csrf_token'
         mock_sha = MagicMock()
         mock_sha.hexdigest = MagicMock(return_value=generated_csrf_token)
@@ -162,6 +162,7 @@ class TestRootResource(unittest.TestCase):
         self.root_resource._mode = MODE_STARTUP
 
         request = DummyRequest(['/child'])
+        request.addCookie = MagicMock(return_value='stubbed')
         request.getCookie = MagicMock(return_value='irrelevant -- stubbed')
 
         d = self.web.get(request)
@@ -182,6 +183,7 @@ class TestRootResource(unittest.TestCase):
         self.root_resource.initialize(provider=mock(), authenticator=mock())
 
         request = DummyRequest(['/child'])
+        request.addCookie = MagicMock(return_value='stubbed')
         request.method = 'POST'
         self._mock_ajax_csrf(request, 'stubbed csrf token')
 
@@ -198,6 +200,7 @@ class TestRootResource(unittest.TestCase):
 
     def test_GET_should_return_503_for_uninitialized_resource(self):
         request = DummyRequest(['/sandbox/'])
+        request.addCookie = MagicMock(return_value='stubbed')
         request.method = 'GET'
 
         request.getCookie = MagicMock(return_value='stubbed csrf token')
@@ -215,6 +218,7 @@ class TestRootResource(unittest.TestCase):
         self.root_resource.initialize(provider=mock(), authenticator=mock())
 
         request = DummyRequest(['non-existing-child'])
+        request.addCookie = MagicMock(return_value='stubbed')
         request.method = 'GET'
         request.getCookie = MagicMock(return_value='stubbed csrf token')
 
@@ -231,6 +235,7 @@ class TestRootResource(unittest.TestCase):
         self.root_resource.initialize(provider=mock(), authenticator=mock())
 
         request = DummyRequest(['non-existing-child'])
+        request.addCookie = MagicMock(return_value='stubbed')
         request.method = 'POST'
         self._mock_ajax_csrf(request, 'stubbed csrf token')
         request.getCookie = MagicMock(return_value='stubbed csrf token')
@@ -246,6 +251,7 @@ class TestRootResource(unittest.TestCase):
 
     def test_should_authorize_child_resource_non_ajax_GET_requests(self):
         request = DummyRequest(['features'])
+        request.addCookie = MagicMock(return_value='stubbed')
 
         request.getCookie = MagicMock(return_value='irrelevant -- stubbed')
         self.root_resource.putChild('features', FeaturesResource())
@@ -270,6 +276,7 @@ class TestRootResource(unittest.TestCase):
         mock_content.read = MagicMock(return_value={})
         request.content = mock_content
 
+        request.addCookie = MagicMock(return_value='stubbed')
         request.getCookie = MagicMock(return_value='mismatched csrf token')
 
         d = self.web.get(request)
@@ -286,6 +293,7 @@ class TestRootResource(unittest.TestCase):
         self.root_resource.initialize(provider=mock(), authenticator=mock())
 
         request = DummyRequest(['assets', 'dummy.json'])
+        request.addCookie = MagicMock(return_value='stubbed')
         d = self.web.get(request)
 
         def assert_response(_):
@@ -299,6 +307,7 @@ class TestRootResource(unittest.TestCase):
         self.root_resource.initialize(provider=mock(), authenticator=mock())
 
         request = DummyRequest(['login'])
+        request.addCookie = MagicMock(return_value='stubbed')
         d = self.web.get(request)
 
         def assert_response(_):
@@ -309,6 +318,7 @@ class TestRootResource(unittest.TestCase):
 
     def test_root_should_be_handled_by_inbox_resource(self):
         request = DummyRequest([])
+        request.addCookie = MagicMock(return_value='stubbed')
         request.prepath = ['']
         request.path = '/'
         # TODO: setup mocked portal
@@ -318,9 +328,23 @@ class TestRootResource(unittest.TestCase):
 
     def test_inbox_should_not_be_public(self):
         request = DummyRequest([])
+        request.addCookie = MagicMock(return_value='stubbed')
         request.prepath = ['']
         request.path = '/'
         # TODO: setup mocked portal
 
         resource = self.root_resource.getChildWithDefault(request.prepath[-1], request)
         self.assertIsInstance(resource, InboxResource)
+
+    def test_every_url_should_get_csrftoken_header(self):
+        # self.root_resource.initialize(provider=mock(), authenticator=mock())
+        request = DummyRequest(['any'])
+        request.addCookie = MagicMock(return_value='stubbed')
+        d = self.web.get(request)
+
+        def assert_add_cookie_called_for_csrftoken(request):
+            csrftoken = IPixelatedSession(request.getSession()).get_csrf_token()
+            self.assertEqual([(('XSRF-TOKEN', csrftoken),)], request.addCookie.call_args_list)
+
+        d.addCallback(assert_add_cookie_called_for_csrftoken)
+        return d