github/overleaf
1
0
Fork 0
mirror of https://github.com/overleaf/overleaf.git synced 2024-11-21 20:47:08 -05:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #148 from overleaf/jpa-secondary-id

Browse source 
[misc] socket.io: use a secondary publicId for public facing usages
This commit is contained in:
Jakob Ackermann 2020-06-08 14:51:01 +02:00 committed by GitHub
commit 6dce45bf2f
14 changed files with 44 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
services/real-time/app/coffee/DocumentUpdaterController.coffee
View file

@ -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

4
services/real-time/app/coffee/Router.coffee
View file

@ -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 = 'P.' + base64id.generateId()
client.emit("connectionAccepted", null, client.publicId)
metrics.inc('socket-io.connection')
metrics.gauge('socket-io.clients', io.sockets.clients()?.length)

12
services/real-time/app/coffee/WebsocketController.coffee
View file

@ -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

2
services/real-time/app/coffee/WebsocketLoadBalancer.coffee
View file

@ -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
services/real-time/package.json
View file

@ -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",

6
services/real-time/test/acceptance/coffee/ApplyUpdateTests.coffee
View file

@ -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

6
services/real-time/test/acceptance/coffee/ClientTrackingTests.coffee
View file

@ -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: ""
}

2
services/real-time/test/acceptance/coffee/JoinProjectTests.coffee
View file

@ -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

4
services/real-time/test/acceptance/coffee/LeaveProjectTests.coffee
View file

@ -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()

4
services/real-time/test/acceptance/coffee/ReceiveUpdateTests.coffee
View file

@ -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
services/real-time/test/acceptance/coffee/helpers/RealTimeClient.coffee
View file

@ -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) ->) ->

2
services/real-time/test/unit/coffee/DocumentUpdaterControllerTests.coffee
View file

@ -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 =

33
services/real-time/test/unit/coffee/WebsocketControllerTests.coffee
View file

@ -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
services/real-time/test/unit/coffee/helpers/MockClient.coffee
View file

@ -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?