Merge pull request #13140 from overleaf/jk-real-time-disconnect-link-sharing

[real-time] Disconnect relevant users when link-sharing is turned off

GitOrigin-RevId: cf44a30a235717b658a759e8a74ae4d0e5abae47

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

@@ -41,6 +41,9 @@ describe('receiveEditorEvent', function () {
     this.user_b_id = null
     this.user_b_client = null
 
+    this.user_c_id = null
+    this.user_c_client = null
+
     async.series(
       [
         /**
@@ -51,6 +54,7 @@ describe('receiveEditorEvent', function () {
             {
               privilegeLevel: 'owner',
               project: { name: 'Test Project' },
+              userMetadata: { isInvitedMember: true },
             },
             (error, { user_id: userId, project_id: projectId }) => {
               if (error) return done(error)
@@ -95,13 +99,14 @@ describe('receiveEditorEvent', function () {
         },
 
         /**
-         * add user_a to project
+         * add user_a to project, as an invited member
          */
         cb => {
           return FixturesManager.setUpProject(
             {
               privilegeLevel: 'readAndWrite',
               project_id: this.project_id,
+              userMetadata: { isTokenMember: false, isInvitedMember: true },
             },
             (error, { user_id: userIdSecond }) => {
               if (error) return done(error)
@@ -133,13 +138,14 @@ describe('receiveEditorEvent', function () {
         },
 
         /**
-         * Set up user_b
+         * Set up user_b, as a token-access/link-sharing user
          */
         cb => {
           return FixturesManager.setUpProject(
             {
               privilegeLevel: 'readAndWrite',
               project_id: this.project_id,
+              userMetadata: { isTokenMember: true, isInvitedMember: false },
             },
             (error, { user_id: userIdThird }) => {
               if (error) return done(error)
@@ -170,23 +176,81 @@ describe('receiveEditorEvent', function () {
           return this.user_b_client.emit('joinDoc', this.doc_id, cb)
         },
 
+        /**
+         * Set up user_c, as a 'restricted' user (anonymous read-only link-sharing)
+         */
+        cb => {
+          return FixturesManager.setUpProject(
+            {
+              privilegeLevel: 'readAndWrite',
+              project_id: this.project_id,
+              userMetadata: {
+                isTokenMember: false,
+                isInvitedMember: false,
+                isRestrictedUser: true,
+              },
+            },
+            (error, { user_id: userIdFourth }) => {
+              if (error) return done(error)
+              this.user_c_id = userIdFourth
+              return cb()
+            }
+          )
+        },
+
+        /**
+         * Connect user_c to project/doc
+         */
+        cb => {
+          this.user_c_client = RealTimeClient.connect()
+          return this.user_c_client.on('connectionAccepted', cb)
+        },
+
+        cb => {
+          return this.user_c_client.emit(
+            'joinProject',
+            {
+              project_id: this.project_id,
+            },
+            cb
+          )
+        },
+        cb => {
+          return this.user_c_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 })
-          )
+          this.user_c_updates = []
+
+          const eventNames = [
+            'userRemovedFromProject',
+            'project:publicAccessLevel:changed',
+            'project:access:revoked',
+          ]
+
+          for (const eventName of eventNames) {
+            this.owner_client.on(eventName, update =>
+              this.owner_updates.push({ [eventName]: update })
+            )
+            this.user_a_client.on(eventName, update =>
+              this.user_a_updates.push({ [eventName]: update })
+            )
+            this.user_b_client.on(eventName, update =>
+              this.user_b_updates.push({ [eventName]: update })
+            )
+            this.user_c_client.on(eventName, update =>
+              this.user_c_updates.push({ [eventName]: update })
+            )
+          }
+
           return cb()
         },
       ],
@@ -204,6 +268,101 @@ describe('receiveEditorEvent', function () {
     if (this.user_b_client) {
       this.user_b_client.disconnect()
     }
+    if (this.user_c_client) {
+      this.user_c_client.disconnect()
+    }
+  })
+
+  describe('event: project:publicAccessLevel:changed, set to private', function () {
+    beforeEach(function (done) {
+      /**
+       * We turn off link sharing
+       */
+      rclient.publish(
+        'editor-events',
+        JSON.stringify({
+          room_id: this.project_id,
+          message: 'project:publicAccessLevel:changed',
+          payload: [{ newAccessLevel: 'private' }],
+        })
+      )
+      setTimeout(done, 200)
+    })
+
+    it('should disconnect the token-access user, and restricted users', function () {
+      expect(this.user_b_client.socket.connected).to.equal(false)
+      expect(this.user_c_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_a_client.socket.connected).to.equal(true)
+    })
+
+    it('should send the event to the remaining connected clients', function () {
+      expect(this.owner_updates).to.deep.equal([
+        { 'project:publicAccessLevel:changed': { newAccessLevel: 'private' } },
+      ])
+
+      expect(this.user_a_updates).to.deep.equal([
+        { 'project:publicAccessLevel:changed': { newAccessLevel: 'private' } },
+      ])
+    })
+
+    it('should send a project:access:revoked message to the disconnected clients', function () {
+      expect(this.user_b_updates).to.deep.equal([
+        { 'project:access:revoked': undefined },
+      ])
+      expect(this.user_c_updates).to.deep.equal([
+        { 'project:access:revoked': undefined },
+      ])
+    })
+  })
+
+  describe('event: project:publicAccessLevel:changed, set to tokenBased', function () {
+    beforeEach(function (done) {
+      /**
+       * We turn on link sharing
+       */
+      rclient.publish(
+        'editor-events',
+        JSON.stringify({
+          room_id: this.project_id,
+          message: 'project:publicAccessLevel:changed',
+          payload: [{ newAccessLevel: 'tokenBased' }],
+        })
+      )
+      setTimeout(done, 200)
+    })
+
+    it('should not disconnect anyone', function () {
+      expect(this.owner_client.socket.connected).to.equal(true)
+      expect(this.user_a_client.socket.connected).to.equal(true)
+      expect(this.user_b_client.socket.connected).to.equal(true)
+      expect(this.user_c_client.socket.connected).to.equal(true)
+    })
+
+    it('should send the event to all non-restricted clients', function () {
+      expect(this.owner_updates).to.deep.equal([
+        {
+          'project:publicAccessLevel:changed': { newAccessLevel: 'tokenBased' },
+        },
+      ])
+
+      expect(this.user_a_updates).to.deep.equal([
+        {
+          'project:publicAccessLevel:changed': { newAccessLevel: 'tokenBased' },
+        },
+      ])
+
+      expect(this.user_b_updates).to.deep.equal([
+        {
+          'project:publicAccessLevel:changed': { newAccessLevel: 'tokenBased' },
+        },
+      ])
+      // restricted users don't receive this type of message
+      expect(this.user_c_updates.length).to.equal(0)
+    })
   })
 
   describe('event: userRemovedFromProject', function () {
@@ -238,11 +397,15 @@ describe('receiveEditorEvent', function () {
         { userRemovedFromProject: removedUserId },
       ])
 
-      expect(this.user_a_updates.length).to.equal(0)
-
       expect(this.user_b_updates).to.deep.equal([
         { userRemovedFromProject: removedUserId },
       ])
     })
+
+    it('should send a project:access:revoked message to the disconnected clients', function () {
+      expect(this.user_a_updates).to.deep.equal([
+        { 'project:access:revoked': undefined },
+      ])
+    })
   })
 })

services/real-time/test/acceptance/js/helpers/FixturesManager.js

@@ -34,6 +34,7 @@ module.exports = FixturesManager = {
       privilegeLevel,
       project,
       publicAccess,
+      userMetadata,
     } = options
 
     const privileges = {}
@@ -42,7 +43,15 @@ module.exports = FixturesManager = {
       privileges['anonymous-user'] = publicAccess
     }
 
-    MockWebServer.createMockProject(projectId, privileges, project)
+    const metadataByUser = {}
+    metadataByUser[userId] = userMetadata
+
+    MockWebServer.createMockProject(
+      projectId,
+      privileges,
+      project,
+      metadataByUser
+    )
     return MockWebServer.run(error => {
       if (error != null) {
         throw error

services/real-time/test/acceptance/js/helpers/MockWebServer.js

@@ -16,9 +16,11 @@ const express = require('express')
 module.exports = MockWebServer = {
   projects: {},
   privileges: {},
+  userMetadata: {},
 
-  createMockProject(projectId, privileges, project) {
+  createMockProject(projectId, privileges, project, metadataByUser) {
     MockWebServer.privileges[projectId] = privileges
+    MockWebServer.userMetadata[projectId] = metadataByUser
     return (MockWebServer.projects[projectId] = project)
   },
 
@@ -26,12 +28,12 @@ module.exports = MockWebServer = {
     if (callback == null) {
       callback = function () {}
     }
-    return callback(
-      null,
-      MockWebServer.projects[projectId],
+    const project = MockWebServer.projects[projectId]
+    const privilegeLevel =
       MockWebServer.privileges[projectId][userId] ||
-        MockWebServer.privileges[projectId]['anonymous-user']
-    )
+      MockWebServer.privileges[projectId]['anonymous-user']
+    const userMetadata = MockWebServer.userMetadata[projectId]?.[userId]
+    return callback(null, project, privilegeLevel, userMetadata)
   },
 
   joinProjectRequest(req, res, next) {
@@ -52,13 +54,16 @@ module.exports = MockWebServer = {
       return MockWebServer.joinProject(
         projectId,
         userId,
-        (error, project, privilegeLevel) => {
+        (error, project, privilegeLevel, userMetadata) => {
           if (error != null) {
             return next(error)
           }
           return res.json({
             project,
             privilegeLevel,
+            isRestrictedUser: !!userMetadata?.isRestrictedUser,
+            isTokenMember: !!userMetadata?.isTokenMember,
+            isInvitedMember: !!userMetadata?.isInvitedMember,
           })
         }
       )

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

@@ -43,6 +43,8 @@ describe('WebApiManager', function () {
           project: { name: 'Test project' },
           privilegeLevel: 'owner',
           isRestrictedUser: true,
+          isTokenMember: true,
+          isInvitedMember: true,
         }
         this.request.post = sinon
           .stub()
@@ -79,7 +81,11 @@ describe('WebApiManager', function () {
             null,
             this.response.project,
             this.response.privilegeLevel,
-            this.response.isRestrictedUser
+            {
+              isRestrictedUser: this.response.isRestrictedUser,
+              isTokenMember: this.response.isTokenMember,
+              isInvitedMember: this.response.isInvitedMember,
+            }
           )
           .should.equal(true)
       })

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

@@ -75,15 +75,15 @@ describe('WebsocketController', function () {
           .stub()
           .callsArgAsync(4)
         this.isRestrictedUser = true
+        this.isTokenMember = true
+        this.isInvitedMember = true
         this.WebApiManager.joinProject = sinon
           .stub()
-          .callsArgWith(
-            2,
-            null,
-            this.project,
-            this.privilegeLevel,
-            this.isRestrictedUser
-          )
+          .callsArgWith(2, null, this.project, this.privilegeLevel, {
+            isRestrictedUser: this.isRestrictedUser,
+            isTokenMember: this.isTokenMember,
+            isInvitedMember: this.isInvitedMember,
+          })
         this.RoomManager.joinProject = sinon.stub().callsArg(2)
         return this.WebsocketController.joinProject(
           this.client,
@@ -150,6 +150,14 @@ describe('WebsocketController', function () {
           this.isRestrictedUser
         )
       })
+      it('should set the is_token_member flag on the client', function () {
+        this.client.ol_context.is_token_member.should.equal(this.isTokenMember)
+      })
+      it('should set the is_invited_member flag on the client', function () {
+        this.client.ol_context.is_invited_member.should.equal(
+          this.isInvitedMember
+        )
+      })
       it('should call the callback with the project, privilegeLevel and protocolVersion', function () {
         return this.callback
           .calledWith(
@@ -212,15 +220,15 @@ describe('WebsocketController', function () {
           .stub()
           .callsArgAsync(4)
         this.isRestrictedUser = true
+        this.isTokenMember = true
+        this.isInvitedMember = true
         this.WebApiManager.joinProject = sinon
           .stub()
-          .callsArgWith(
-            2,
-            null,
-            this.project,
-            this.privilegeLevel,
-            this.isRestrictedUser
-          )
+          .callsArgWith(2, null, this.project, this.privilegeLevel, {
+            isRestrictedUser: this.isRestrictedUser,
+            isTokenMember: this.isTokenMember,
+            isInvitedMember: this.isInvitedMember,
+          })
         this.RoomManager.joinProject = sinon
           .stub()
           .callsArgWith(2, new Error('subscribe failed'))
@@ -273,12 +281,11 @@ describe('WebsocketController', function () {
       beforeEach(function () {
         this.WebApiManager.joinProject = (project, user, cb) => {
           this.client.disconnected = true
-          return cb(
-            null,
-            this.project,
-            this.privilegeLevel,
-            this.isRestrictedUser
-          )
+          return cb(null, this.project, this.privilegeLevel, {
+            isRestrictedUser: this.isRestrictedUser,
+            isTokenMember: this.isTokenMember,
+            isInvitedMember: this.isInvitedMember,
+          })
         }
 
         return this.WebsocketController.joinProject(

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

@@ -23,6 +23,7 @@ describe('WebsocketLoadBalancer', function () {
     this.RoomEvents = { on: sinon.stub() }
     this.WebsocketLoadBalancer = SandboxedModule.require(modulePath, {
       requires: {
+        '@overleaf/settings': (this.Settings = { redis: {} }),
         './RedisClientManager': {
           createClientList: () => [],
         },
@@ -55,11 +56,10 @@ describe('WebsocketLoadBalancer', function () {
   })
 
   describe('shouldDisconnectClient', function () {
-    const client = {
-      ol_context: { user_id: 'abcd' },
-    }
-
     it('should return false for general messages', function () {
+      const client = {
+        ol_context: { user_id: 'abcd' },
+      }
       const message = {
         message: 'someNiceMessage',
         payload: [{ data: 'whatever' }],
@@ -69,24 +69,103 @@ describe('WebsocketLoadBalancer', function () {
       ).to.equal(false)
     })
 
-    it('should return false for userRemovedFromProject, when the user_id does not match', function () {
-      const message = {
-        message: 'userRemovedFromProject',
-        payload: ['xyz'],
+    describe('user removed from project', function () {
+      const messageName = 'userRemovedFromProject'
+      const client = {
+        ol_context: { user_id: 'abcd' },
       }
-      expect(
-        this.WebsocketLoadBalancer.shouldDisconnectClient(client, message)
-      ).to.equal(false)
+      it('should return false, when the user_id does not match', function () {
+        const message = {
+          message: messageName,
+          payload: ['xyz'],
+        }
+        expect(
+          this.WebsocketLoadBalancer.shouldDisconnectClient(client, message)
+        ).to.equal(false)
+      })
+
+      it('should return true, if the user_id matches', function () {
+        const message = {
+          message: messageName,
+          payload: [`${client.ol_context.user_id}`],
+        }
+        expect(
+          this.WebsocketLoadBalancer.shouldDisconnectClient(client, message)
+        ).to.equal(true)
+      })
     })
 
-    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('link-sharing turned off', function () {
+      const messageName = 'project:publicAccessLevel:changed'
+
+      describe('when the new access level is set to "private"', function () {
+        const message = {
+          message: messageName,
+          payload: [{ newAccessLevel: 'private' }],
+        }
+        describe('when the user is an invited member', function () {
+          const client = {
+            ol_context: {
+              is_invited_member: true,
+            },
+          }
+
+          it('should return false', function () {
+            expect(
+              this.WebsocketLoadBalancer.shouldDisconnectClient(client, message)
+            ).to.equal(false)
+          })
+        })
+
+        describe('when the user not an invited member', function () {
+          const client = {
+            ol_context: {
+              is_invited_member: false,
+            },
+          }
+
+          it('should return true', function () {
+            expect(
+              this.WebsocketLoadBalancer.shouldDisconnectClient(client, message)
+            ).to.equal(true)
+          })
+        })
+      })
+
+      describe('when the new access level is "tokenBased"', function () {
+        const message = {
+          message: messageName,
+          payload: [{ newAccessLevel: 'tokenBased' }],
+        }
+
+        describe('when the user is an invited member', function () {
+          const client = {
+            ol_context: {
+              is_invited_member: true,
+            },
+          }
+
+          it('should return false', function () {
+            expect(
+              this.WebsocketLoadBalancer.shouldDisconnectClient(client, message)
+            ).to.equal(false)
+          })
+        })
+
+        describe('when the user not an invited member', function () {
+          const client = {
+            ol_context: {
+              is_invited_member: false,
+            },
+          }
+
+          it('should return false', function () {
+            expect(
+              this.WebsocketLoadBalancer.shouldDisconnectClient(client, message)
+            ).to.equal(false)
+          })
+        })
+      })
     })
   })
 
@@ -392,7 +471,7 @@ describe('WebsocketLoadBalancer', function () {
         client1.emit
           .calledWith(message, ...Array.from(payload))
           .should.equal(true)
-        client2.emit.called.should.equal(false) // disconnected client should not be called
+        client2.emit.calledWith('project:access:revoked').should.equal(true) // disconnected client should get informative message
         client3.emit
           .calledWith(message, ...Array.from(payload))
           .should.equal(true)