Merge pull request #2975 from overleaf/cmg-sk-restricted-users-names

Hide data from restricted users in history, setting history label usernames on server side

GitOrigin-RevId: 820d92064d2c0bc99ba756cca4be1adab0da5753

services/web/app/src/Features/Editor/EditorHttpController.js

@@ -68,8 +68,10 @@ async function joinProject(req, res, next) {
   }
   // Hide access tokens if this is not the project owner
   TokenAccessHandler.protectTokens(project, privilegeLevel)
+  // Hide sensitive data if the user is restricted
   if (isRestrictedUser) {
     project.owner = { _id: project.owner._id }
+    project.members = []
   }
   // Only show the 'renamed or deleted' message once
   if (project.deletedByExternalDataSource) {

services/web/app/src/Features/History/HistoryController.js

@@ -21,6 +21,7 @@ const logger = require('logger-sharelatex')
 const request = require('request')
 const settings = require('settings-sharelatex')
 const AuthenticationController = require('../Authentication/AuthenticationController')
+const UserGetter = require('../User/UserGetter')
 const Errors = require('../Errors/Errors')
 const HistoryManager = require('./HistoryManager')
 const ProjectDetailsHandler = require('../Project/ProjectDetailsHandler')
@@ -201,7 +202,12 @@ module.exports = HistoryController = {
         if (error != null) {
           return next(error)
         }
+        HistoryController._enrichLabels(labels, (err, labels) => {
+          if (err) {
+            return next(err)
+          }
           return res.json(labels)
+        })
       }
     )
   },
@@ -222,11 +228,86 @@ module.exports = HistoryController = {
         if (error != null) {
           return next(error)
         }
+        HistoryController._enrichLabel(label, (err, label) => {
+          if (err) {
+            return next(err)
+          }
           return res.json(label)
+        })
       }
     )
   },
 
+  _enrichLabel(label, callback) {
+    if (!label.user_id) {
+      return callback(null, label)
+    }
+    UserGetter.getUser(
+      label.user_id,
+      { first_name: 1, last_name: 1, email: 1 },
+      (err, user) => {
+        if (err) {
+          return callback(err)
+        }
+        const newLabel = Object.assign({}, label)
+        newLabel.user_display_name = HistoryController._displayNameForUser(user)
+        callback(null, newLabel)
+      }
+    )
+  },
+
+  _enrichLabels(labels, callback) {
+    if (!labels || !labels.length) {
+      return callback(null, [])
+    }
+    const uniqueUsers = new Set(labels.map(label => label.user_id))
+
+    // For backwards compatibility expect missing user_id fields
+    uniqueUsers.delete(undefined)
+
+    if (!uniqueUsers.size) {
+      return callback(null, labels)
+    }
+
+    UserGetter.getUsers(
+      Array.from(uniqueUsers),
+      { first_name: 1, last_name: 1, email: 1 },
+      function(err, rawUsers) {
+        if (err) {
+          return callback(err)
+        }
+        const users = new Map(rawUsers.map(user => [String(user._id), user]))
+
+        labels.forEach(label => {
+          const user = users.get(label.user_id)
+          if (!user) return
+          label.user_display_name = HistoryController._displayNameForUser(user)
+        })
+        callback(null, labels)
+      }
+    )
+  },
+
+  _displayNameForUser(user) {
+    if (user == null) {
+      return 'Anonymous'
+    }
+    if (user.name != null) {
+      return user.name
+    }
+    let name = [user.first_name, user.last_name]
+      .filter(n => n != null)
+      .join(' ')
+      .trim()
+    if (name === '') {
+      name = user.email.split('@')[0]
+    }
+    if (name == null || name === '') {
+      return '?'
+    }
+    return name
+  },
+
   deleteLabel(req, res, next) {
     const project_id = req.params.Project_id
     const { label_id } = req.params

services/web/app/views/project/editor/history/entriesListV2.pug

@@ -129,8 +129,9 @@ script(type="text/ng-template", id="historyEntryTpl")
 
 			history-label(				
 				ng-repeat="label in $ctrl.entry.labels | orderBy : '-created_at'"
+				ng-init="user = $ctrl.buildUserView(label)"
 				label-text="label.comment"
-				label-owner-name="$ctrl.displayNameById(label.user_id)"
+				label-owner-name="$ctrl.displayNameById(label.user_id) || 'Anonymous'"
 				label-creation-date-time="label.created_at"
 				is-owned-by-current-user="label.user_id === $ctrl.currentUser.id"
 				on-label-delete="$ctrl.onLabelDelete({ label: label })"
@@ -223,13 +224,13 @@ script(type="text/ng-template", id="historyLabelsListTpl")
 				.history-entry-label-metadata
 					.history-entry-label-metadata-user(
 						ng-if="!label.isPseudoCurrentStateLabel"
-						ng-init="user = $ctrl.getUserById(label.user_id)"
+						ng-init="user = $ctrl.buildUserView(label)"
 					)
 						| Saved by 
 						span.name(
 							ng-if="user && user._id !== $ctrl.currentUser.id"
 							ng-style="$ctrl.getUserCSSStyle(user, versionWithLabel);"
-						) {{ ::$ctrl.displayName(user) }} 
+						) {{ ::user.displayName }}
 						span.name(
 							ng-if="user && user._id == $ctrl.currentUser.id"
 							ng-style="$ctrl.getUserCSSStyle(user, versionWithLabel);"

services/web/frontend/js/ide/history/components/historyLabelsList.js

@@ -129,16 +129,13 @@ const historyLabelsListController = function($scope, $element, $attrs) {
       })
     }
   }
-  // This method (and maybe the one below) will be removed soon. User details data will be
-  // injected into the history API responses, so we won't need to fetch user data from other
-  // local data structures.
-  ctrl.getUserById = id =>
-    _.find(ctrl.users, function(user) {
-      const curUserId =
-        (user != null ? user._id : undefined) ||
-        (user != null ? user.id : undefined)
-      return curUserId === id
-    })
+  ctrl.buildUserView = label => {
+    const user = {
+      _id: label.user_id,
+      displayName: label.user_display_name
+    }
+    return user
+  }
   ctrl.displayName = displayNameForUser
   ctrl.getUserCSSStyle = function(user, versionWithLabel) {
     const curUserId =

services/web/test/unit/src/Editor/EditorHttpControllerTests.js

@@ -23,11 +23,13 @@ describe('EditorHttpController', function() {
         _id: 'owner',
         email: 'owner@example.com',
         other_property: true
-      }
+      },
+      members: [{ one: 1 }, { two: 2 }]
     }
     this.reducedProjectView = {
       _id: this.projectView._id,
-      owner: { _id: this.projectView.owner._id }
+      owner: { _id: this.projectView.owner._id },
+      members: []
     }
     this.doc = { _id: new ObjectId(), name: 'excellent-original-idea.tex' }
     this.file = { _id: new ObjectId() }

services/web/test/unit/src/History/HistoryControllerTests.js

@@ -43,6 +43,7 @@ describe('HistoryController', function() {
         './HistoryManager': (this.HistoryManager = {}),
         '../Project/ProjectDetailsHandler': (this.ProjectDetailsHandler = {}),
         '../Project/ProjectEntityUpdateHandler': (this.ProjectEntityUpdateHandler = {}),
+        '../User/UserGetter': (this.UserGetter = {}),
         './RestoreManager': (this.RestoreManager = {})
       }
     })