Jelajahi Sumber

Merge pull request #15822 from overleaf/mj-audit-log-tokens

[web] Add audit logs for token expiration operations

GitOrigin-RevId: 220fe017cf508ead986a4cd2bd9009035418ce43
Mathias Jakobsen 2 tahun lalu
induk
melakukan
9ca43ebc4e

+ 4 - 1
services/web/app/src/Features/PasswordReset/PasswordResetHandler.js

@@ -6,6 +6,8 @@ const EmailHandler = require('../Email/EmailHandler')
 const AuthenticationManager = require('../Authentication/AuthenticationManager')
 const { callbackify, promisify } = require('util')
 
+const AUDIT_LOG_TOKEN_PREFIX_LENGTH = 10
+
 function generateAndEmailResetToken(email, callback) {
   UserGetter.getUserByAnyEmail(email, (err, user) => {
     if (err || !user) {
@@ -103,7 +105,8 @@ async function setNewUserPassword(token, password, auditLog) {
     user._id,
     'reset-password',
     auditLog.initiatorId,
-    auditLog.ip
+    auditLog.ip,
+    { token: token.substring(0, AUDIT_LOG_TOKEN_PREFIX_LENGTH) }
   )
 
   const reset = await AuthenticationManager.promises.setUserPassword(

+ 6 - 1
services/web/app/src/Features/User/UserEmailsConfirmationHandler.js

@@ -125,7 +125,12 @@ const UserEmailsConfirmationHandler = {
           if (!emailExists) {
             return callback(new Errors.NotFoundError('email missing for user'))
           }
-          UserUpdater.confirmEmail(userId, email, callback)
+          UserUpdater.confirmEmail(userId, email, function (error) {
+            if (error) {
+              return callback(error)
+            }
+            callback(null, { userId, email })
+          })
         })
       }
     )

+ 22 - 2
services/web/app/src/Features/User/UserEmailsController.js

@@ -14,6 +14,9 @@ const { expressify } = require('@overleaf/promise-utils')
 const AsyncFormHelper = require('../Helpers/AsyncFormHelper')
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
 const UserPrimaryEmailCheckHandler = require('../User/UserPrimaryEmailCheckHandler')
+const UserAuditLogHandler = require('./UserAuditLogHandler')
+
+const AUDIT_LOG_TOKEN_PREFIX_LENGTH = 10
 
 async function _sendSecurityAlertEmail(user, email) {
   const emailOptions = {
@@ -267,7 +270,7 @@ const UserEmailsController = {
     }
     UserEmailsConfirmationHandler.confirmEmailFromToken(
       token,
-      function (error) {
+      function (error, userData) {
         if (error) {
           if (error instanceof Errors.NotFoundError) {
             res.status(404).json({
@@ -277,7 +280,24 @@ const UserEmailsController = {
             next(error)
           }
         } else {
-          res.sendStatus(200)
+          const { userId, email } = userData
+          const tokenPrefix = token.substring(0, AUDIT_LOG_TOKEN_PREFIX_LENGTH)
+          UserAuditLogHandler.addEntry(
+            userId,
+            'confirm-email',
+            userId,
+            req.ip,
+            { token: tokenPrefix, email },
+            auditLogError => {
+              if (auditLogError) {
+                logger.error(
+                  { error: auditLogError, userId, token: tokenPrefix },
+                  'failed to add audit log entry'
+                )
+              }
+              res.sendStatus(200)
+            }
+          )
         }
       }
     )

+ 16 - 14
services/web/test/unit/src/PasswordReset/PasswordResetHandlerTests.js

@@ -290,14 +290,15 @@ describe('PasswordResetHandler', function () {
               this.auditLog,
               (error, result) => {
                 const { reset, userId } = result
+                sinon.assert.calledWith(
+                  this.UserAuditLogHandler.promises.addEntry,
+                  this.user_id,
+                  'reset-password',
+                  undefined,
+                  this.auditLog.ip,
+                  { token: this.token.substring(0, 10) }
+                )
                 expect(error).to.not.exist
-                const logCall =
-                  this.UserAuditLogHandler.promises.addEntry.lastCall
-                expect(logCall.args[0]).to.equal(this.user_id)
-                expect(logCall.args[1]).to.equal('reset-password')
-                expect(logCall.args[2]).to.equal(undefined)
-                expect(logCall.args[3]).to.equal(this.auditLog.ip)
-                expect(logCall.args[4]).to.equal(undefined)
                 done()
               }
             )
@@ -344,13 +345,14 @@ describe('PasswordResetHandler', function () {
                 (error, result) => {
                   const { reset, userId } = result
                   expect(error).to.not.exist
-                  const logCall =
-                    this.UserAuditLogHandler.promises.addEntry.lastCall
-                  expect(logCall.args[0]).to.equal(this.user_id)
-                  expect(logCall.args[1]).to.equal('reset-password')
-                  expect(logCall.args[2]).to.equal(this.user_id)
-                  expect(logCall.args[3]).to.equal(this.auditLog.ip)
-                  expect(logCall.args[4]).to.equal(undefined)
+                  sinon.assert.calledWith(
+                    this.UserAuditLogHandler.promises.addEntry,
+                    this.user_id,
+                    'reset-password',
+                    this.user_id,
+                    this.auditLog.ip,
+                    { token: this.token.substring(0, 10) }
+                  )
                   done()
                 }
               )

+ 20 - 1
services/web/test/unit/src/User/UserEmailsControllerTests.js

@@ -56,6 +56,9 @@ describe('UserEmailsController', function () {
     this.AnalyticsManager = {
       recordEventForUser: sinon.stub(),
     }
+    this.UserAuditLogHandler = {
+      addEntry: sinon.stub().yields(),
+    }
     this.UserEmailsController = SandboxedModule.require(modulePath, {
       requires: {
         '../Authentication/SessionManager': this.SessionManager,
@@ -79,6 +82,7 @@ describe('UserEmailsController', function () {
         '../Institutions/InstitutionsAPI': this.InstitutionsAPI,
         '../Errors/HttpErrorHandler': this.HttpErrorHandler,
         '../Analytics/AnalyticsManager': this.AnalyticsManager,
+        './UserAuditLogHandler': this.UserAuditLogHandler,
       },
     })
   })
@@ -416,7 +420,7 @@ describe('UserEmailsController', function () {
     beforeEach(function () {
       this.UserEmailsConfirmationHandler.confirmEmailFromToken = sinon
         .stub()
-        .yields()
+        .yields(null, { userId: this.user._id, email: this.user.email })
       this.res = {
         sendStatus: sinon.stub(),
         json: sinon.stub(),
@@ -425,6 +429,7 @@ describe('UserEmailsController', function () {
       this.next = sinon.stub()
       this.token = 'mock-token'
       this.req.body = { token: this.token }
+      this.req.ip = '0.0.0.0'
     })
 
     describe('successfully', function () {
@@ -441,6 +446,20 @@ describe('UserEmailsController', function () {
       it('should return a 200 status', function () {
         this.res.sendStatus.calledWith(200).should.equal(true)
       })
+
+      it('should log the confirmation to the audit log', function () {
+        sinon.assert.calledWith(
+          this.UserAuditLogHandler.addEntry,
+          this.user._id,
+          'confirm-email',
+          this.user._id,
+          this.req.ip,
+          {
+            token: this.token.substring(0, 10),
+            email: this.user.email,
+          }
+        )
+      })
     })
 
     describe('without a token', function () {