diff --git a/services/web/app/coffee/Features/User/UserSessionsManager.coffee b/services/web/app/coffee/Features/User/UserSessionsManager.coffee index af3efd55cd..1e05a576ad 100644 --- a/services/web/app/coffee/Features/User/UserSessionsManager.coffee +++ b/services/web/app/coffee/Features/User/UserSessionsManager.coffee @@ -68,7 +68,8 @@ module.exports = UserSessionsManager = if sessionKeys.length == 0 logger.log {user_id: user._id}, "no other sessions found, returning" return callback(null, []) - rclient.mget sessionKeys, (err, sessions) -> + + Async.map sessionKeys, ((k, cb) -> rclient.get(k, cb)), (err, sessions) -> if err? logger.err {user_id: user._id}, "error getting all sessions for user from redis" return callback(err) @@ -104,12 +105,18 @@ module.exports = UserSessionsManager = logger.log {user_id: user._id}, "no sessions in UserSessions set to delete, returning" return callback(null) logger.log {user_id: user._id, count: keysToDelete.length}, "deleting sessions for user" - rclient.multi() - .del(keysToDelete) - .srem(sessionSetKey, keysToDelete) - .exec (err, result) -> + + deletions = keysToDelete.map (k) -> + (cb) -> + rclient.del k, cb + + Async.series deletions, (err, _result) -> + if err? + logger.err {err, user_id: user._id, sessionSetKey}, "errror revoking all sessions for user" + return callback(err) + rclient.srem sessionSetKey, keysToDelete, (err) -> if err? - logger.err {err, user_id: user._id, sessionSetKey}, "error revoking all sessions for user" + logger.err {err, user_id: user._id, sessionSetKey}, "error removing session set for user" return callback(err) callback(null) diff --git a/services/web/test/UnitTests/coffee/User/UserSessionsManagerTests.coffee b/services/web/test/UnitTests/coffee/User/UserSessionsManagerTests.coffee index a9e8aa6971..8e75be83f5 100644 --- a/services/web/test/UnitTests/coffee/User/UserSessionsManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/User/UserSessionsManagerTests.coffee @@ -4,6 +4,7 @@ should = chai.should() expect = chai.expect modulePath = "../../../../app/js/Features/User/UserSessionsManager.js" SandboxedModule = require('sandboxed-module') +Async = require('async') describe 'UserSessionsManager', -> @@ -45,6 +46,7 @@ describe 'UserSessionsManager', -> "logger-sharelatex": @logger "settings-sharelatex": @settings './UserSessionsRedis': @redis + 'async': Async describe '_sessionSetKey', -> @@ -247,14 +249,14 @@ describe 'UserSessionsManager', -> @_checkSessions.callCount.should.equal 0 done() - ## describe 'revokeAllUserSessions', -> beforeEach -> @sessionKeys = ['sess:one', 'sess:two'] @retain = [] @rclient.smembers.callsArgWith(1, null, @sessionKeys) - @rclient.exec.callsArgWith(0, null) + @rclient.del = sinon.stub().callsArgWith(1, null) + @rclient.srem = sinon.stub().callsArgWith(2, null) @call = (callback) => @UserSessionsManager.revokeAllUserSessions @user, @retain, callback @@ -267,15 +269,14 @@ describe 'UserSessionsManager', -> it 'should call the appropriate redis methods', (done) -> @call (err) => @rclient.smembers.callCount.should.equal 1 - @rclient.multi.callCount.should.equal 1 - @rclient.del.callCount.should.equal 1 - expect(@rclient.del.firstCall.args[0]).to.deep.equal @sessionKeys + @rclient.del.callCount.should.equal 2 + expect(@rclient.del.firstCall.args[0]).to.deep.equal @sessionKeys[0] + expect(@rclient.del.secondCall.args[0]).to.deep.equal @sessionKeys[1] @rclient.srem.callCount.should.equal 1 expect(@rclient.srem.firstCall.args[1]).to.deep.equal @sessionKeys - @rclient.exec.callCount.should.equal 1 done() describe 'when a session is retained', -> @@ -284,7 +285,7 @@ describe 'UserSessionsManager', -> @sessionKeys = ['sess:one', 'sess:two', 'sess:three', 'sess:four'] @retain = ['two'] @rclient.smembers.callsArgWith(1, null, @sessionKeys) - @rclient.exec.callsArgWith(0, null) + @rclient.del = sinon.stub().callsArgWith(1, null) @call = (callback) => @UserSessionsManager.revokeAllUserSessions @user, @retain, callback @@ -297,28 +298,33 @@ describe 'UserSessionsManager', -> it 'should call the appropriate redis methods', (done) -> @call (err) => @rclient.smembers.callCount.should.equal 1 - @rclient.multi.callCount.should.equal 1 - @rclient.del.callCount.should.equal 1 + @rclient.del.callCount.should.equal @sessionKeys.length - 1 @rclient.srem.callCount.should.equal 1 - @rclient.exec.callCount.should.equal 1 done() it 'should remove all sessions except for the retained one', (done) -> @call (err) => - expect(@rclient.del.firstCall.args[0]).to.deep.equal(['sess:one', 'sess:three', 'sess:four']) + expect(@rclient.del.firstCall.args[0]).to.deep.equal('sess:one') + expect(@rclient.del.secondCall.args[0]).to.deep.equal('sess:three') + expect(@rclient.del.thirdCall.args[0]).to.deep.equal('sess:four') expect(@rclient.srem.firstCall.args[1]).to.deep.equal(['sess:one', 'sess:three', 'sess:four']) done() describe 'when rclient produces an error', -> beforeEach -> - @rclient.exec.callsArgWith(0, new Error('woops')) + @rclient.del = sinon.stub().callsArgWith(1, new Error('woops')) it 'should produce an error', (done) -> @call (err) => expect(err).to.be.instanceof Error done() + it 'should not call rclient.srem', (done) -> + @call (err) => + @rclient.srem.callCount.should.equal 0 + done() + describe 'when no user is supplied', -> beforeEach -> @@ -334,10 +340,8 @@ describe 'UserSessionsManager', -> it 'should not call the appropriate redis methods', (done) -> @call (err) => @rclient.smembers.callCount.should.equal 0 - @rclient.multi.callCount.should.equal 0 @rclient.del.callCount.should.equal 0 @rclient.srem.callCount.should.equal 0 - @rclient.exec.callCount.should.equal 0 done() describe 'when there are no keys to delete', -> @@ -354,10 +358,8 @@ describe 'UserSessionsManager', -> it 'should not do the delete operation', (done) -> @call (err) => @rclient.smembers.callCount.should.equal 1 - @rclient.multi.callCount.should.equal 0 @rclient.del.callCount.should.equal 0 @rclient.srem.callCount.should.equal 0 - @rclient.exec.callCount.should.equal 0 done() describe 'touch', -> @@ -415,7 +417,10 @@ describe 'UserSessionsManager', -> ] @exclude = ['two'] @rclient.smembers.callsArgWith(1, null, @sessionKeys) - @rclient.mget.callsArgWith(1, null, @sessions) + @rclient.get = sinon.stub() + @rclient.get.onCall(0).callsArgWith(1, null, @sessions[0]) + @rclient.get.onCall(1).callsArgWith(1, null, @sessions[1]) + @call = (callback) => @UserSessionsManager.getAllUserSessions @user, @exclude, callback @@ -437,9 +442,9 @@ describe 'UserSessionsManager', -> @rclient.smembers.callCount.should.equal 1 done() - it 'should have called rclient.mget', (done) -> + it 'should have called rclient.get', (done) -> @call (err, sessions) => - @rclient.mget.callCount.should.equal 1 + @rclient.get.callCount.should.equal @sessionKeys.length - 1 done() describe 'when there are no other sessions', -> @@ -484,10 +489,10 @@ describe 'UserSessionsManager', -> @rclient.mget.callCount.should.equal 0 done() - describe 'when mget produces an error', -> + describe 'when get produces an error', -> beforeEach -> - @rclient.mget.callsArgWith(1, new Error('woops')) + @rclient.get = sinon.stub().callsArgWith(1, new Error('woops')) it 'should produce an error', (done) -> @call (err, sessions) => diff --git a/services/web/test/acceptance/coffee/helpers/redis.coffee b/services/web/test/acceptance/coffee/helpers/redis.coffee index 2611e4fc57..dd98a39b59 100644 --- a/services/web/test/acceptance/coffee/helpers/redis.coffee +++ b/services/web/test/acceptance/coffee/helpers/redis.coffee @@ -3,25 +3,32 @@ redis = require('redis-sharelatex') logger = require("logger-sharelatex") Async = require('async') -rclient = redis.createClient(Settings.redis.web) +UserSessionsRedis = require('../../../../app/js/Features/User/UserSessionsRedis') + +# rclient = redis.createClient(Settings.redis.web) +rclient = UserSessionsRedis.client() module.exports = getUserSessions: (user, callback=(err, sessionsSet)->) -> - rclient.smembers "UserSessions:#{user._id}", (err, result) -> + rclient.smembers "UserSessions:{#{user._id}}", (err, result) -> return callback(err, result) clearUserSessions: (user, callback=(err)->) -> - sessionSetKey = "UserSessions:#{user._id}" + sessionSetKey = "UserSessions:{#{user._id}}" rclient.smembers sessionSetKey, (err, sessionKeys) -> if err return callback(err) if sessionKeys.length == 0 return callback(null) - rclient.multi() - .del(sessionKeys) - .srem(sessionSetKey, sessionKeys) - .exec (err, result) -> - if err - return callback(err) - callback(null) + actions = sessionKeys.map (k) -> + (cb) -> + rclient.del k, (err) -> + cb(err) + Async.series( + actions, (err, results) -> + rclient.srem sessionSetKey, sessionKeys, (err) -> + if err + return callback(err) + callback(null) + )