Просмотр исходного кода

refactor safe-redirect and convert test by jest to vitest

Yuki Takei 2 лет назад
Родитель
Сommit
37eb30ce7e

+ 3 - 1
apps/app/src/server/crowi/express-init.js

@@ -5,6 +5,8 @@ import qs from 'qs';
 import loggerFactory from '~/utils/logger';
 import { resolveFromRoot } from '~/utils/project-dir-utils';
 
+import registerSafeRedirectFactory from '../middlewares/safe-redirect';
+
 const logger = loggerFactory('growi:crowi:express-init');
 
 module.exports = function(crowi, app) {
@@ -22,7 +24,7 @@ module.exports = function(crowi, app) {
   const mongoSanitize = require('express-mongo-sanitize');
 
   const promster = require('../middlewares/promster')(crowi, app);
-  const registerSafeRedirect = require('../middlewares/safe-redirect')();
+  const registerSafeRedirect = registerSafeRedirectFactory();
   const injectCurrentuserToLocalvars = require('../middlewares/inject-currentuser-to-localvars')();
   const autoReconnectToS2sMsgServer = require('../middlewares/auto-reconnect-to-s2s-msg-server')(crowi);
 

+ 30 - 30
apps/app/test/unit/middlewares/safe-redirect.test.js → apps/app/src/server/middlewares/safe-redirect.spec.ts

@@ -1,105 +1,105 @@
-/* eslint-disable arrow-body-style */
+import {
+  vi,
+  beforeEach,
+  describe, test, expect,
+} from 'vitest';
 
-describe('safeRedirect', () => {
-  let registerSafeRedirect;
+import type { Request } from 'express';
+
+import registerSafeRedirectFactory, { type ResWithSafeRedirect } from './safe-redirect';
 
+describe('safeRedirect', () => {
   const whitelistOfHosts = [
     'white1.example.com:8080',
     'white2.example.com',
   ];
-
-  beforeEach(async() => {
-    registerSafeRedirect = require('~/server/middlewares/safe-redirect')(whitelistOfHosts);
-  });
+  const registerSafeRedirect = registerSafeRedirectFactory(whitelistOfHosts);
 
   describe('res.safeRedirect', () => {
     // setup req/res/next
+    const getFunc = vi.fn().mockReturnValue('example.com');
     const req = {
       protocol: 'http',
       hostname: 'example.com',
-      get: jest.fn().mockReturnValue('example.com'),
-    };
+      get: getFunc,
+    } as any as Request;
+
+    const redirect = vi.fn();
     const res = {
-      redirect: jest.fn().mockReturnValue('redirect'),
-    };
-    const next = jest.fn();
+      redirect,
+    } as any as ResWithSafeRedirect;
+    const next = vi.fn();
+
+    beforeEach(() => {
+      getFunc.mockClear();
+      redirect.mockClear();
+      next.mockClear();
+    });
 
     test('redirects to \'/\' because specified url causes open redirect vulnerability', () => {
       registerSafeRedirect(req, res, next);
 
-      const result = res.safeRedirect('//evil.example.com');
+      res.safeRedirect('//evil.example.com');
 
       expect(next).toHaveBeenCalledTimes(1);
-      expect(req.get).toHaveBeenCalledTimes(1);
       expect(req.get).toHaveBeenCalledWith('host');
       expect(res.redirect).toHaveBeenCalledTimes(1);
       expect(res.redirect).toHaveBeenCalledWith('/');
-      expect(result).toBe('redirect');
     });
 
     test('redirects to \'/\' because specified host without port is not in whitelist', () => {
       registerSafeRedirect(req, res, next);
 
-      const result = res.safeRedirect('http://white1.example.com/path/to/page');
+      res.safeRedirect('http://white1.example.com/path/to/page');
 
       expect(next).toHaveBeenCalledTimes(1);
-      expect(req.get).toHaveBeenCalledTimes(1);
       expect(req.get).toHaveBeenCalledWith('host');
       expect(res.redirect).toHaveBeenCalledTimes(1);
       expect(res.redirect).toHaveBeenCalledWith('/');
-      expect(result).toBe('redirect');
     });
 
     test('redirects to the specified local url', () => {
       registerSafeRedirect(req, res, next);
 
-      const result = res.safeRedirect('/path/to/page');
+      res.safeRedirect('/path/to/page');
 
       expect(next).toHaveBeenCalledTimes(1);
-      expect(req.get).toHaveBeenCalledTimes(1);
       expect(req.get).toHaveBeenCalledWith('host');
       expect(res.redirect).toHaveBeenCalledTimes(1);
       expect(res.redirect).toHaveBeenCalledWith('http://example.com/path/to/page');
-      expect(result).toBe('redirect');
     });
 
     test('redirects to the specified local url (fqdn)', () => {
       registerSafeRedirect(req, res, next);
 
-      const result = res.safeRedirect('http://example.com/path/to/page');
+      res.safeRedirect('http://example.com/path/to/page');
 
       expect(next).toHaveBeenCalledTimes(1);
-      expect(req.get).toHaveBeenCalledTimes(1);
       expect(req.get).toHaveBeenCalledWith('host');
       expect(res.redirect).toHaveBeenCalledTimes(1);
       expect(res.redirect).toHaveBeenCalledWith('http://example.com/path/to/page');
-      expect(result).toBe('redirect');
     });
 
     test('redirects to the specified whitelisted url (white1.example.com:8080)', () => {
       registerSafeRedirect(req, res, next);
 
-      const result = res.safeRedirect('http://white1.example.com:8080/path/to/page');
+      res.safeRedirect('http://white1.example.com:8080/path/to/page');
 
       expect(next).toHaveBeenCalledTimes(1);
-      expect(req.get).toHaveBeenCalledTimes(1);
       expect(req.get).toHaveBeenCalledWith('host');
       expect(res.redirect).toHaveBeenCalledTimes(1);
       expect(res.redirect).toHaveBeenCalledWith('http://white1.example.com:8080/path/to/page');
-      expect(result).toBe('redirect');
     });
 
     test('redirects to the specified whitelisted url (white2.example.com:8080)', () => {
       registerSafeRedirect(req, res, next);
 
-      const result = res.safeRedirect('http://white2.example.com:8080/path/to/page');
+      res.safeRedirect('http://white2.example.com:8080/path/to/page');
 
       expect(next).toHaveBeenCalledTimes(1);
-      expect(req.get).toHaveBeenCalledTimes(1);
       expect(req.get).toHaveBeenCalledWith('host');
       expect(res.redirect).toHaveBeenCalledTimes(1);
       expect(res.redirect).toHaveBeenCalledWith('http://white2.example.com:8080/path/to/page');
-      expect(result).toBe('redirect');
     });
 
   });

+ 6 - 4
apps/app/src/server/middlewares/safe-redirect.ts

@@ -4,7 +4,7 @@
  * Usage: app.use(require('middlewares/safe-redirect')(['example.com', 'some.example.com:8080']))
  */
 
-import {
+import type {
   Request, Response, NextFunction,
 } from 'express';
 
@@ -31,13 +31,13 @@ function isInWhitelist(whitelistOfHosts: string[], redirectToFqdn: string): bool
 }
 
 
-type ResWithSafeRedirect = Response & {
+export type ResWithSafeRedirect = Response & {
   safeRedirect: (redirectTo?: string) => void,
 }
 
-module.exports = (whitelistOfHosts: string[]) => {
+const factory = (whitelistOfHosts: string[]) => {
 
-  return function(req: Request, res: ResWithSafeRedirect, next: NextFunction) {
+  return (req: Request, res: ResWithSafeRedirect, next: NextFunction): void => {
 
     // extend res object
     res.safeRedirect = function(redirectTo?: string) {
@@ -75,3 +75,5 @@ module.exports = (whitelistOfHosts: string[]) => {
   };
 
 };
+
+export default factory;