diff --git a/services/real-time/app/js/Router.js b/services/real-time/app/js/Router.js index ec6de5136f..1aae3ea64e 100644 --- a/services/real-time/app/js/Router.js +++ b/services/real-time/app/js/Router.js @@ -33,6 +33,7 @@ module.exports = Router = { } attrs.client_id = client.id attrs.err = error + attrs.method = method if (error.name === 'CodedError') { logger.warn(attrs, error.message) const serializedError = { message: error.message, code: error.info.code } diff --git a/services/real-time/app/js/WebApiManager.js b/services/real-time/app/js/WebApiManager.js index 71cba0d526..403de53cfe 100644 --- a/services/real-time/app/js/WebApiManager.js +++ b/services/real-time/app/js/WebApiManager.js @@ -8,6 +8,7 @@ const logger = require('logger-sharelatex') const { CodedError, CorruptedJoinProjectResponseError, + NotAuthorizedError, WebApiRequestFailedError } = require('./Errors') @@ -55,6 +56,10 @@ module.exports = { 'TooManyRequests' ) ) + } else if (response.statusCode === 403) { + callback(new NotAuthorizedError()) + } else if (response.statusCode === 404) { + callback(new CodedError('project not found', 'ProjectNotFound')) } else { callback(new WebApiRequestFailedError(response.statusCode)) } diff --git a/services/real-time/app/js/WebsocketController.js b/services/real-time/app/js/WebsocketController.js index a7b2cd43fa..89066a9661 100644 --- a/services/real-time/app/js/WebsocketController.js +++ b/services/real-time/app/js/WebsocketController.js @@ -356,7 +356,7 @@ module.exports = WebsocketController = { cursorData.doc_id, function (error) { if (error) { - logger.warn( + logger.info( { err: error, client_id: client.id, project_id, user_id }, "silently ignoring unauthorized updateClientPosition. Client likely hasn't called joinProject yet." ) diff --git a/services/real-time/test/acceptance/js/JoinProjectTests.js b/services/real-time/test/acceptance/js/JoinProjectTests.js index 051c33d0c7..508e2b6614 100644 --- a/services/real-time/test/acceptance/js/JoinProjectTests.js +++ b/services/real-time/test/acceptance/js/JoinProjectTests.js @@ -176,6 +176,128 @@ describe('joinProject', function () { }) }) + describe('when not authorized and web replies with a 403', function () { + before(function (done) { + return async.series( + [ + (cb) => { + return FixturesManager.setUpProject( + { + project_id: 'forbidden', + privilegeLevel: 'owner', + project: { + name: 'Test Project' + } + }, + (e, { project_id, user_id }) => { + this.project_id = project_id + this.user_id = user_id + cb(e) + } + ) + }, + + (cb) => { + this.client = RealTimeClient.connect() + this.client.on('connectionAccepted', cb) + }, + + (cb) => { + this.client.emit( + 'joinProject', + { project_id: this.project_id }, + (error, project, privilegeLevel, protocolVersion) => { + this.error = error + this.project = project + this.privilegeLevel = privilegeLevel + this.protocolVersion = protocolVersion + cb() + } + ) + } + ], + done + ) + }) + + it('should return an error', function () { + this.error.message.should.equal('not authorized') + }) + + it('should not have joined the project room', function (done) { + RealTimeClient.getConnectedClient( + this.client.socket.sessionid, + (error, client) => { + expect(Array.from(client.rooms).includes(this.project_id)).to.equal( + false + ) + done() + } + ) + }) + }) + + describe('when deleted and web replies with a 404', function () { + before(function (done) { + return async.series( + [ + (cb) => { + return FixturesManager.setUpProject( + { + project_id: 'not-found', + privilegeLevel: 'owner', + project: { + name: 'Test Project' + } + }, + (e, { project_id, user_id }) => { + this.project_id = project_id + this.user_id = user_id + cb(e) + } + ) + }, + + (cb) => { + this.client = RealTimeClient.connect() + this.client.on('connectionAccepted', cb) + }, + + (cb) => { + this.client.emit( + 'joinProject', + { project_id: this.project_id }, + (error, project, privilegeLevel, protocolVersion) => { + this.error = error + this.project = project + this.privilegeLevel = privilegeLevel + this.protocolVersion = protocolVersion + cb() + } + ) + } + ], + done + ) + }) + + it('should return an error', function () { + this.error.code.should.equal('ProjectNotFound') + }) + + it('should not have joined the project room', function (done) { + RealTimeClient.getConnectedClient( + this.client.socket.sessionid, + (error, client) => { + expect(Array.from(client.rooms).includes(this.project_id)).to.equal( + false + ) + done() + } + ) + }) + }) + return describe('when over rate limit', function () { before(function (done) { return async.series( diff --git a/services/real-time/test/acceptance/js/helpers/MockWebServer.js b/services/real-time/test/acceptance/js/helpers/MockWebServer.js index a2cf5af50b..2de9e61275 100644 --- a/services/real-time/test/acceptance/js/helpers/MockWebServer.js +++ b/services/real-time/test/acceptance/js/helpers/MockWebServer.js @@ -38,6 +38,12 @@ module.exports = MockWebServer = { joinProjectRequest(req, res, next) { const { project_id } = req.params const { user_id } = req.query + if (project_id === 'not-found') { + return res.status(404).send() + } + if (project_id === 'forbidden') { + return res.status(403).send() + } if (project_id === 'rate-limited') { return res.status(429).send() } else { diff --git a/services/real-time/test/unit/js/WebApiManagerTests.js b/services/real-time/test/unit/js/WebApiManagerTests.js index 4435bc14f9..a366a2e75d 100644 --- a/services/real-time/test/unit/js/WebApiManagerTests.js +++ b/services/real-time/test/unit/js/WebApiManagerTests.js @@ -91,6 +91,53 @@ describe('WebApiManager', function () { }) }) + describe('when web replies with a 403', function () { + beforeEach(function () { + this.request.post = sinon + .stub() + .callsArgWith(1, null, { statusCode: 403 }, null) + this.WebApiManager.joinProject( + this.project_id, + this.user_id, + this.callback + ) + }) + + it('should call the callback with an error', function () { + this.callback + .calledWith( + sinon.match({ + message: 'not authorized' + }) + ) + .should.equal(true) + }) + }) + + describe('when web replies with a 404', function () { + beforeEach(function () { + this.request.post = sinon + .stub() + .callsArgWith(1, null, { statusCode: 404 }, null) + this.WebApiManager.joinProject( + this.project_id, + this.user_id, + this.callback + ) + }) + + it('should call the callback with an error', function () { + this.callback + .calledWith( + sinon.match({ + message: 'project not found', + info: { code: 'ProjectNotFound' } + }) + ) + .should.equal(true) + }) + }) + describe('with an error from web', function () { beforeEach(function () { this.request.post = sinon