Merge pull request #1672 from weseek/imprv/guest-mode

Imprv/guest mode

resource/locales/en-US/translation.json

@@ -46,6 +46,7 @@
   "Timeline View": "Timeline",
   "History": "History",
   "Presentation Mode": "Presentation",
+  "Not available for guest": "Not available for guest",
   "username": "Username",
   "Created": "Created",
   "Last updated": "Updated",

resource/locales/ja/translation.json

@@ -46,6 +46,7 @@
   "Timeline View": "タイムライン表示",
   "History": "更新履歴",
   "Presentation Mode": "プレゼンテーション",
+  "Not available for guest": "ゲストユーザーは利用できません",
   "username": "ユーザー名",
   "Created": "作成日",
   "Last updated": "最終更新",

src/client/js/legacy/crowi.js

@@ -596,6 +596,11 @@ $(() => {
 window.addEventListener('load', (e) => {
   const { appContainer } = window;
 
+  // do nothing if user is guest
+  if (appContainer.currentUser == null) {
+    return;
+  }
+
   // hash on page
   if (window.location.hash) {
     if ((window.location.hash === '#edit' || window.location.hash === '#edit-form') && $('.tab-pane#edit').length > 0) {
@@ -619,7 +624,9 @@ window.addEventListener('load', (e) => {
       $('a[data-toggle="tab"][href="#revision-history"]').tab('show');
     }
   }
+});
 
+window.addEventListener('load', (e) => {
   const crowi = window.crowi;
   if (crowi && crowi.users && crowi.users.length !== 0) {
     const totalUsers = crowi.users.length;

src/client/styles/scss/style-app.scss

@@ -48,7 +48,7 @@
 /*
  * for Guest User Mode
  */
-.dropdown-disabled {
+.dropdown-toggle.dropdown-toggle-disabled {
   cursor: not-allowed;
 }
 

src/server/crowi/express-init.js

@@ -19,6 +19,8 @@ module.exports = function(crowi, app) {
   const i18nSprintf = require('i18next-sprintf-postprocessor');
   const i18nMiddleware = require('i18next-express-middleware');
 
+  const safeRedirect = require('../middleware/safe-redirect')();
+
   const avoidSessionRoutes = require('../routes/avoid-session-routes');
   const i18nUserSettingDetector = require('../util/i18nUserSettingDetector');
 
@@ -113,6 +115,8 @@ module.exports = function(crowi, app) {
 
   app.use(flash());
 
+  app.use(safeRedirect);
+
   const middlewares = require('../util/middlewares')(crowi, app);
 
   app.use(middlewares.swigFilters(swig));

src/server/middleware/login-required.js

@@ -42,7 +42,7 @@ module.exports = (crowi, isGuestAllowed = false) => {
       return res.sendStatus(403);
     }
 
-    req.session.jumpTo = req.originalUrl;
+    req.session.redirectTo = req.originalUrl;
     return res.redirect('/login');
   };
 

src/server/middleware/safe-redirect.js

@@ -0,0 +1,65 @@
+/**
+ * Redirect with prevention from Open Redirect
+ *
+ * Usage: app.use(require('middleware/safe-redirect')(['example.com', 'some.example.com:8080']))
+ */
+
+const loggerFactory = require('@alias/logger');
+
+const logger = loggerFactory('growi:middleware:safe-redirect');
+
+/**
+ * Check whether the redirect url host is in specified whitelist
+ * @param {Array<string>} whitelistOfHosts
+ * @param {string} redirectToFqdn
+ */
+function isInWhitelist(whitelistOfHosts, redirectToFqdn) {
+  if (whitelistOfHosts == null || whitelistOfHosts.length === 0) {
+    return false;
+  }
+
+  const redirectUrl = new URL(redirectToFqdn);
+  return whitelistOfHosts.includes(redirectUrl.hostname) || whitelistOfHosts.includes(redirectUrl.host);
+}
+
+
+module.exports = (whitelistOfHosts) => {
+
+  return function(req, res, next) {
+
+    // extend res object
+    res.safeRedirect = function(redirectTo) {
+      if (redirectTo == null) {
+        return res.redirect('/');
+      }
+
+      try {
+        // check inner redirect
+        const redirectUrl = new URL(redirectTo, `${req.protocol}://${req.get('host')}`);
+        if (redirectUrl.hostname === req.hostname) {
+          logger.debug(`Requested redirect URL (${redirectTo}) is local.`);
+          return res.redirect(redirectUrl.href);
+        }
+        logger.debug(`Requested redirect URL (${redirectTo}) is NOT local.`);
+
+        // check whitelisted redirect
+        const isWhitelisted = isInWhitelist(whitelistOfHosts, redirectTo);
+        if (isWhitelisted) {
+          logger.debug(`Requested redirect URL (${redirectTo}) is in whitelist.`, `whitelist=${whitelistOfHosts}`);
+          return res.redirect(redirectTo);
+        }
+        logger.debug(`Requested redirect URL (${redirectTo}) is NOT in whitelist.`, `whitelist=${whitelistOfHosts}`);
+      }
+      catch (err) {
+        logger.warn(`Requested redirect URL (${redirectTo}) is invalid.`, err);
+      }
+
+      logger.warn(`Requested redirect URL (${redirectTo}) is UNSAFE, redirecting to root page.`);
+      return res.redirect('/');
+    };
+
+    next();
+
+  };
+
+};

src/server/routes/index.js

@@ -47,14 +47,14 @@ module.exports = function(crowi, app) {
   }
 
   app.get('/login/error/:reason'     , login.error);
-  app.get('/login'                   , middlewares.applicationInstalled    , login.login);
+  app.get('/login'                   , middlewares.applicationInstalled     , login.preLogin, login.login);
   app.get('/login/invited'           , login.invited);
   app.post('/login/activateInvited'  , form.invited                         , csrf, login.invited);
   app.post('/login'                  , form.login                           , csrf, loginPassport.loginWithLocal, loginPassport.loginWithLdap, loginPassport.loginFailure);
   app.post('/_api/login/testLdap'    , loginRequiredStrictly , form.login , loginPassport.testLdapCredentials);
 
   app.post('/register'               , form.register                        , csrf, login.register);
-  app.get('/register'                , middlewares.applicationInstalled    , login.register);
+  app.get('/register'                , middlewares.applicationInstalled     , login.preLogin, login.register);
   app.get('/logout'                  , logout.logout);
 
   app.get('/admin'                          , loginRequiredStrictly , adminRequired , admin.index);

src/server/routes/login-passport.js

@@ -4,7 +4,6 @@ module.exports = function(crowi, app) {
   const debug = require('debug')('growi:routes:login-passport');
   const logger = require('@alias/logger')('growi:routes:login-passport');
   const passport = require('passport');
-  const { URL } = require('url');
   const ExternalAccount = crowi.model('ExternalAccount');
   const passportService = crowi.passportService;
 
@@ -22,25 +21,10 @@ module.exports = function(crowi, app) {
       }
     });
 
-    const jumpTo = req.session.jumpTo;
-    if (jumpTo) {
-      req.session.jumpTo = null;
-
-      // prevention from open redirect
-      try {
-        const redirectUrl = new URL(jumpTo, `${req.protocol}://${req.get('host')}`);
-        if (redirectUrl.hostname === req.hostname) {
-          return res.redirect(redirectUrl);
-        }
-        logger.warn('Requested redirect URL is invalid, redirect to root page');
-      }
-      catch (err) {
-        logger.warn('Requested redirect URL is invalid, redirect to root page', err);
-        return res.redirect('/');
-      }
-    }
-
-    return res.redirect('/');
+    const { redirectTo } = req.session;
+    // remove session.redirectTo
+    delete req.session.redirectTo;
+    return res.safeRedirect(redirectTo);
   };
 
   /**

src/server/routes/login.js

@@ -30,30 +30,10 @@ module.exports = function(crowi, app) {
       return res.redirect('/me/password');
     }
 
-    const jumpTo = req.session.jumpTo;
-    if (jumpTo) {
-      req.session.jumpTo = null;
-
-      // prevention from open redirect
-      try {
-        const redirectUrl = new URL(jumpTo, `${req.protocol}://${req.get('host')}`);
-        if (redirectUrl.hostname === req.hostname) {
-          return res.redirect(redirectUrl);
-        }
-        logger.warn('Requested redirect URL is invalid, redirect to root page');
-      }
-      catch (err) {
-        logger.warn('Requested redirect URL is invalid, redirect to root page', err);
-        return res.redirect('/');
-      }
-    }
-
-    return res.redirect('/');
-  };
-
-  const loginFailure = function(req, res) {
-    req.flash('warningMessage', 'Sign in failure.');
-    return res.redirect('/login');
+    const { redirectTo } = req.session;
+    // remove session.redirectTo
+    delete req.session.redirectTo;
+    return res.safeRedirect(redirectTo);
   };
 
   actions.error = function(req, res) {
@@ -74,30 +54,29 @@ module.exports = function(crowi, app) {
     });
   };
 
-  actions.login = function(req, res) {
-    const loginForm = req.body.loginForm;
+  actions.preLogin = function(req, res, next) {
+    // user has already logged in
+    if (req.user != null) {
+      const { redirectTo } = req.session;
+      // remove session.redirectTo
+      delete req.session.redirectTo;
+      return res.safeRedirect(redirectTo);
+    }
 
-    if (req.method == 'POST' && req.form.isValid) {
-      const username = loginForm.username;
-      const password = loginForm.password;
-
-      // find user
-      User.findUserByUsernameOrEmail(username, password, (err, user) => {
-        if (err) { return loginFailure(req, res) }
-        // check existence and password
-        if (!user || !user.isPasswordValid(password)) {
-          return loginFailure(req, res);
-        }
-        return loginSuccess(req, res, user);
-      });
+    // set referer to 'redirectTo'
+    if (req.session.redirectTo == null && req.headers.referer != null) {
+      req.session.redirectTo = req.headers.referer;
     }
-    else { // method GET
-      if (req.form) {
-        debug(req.form.errors);
-      }
-      return res.render('login', {
-      });
+
+    next();
+  }
+
+  actions.login = function(req, res) {
+    if (req.form) {
+      debug(req.form.errors);
     }
+
+    return res.render('login', {});
   };
 
   actions.register = function(req, res) {

src/server/routes/logout.js

@@ -2,7 +2,10 @@ module.exports = function(crowi, app) {
   return {
     logout(req, res) {
       req.session.destroy();
-      return res.redirect('/');
+
+      // redirect
+      const redirectTo = req.headers.referer || '/';
+      return res.safeRedirect(redirectTo);
     },
   };
 };

src/server/views/widget/not_found_tabs.html

@@ -7,7 +7,10 @@
 
   {% if !isTrashPage() and !page.isDeleted() %}
   <li class="nav-main-left-tab">
-    <a {% if user %}href="#edit" data-toggle="tab"{% endif %} class="edit-button {% if not user %}edit-button-disabled{% endif %}">
+    <a
+      {% if user %} href="#edit" data-toggle="tab" class="edit-button" {% endif %}
+      {% if not user %} class="edit-button edit-button-disabled" data-toggle="tooltip" data-placement="top" data-container="body" title="{{ t('Not available for guest') }}" {% endif %}
+    >
       <i class="icon-note"></i> {{ t('Create') }}
     </a>
   </li>

src/server/views/widget/page_tabs.html

@@ -12,13 +12,25 @@
 
   {% if !isTrashPage() %}
   <li class="nav-main-left-tab nav-tab-edit">
-    <a {% if user %}href="#edit" data-toggle="tab"{% endif %} class="edit-button {% if not user %}edit-button-disabled{% endif %}">
+    <a
+      {% if user %} href="#edit" data-toggle="tab" class="edit-button" {% endif %}
+      {% if not user %}
+        class="edit-button edit-button-disabled"
+        data-toggle="tooltip" data-placement="top" data-container="body" title="{{ t('Not available for guest') }}"
+      {% endif %}
+    >
       <i class="icon-note"></i> {{ t('Edit') }}
     </a>
   </li>
   {% if isHackmdSetup() %}
   <li class="nav-main-left-tab nav-tab-hackmd">
-    <a {% if user %}href="#hackmd" data-toggle="tab"{% endif %} class="{% if not user %}edit-button-disabled{% endif %}">
+    <a
+      {% if user %} href="#hackmd" data-toggle="tab" class="edit-button" {% endif %}
+      {% if not user %}
+        class="edit-button edit-button-disabled"
+        data-toggle="tooltip" data-placement="top" data-container="body" title="{{ t('Not available for guest') }}"
+      {% endif %}
+    >
       <i class="fa fa-file-text-o"></i> {{ t('HackMD') }}
     </a>
   </li>
@@ -31,7 +43,13 @@
   {% if !isTrashPage() %}
     {% if page.isPortal() %}
     <li class="nav-main-right-tab dropdown pull-right">
-      <a class="dropdown-toggle {% if not user %}dropdown-disabled{% endif %}" {% if user %}data-toggle="dropdown" href="#"{% endif %}>
+      <a
+        {% if user %} role="button" class="dropdown-toggle" data-toggle="dropdown" {% endif %}
+        {% if not user %}
+          class="dropdown-toggle dropdown-toggle-disabled"
+          data-toggle="tooltip" data-placement="top" data-container="body" title="{{ t('Not available for guest') }}"
+        {% endif %}
+      >
         <i class="icon-options-vertical"></i>
       </a>
       <ul class="dropdown-menu">
@@ -47,7 +65,13 @@
     </li>
     {% else %}
     <li class="nav-main-right-tab dropdown pull-right">
-      <a class="dropdown-toggle {% if not user %}dropdown-disabled{% endif %}" {% if user %}data-toggle="dropdown" href="#"{% endif %}>
+      <a
+        {% if user %} role="button" class="dropdown-toggle" data-toggle="dropdown" {% endif %}
+        {% if not user %}
+          class="dropdown-toggle dropdown-toggle-disabled"
+          data-toggle="tooltip" data-placement="top" data-container="body" title="{{ t('Not available for guest') }}"
+        {% endif %}
+      >
         <i class="icon-options-vertical"></i>
       </a>
       <ul class="dropdown-menu">

src/server/views/widget/page_tabs_kibela.html

@@ -12,13 +12,25 @@
 
   {% if !isTrashPage() %}
   <li class="nav-main-left-tab nav-tab-edit">
-    <a {% if user %}href="#edit" data-toggle="tab"{% endif %} class="edit-button {% if not user %}edit-button-disabled{% endif %}">
+    <a
+      {% if user %} href="#edit" data-toggle="tab" class="edit-button" {% endif %}
+      {% if not user %}
+        class="edit-button edit-button-disabled"
+        data-toggle="tooltip" data-placement="top" data-container="body" title="{{ t('Not available for guest') }}"
+      {% endif %}
+    >
       <i class="icon-note"></i> {{ t('Edit') }}
     </a>
   </li>
   {% if isHackmdSetup() %}
   <li class="nav-main-left-tab nav-tab-hackmd">
-    <a {% if user %}href="#hackmd" data-toggle="tab"{% endif %} class="{% if not user %}edit-button-disabled{% endif %}">
+    <a
+      {% if user %} href="#hackmd" data-toggle="tab" class="edit-button" {% endif %}
+      {% if not user %}
+        class="edit-button edit-button-disabled"
+        data-toggle="tooltip" data-placement="top" data-container="body" title="{{ t('Not available for guest') }}"
+      {% endif %}
+    >
       <i class="fa fa-file-text-o"></i> {{ t('HackMD') }}
     </a>
   </li>
@@ -31,7 +43,13 @@
   {% if !isTrashPage() %}
     {% if page.isPortal() %}
     <li class="nav-main-right-tab dropdown pull-right">
-      <a class="dropdown-toggle {% if not user %}dropdown-disabled{% endif %}" {% if user %}data-toggle="dropdown" href="#"{% endif %}>
+      <a
+        {% if user %} role="button" class="dropdown-toggle" data-toggle="dropdown" {% endif %}
+        {% if not user %}
+          class="dropdown-toggle dropdown-toggle-disabled"
+          data-toggle="tooltip" data-placement="top" data-container="body" title="{{ t('Not available for guest') }}"
+        {% endif %}
+      >
         <i class="icon-options-vertical"></i>
       </a>
       <ul class="dropdown-menu">
@@ -44,7 +62,13 @@
     </li>
     {% else %}
     <li class="nav-main-right-tab dropdown pull-right">
-      <a class="dropdown-toggle {% if not user %}dropdown-disabled{% endif %}" {% if user %}data-toggle="dropdown" href="#"{% endif %}>
+      <a
+        {% if user %} role="button" class="dropdown-toggle" data-toggle="dropdown" {% endif %}
+        {% if not user %}
+          class="dropdown-toggle dropdown-toggle-disabled"
+          data-toggle="tooltip" data-placement="top" data-container="body" title="{{ t('Not available for guest') }}"
+        {% endif %}
+      >
         <i class="icon-options-vertical"></i>
       </a>
       <ul class="dropdown-menu">

src/test/middleware/login-required.test.js

@@ -101,7 +101,7 @@ describe('loginRequired', () => {
       expect(res.redirect).toHaveBeenCalledTimes(1);
       expect(res.redirect).toHaveBeenCalledWith('/login');
       expect(result).toBe('redirect');
-      expect(req.session.jumpTo).toBe('original url 1');
+      expect(req.session.redirectTo).toBe('original url 1');
     });
 
     test('pass user who logged in', () => {
@@ -119,7 +119,7 @@ describe('loginRequired', () => {
       expect(res.redirect).not.toHaveBeenCalled();
       expect(next).toHaveBeenCalledTimes(1);
       expect(result).toBe('next');
-      expect(req.session.jumpTo).toBe(undefined);
+      expect(req.session.redirectTo).toBe(undefined);
     });
 
     /* eslint-disable indent */
@@ -142,7 +142,7 @@ describe('loginRequired', () => {
       expect(res.redirect).toHaveBeenCalledTimes(1);
       expect(res.redirect).toHaveBeenCalledWith(expectedPath);
       expect(result).toBe('redirect');
-      expect(req.session.jumpTo).toBe(undefined);
+      expect(req.session.redirectTo).toBe(undefined);
     });
     /* eslint-disable indent */
 
@@ -163,7 +163,7 @@ describe('loginRequired', () => {
       expect(res.redirect).toHaveBeenCalledTimes(1);
       expect(res.redirect).toHaveBeenCalledWith('/login');
       expect(result).toBe('redirect');
-      expect(req.session.jumpTo).toBe('original url 1');
+      expect(req.session.redirectTo).toBe('original url 1');
     });
 
   });

src/test/middleware/safe-redirect.test.js

@@ -0,0 +1,108 @@
+/* eslint-disable arrow-body-style */
+
+describe('safeRedirect', () => {
+  let safeRedirect;
+
+  const whitelistOfHosts = [
+    'white1.example.com:8080',
+    'white2.example.com',
+  ];
+
+  beforeEach(async(done) => {
+    safeRedirect = require('@server/middleware/safe-redirect')(whitelistOfHosts);
+    done();
+  });
+
+  describe('res.safeRedirect', () => {
+    // setup req/res/next
+    const req = {
+      protocol: 'http',
+      hostname: 'example.com',
+      get: jest.fn().mockReturnValue('example.com'),
+    };
+    const res = {
+      redirect: jest.fn().mockReturnValue('redirect'),
+    };
+    const next = jest.fn();
+
+    test('redirects to \'/\' because specified url causes open redirect vulnerability', () => {
+      safeRedirect(req, res, next);
+
+      const result = 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', () => {
+      safeRedirect(req, res, next);
+
+      const result = 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', () => {
+      safeRedirect(req, res, next);
+
+      const result = 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)', () => {
+      safeRedirect(req, res, next);
+
+      const result = 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)', () => {
+      safeRedirect(req, res, next);
+
+      const result = 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)', () => {
+      safeRedirect(req, res, next);
+
+      const result = 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');
+    });
+
+  });
+
+});