refactor(realtime): ifMayEdit

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

lib/realtime.js

@@ -4,7 +4,6 @@
 const cookie = require('cookie')
 const cookieParser = require('cookie-parser')
 const url = require('url')
-const async = require('async')
 const randomcolor = require('randomcolor')
 const Chance = require('chance')
 const chance = new Chance()
@@ -44,14 +43,17 @@ const updateDirtyNoteJob = new UpdateDirtyNoteJob(realtime)
 const cleanDanglingUserJob = new CleanDanglingUserJob(realtime)
 const saveRevisionJob = new SaveRevisionJob(realtime)
 
+// TODO: test it
 function onAuthorizeSuccess (data, accept) {
   accept()
 }
 
+// TODO: test it
 function onAuthorizeFail (data, message, error, accept) {
   accept() // accept whether authorize or not to allow anonymous usage
 }
 
+// TODO: test it
 // secure the origin by the cookie
 function secure (socket, next) {
   try {
@@ -78,6 +80,7 @@ function secure (socket, next) {
 }
 
 // TODO: only use in `updateDirtyNote`
+// TODO: test it
 function emitCheck (note) {
   var out = {
     title: note.title,
@@ -202,6 +205,7 @@ async function _updateNoteAsync (note) {
   return noteModel
 }
 
+// TODO: test it
 function getStatus (callback) {
   models.Note.count().then(function (notecount) {
     var distinctaddresses = []
@@ -257,6 +261,7 @@ function getStatus (callback) {
   })
 }
 
+// TODO: test it
 function isReady () {
   return realtime.io &&
     Object.keys(notes).length === 0 && Object.keys(users).length === 0 &&
@@ -322,6 +327,7 @@ function parseNoteIdFromSocket (socket, callback) {
   })
 }
 
+// TODO: test it
 function emitOnlineUsers (socket) {
   var noteId = socket.noteId
   if (!noteId || !notes[noteId]) return
@@ -338,6 +344,7 @@ function emitOnlineUsers (socket) {
   realtime.io.to(noteId).emit('online users', out)
 }
 
+// TODO: test it
 function emitUserStatus (socket) {
   var noteId = socket.noteId
   var user = users[socket.id]
@@ -346,6 +353,7 @@ function emitUserStatus (socket) {
   socket.broadcast.to(noteId).emit('user status', out)
 }
 
+// TODO: test it
 function emitRefresh (socket) {
   var noteId = socket.noteId
   if (!noteId || !notes[noteId]) return
@@ -366,6 +374,7 @@ function emitRefresh (socket) {
   socket.emit('refresh', out)
 }
 
+// TODO: test it
 function isDuplicatedInSocketQueue (queue, socket) {
   for (var i = 0; i < queue.length; i++) {
     if (queue[i] && queue[i].id === socket.id) {
@@ -375,6 +384,7 @@ function isDuplicatedInSocketQueue (queue, socket) {
   return false
 }
 
+// TODO: test it
 function clearSocketQueue (queue, socket) {
   for (var i = 0; i < queue.length; i++) {
     if (!queue[i] || queue[i].id === socket.id) {
@@ -384,6 +394,7 @@ function clearSocketQueue (queue, socket) {
   }
 }
 
+// TODO: test it
 function connectNextSocket () {
   setTimeout(function () {
     isConnectionBusy = false
@@ -393,6 +404,7 @@ function connectNextSocket () {
   }, 1)
 }
 
+// TODO: test it
 function interruptConnection (socket, noteId, socketId) {
   if (notes[noteId]) delete notes[noteId]
   if (users[socketId]) delete users[socketId]
@@ -425,6 +437,7 @@ function checkViewPermission (req, note) {
 var isConnectionBusy = false
 var connectionSocketQueue = []
 
+// TODO: test it
 function finishConnection (socket, noteId, socketId) {
   // if no valid info provided will drop the client
   if (!socket || !notes[noteId] || !users[socketId]) {
@@ -469,6 +482,7 @@ function finishConnection (socket, noteId, socketId) {
   }
 }
 
+// TODO: test it
 function startConnection (socket) {
   if (isConnectionBusy) return
   isConnectionBusy = true
@@ -556,6 +570,7 @@ function startConnection (socket) {
   }
 }
 
+// TODO: test it
 function failConnection (code, err, socket) {
   logger.error(err)
   // clear error socket in queue
@@ -624,6 +639,7 @@ function buildUserOutData (user) {
   return out
 }
 
+// TODO: test it
 function updateUserData (socket, user) {
   // retrieve user data from passport
   if (socket.request.user && socket.request.user.logged_in) {
@@ -639,31 +655,26 @@ function updateUserData (socket, user) {
   }
 }
 
-function ifMayEdit (socket, callback) {
-  var noteId = socket.noteId
-  if (!noteId || !notes[noteId]) return
-  var note = notes[noteId]
-  var mayEdit = true
-  switch (note.permission) {
+function canEditNote(notePermission, noteOwnerId, currentUserId) {
+  switch (notePermission) {
     case 'freely':
-      // not blocking anyone
-      break
+      return true
     case 'editable':
     case 'limited':
       // only login user can change
-      if (!socket.request.user || !socket.request.user.logged_in) {
-        mayEdit = false
-      }
-      break
+      return !!currentUserId
     case 'locked':
     case 'private':
     case 'protected':
       // only owner can change
-      if (!note.owner || note.owner !== socket.request.user.id) {
-        mayEdit = false
+      return noteOwnerId === currentUserId
   }
-      break
 }
+
+function ifMayEdit (socket, callback) {
+  const note = getNoteFromNotePool(socket.noteId)
+  if (!note) return
+  const mayEdit = canEditNote(note.permission, note.owner, socket.request.user.id)
   // if user may edit and this is a text operation
   if (socket.origin === 'operation' && mayEdit) {
     // save for the last change user id
@@ -676,6 +687,7 @@ function ifMayEdit (socket, callback) {
   return callback(mayEdit)
 }
 
+// TODO: test it
 function operationCallback (socket, operation) {
   var noteId = socket.noteId
   if (!noteId || !notes[noteId]) return
@@ -718,6 +730,7 @@ function operationCallback (socket, operation) {
   })
 }
 
+// TODO: test it
 function updateHistory (userId, note, time) {
   var noteId = note.alias ? note.alias : models.Note.encodeNoteId(note.id)
   if (note.server) history.updateHistory(userId, noteId, note.server.document, time)
@@ -739,6 +752,7 @@ function getNoteFromNotePool (noteId) {
   return notes[noteId]
 }
 
+// TODO: test it
 function connection (socket) {
   if (realtime.maintenance) return
   exports.parseNoteIdFromSocket(socket, function (err, noteId) {
@@ -798,6 +812,7 @@ function connection (socket) {
   socketClient.registerEventHandler()
 }
 
+// TODO: test it
 function terminate () {
   disconnectProcessQueue.stop()
   updateDirtyNoteJob.stop()
@@ -825,6 +840,7 @@ exports.queueForDisconnect = queueForDisconnect
 exports.terminate = terminate
 exports.getUserPool = getUserPool
 exports.updateHistory = updateHistory
+exports.ifMayEdit = ifMayEdit
 exports.disconnectProcessQueue = disconnectProcessQueue
 exports.notes = notes
 exports.users = users

test/realtime/ifMayEdit.test.js

@@ -0,0 +1,126 @@
+/* eslint-env node, mocha */
+'use strict'
+
+const assert = require('assert')
+const mock = require('mock-require')
+const sinon = require('sinon')
+
+const { createFakeLogger } = require('../testDoubles/loggerFake')
+const realtimeJobStub = require('../testDoubles/realtimeJobStub')
+const { removeLibModuleCache, makeMockSocket } = require('./utils')
+
+describe('realtime#ifMayEdit', function () {
+  let modelsStub
+  beforeEach(() => {
+    removeLibModuleCache()
+    mock('../../lib/config', {})
+    mock('../../lib/logger', createFakeLogger())
+    mock('../../lib/models', modelsStub)
+    mock('../../lib/realtimeUpdateDirtyNoteJob', realtimeJobStub)
+    mock('../../lib/realtimeCleanDanglingUserJob', realtimeJobStub)
+    mock('../../lib/realtimeSaveRevisionJob', realtimeJobStub)
+  })
+
+  afterEach(() => {
+    mock.stopAll()
+    sinon.restore()
+  })
+
+  const Role = {
+    Guest: 'guest',
+    LoggedIn: 'LoggedIn',
+    Owner: 'Owner'
+  }
+
+  const Permission = {
+    Freely: 'freely',
+    Editable: 'editable',
+    Limited: 'limited',
+    Locked: 'locked',
+    Protected: 'protected',
+    Private: 'private'
+  }
+
+  const testcases = [
+    { role: Role.Guest, permission: Permission.Freely, canEdit: true },
+    { role: Role.LoggedIn, permission: Permission.Freely, canEdit: true },
+    { role: Role.Owner, permission: Permission.Freely, canEdit: true },
+    { role: Role.Guest, permission: Permission.Editable, canEdit: false },
+    { role: Role.LoggedIn, permission: Permission.Editable, canEdit: true },
+    { role: Role.Owner, permission: Permission.Editable, canEdit: true },
+    { role: Role.Guest, permission: Permission.Limited, canEdit: false },
+    { role: Role.LoggedIn, permission: Permission.Limited, canEdit: true },
+    { role: Role.Owner, permission: Permission.Limited, canEdit: true },
+    { role: Role.Guest, permission: Permission.Locked, canEdit: false },
+    { role: Role.LoggedIn, permission: Permission.Locked, canEdit: false },
+    { role: Role.Owner, permission: Permission.Locked, canEdit: true },
+    { role: Role.Guest, permission: Permission.Protected, canEdit: false},
+    { role: Role.LoggedIn, permission: Permission.Protected, canEdit: false },
+    { role: Role.Owner, permission: Permission.Protected, canEdit: true },
+    { role: Role.Guest, permission: Permission.Private, canEdit: false },
+    { role: Role.LoggedIn, permission: Permission.Private, canEdit: false },
+    { role: Role.Owner, permission: Permission.Private, canEdit: true }
+  ]
+
+  const noteOwnerId = 'owner'
+  const loggedInUserId = 'user1'
+  const noteId = 'noteId'
+
+  testcases.forEach((tc) => {
+    it(`${tc.role} ${tc.canEdit ? 'can' : 'can\'t'} edit note with permission ${tc.permission}`, function () {
+      const client = makeMockSocket()
+      const note = {
+        permission: tc.permission,
+        owner: noteOwnerId
+      }
+      if (tc.role === Role.LoggedIn) {
+        client.request.user.logged_in = true
+        client.request.user.id = loggedInUserId
+      } else if (tc.role === Role.Owner) {
+        client.request.user.logged_in = true
+        client.request.user.id = noteOwnerId
+      }
+      client.noteId = noteId
+      const realtime = require('../../lib/realtime')
+      realtime.getNotePool()[noteId] = note
+      const callback = sinon.stub()
+      realtime.ifMayEdit(client, callback)
+      assert(callback.calledOnce)
+      assert(callback.lastCall.args[0] === tc.canEdit)
+    })
+  })
+
+  it('should set lsatchangeuser to null if guest edit operation', function () {
+    const note = {
+      permission: Permission.Freely
+    }
+    const client = makeMockSocket()
+    client.noteId = noteId
+    const callback = sinon.stub()
+    client.origin = 'operation'
+    const realtime = require('../../lib/realtime')
+    realtime.getNotePool()[noteId] = note
+    realtime.ifMayEdit(client, callback)
+    assert(callback.calledOnce)
+    assert(callback.lastCall.args[0])
+    assert(note.lastchangeuser === null)
+  })
+
+  it('should set lastchangeuser to logged_in user id if user edit', function () {
+    const note = {
+      permission: Permission.Freely
+    }
+    const client = makeMockSocket()
+    client.noteId = noteId
+    client.request.user.logged_in = true
+    client.request.user.id = loggedInUserId
+    const callback = sinon.stub()
+    client.origin = 'operation'
+    const realtime = require('../../lib/realtime')
+    realtime.getNotePool()[noteId] = note
+    realtime.ifMayEdit(client, callback)
+    assert(callback.calledOnce)
+    assert(callback.lastCall.args[0])
+    assert(note.lastchangeuser === loggedInUserId)
+  })
+})

test/realtime/utils.js

@@ -7,6 +7,9 @@ function makeMockSocket (headers, query) {
   const broadCastChannelCache = {}
   return {
     id: Math.round(Math.random() * 10000),
+    request: {
+      user: {}
+    },
     handshake: {
       headers: Object.assign({}, headers),
       query: Object.assign({}, query)