Преглед изворни кода

track permissions when clients join and leave docs

Brian Gough пре 10 година
родитељ
комит
ef85bce3b8

+ 25 - 1
services/real-time/app/coffee/AuthorizationManager.coffee

@@ -12,4 +12,28 @@ module.exports = AuthorizationManager =
 			if allowed
 				callback null
 			else
-				callback new Error("not authorized")
+				callback new Error("not authorized")
+
+	assertClientCanViewProjectAndDoc: (client, doc_id, callback = (error) ->) ->
+		AuthorizationManager.assertClientCanViewProject client, (error) ->
+			return callback(error) if error?
+			AuthorizationManager._assertClientCanAccessDoc client, doc_id, callback
+
+	assertClientCanEditProjectAndDoc: (client, doc_id, callback = (error) ->) ->
+		AuthorizationManager.assertClientCanEditProject client, (error) ->
+			return callback(error) if error?
+			AuthorizationManager._assertClientCanAccessDoc client, doc_id, callback
+
+	_assertClientCanAccessDoc: (client, doc_id, callback = (error) ->) ->
+		client.get "doc:#{doc_id}", (error, status) ->
+			return callback(error) if error?
+			if status? and status is "allowed"
+				callback null
+			else
+				callback new Error("not authorized")
+
+	addAccessToDoc: (client, doc_id, callback = (error) ->) ->
+		client.set("doc:#{doc_id}", "allowed", callback)
+
+	removeAccessToDoc: (client, doc_id, callback = (error) ->) ->
+		client.del("doc:#{doc_id}", callback)

+ 7 - 5
services/real-time/app/coffee/WebsocketController.coffee

@@ -91,6 +91,7 @@ module.exports = WebsocketController =
 							logger.err {err, project_id, doc_id, fromVersion, line, client_id: client.id}, "error encoding line uri component"
 							return callback(err)
 						escapedLines.push line
+					AuthorizationManager.addAccessToDoc client, doc_id
 					client.join(doc_id)
 					callback null, escapedLines, version, ops
 					logger.log {user_id, project_id, doc_id, fromVersion, client_id: client.id}, "client joined doc"
@@ -99,8 +100,9 @@ module.exports = WebsocketController =
 		metrics.inc "editor.leave-doc"
 		Utils.getClientAttributes client, ["project_id", "user_id"], (error, {project_id, user_id}) ->
 			logger.log {user_id, project_id, doc_id, client_id: client.id}, "client leaving doc"
-		client.leave doc_id
-		callback()
+			client.leave doc_id
+			AuthorizationManager.removeAccessToDoc client, doc_id # may not be needed, could block updates?
+			callback()
 		
 	updateClientPosition: (client, cursorData, callback = (error) ->) ->
 		metrics.inc "editor.update-client-position", 0.1
@@ -110,7 +112,7 @@ module.exports = WebsocketController =
 			return callback(error) if error?
 			logger.log {user_id, project_id, client_id: client.id, cursorData: cursorData}, "updating client position"
 					
-			AuthorizationManager.assertClientCanViewProject client, (error) ->
+			AuthorizationManager.assertClientCanViewProjectAndDoc client, cursorData.doc_id, (error) ->
 				if error?
 					logger.warn {client_id: client.id, project_id, user_id}, "silently ignoring unauthorized updateClientPosition. Client likely hasn't called joinProject yet."
 					return callback()
@@ -133,7 +135,7 @@ module.exports = WebsocketController =
 					cursorData.name = "Anonymous"
 					callback()
 				WebsocketLoadBalancer.emitToRoom(project_id, "clientTracking.clientUpdated", cursorData)
-		
+
 	getConnectedUsers: (client, callback = (error, users) ->) ->
 		metrics.inc "editor.get-connected-users"
 		Utils.getClientAttributes client, ["project_id", "user_id"], (error, {project_id, user_id}) ->
@@ -157,7 +159,7 @@ module.exports = WebsocketController =
 			return callback(new Error("no project_id found on client")) if !project_id?
 			# Omit this logging for now since it's likely too noisey
 			#logger.log {user_id, project_id, doc_id, client_id: client.id, update: update}, "applying update"
-			AuthorizationManager.assertClientCanEditProject client, (error) ->
+			AuthorizationManager.assertClientCanEditProjectAndDoc client, doc_id, (error) ->
 				cbc_1++
 				if error?
 					logger.error {err: error, doc_id, client_id: client.id, version: update.v}, "client is not authorized to make update"

+ 128 - 2
services/real-time/test/unit/coffee/AuthorizationManagerTests.coffee

@@ -10,7 +10,15 @@ describe 'AuthorizationManager', ->
 	beforeEach ->
 		@client =
 			params: {}
-			get: (param, cb) -> cb null, @params[param]
+			get: (param, cb) ->
+				cb null, @params[param]
+			set: (param, value, cb) ->
+				@params[param] = value
+				cb()
+			del: (param, cb) ->
+				delete @params[param]
+				cb()
+
 		@AuthorizationManager = SandboxedModule.require modulePath, requires: {}
 
 	describe "assertClientCanViewProject", ->
@@ -61,4 +69,122 @@ describe 'AuthorizationManager', ->
 			@client.params.privilege_level = "unknown"
 			@AuthorizationManager.assertClientCanEditProject @client, (error) ->
 				error.message.should.equal "not authorized"
-				done()
+				done()
+
+	# check doc access for project
+
+	describe "assertClientCanViewProjectAndDoc", ->
+		beforeEach () ->
+			@doc_id = "12345"
+			@callback = sinon.stub()
+			@client.params = {}
+
+		describe "when not authorised at the project level", ->
+			beforeEach () ->
+				@client.params.privilege_level = "unknown"
+
+			it "should not allow access", () ->
+				@AuthorizationManager.assertClientCanViewProjectAndDoc @client, @doc_id, @callback
+				@callback
+					.calledWith(new Error("not authorised"))
+					.should.equal true
+
+			describe "even when authorised at the doc level", ->
+				beforeEach (done) ->
+					@AuthorizationManager.addAccessToDoc @client, @doc_id, done
+
+				it "should not allow access", () ->
+					@AuthorizationManager.assertClientCanViewProjectAndDoc @client, @doc_id, @callback
+					@callback
+						.calledWith(new Error("not authorised"))
+						.should.equal true
+
+		describe "when authorised at the project level", ->
+			beforeEach () ->
+				@client.params.privilege_level = "readOnly"
+
+			describe "and not authorised at the document level", ->
+				it "should not allow access", () ->
+					@AuthorizationManager.assertClientCanViewProjectAndDoc @client, @doc_id, @callback
+					@callback
+						.calledWith(new Error("not authorised"))
+						.should.equal true
+
+			describe "and authorised at the document level", ->
+				beforeEach (done) ->
+					@AuthorizationManager.addAccessToDoc @client, @doc_id, done
+
+				it "should allow access", () ->
+					@AuthorizationManager.assertClientCanViewProjectAndDoc @client, @doc_id, @callback
+					@callback
+						.calledWith(null)
+						.should.equal true
+
+			describe "when document authorisation is added and then removed", ->
+				beforeEach (done) ->
+					@AuthorizationManager.addAccessToDoc @client, @doc_id, () =>
+						@AuthorizationManager.removeAccessToDoc @client, @doc_id, done
+
+				it "should deny access", () ->
+					@AuthorizationManager.assertClientCanViewProjectAndDoc @client, @doc_id, @callback
+					@callback
+						.calledWith(new Error("not authorised"))
+						.should.equal true
+
+	describe "assertClientCanEditProjectAndDoc", ->
+		beforeEach () ->
+			@doc_id = "12345"
+			@callback = sinon.stub()
+			@client.params = {}
+
+		describe "when not authorised at the project level", ->
+			beforeEach () ->
+				@client.params.privilege_level = "readOnly"
+
+			it "should not allow access", () ->
+				@AuthorizationManager.assertClientCanEditProjectAndDoc @client, @doc_id, @callback
+				@callback
+					.calledWith(new Error("not authorised"))
+					.should.equal true
+
+			describe "even when authorised at the doc level", ->
+				beforeEach (done) ->
+					@AuthorizationManager.addAccessToDoc @client, @doc_id, done
+
+				it "should not allow access", () ->
+					@AuthorizationManager.assertClientCanEditProjectAndDoc @client, @doc_id, @callback
+					@callback
+						.calledWith(new Error("not authorised"))
+						.should.equal true
+
+		describe "when authorised at the project level", ->
+			beforeEach () ->
+				@client.params.privilege_level = "readAndWrite"
+
+			describe "and not authorised at the document level", ->
+				it "should not allow access", () ->
+					@AuthorizationManager.assertClientCanEditProjectAndDoc @client, @doc_id, @callback
+					@callback
+						.calledWith(new Error("not authorised"))
+						.should.equal true
+
+			describe "and authorised at the document level", ->
+				beforeEach (done) ->
+					@AuthorizationManager.addAccessToDoc @client, @doc_id, done
+
+				it "should allow access", () ->
+					@AuthorizationManager.assertClientCanEditProjectAndDoc @client, @doc_id, @callback
+					@callback
+						.calledWith(null)
+						.should.equal true
+
+			describe "when document authorisation is added and then removed", ->
+				beforeEach (done) ->
+					@AuthorizationManager.addAccessToDoc @client, @doc_id, () =>
+						@AuthorizationManager.removeAccessToDoc @client, @doc_id, done
+
+				it "should deny access", () ->
+					@AuthorizationManager.assertClientCanEditProjectAndDoc @client, @doc_id, @callback
+					@callback
+						.calledWith(new Error("not authorised"))
+						.should.equal true

+ 9 - 5
services/real-time/test/unit/coffee/WebsocketControllerTests.coffee

@@ -172,7 +172,7 @@ describe 'WebsocketController', ->
 			@ops = ["mock", "ops"]
 			
 			@client.params.project_id = @project_id
-			
+			@AuthorizationManager.addAccessToDoc = sinon.stub()
 			@AuthorizationManager.assertClientCanViewProject = sinon.stub().callsArgWith(1, null)
 			@DocumentUpdaterManager.getDocument = sinon.stub().callsArgWith(3, null, @doc_lines, @version, @ops)
 			
@@ -190,6 +190,11 @@ describe 'WebsocketController', ->
 				@DocumentUpdaterManager.getDocument
 					.calledWith(@project_id, @doc_id, @fromVersion)
 					.should.equal true
+
+			it "should add permissions for the client to access the doc", ->
+				@AuthorizationManager.addAccessToDoc
+					.calledWith(@client, @doc_id)
+					.should.equal true
 					
 			it "should join the client to room for the doc_id", ->
 				@client.join
@@ -287,7 +292,7 @@ describe 'WebsocketController', ->
 		beforeEach ->
 			@WebsocketLoadBalancer.emitToRoom = sinon.stub()
 			@ConnectedUsersManager.updateUserPosition = sinon.stub().callsArgWith(4)
-			@AuthorizationManager.assertClientCanViewProject = sinon.stub().callsArgWith(1, null)
+			@AuthorizationManager.assertClientCanViewProjectAndDoc = sinon.stub().callsArgWith(2, null)
 			@update = {
 				doc_id: @doc_id = "doc-id-123"
 				row: @row = 42
@@ -362,7 +367,7 @@ describe 'WebsocketController', ->
 			@update = {op: {p: 12, t: "foo"}}
 			@client.params.user_id = @user_id
 			@client.params.project_id = @project_id
-			@AuthorizationManager.assertClientCanEditProject = sinon.stub().callsArg(1)
+			@AuthorizationManager.assertClientCanEditProjectAndDoc = sinon.stub().callsArg(2)
 			@DocumentUpdaterManager.queueChange = sinon.stub().callsArg(3)
 
 		describe "succesfully", ->
@@ -410,7 +415,7 @@ describe 'WebsocketController', ->
 		describe "when not authorized", ->
 			beforeEach ->
 				@client.disconnect = sinon.stub()
-				@AuthorizationManager.assertClientCanEditProject = sinon.stub().callsArgWith(1, @error = new Error("not authorized"))
+				@AuthorizationManager.assertClientCanEditProjectAndDoc = sinon.stub().callsArgWith(2, @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.
@@ -423,4 +428,3 @@ describe 'WebsocketController', ->
 
 			it "should call the callback with the error", ->
 				@callback.calledWith(@error).should.equal true
-