Parcourir la source

[misc] socket.io: use a secondary publicId for public facing usages

Jakob Ackermann il y a 6 ans
Parent
commit
c6d08647c7

+ 1 - 1
services/real-time/app/coffee/DocumentUpdaterController.coffee

@@ -69,7 +69,7 @@ module.exports = DocumentUpdaterController =
 		# send messages only to unique clients (due to duplicate entries in io.sockets.clients)
 		for client in clientList when not seen[client.id]
 			seen[client.id] = true
-			if client.id == update.meta.source
+			if client.publicId == update.meta.source
 				logger.log doc_id: doc_id, version: update.v, source: update.meta?.source, "distributing update to sender"
 				client.emit "otUpdateApplied", v: update.v, doc: update.doc
 			else if !update.dup # Duplicate ops should just be sent back to sending client for acknowledgement

+ 3 - 1
services/real-time/app/coffee/Router.coffee

@@ -6,6 +6,7 @@ HttpController = require "./HttpController"
 HttpApiController = require "./HttpApiController"
 Utils = require "./Utils"
 bodyParser = require "body-parser"
+base64id = require("base64id")
 
 basicAuth = require('basic-auth-connect')
 httpAuth = basicAuth (user, pass)->
@@ -67,7 +68,8 @@ module.exports = Router =
 				return
 
 			# send positive confirmation that the client has a valid connection
-			client.emit("connectionAccepted")
+			client.publicId = base64id.generateId()
+			client.emit("connectionAccepted", null, client.publicId)
 
 			metrics.inc('socket-io.connection')
 			metrics.gauge('socket-io.clients', io.sockets.clients()?.length)

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

@@ -45,7 +45,7 @@ module.exports = WebsocketController =
 				callback null, project, privilegeLevel, WebsocketController.PROTOCOL_VERSION
 
 			# No need to block for setting the user as connected in the cursor tracking
-			ConnectedUsersManager.updateUserPosition project_id, client.id, user, null, () ->
+			ConnectedUsersManager.updateUserPosition project_id, client.publicId, user, null, () ->
 
 	# We want to flush a project if there are no more (local) connected clients
 	# but we need to wait for the triggering client to disconnect. How long we wait
@@ -58,10 +58,10 @@ module.exports = WebsocketController =
 
 			metrics.inc "editor.leave-project"
 			logger.log {project_id, user_id, client_id: client.id}, "client leaving project"
-			WebsocketLoadBalancer.emitToRoom project_id, "clientTracking.clientDisconnected", client.id
+			WebsocketLoadBalancer.emitToRoom project_id, "clientTracking.clientDisconnected", client.publicId
 
 			# We can do this in the background
-			ConnectedUsersManager.markUserAsDisconnected project_id, client.id, (err) ->
+			ConnectedUsersManager.markUserAsDisconnected project_id, client.publicId, (err) ->
 				if err?
 					logger.error {err, project_id, user_id, client_id: client.id}, "error marking client as disconnected"
 
@@ -143,7 +143,7 @@ module.exports = WebsocketController =
 				if error?
 					logger.warn {err: error, client_id: client.id, project_id, user_id}, "silently ignoring unauthorized updateClientPosition. Client likely hasn't called joinProject yet."
 					return callback()
-				cursorData.id      = client.id
+				cursorData.id      = client.publicId
 				cursorData.user_id = user_id if user_id?
 				cursorData.email   = email   if email?
 				# Don't store anonymous users in redis to avoid influx
@@ -159,7 +159,7 @@ module.exports = WebsocketController =
 						last_name
 					else
 						""
-					ConnectedUsersManager.updateUserPosition(project_id, client.id, {
+					ConnectedUsersManager.updateUserPosition(project_id, client.publicId, {
 						first_name: first_name,
 						last_name:  last_name,
 						email:      email,
@@ -205,7 +205,7 @@ module.exports = WebsocketController =
 					, 100
 					return callback(error)
 				update.meta ||= {}
-				update.meta.source = client.id
+				update.meta.source = client.publicId
 				update.meta.user_id = user_id
 				metrics.inc "editor.doc-update", 0.3
 

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

@@ -72,7 +72,7 @@ module.exports = WebsocketLoadBalancer =
 				clientList = io.sockets.clients(message.room_id)
 				logger.log {channel:channel, message: message.message, room_id: message.room_id, message_id: message._id, socketIoClients: (client.id for client in clientList)}, "refreshing client list"
 				for client in clientList
-					ConnectedUsersManager.refreshClient(message.room_id, client.id)
+					ConnectedUsersManager.refreshClient(message.room_id, client.publicId)
 			else if message.room_id?
 				if message._id? && Settings.checkEventOrder
 					status = EventLogger.checkEventOrder("editor-events", message._id, message)

+ 1 - 0
services/real-time/package.json

@@ -22,6 +22,7 @@
   },
   "dependencies": {
     "async": "^0.9.0",
+    "base64id": "0.1.0",
     "basic-auth-connect": "^1.0.0",
     "body-parser": "^1.12.0",
     "connect-redis": "^2.1.0",

+ 3 - 3
services/real-time/test/acceptance/coffee/ApplyUpdateTests.coffee

@@ -55,7 +55,7 @@ describe "applyOtUpdate", ->
 				update = JSON.parse(update)
 				update.op.should.deep.equal @update.op
 				update.meta.should.deep.equal {
-					source: @client.socket.sessionid
+					source: @client.publicId
 					user_id: @user_id
 				}
 				done()
@@ -208,7 +208,7 @@ describe "applyOtUpdate", ->
 				update = JSON.parse(update)
 				update.op.should.deep.equal @comment_update.op
 				update.meta.should.deep.equal {
-					source: @client.socket.sessionid
+					source: @client.publicId
 					user_id: @user_id
 				}
 				done()
@@ -219,4 +219,4 @@ describe "applyOtUpdate", ->
 				(cb) => rclient.del "pending-updates-list", cb
 				(cb) => rclient.del "DocsWithPendingUpdates", "#{@project_id}:#{@doc_id}", cb
 				(cb) => rclient.del redisSettings.documentupdater.key_schema.pendingUpdates({@doc_id}), cb
-			], done
+			], done

+ 3 - 3
services/real-time/test/acceptance/coffee/ClientTrackingTests.coffee

@@ -63,7 +63,7 @@ describe "clientTracking", ->
 					row: @row
 					column: @column
 					doc_id: @doc_id
-					id: @clientA.socket.sessionid
+					id: @clientA.publicId
 					user_id: @user_id
 					name: "Joe Bloggs"
 				}
@@ -72,7 +72,7 @@ describe "clientTracking", ->
 		it "should record the update in getConnectedUsers", (done) ->
 			@clientB.emit "clientTracking.getConnectedUsers", (error, users) =>
 				for user in users
-					if user.client_id == @clientA.socket.sessionid
+					if user.client_id == @clientA.publicId
 						expect(user.cursorData).to.deep.equal({
 							row: @row
 							column: @column
@@ -139,7 +139,7 @@ describe "clientTracking", ->
 					row: @row
 					column: @column
 					doc_id: @doc_id
-					id: @anonymous.socket.sessionid
+					id: @anonymous.publicId
 					user_id: "anonymous-user"
 					name: ""
 				}

+ 1 - 1
services/real-time/test/acceptance/coffee/JoinProjectTests.coffee

@@ -55,7 +55,7 @@ describe "joinProject", ->
 			@client.emit "clientTracking.getConnectedUsers", (error, users) =>
 				connected = false
 				for user in users
-					if user.client_id == @client.socket.sessionid and user.user_id == @user_id
+					if user.client_id == @client.publicId and user.user_id == @user_id
 						connected = true
 						break
 				expect(connected).to.equal true

+ 2 - 2
services/real-time/test/acceptance/coffee/LeaveProjectTests.coffee

@@ -64,12 +64,12 @@ describe "leaveProject", ->
 			], done
 
 		it "should emit a disconnect message to the room", ->
-			@clientBDisconnectMessages.should.deep.equal [@clientA.socket.sessionid]
+			@clientBDisconnectMessages.should.deep.equal [@clientA.publicId]
 
 		it "should no longer list the client in connected users", (done) ->
 			@clientB.emit "clientTracking.getConnectedUsers", (error, users) =>
 				for user in users
-					if user.client_id == @clientA.socket.sessionid
+					if user.client_id == @clientA.publicId
 						throw "Expected clientA to not be listed in connected users"
 				return done()
 

+ 2 - 2
services/real-time/test/acceptance/coffee/ReceiveUpdateTests.coffee

@@ -65,7 +65,7 @@ describe "receiveUpdate", ->
 				doc_id: @doc_id
 				op:
 					meta:
-						source: @clientA.socket.sessionid
+						source: @clientA.publicId
 					v: @version
 					doc: @doc_id
 					op: [{i: "foo", p: 50}]				
@@ -97,4 +97,4 @@ describe "receiveUpdate", ->
 			
 		it "should disconnect the clients", ->
 			@clientA.socket.connected.should.equal false
-			@clientB.socket.connected.should.equal false
+			@clientB.socket.connected.should.equal false

+ 2 - 0
services/real-time/test/acceptance/coffee/helpers/RealTimeClient.coffee

@@ -37,6 +37,8 @@ module.exports = Client =
 			
 	connect: (cookie) ->
 		client = io.connect("http://localhost:3026", 'force new connection': true)
+		client.on 'connectionAccepted', (_, publicId) ->
+			client.publicId = publicId
 		return client
 		
 	getConnectedClients: (callback = (error, clients) ->) ->

+ 1 - 1
services/real-time/test/unit/coffee/DocumentUpdaterControllerTests.coffee

@@ -93,7 +93,7 @@ describe "DocumentUpdaterController", ->
 			@otherClients = [new MockClient(), new MockClient()]
 			@update =
 				op: [ t: "foo", p: 12 ]
-				meta: source: @sourceClient.id
+				meta: source: @sourceClient.publicId
 				v: @version = 42
 				doc: @doc_id
 			@io.sockets =

+ 17 - 16
services/real-time/test/unit/coffee/WebsocketControllerTests.coffee

@@ -21,6 +21,7 @@ describe 'WebsocketController', ->
 		@callback = sinon.stub()
 		@client =
 			id: @client_id = "mock-client-id-123"
+			publicId: "other-id-#{Math.random()}"
 			params: {}
 			set: sinon.stub()
 			get: (param, cb) -> cb null, @params[param]
@@ -106,7 +107,7 @@ describe 'WebsocketController', ->
 
 			it "should mark the user as connected in ConnectedUsersManager", ->
 				@ConnectedUsersManager.updateUserPosition
-					.calledWith(@project_id, @client.id, @user, null)
+					.calledWith(@project_id, @client.publicId, @user, null)
 					.should.equal true
 
 			it "should increment the join-project metric", ->
@@ -185,12 +186,12 @@ describe 'WebsocketController', ->
 
 			it "should end clientTracking.clientDisconnected to the project room", ->
 				@WebsocketLoadBalancer.emitToRoom
-					.calledWith(@project_id, "clientTracking.clientDisconnected", @client.id)
+					.calledWith(@project_id, "clientTracking.clientDisconnected", @client.publicId)
 					.should.equal true
 
 			it "should mark the user as disconnected", ->
 				@ConnectedUsersManager.markUserAsDisconnected
-					.calledWith(@project_id, @client.id)
+					.calledWith(@project_id, @client.publicId)
 					.should.equal true
 
 			it "should flush the project in the document updater", ->
@@ -223,12 +224,12 @@ describe 'WebsocketController', ->
 
 			it "should not end clientTracking.clientDisconnected to the project room", ->
 				@WebsocketLoadBalancer.emitToRoom
-					.calledWith(@project_id, "clientTracking.clientDisconnected", @client.id)
+					.calledWith(@project_id, "clientTracking.clientDisconnected", @client.publicId)
 					.should.equal false
 
 			it "should not mark the user as disconnected", ->
 				@ConnectedUsersManager.markUserAsDisconnected
-					.calledWith(@project_id, @client.id)
+					.calledWith(@project_id, @client.publicId)
 					.should.equal false
 
 			it "should not flush the project in the document updater", ->
@@ -247,12 +248,12 @@ describe 'WebsocketController', ->
 
 			it "should not end clientTracking.clientDisconnected to the project room", ->
 				@WebsocketLoadBalancer.emitToRoom
-					.calledWith(@project_id, "clientTracking.clientDisconnected", @client.id)
+					.calledWith(@project_id, "clientTracking.clientDisconnected", @client.publicId)
 					.should.equal false
 
 			it "should not mark the user as disconnected", ->
 				@ConnectedUsersManager.markUserAsDisconnected
-					.calledWith(@project_id, @client.id)
+					.calledWith(@project_id, @client.publicId)
 					.should.equal false
 
 			it "should not flush the project in the document updater", ->
@@ -488,7 +489,7 @@ describe 'WebsocketController', ->
 
 				@populatedCursorData =
 					doc_id: @doc_id,
-					id: @client.id
+					id: @client.publicId
 					name: "#{@first_name} #{@last_name}"
 					row: @row
 					column: @column
@@ -499,7 +500,7 @@ describe 'WebsocketController', ->
 				@WebsocketLoadBalancer.emitToRoom.calledWith(@project_id, "clientTracking.clientUpdated", @populatedCursorData).should.equal true
 
 			it "should send the  cursor data to the connected user manager", (done)->
-				@ConnectedUsersManager.updateUserPosition.calledWith(@project_id, @client.id, {
+				@ConnectedUsersManager.updateUserPosition.calledWith(@project_id, @client.publicId, {
 					_id: @user_id,
 					email: @email,
 					first_name: @first_name,
@@ -528,7 +529,7 @@ describe 'WebsocketController', ->
 
 				@populatedCursorData =
 					doc_id: @doc_id,
-					id: @client.id
+					id: @client.publicId
 					name: "#{@first_name}"
 					row: @row
 					column: @column
@@ -539,7 +540,7 @@ describe 'WebsocketController', ->
 				@WebsocketLoadBalancer.emitToRoom.calledWith(@project_id, "clientTracking.clientUpdated", @populatedCursorData).should.equal true
 
 			it "should send the  cursor data to the connected user manager", (done)->
-				@ConnectedUsersManager.updateUserPosition.calledWith(@project_id, @client.id, {
+				@ConnectedUsersManager.updateUserPosition.calledWith(@project_id, @client.publicId, {
 					_id: @user_id,
 					email: @email,
 					first_name: @first_name,
@@ -568,7 +569,7 @@ describe 'WebsocketController', ->
 
 				@populatedCursorData =
 					doc_id: @doc_id,
-					id: @client.id
+					id: @client.publicId
 					name: "#{@last_name}"
 					row: @row
 					column: @column
@@ -579,7 +580,7 @@ describe 'WebsocketController', ->
 				@WebsocketLoadBalancer.emitToRoom.calledWith(@project_id, "clientTracking.clientUpdated", @populatedCursorData).should.equal true
 
 			it "should send the  cursor data to the connected user manager", (done)->
-				@ConnectedUsersManager.updateUserPosition.calledWith(@project_id, @client.id, {
+				@ConnectedUsersManager.updateUserPosition.calledWith(@project_id, @client.publicId, {
 					_id: @user_id,
 					email: @email,
 					first_name: undefined,
@@ -609,7 +610,7 @@ describe 'WebsocketController', ->
 				@WebsocketLoadBalancer.emitToRoom
 					.calledWith(@project_id, "clientTracking.clientUpdated", {
 						doc_id: @doc_id,
-						id: @client.id,
+						id: @client.publicId,
 						user_id: @user_id,
 						name: "",
 						row: @row,
@@ -631,7 +632,7 @@ describe 'WebsocketController', ->
 				@WebsocketLoadBalancer.emitToRoom
 					.calledWith(@project_id, "clientTracking.clientUpdated", {
 						doc_id: @doc_id,
-						id: @client.id
+						id: @client.publicId
 						name: ""
 						row: @row
 						column: @column
@@ -655,7 +656,7 @@ describe 'WebsocketController', ->
 				@WebsocketController.applyOtUpdate @client, @doc_id, @update, @callback
 
 			it "should set the source of the update to the client id", ->
-				@update.meta.source.should.equal @client.id
+				@update.meta.source.should.equal @client.publicId
 
 			it "should set the user_id of the update to the user id", ->
 				@update.meta.user_id.should.equal @user_id

+ 1 - 0
services/real-time/test/unit/coffee/helpers/MockClient.coffee

@@ -9,6 +9,7 @@ module.exports = class MockClient
 		@emit = sinon.stub()
 		@disconnect = sinon.stub()
 		@id = idCounter++
+		@publicId = idCounter++
 	set : (key, value, callback) ->
 		@attributes[key] = value
 		callback() if callback?