Revert "Revert "Track the isRestrictedUser flag on clients""

This reverts commit 651e392a7c644403f199e1b03e7494b61ce71d0c.

services/real-time/app/coffee/WebApiManager.coffee

@@ -4,7 +4,7 @@ logger = require "logger-sharelatex"
 { CodedError } = require "./Errors"
 
 module.exports = WebApiManager =
-	joinProject: (project_id, user, callback = (error, project, privilegeLevel) ->) ->
+	joinProject: (project_id, user, callback = (error, project, privilegeLevel, isRestrictedUser) ->) ->
 		user_id = user._id
 		logger.log {project_id, user_id}, "sending join project request to web"
 		url = "#{settings.apis.web.url}/project/#{project_id}/join"
@@ -24,7 +24,11 @@ module.exports = WebApiManager =
 		}, (error, response, data) ->
 			return callback(error) if error?
 			if 200 <= response.statusCode < 300
-				callback null, data?.project, data?.privilegeLevel
+				if !data? || !data?.project?
+					err = new Error('no data returned from joinProject request')
+					logger.error {err, project_id, user_id}, "error accessing web api"
+					return callback(err)
+				callback null, data.project, data.privilegeLevel, data.isRestrictedUser
 			else if response.statusCode == 429
 				logger.log(project_id, user_id, "rate-limit hit when joining project")
 				callback(new CodedError("rate-limit hit when joining project", "TooManyRequests"))

services/real-time/app/coffee/WebsocketController.coffee

@@ -18,7 +18,7 @@ module.exports = WebsocketController =
 		user_id = user?._id
 		logger.log {user_id, project_id, client_id: client.id}, "user joining project"
 		metrics.inc "editor.join-project"
-		WebApiManager.joinProject project_id, user, (error, project, privilegeLevel) ->
+		WebApiManager.joinProject project_id, user, (error, project, privilegeLevel, isRestrictedUser) ->
 			return callback(error) if error?
 
 			if !privilegeLevel or privilegeLevel == ""
@@ -26,7 +26,6 @@ module.exports = WebsocketController =
 				logger.warn {err, project_id, user_id, client_id: client.id}, "user is not authorized to join project"
 				return callback(err)
 
-
 			client.set("privilege_level", privilegeLevel)
 			client.set("user_id", user_id)
 			client.set("project_id", project_id)
@@ -37,6 +36,7 @@ module.exports = WebsocketController =
 			client.set("connected_time", new Date())
 			client.set("signup_date", user?.signUpDate)
 			client.set("login_count", user?.loginCount)
+			client.set("is_restricted_user", !!(isRestrictedUser))
 
 			RoomManager.joinProject client, project_id, (err) ->
 				logger.log {user_id, project_id, client_id: client.id}, "user joined project"
@@ -178,8 +178,11 @@ module.exports = WebsocketController =
 	CLIENT_REFRESH_DELAY: 1000
 	getConnectedUsers: (client, callback = (error, users) ->) ->
 		metrics.inc "editor.get-connected-users"
-		Utils.getClientAttributes client, ["project_id", "user_id"], (error, {project_id, user_id}) ->
+		Utils.getClientAttributes client, ["project_id", "user_id", "is_restricted_user"], (error, clientAttributes) ->
 			return callback(error) if error?
+			{project_id, user_id, is_restricted_user} = clientAttributes
+			if is_restricted_user
+				return callback(null, [])
 			return callback(new Error("no project_id found on client")) if !project_id?
 			logger.log {user_id, project_id, client_id: client.id}, "getting connected users"
 			AuthorizationManager.assertClientCanViewProject client, (error) ->

services/real-time/app/coffee/WebsocketLoadBalancer.coffee

@@ -7,6 +7,19 @@ HealthCheckManager = require "./HealthCheckManager"
 RoomManager = require "./RoomManager"
 ChannelManager = require "./ChannelManager"
 ConnectedUsersManager = require "./ConnectedUsersManager"
+Utils = require './Utils'
+Async = require 'async'
+
+RESTRICTED_USER_MESSAGE_TYPE_PASS_LIST = [
+	'connectionAccepted',
+	'otUpdateApplied',
+	'otUpdateError',
+	'joinDoc',
+	'reciveNewDoc',
+	'reciveNewFile',
+	'reciveNewFolder',
+	'removeEntity'
+]
 
 module.exports = WebsocketLoadBalancer =
 	rclientPubList: RedisClientManager.createClientList(Settings.redis.pubsub)
@@ -69,12 +82,28 @@ module.exports = WebsocketLoadBalancer =
 				clientList = io.sockets.clients(message.room_id)
 				# avoid unnecessary work if no clients are connected
 				return if clientList.length is 0
-				logger.log {channel:channel, message: message.message, room_id: message.room_id, message_id: message._id, socketIoClients: (client.id for client in clientList)}, "distributing event to clients"
+				logger.log {
+					channel: channel,
+					message: message.message,
+					room_id: message.room_id,
+					message_id: message._id,
+					socketIoClients: (client.id for client in clientList)
+				}, "distributing event to clients"
 				seen = {}
-				for client in clientList when not seen[client.id]
+				# Send the messages to clients async, don't wait for them all to finish
+				Async.eachLimit clientList
+					, 2
+					, (client, cb) ->
+						Utils.getClientAttributes client, ['is_restricted_user'], (err, {is_restricted_user}) ->
+							return cb(err) if err?
+							if !seen[client.id]
 								seen[client.id] = true
+								if !(is_restricted_user && message.message not in RESTRICTED_USER_MESSAGE_TYPE_PASS_LIST)
 									client.emit(message.message, message.payload...)
+							cb()
+					, (err) ->
+						if err?
+							logger.err {err, message}, "Error sending message to clients"
 			else if message.health_check?
 				logger.debug {message}, "got health check message in editor events channel"
 				HealthCheckManager.check channel, message.key
-		

services/real-time/test/unit/coffee/WebApiManagerTests.coffee

@@ -26,7 +26,8 @@ describe 'WebApiManager', ->
 			beforeEach ->
 				@response = {
 					project: { name: "Test project" }
-					privilegeLevel: "owner"
+					privilegeLevel: "owner",
+					isRestrictedUser: true
 				}
 				@request.post = sinon.stub().callsArgWith(1, null, {statusCode: 200}, @response)
 				@WebApiManager.joinProject @project_id, @user, @callback
@@ -47,9 +48,9 @@ describe 'WebApiManager', ->
 					})
 					.should.equal true
 
-			it "should return the project and privilegeLevel", ->
+			it "should return the project, privilegeLevel, and restricted flag", ->
 				@callback
-					.calledWith(null, @response.project, @response.privilegeLevel)
+					.calledWith(null, @response.project, @response.privilegeLevel, @response.isRestrictedUser)
 					.should.equal true
 
 		describe "with an error from web", ->
@@ -62,6 +63,16 @@ describe 'WebApiManager', ->
 					.calledWith(new Error("non-success code from web: 500"))
 					.should.equal true
 
+		describe "with no data from web", ->
+			beforeEach ->
+				@request.post = sinon.stub().callsArgWith(1, null, {statusCode: 200}, null)
+				@WebApiManager.joinProject @project_id, @user_id, @callback
+
+			it "should call the callback with an error", ->
+				@callback
+					.calledWith(new Error("no data returned from joinProject request"))
+					.should.equal true
+
 		describe "when the project is over its rate limit", ->
 			beforeEach ->
 				@request.post = sinon.stub().callsArgWith(1, null, {statusCode: 429}, null)

services/real-time/test/unit/coffee/WebsocketControllerTests.coffee

@@ -53,7 +53,8 @@ describe 'WebsocketController', ->
 				}
 				@privilegeLevel = "owner"
 				@ConnectedUsersManager.updateUserPosition = sinon.stub().callsArg(4)
-				@WebApiManager.joinProject = sinon.stub().callsArgWith(2, null, @project, @privilegeLevel)
+				@isRestrictedUser = true
+				@WebApiManager.joinProject = sinon.stub().callsArgWith(2, null, @project, @privilegeLevel, @isRestrictedUser)
 				@RoomManager.joinProject = sinon.stub().callsArg(2)
 				@WebsocketController.joinProject @client, @user, @project_id, @callback
 
@@ -95,6 +96,9 @@ describe 'WebsocketController', ->
 			it "should set the project owner id on the client", ->
 				@client.set.calledWith("owner_id", @owner_id).should.equal true
 
+			it "should set the is_restricted_user flag on the client", ->
+				@client.set.calledWith("is_restricted_user", @isRestrictedUser).should.equal true
+
 			it "should call the callback with the project, privilegeLevel and protocolVersion", ->
 				@callback
 					.calledWith(null, @project, @privilegeLevel, @WebsocketController.PROTOCOL_VERSION)
@@ -399,6 +403,20 @@ describe 'WebsocketController', ->
 			it "should return an error", ->
 				@callback.calledWith(@err).should.equal true
 
+		describe "when restricted user", ->
+			beforeEach ->
+				@client.params.is_restricted_user = true
+				@AuthorizationManager.assertClientCanViewProject = sinon.stub().callsArgWith(1, null)
+				@WebsocketController.getConnectedUsers @client, @callback
+
+			it "should return an empty array of users", ->
+				@callback.calledWith(null, []).should.equal true
+
+			it "should not get the connected users for the project", ->
+				@ConnectedUsersManager.getConnectedUsers
+					.called
+					.should.equal false
+
 	describe "updateClientPosition", ->
 		beforeEach ->
 			@WebsocketLoadBalancer.emitToRoom = sinon.stub()

services/real-time/test/unit/coffee/WebsocketLoadBalancerTests.coffee

@@ -18,6 +18,12 @@ describe "WebsocketLoadBalancer", ->
 			"./RoomManager" : @RoomManager = {eventSource: sinon.stub().returns @RoomEvents}
 			"./ChannelManager": @ChannelManager = {publish: sinon.stub()}
 			"./ConnectedUsersManager": @ConnectedUsersManager = {refreshClient: sinon.stub()}
+			"./Utils": @Utils = {
+				getClientAttributes: sinon.spy(
+					(client, _attrs, callback) ->
+						callback(null, {is_restricted_user: !!client.__isRestricted})
+				)
+			}
 		@io = {}
 		@WebsocketLoadBalancer.rclientPubList = [{publish: sinon.stub()}]
 		@WebsocketLoadBalancer.rclientSubList = [{
@@ -26,7 +32,7 @@ describe "WebsocketLoadBalancer", ->
 		}]
 
 		@room_id = "room-id"
-		@message = "message-to-editor"
+		@message = "otUpdateApplied"
 		@payload = ["argument one", 42]
 
 	describe "emitToRoom", ->
@@ -70,6 +76,7 @@ describe "WebsocketLoadBalancer", ->
 	describe "_processEditorEvent", ->
 		describe "with bad JSON", ->
 			beforeEach ->
+				@isRestrictedUser = false
 				@SafeJsonParse.parse = sinon.stub().callsArgWith 1, new Error("oops")
 				@WebsocketLoadBalancer._processEditorEvent(@io, "editor-events", "blah")
 
@@ -98,6 +105,54 @@ describe "WebsocketLoadBalancer", ->
 				@emit2.calledWith(@message, @payload...).should.equal true
 				@emit3.called.should.equal false # duplicate client should be ignored
 
+		describe "with a designated room, and restricted clients, not restricted message", ->
+			beforeEach ->
+				@io.sockets =
+					clients: sinon.stub().returns([
+						{id: 'client-id-1', emit: @emit1 = sinon.stub()}
+						{id: 'client-id-2', emit: @emit2 = sinon.stub()}
+						{id: 'client-id-1', emit: @emit3 = sinon.stub()} # duplicate client
+						{id: 'client-id-4', emit: @emit4 = sinon.stub(), __isRestricted: true}
+					])
+				data = JSON.stringify
+					room_id: @room_id
+					message: @message
+					payload: @payload
+				@WebsocketLoadBalancer._processEditorEvent(@io, "editor-events", data)
+
+			it "should send the message to all (unique) clients in the room", ->
+				@io.sockets.clients
+					.calledWith(@room_id)
+					.should.equal true
+				@emit1.calledWith(@message, @payload...).should.equal true
+				@emit2.calledWith(@message, @payload...).should.equal true
+				@emit3.called.should.equal false # duplicate client should be ignored
+				@emit4.called.should.equal true  # restricted client, but should be called
+
+		describe "with a designated room, and restricted clients, restricted message", ->
+			beforeEach ->
+				@io.sockets =
+					clients: sinon.stub().returns([
+						{id: 'client-id-1', emit: @emit1 = sinon.stub()}
+						{id: 'client-id-2', emit: @emit2 = sinon.stub()}
+						{id: 'client-id-1', emit: @emit3 = sinon.stub()} # duplicate client
+						{id: 'client-id-4', emit: @emit4 = sinon.stub(), __isRestricted: true}
+					])
+				data = JSON.stringify
+					room_id: @room_id
+					message: @restrictedMessage = 'new-comment'
+					payload: @payload
+				@WebsocketLoadBalancer._processEditorEvent(@io, "editor-events", data)
+
+			it "should send the message to all (unique) clients in the room, who are not restricted", ->
+				@io.sockets.clients
+					.calledWith(@room_id)
+					.should.equal true
+				@emit1.calledWith(@restrictedMessage, @payload...).should.equal true
+				@emit2.calledWith(@restrictedMessage, @payload...).should.equal true
+				@emit3.called.should.equal false # duplicate client should be ignored
+				@emit4.called.should.equal false # restricted client, should not be called
+
 		describe "when emitting to all", ->
 			beforeEach ->
 				@io.sockets =
@@ -110,4 +165,3 @@ describe "WebsocketLoadBalancer", ->
 
 			it "should send the message to all clients", ->
 				@emit.calledWith(@message, @payload...).should.equal true
-