From 098fadb4d7984405de9a255a77aa4f7341e1846e Mon Sep 17 00:00:00 2001 From: Jessica Lawshe <5312836+lawshe@users.noreply.github.com> Date: Mon, 30 Oct 2023 08:52:25 -0500 Subject: [PATCH] Merge pull request #15491 from overleaf/jel-token-hash-log-user-id [web] Log the user ID on token hash prefix mismatches GitOrigin-RevId: 78d298051a9c1794ed38422bef24c852dcee3bb2 --- .../TokenAccess/TokenAccessController.js | 14 +++++++++++-- .../TokenAccess/TokenAccessHandler.js | 4 ++-- .../TokenAccess/TokenAccessControllerTests.js | 21 ++++++++++++++++--- .../TokenAccess/TokenAccessHandlerTests.js | 6 ++++-- 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/services/web/app/src/Features/TokenAccess/TokenAccessController.js b/services/web/app/src/Features/TokenAccess/TokenAccessController.js index 4e0fe30b89..96bc6b69a8 100644 --- a/services/web/app/src/Features/TokenAccess/TokenAccessController.js +++ b/services/web/app/src/Features/TokenAccess/TokenAccessController.js @@ -233,7 +233,12 @@ async function grantTokenAccessReadAndWrite(req, res, next) { } const tokenType = TokenAccessHandler.TOKEN_TYPES.READ_AND_WRITE - TokenAccessHandler.checkTokenHashPrefix(token, tokenHashPrefix, tokenType) + TokenAccessHandler.checkTokenHashPrefix( + token, + tokenHashPrefix, + tokenType, + userId + ) try { const [project, action] = await checkAndGetProjectOrResponseAction( @@ -298,7 +303,12 @@ async function grantTokenAccessReadOnly(req, res, next) { const tokenType = TokenAccessHandler.TOKEN_TYPES.READ_ONLY - TokenAccessHandler.checkTokenHashPrefix(token, tokenHashPrefix, tokenType) + TokenAccessHandler.checkTokenHashPrefix( + token, + tokenHashPrefix, + tokenType, + userId + ) const docPublishedInfo = await TokenAccessHandler.promises.getV1DocPublishedInfo(token) diff --git a/services/web/app/src/Features/TokenAccess/TokenAccessHandler.js b/services/web/app/src/Features/TokenAccess/TokenAccessHandler.js index 234d3e836a..2d6e4ad7d4 100644 --- a/services/web/app/src/Features/TokenAccess/TokenAccessHandler.js +++ b/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) { + checkTokenHashPrefix(token, tokenHashPrefix, type, userId) { let hashPrefixStatus if (tokenHashPrefix) { @@ -307,7 +307,7 @@ const TokenAccessHandler = { if (hashPrefixStatus === 'mismatch') { logger.info( - { tokenHashPrefix, hashPrefixStatus }, + { tokenHashPrefix, hashPrefixStatus, userId }, 'mismatched token hash prefix' ) } diff --git a/services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js b/services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js index 0beb92d794..2da7cb6f4b 100644 --- a/services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js +++ b/services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js @@ -109,7 +109,12 @@ describe('TokenAccessController', function () { it('checks token hash', function () { expect( this.TokenAccessHandler.checkTokenHashPrefix - ).to.have.been.calledWith(this.token, 'prefix', 'readAndWrite') + ).to.have.been.calledWith( + this.token, + 'prefix', + 'readAndWrite', + this.user._id + ) }) }) @@ -153,7 +158,12 @@ describe('TokenAccessController', function () { it('sends missing hash to metrics', function () { expect( this.TokenAccessHandler.checkTokenHashPrefix - ).to.have.been.calledWith(this.token, undefined, 'readAndWrite') + ).to.have.been.calledWith( + this.token, + undefined, + 'readAndWrite', + this.user._id + ) }) }) }) @@ -192,7 +202,12 @@ describe('TokenAccessController', function () { it('sends checks if hash prefix matches', function () { expect( this.TokenAccessHandler.checkTokenHashPrefix - ).to.have.been.calledWith(this.token, 'prefix', 'readOnly') + ).to.have.been.calledWith( + this.token, + 'prefix', + 'readOnly', + this.user._id + ) }) }) diff --git a/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js b/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js index 3d8d681a18..824c000f11 100644 --- a/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js +++ b/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js @@ -650,6 +650,7 @@ describe('TokenAccessHandler', function () { }) describe('checkTokenHashPrefix', function () { + const userId = 'abc123' 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) @@ -674,7 +675,8 @@ describe('TokenAccessHandler', function () { this.TokenAccessHandler.checkTokenHashPrefix( 'anothertoken', `#${prefix}`, - 'readOnly' + 'readOnly', + userId ) expect(this.Metrics.inc).to.have.been.calledWith( @@ -685,7 +687,7 @@ describe('TokenAccessHandler', function () { } ) expect(this.logger.info).to.have.been.calledWith( - { tokenHashPrefix: prefix, hashPrefixStatus: 'mismatch' }, + { tokenHashPrefix: prefix, hashPrefixStatus: 'mismatch', userId }, 'mismatched token hash prefix' ) })