Require a CSRF token to use the API auth cookie

Require any JSON API request authenticated with an API auth cookie to
also have a valid CSRF token in an `X-CSRF-Token` header and to have the
first-party domain in the `Referer` header.

This helps prevent CSRF attacks against any API endpoints that are
authenticatable with API auth cookies.

h/security/policy/_api_cookie.py

@@ -1,3 +1,5 @@
+from pyramid.csrf import check_csrf_origin, check_csrf_token
+from pyramid.exceptions import BadCSRFOrigin, BadCSRFToken
 from pyramid.request import Request
 from pyramid.security import Allowed, Denied
 from webob.cookies import SignedCookieProfile
@@ -28,7 +30,18 @@ def handles(request: Request) -> bool:
         ) in COOKIE_AUTHENTICATABLE_API_REQUESTS
 
     def identity(self, request: Request) -> Identity | None:
+        try:
+            check_csrf_origin(request)
+        except BadCSRFOrigin:
+            return None
+
+        try:
+            check_csrf_token(request)
+        except BadCSRFToken:
+            return None
+
         self.helper.add_vary_by_cookie(request)
+
         return self.helper.identity(self.cookie, request)
 
     def authenticated_userid(self, request: Request) -> str | None:

h/static/scripts/group-forms/components/CreateGroupForm.tsx

@@ -146,14 +146,14 @@ export default function CreateGroupForm() {
     setSaving(true);
 
     try {
-      response = (await callAPI(
-        config.api.createGroup.url,
-        config.api.createGroup.method,
-        {
+      response = (await callAPI(config.api.createGroup.url, {
+        method: config.api.createGroup.method,
+        headers: config.api.createGroup.headers,
+        json: {
           name,
           description,
         },
-      )) as CreateGroupAPIResponse;
+      })) as CreateGroupAPIResponse;
     } catch (apiError) {
       setErrorMessage(apiError.message);
       setSaving(false);

h/static/scripts/group-forms/components/test/CreateGroupForm-test.js

@@ -8,6 +8,7 @@ const config = {
     createGroup: {
       method: 'POST',
       url: 'https://example.com/api/groups',
+      headers: { foo: 'bar' },
     },
   },
 };
@@ -209,14 +210,14 @@ describe('CreateGroupForm', () => {
     await wrapper.find('form[data-testid="form"]').simulate('submit');
 
     assert.isTrue(
-      fakeCallAPI.calledOnceWithExactly(
-        config.api.createGroup.url,
-        config.api.createGroup.method,
-        {
+      fakeCallAPI.calledOnceWithExactly(config.api.createGroup.url, {
+        method: config.api.createGroup.method,
+        headers: config.api.createGroup.headers,
+        json: {
           name,
           description,
         },
-      ),
+      }),
     );
     assert.isTrue(fakeSetLocation.calledOnceWithExactly(groupURL));
   });

h/static/scripts/group-forms/config.ts

@@ -1,6 +1,7 @@
 export type APIConfig = {
   method: string;
   url: string;
+  headers: object;
 };
 
 export type ConfigObject = {

h/static/scripts/group-forms/utils/api.ts

@@ -45,15 +45,24 @@ export class APIError extends Error {
 /* Make an API call and return the parsed JSON body or throw APIError. */
 export async function callAPI(
   url: string,
-  method: string = 'GET',
-  json: object | undefined,
+  {
+    method = 'GET',
+    json = null,
+    headers = {},
+  }: {
+    method?: string;
+    json?: object | null;
+    headers?: any;
+  } = {},
 ): Promise<object> {
+  headers['Content-Type'] = 'application/json; charset=UTF-8';
+
   const options: RequestInit = {
-    method: method,
-    headers: { 'Content-Type': 'application/json; charset=UTF-8' },
+    method,
+    headers,
   };
 
-  if (json !== undefined) {
+  if (json !== null) {
     options.body = JSON.stringify(json);
   }
 

h/static/scripts/group-forms/utils/test/api-test.js

@@ -37,7 +37,7 @@ describe('callAPI', () => {
     for (const method of ['GET', 'POST', 'PUT', 'PATCH', 'DELETE']) {
       context(`when the request method is ${method}`, () => {
         it('makes a request using the given method', async () => {
-          await callAPI(url, method);
+          await callAPI(url, { method });
 
           assert.equal(fakeFetch.lastCall.args[1].method, method);
         });
@@ -47,11 +47,20 @@ describe('callAPI', () => {
     it('makes a request with the given JSON body', async () => {
       const json = { foo: 'bar' };
 
-      await callAPI(url, 'POST', json);
+      await callAPI(url, { method: 'POST', json });
 
       assert.equal(fakeFetch.lastCall.args[1].body, JSON.stringify(json));
     });
 
+    it('makes a request with the given headers', async () => {
+      const headers = { foo: 'bar' };
+
+      await callAPI(url, { method: 'POST', headers });
+
+      headers['Content-Type'] = 'application/json; charset=UTF-8';
+      assert.equal(fakeFetch.lastCall.args[1].headers, headers);
+    });
+
     it('returns the parsed JSON object', async () => {
       const result = await callAPI(url);
 

h/templates/groups/create.html.jinja2

@@ -10,7 +10,10 @@
       "api": {
         "createGroup": {
           "method": "POST",
-          "url": "{{ request.route_url("api.groups") }}"
+          "url": "{{ request.route_url("api.groups") }}",
+          "headers": {
+            "X-CSRF-Token": "{{ get_csrf_token() }}"
+          }
         }
       }
     }

tests/unit/h/conftest.py

@@ -168,6 +168,8 @@ def pyramid_request(db_session, db_session_replica, fake_feature, pyramid_settin
 def pyramid_csrf_request(pyramid_request):
     """Return a dummy Pyramid request object with a valid CSRF token."""
     pyramid_request.headers["X-CSRF-Token"] = pyramid_request.session.get_csrf_token()
+    pyramid_request.referrer = "https://example.com"
+    pyramid_request.host_port = "80"
     return pyramid_request
 
 

tests/unit/h/security/policy/_api_cookie_test.py

@@ -1,6 +1,7 @@
 from unittest.mock import create_autospec, sentinel
 
 import pytest
+from pyramid.csrf import SessionCSRFStoragePolicy
 
 from h.security.policy._api_cookie import APICookiePolicy
 from h.security.policy.helpers import AuthTicketCookieHelper
@@ -17,41 +18,55 @@ class TestAPICookiePolicy:
         ],
     )
     def test_handles(
-        self, pyramid_request, route_name, request_method, expected_result
+        self, pyramid_csrf_request, route_name, request_method, expected_result
     ):
-        pyramid_request.matched_route.name = route_name
-        pyramid_request.method = request_method
+        pyramid_csrf_request.matched_route.name = route_name
+        pyramid_csrf_request.method = request_method
 
-        assert APICookiePolicy.handles(pyramid_request) == expected_result
+        assert APICookiePolicy.handles(pyramid_csrf_request) == expected_result
 
-    def test_identity(self, api_cookie_policy, helper, pyramid_request):
-        identity = api_cookie_policy.identity(pyramid_request)
+    def test_identity(self, api_cookie_policy, helper, pyramid_csrf_request):
+        identity = api_cookie_policy.identity(pyramid_csrf_request)
 
-        helper.add_vary_by_cookie.assert_called_once_with(pyramid_request)
-        helper.identity.assert_called_once_with(sentinel.cookie, pyramid_request)
+        helper.add_vary_by_cookie.assert_called_once_with(pyramid_csrf_request)
+        helper.identity.assert_called_once_with(sentinel.cookie, pyramid_csrf_request)
         assert identity == helper.identity.return_value
 
+    def test_identity_with_wrong_origin(self, api_cookie_policy, pyramid_csrf_request):
+        pyramid_csrf_request.referrer = "https://evil.com"
+
+        assert api_cookie_policy.identity(pyramid_csrf_request) is None
+
+    def test_identity_with_wrong_csrf_token(
+        self, api_cookie_policy, pyramid_csrf_request
+    ):
+        del pyramid_csrf_request.headers["X-CSRF-Token"]
+
+        assert api_cookie_policy.identity(pyramid_csrf_request) is None
+
     def test_authenticated_userid(
-        self, api_cookie_policy, helper, pyramid_request, Identity
+        self, api_cookie_policy, helper, pyramid_csrf_request, Identity
     ):
-        authenticated_userid = api_cookie_policy.authenticated_userid(pyramid_request)
+        authenticated_userid = api_cookie_policy.authenticated_userid(
+            pyramid_csrf_request
+        )
 
-        helper.add_vary_by_cookie.assert_called_once_with(pyramid_request)
-        helper.identity.assert_called_once_with(sentinel.cookie, pyramid_request)
+        helper.add_vary_by_cookie.assert_called_once_with(pyramid_csrf_request)
+        helper.identity.assert_called_once_with(sentinel.cookie, pyramid_csrf_request)
         Identity.authenticated_userid.assert_called_once_with(
             helper.identity.return_value
         )
         assert authenticated_userid == Identity.authenticated_userid.return_value
 
     def test_permits(
-        self, api_cookie_policy, helper, pyramid_request, identity_permits
+        self, api_cookie_policy, helper, pyramid_csrf_request, identity_permits
     ):
         permits = api_cookie_policy.permits(
-            pyramid_request, sentinel.context, sentinel.permission
+            pyramid_csrf_request, sentinel.context, sentinel.permission
         )
 
-        helper.add_vary_by_cookie.assert_called_once_with(pyramid_request)
-        helper.identity.assert_called_once_with(sentinel.cookie, pyramid_request)
+        helper.add_vary_by_cookie.assert_called_once_with(pyramid_csrf_request)
+        helper.identity.assert_called_once_with(sentinel.cookie, pyramid_csrf_request)
         identity_permits.assert_called_once_with(
             helper.identity.return_value, sentinel.context, sentinel.permission
         )
@@ -78,3 +93,9 @@ def identity_permits(mocker):
     return mocker.patch(
         "h.security.policy._api_cookie.identity_permits", autospec=True, spec_set=True
     )
+
+
+@pytest.fixture(autouse=True)
+def pyramid_config(pyramid_config):
+    pyramid_config.set_csrf_storage_policy(SessionCSRFStoragePolicy())
+    return pyramid_config