From f317171aa052e9bc171b124abb0f0c528b49d996 Mon Sep 17 00:00:00 2001 From: BoHong Li Date: Mon, 6 May 2019 15:14:40 +0800 Subject: [PATCH] refactor(realtime): extract user event "permission" to SocketClient Signed-off-by: BoHong Li --- lib/realtime.js | 220 +++++++++++++++------------- test/connectionQueue.test.js | 6 +- test/realtime/realtime.test.js | 64 ++++++++ test/realtime/socket-events.test.js | 160 +++++++++++++++++++- 4 files changed, 340 insertions(+), 110 deletions(-) diff --git a/lib/realtime.js b/lib/realtime.js index f24d9445..16307920 100644 --- a/lib/realtime.js +++ b/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 () { - return get(this.socket, 'request.user.id') + isNoteAndUserExists () { + 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,30 +841,85 @@ 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 () { - // need login to do more actions - if (this.isUserLoggedIn() && this.isNoteAndUserExists()) { - 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) - }) + async changeNotePermission (newPermission) { + const changedRows = await models.Note.update({ + permission: newPermission + }, { + where: { + id: this.getCurrentNote().id } + }) + 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 () { logger.info('user changed') @@ -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 () { - this.socket.emit('version', { - version: config.fullversion, - minimumCompatibleVersion: config.minimumCompatibleVersion - }) + deleteNoteEventHandler () { + // need login to do more actions + if (this.isUserLoggedIn() && this.isNoteAndUserExists()) { + 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) { - 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) + + const note = this.getCurrentNote() + // Only owner can change permission + if (!this.isNoteOwner()) return + if (!this.isAnonymousEnable() && permission === 'freely') return + + this.changeNotePermission(permission) + .then(() => { + 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 diff --git a/test/connectionQueue.test.js b/test/connectionQueue.test.js index 589d865b..58c6f820 100644 --- a/test/connectionQueue.test.js +++ b/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) }) }) diff --git a/test/realtime/realtime.test.js b/test/realtime/realtime.test.js index c6f4276d..3bd7fcaa 100644 --- a/test/realtime/realtime.test.js +++ b/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]) + }) + }) + }) + }) }) diff --git a/test/realtime/socket-events.test.js b/test/realtime/socket-events.test.js index 602f83ed..96256c48 100644 --- a/test/realtime/socket-events.test.js +++ b/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', { - fullversion: '1.5.0', - minimumCompatibleVersion: '1.0.0' - }) + mock('../../lib/config', configMock) 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) + }) + + }) })