Merge pull request #3084 from weseek/imprv/omit-insecure-attributes

Imprv/omit insecure attributes

src/client/js/services/PageContainer.js

@@ -239,12 +239,18 @@ export default class PageContainer extends Container {
     return this.appContainer.getContainer('NavigationContainer');
   }
 
-  setLatestRemotePageData(page, user) {
-    this.setState({
-      remoteRevisionId: page.revision._id,
-      revisionIdHackmdSynced: page.revisionHackmdSynced,
-      lastUpdateUsername: user.name,
-    });
+  setLatestRemotePageData(s2cMessagePageUpdated) {
+    const newState = {
+      remoteRevisionId: s2cMessagePageUpdated.revisionId,
+      revisionIdHackmdSynced: s2cMessagePageUpdated.revisionIdHackmdSynced,
+      lastUpdateUsername: s2cMessagePageUpdated.lastUpdateUsername,
+    };
+
+    if (s2cMessagePageUpdated.hasDraftOnHackmd != null) {
+      newState.hasDraftOnHackmd = s2cMessagePageUpdated.hasDraftOnHackmd;
+    }
+
+    this.setState(newState);
   }
 
   setTocHtml(tocHtml) {
@@ -493,9 +499,10 @@ export default class PageContainer extends Container {
 
       logger.debug({ obj: data }, `websocket on 'page:create'`); // eslint-disable-line quotes
 
-      // update PageStatusAlert
-      if (data.page.path === pageContainer.state.path) {
-        this.setLatestRemotePageData(data.page, data.user);
+      // update remote page data
+      const { s2cMessagePageUpdated } = data;
+      if (s2cMessagePageUpdated.pageId === pageContainer.state.pageId) {
+        pageContainer.setLatestRemotePageData(s2cMessagePageUpdated);
       }
     });
 
@@ -507,16 +514,10 @@ export default class PageContainer extends Container {
 
       logger.debug({ obj: data }, `websocket on 'page:update'`); // eslint-disable-line quotes
 
-      if (data.page.path === pageContainer.state.path) {
-        // update PageStatusAlert
-        pageContainer.setLatestRemotePageData(data.page, data.user);
-        // update remote data
-        const page = data.page;
-        pageContainer.setState({
-          remoteRevisionId: page.revision._id,
-          revisionIdHackmdSynced: page.revisionHackmdSynced,
-          hasDraftOnHackmd: page.hasDraftOnHackmd,
-        });
+      // update remote page data
+      const { s2cMessagePageUpdated } = data;
+      if (s2cMessagePageUpdated.pageId === pageContainer.state.pageId) {
+        pageContainer.setLatestRemotePageData(s2cMessagePageUpdated);
       }
     });
 
@@ -528,9 +529,10 @@ export default class PageContainer extends Container {
 
       logger.debug({ obj: data }, `websocket on 'page:delete'`); // eslint-disable-line quotes
 
-      // update PageStatusAlert
-      if (data.page.path === pageContainer.state.path) {
-        pageContainer.setLatestRemotePageData(data.page, data.user);
+      // update remote page data
+      const { s2cMessagePageUpdated } = data;
+      if (s2cMessagePageUpdated.pageId === pageContainer.state.pageId) {
+        pageContainer.setLatestRemotePageData(s2cMessagePageUpdated);
       }
     });
 
@@ -542,7 +544,9 @@ export default class PageContainer extends Container {
 
       logger.debug({ obj: data }, `websocket on 'page:editingWithHackmd'`); // eslint-disable-line quotes
 
-      if (data.page.path === pageContainer.state.path) {
+      // update isHackmdDraftUpdatingInRealtime
+      const { s2cMessagePageUpdated } = data;
+      if (s2cMessagePageUpdated.pageId === pageContainer.state.pageId) {
         pageContainer.setState({ isHackmdDraftUpdatingInRealtime: true });
       }
     });

src/server/models/serializers/page-serializer.js

@@ -0,0 +1,48 @@
+const { serializeUserSecurely } = require('./user-serializer');
+
+function depopulate(page, attributeName) {
+  // revert the ObjectID
+  if (page[attributeName] != null && page[attributeName]._id != null) {
+    page[attributeName] = page[attributeName]._id;
+  }
+}
+
+function depopulateRevisions(page) {
+  depopulate(page, 'revision');
+  depopulate(page, 'revisionHackmdSynced');
+}
+
+function serializeInsecureUserAttributes(page) {
+  if (page.lastUpdateUser != null && page.lastUpdateUser._id != null) {
+    page.lastUpdateUser = serializeUserSecurely(page.lastUpdateUser);
+  }
+  if (page.creator != null && page.creator._id != null) {
+    page.creator = serializeUserSecurely(page.creator);
+  }
+  if (page.revision != null && page.revision.author != null && page.revision.author._id != null) {
+    page.revision.author = serializeUserSecurely(page.revision.author);
+  }
+  return page;
+}
+
+function serializePageSecurely(page, shouldDepopulateRevisions = false) {
+  let serialized = page;
+
+  // invoke toObject if page is a model instance
+  if (page.toObject != null) {
+    serialized = page.toObject();
+  }
+
+  // optional depopulation
+  if (shouldDepopulateRevisions) {
+    depopulateRevisions(serialized);
+  }
+
+  serializeInsecureUserAttributes(serialized);
+
+  return serialized;
+}
+
+module.exports = {
+  serializePageSecurely,
+};

src/server/models/serializers/user-serializer.js

@@ -0,0 +1,27 @@
+function omitInsecureAttributes(user) {
+  // omit password
+  delete user.password;
+  // omit email
+  if (!user.isEmailPublished) {
+    delete user.email;
+  }
+  return user;
+}
+
+function serializeUserSecurely(user) {
+  let serialized = user;
+
+  // invoke toObject if page is a model instance
+  if (user.toObject != null) {
+    serialized = user.toObject();
+  }
+
+  omitInsecureAttributes(serialized);
+
+  return serialized;
+}
+
+module.exports = {
+  omitInsecureAttributes,
+  serializeUserSecurely,
+};

src/server/models/user.js

@@ -13,6 +13,8 @@ const crypto = require('crypto');
 
 const { listLocaleIds, migrateDeprecatedLocaleId } = require('@commons/util/locale-utils');
 
+const { omitInsecureAttributes } = require('./serializers/user-serializer');
+
 module.exports = function(crowi) {
   const STATUS_REGISTERED = 1;
   const STATUS_ACTIVE = 2;
@@ -65,13 +67,7 @@ module.exports = function(crowi) {
   }, {
     toObject: {
       transform: (doc, ret, opt) => {
-        // omit password
-        delete ret.password;
-        // omit email
-        if (!doc.isEmailPublished) {
-          delete ret.email;
-        }
-        return ret;
+        return omitInsecureAttributes(ret);
       },
     },
   });

src/server/models/vo/s2c-message.js

@@ -0,0 +1,30 @@
+const { serializePageSecurely } = require('../serializers/page-serializer');
+
+/**
+ * Server-to-client message VO
+ */
+class S2cMessagePageUpdated {
+
+
+  constructor(page, user) {
+    const serializedPage = serializePageSecurely(page, true);
+
+    const {
+      _id, revision, revisionHackmdSynced, hasDraftOnHackmd,
+    } = serializedPage;
+
+    this.pageId = _id;
+    this.revisionId = revision;
+    this.revisionIdHackmdSynced = revisionHackmdSynced;
+    this.hasDraftOnHackmd = hasDraftOnHackmd;
+
+    if (user != null) {
+      this.lastUpdateUsername = user.name;
+    }
+  }
+
+}
+
+module.exports = {
+  S2cMessagePageUpdated,
+};

src/server/routes/page.js

@@ -1,3 +1,5 @@
+const { serializePageSecurely } = require('../models/serializers/page-serializer');
+
 /**
  * @swagger
  *  tags:
@@ -143,7 +145,6 @@ module.exports = function(crowi, app) {
   const { slackNotificationService, configManager } = crowi;
   const interceptorManager = crowi.getInterceptorManager();
   const globalNotificationService = crowi.getGlobalNotificationService();
-  const pageService = crowi.pageService;
 
   const actions = {};
 
@@ -780,7 +781,7 @@ module.exports = function(crowi, app) {
       savedTags = await PageTagRelation.listTagNamesByPage(createdPage.id);
     }
 
-    const result = { page: pageService.serializeToObj(createdPage), tags: savedTags };
+    const result = { page: serializePageSecurely(createdPage), tags: savedTags };
     res.json(ApiResponse.success(result));
 
     // update scopes for descendants
@@ -909,7 +910,7 @@ module.exports = function(crowi, app) {
       savedTags = await PageTagRelation.listTagNamesByPage(pageId);
     }
 
-    const result = { page: pageService.serializeToObj(page), tags: savedTags };
+    const result = { page: serializePageSecurely(page), tags: savedTags };
     res.json(ApiResponse.success(result));
 
     // update scopes for descendants
@@ -1009,7 +1010,7 @@ module.exports = function(crowi, app) {
     }
 
     const result = {};
-    result.page = page; // TODO consider to use serializeToObj method -- 2018.08.06 Yuki Takei
+    result.page = page; // TODO consider to use serializePageSecurely method -- 2018.08.06 Yuki Takei
 
     return res.json(ApiResponse.success(result));
   };
@@ -1240,7 +1241,7 @@ module.exports = function(crowi, app) {
 
     debug('Page deleted', page.path);
     const result = {};
-    result.page = page; // TODO consider to use serializeToObj method -- 2018.08.06 Yuki Takei
+    result.page = page; // TODO consider to use serializePageSecurely method -- 2018.08.06 Yuki Takei
 
     res.json(ApiResponse.success(result));
 
@@ -1287,7 +1288,7 @@ module.exports = function(crowi, app) {
     }
 
     const result = {};
-    result.page = page; // TODO consider to use serializeToObj method -- 2018.08.06 Yuki Takei
+    result.page = page; // TODO consider to use serializePageSecurely method -- 2018.08.06 Yuki Takei
 
     return res.json(ApiResponse.success(result));
   };
@@ -1398,7 +1399,7 @@ module.exports = function(crowi, app) {
     }
 
     const result = {};
-    result.page = page; // TODO consider to use serializeToObj method -- 2018.08.06 Yuki Takei
+    result.page = page; // TODO consider to use serializePageSecurely method -- 2018.08.06 Yuki Takei
 
     res.json(ApiResponse.success(result));
 

src/server/service/page.js

@@ -4,26 +4,6 @@ class PageService {
     this.crowi = crowi;
   }
 
-  serializeToObj(page) {
-    const { User } = this.crowi.models;
-
-    const returnObj = page.toObject();
-
-    // set the ObjectID to revisionHackmdSynced
-    if (page.revisionHackmdSynced != null && page.revisionHackmdSynced._id != null) {
-      returnObj.revisionHackmdSynced = page.revisionHackmdSynced._id;
-    }
-
-    if (page.lastUpdateUser != null && page.lastUpdateUser instanceof User) {
-      returnObj.lastUpdateUser = page.lastUpdateUser.toObject();
-    }
-    if (page.creator != null && page.creator instanceof User) {
-      returnObj.creator = page.creator.toObject();
-    }
-
-    return returnObj;
-  }
-
   async deleteCompletely(pageId, pagePath) {
     // Delete Bookmarks, Attachments, Revisions, Pages and emit delete
     const Bookmark = this.crowi.model('Bookmark');

src/server/service/system-events/sync-page-status.js

@@ -1,6 +1,7 @@
 const logger = require('@alias/logger')('growi:service:system-events:SyncPageStatusService');
 
 const S2sMessage = require('../../models/vo/s2s-message');
+const { S2cMessagePageUpdated } = require('../../models/vo/s2c-message');
 const S2sMessageHandlable = require('../s2s-messaging/handlable');
 
 /**
@@ -46,20 +47,20 @@ class SyncPageStatusService extends S2sMessageHandlable {
    * @inheritdoc
    */
   async handleS2sMessage(s2sMessage) {
-    const { socketIoEventName, page, user } = s2sMessage;
+    const { socketIoEventName, s2cMessageBody } = s2sMessage;
     const { socketIoService } = this;
 
     // emit the updated information to clients
     if (socketIoService.isInitialized) {
-      socketIoService.getDefaultSocket().emit(socketIoEventName, { page, user });
+      socketIoService.getDefaultSocket().emit(socketIoEventName, s2cMessageBody);
     }
   }
 
-  async publishToOtherServers(socketIoEventName, page, user) {
+  async publishToOtherServers(socketIoEventName, s2cMessageBody) {
     const { s2sMessagingService } = this;
 
     if (s2sMessagingService != null) {
-      const s2sMessage = new S2sMessage('pageStatusUpdated', { socketIoEventName, page, user });
+      const s2sMessage = new S2sMessage('pageStatusUpdated', { socketIoEventName, s2cMessageBody });
 
       try {
         await s2sMessagingService.publish(s2sMessage);
@@ -72,36 +73,36 @@ class SyncPageStatusService extends S2sMessageHandlable {
 
   initSystemEventListeners() {
     const { socketIoService } = this;
-    const { pageService } = this.crowi;
 
     // register events
     this.emitter.on('create', (page, user, socketClientId) => {
       logger.debug('\'create\' event emitted.');
 
-      page = pageService.serializeToObj(page); // eslint-disable-line no-param-reassign
-      socketIoService.getDefaultSocket().emit('page:create', { page, user, socketClientId });
+      const s2cMessagePageUpdated = new S2cMessagePageUpdated(page, user);
+      socketIoService.getDefaultSocket().emit('page:create', { s2cMessagePageUpdated, socketClientId });
 
-      this.publishToOtherServers('page:create', page, user);
+      this.publishToOtherServers('page:create', { s2cMessagePageUpdated });
     });
     this.emitter.on('update', (page, user, socketClientId) => {
       logger.debug('\'update\' event emitted.');
 
-      page = pageService.serializeToObj(page); // eslint-disable-line no-param-reassign
-      socketIoService.getDefaultSocket().emit('page:update', { page, user, socketClientId });
+      const s2cMessagePageUpdated = new S2cMessagePageUpdated(page, user);
+      socketIoService.getDefaultSocket().emit('page:update', { s2cMessagePageUpdated, socketClientId });
 
-      this.publishToOtherServers('page:update', page, user);
+      this.publishToOtherServers('page:update', { s2cMessagePageUpdated });
     });
     this.emitter.on('delete', (page, user, socketClientId) => {
       logger.debug('\'delete\' event emitted.');
 
-      page = pageService.serializeToObj(page); // eslint-disable-line no-param-reassign
-      socketIoService.getDefaultSocket().emit('page:delete', { page, user, socketClientId });
+      const s2cMessagePageUpdated = new S2cMessagePageUpdated(page, user);
+      socketIoService.getDefaultSocket().emit('page:delete', { s2cMessagePageUpdated, socketClientId });
 
-      this.publishToOtherServers('page:delete', page, user);
+      this.publishToOtherServers('page:delete', { s2cMessagePageUpdated });
     });
     this.emitter.on('saveOnHackmd', (page) => {
-      socketIoService.getDefaultSocket().emit('page:editingWithHackmd', { page });
-      this.publishToOtherServers('page:editingWithHackmd', page);
+      const s2cMessagePageUpdated = new S2cMessagePageUpdated(page);
+      socketIoService.getDefaultSocket().emit('page:editingWithHackmd', { s2cMessagePageUpdated });
+      this.publishToOtherServers('page:editingWithHackmd', { s2cMessagePageUpdated });
     });
   }