Merge pull request #15558 from overleaf/jel-log-token-hash-mismatch

[web] Log the project ID on link sharing token hash prefix mismatches

GitOrigin-RevId: 37d15206f49920f49d61f22479b98dd4d448f6bd

services/web/app/src/Features/TokenAccess/TokenAccessController.js

@@ -233,13 +233,6 @@ async function grantTokenAccessReadAndWrite(req, res, next) {
   }
   const tokenType = TokenAccessHandler.TOKEN_TYPES.READ_AND_WRITE
 
-  TokenAccessHandler.checkTokenHashPrefix(
-    token,
-    tokenHashPrefix,
-    tokenType,
-    userId
-  )
-
   try {
     const [project, action] = await checkAndGetProjectOrResponseAction(
       tokenType,
@@ -249,12 +242,22 @@ async function grantTokenAccessReadAndWrite(req, res, next) {
       res,
       next
     )
+
+    TokenAccessHandler.checkTokenHashPrefix(
+      token,
+      tokenHashPrefix,
+      tokenType,
+      userId,
+      project?._id
+    )
+
     if (action) {
       return action()
     }
     if (!project) {
       return next(new Errors.NotFoundError())
     }
+
     if (!confirmedByUser) {
       return res.json({
         requireAccept: {
@@ -303,13 +306,6 @@ async function grantTokenAccessReadOnly(req, res, next) {
 
   const tokenType = TokenAccessHandler.TOKEN_TYPES.READ_ONLY
 
-  TokenAccessHandler.checkTokenHashPrefix(
-    token,
-    tokenHashPrefix,
-    tokenType,
-    userId
-  )
-
   const docPublishedInfo =
     await TokenAccessHandler.promises.getV1DocPublishedInfo(token)
   if (docPublishedInfo.allow === false) {
@@ -324,12 +320,22 @@ async function grantTokenAccessReadOnly(req, res, next) {
       res,
       next
     )
+
+    TokenAccessHandler.checkTokenHashPrefix(
+      token,
+      tokenHashPrefix,
+      tokenType,
+      userId,
+      project?._id
+    )
+
     if (action) {
       return action()
     }
     if (!project) {
       return next(new Errors.NotFoundError())
     }
+
     if (!confirmedByUser) {
       return res.json({
         requireAccept: {

services/web/app/src/Features/TokenAccess/TokenAccessHandler.js

@@ -287,7 +287,7 @@ const TokenAccessHandler = {
     return hash.digest('hex').slice(0, 6)
   },
 
-  checkTokenHashPrefix(token, tokenHashPrefix, type, userId) {
+  checkTokenHashPrefix(token, tokenHashPrefix, type, userId, projectId) {
     let hashPrefixStatus
 
     if (tokenHashPrefix) {
@@ -307,7 +307,7 @@ const TokenAccessHandler = {
 
     if (hashPrefixStatus === 'mismatch') {
       logger.info(
-        { tokenHashPrefix, hashPrefixStatus, userId },
+        { tokenHashPrefix, hashPrefixStatus, userId, projectId },
         'mismatched token hash prefix'
       )
     }

services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js

@@ -113,7 +113,8 @@ describe('TokenAccessController', function () {
           this.token,
           'prefix',
           'readAndWrite',
-          this.user._id
+          this.user._id,
+          this.project._id
         )
       })
     })
@@ -162,7 +163,8 @@ describe('TokenAccessController', function () {
           this.token,
           undefined,
           'readAndWrite',
-          this.user._id
+          this.user._id,
+          this.project._id
         )
       })
     })
@@ -199,14 +201,15 @@ describe('TokenAccessController', function () {
         )
       })
 
-      it('sends checks if hash prefix matches', function () {
+      it('checks if hash prefix matches', function () {
         expect(
           this.TokenAccessHandler.checkTokenHashPrefix
         ).to.have.been.calledWith(
           this.token,
           'prefix',
           'readOnly',
-          this.user._id
+          this.user._id,
+          this.project._id
         )
       })
     })
@@ -228,6 +231,18 @@ describe('TokenAccessController', function () {
         expect(this.ProjectAuditLogHandler.promises.addEntry).to.not.have.been
           .called
       })
+
+      it('still checks if hash prefix matches', function () {
+        expect(
+          this.TokenAccessHandler.checkTokenHashPrefix
+        ).to.have.been.calledWith(
+          this.token,
+          undefined,
+          'readOnly',
+          this.user._id,
+          this.project._id
+        )
+      })
     })
   })
 })

services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js

@@ -651,6 +651,7 @@ describe('TokenAccessHandler', function () {
 
   describe('checkTokenHashPrefix', function () {
     const userId = 'abc123'
+    const projectId = 'def456'
     it('sends "match" to metrics when prefix matches the prefix of the hash of the token', function () {
       const token = 'zxpxjrwdtsgd'
       const prefix = this.TokenAccessHandler.createTokenHashPrefix(token)
@@ -658,7 +659,9 @@ describe('TokenAccessHandler', function () {
       this.TokenAccessHandler.checkTokenHashPrefix(
         token,
         `#${prefix}`,
-        'readOnly'
+        'readOnly',
+        userId,
+        projectId
       )
 
       expect(this.Metrics.inc).to.have.been.calledWith(
@@ -676,7 +679,8 @@ describe('TokenAccessHandler', function () {
         'anothertoken',
         `#${prefix}`,
         'readOnly',
-        userId
+        userId,
+        projectId
       )
 
       expect(this.Metrics.inc).to.have.been.calledWith(
@@ -687,7 +691,12 @@ describe('TokenAccessHandler', function () {
         }
       )
       expect(this.logger.info).to.have.been.calledWith(
-        { tokenHashPrefix: prefix, hashPrefixStatus: 'mismatch', userId },
+        {
+          tokenHashPrefix: prefix,
+          hashPrefixStatus: 'mismatch',
+          userId,
+          projectId,
+        },
         'mismatched token hash prefix'
       )
     })
@@ -695,7 +704,9 @@ describe('TokenAccessHandler', function () {
       this.TokenAccessHandler.checkTokenHashPrefix(
         'anothertoken',
         undefined,
-        'readOnly'
+        'readOnly',
+        userId,
+        projectId
       )
 
       expect(this.Metrics.inc).to.have.been.calledWith(
@@ -710,7 +721,9 @@ describe('TokenAccessHandler', function () {
       this.TokenAccessHandler.checkTokenHashPrefix(
         'anothertoken',
         '#',
-        'readOnly'
+        'readOnly',
+        userId,
+        projectId
       )
 
       expect(this.Metrics.inc).to.have.been.calledWith(
@@ -728,7 +741,9 @@ describe('TokenAccessHandler', function () {
       this.TokenAccessHandler.checkTokenHashPrefix(
         token,
         `%23${prefix}`,
-        'readOnly'
+        'readOnly',
+        userId,
+        projectId
       )
 
       expect(this.Metrics.inc).to.have.been.calledWith(