refactor: move CSRF code into plugin (#56447)

api/src/app.ts

@@ -1,5 +1,4 @@
 import fastifyAccepts from '@fastify/accepts';
-import fastifyCsrfProtection from '@fastify/csrf-protection';
 import fastifySwagger from '@fastify/swagger';
 import fastifySwaggerUI from '@fastify/swagger-ui';
 import type { TypeBoxTypeProvider } from '@fastify/type-provider-typebox';
@@ -26,6 +25,7 @@ import security from './plugins/security';
 import auth from './plugins/auth';
 import bouncer from './plugins/bouncer';
 import errorHandling from './plugins/error-handling';
+import csrf, { CSRF_COOKIE, CSRF_HEADER } from './plugins/csrf';
 import notFound from './plugins/not-found';
 import * as publicRoutes from './routes/public';
 import * as protectedRoutes from './routes/protected';
@@ -88,33 +88,7 @@ export const build = async (
 
   await fastify.register(cors);
   await fastify.register(cookies);
-
-  void fastify.register(fastifyCsrfProtection, {
-    // TODO: consider signing cookies. We don't on the api-server, but we could
-    // as an extra layer of security.
-
-    ///Ignore all other possible sources of CSRF
-    // tokens since we know we can provide this one
-    getToken: req => req.headers['csrf-token'] as string,
-    cookieOpts: { signed: false, sameSite: 'strict' }
-  });
-
-  // All routes except signout should add a CSRF token to the response
-  fastify.addHook('onRequest', (_req, reply, done) => {
-    const isSignout = _req.url === '/signout' || _req.url === '/signout/';
-
-    if (!isSignout) {
-      const token = reply.generateCsrf();
-      void reply.setCookie('csrf_token', token, {
-        sameSite: 'strict',
-        signed: false,
-        // it needs to be read by the client, so that it can be sent in the
-        // header of the next request:
-        httpOnly: false
-      });
-    }
-    done();
-  });
+  await fastify.register(csrf);
 
   const provider =
     EMAIL_PROVIDER === 'ses' ? new SESProvider() : new NodemailerProvider();
@@ -136,11 +110,11 @@ export const build = async (
         requestInterceptor: req => {
           const csrfTokenCookie = document.cookie
             .split(';')
-            .find(str => str.includes('csrf_token'));
+            .find(str => str.includes(CSRF_COOKIE));
           const [_key, csrfToken] = csrfTokenCookie?.split('=') ?? [];
 
           // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-          if (csrfToken) req.headers['csrf-token'] = csrfToken.trim();
+          if (csrfToken) req.headers[CSRF_HEADER] = csrfToken.trim();
           return req;
         }
       }

api/src/plugins/cookies.ts

@@ -7,6 +7,7 @@ import {
   COOKIE_SECRET,
   FREECODECAMP_NODE_ENV
 } from '../utils/env';
+import { CSRF_COOKIE, CSRF_SECRET_COOKIE } from './csrf';
 
 export { type CookieSerializeOptions } from '@fastify/cookie';
 
@@ -68,8 +69,8 @@ const cookies: FastifyPluginCallback = (fastify, _options, done) => {
 
   void fastify.decorateReply('clearOurCookies', function () {
     void this.clearCookie('jwt_access_token');
-    void this.clearCookie('_csrf');
-    void this.clearCookie('csrf_token');
+    void this.clearCookie(CSRF_SECRET_COOKIE);
+    void this.clearCookie(CSRF_COOKIE);
   });
 
   done();

api/src/plugins/csrf.test.ts

@@ -0,0 +1,107 @@
+import Fastify, { type FastifyInstance } from 'fastify';
+
+import { COOKIE_DOMAIN } from '../utils/env';
+import cookies from './cookies';
+import csrf, { CSRF_COOKIE, CSRF_SECRET_COOKIE } from './csrf';
+
+// eslint-disable-next-line @typescript-eslint/no-unsafe-return
+jest.mock('../utils/env', () => ({
+  ...jest.requireActual('../utils/env'),
+  COOKIE_DOMAIN: 'www.example.com',
+  FREECODECAMP_NODE_ENV: 'production'
+}));
+
+async function setupServer() {
+  const fastify = Fastify();
+  await fastify.register(cookies);
+  await fastify.register(csrf);
+  // @ts-expect-error - @fastify/csrf-protection needs to update their types
+  // eslint-disable-next-line @typescript-eslint/unbound-method
+  fastify.addHook('onRequest', fastify.csrfProtection);
+
+  fastify.get('/', (_req, reply) => {
+    void reply.send({ foo: 'bar' });
+  });
+  return fastify;
+}
+
+describe('CSRF protection', () => {
+  let fastify: FastifyInstance;
+  beforeEach(async () => {
+    fastify = await setupServer();
+  });
+  it('should receive a new CSRF token with the expected properties', async () => {
+    const response = await fastify.inject({
+      method: 'GET',
+      url: '/'
+    });
+    const newCookies = response.cookies;
+    const csrfTokenCookie = newCookies.find(
+      cookie => cookie.name === CSRF_COOKIE
+    );
+
+    const { value, ...rest } = csrfTokenCookie!;
+
+    // The value is a random string - it's enough to check that it's not empty
+    expect(value).toHaveLength(52);
+
+    expect(rest).toStrictEqual({
+      name: CSRF_COOKIE,
+      path: '/',
+      sameSite: 'Strict',
+      domain: COOKIE_DOMAIN,
+      secure: true
+    });
+  });
+
+  it('should return 403 if the _csrf secret is missing', async () => {
+    const response = await fastify.inject({
+      method: 'GET',
+      url: '/'
+    });
+
+    expect(response.statusCode).toEqual(403);
+    // The response body is determined by the error-handling plugin, so we don't
+    // check it here.
+  });
+
+  it('should return 403 if the csrf_token is invalid', async () => {
+    const response = await fastify.inject({
+      method: 'GET',
+      url: '/',
+      cookies: {
+        _csrf: 'foo',
+        csrf_token: 'bar'
+      }
+    });
+    expect(response.statusCode).toEqual(403);
+  });
+
+  it('should allow the request if the csrf_token is valid', async () => {
+    const csrfResponse = await fastify.inject({
+      method: 'GET',
+      url: '/'
+    });
+
+    const csrfTokenCookie = csrfResponse.cookies.find(
+      cookie => cookie.name === CSRF_COOKIE
+    );
+    const csrfSecretCookie = csrfResponse.cookies.find(
+      cookie => cookie.name === CSRF_SECRET_COOKIE
+    );
+
+    const res = await fastify.inject({
+      method: 'GET',
+      url: '/',
+      cookies: {
+        _csrf: csrfSecretCookie!.value
+      },
+      headers: {
+        'csrf-token': csrfTokenCookie!.value
+      }
+    });
+
+    expect(res.json()).toEqual({ foo: 'bar' });
+    expect(res.statusCode).toEqual(200);
+  });
+});

api/src/plugins/csrf.ts

@@ -0,0 +1,48 @@
+import type { FastifyPluginCallback } from 'fastify';
+import fastifyCsrfProtection from '@fastify/csrf-protection';
+
+import fp from 'fastify-plugin';
+
+export const CSRF_COOKIE = 'csrf_token';
+export const CSRF_HEADER = 'csrf-token';
+export const CSRF_SECRET_COOKIE = '_csrf';
+
+/**
+ * Plugin for preventing CSRF attacks.
+ *
+ * @param fastify The Fastify instance.
+ * @param _options Options passed to the plugin via `fastify.register(plugin, options)`.
+ * @param done Callback to signal that the logic has completed.
+ */
+const csrf: FastifyPluginCallback = (fastify, _options, done) => {
+  void fastify.register(fastifyCsrfProtection, {
+    // TODO: consider signing cookies. We don't on the api-server, but we could
+    // as an extra layer of security.
+
+    ///Ignore all other possible sources of CSRF
+    // tokens since we know we can provide this one
+    getToken: req => req.headers[CSRF_HEADER] as string,
+    cookieOpts: { signed: false, sameSite: 'strict' }
+  });
+
+  // All routes except signout should add a CSRF token to the response
+  fastify.addHook('onRequest', (_req, reply, done) => {
+    const isSignout = _req.url === '/signout' || _req.url === '/signout/';
+
+    if (!isSignout) {
+      const token = reply.generateCsrf();
+      void reply.setCookie(CSRF_COOKIE, token, {
+        sameSite: 'strict',
+        signed: false,
+        // it needs to be read by the client, so that it can be sent in the
+        // header of the next request:
+        httpOnly: false
+      });
+    }
+    done();
+  });
+
+  done();
+};
+
+export default fp(csrf, { dependencies: ['cookies'] });

api/src/routes/protected/settings.test.ts

@@ -53,52 +53,6 @@ const updateErrorResponse = {
 describe('settingRoutes', () => {
   setupServer();
 
-  // TODO: rather than testing every single route, create a single test for the
-  // rest api plugin that checks that the appropriate hooks are added to all
-
-  // This only tests one route, but all of the routes in the settings plugin add
-  // the same hooks. So if this suite passes, the other routes should be
-  // protected.
-  describe('CSRF protection', () => {
-    it('should return 403 if the _csrf secret is missing', async () => {
-      const response = await superRequest('/update-my-profileui', {
-        method: 'PUT'
-      });
-
-      expect(response.body).toEqual({
-        message: 'flash.generic-error',
-        type: 'danger'
-      });
-      expect(response.statusCode).toEqual(403);
-    });
-
-    it('should return 403 if the csrf_token is invalid', async () => {
-      const response = await superRequest('/update-my-profileui', {
-        method: 'PUT'
-      }).set('Cookie', ['_csrf=foo', 'csrf-token=bar']);
-
-      expect(response.body).toEqual({
-        message: 'flash.generic-error',
-        type: 'danger'
-      });
-      expect(response.statusCode).toEqual(403);
-    });
-
-    it('should receive a new CSRF token + secret in the response', async () => {
-      const response = await superRequest('/update-my-profileui', {
-        method: 'PUT'
-      });
-
-      const newCookies = response.get('Set-Cookie');
-      expect(newCookies).toEqual(
-        expect.arrayContaining([
-          expect.stringContaining('_csrf'),
-          expect.stringContaining('csrf_token')
-        ])
-      );
-    });
-  });
-
   describe('Authenticated user', () => {
     let superPut: ReturnType<typeof createSuperRequest>;
     let superGet: ReturnType<typeof createSuperRequest>;

api/src/routes/public/signout.ts

@@ -16,9 +16,7 @@ export const signoutRoute: FastifyPluginCallback = (
   done
 ) => {
   fastify.get('/signout', async (req, reply) => {
-    void reply.clearCookie('jwt_access_token');
-    void reply.clearCookie('csrf_token');
-    void reply.clearCookie('_csrf');
+    void reply.clearOurCookies();
 
     const { returnTo } = getRedirectParams(req);
     await reply.redirect(returnTo);

api/src/server.test.ts

@@ -1,5 +1,5 @@
 import { setupServer, superRequest } from '../jest.utils';
-import { HOME_LOCATION, COOKIE_DOMAIN } from './utils/env';
+import { HOME_LOCATION } from './utils/env';
 
 jest.mock('./utils/env', () => {
   // eslint-disable-next-line @typescript-eslint/no-unsafe-return
@@ -12,26 +12,6 @@ jest.mock('./utils/env', () => {
 describe('server', () => {
   setupServer();
 
-  describe('CSRF protection', () => {
-    it('should receive a new CSRF token with the expected properties', async () => {
-      const response = await superRequest('/status/ping', { method: 'GET' });
-      const newCookies = response.get('Set-Cookie');
-      const csrfTokenCookie = newCookies.find(cookie =>
-        cookie.includes('csrf_token')
-      );
-
-      expect(csrfTokenCookie).toEqual(
-        expect.stringContaining('SameSite=Strict')
-      );
-      expect(csrfTokenCookie).toEqual(
-        expect.stringContaining(`Domain=${COOKIE_DOMAIN}`)
-      );
-      expect(csrfTokenCookie).toEqual(expect.stringContaining('Path=/'));
-      // Since we're not mocking FREECODECAMP_NODE_ENV to production, there's no
-      // point checking if it is secure (it won't be in testing).
-    });
-  });
-
   describe('GET /', () => {
     test('should have OWASP recommended headers', async () => {
       const res = await superRequest('/', { method: 'GET' });