Merge pull request #1140 from FoalTS/verify-csrf-token

Add `getCsrfTokenFromCookie` and `shouldVerifyCsrfToken` utils
FoalTS · Aug 19, 2022 · f970d92 · f970d92
2 parents 45d91fa + 62173f4
commit f970d92
Show file tree

Hide file tree

Showing 12 changed files with 628 additions and 338 deletions.
diff --git a/packages/core/src/sessions/http/use-sessions.hook.spec.ts b/packages/core/src/sessions/http/use-sessions.hook.spec.ts
@@ -493,141 +493,60 @@ describe('UseSessions', () => {
 
   describe('should verify the CSRF token and', () => {
 
-    context('given settings.session.csrf.enabled is true', () => {
+    context('given the request needs a CSRF check', () => {
 
-      beforeEach(() => {
-        Config.set('settings.session.csrf.enabled', true);
-      });
+      function createContextWithPostMethod(headers: {[name:string]: string}, cookies: {[name:string]: string}): Context {
+        return createContext(headers, cookies, {}, 'POST');
+      }
 
-      afterEach(() => Config.remove('settings.session.csrf.enabled'));
+      beforeEach(() => hook = getHookFunction(UseSessions({ store: Store, cookie: true, csrf: true })));
 
-      context('given options.cookie is false or not defined', () => {
+      it('should return an HttpResponseForbidden instance if the request has no CSRF token.', async () => {
+        ctx = createContextWithPostMethod({}, { [SESSION_DEFAULT_COOKIE_NAME]: csrfSessionID });
+        const response = await hook(ctx, services);
+        if (!isHttpResponseForbidden(response)) {
+          throw new Error('The hook should have returned a HttpResponseForbidden instance.');
+        }
 
-        beforeEach(() => ctx = createContext({ Authorization: `Bearer ${csrfSessionID}`}, {}, {}, 'POST'));
+        strictEqual(response.body, 'CSRF token missing or incorrect.');
+      });
 
-        it('should not return an HttpResponseForbidden instance if the request has no CSRF token.', async () => {
-          const response = await hook(ctx, services);
-          if (isHttpResponseForbidden(response)) {
-            throw new Error('The hook should not have returned a HttpResponseForbidden instance.');
+      it('should throw an error if the session state has no CSRF token.', async () => {
+        ctx = createContextWithPostMethod({}, { [SESSION_DEFAULT_COOKIE_NAME]: anonymousSessionID });
+        return rejects(
+          () => hook(ctx, services),
+          {
+            message: 'Unexpected error: the session content does not have a "csrfToken" field. '
+              + 'Are you sure you created the session with "createSession"?'
           }
-        });
-
+        );
       });
 
-      context('given options.cookie is true', () => {
-
-        context('given options.csrf is false', () => {
-
-          beforeEach(() => hook = getHookFunction(UseSessions({ store: Store, cookie: true, csrf: false })));
-
-          it('should NOT return an HttpResponseForbidden instance if the request has no CSRF token.', async () => {
-            ctx = createContext({}, { [SESSION_DEFAULT_COOKIE_NAME]: csrfSessionID }, {}, 'POST');
-            const response = await hook(ctx, services);
-            if (isHttpResponseForbidden(response)) {
-              throw new Error('The hook should NOT have returned a HttpResponseForbidden instance.');
-            }
-          });
-
-        });
-
-        context('given options.csrf is undefined', () => {
-
-          beforeEach(() => hook = getHookFunction(UseSessions({ store: Store, cookie: true })));
+      context('given a CSRF token is sent in the request', () => {
+        it('should return an HttpResponseForbidden instance if the CSRF token is incorrect.', async () => {
+          ctx = createContextWithPostMethod(
+            { 'X-CSRF-Token': incorrectCsrfToken },
+            { [SESSION_DEFAULT_COOKIE_NAME]: csrfSessionID },
+          );
 
-          function testUnprotectedMethod(method: HttpMethod) {
-            it('should not return an HttpResponseForbidden instance if the request has no CSRF token.', async () => {
-              ctx = createContext({}, { [SESSION_DEFAULT_COOKIE_NAME]: csrfSessionID }, {}, method);
-              const response = await hook(ctx, services);
-              if (isHttpResponseForbidden(response)) {
-                throw new Error('The hook should not have returned a HttpResponseForbidden instance.');
-              }
-            });
+          const response = await hook(ctx, services);
+          if (!isHttpResponseForbidden(response)) {
+            throw new Error('The hook should have returned a HttpResponseForbidden instance.');
           }
 
-          context('given the request HTTP method is "GET"', () => {
-            testUnprotectedMethod('GET');
-          });
-
-          context('given the request HTTP method is "HEAD"', () => {
-            testUnprotectedMethod('HEAD');
-          });
-
-          context('given the request HTTP method is "OPTIONS"', () => {
-            testUnprotectedMethod('OPTIONS');
-          });
-
-          function testProtectedMethod(method: HttpMethod) {
-            it('should return an HttpResponseForbidden instance if the request has no CSRF token.', async () => {
-              ctx = createContext({}, { [SESSION_DEFAULT_COOKIE_NAME]: csrfSessionID }, {}, method);
-              const response = await hook(ctx, services);
-              if (!isHttpResponseForbidden(response)) {
-                throw new Error('The hook should have returned a HttpResponseForbidden instance.');
-              }
-
-              strictEqual(response.body, 'CSRF token missing or incorrect.');
-            });
-
-            it('should throw an error if the session state has no CSRF token.', async () => {
-              ctx = createContext({}, { [SESSION_DEFAULT_COOKIE_NAME]: anonymousSessionID }, {}, method);
-              return rejects(
-                () => hook(ctx, services),
-                {
-                  message: 'Unexpected error: the session content does not have a "csrfToken" field. '
-                    + 'Are you sure you created the session with "createSession"?'
-                }
-              );
-            });
-
-            context('given a CSRF token is sent in the request', () => {
-              it('should return an HttpResponseForbidden instance if the CSRF token is incorrect.', async () => {
-                ctx = createContext(
-                  { 'X-CSRF-Token': incorrectCsrfToken },
-                  { [SESSION_DEFAULT_COOKIE_NAME]: csrfSessionID },
-                  {},
-                  method,
-                );
-
-                const response = await hook(ctx, services);
-                if (!isHttpResponseForbidden(response)) {
-                  throw new Error('The hook should have returned a HttpResponseForbidden instance.');
-                }
-
-                strictEqual(response.body, 'CSRF token missing or incorrect.');
-              });
-
-              it('should not return an HttpResponseForbidden instance if the CSRF token is correct.', async () => {
-                ctx = createContext(
-                  { 'X-CSRF-Token': csrfToken },
-                  { [SESSION_DEFAULT_COOKIE_NAME]: csrfSessionID },
-                  {},
-                  method,
-                );
+          strictEqual(response.body, 'CSRF token missing or incorrect.');
+        });
 
-                const response = await hook(ctx, services);
-                if (isHttpResponseForbidden(response)) {
-                  throw new Error('The hook should NOT have returned a HttpResponseForbidden instance.');
-                }
-              });
+        it('should not return an HttpResponseForbidden instance if the CSRF token is correct.', async () => {
+          ctx = createContextWithPostMethod(
+            { 'X-CSRF-Token': csrfToken },
+            { [SESSION_DEFAULT_COOKIE_NAME]: csrfSessionID },
+          );
 
-            });
+          const response = await hook(ctx, services);
+          if (isHttpResponseForbidden(response)) {
+            throw new Error('The hook should NOT have returned a HttpResponseForbidden instance.');
           }
-
-          context('given the request HTTP method is "POST"', () => {
-            testProtectedMethod('POST');
-          });
-
-          context('given the request HTTP method is "PUT"', () => {
-            testProtectedMethod('PUT');
-          });
-
-          context('given the request HTTP method is "PATCH"', () => {
-            testProtectedMethod('PATCH');
-          });
-
-          context('given the request HTTP method is "DELETE"', () => {
-            testProtectedMethod('DELETE');
-          });
-
         });
 
       });

diff --git a/packages/core/src/sessions/http/use-sessions.hook.ts b/packages/core/src/sessions/http/use-sessions.hook.ts
@@ -23,6 +23,7 @@ import { FetchUser } from './fetch-user.interface';
 import { removeSessionCookie } from './remove-session-cookie';
 import { setSessionCookie } from './set-session-cookie';
 import { getCsrfTokenFromRequest } from './get-csrf-token-from-request';
+import { shouldVerifyCsrfToken } from './utils';
 
 export interface UseSessionOptions {
   user?: FetchUser;
@@ -134,11 +135,7 @@ export function UseSessions(options: UseSessionOptions = {}): HookDecorator {
 
     /* Verify CSRF token */
 
-    if (
-      options.cookie &&
-      (options.csrf ?? Config.get('settings.session.csrf.enabled', 'boolean', false)) &&
-      ![ 'GET', 'HEAD', 'OPTIONS' ].includes(ctx.request.method)
-    ) {
+    if (shouldVerifyCsrfToken(ctx.request, options)) {
       const expectedCsrftoken = session.get<string|undefined>('csrfToken');
       if (!expectedCsrftoken) {
         throw new Error(

diff --git a/packages/core/src/sessions/http/utils/index.ts b/packages/core/src/sessions/http/utils/index.ts
@@ -0,0 +1 @@
+export { shouldVerifyCsrfToken } from './should-verify-csrf-token';
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		export { shouldVerifyCsrfToken } from './should-verify-csrf-token';