Merge pull request #13052 from overleaf/jk-realtime-disconnect-users

[real-time] Disconnect client when user is removed from project

GitOrigin-RevId: e3d31b9192c639b2988e488e34e19050e7caeb4a

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

@@ -25,6 +25,16 @@ module.exports = WebsocketLoadBalancer = {
   rclientPubList: RedisClientManager.createClientList(Settings.redis.pubsub),
   rclientSubList: RedisClientManager.createClientList(Settings.redis.pubsub),
 
+  shouldDisconnectClient(client, message) {
+    const userId = client.ol_context.user_id
+    if (message?.message === 'userRemovedFromProject') {
+      if (message?.payload?.includes(userId)) {
+        return true
+      }
+    }
+    return false
+  },
+
   emitToRoom(roomId, message, ...payload) {
     if (!roomId) {
       logger.warn(
@@ -154,7 +164,19 @@ module.exports = WebsocketLoadBalancer = {
         for (const client of clientList) {
           if (!seen.has(client.id)) {
             seen.set(client.id, true)
-            client.emit(message.message, ...message.payload)
+            if (WebsocketLoadBalancer.shouldDisconnectClient(client, message)) {
+              logger.debug(
+                {
+                  message,
+                  userId: client?.ol_context?.user_id,
+                  projectId: client?.ol_context?.project_id,
+                },
+                'disconnecting client'
+              )
+              client.disconnect()
+            } else {
+              client.emit(message.message, ...message.payload)
+            }
           }
         }
       } else if (message.health_check) {

services/real-time/test/acceptance/js/ReceiveEditorEventTests.js

@@ -0,0 +1,248 @@
+/* eslint-disable
+    no-unused-vars,
+*/
+// TODO: This file was created by bulk-decaffeinate.
+// Fix any style issues and re-enable lint.
+/*
+ * decaffeinate suggestions:
+ * DS102: Remove unnecessary code created because of implicit returns
+ * DS207: Consider shorter variations of null checks
+ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
+ */
+const { expect } = require('chai')
+
+const RealTimeClient = require('./helpers/RealTimeClient')
+const FixturesManager = require('./helpers/FixturesManager')
+
+const async = require('async')
+
+const settings = require('@overleaf/settings')
+const redis = require('@overleaf/redis-wrapper')
+const rclient = redis.createClient(settings.redis.pubsub)
+
+describe('receiveEditorEvent', function () {
+  beforeEach(function (done) {
+    this.lines = ['test', 'doc', 'lines']
+    this.version = 42
+    this.ops = ['mock', 'doc', 'ops']
+
+    /**
+     * We will set up a project, a doc, and three users: the owner, user 'a' and user 'b'
+     */
+    this.project_id = null
+    this.doc_id = null
+
+    this.owner_user_id = null
+    this.owner_client = null
+
+    this.user_a_id = null
+    this.user_a_client = null
+
+    this.user_b_id = null
+    this.user_b_client = null
+
+    async.series(
+      [
+        /**
+         * Create the project, doc, and owner
+         */
+        cb => {
+          return FixturesManager.setUpProject(
+            {
+              privilegeLevel: 'owner',
+              project: { name: 'Test Project' },
+            },
+            (error, { user_id: userId, project_id: projectId }) => {
+              if (error) return done(error)
+              this.owner_user_id = userId
+              this.project_id = projectId
+              return cb()
+            }
+          )
+        },
+
+        cb => {
+          return FixturesManager.setUpDoc(
+            this.project_id,
+            { lines: this.lines, version: this.version, ops: this.ops },
+            (e, { doc_id: docId }) => {
+              this.doc_id = docId
+              return cb(e)
+            }
+          )
+        },
+
+        /**
+         * Connect owner to project/doc
+         */
+        cb => {
+          this.owner_client = RealTimeClient.connect()
+          return this.owner_client.on('connectionAccepted', cb)
+        },
+
+        cb => {
+          return this.owner_client.emit(
+            'joinProject',
+            {
+              project_id: this.project_id,
+            },
+            cb
+          )
+        },
+
+        cb => {
+          return this.owner_client.emit('joinDoc', this.doc_id, cb)
+        },
+
+        /**
+         * add user_a to project
+         */
+        cb => {
+          return FixturesManager.setUpProject(
+            {
+              privilegeLevel: 'readAndWrite',
+              project_id: this.project_id,
+            },
+            (error, { user_id: userIdSecond }) => {
+              if (error) return done(error)
+              this.user_a_id = userIdSecond
+              return cb()
+            }
+          )
+        },
+
+        /**
+         * Connect user_a to project/doc
+         */
+        cb => {
+          this.user_a_client = RealTimeClient.connect()
+          return this.user_a_client.on('connectionAccepted', cb)
+        },
+
+        cb => {
+          return this.user_a_client.emit(
+            'joinProject',
+            {
+              project_id: this.project_id,
+            },
+            cb
+          )
+        },
+        cb => {
+          return this.user_a_client.emit('joinDoc', this.doc_id, cb)
+        },
+
+        /**
+         * Set up user_b
+         */
+        cb => {
+          return FixturesManager.setUpProject(
+            {
+              privilegeLevel: 'readAndWrite',
+              project_id: this.project_id,
+            },
+            (error, { user_id: userIdThird }) => {
+              if (error) return done(error)
+              this.user_b_id = userIdThird
+              return cb()
+            }
+          )
+        },
+
+        /**
+         * Connect user_b to project/doc
+         */
+        cb => {
+          this.user_b_client = RealTimeClient.connect()
+          return this.user_b_client.on('connectionAccepted', cb)
+        },
+
+        cb => {
+          return this.user_b_client.emit(
+            'joinProject',
+            {
+              project_id: this.project_id,
+            },
+            cb
+          )
+        },
+        cb => {
+          return this.user_b_client.emit('joinDoc', this.doc_id, cb)
+        },
+
+        /**
+         * Listen for updates
+         */
+        cb => {
+          const eventName = 'userRemovedFromProject'
+          this.owner_updates = []
+          this.owner_client.on(eventName, update =>
+            this.owner_updates.push({ [eventName]: update })
+          )
+          this.user_a_updates = []
+          this.user_a_client.on(eventName, update =>
+            this.user_a_updates.push({ [eventName]: update })
+          )
+          this.user_b_updates = []
+          this.user_b_client.on(eventName, update =>
+            this.user_b_updates.push({ [eventName]: update })
+          )
+          return cb()
+        },
+      ],
+      done
+    )
+  })
+
+  afterEach(function () {
+    if (this.owner_client) {
+      this.owner_client.disconnect()
+    }
+    if (this.user_a_client) {
+      this.user_a_client.disconnect()
+    }
+    if (this.user_b_client) {
+      this.user_b_client.disconnect()
+    }
+  })
+
+  describe('event: userRemovedFromProject', function () {
+    let removedUserId
+    beforeEach(function (done) {
+      /**
+       * We remove user_a from the project
+       */
+      removedUserId = `${this.user_a_id}`
+      rclient.publish(
+        'editor-events',
+        JSON.stringify({
+          room_id: this.project_id,
+          message: 'userRemovedFromProject',
+          payload: [removedUserId],
+        })
+      )
+      setTimeout(done, 200)
+    })
+
+    it('should disconnect the removed user', function () {
+      expect(this.user_a_client.socket.connected).to.equal(false)
+    })
+
+    it('should not disconnect the other users', function () {
+      expect(this.owner_client.socket.connected).to.equal(true)
+      expect(this.user_b_client.socket.connected).to.equal(true)
+    })
+
+    it('should send the event to the remaining connected clients', function () {
+      expect(this.owner_updates).to.deep.equal([
+        { userRemovedFromProject: removedUserId },
+      ])
+
+      expect(this.user_a_updates.length).to.equal(0)
+
+      expect(this.user_b_updates).to.deep.equal([
+        { userRemovedFromProject: removedUserId },
+      ])
+    })
+  })
+})

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

@@ -11,6 +11,7 @@
  */
 const SandboxedModule = require('sandboxed-module')
 const sinon = require('sinon')
+const expect = require('chai').expect
 const modulePath = require('path').join(
   __dirname,
   '../../../app/js/WebsocketLoadBalancer'
@@ -53,6 +54,42 @@ describe('WebsocketLoadBalancer', function () {
     return (this.payload = ['argument one', 42])
   })
 
+  describe('shouldDisconnectClient', function () {
+    const client = {
+      ol_context: { user_id: 'abcd' },
+    }
+
+    it('should return false for general messages', function () {
+      const message = {
+        message: 'someNiceMessage',
+        payload: [{ data: 'whatever' }],
+      }
+      expect(
+        this.WebsocketLoadBalancer.shouldDisconnectClient(client, message)
+      ).to.equal(false)
+    })
+
+    it('should return false for userRemovedFromProject, when the user_id does not match', function () {
+      const message = {
+        message: 'userRemovedFromProject',
+        payload: ['xyz'],
+      }
+      expect(
+        this.WebsocketLoadBalancer.shouldDisconnectClient(client, message)
+      ).to.equal(false)
+    })
+
+    it('should return true for userRemovedFromProject, if the user_id matches', function () {
+      const message = {
+        message: 'userRemovedFromProject',
+        payload: [`${client.ol_context.user_id}`],
+      }
+      expect(
+        this.WebsocketLoadBalancer.shouldDisconnectClient(client, message)
+      ).to.equal(true)
+    })
+  })
+
   describe('emitToRoom', function () {
     beforeEach(function () {
       return this.WebsocketLoadBalancer.emitToRoom(
@@ -279,7 +316,7 @@ describe('WebsocketLoadBalancer', function () {
       })
     }) // restricted client, should not be called
 
-    return describe('when emitting to all', function () {
+    describe('when emitting to all', function () {
       beforeEach(function () {
         this.io.sockets = { emit: (this.emit = sinon.stub()) }
         const data = JSON.stringify({
@@ -300,5 +337,66 @@ describe('WebsocketLoadBalancer', function () {
           .should.equal(true)
       })
     })
+
+    describe('when it should disconnect one of the clients', function () {
+      const targetUserId = 'bbb'
+      const message = 'userRemovedFromProject'
+      const payload = [`${targetUserId}`]
+      const clients = [
+        {
+          id: 'client-id-1',
+          emit: sinon.stub(),
+          ol_context: { user_id: 'aaa' },
+          disconnect: sinon.stub(),
+        },
+        {
+          id: 'client-id-2',
+          emit: sinon.stub(),
+          ol_context: { user_id: `${targetUserId}` },
+          disconnect: sinon.stub(),
+        },
+        {
+          id: 'client-id-3',
+          emit: sinon.stub(),
+          ol_context: { user_id: 'ccc' },
+          disconnect: sinon.stub(),
+        },
+      ]
+      beforeEach(function () {
+        this.io.sockets = {
+          clients: sinon.stub().returns(clients),
+        }
+        const data = JSON.stringify({
+          room_id: this.room_id,
+          message,
+          payload,
+        })
+        return this.WebsocketLoadBalancer._processEditorEvent(
+          this.io,
+          'editor-events',
+          data
+        )
+      })
+
+      it('should disconnect the matching client, while sending message to other clients', function () {
+        this.io.sockets.clients.calledWith(this.room_id).should.equal(true)
+
+        const [client1, client2, client3] = clients
+
+        // disconnecting one client
+        client1.disconnect.called.should.equal(false)
+        client2.disconnect.called.should.equal(true)
+        client3.disconnect.called.should.equal(false)
+
+        // emitting to remaining clients
+        client1.emit
+          .calledWith(message, ...Array.from(payload))
+          .should.equal(true)
+        client2.emit.called.should.equal(false) // disconnected client should not be called
+        client3.emit
+          .calledWith(message, ...Array.from(payload))
+          .should.equal(true)
+      })
+    })
   })
 })