refactor(realtime): extract user event "permission" to SocketClient

Signed-off-by: BoHong Li <a60814billy@gmail.com>

lib/realtime.js

@@ -805,15 +805,29 @@ class SocketClient {
     // reveiced when user logout or changed
     this.socket.on('user changed', this.userChangedEventHandler.bind(this))
     // delete a note
-    this.socket.on('delete', this.deleteNote.bind(this))
+    this.socket.on('delete', this.deleteNoteEventHandler.bind(this))
+    // received note permission change request
+    this.socket.on('permission', this.permissionChangeEventHandler.bind(this))
   }
 
   isUserLoggedIn () {
     return this.socket.request.user && this.socket.request.user.logged_in
   }
 
-  getCurrentLoggedInUserId () {
+  isNoteAndUserExists () {
-    return get(this.socket, 'request.user.id')
+    const note = getNoteFromNotePool(this.socket.noteId)
+    const user = getUserFromUserPool(this.socket.id)
+    return note && user
+  }
+
+  isNoteOwner () {
+    const note = this.getCurrentNote()
+    return get(note, 'owner') === this.getCurrentLoggedInUserId()
+  }
+
+  isAnonymousEnable () {
+    //TODO: move this method to config module
+    return config.allowAnonymous || config.allowAnonymousEdits
   }
 
   disconnectSocketOnNote (note) {
@@ -827,28 +841,83 @@ class SocketClient {
     })
   }
 
+  getCurrentUser () {
+    if (!this.socket.id) return
+    return getUserFromUserPool(this.socket.id)
+  }
+
+  getCurrentLoggedInUserId () {
+    return get(this.socket, 'request.user.id')
+  }
+
+  getCurrentNote () {
+    if (!this.socket.noteId) return
+    return getNoteFromNotePool(this.socket.noteId)
+  }
+
+  getNoteChannel () {
+    return this.socket.broadcast.to(this.socket.noteId)
+  }
+
   async destroyNote (id) {
     return models.Note.destroy({
       where: { id: id }
     })
   }
 
-  deleteNote () {
+  async changeNotePermission (newPermission) {
-    // need login to do more actions
+    const changedRows = await models.Note.update({
-    if (this.isUserLoggedIn() && this.isNoteAndUserExists()) {
+      permission: newPermission
-      const note = this.getCurrentNote()
+    }, {
-      // Only owner can delete note
+      where: {
-      if (note.owner && note.owner === this.getCurrentLoggedInUserId()) {
+        id: this.getCurrentNote().id
-        this.destroyNote(note.id)
+      }
-          .then((successRows) => {
-            if (!successRows) return
-            this.disconnectSocketOnNote(note)
-          })
-          .catch(function (err) {
-            return logger.error('delete note failed: ' + err)
     })
+    if (changedRows !== 1) {
+      throw new Error(`update database failed, cannot set permission ${newPermission} to note ${this.getCurrentNote().id}`)
     }
   }
+
+  notifyPermissionChanged () {
+    realtime.io.to(this.getCurrentNote().id).emit('permission', {
+      permission: this.getCurrentNote().permission
+    })
+    this.getCurrentNote().socks.forEach((sock) => {
+      if (sock) {
+        if (!exports.checkViewPermission(sock.request, this.getCurrentNote())) {
+          sock.emit('info', {
+            code: 403
+          })
+          setTimeout(function () {
+            sock.disconnect(true)
+          }, 0)
+        }
+      }
+    })
+  }
+
+  refreshEventHandler () {
+    exports.emitRefresh(this.socket)
+  }
+
+  checkVersionEventHandler () {
+    this.socket.emit('version', {
+      version: config.fullversion,
+      minimumCompatibleVersion: config.minimumCompatibleVersion
+    })
+  }
+
+  userStatusEventHandler (data) {
+    if (!this.isNoteAndUserExists()) return
+    const user = this.getCurrentUser()
+    if (config.debug) {
+      logger.info('SERVER received [' + this.socket.noteId + '] user status from [' + this.socket.id + ']: ' + JSON.stringify(data))
+    }
+    if (data) {
+      user.idle = data.idle
+      user.type = data.type
+    }
+    exports.emitUserStatus(this.socket)
   }
 
   userChangedEventHandler () {
@@ -863,26 +932,6 @@ class SocketClient {
     exports.emitOnlineUsers(this.socket)
   }
 
-  getCurrentUser () {
-    if (!this.socket.id) return
-    return getUserFromUserPool(this.socket.id)
-  }
-
-  getCurrentNote () {
-    if (!this.socket.noteId) return
-    return getNoteFromNotePool(this.socket.noteId)
-  }
-
-  getNoteChannel () {
-    return this.socket.broadcast.to(this.socket.noteId)
-  }
-
-  isNoteAndUserExists () {
-    const note = getNoteFromNotePool(this.socket.noteId)
-    const user = getUserFromUserPool(this.socket.id)
-    return note && user
-  }
-
   onlineUsersEventHandler () {
     if (!this.isNoteAndUserExists()) return
 
@@ -921,28 +970,40 @@ class SocketClient {
     })
   }
 
-  checkVersionEventHandler () {
+  deleteNoteEventHandler () {
-    this.socket.emit('version', {
+    // need login to do more actions
-      version: config.fullversion,
+    if (this.isUserLoggedIn() && this.isNoteAndUserExists()) {
-      minimumCompatibleVersion: config.minimumCompatibleVersion
+      const note = this.getCurrentNote()
+      // Only owner can delete note
+      if (note.owner && note.owner === this.getCurrentLoggedInUserId()) {
+        this.destroyNote(note.id)
+          .then((successRows) => {
+            if (!successRows) return
+            this.disconnectSocketOnNote(note)
+          })
+          .catch(function (err) {
+            return logger.error('delete note failed: ' + err)
           })
       }
-
+    }
-  refreshEventHandler () {
-    exports.emitRefresh(this.socket)
   }
 
-  userStatusEventHandler (data) {
+  permissionChangeEventHandler (permission) {
+    if (!this.isUserLoggedIn()) return
     if (!this.isNoteAndUserExists()) return
-    const user = this.getCurrentUser()
+
-    if (config.debug) {
+    const note = this.getCurrentNote()
-      logger.info('SERVER received [' + this.socket.noteId + '] user status from [' + this.socket.id + ']: ' + JSON.stringify(data))
+    // Only owner can change permission
-    }
+    if (!this.isNoteOwner()) return
-    if (data) {
+    if (!this.isAnonymousEnable() && permission === 'freely') return
-      user.idle = data.idle
+
-      user.type = data.type
+    this.changeNotePermission(permission)
-    }
+      .then(() => {
-    exports.emitUserStatus(this.socket)
+        console.log('---')
+        note.permission = permission
+        this.notifyPermissionChanged()
+      })
+      .catch(err => logger.error('update note permission failed: ' + err))
   }
 
   disconnectEventHandler () {
@@ -1009,52 +1070,6 @@ function connection (socket) {
 
   const socketClient = new SocketClient(socket)
   socketClient.registerEventHandler()
-
-  // received note permission change request
-  socket.on('permission', function (permission) {
-    // need login to do more actions
-    if (socket.request.user && socket.request.user.logged_in) {
-      var noteId = socket.noteId
-      if (!noteId || !notes[noteId]) return
-      var note = notes[noteId]
-      // Only owner can change permission
-      if (note.owner && note.owner === socket.request.user.id) {
-        if (permission === 'freely' && !config.allowAnonymous && !config.allowAnonymousEdits) return
-        note.permission = permission
-        models.Note.update({
-          permission: permission
-        }, {
-          where: {
-            id: noteId
-          }
-        }).then(function (count) {
-          if (!count) {
-            return
-          }
-          var out = {
-            permission: permission
-          }
-          realtime.io.to(note.id).emit('permission', out)
-          for (var i = 0, l = note.socks.length; i < l; i++) {
-            var sock = note.socks[i]
-            if (typeof sock !== 'undefined' && sock) {
-              // check view permission
-              if (!checkViewPermission(sock.request, note)) {
-                sock.emit('info', {
-                  code: 403
-                })
-                setTimeout(function () {
-                  sock.disconnect(true)
-                }, 0)
-              }
-            }
-          }
-        }).catch(function (err) {
-          return logger.error('update note permission failed: ' + err)
-        })
-      }
-    }
-  })
 }
 
 exports = module.exports = realtime
@@ -1070,6 +1085,7 @@ exports.emitRefresh = emitRefresh
 exports.emitUserStatus = emitUserStatus
 exports.disconnect = disconnect
 exports.emitOnlineUsers = emitOnlineUsers
+exports.checkViewPermission = checkViewPermission
 exports.notes = notes
 exports.users = users
 exports.disconnectSocketQueue = disconnectSocketQueue

test/connectionQueue.test.js

@@ -78,7 +78,7 @@ describe('ConnectionQueue', function () {
     setTimeout(() => {
       assert(processSpy.called)
       done()
-    }, 1)
+    }, 10)
   })
 
   it('should run process although error occurred', (done) => {
@@ -100,7 +100,7 @@ describe('ConnectionQueue', function () {
       assert(failedTask.called)
       assert(normalTask.called)
       done()
-    }, 5)
+    }, 10)
   })
 
   it('should ignore trigger when event not complete', (done) => {
@@ -125,6 +125,6 @@ describe('ConnectionQueue', function () {
     setTimeout(() => {
       assert(processSpy.calledOnce)
       done()
-    }, 2)
+    }, 10)
   })
 })

test/realtime/realtime.test.js

@@ -258,4 +258,68 @@ describe('realtime', function () {
     })
   })
 
+  describe('checkViewPermission', function () {
+    // role -> guest, loggedInUser, loggedInOwner
+    const viewPermission = {
+      freely: [true, true, true],
+      editable: [true, true, true],
+      limited: [false, true, true],
+      locked: [true, true, true],
+      protected: [false, true, true],
+      private: [false, false, true]
+    }
+    const loggedInUserId = 'user1_id'
+    const ownerUserId = 'user2_id'
+    const guestReq = {}
+    const loggedInUserReq = {
+      user: {
+        id: loggedInUserId,
+        logged_in: true
+      }
+    }
+    const loggedInOwnerReq = {
+      user: {
+        id: ownerUserId,
+        logged_in: true
+      }
+    }
+
+    const note = {
+      owner: ownerUserId
+    }
+
+    let realtime
+
+    beforeEach(() => {
+      mock('../../lib/logger', {
+        error: () => {
+        }
+      })
+      mock('../../lib/history', {})
+      mock('../../lib/models', {
+        Note: {
+          parseNoteTitle: (data) => (data)
+        }
+      })
+      mock('../../lib/config', {})
+      realtime = require('../../lib/realtime')
+    })
+
+    Object.keys(viewPermission).forEach(function (permission) {
+      describe(permission, function () {
+        beforeEach(() => {
+          note.permission = permission
+        })
+        it('guest permission test', function () {
+          assert(realtime.checkViewPermission(guestReq, note) === viewPermission[permission][0])
+        })
+        it('loggedIn User permission test', function () {
+          assert(realtime.checkViewPermission(loggedInUserReq, note) === viewPermission[permission][1])
+        })
+        it('loggedIn Owner permission test', function () {
+          assert(realtime.checkViewPermission(loggedInOwnerReq, note) === viewPermission[permission][2])
+        })
+      })
+    })
+  })
 })

test/realtime/socket-events.test.js

@@ -13,15 +13,21 @@ describe('realtime#socket event', function () {
   let clientSocket
   let modelsMock
   let eventFuncMap
+  let configMock
 
   beforeEach(function () {
     eventFuncMap = new Map()
     modelsMock = {
       Note: {
         parseNoteTitle: (data) => (data),
-        destroy: sinon.stub().returns(Promise.resolve(1))
+        destroy: sinon.stub().returns(Promise.resolve(1)),
+        update: sinon.stub().returns(Promise.resolve(1))
       }
     }
+    configMock = {
+      fullversion: '1.5.0',
+      minimumCompatibleVersion: '1.0.0'
+    }
     mock('../../lib/logger', {
       error: () => {
       },
@@ -30,10 +36,7 @@ describe('realtime#socket event', function () {
     })
     mock('../../lib/history', {})
     mock('../../lib/models', modelsMock)
-    mock('../../lib/config', {
+    mock('../../lib/config', configMock)
-      fullversion: '1.5.0',
-      minimumCompatibleVersion: '1.0.0'
-    })
     realtime = require('../../lib/realtime')
 
     // get all socket event handler
@@ -383,4 +386,151 @@ describe('realtime#socket event', function () {
       }, 10)
     })
   })
+
+  describe('permission', function () {
+    let ownerId = 'user1_id'
+    let otherSignInUserId = 'user2_id'
+    let otherClient
+    let checkViewPermissionSpy
+    let permissionFunc
+
+    beforeEach(function () {
+      otherClient = makeMockSocket()
+      clientSocket.request = {
+        user: {
+          id: ownerId,
+          logged_in: true
+        }
+      }
+
+      otherClient.request = {
+        user: {
+          id: otherSignInUserId,
+          logged_in: true
+        }
+      }
+
+      realtime.notes[noteId] = {
+        owner: ownerId
+      }
+
+      realtime.io = {
+        to: function () {
+          return {
+            emit: sinon.stub()
+          }
+        }
+      }
+
+      checkViewPermissionSpy = sinon.spy(realtime, 'checkViewPermission')
+      permissionFunc = eventFuncMap.get('permission')
+    })
+
+    it('should disconnect when lose view permission', function (done) {
+      realtime.notes[noteId].permission = 'editable'
+      realtime.notes[noteId].socks = [clientSocket, undefined, otherClient]
+
+      permissionFunc('private')
+
+      setTimeout(() => {
+        assert(checkViewPermissionSpy.callCount === 2)
+        assert(otherClient.emit.calledOnce)
+        assert(otherClient.disconnect.calledOnce)
+        done()
+      }, 5)
+    })
+
+    it('should not do anything when user not logged in', function (done) {
+      clientSocket.request = {}
+      permissionFunc('private')
+      setTimeout(() => {
+        assert(modelsMock.Note.update.called === false)
+        done()
+      }, 5)
+    })
+
+    it('should not do anything when note not exists', function (done) {
+      delete realtime.notes[noteId]
+      permissionFunc('private')
+      setTimeout(() => {
+        assert(modelsMock.Note.update.called === false)
+        done()
+      }, 5)
+    })
+
+    it('should not do anything when not note owner', function (done) {
+      clientSocket.request.user.id = 'other_user_id'
+      permissionFunc('private')
+      setTimeout(() => {
+        assert(modelsMock.Note.update.called === false)
+        done()
+      }, 5)
+    })
+
+    it('should change permission to freely when config allowAnonymous and allowAnonymousEdits are true', function (done) {
+      configMock.allowAnonymous = true
+      configMock.allowAnonymousEdits = true
+      realtime.notes[noteId].socks = [clientSocket, undefined, otherClient]
+
+      permissionFunc('freely')
+
+      setTimeout(() => {
+        assert(checkViewPermissionSpy.callCount === 2)
+        assert(otherClient.emit.called === false)
+        assert(otherClient.disconnect.called === false)
+        assert(clientSocket.emit.called === false)
+        assert(clientSocket.disconnect.called === false)
+        done()
+      }, 5)
+    })
+
+    it('should not change permission to freely when config allowAnonymous and allowAnonymousEdits are false', function (done) {
+      configMock.allowAnonymous = false
+      configMock.allowAnonymousEdits = false
+      realtime.notes[noteId].socks = [clientSocket, undefined, otherClient]
+
+      permissionFunc('freely')
+
+      setTimeout(() => {
+        assert(modelsMock.Note.update.called === false)
+        assert(checkViewPermissionSpy.called === false)
+        done()
+      }, 5)
+    })
+
+    it('should change permission to freely when config allowAnonymous is true', function (done) {
+      configMock.allowAnonymous = true
+      configMock.allowAnonymousEdits = false
+      realtime.notes[noteId].socks = [clientSocket, undefined, otherClient]
+
+      permissionFunc('freely')
+
+      setTimeout(() => {
+        assert(checkViewPermissionSpy.callCount === 2)
+        assert(otherClient.emit.called === false)
+        assert(otherClient.disconnect.called === false)
+        assert(clientSocket.emit.called === false)
+        assert(clientSocket.disconnect.called === false)
+        done()
+      }, 5)
+    })
+
+    it('should change permission to freely when config allowAnonymousEdits is true', function (done) {
+      configMock.allowAnonymous = false
+      configMock.allowAnonymousEdits = true
+      realtime.notes[noteId].socks = [clientSocket, undefined, otherClient]
+
+      permissionFunc('freely')
+
+      setTimeout(() => {
+        assert(checkViewPermissionSpy.callCount === 2)
+        assert(otherClient.emit.called === false)
+        assert(otherClient.disconnect.called === false)
+        assert(clientSocket.emit.called === false)
+        assert(clientSocket.disconnect.called === false)
+        done()
+      }, 5)
+    })
+
+  })
 })