Merge pull request #3749 from overleaf/jpa-refactor-audit-log-access-in-tests

[misc] test/acceptance: refactor access on user audit log

GitOrigin-RevId: fab5b81a7469ef4fa0daa553169a47e3427c49fd

services/web/test/acceptance/src/PasswordResetTests.js

@@ -59,13 +59,14 @@ describe('PasswordReset', function() {
         expect(user.password).to.not.exist
       })
       it('log the change with initiatorId', async function() {
-        expect(user.auditLog).to.exist
-        expect(user.auditLog[0]).to.exist
-        expect(typeof user.auditLog[0].initiatorId).to.equal('object')
-        expect(user.auditLog[0].initiatorId).to.deep.equal(user._id)
-        expect(user.auditLog[0].operation).to.equal('reset-password')
-        expect(user.auditLog[0].ipAddress).to.equal('127.0.0.1')
-        expect(user.auditLog[0].timestamp).to.exist
+        const auditLog = userHelper.getAuditLogWithoutNoise()
+        expect(auditLog).to.exist
+        expect(auditLog[0]).to.exist
+        expect(typeof auditLog[0].initiatorId).to.equal('object')
+        expect(auditLog[0].initiatorId).to.deep.equal(user._id)
+        expect(auditLog[0].operation).to.equal('reset-password')
+        expect(auditLog[0].ipAddress).to.equal('127.0.0.1')
+        expect(auditLog[0].timestamp).to.exist
       })
     })
     describe('when logged in as another user', function() {
@@ -99,13 +100,14 @@ describe('PasswordReset', function() {
         expect(user.password).to.not.exist
       })
       it('log the change with the logged in user as the initiatorId', async function() {
-        expect(user.auditLog).to.exist
-        expect(user.auditLog[0]).to.exist
-        expect(typeof user.auditLog[0].initiatorId).to.equal('object')
-        expect(user.auditLog[0].initiatorId).to.deep.equal(otherUser._id)
-        expect(user.auditLog[0].operation).to.equal('reset-password')
-        expect(user.auditLog[0].ipAddress).to.equal('127.0.0.1')
-        expect(user.auditLog[0].timestamp).to.exist
+        const auditLog = userHelper.getAuditLogWithoutNoise()
+        expect(auditLog).to.exist
+        expect(auditLog[0]).to.exist
+        expect(typeof auditLog[0].initiatorId).to.equal('object')
+        expect(auditLog[0].initiatorId).to.deep.equal(otherUser._id)
+        expect(auditLog[0].operation).to.equal('reset-password')
+        expect(auditLog[0].ipAddress).to.equal('127.0.0.1')
+        expect(auditLog[0].timestamp).to.exist
       })
     })
     describe('when not logged in', function() {
@@ -131,12 +133,13 @@ describe('PasswordReset', function() {
         expect(user.password).to.not.exist
       })
       it('log the change', async function() {
-        expect(user.auditLog).to.exist
-        expect(user.auditLog[0]).to.exist
-        expect(user.auditLog[0].initiatorId).to.equal(null)
-        expect(user.auditLog[0].operation).to.equal('reset-password')
-        expect(user.auditLog[0].ipAddress).to.equal('127.0.0.1')
-        expect(user.auditLog[0].timestamp).to.exist
+        const auditLog = userHelper.getAuditLogWithoutNoise()
+        expect(auditLog).to.exist
+        expect(auditLog[0]).to.exist
+        expect(auditLog[0].initiatorId).to.equal(null)
+        expect(auditLog[0].operation).to.equal('reset-password')
+        expect(auditLog[0].ipAddress).to.equal('127.0.0.1')
+        expect(auditLog[0].timestamp).to.exist
       })
     })
     describe('password checks', function() {
@@ -158,8 +161,9 @@ describe('PasswordReset', function() {
         })
         expect(response.statusCode).to.equal(400)
         userHelper = await UserHelper.getUser({ email })
-        user = userHelper.user
-        expect(user.auditLog).to.deep.equal([])
+
+        const auditLog = userHelper.getAuditLogWithoutNoise()
+        expect(auditLog).to.deep.equal([])
       })
 
       it('without a valid password should return 400 and not log the change', async function() {
@@ -173,8 +177,9 @@ describe('PasswordReset', function() {
         })
         expect(response.statusCode).to.equal(400)
         userHelper = await UserHelper.getUser({ email })
-        user = userHelper.user
-        expect(user.auditLog).to.deep.equal([])
+
+        const auditLog = userHelper.getAuditLogWithoutNoise()
+        expect(auditLog).to.deep.equal([])
       })
     })
   })

services/web/test/acceptance/src/PasswordUpdateTests.js

@@ -31,18 +31,20 @@ describe('PasswordUpdate', function() {
         },
         simple: false
       })
-      user = (await UserHelper.getUser({ email })).user
+      userHelper = await UserHelper.getUser({ email })
+      user = userHelper.user
     })
     it('should return 200', async function() {
       expect(response.statusCode).to.equal(200)
     })
     it('should update the audit log', function() {
-      expect(user.auditLog[0]).to.exist
-      expect(typeof user.auditLog[0].initiatorId).to.equal('object')
-      expect(user.auditLog[0].initiatorId).to.deep.equal(user._id)
-      expect(user.auditLog[0].operation).to.equal('update-password')
-      expect(user.auditLog[0].ipAddress).to.equal('127.0.0.1')
-      expect(user.auditLog[0].timestamp).to.exist
+      const auditLog = userHelper.getAuditLogWithoutNoise()
+      expect(auditLog[0]).to.exist
+      expect(typeof auditLog[0].initiatorId).to.equal('object')
+      expect(auditLog[0].initiatorId).to.deep.equal(user._id)
+      expect(auditLog[0].operation).to.equal('update-password')
+      expect(auditLog[0].ipAddress).to.equal('127.0.0.1')
+      expect(auditLog[0].timestamp).to.exist
     })
   })
   describe('errors', function() {
@@ -55,13 +57,14 @@ describe('PasswordUpdate', function() {
           },
           simple: false
         })
-        user = (await UserHelper.getUser({ email })).user
+        userHelper = await UserHelper.getUser({ email })
       })
       it('should return 500', async function() {
         expect(response.statusCode).to.equal(500)
       })
       it('should not update audit log', async function() {
-        expect(user.auditLog[0]).to.not.exist
+        const auditLog = userHelper.getAuditLogWithoutNoise()
+        expect(auditLog).to.deep.equal([])
       })
     })
     describe('wrong current password', function() {
@@ -74,13 +77,14 @@ describe('PasswordUpdate', function() {
           },
           simple: false
         })
-        user = (await UserHelper.getUser({ email })).user
+        userHelper = await UserHelper.getUser({ email })
       })
       it('should return 400', async function() {
         expect(response.statusCode).to.equal(400)
       })
       it('should not update audit log', async function() {
-        expect(user.auditLog[0]).to.not.exist
+        const auditLog = userHelper.getAuditLogWithoutNoise()
+        expect(auditLog).to.deep.equal([])
       })
     })
     describe('newPassword1 does not match newPassword2', function() {
@@ -94,7 +98,7 @@ describe('PasswordUpdate', function() {
           json: true,
           simple: false
         })
-        user = (await UserHelper.getUser({ email })).user
+        userHelper = await UserHelper.getUser({ email })
       })
       it('should return 400', async function() {
         expect(response.statusCode).to.equal(400)
@@ -103,7 +107,8 @@ describe('PasswordUpdate', function() {
         expect(response.body.message).to.equal('Passwords do not match')
       })
       it('should not update audit log', async function() {
-        expect(user.auditLog[0]).to.not.exist
+        const auditLog = userHelper.getAuditLogWithoutNoise()
+        expect(auditLog).to.deep.equal([])
       })
     })
     describe('new password is not valid', function() {
@@ -117,7 +122,7 @@ describe('PasswordUpdate', function() {
           json: true,
           simple: false
         })
-        user = (await UserHelper.getUser({ email })).user
+        userHelper = await UserHelper.getUser({ email })
       })
       it('should return 400', async function() {
         expect(response.statusCode).to.equal(400)
@@ -126,7 +131,8 @@ describe('PasswordUpdate', function() {
         expect(response.body.message).to.equal('password is too short')
       })
       it('should not update audit log', async function() {
-        expect(user.auditLog[0]).to.not.exist
+        const auditLog = userHelper.getAuditLogWithoutNoise()
+        expect(auditLog).to.deep.equal([])
       })
     })
   })

services/web/test/acceptance/src/SessionTests.js

@@ -455,14 +455,14 @@ describe('Sessions', function() {
 
           // the user audit log should have been updated
           next => {
-            this.user1.get((error, user) => {
+            this.user1.getAuditLogWithoutNoise((error, auditLog) => {
               expect(error).not.to.exist
-              expect(user.auditLog).to.exist
-              expect(user.auditLog[0].operation).to.equal('clear-sessions')
-              expect(user.auditLog[0].ipAddress).to.exist
-              expect(user.auditLog[0].initiatorId).to.exist
-              expect(user.auditLog[0].timestamp).to.exist
-              expect(user.auditLog[0].info.sessions.length).to.equal(2)
+              expect(auditLog).to.exist
+              expect(auditLog[0].operation).to.equal('clear-sessions')
+              expect(auditLog[0].ipAddress).to.exist
+              expect(auditLog[0].initiatorId).to.exist
+              expect(auditLog[0].timestamp).to.exist
+              expect(auditLog[0].info.sessions.length).to.equal(2)
               next()
             })
           }

services/web/test/acceptance/src/UserEmailsTests.js

@@ -857,7 +857,8 @@ describe('UserEmails', function() {
         user = userHelper.user
       })
       it('should be updated', function() {
-        const entry = user.auditLog[user.auditLog.length - 1]
+        const auditLog = userHelper.getAuditLogWithoutNoise()
+        const entry = auditLog[auditLog.length - 1]
         expect(typeof entry.initiatorId).to.equal('object')
         expect(entry.initiatorId).to.deep.equal(user._id)
         expect(entry.ipAddress).to.equal('127.0.0.1')
@@ -994,10 +995,11 @@ describe('UserEmails', function() {
       expect(user.emails[1].email).to.equal(newEmail)
     })
     it('should add to the user audit log', async function() {
-      expect(typeof user.auditLog[0].initiatorId).to.equal('object')
-      expect(user.auditLog[0].initiatorId).to.deep.equal(user._id)
-      expect(user.auditLog[0].info.newSecondaryEmail).to.equal(newEmail)
-      expect(user.auditLog[0].ip).to.equal(this.user.request.ip)
+      const auditLog = userHelper.getAuditLogWithoutNoise()
+      expect(typeof auditLog[0].initiatorId).to.equal('object')
+      expect(auditLog[0].initiatorId).to.deep.equal(user._id)
+      expect(auditLog[0].info.newSecondaryEmail).to.equal(newEmail)
+      expect(auditLog[0].ip).to.equal(this.user.request.ip)
     })
   })
 

services/web/test/acceptance/src/helpers/User.js

@@ -44,6 +44,20 @@ class User {
     db.users.findOne({ _id: ObjectId(this._id) }, callback)
   }
 
+  getAuditLogWithoutNoise(callback) {
+    this.get((error, user) => {
+      if (error) return callback(error)
+      if (!user) return callback(new Error('User not found'))
+
+      callback(
+        null,
+        (user.auditLog || []).filter(entry => {
+          return entry.operation !== 'login'
+        })
+      )
+    })
+  }
+
   mongoUpdate(updateOp, callback) {
     db.users.updateOne({ _id: ObjectId(this._id) }, updateOp, callback)
   }

services/web/test/acceptance/src/helpers/UserHelper.js

@@ -27,6 +27,16 @@ class UserHelper {
 
   /* sync functions */
 
+  /**
+   * Get auditLog, ignore the login
+   * @return {object[]}
+   */
+  getAuditLogWithoutNoise() {
+    return (this.user.auditLog || []).filter(entry => {
+      return entry.operation !== 'login'
+    })
+  }
+
   /**
    * Generate default email from unique (per instantiation) user number
    * @returns {string} email
@@ -292,6 +302,13 @@ class UserHelper {
     return userHelper
   }
 
+  async refreshMongoUser() {
+    this.user = await UserGetter.promises.getUser({
+      _id: this.user._id
+    })
+    return this.user
+  }
+
   async addEmail(email) {
     let response = await this.request.post({
       form: {