diff --git a/services/real-time/app/coffee/WebsocketController.coffee b/services/real-time/app/coffee/WebsocketController.coffee index 37fe0ed511..bf893af78a 100644 --- a/services/real-time/app/coffee/WebsocketController.coffee +++ b/services/real-time/app/coffee/WebsocketController.coffee @@ -157,7 +157,7 @@ module.exports = WebsocketController = Utils.getClientAttributes client, ["user_id", "project_id"], (error, {user_id, project_id}) -> return callback(error) if error? return callback(new Error("no project_id found on client")) if !project_id? - AuthorizationManager.assertClientCanEditProjectAndDoc client, doc_id, (error) -> + WebsocketController._assertClientCanApplyUpdate client, doc_id, update, (error) -> if error? logger.error {err: error, doc_id, client_id: client.id, version: update.v}, "client is not authorized to make update" setTimeout () -> @@ -179,3 +179,20 @@ module.exports = WebsocketController = logger.error {err: error, project_id, doc_id, client_id: client.id, version: update.v}, "document was not available for update" client.disconnect() callback(error) + + _assertClientCanApplyUpdate: (client, doc_id, update, callback) -> + AuthorizationManager.assertClientCanEditProjectAndDoc client, doc_id, (error) -> + if error? + if error.message == "not authorized" and WebsocketController._isCommentUpdate(update) + # This might be a comment op, which we only need read-only priveleges for + AuthorizationManager.assertClientCanViewProjectAndDoc client, doc_id, callback + else + return callback(error) + else + return callback(null) + + _isCommentUpdate: (update) -> + for op in update.op + if !op.c? + return false + return true \ No newline at end of file diff --git a/services/real-time/test/acceptance/coffee/ApplyUpdateTests.coffee b/services/real-time/test/acceptance/coffee/ApplyUpdateTests.coffee index 31d8dc2bc0..bb2df04ae2 100644 --- a/services/real-time/test/acceptance/coffee/ApplyUpdateTests.coffee +++ b/services/real-time/test/acceptance/coffee/ApplyUpdateTests.coffee @@ -69,7 +69,7 @@ describe "applyOtUpdate", -> (cb) => rclient.del "PendingUpdates:#{@doc_id}", cb ], done - describe "when not authorized", -> + describe "when authorized to read-only with an edit update", -> before (done) -> async.series [ (cb) => @@ -109,4 +109,61 @@ describe "applyOtUpdate", -> it "should not put the update in redis", (done) -> rclient.llen "PendingUpdates:#{@doc_id}", (error, len) => len.should.equal 0 - done() \ No newline at end of file + done() + + describe "when authorized to read-only with a comment update", -> + before (done) -> + @comment_update = { + op: [{c: "foo", p: 42}] + } + async.series [ + (cb) => + FixturesManager.setUpProject { + privilegeLevel: "readOnly" + }, (e, {@project_id, @user_id}) => + cb(e) + + (cb) => + FixturesManager.setUpDoc @project_id, {@lines, @version, @ops}, (e, {@doc_id}) => + cb(e) + + (cb) => + @client = RealTimeClient.connect() + @client.on "connectionAccepted", cb + + (cb) => + @client.emit "joinProject", project_id: @project_id, cb + + (cb) => + @client.emit "joinDoc", @doc_id, cb + + (cb) => + @client.emit "applyOtUpdate", @doc_id, @comment_update, cb + ], done + + it "should push the doc into the pending updates list", (done) -> + rclient.lrange "pending-updates-list", 0, -1, (error, [doc_id]) => + doc_id.should.equal "#{@project_id}:#{@doc_id}" + done() + + it "should add the doc to the pending updates set in redis", (done) -> + rclient.sismember "DocsWithPendingUpdates", "#{@project_id}:#{@doc_id}", (error, isMember) => + isMember.should.equal 1 + done() + + it "should push the update into redis", (done) -> + rclient.lrange "PendingUpdates:#{@doc_id}", 0, -1, (error, [update]) => + update = JSON.parse(update) + update.op.should.deep.equal @comment_update.op + update.meta.should.deep.equal { + source: @client.socket.sessionid + user_id: @user_id + } + done() + + after (done) -> + async.series [ + (cb) => rclient.del "pending-updates-list", cb + (cb) => rclient.del "DocsWithPendingUpdates", "#{@project_id}:#{@doc_id}", cb + (cb) => rclient.del "PendingUpdates:#{@doc_id}", cb + ], done \ No newline at end of file diff --git a/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee b/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee index 7777327282..534a999ee8 100644 --- a/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee +++ b/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee @@ -1,6 +1,7 @@ chai = require('chai') should = chai.should() sinon = require("sinon") +expect = chai.expect modulePath = "../../../app/js/WebsocketController.js" SandboxedModule = require('sandboxed-module') tk = require "timekeeper" @@ -368,7 +369,7 @@ describe 'WebsocketController', -> @update = {op: {p: 12, t: "foo"}} @client.params.user_id = @user_id @client.params.project_id = @project_id - @AuthorizationManager.assertClientCanEditProjectAndDoc = sinon.stub().callsArg(2) + @WebsocketController._assertClientCanApplyUpdate = sinon.stub().yields() @DocumentUpdaterManager.queueChange = sinon.stub().callsArg(3) describe "succesfully", -> @@ -416,7 +417,7 @@ describe 'WebsocketController', -> describe "when not authorized", -> beforeEach -> @client.disconnect = sinon.stub() - @AuthorizationManager.assertClientCanEditProjectAndDoc = sinon.stub().callsArgWith(2, @error = new Error("not authorized")) + @WebsocketController._assertClientCanApplyUpdate = sinon.stub().yields(@error = new Error("not authorized")) @WebsocketController.applyOtUpdate @client, @doc_id, @update, @callback # This happens in a setTimeout to allow the client a chance to receive the error first. @@ -429,3 +430,41 @@ describe 'WebsocketController', -> it "should call the callback with the error", -> @callback.calledWith(@error).should.equal true + + describe "_assertClientCanApplyUpdate", -> + beforeEach -> + @edit_update = { op: [{i: "foo", p: 42}, {c: "bar", p: 132}] } # comments may still be in an edit op + @comment_update = { op: [{c: "bar", p: 132}] } + @AuthorizationManager.assertClientCanEditProjectAndDoc = sinon.stub() + @AuthorizationManager.assertClientCanViewProjectAndDoc = sinon.stub() + + describe "with a read-write client", -> + it "should return successfully", (done) -> + @AuthorizationManager.assertClientCanEditProjectAndDoc.yields(null) + @WebsocketController._assertClientCanApplyUpdate @client, @doc_id, @edit_update, (error) -> + expect(error).to.be.null + done() + + describe "with a read-only client and an edit op", -> + it "should return an error", (done) -> + @AuthorizationManager.assertClientCanEditProjectAndDoc.yields(new Error("not authorized")) + @AuthorizationManager.assertClientCanViewProjectAndDoc.yields(null) + @WebsocketController._assertClientCanApplyUpdate @client, @doc_id, @edit_update, (error) -> + expect(error.message).to.equal "not authorized" + done() + + describe "with a read-only client and a comment op", -> + it "should return successfully", (done) -> + @AuthorizationManager.assertClientCanEditProjectAndDoc.yields(new Error("not authorized")) + @AuthorizationManager.assertClientCanViewProjectAndDoc.yields(null) + @WebsocketController._assertClientCanApplyUpdate @client, @doc_id, @comment_update, (error) -> + expect(error).to.be.null + done() + + describe "with a totally unauthorized client", -> + it "should return an error", (done) -> + @AuthorizationManager.assertClientCanEditProjectAndDoc.yields(new Error("not authorized")) + @AuthorizationManager.assertClientCanViewProjectAndDoc.yields(new Error("not authorized")) + @WebsocketController._assertClientCanApplyUpdate @client, @doc_id, @comment_update, (error) -> + expect(error.message).to.equal "not authorized" + done()