Merge branch 'master' into imprv/gw-4349-me-setting-layout

CHANGES.md

@@ -15,6 +15,11 @@
     * migrate-mongo
     * mongoose
 
+## v4.1.10
+
+* Fix: Make listing users API secure
+* Fix: Error message when the server denies guest user connecting with socket.io
+
 ## v4.1.9
 
 * Feature: Environment variables to set max connection size to deliver push messages to all clients

src/client/js/components/Navbar/GrowiNavbar.jsx

@@ -76,10 +76,9 @@ class GrowiNavbar extends React.Component {
         {/* Navbar Right  */}
         <ul className="navbar-nav ml-auto">
           {this.renderNavbarRight()}
+          {crowi.confidential != null && this.renderConfidential()}
         </ul>
 
-        {crowi.confidential != null && this.renderConfidential()}
-
         { isSearchServiceConfigured && !isDeviceSmallerThanMd && (
           <div className="grw-global-search grw-global-search-top position-absolute">
             <GlobalSearch />

src/client/js/components/Navbar/GrowiSubNavigation.jsx

@@ -1,4 +1,4 @@
-import React from 'react';
+import React, { useMemo } from 'react';
 import PropTypes from 'prop-types';
 
 import { withTranslation } from 'react-i18next';
@@ -71,12 +71,16 @@ const PagePathNav = ({ pageId, pagePath, isPageForbidden }) => {
 const PageReactionButtons = ({ appContainer, pageContainer }) => {
 
   const {
-    pageId, isLiked, pageUser,
+    pageId, isLiked, pageUser, shareLinkId,
   } = pageContainer.state;
 
+  const isSharedPage = useMemo(() => {
+    return shareLinkId != null;
+  }, [shareLinkId]);
+
   return (
     <>
-      {pageUser == null && (
+      {pageUser == null && !isSharedPage && (
       <span className="mr-2">
         <LikeButton pageId={pageId} isLiked={isLiked} />
       </span>

src/client/js/components/Page/ShareLinkAlert.jsx

@@ -41,7 +41,7 @@ const ShareLinkAlert = (props) => {
   }
 
   return (
-    <p className={`alert alert-${specifyColor()} py-3 px-4`}>
+    <p className={`alert alert-${specifyColor()} py-3 px-4 d-edit-none`}>
       <i className="icon-fw icon-link"></i>
       {(expiredAt === '' ? <span>{t('page_page.notice.no_deadline')}</span>
       // eslint-disable-next-line react/no-danger

src/client/js/components/PageContentFooter.jsx

@@ -16,10 +16,10 @@ const PageContentFooter = (props) => {
   return (
     <div className="page-content-footer mt-5 py-4 d-edit-none d-print-none">
       <div className="container-lg">
-        <p className="page-meta">
+        <div className="page-meta">
           <AuthorInfo user={creator} date={createdAt} mode="create" locate="footer" />
           <AuthorInfo user={revisionAuthor} date={updatedAt} mode="update" locate="footer" />
-        </p>
+        </div>
       </div>
     </div>
   );

src/client/js/legacy/crowi.js

@@ -156,36 +156,12 @@ Crowi.highlightSelectedSection = function(hash) {
 
 $(() => {
   const pageId = $('#content-main').data('page-id');
-  // const revisionId = $('#content-main').data('page-revision-id');
-  // const revisionCreatedAt = $('#content-main').data('page-revision-created');
-  // const currentUser = $('#content-main').data('current-user');
   const isSeen = $('#content-main').data('page-is-seen');
 
   $('[data-toggle="popover"]').popover();
   $('[data-toggle="tooltip"]').tooltip();
   $('[data-tooltip-stay]').tooltip('show');
 
-  $('#toggle-crowi-sidebar').click((e) => {
-    const $body = $('body');
-    if ($body.hasClass('aside-hidden')) {
-      $body.removeClass('aside-hidden');
-      $.cookie('aside-hidden', 0, { expires: 30, path: '/' });
-    }
-    else {
-      $body.addClass('aside-hidden');
-      $.cookie('aside-hidden', 1, { expires: 30, path: '/' });
-    }
-    return false;
-  });
-
-  if ($.cookie('aside-hidden') === 1) {
-    $('body').addClass('aside-hidden');
-  }
-
-  $('.copy-link').on('click', function() {
-    $(this).select();
-  });
-
   if (pageId) {
 
     if (!isSeen) {

src/client/js/services/PageContainer.js

@@ -101,9 +101,15 @@ export default class PageContainer extends Container {
     interceptorManager.addInterceptor(new DrawioInterceptor(appContainer), 20);
     interceptorManager.addInterceptor(new RestoreCodeBlockInterceptor(appContainer), 900); // process as late as possible
 
-    this.retrieveSeenUsers();
     this.initStateMarkdown();
-    this.initStateOthers();
+    this.checkAndUpdateImageUrlCached(this.state.likerUsers);
+
+    // skip if shared page
+    if (this.state.shareLinkId == null) {
+      this.retrieveSeenUsers();
+      this.retrieveLikeInfo();
+      this.retrieveBookmarkInfo();
+    }
 
     this.setTocHtml = this.setTocHtml.bind(this);
     this.save = this.save.bind(this);
@@ -155,13 +161,6 @@ export default class PageContainer extends Container {
     this.checkAndUpdateImageUrlCached(users);
   }
 
-  async initStateOthers() {
-
-    this.retrieveLikeInfo();
-    this.retrieveBookmarkInfo();
-    this.checkAndUpdateImageUrlCached(this.state.likerUsers);
-  }
-
   async retrieveLikeInfo() {
     const like = await this.appContainer.apiv3Get('/page/like-info', { _id: this.state.pageId });
     this.setState({
@@ -180,14 +179,11 @@ export default class PageContainer extends Container {
   }
 
   async retrieveBookmarkInfo() {
-    const response = await this.appContainer.apiv3Get('/bookmarks', { pageId: this.state.pageId });
-    if (response.data.bookmarks != null) {
-      this.setState({ isBookmarked: true });
-    }
-    else {
-      this.setState({ isBookmarked: false });
-    }
-    this.setState({ sumOfBookmarks: response.data.sumOfBookmarks });
+    const response = await this.appContainer.apiv3Get('/bookmarks/info', { pageId: this.state.pageId });
+    this.setState({
+      sumOfBookmarks: response.data.sumOfBookmarks,
+      isBookmarked: response.data.isBookmarked,
+    });
   }
 
   async toggleBookmark() {

src/client/js/services/TagContainer.js

@@ -39,11 +39,15 @@ export default class TagContainer extends Container {
       return;
     }
 
-    const { pageId, templateTagData } = pageContainer.state;
+    const { pageId, templateTagData, shareLinkId } = pageContainer.state;
+
+    if (shareLinkId != null) {
+      return;
+    }
 
     let tags = [];
-    // when the page exists
-    if (pageId != null) {
+    // when the page exists or shared page
+    if (pageId != null && shareLinkId == null) {
       const res = await this.appContainer.apiGet('/pages.getPageTag', { pageId });
       tags = res.tags;
     }

src/client/styles/scss/_layout.scss

@@ -36,6 +36,10 @@ body {
   }
 }
 
+.grw-side-contents-container {
+  margin-left: 30px;
+}
+
 .grw-side-contents-sticky-container {
   position: sticky;
   // growisubnavigation + grw-navbar-boder

src/client/styles/scss/_subnav.scss

@@ -106,7 +106,7 @@ $easeInOutCubic: cubic-bezier(0.65, 0, 0.35, 1);
   z-index: $zindex-sticky - 5;
 
   .grw-subnav {
-    box-shadow: 0px 6px 6px 3px rgba(black, 0.15);
+    box-shadow: 0px 0px 6px 3px rgba(black, 0.15);
   }
 }
 

src/server/middlewares/login-required.js

@@ -47,13 +47,13 @@ module.exports = (crowi, isGuestAllowed = false, fallback = null) => {
     const path = req.path || '';
     if (path.match(/^\/_api\/.+$/)) {
       if (fallback != null) {
-        return fallback(req, res);
+        return fallback(req, res, next);
       }
       return res.sendStatus(403);
     }
 
     if (fallback != null) {
-      return fallback(req, res);
+      return fallback(req, res, next);
     }
     req.session.redirectTo = req.originalUrl;
     return res.redirect('/login');

src/server/routes/apiv3/bookmarks.js

@@ -50,11 +50,23 @@ const router = express.Router();
  *          bool:
  *            type: boolean
  *            description: boolean for bookmark status
+ *
+ *      BookmarkInfo:
+ *        description: BookmarkInfo
+ *        type: object
+ *        properties:
+ *          sumOfBookmarks:
+ *            type: number
+ *            description: how many people bookmarked the page
+ *          isBookmarked:
+ *            type: boolean
+ *            description: Whether the request user bookmarked (will be returned if the user is included in the request)
  */
 
 module.exports = (crowi) => {
   const accessTokenParser = require('../../middlewares/access-token-parser')(crowi);
-  const loginRequired = require('../../middlewares/login-required')(crowi);
+  const loginRequiredStrictly = require('@server/middlewares/login-required')(crowi);
+  const loginRequired = require('../../middlewares/login-required')(crowi, true);
   const csrf = require('../../middlewares/csrf')(crowi);
   const apiV3FormValidator = require('../../middlewares/apiv3-form-validator')(crowi);
 
@@ -73,12 +85,12 @@ module.exports = (crowi) => {
   /**
    * @swagger
    *
-   *    /bookmarks:
+   *    /bookmarks/info:
    *      get:
    *        tags: [Bookmarks]
-   *        summary: /bookmarks
-   *        description: Get bookmarked status
-   *        operationId: getBookmarkedStatus
+   *        summary: /bookmarks/info
+   *        description: Get bookmarked info
+   *        operationId: getBookmarkedInfo
    *        parameters:
    *          - name: pageId
    *            in: query
@@ -87,24 +99,41 @@ module.exports = (crowi) => {
    *              type: string
    *        responses:
    *          200:
-   *            description: Succeeded to get bookmarked status.
+   *            description: Succeeded to get bookmark info.
    *            content:
    *              application/json:
    *                schema:
-   *                  $ref: '#/components/schemas/Bookmark'
+   *                  $ref: '#/components/schemas/BookmarkInfo'
    */
-  router.get('/', accessTokenParser, loginRequired, validator.bookmarkInfo, async(req, res) => {
+  router.get('/info', accessTokenParser, loginRequired, validator.bookmarkInfo, apiV3FormValidator, async(req, res) => {
+    const { user } = req;
     const { pageId } = req.query;
 
+    const responsesParams = {};
+
     try {
-      const bookmarks = await Bookmark.findByPageIdAndUserId(pageId, req.user);
-      const sumOfBookmarks = await Bookmark.countByPageId(pageId);
-      return res.apiv3({ bookmarks, sumOfBookmarks });
+      responsesParams.sumOfBookmarks = await Bookmark.countByPageId(pageId);
     }
     catch (err) {
-      logger.error('get-bookmark-failed', err);
+      logger.error('get-bookmark-count-failed', err);
       return res.apiv3Err(err, 500);
     }
+
+    // guest user only get bookmark count
+    if (user == null) {
+      return res.apiv3(responsesParams);
+    }
+
+    try {
+      const bookmark = await Bookmark.findByPageIdAndUserId(pageId, user._id);
+      responsesParams.isBookmarked = (bookmark != null);
+      return res.apiv3(responsesParams);
+    }
+    catch (err) {
+      logger.error('get-bookmark-state-failed', err);
+      return res.apiv3Err(err, 500);
+    }
+
   });
 
   // select page from bookmark where userid = userid
@@ -152,7 +181,7 @@ module.exports = (crowi) => {
     query('limit').if(value => value != null).isInt({ max: 300 }).withMessage('You should set less than 300 or not to set limit.'),
   ];
 
-  router.get('/:userId', accessTokenParser, loginRequired, validator.myBookmarkList, apiV3FormValidator, async(req, res) => {
+  router.get('/:userId', accessTokenParser, loginRequiredStrictly, validator.myBookmarkList, apiV3FormValidator, async(req, res) => {
     const { userId } = req.params;
     const page = req.query.page;
     const limit = parseInt(req.query.limit) || await crowi.configManager.getConfig('crowi', 'customize:showPageLimitationM') || 30;
@@ -213,7 +242,7 @@ module.exports = (crowi) => {
    *                schema:
    *                  $ref: '#/components/schemas/Bookmark'
    */
-  router.put('/', accessTokenParser, loginRequired, csrf, validator.bookmarks, apiV3FormValidator, async(req, res) => {
+  router.put('/', accessTokenParser, loginRequiredStrictly, csrf, validator.bookmarks, apiV3FormValidator, async(req, res) => {
     const { pageId, bool } = req.body;
 
     let bookmark;
@@ -240,27 +269,5 @@ module.exports = (crowi) => {
     return res.apiv3({ bookmark });
   });
 
-  /**
-   * @swagger
-   *
-   *    /count-bookmarks:
-   *      get:
-   *        tags: [Bookmarks]
-   *        summary: /bookmarks
-   *        description: Count bookmsrks
-   *        requestBody:
-   *          content:
-   *            application/json:
-   *              schema:
-   *                $ref: '#/components/schemas/BookmarkParams'
-   *        responses:
-   *          200:
-   *            description: Succeeded to count bookmarks.
-   *            content:
-   *              application/json:
-   *                schema:
-   *                  $ref: '#/components/schemas/Bookmark'
-   */
-
   return router;
 };

src/server/views/widget/page_content.html

@@ -54,7 +54,8 @@
   <div id="page-editor-navbar-bottom-container" class="d-none d-edit-block"></div>
 </div>
 
-<div class="d-none d-lg-block d-editor-none grw-side-contents-container ml-4">
+{% if revision %}
+<div class="d-none d-lg-block d-editor-none grw-side-contents-container">
   <div class="grw-side-contents-sticky-container">
     <div id="page-accessories" class="page-accessories"></div>
     <div id="revision-toc" class="revision-toc sps sps--abv" data-sps-offset="123">
@@ -65,6 +66,8 @@
     {% endif %}
   </div>
 </div>
+{% endif %}
+
 
 <div id="grw-page-status-alert-container"></div>
 

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

@@ -228,7 +228,7 @@ describe('loginRequired', () => {
       expect(res.redirect).not.toHaveBeenCalled();
       expect(res.sendStatus).not.toHaveBeenCalled();
       expect(fallbackMock).toHaveBeenCalledTimes(1);
-      expect(fallbackMock).toHaveBeenCalledWith(req, res);
+      expect(fallbackMock).toHaveBeenCalledWith(req, res, next);
       expect(result).toBe('fallback');
     });
 
@@ -242,7 +242,7 @@ describe('loginRequired', () => {
       expect(res.sendStatus).not.toHaveBeenCalled();
       expect(res.redirect).not.toHaveBeenCalled();
       expect(fallbackMock).toHaveBeenCalledTimes(1);
-      expect(fallbackMock).toHaveBeenCalledWith(req, res);
+      expect(fallbackMock).toHaveBeenCalledWith(req, res, next);
       expect(result).toBe('fallback');
     });