Merge branch 'master' into feat/gw7759-change-logo-on-top-navigation-bar

packages/app/src/server/middlewares/login-required.js

@@ -12,18 +12,6 @@ module.exports = (crowi, isGuestAllowed = false, fallback = null) => {
 
   return function(req, res, next) {
 
-    // check the route config and ACL
-    if (isGuestAllowed && crowi.aclService.isGuestAllowedToRead()) {
-      logger.debug('Allowed to read: ', req.path);
-      return next();
-    }
-
-    // check the page is shared
-    if (isGuestAllowed && req.isSharedPage) {
-      logger.debug('Target page is shared page');
-      return next();
-    }
-
     const User = crowi.model('User');
 
     // check the user logged in
@@ -43,6 +31,18 @@ module.exports = (crowi, isGuestAllowed = false, fallback = null) => {
       }
     }
 
+    // check the route config and ACL
+    if (isGuestAllowed && crowi.aclService.isGuestAllowedToRead()) {
+      logger.debug('Allowed to read: ', req.path);
+      return next();
+    }
+
+    // check the page is shared
+    if (isGuestAllowed && req.isSharedPage) {
+      logger.debug('Target page is shared page');
+      return next();
+    }
+
     // is api path
     const baseUrl = req.baseUrl || '';
     if (baseUrl.match(/^\/_api\/.+$/)) {

packages/app/src/server/routes/apiv3/forgot-password.js

@@ -42,10 +42,10 @@ module.exports = (crowi) => {
   };
 
   const apiLimiter = rateLimit({
-    windowMs: 15 * 60 * 1000, // 15 minutes
-    max: 10, // limit each IP to 10 requests per windowMs
+    windowMs: 1 * 60 * 1000, // 1 minutes
+    max: 30, // limit each IP to 30 requests per windowMs
     message:
-      'Too many requests were sent from this IP. Please try a password reset request again on the password reset request form',
+    'Too many requests were sent from this IP. Please try a password reset request again on the password reset request form',
   });
 
   const checkPassportStrategyMiddleware = checkForgotPasswordEnabledMiddlewareFactory(crowi, true);

packages/app/src/server/routes/index.js

@@ -20,10 +20,10 @@ const multer = require('multer');
 const autoReap = require('multer-autoreap');
 
 const apiLimiter = rateLimit({
-  windowMs: 15 * 60 * 1000, // 15 minutes
-  max: 10, // limit each IP to 10 requests per windowMs
+  windowMs: 1 * 60 * 1000, // 1 minutes
+  max: 60, // limit each IP to 60 requests per windowMs
   message:
-    'Too many requests sent from this IP, please try again after 15 minutes',
+    'Too many requests sent from this IP, please try again after 1 minute',
 });
 
 autoReap.options.reapOnError = true; // continue reaping the file even if an error occurs

packages/app/src/server/routes/login.js

@@ -217,8 +217,7 @@ module.exports = function(crowi, app) {
       }
     }
     else {
-      return res.render('invited', {
-      });
+      return res.render('invited');
     }
   };
 

packages/app/test/integration/middlewares/login-required.test.js

@@ -18,60 +18,157 @@ describe('loginRequired', () => {
   });
 
   describe('not strict mode', () => {
-    // setup req/res/next
-    const req = {
-      originalUrl: 'original url 1',
-      session: {},
-    };
     const res = {
       redirect: jest.fn().mockReturnValue('redirect'),
+      sendStatus: jest.fn().mockReturnValue('sendStatus'),
     };
     const next = jest.fn().mockReturnValue('next');
 
-    test('pass guest user when aclService.isGuestAllowedToRead() returns true', () => {
-      // prepare spy for AclService.isGuestAllowedToRead
-      const isGuestAllowedToReadSpy = jest.spyOn(crowi.aclService, 'isGuestAllowedToRead')
-        .mockImplementation(() => true);
-
-      const result = loginRequired(req, res, next);
-
-      expect(isGuestAllowedToReadSpy).toHaveBeenCalledTimes(1);
-      expect(fallbackMock).not.toHaveBeenCalled();
-      expect(next).toHaveBeenCalled();
-      expect(res.redirect).not.toHaveBeenCalled();
-      expect(result).toBe('next');
-    });
-
-    test('redirect to \'/login\' when aclService.isGuestAllowedToRead() returns false', () => {
-      // prepare spy for AclService.isGuestAllowedToRead
-      const isGuestAllowedToReadSpy = jest.spyOn(crowi.aclService, 'isGuestAllowedToRead')
-        .mockImplementation(() => false);
+    describe('and when aclService.isGuestAllowedToRead() returns false', () => {
+      let req;
+
+      let isGuestAllowedToReadSpy;
+
+      beforeEach(async() => {
+        // setup req
+        req = {
+          originalUrl: 'original url 1',
+          session: {},
+        };
+        // reset session object
+        req.session = {};
+        // prepare spy for AclService.isGuestAllowedToRead
+        isGuestAllowedToReadSpy = jest.spyOn(crowi.aclService, 'isGuestAllowedToRead')
+          .mockImplementation(() => false);
+      });
+
+      /* eslint-disable indent */
+      test.each`
+        userStatus  | expectedPath
+        ${1}        | ${'/login/error/registered'}
+        ${3}        | ${'/login/error/suspended'}
+        ${5}        | ${'/login/invited'}
+      `('redirect to \'$expectedPath\' when user.status is \'$userStatus\'', ({ userStatus, expectedPath }) => {
+
+        req.user = {
+          _id: 'user id',
+          status: userStatus,
+        };
+
+        const result = loginRequired(req, res, next);
+
+        expect(isGuestAllowedToReadSpy).not.toHaveBeenCalled();
+        expect(next).not.toHaveBeenCalled();
+        expect(fallbackMock).not.toHaveBeenCalled();
+        expect(res.sendStatus).not.toHaveBeenCalled();
+        expect(res.redirect).toHaveBeenCalledTimes(1);
+        expect(res.redirect).toHaveBeenCalledWith(expectedPath);
+        expect(result).toBe('redirect');
+        expect(req.session.redirectTo).toBe(undefined);
+      });
+      /* eslint-disable indent */
+
+      test('redirect to \'/login\' when the user does not loggedin', () => {
+        req.baseUrl = '/path/that/requires/loggedin';
+
+        const result = loginRequired(req, res, next);
+
+        expect(isGuestAllowedToReadSpy).toHaveBeenCalled();
+        expect(next).not.toHaveBeenCalled();
+        expect(fallbackMock).not.toHaveBeenCalled();
+        expect(res.sendStatus).not.toHaveBeenCalled();
+        expect(res.redirect).toHaveBeenCalledTimes(1);
+        expect(res.redirect).toHaveBeenCalledWith('/login');
+        expect(result).toBe('redirect');
+        expect(req.session.redirectTo).toBe('original url 1');
+      });
+
+      test('pass anyone into sharedPage', () => {
+
+        req.isSharedPage = true;
+
+        const result = loginRequired(req, res, next);
+
+        expect(isGuestAllowedToReadSpy).toHaveBeenCalled();
+        expect(fallbackMock).not.toHaveBeenCalled();
+        expect(res.sendStatus).not.toHaveBeenCalled();
+        expect(next).toHaveBeenCalled();
+        expect(res.redirect).not.toHaveBeenCalled();
+        expect(result).toBe('next');
+      });
 
-      const result = loginRequired(req, res, next);
-
-      expect(isGuestAllowedToReadSpy).toHaveBeenCalled();
-      expect(fallbackMock).not.toHaveBeenCalled();
-      expect(next).not.toHaveBeenCalled();
-      expect(res.redirect).toHaveBeenCalledTimes(1);
-      expect(res.redirect).toHaveBeenCalledWith('/login');
-      expect(result).toBe('redirect');
     });
 
-    test('pass anyone into sharedPage when aclService.isGuestAllowedToRead() returns false', () => {
+    describe('and when aclService.isGuestAllowedToRead() returns true', () => {
+      let req;
+
+      let isGuestAllowedToReadSpy;
+
+      beforeEach(async() => {
+        // setup req
+        req = {
+          originalUrl: 'original url 1',
+          session: {},
+        };
+        // reset session object
+        req.session = {};
+        // prepare spy for AclService.isGuestAllowedToRead
+        isGuestAllowedToReadSpy = jest.spyOn(crowi.aclService, 'isGuestAllowedToRead')
+          .mockImplementation(() => true);
+      });
+
+      /* eslint-disable indent */
+      test.each`
+        userStatus  | expectedPath
+        ${1}        | ${'/login/error/registered'}
+        ${3}        | ${'/login/error/suspended'}
+        ${5}        | ${'/login/invited'}
+      `('redirect to \'$expectedPath\' when user.status is \'$userStatus\'', ({ userStatus, expectedPath }) => {
+
+        req.user = {
+          _id: 'user id',
+          status: userStatus,
+        };
+
+        const result = loginRequired(req, res, next);
+
+        expect(isGuestAllowedToReadSpy).not.toHaveBeenCalled();
+        expect(next).not.toHaveBeenCalled();
+        expect(fallbackMock).not.toHaveBeenCalled();
+        expect(res.sendStatus).not.toHaveBeenCalled();
+        expect(res.redirect).toHaveBeenCalledTimes(1);
+        expect(res.redirect).toHaveBeenCalledWith(expectedPath);
+        expect(result).toBe('redirect');
+        expect(req.session.redirectTo).toBe(undefined);
+      });
+      /* eslint-disable indent */
+
+      test('pass guest user', () => {
+
+        const result = loginRequired(req, res, next);
+
+        expect(isGuestAllowedToReadSpy).toHaveBeenCalledTimes(1);
+        expect(fallbackMock).not.toHaveBeenCalled();
+        expect(res.sendStatus).not.toHaveBeenCalled();
+        expect(next).toHaveBeenCalled();
+        expect(res.redirect).not.toHaveBeenCalled();
+        expect(result).toBe('next');
+      });
+
+      test('pass anyone into sharedPage', () => {
+
+        req.isSharedPage = true;
+
+        const result = loginRequired(req, res, next);
+
+        expect(isGuestAllowedToReadSpy).toHaveBeenCalled();
+        expect(fallbackMock).not.toHaveBeenCalled();
+        expect(res.sendStatus).not.toHaveBeenCalled();
+        expect(next).toHaveBeenCalled();
+        expect(res.redirect).not.toHaveBeenCalled();
+        expect(result).toBe('next');
+      });
 
-      req.isSharedPage = true;
-
-      // prepare spy for AclService.isGuestAllowedToRead
-      const isGuestAllowedToReadSpy = jest.spyOn(crowi.aclService, 'isGuestAllowedToRead')
-        .mockImplementation(() => false);
-
-      const result = loginRequired(req, res, next);
-
-      expect(isGuestAllowedToReadSpy).toHaveBeenCalled();
-      expect(fallbackMock).not.toHaveBeenCalled();
-      expect(next).toHaveBeenCalled();
-      expect(res.redirect).not.toHaveBeenCalled();
-      expect(result).toBe('next');
     });
 
   });