feat(api): add csrf protection (#50275)

Co-authored-by: Sboonny <muhammed@freecodecamp.org>

api/jest.utils.test.ts

@@ -0,0 +1,19 @@
+import { getCsrfToken } from './jest.utils';
+
+const fakeCookies = [
+  '_csrf=123; Path=/; HttpOnly; SameSite=Strict',
+  'csrf_token=abc-123; Path=/',
+  'sessionId=CV-abc.123; Path=/; Expires=Wed, 03 May 2023 16:29:53 GMT; HttpOnly'
+];
+
+describe('getCsrfToken', () => {
+  test('returns csrf token if there is one', () => {
+    expect(getCsrfToken(fakeCookies)).toEqual('abc-123');
+  });
+
+  test('returns undefined if there is no csrf token', () => {
+    expect(
+      getCsrfToken(['_csrf=123; Path=/; HttpOnly; SameSite=Strict'])
+    ).toBeUndefined();
+  });
+});

api/jest.utils.ts

@@ -2,31 +2,17 @@ import request from 'supertest';
 
 import { build } from './src/app';
 
+type FastifyTestInstance = Awaited<ReturnType<typeof build>>;
+
 declare global {
   // eslint-disable-next-line no-var
-  var fastifyTestInstance: Awaited<ReturnType<typeof build>> | undefined;
+  var fastifyTestInstance: FastifyTestInstance;
 }
 
 type Options = {
   sendCSRFToken: boolean;
 };
 
-// TODO: remove this function and use superRequest instead
-export function superPut(
-  resource: string,
-  setCookies: string[],
-  opts?: Options
-): request.Test {
-  return superRequest(
-    resource,
-    {
-      method: 'PUT',
-      setCookies
-    },
-    opts
-  );
-}
-
 /* eslint-disable @typescript-eslint/naming-convention */
 const requests = {
   GET: (resource: string) => request(fastifyTestInstance?.server).get(resource),
@@ -72,7 +58,7 @@ export function superRequest(
 }
 
 export function setupServer(): void {
-  let fastify: Awaited<ReturnType<typeof build>> | undefined;
+  let fastify: FastifyTestInstance;
   beforeAll(async () => {
     fastify = await build();
     await fastify.ready();
@@ -83,6 +69,6 @@ export function setupServer(): void {
   afterAll(async () => {
     // Due to a prisma bug, this is not enough, we need to --force-exit jest:
     // https://github.com/prisma/prisma/issues/18146
-    await fastifyTestInstance?.close();
+    await fastifyTestInstance.close();
   });
 }

api/package.json

@@ -6,6 +6,7 @@
   "dependencies": {
     "@fastify/cookie": "^8.3.0",
     "@fastify/middie": "8.3",
+    "@fastify/csrf-protection": "6.3.0",
     "@fastify/session": "^10.1.1",
     "@fastify/swagger": "^8.3.1",
     "@fastify/swagger-ui": "^1.5.0",

api/src/app.ts

@@ -14,8 +14,9 @@ import MongoStore from 'connect-mongo';
 import type { TypeBoxTypeProvider } from '@fastify/type-provider-typebox';
 import fastifySwagger from '@fastify/swagger';
 import fastifySwaggerUI from '@fastify/swagger-ui';
-import fastifySentry from './plugins/fastify-sentry';
+import fastifyCsrfProtection from '@fastify/csrf-protection';
 
+import fastifySentry from './plugins/fastify-sentry';
 import cors from './plugins/cors';
 import jwtAuthz from './plugins/fastify-jwt-authz';
 import security from './plugins/security';
@@ -29,6 +30,7 @@ import prismaPlugin from './db/prisma';
 import {
   AUTH0_AUDIENCE,
   AUTH0_DOMAIN,
+  COOKIE_DOMAIN,
   FREECODECAMP_NODE_ENV,
   MONGOHQ_URL,
   SESSION_SECRET,
@@ -66,6 +68,33 @@ export const build = async (
 
   await fastify.register(cors);
   await fastify.register(fastifyCookie);
+
+  // @ts-expect-error @fastify/csrf-protection is overly restrictive, here. It
+  // requires an hmacKey if getToken is provided, but that should only be a
+  // requirement if the getUserInfo function is provided.
+  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
+  });
+
+  // All routes should add a CSRF token to the response
+  fastify.addHook('onRequest', (_req, reply, done) => {
+    const token = reply.generateCsrf();
+    // Path is necessary to ensure that only one cookie is set and it is valid
+    // for all routes.
+    void reply.setCookie('csrf_token', token, {
+      path: '/',
+      sameSite: 'strict',
+      domain: COOKIE_DOMAIN,
+      secure: FREECODECAMP_NODE_ENV === 'production'
+    });
+    done();
+  });
+
   // @ts-expect-error - @fastify/session's types are not, yet, compatible with
   // express-session's types
   await fastify.register(fastifySession, {
@@ -101,7 +130,21 @@ export const build = async (
         security: [{ session: [] }]
       }
     });
-    void fastify.register(fastifySwaggerUI);
+    void fastify.register(fastifySwaggerUI, {
+      uiConfig: {
+        // Convert csrf_token cookie to csrf-token header
+        requestInterceptor: req => {
+          const csrfTokenCookie = document.cookie
+            .split(';')
+            .find(str => str.includes('csrf_token'));
+          const [_key, csrfToken] = csrfTokenCookie?.split('=') ?? [];
+
+          // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
+          if (csrfToken) req.headers['csrf-token'] = csrfToken.trim();
+          return req;
+        }
+      }
+    });
     fastify.log.info(`Swagger UI available at ${API_LOCATION}/documentation`);
   }
 

api/src/routes/auth.ts

@@ -108,7 +108,7 @@ export const devLoginCallback: FastifyPluginCallback = (
   _options,
   done
 ) => {
-  fastify.get('/dev-callback', async (req, _res) => {
+  fastify.get('/dev-callback', async req => {
     const email = 'foo@bar.com';
 
     const { id } = await findOrCreateUser(fastify, email);
@@ -122,7 +122,7 @@ export const devLoginCallback: FastifyPluginCallback = (
 export const auth0Routes: FastifyPluginCallback = (fastify, _options, done) => {
   fastify.addHook('onRequest', fastify.authenticate);
 
-  fastify.get('/callback', async (req, _res) => {
+  fastify.get('/callback', async req => {
     const email = await getEmailFromAuth0(req);
 
     const { id } = await findOrCreateUser(fastify, email);

api/src/routes/settings.test.ts

@@ -1,6 +1,6 @@
 import request from 'supertest';
 
-import { build } from '../app';
+import { setupServer, superRequest } from '../../jest.utils';
 
 const baseProfileUI = {
   isLocked: false,
@@ -26,42 +26,88 @@ const profileUI = {
 };
 
 describe('settingRoutes', () => {
-  let fastify: undefined | Awaited<ReturnType<typeof build>>;
+  setupServer();
 
-  beforeAll(async () => {
-    fastify = await build();
-    await fastify.ready();
-  }, 20000);
+  // 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
 
-  afterAll(async () => {
-    await fastify?.close();
+  // 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 request(fastifyTestInstance.server).put(
+        '/update-my-profileui'
+      );
+
+      expect(response.statusCode).toEqual(403);
+      expect(response.body).toEqual({
+        code: 'FST_CSRF_MISSING_SECRET',
+        error: 'Forbidden',
+        message: 'Missing csrf secret',
+        statusCode: 403
+      });
+    });
+
+    it('should return 403 if the csrf_token is invalid', async () => {
+      const response = await request(fastifyTestInstance.server)
+        .put('/update-my-profileui')
+        .set('Cookie', ['_csrf=foo', 'csrf-token=bar']);
+
+      expect(response.statusCode).toEqual(403);
+      expect(response.body).toEqual({
+        code: 'FST_CSRF_INVALID_TOKEN',
+        error: 'Forbidden',
+        message: 'Invalid csrf token',
+        statusCode: 403
+      });
+    });
+
+    it('should receive a new CSRF token + secret in the response', async () => {
+      const response = await request(fastifyTestInstance.server).put(
+        '/update-my-profileui'
+      );
+
+      const newCookies = response.get('Set-Cookie');
+      expect(newCookies).toEqual(
+        expect.arrayContaining([
+          expect.stringContaining('_csrf'),
+          expect.stringContaining('csrf_token')
+        ])
+      );
+    });
   });
 
   describe('Authenticated user', () => {
-    let cookies: string[];
+    let setCookies: string[];
 
+    // Authenticate user
     beforeAll(async () => {
-      await fastify?.prisma.user.updateMany({
+      await fastifyTestInstance.prisma.user.updateMany({
         where: { email: 'foo@bar.com' },
         data: { profileUI: baseProfileUI }
       });
-      const res = await request(fastify?.server).get('/auth/dev-callback');
-      cookies = res.get('Set-Cookie');
+      const res = await request(fastifyTestInstance.server).get(
+        '/auth/dev-callback'
+      );
+      setCookies = res.get('Set-Cookie');
     });
 
     describe('/update-my-profileui', () => {
       test('PUT returns 200 status code with "success" message', async () => {
-        const response = await request(fastify?.server)
-          .put('/update-my-profileui')
-          .set('Cookie', cookies)
-          .send({ profileUI });
+        const response = await superRequest('/update-my-profileui', {
+          method: 'PUT',
+          setCookies
+        }).send({
+          profileUI
+        });
 
-        const user = await fastify?.prisma.user.findFirst({
+        const user = await fastifyTestInstance.prisma.user.findFirst({
           where: { email: 'foo@bar.com' }
         });
 
-        expect(response?.statusCode).toEqual(200);
-        expect(response?.body).toEqual({
+        expect(response.statusCode).toEqual(200);
+        expect(response.body).toEqual({
           message: 'flash.privacy-updated',
           type: 'success'
         });
@@ -69,40 +115,40 @@ describe('settingRoutes', () => {
       });
 
       test('PUT ignores invalid keys', async () => {
-        const response = await request(fastify?.server)
-          .put('/update-my-profileui')
-          .set('Cookie', cookies)
-          .send({
-            profileUI: {
-              ...profileUI,
-              invalidKey: 'invalidValue'
-            }
-          });
+        const response = await superRequest('/update-my-profileui', {
+          method: 'PUT',
+          setCookies
+        }).send({
+          profileUI: {
+            ...profileUI,
+            invalidKey: 'invalidValue'
+          }
+        });
 
-        const user = await fastify?.prisma.user.findFirst({
+        const user = await fastifyTestInstance.prisma.user.findFirst({
           where: { email: 'foo@bar.com' }
         });
 
-        expect(response?.statusCode).toEqual(200);
+        expect(response.statusCode).toEqual(200);
         expect(user?.profileUI).toEqual(profileUI);
       });
 
       test('PUT returns 400 status code with missing keys', async () => {
-        const response = await request(fastify?.server)
-          .put('/update-my-profileui')
-          .set('Cookie', cookies)
-          .send({
-            profileUI: {
-              isLocked: true,
-              showName: true,
-              showPoints: false,
-              showPortfolio: true,
-              showTimeLine: false
-            }
-          });
+        const response = await superRequest('/update-my-profileui', {
+          method: 'PUT',
+          setCookies
+        }).send({
+          profileUI: {
+            isLocked: true,
+            showName: true,
+            showPoints: false,
+            showPortfolio: true,
+            showTimeLine: false
+          }
+        });
 
-        expect(response?.statusCode).toEqual(400);
-        expect(response?.body).toEqual({
+        expect(response.statusCode).toEqual(400);
+        expect(response.body).toEqual({
           code: 'FST_ERR_VALIDATION',
           error: 'Bad Request',
           message: `body/profileUI must have required property 'showAbout'`,
@@ -113,35 +159,39 @@ describe('settingRoutes', () => {
 
     describe('/update-my-theme', () => {
       test('PUT returns 200 status code with "success" message', async () => {
-        const response = await request(fastify?.server)
-          .put('/update-my-theme')
-          .set('Cookie', cookies)
-          .send({ theme: 'night' });
+        const response = await superRequest('/update-my-theme', {
+          method: 'PUT',
+          setCookies
+        }).send({
+          theme: 'night'
+        });
 
-        expect(response?.statusCode).toEqual(200);
+        expect(response.statusCode).toEqual(200);
 
-        expect(response?.body).toEqual({
+        expect(response.body).toEqual({
           message: 'flash.updated-themes',
           type: 'success'
         });
       });
 
       test('PUT returns 400 status code with invalid theme', async () => {
-        const response = await request(fastify?.server)
-          .put('/update-my-theme')
-          .set('Cookie', cookies)
-          .send({ theme: 'invalid' });
+        const response = await superRequest('/update-my-theme', {
+          method: 'PUT',
+          setCookies
+        }).send({
+          theme: 'invalid'
+        });
 
-        expect(response?.statusCode).toEqual(400);
+        expect(response.statusCode).toEqual(400);
       });
     });
 
     describe('/update-my-keyboard-shortcuts', () => {
       test('PUT returns 200 status code with "success" message', async () => {
-        const response = await request(fastify?.server)
-          .put('/update-my-keyboard-shortcuts')
-          .set('Cookie', cookies)
-          .send({ keyboardShortcuts: true });
+        const response = await superRequest('/update-my-keyboard-shortcuts', {
+          method: 'PUT',
+          setCookies
+        }).send({ keyboardShortcuts: true });
 
         expect(response?.statusCode).toEqual(200);
 
@@ -152,21 +202,21 @@ describe('settingRoutes', () => {
       });
 
       test('PUT returns 400 status code with invalid shortcuts setting', async () => {
-        const response = await request(fastify?.server)
-          .put('/update-my-keyboard-shortcuts')
-          .set('Cookie', cookies)
-          .send({ keyboardShortcuts: 'invalid' });
+        const response = await superRequest('/update-my-keyboard-shortcuts', {
+          method: 'PUT',
+          setCookies
+        }).send({ keyboardShortcuts: 'invalid' });
 
-        expect(response?.statusCode).toEqual(400);
+        expect(response.statusCode).toEqual(400);
       });
     });
 
     describe('/update-my-quincy-email', () => {
       test('PUT returns 200 status code with "success" message', async () => {
-        const response = await request(fastify?.server)
-          .put('/update-my-quincy-email')
-          .set('Cookie', cookies)
-          .send({ sendQuincyEmail: true });
+        const response = await superRequest('/update-my-quincy-email', {
+          method: 'PUT',
+          setCookies
+        }).send({ sendQuincyEmail: true });
 
         expect(response?.statusCode).toEqual(200);
 
@@ -177,10 +227,10 @@ describe('settingRoutes', () => {
       });
 
       test('PUT returns 400 status code with invalid sendQuincyEmail', async () => {
-        const response = await request(fastify?.server)
-          .put('/update-my-quincy-email')
-          .set('Cookie', cookies)
-          .send({ sendQuincyEmail: 'invalid' });
+        const response = await superRequest('/update-my-quincy-email', {
+          method: 'PUT',
+          setCookies
+        }).send({ sendQuincyEmail: 'invalid' });
 
         expect(response?.statusCode).toEqual(400);
       });
@@ -188,10 +238,10 @@ describe('settingRoutes', () => {
 
     describe('/update-my-honesty', () => {
       test('PUT returns 200 status code with "success" message', async () => {
-        const response = await request(fastify?.server)
-          .put('/update-my-honesty')
-          .set('Cookie', cookies)
-          .send({ isHonest: true });
+        const response = await superRequest('/update-my-honesty', {
+          method: 'PUT',
+          setCookies
+        }).send({ isHonest: true });
 
         expect(response?.statusCode).toEqual(200);
 
@@ -202,10 +252,10 @@ describe('settingRoutes', () => {
       });
 
       test('PUT returns 400 status code with invalid honesty', async () => {
-        const response = await request(fastify?.server)
-          .put('/update-my-honesty')
-          .set('Cookie', cookies)
-          .send({ isHonest: false });
+        const response = await superRequest('/update-my-honesty', {
+          method: 'PUT',
+          setCookies
+        }).send({ isHonest: false });
 
         expect(response?.statusCode).toEqual(400);
       });
@@ -213,10 +263,10 @@ describe('settingRoutes', () => {
 
     describe('/update-privacy-terms', () => {
       test('PUT returns 200 status code with "success" message', async () => {
-        const response = await request(fastify?.server)
-          .put('/update-privacy-terms')
-          .set('Cookie', cookies)
-          .send({ quincyEmails: true });
+        const response = await superRequest('/update-privacy-terms', {
+          method: 'PUT',
+          setCookies
+        }).send({ quincyEmails: true });
 
         expect(response?.statusCode).toEqual(200);
 
@@ -227,10 +277,10 @@ describe('settingRoutes', () => {
       });
 
       test('PUT returns 400 status code with non-boolean data', async () => {
-        const response = await request(fastify?.server)
-          .put('/update-privacy-terms')
-          .set('Cookie', cookies)
-          .send({ quincyEmails: '123' });
+        const response = await superRequest('/update-privacy-terms', {
+          method: 'PUT',
+          setCookies
+        }).send({ quincyEmails: '123' });
 
         expect(response?.statusCode).toEqual(400);
         expect(response?.body).toEqual({
@@ -244,24 +294,37 @@ describe('settingRoutes', () => {
   });
 
   describe('Unauthenticated User', () => {
-    test('PUT /update-my-profileui returns 401 status code for un-authenticated users', async () => {
-      const response = await request(fastify?.server).put(
-        '/update-my-profileui'
-      );
+    let setCookies: string[];
 
-      expect(response?.statusCode).toEqual(401);
+    // Get the CSRF cookies from an unprotected route
+    beforeAll(async () => {
+      const res = await request(fastifyTestInstance.server).get('/');
+      setCookies = res.get('Set-Cookie');
+    });
+
+    test('PUT /update-my-profileui returns 401 status code for un-authenticated users', async () => {
+      const response = await superRequest('/update-my-profileui', {
+        method: 'PUT',
+        setCookies
+      });
+
+      expect(response.statusCode).toEqual(401);
     });
 
     test('PUT /update-my-theme returns 401 status code for un-authenticated users', async () => {
-      const response = await request(fastify?.server).put('/update-my-theme');
+      const response = await superRequest('/update-my-theme', {
+        method: 'PUT',
+        setCookies
+      });
 
-      expect(response?.statusCode).toEqual(401);
+      expect(response.statusCode).toEqual(401);
     });
 
     test('PUT /update-privacy-terms returns 401 status code for un-authenticated users', async () => {
-      const response = await request(fastify?.server).put(
-        '/update-privacy-terms'
-      );
+      const response = await superRequest('/update-privacy-terms', {
+        method: 'PUT',
+        setCookies
+      });
 
       expect(response?.statusCode).toEqual(401);
     });

api/src/routes/settings.ts

@@ -8,6 +8,10 @@ export const settingRoutes: FastifyPluginCallbackTypebox = (
   _options,
   done
 ) => {
+  // The order matters here, since we want to reject invalid cross site requests
+  // before checking if the user is authenticated.
+  // eslint-disable-next-line @typescript-eslint/unbound-method
+  fastify.addHook('onRequest', fastify.csrfProtection);
   fastify.addHook('onRequest', fastify.authenticateSession);
 
   fastify.put(

api/src/server.test.ts

@@ -1,8 +1,38 @@
 import { setupServer, superRequest } from '../jest.utils';
-import { HOME_LOCATION } from './utils/env';
+import { HOME_LOCATION, COOKIE_DOMAIN } from './utils/env';
+
+jest.mock('./utils/env', () => {
+  // eslint-disable-next-line @typescript-eslint/no-unsafe-return
+  return {
+    ...jest.requireActual('./utils/env'),
+    // eslint-disable-next-line @typescript-eslint/naming-convention
+    COOKIE_DOMAIN: '.freecodecamp.org'
+  };
+});
 
 describe('server', () => {
   setupServer();
+
+  describe('CSRF protection', () => {
+    it('should receive a new CSRF token with the expected properties', async () => {
+      const response = await superRequest('/', { 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('have a 200 response', async () => {
       const res = await superRequest('/', { method: 'GET' });

api/src/utils/env.ts

@@ -33,6 +33,7 @@ assert.ok(process.env.FCC_ENABLE_SWAGGER_UI);
 assert.ok(process.env.FCC_ENABLE_DEV_LOGIN_MODE);
 
 if (process.env.FREECODECAMP_NODE_ENV !== 'development') {
+  assert.ok(process.env.COOKIE_DOMAIN);
   assert.ok(process.env.PORT);
   assert.ok(process.env.MONGOHQ_URL);
   assert.ok(process.env.SENTRY_DSN);
@@ -70,3 +71,4 @@ export const SENTRY_DSN =
   process.env.SENTRY_DSN === 'dsn_from_sentry_dashboard'
     ? ''
     : process.env.SENTRY_DSN;
+export const COOKIE_DOMAIN = process.env.COOKIE_DOMAIN || 'localhost';

api/tsconfig.json

@@ -7,6 +7,7 @@
     "forceConsistentCasingInFileNames": true,
     "esModuleInterop": true,
     "resolveJsonModule": true,
-    "noEmit": true
+    "noEmit": true,
+    "noUncheckedIndexedAccess": true
   }
 }

pnpm-lock.yaml

@@ -167,6 +167,9 @@ importers:
       '@fastify/cookie':
         specifier: ^8.3.0
         version: 8.3.0
+      '@fastify/csrf-protection':
+        specifier: 6.3.0
+        version: 6.3.0
       '@fastify/middie':
         specifier: '8.3'
         version: 8.3.0
@@ -6236,6 +6239,18 @@ packages:
       fastify-plugin: 4.5.0
     dev: false
 
+  /@fastify/csrf-protection@6.3.0:
+    resolution: {integrity: sha512-RktewyPbVMSbKrRKmn0VzKfqtRp+0mYodAz7tbJMmz1QJFWFeP5F5mS8Ym1djaM0EFqCBItIceJizeLOD+UILA==}
+    dependencies:
+      '@fastify/csrf': 6.2.0
+      '@fastify/error': 3.2.0
+      fastify-plugin: 4.5.0
+    dev: false
+
+  /@fastify/csrf@6.2.0:
+    resolution: {integrity: sha512-DvBvhhAjTh4JxH7u/HAaXY6uawrEA3lGrJNTDF2R0sojwXdmFF50an2nnnHMBIsABRyNh9TueZjzgznRCe74yA==}
+    dev: false
+
   /@fastify/deepmerge@1.3.0:
     resolution: {integrity: sha512-J8TOSBq3SoZbDhM9+R/u77hP93gz/rajSA+K2kGyijPpORPWUXHUpTaleoj+92As0S9uPRP7Oi8IqMf0u+ro6A==}
     dev: false