diff --git a/services/web/app/src/Features/TokenAccess/TokenAccessController.js b/services/web/app/src/Features/TokenAccess/TokenAccessController.js index 54d33fbf91..6802f519c7 100644 --- a/services/web/app/src/Features/TokenAccess/TokenAccessController.js +++ b/services/web/app/src/Features/TokenAccess/TokenAccessController.js @@ -134,6 +134,7 @@ async function checkAndGetProjectOrResponseAction( anonWriteAccessDenied: true, }) }, + { action: 'denied anonymous read-and-write token access' }, ] } @@ -155,13 +156,15 @@ async function checkAndGetProjectOrResponseAction( res.sendStatus(404) } }, + { action: v1ImportData ? 'import v1' : '404' }, ] } else { - return [null, null] + return [null, null, { action: '404' }] } } const projectId = project._id + const tokenAccessEnabled = TokenAccessHandler.tokenAccessEnabledForProject(project) if (isAnonymousUser && tokenAccessEnabled) { @@ -177,6 +180,7 @@ async function checkAndGetProjectOrResponseAction( grantAnonymousAccess: tokenType, }) }, + { projectId, action: 'granting read-write anonymous access' }, ] } else { // anonymous read-and-write token access should have been denied already @@ -195,6 +199,7 @@ async function checkAndGetProjectOrResponseAction( grantAnonymousAccess: tokenType, }) }, + { projectId, action: 'granting read-only anonymous access' }, ] } else { throw new Error('unreachable') @@ -211,6 +216,7 @@ async function checkAndGetProjectOrResponseAction( () => { res.json({ redirect: `/project/${project._id}`, higherAccess: true }) }, + { projectId, action: 'user already has higher or same privilege' }, ] } if (!tokenAccessEnabled) { @@ -219,9 +225,10 @@ async function checkAndGetProjectOrResponseAction( () => { next(new Errors.NotFoundError()) }, + { projectId, action: 'token access not enabled' }, ] } - return [project, null] + return [project, null, { projectId, action: 'continue' }] } async function grantTokenAccessReadAndWrite(req, res, next) { @@ -234,7 +241,7 @@ async function grantTokenAccessReadAndWrite(req, res, next) { const tokenType = TokenAccessHandler.TOKEN_TYPES.READ_AND_WRITE try { - const [project, action] = await checkAndGetProjectOrResponseAction( + const [project, action, logData] = await checkAndGetProjectOrResponseAction( tokenType, token, userId, @@ -248,7 +255,7 @@ async function grantTokenAccessReadAndWrite(req, res, next) { tokenHashPrefix, tokenType, userId, - project?._id + logData ) if (action) { @@ -312,7 +319,7 @@ async function grantTokenAccessReadOnly(req, res, next) { return res.json({ redirect: docPublishedInfo.published_path }) } try { - const [project, action] = await checkAndGetProjectOrResponseAction( + const [project, action, logData] = await checkAndGetProjectOrResponseAction( tokenType, token, userId, @@ -326,7 +333,7 @@ async function grantTokenAccessReadOnly(req, res, next) { tokenHashPrefix, tokenType, userId, - project?._id + logData ) if (action) { diff --git a/services/web/app/src/Features/TokenAccess/TokenAccessHandler.js b/services/web/app/src/Features/TokenAccess/TokenAccessHandler.js index da8a931b24..b1ee4483d0 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, userId, projectId) { + checkTokenHashPrefix(token, tokenHashPrefix, type, userId, logData = {}) { let hashPrefixStatus if (tokenHashPrefix) { @@ -307,7 +307,7 @@ const TokenAccessHandler = { if (hashPrefixStatus === 'mismatch') { logger.info( - { tokenHashPrefix, hashPrefixStatus, userId, projectId }, + { tokenHashPrefix, hashPrefixStatus, userId, ...logData, type }, '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 05747514cf..4e57ba0e38 100644 --- a/services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js +++ b/services/web/test/unit/src/TokenAccess/TokenAccessControllerTests.js @@ -11,6 +11,7 @@ const MODULE_PATH = describe('TokenAccessController', function () { beforeEach(function () { + this.token = 'abc123' this.user = { _id: ObjectId() } this.project = { _id: ObjectId(), @@ -30,11 +31,14 @@ describe('TokenAccessController', function () { isReadOnlyToken: sinon.stub().returns(true), tokenAccessEnabledForProject: sinon.stub().returns(true), checkTokenHashPrefix: sinon.stub(), + makeTokenUrl: sinon.stub().returns('/'), + grantSessionTokenAccess: sinon.stub(), promises: { addReadAndWriteUserToProject: sinon.stub().resolves(), addReadOnlyUserToProject: sinon.stub().resolves(), getProjectByToken: sinon.stub().resolves(this.project), getV1DocPublishedInfo: sinon.stub().resolves({ allow: true }), + getV1DocInfo: sinon.stub(), }, } @@ -42,7 +46,9 @@ describe('TokenAccessController', function () { getLoggedInUserId: sinon.stub().returns(this.user._id), } - this.AuthenticationController = {} + this.AuthenticationController = { + setRedirectInSession: sinon.stub(), + } this.AuthorizationManager = { promises: { @@ -71,6 +77,7 @@ describe('TokenAccessController', function () { '../Authorization/AuthorizationMiddleware': this.AuthorizationMiddleware, '../Project/ProjectAuditLogHandler': this.ProjectAuditLogHandler, + '../Errors/Errors': (this.Errors = { NotFoundError: sinon.stub() }), }, }) }) @@ -114,7 +121,7 @@ describe('TokenAccessController', function () { 'prefix', 'readAndWrite', this.user._id, - this.project._id + { projectId: this.project._id, action: 'continue' } ) }) }) @@ -136,6 +143,18 @@ describe('TokenAccessController', function () { expect(this.ProjectAuditLogHandler.promises.addEntry).to.not.have.been .called }) + + it('checks token hash', function () { + expect( + this.TokenAccessHandler.checkTokenHashPrefix + ).to.have.been.calledWith( + this.token, + undefined, + 'readAndWrite', + this.user._id, + { projectId: this.project._id, action: 'continue' } + ) + }) }) describe('hash prefix missing in request', function () { @@ -156,7 +175,7 @@ describe('TokenAccessController', function () { ).to.have.been.calledWith(this.user._id, this.project._id) }) - it('sends missing hash to metrics', function () { + it('checks the hash prefix', function () { expect( this.TokenAccessHandler.checkTokenHashPrefix ).to.have.been.calledWith( @@ -164,10 +183,264 @@ describe('TokenAccessController', function () { undefined, 'readAndWrite', this.user._id, - this.project._id + { projectId: this.project._id, action: 'continue' } ) }) }) + + describe('user is owner of project', function () { + beforeEach(function (done) { + this.AuthorizationManager.promises.getPrivilegeLevelForProject.returns( + PrivilegeLevels.OWNER + ) + this.req.params = { token: this.token } + this.req.body = {} + this.res.callback = done + this.TokenAccessController.grantTokenAccessReadAndWrite( + this.req, + this.res, + done + ) + }) + it('checks token hash and includes log data', function () { + expect( + this.TokenAccessHandler.checkTokenHashPrefix + ).to.have.been.calledWith( + this.token, + undefined, + 'readAndWrite', + this.user._id, + { + projectId: this.project._id, + action: 'user already has higher or same privilege', + } + ) + }) + }) + + describe('when user is not logged in', function () { + beforeEach(function () { + this.SessionManager.getLoggedInUserId.returns(null) + this.req.params = { token: this.token } + this.req.body = { tokenHashPrefix: 'prefix' } + }) + describe('ANONYMOUS_READ_AND_WRITE_ENABLED is undefined', function () { + beforeEach(function (done) { + this.res.callback = done + this.TokenAccessController.grantTokenAccessReadAndWrite( + this.req, + this.res, + done + ) + }) + + it('redirects to restricted', function () { + expect(this.res.json).to.have.been.calledWith({ + redirect: '/restricted', + anonWriteAccessDenied: true, + }) + }) + + it('checks the hash prefix and includes log data', function () { + expect( + this.TokenAccessHandler.checkTokenHashPrefix + ).to.have.been.calledWith( + this.token, + 'prefix', + 'readAndWrite', + null, + { + action: 'denied anonymous read-and-write token access', + } + ) + }) + }) + + describe('ANONYMOUS_READ_AND_WRITE_ENABLED is true', function () { + beforeEach(function (done) { + this.TokenAccessHandler.ANONYMOUS_READ_AND_WRITE_ENABLED = true + this.res.callback = done + + this.TokenAccessController.grantTokenAccessReadAndWrite( + this.req, + this.res, + done + ) + }) + + it('redirects to project', function () { + expect(this.res.json).to.have.been.calledWith({ + redirect: `/project/${this.project._id}`, + grantAnonymousAccess: 'readAndWrite', + }) + }) + + it('checks the hash prefix and includes log data', function () { + expect( + this.TokenAccessHandler.checkTokenHashPrefix + ).to.have.been.calledWith( + this.token, + 'prefix', + 'readAndWrite', + null, + { + projectId: this.project._id, + action: 'granting read-write anonymous access', + } + ) + }) + }) + }) + + describe('when Overleaf SaaS', function () { + beforeEach(function () { + this.Settings.overleaf = {} + }) + describe('when token is for v1 project', function () { + beforeEach(function (done) { + this.TokenAccessHandler.promises.getProjectByToken.resolves(undefined) + this.TokenAccessHandler.promises.getV1DocInfo.resolves({ + exists: true, + has_owner: true, + }) + this.req.params = { token: this.token } + this.req.body = { tokenHashPrefix: 'prefix' } + this.res.callback = done + this.TokenAccessController.grantTokenAccessReadAndWrite( + this.req, + this.res, + done + ) + }) + it('returns v1 import data', function () { + expect(this.res.json).to.have.been.calledWith({ + v1Import: { + status: 'canDownloadZip', + projectId: this.token, + hasOwner: true, + name: 'Untitled', + brandInfo: undefined, + }, + }) + }) + it('checks the hash prefix and includes log data', function () { + expect( + this.TokenAccessHandler.checkTokenHashPrefix + ).to.have.been.calledWith( + this.token, + 'prefix', + 'readAndWrite', + this.user._id, + { + action: 'import v1', + } + ) + }) + }) + + describe('when token is not for a v1 or v2 project', function () { + beforeEach(function (done) { + this.TokenAccessHandler.promises.getProjectByToken.resolves(undefined) + this.TokenAccessHandler.promises.getV1DocInfo.resolves({ + exists: false, + }) + this.req.params = { token: this.token } + this.req.body = { tokenHashPrefix: 'prefix' } + this.res.callback = done + this.TokenAccessController.grantTokenAccessReadAndWrite( + this.req, + this.res, + done + ) + }) + it('returns 404', function () { + expect(this.res.sendStatus).to.have.been.calledWith(404) + }) + it('checks the hash prefix and includes log data', function () { + expect( + this.TokenAccessHandler.checkTokenHashPrefix + ).to.have.been.calledWith( + this.token, + 'prefix', + 'readAndWrite', + this.user._id, + { + action: '404', + } + ) + }) + }) + }) + + describe('not Overleaf SaaS', function () { + beforeEach(function () { + this.TokenAccessHandler.promises.getProjectByToken.resolves(undefined) + this.req.params = { token: this.token } + this.req.body = { tokenHashPrefix: 'prefix' } + }) + it('passes Errors.NotFoundError to next when project not found and still checks token hash', function (done) { + this.TokenAccessController.grantTokenAccessReadAndWrite( + this.req, + this.res, + args => { + expect(args).to.be.instanceof(this.Errors.NotFoundError) + + expect( + this.TokenAccessHandler.checkTokenHashPrefix + ).to.have.been.calledWith( + this.token, + 'prefix', + 'readAndWrite', + this.user._id, + { + action: '404', + } + ) + + done() + } + ) + }) + }) + + it('passes Errors.NotFoundError to next when token access is not enabled but still checks token hash', function (done) { + this.TokenAccessHandler.tokenAccessEnabledForProject.returns(false) + this.req.params = { token: this.token } + this.req.body = { tokenHashPrefix: 'prefix' } + this.TokenAccessController.grantTokenAccessReadAndWrite( + this.req, + this.res, + args => { + expect(args).to.be.instanceof(this.Errors.NotFoundError) + + expect( + this.TokenAccessHandler.checkTokenHashPrefix + ).to.have.been.calledWith( + this.token, + 'prefix', + 'readAndWrite', + this.user._id, + { + projectId: this.project._id, + action: 'token access not enabled', + } + ) + + done() + } + ) + }) + + it('returns 400 when not using a read write token', function () { + this.TokenAccessHandler.isReadAndWriteToken.returns(false) + this.req.params = { token: this.token } + this.req.body = { tokenHashPrefix: 'prefix' } + this.TokenAccessController.grantTokenAccessReadAndWrite( + this.req, + this.res + ) + expect(this.res.sendStatus).to.have.been.calledWith(400) + }) }) describe('grantTokenAccessReadOnly', function () { @@ -209,7 +482,7 @@ describe('TokenAccessController', function () { 'prefix', 'readOnly', this.user._id, - this.project._id + { projectId: this.project._id, action: 'continue' } ) }) }) @@ -240,9 +513,103 @@ describe('TokenAccessController', function () { undefined, 'readOnly', this.user._id, - this.project._id + { projectId: this.project._id, action: 'continue' } ) }) }) + + it('returns 400 when not using a read only token', function () { + this.TokenAccessHandler.isReadOnlyToken.returns(false) + this.req.params = { token: this.token } + this.req.body = { tokenHashPrefix: 'prefix' } + this.TokenAccessController.grantTokenAccessReadOnly(this.req, this.res) + expect(this.res.sendStatus).to.have.been.calledWith(400) + }) + + describe('anonymous users', function () { + beforeEach(function (done) { + this.req.params = { token: this.token } + this.SessionManager.getLoggedInUserId.returns(null) + this.res.callback = done + + this.TokenAccessController.grantTokenAccessReadOnly( + this.req, + this.res, + done + ) + }) + + it('allows anonymous users and checks the token hash', function () { + expect(this.res.json).to.have.been.calledWith({ + redirect: `/project/${this.project._id}`, + grantAnonymousAccess: 'readOnly', + }) + + expect( + this.TokenAccessHandler.checkTokenHashPrefix + ).to.have.been.calledWith(this.token, undefined, 'readOnly', null, { + projectId: this.project._id, + action: 'granting read-only anonymous access', + }) + }) + }) + + describe('user is owner of project', function () { + beforeEach(function (done) { + this.AuthorizationManager.promises.getPrivilegeLevelForProject.returns( + PrivilegeLevels.OWNER + ) + this.req.params = { token: this.token } + this.req.body = {} + this.res.callback = done + this.TokenAccessController.grantTokenAccessReadOnly( + this.req, + this.res, + done + ) + }) + it('checks token hash and includes log data', function () { + expect( + this.TokenAccessHandler.checkTokenHashPrefix + ).to.have.been.calledWith( + this.token, + undefined, + 'readOnly', + this.user._id, + { + projectId: this.project._id, + action: 'user already has higher or same privilege', + } + ) + }) + }) + + it('passes Errors.NotFoundError to next when token access is not enabled but still checks token hash', function (done) { + this.TokenAccessHandler.tokenAccessEnabledForProject.returns(false) + this.req.params = { token: this.token } + this.req.body = { tokenHashPrefix: 'prefix' } + this.TokenAccessController.grantTokenAccessReadOnly( + this.req, + this.res, + args => { + expect(args).to.be.instanceof(this.Errors.NotFoundError) + + expect( + this.TokenAccessHandler.checkTokenHashPrefix + ).to.have.been.calledWith( + this.token, + 'prefix', + 'readOnly', + this.user._id, + { + projectId: this.project._id, + action: 'token access not enabled', + } + ) + + done() + } + ) + }) }) }) diff --git a/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js b/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js index 5a516751e5..9b0a0a3e99 100644 --- a/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js +++ b/services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js @@ -680,7 +680,7 @@ describe('TokenAccessHandler', function () { `#${prefix}`, 'readOnly', userId, - projectId + { projectId } ) expect(this.Metrics.inc).to.have.been.calledWith( @@ -696,6 +696,7 @@ describe('TokenAccessHandler', function () { hashPrefixStatus: 'mismatch', userId, projectId, + type: 'readOnly', }, 'mismatched token hash prefix' )