diff --git a/services/real-time/app/js/AuthorizationManager.js b/services/real-time/app/js/AuthorizationManager.js index 261e2e91ee..b8633ad4d0 100644 --- a/services/real-time/app/js/AuthorizationManager.js +++ b/services/real-time/app/js/AuthorizationManager.js @@ -18,6 +18,14 @@ module.exports = AuthorizationManager = { ) }, + assertClientCanReviewProject(client, callback) { + AuthorizationManager._assertClientHasPrivilegeLevel( + client, + ['readAndWrite', 'owner', 'review'], + callback + ) + }, + _assertClientHasPrivilegeLevel(client, allowedLevels, callback) { if (allowedLevels.includes(client.ol_context.privilege_level)) { callback(null) @@ -44,6 +52,15 @@ module.exports = AuthorizationManager = { }) }, + assertClientCanReviewProjectAndDoc(client, docId, callback) { + AuthorizationManager.assertClientCanReviewProject(client, function (error) { + if (error) { + return callback(error) + } + AuthorizationManager._assertClientCanAccessDoc(client, docId, callback) + }) + }, + _assertClientCanAccessDoc(client, docId, callback) { if (client.ol_context[`doc:${docId}`] === 'allowed') { callback(null) diff --git a/services/real-time/app/js/WebsocketController.js b/services/real-time/app/js/WebsocketController.js index 4888f56097..dec567709a 100644 --- a/services/real-time/app/js/WebsocketController.js +++ b/services/real-time/app/js/WebsocketController.js @@ -622,26 +622,25 @@ module.exports = WebsocketController = { }, _assertClientCanApplyUpdate(client, docId, update, callback) { - AuthorizationManager.assertClientCanEditProjectAndDoc( - client, - docId, - function (error) { - if ( - error && - error.message === 'not authorized' && - WebsocketController._isCommentUpdate(update) - ) { - // This might be a comment op, which we only need read-only priveleges for - AuthorizationManager.assertClientCanViewProjectAndDoc( - client, - docId, - callback - ) - return - } - callback(error) - } - ) + if (WebsocketController._isCommentUpdate(update)) { + return AuthorizationManager.assertClientCanViewProjectAndDoc( + client, + docId, + callback + ) + } else if (update.meta?.tc) { + return AuthorizationManager.assertClientCanReviewProjectAndDoc( + client, + docId, + callback + ) + } else { + return AuthorizationManager.assertClientCanEditProjectAndDoc( + client, + docId, + callback + ) + } }, _isCommentUpdate(update) { diff --git a/services/real-time/test/unit/js/AuthorizationManagerTests.js b/services/real-time/test/unit/js/AuthorizationManagerTests.js index 57882eaacc..a23d814806 100644 --- a/services/real-time/test/unit/js/AuthorizationManagerTests.js +++ b/services/real-time/test/unit/js/AuthorizationManagerTests.js @@ -228,7 +228,7 @@ describe('AuthorizationManager', function () { }) }) - return describe('assertClientCanEditProjectAndDoc', function () { + describe('assertClientCanEditProjectAndDoc', function () { beforeEach(function () { this.doc_id = '12345' this.callback = sinon.stub() @@ -326,4 +326,103 @@ describe('AuthorizationManager', function () { }) }) }) + + return describe('assertClientCanReviewProjectAndDoc', function () { + beforeEach(function () { + this.doc_id = '12345' + this.callback = sinon.stub() + return (this.client.ol_context = {}) + }) + + describe('when not authorised at the project level', function () { + beforeEach(function () { + return (this.client.ol_context.privilege_level = 'readOnly') + }) + + it('should not allow access', function () { + return this.AuthorizationManager.assertClientCanReviewProjectAndDoc( + this.client, + this.doc_id, + err => err.message.should.equal('not authorized') + ) + }) + + return describe('even when authorised at the doc level', function () { + beforeEach(function (done) { + return this.AuthorizationManager.addAccessToDoc( + this.client, + this.doc_id, + done + ) + }) + + return it('should not allow access', function () { + return this.AuthorizationManager.assertClientCanReviewProjectAndDoc( + this.client, + this.doc_id, + err => err.message.should.equal('not authorized') + ) + }) + }) + }) + + return describe('when authorised at the project level', function () { + beforeEach(function () { + return (this.client.ol_context.privilege_level = 'review') + }) + + describe('and not authorised at the document level', function () { + return it('should not allow access', function () { + return this.AuthorizationManager.assertClientCanReviewProjectAndDoc( + this.client, + this.doc_id, + err => err.message.should.equal('not authorized') + ) + }) + }) + + describe('and authorised at the document level', function () { + beforeEach(function (done) { + return this.AuthorizationManager.addAccessToDoc( + this.client, + this.doc_id, + done + ) + }) + + return it('should allow access', function () { + this.AuthorizationManager.assertClientCanReviewProjectAndDoc( + this.client, + this.doc_id, + this.callback + ) + return this.callback.calledWith(null).should.equal(true) + }) + }) + + return describe('when document authorisation is added and then removed', function () { + beforeEach(function (done) { + return this.AuthorizationManager.addAccessToDoc( + this.client, + this.doc_id, + () => { + return this.AuthorizationManager.removeAccessToDoc( + this.client, + this.doc_id, + done + ) + } + ) + }) + + return it('should deny access', function () { + return this.AuthorizationManager.assertClientCanReviewProjectAndDoc( + this.client, + this.doc_id, + err => err.message.should.equal('not authorized') + ) + }) + }) + }) + }) }) diff --git a/services/real-time/test/unit/js/WebsocketControllerTests.js b/services/real-time/test/unit/js/WebsocketControllerTests.js index 532a93d5be..1f3ca67724 100644 --- a/services/real-time/test/unit/js/WebsocketControllerTests.js +++ b/services/real-time/test/unit/js/WebsocketControllerTests.js @@ -1578,6 +1578,8 @@ describe('WebsocketController', function () { } // comments may still be in an edit op this.comment_update = { op: [{ c: 'bar', p: 132 }] } this.AuthorizationManager.assertClientCanEditProjectAndDoc = sinon.stub() + this.AuthorizationManager.assertClientCanReviewProjectAndDoc = + sinon.stub() return (this.AuthorizationManager.assertClientCanViewProjectAndDoc = sinon.stub()) }) @@ -1633,7 +1635,7 @@ describe('WebsocketController', function () { }) }) - return describe('with a totally unauthorized client', function () { + describe('with a totally unauthorized client', function () { return it('should return an error', function (done) { this.AuthorizationManager.assertClientCanEditProjectAndDoc.yields( new Error('not authorized') @@ -1652,5 +1654,45 @@ describe('WebsocketController', function () { ) }) }) + + describe('with a review client', function () { + it('op with tc should succeed', function (done) { + this.AuthorizationManager.assertClientCanEditProjectAndDoc.yields( + new Error('not authorized') + ) + this.AuthorizationManager.assertClientCanViewProjectAndDoc.yields(null) + this.AuthorizationManager.assertClientCanReviewProjectAndDoc.yields( + null + ) + return this.WebsocketController._assertClientCanApplyUpdate( + this.client, + this.doc_id, + { op: [{ p: 10, i: 'a' }], meta: { tc: '123456' } }, + error => { + expect(error).to.be.null + return done() + } + ) + }) + + return it('op without tc should fail', function (done) { + this.AuthorizationManager.assertClientCanEditProjectAndDoc.yields( + new Error('not authorized') + ) + this.AuthorizationManager.assertClientCanViewProjectAndDoc.yields(null) + this.AuthorizationManager.assertClientCanReviewProjectAndDoc.yields( + null + ) + return this.WebsocketController._assertClientCanApplyUpdate( + this.client, + this.doc_id, + { op: [{ p: 10, i: 'a' }] }, + error => { + expect(error.message).to.equal('not authorized') + return done() + } + ) + }) + }) }) })