Explorar o código

Refactor method names in `UserSessionsManager`

Shane Kilkelly %!s(int64=10) %!d(string=hai) anos
pai
achega
f1653d01b7

+ 5 - 5
services/web/app/coffee/Features/Authentication/AuthenticationController.coffee

@@ -14,7 +14,7 @@ UserSessionsManager = require("../User/UserSessionsManager")
 module.exports = AuthenticationController =
 module.exports = AuthenticationController =
 	login: (req, res, next = (error) ->) ->
 	login: (req, res, next = (error) ->) ->
 		AuthenticationController.doLogin req.body, req, res, next
 		AuthenticationController.doLogin req.body, req, res, next
-	
+
 	doLogin: (options, req, res, next) ->
 	doLogin: (options, req, res, next) ->
 		email = options.email?.toLowerCase()
 		email = options.email?.toLowerCase()
 		password = options.password
 		password = options.password
@@ -62,7 +62,7 @@ module.exports = AuthenticationController =
 	requireLogin: () ->
 	requireLogin: () ->
 		doRequest = (req, res, next = (error) ->) ->
 		doRequest = (req, res, next = (error) ->) ->
 			if !req.session.user?
 			if !req.session.user?
-				AuthenticationController._redirectToLoginOrRegisterPage(req, res) 
+				AuthenticationController._redirectToLoginOrRegisterPage(req, res)
 			else
 			else
 				req.user = req.session.user
 				req.user = req.session.user
 				return next()
 				return next()
@@ -93,9 +93,9 @@ module.exports = AuthenticationController =
 
 
 	_redirectToLoginOrRegisterPage: (req, res)->
 	_redirectToLoginOrRegisterPage: (req, res)->
 		if req.query.zipUrl? or req.query.project_name?
 		if req.query.zipUrl? or req.query.project_name?
-			return AuthenticationController._redirectToRegisterPage(req, res) 
+			return AuthenticationController._redirectToRegisterPage(req, res)
 		else
 		else
-			AuthenticationController._redirectToLoginPage(req, res) 
+			AuthenticationController._redirectToLoginPage(req, res)
 
 
 
 
 	_redirectToLoginPage: (req, res) ->
 	_redirectToLoginPage: (req, res) ->
@@ -143,5 +143,5 @@ module.exports = AuthenticationController =
 
 
 		req.session.user = lightUser
 		req.session.user = lightUser
 
 
-		UserSessionsManager.onLogin(user, req.sessionID, () ->)
+		UserSessionsManager.trackSession(user, req.sessionID, () ->)
 		callback()
 		callback()

+ 2 - 2
services/web/app/coffee/Features/User/UserController.coffee

@@ -87,7 +87,7 @@ module.exports = UserController =
 		req.session.destroy (err)->
 		req.session.destroy (err)->
 			if err
 			if err
 				logger.err err: err, 'error destorying session'
 				logger.err err: err, 'error destorying session'
-			UserSessionsManager.onLogout(user, sessionId)
+			UserSessionsManager.untrackSession(user, sessionId)
 			res.redirect '/login'
 			res.redirect '/login'
 
 
 	register : (req, res, next = (error) ->)->
 	register : (req, res, next = (error) ->)->
@@ -121,7 +121,7 @@ module.exports = UserController =
 					logger.log user: user, "password changed"
 					logger.log user: user, "password changed"
 					AuthenticationManager.setUserPassword user._id, newPassword1, (error) ->
 					AuthenticationManager.setUserPassword user._id, newPassword1, (error) ->
 						return next(error) if error?
 						return next(error) if error?
-						UserSessionsManager.revokeAllSessions user, (err) ->
+						UserSessionsManager.revokeAllUserSessions user, (err) ->
 							return next(err) if err
 							return next(err) if err
 							res.send
 							res.send
 								message:
 								message:

+ 3 - 3
services/web/app/coffee/Features/User/UserSessionsManager.coffee

@@ -12,7 +12,7 @@ module.exports = UserSessionsManager =
 	_sessionKey: (sessionId) ->
 	_sessionKey: (sessionId) ->
 		return "sess:#{sessionId}"
 		return "sess:#{sessionId}"
 
 
-	onLogin: (user, sessionId, callback=(err)-> ) ->
+	trackSession: (user, sessionId, callback=(err)-> ) ->
 		logger.log {user_id: user._id, sessionId}, "onLogin handler"
 		logger.log {user_id: user._id, sessionId}, "onLogin handler"
 		sessionSetKey = UserSessionsManager._sessionSetKey(user)
 		sessionSetKey = UserSessionsManager._sessionSetKey(user)
 		value = UserSessionsManager._sessionKey sessionId
 		value = UserSessionsManager._sessionKey sessionId
@@ -25,7 +25,7 @@ module.exports = UserSessionsManager =
 					return callback(err)
 					return callback(err)
 				callback()
 				callback()
 
 
-	onLogout: (user, sessionId, callback=(err)-> ) ->
+	untrackSession: (user, sessionId, callback=(err)-> ) ->
 		logger.log {user_id: user._id, sessionId}, "onLogout handler"
 		logger.log {user_id: user._id, sessionId}, "onLogout handler"
 		if !user
 		if !user
 			logger.log {sessionId}, "no user, for some reason"
 			logger.log {sessionId}, "no user, for some reason"
@@ -41,7 +41,7 @@ module.exports = UserSessionsManager =
 					return callback(err)
 					return callback(err)
 				callback()
 				callback()
 
 
-	revokeAllSessions: (user, callback=(err)->) ->
+	revokeAllUserSessions: (user, callback=(err)->) ->
 		logger.log {user_id: user._id}, "revoking all existing sessions for user"
 		logger.log {user_id: user._id}, "revoking all existing sessions for user"
 		sessionSetKey = UserSessionsManager._sessionSetKey(user)
 		sessionSetKey = UserSessionsManager._sessionSetKey(user)
 		rclient.smembers sessionSetKey, (err, sessionKeys) ->
 		rclient.smembers sessionSetKey, (err, sessionKeys) ->

+ 28 - 25
services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee

@@ -21,6 +21,10 @@ describe "AuthenticationController", ->
 			"../User/UserHandler": @UserHandler = {setupLoginData:sinon.stub()}
 			"../User/UserHandler": @UserHandler = {setupLoginData:sinon.stub()}
 			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() }
 			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() }
 			"settings-sharelatex": {}
 			"settings-sharelatex": {}
+			"../User/UserSessionsManager": @UserSessionsManager =
+				trackSession: sinon.stub()
+				untrackSession: sinon.stub()
+				revokeAllUserSessions: sinon.stub().callsArgWith(1, null)
 		@user =
 		@user =
 			_id: ObjectId()
 			_id: ObjectId()
 			email: @email = "USER@example.com"
 			email: @email = "USER@example.com"
@@ -57,7 +61,7 @@ describe "AuthenticationController", ->
 						@res.statusCode.should.equal 429
 						@res.statusCode.should.equal 429
 						done()
 						done()
 				@AuthenticationController.login(@req, @res)
 				@AuthenticationController.login(@req, @res)
-			
+
 		describe 'when the user is authenticated', ->
 		describe 'when the user is authenticated', ->
 			beforeEach ->
 			beforeEach ->
 				@LoginRateLimiter.processLoginRequest.callsArgWith(1, null, true)
 				@LoginRateLimiter.processLoginRequest.callsArgWith(1, null, true)
@@ -76,7 +80,7 @@ describe "AuthenticationController", ->
 				@AuthenticationController.establishUserSession
 				@AuthenticationController.establishUserSession
 					.calledWith(@req, @user)
 					.calledWith(@req, @user)
 					.should.equal true
 					.should.equal true
-					
+
 			it "should set res.session.justLoggedIn", ->
 			it "should set res.session.justLoggedIn", ->
 				@req.session.justLoggedIn.should.equal true
 				@req.session.justLoggedIn.should.equal true
 
 
@@ -95,7 +99,7 @@ describe "AuthenticationController", ->
 				@logger.log
 				@logger.log
 					.calledWith(email: @email.toLowerCase(), user_id: @user._id.toString(), "successful log in")
 					.calledWith(email: @email.toLowerCase(), user_id: @user._id.toString(), "successful log in")
 					.should.equal true
 					.should.equal true
-				
+
 
 
 		describe 'when the user is not authenticated', ->
 		describe 'when the user is not authenticated', ->
 			beforeEach ->
 			beforeEach ->
@@ -112,7 +116,7 @@ describe "AuthenticationController", ->
 
 
 			it "should not establish a session", ->
 			it "should not establish a session", ->
 				@AuthenticationController.establishUserSession.called.should.equal false
 				@AuthenticationController.establishUserSession.called.should.equal false
-			
+
 			it "should not setup the user data in the background", ->
 			it "should not setup the user data in the background", ->
 				@UserHandler.setupLoginData.called.should.equal false
 				@UserHandler.setupLoginData.called.should.equal false
 
 
@@ -137,12 +141,12 @@ describe "AuthenticationController", ->
 	describe "getLoggedInUserId", ->
 	describe "getLoggedInUserId", ->
 
 
 		beforeEach ->
 		beforeEach ->
-			@req = 
+			@req =
 				session :{}
 				session :{}
 
 
 		it "should return the user id from the session", (done)->
 		it "should return the user id from the session", (done)->
 			@user_id = "2134"
 			@user_id = "2134"
-			@req.session.user = 
+			@req.session.user =
 				_id:@user_id
 				_id:@user_id
 			@AuthenticationController.getLoggedInUserId @req, (err, user_id)=>
 			@AuthenticationController.getLoggedInUserId @req, (err, user_id)=>
 				expect(user_id).to.equal @user_id
 				expect(user_id).to.equal @user_id
@@ -168,7 +172,7 @@ describe "AuthenticationController", ->
 	describe "getLoggedInUser", ->
 	describe "getLoggedInUser", ->
 		beforeEach ->
 		beforeEach ->
 			@UserGetter.getUser = sinon.stub().callsArgWith(1, null, @user)
 			@UserGetter.getUser = sinon.stub().callsArgWith(1, null, @user)
-			
+
 		describe "with an established session", ->
 		describe "with an established session", ->
 			beforeEach ->
 			beforeEach ->
 				@req.session =
 				@req.session =
@@ -189,7 +193,7 @@ describe "AuthenticationController", ->
 				_id: "user-id-123"
 				_id: "user-id-123"
 				email: "user@sharelatex.com"
 				email: "user@sharelatex.com"
 			@middleware = @AuthenticationController.requireLogin()
 			@middleware = @AuthenticationController.requireLogin()
-			
+
 		describe "when the user is logged in", ->
 		describe "when the user is logged in", ->
 			beforeEach ->
 			beforeEach ->
 				@req.session =
 				@req.session =
@@ -219,13 +223,13 @@ describe "AuthenticationController", ->
 		beforeEach ->
 		beforeEach ->
 			@req.headers = {}
 			@req.headers = {}
 			@AuthenticationController.httpAuth = sinon.stub()
 			@AuthenticationController.httpAuth = sinon.stub()
-		
+
 		describe "with white listed url", ->
 		describe "with white listed url", ->
 			beforeEach ->
 			beforeEach ->
 				@AuthenticationController.addEndpointToLoginWhitelist "/login"
 				@AuthenticationController.addEndpointToLoginWhitelist "/login"
 				@req._parsedUrl.pathname = "/login"
 				@req._parsedUrl.pathname = "/login"
 				@AuthenticationController.requireGlobalLogin @req, @res, @next
 				@AuthenticationController.requireGlobalLogin @req, @res, @next
-			
+
 			it "should call next() directly", ->
 			it "should call next() directly", ->
 				@next.called.should.equal true
 				@next.called.should.equal true
 
 
@@ -235,9 +239,9 @@ describe "AuthenticationController", ->
 				@req._parsedUrl.pathname = "/login"
 				@req._parsedUrl.pathname = "/login"
 				@req.url = "/login?query=something"
 				@req.url = "/login?query=something"
 				@AuthenticationController.requireGlobalLogin @req, @res, @next
 				@AuthenticationController.requireGlobalLogin @req, @res, @next
-			
+
 			it "should call next() directly", ->
 			it "should call next() directly", ->
-				@next.called.should.equal true		
+				@next.called.should.equal true
 
 
 		describe "with http auth", ->
 		describe "with http auth", ->
 			beforeEach ->
 			beforeEach ->
@@ -248,20 +252,20 @@ describe "AuthenticationController", ->
 				@AuthenticationController.httpAuth
 				@AuthenticationController.httpAuth
 					.calledWith(@req, @res, @next)
 					.calledWith(@req, @res, @next)
 					.should.equal true
 					.should.equal true
-			
+
 		describe "with a user session", ->
 		describe "with a user session", ->
 			beforeEach ->
 			beforeEach ->
 				@req.session =
 				@req.session =
 					user: {"mock": "user"}
 					user: {"mock": "user"}
 				@AuthenticationController.requireGlobalLogin @req, @res, @next
 				@AuthenticationController.requireGlobalLogin @req, @res, @next
-			
+
 			it "should call next() directly", ->
 			it "should call next() directly", ->
 				@next.called.should.equal true
 				@next.called.should.equal true
-			
+
 		describe "with no login credentials", ->
 		describe "with no login credentials", ->
 			beforeEach ->
 			beforeEach ->
 				@AuthenticationController.requireGlobalLogin @req, @res, @next
 				@AuthenticationController.requireGlobalLogin @req, @res, @next
-				
+
 			it "should redirect to the /login page", ->
 			it "should redirect to the /login page", ->
 				@res.redirectedTo.should.equal "/login"
 				@res.redirectedTo.should.equal "/login"
 
 
@@ -274,7 +278,7 @@ describe "AuthenticationController", ->
 			@req.query = {}
 			@req.query = {}
 
 
 		describe "they have come directly to the url", ->
 		describe "they have come directly to the url", ->
-			beforeEach -> 
+			beforeEach ->
 				@req.query = {}
 				@req.query = {}
 				@middleware(@req, @res, @next)
 				@middleware(@req, @res, @next)
 
 
@@ -284,7 +288,7 @@ describe "AuthenticationController", ->
 
 
 		describe "they have come via a templates link", ->
 		describe "they have come via a templates link", ->
 
 
-			beforeEach -> 
+			beforeEach ->
 				@req.query.zipUrl = "something"
 				@req.query.zipUrl = "something"
 				@middleware(@req, @res, @next)
 				@middleware(@req, @res, @next)
 
 
@@ -293,8 +297,8 @@ describe "AuthenticationController", ->
 				@AuthenticationController._redirectToLoginPage.calledWith(@req, @res).should.equal false
 				@AuthenticationController._redirectToLoginPage.calledWith(@req, @res).should.equal false
 
 
 		describe "they have been invited to a project", ->
 		describe "they have been invited to a project", ->
-			
-			beforeEach -> 
+
+			beforeEach ->
 				@req.query.project_name = "something"
 				@req.query.project_name = "something"
 				@middleware(@req, @res, @next)
 				@middleware(@req, @res, @next)
 
 
@@ -333,7 +337,7 @@ describe "AuthenticationController", ->
 		beforeEach ->
 		beforeEach ->
 			@UserUpdater.updateUser = sinon.stub().callsArg(2)
 			@UserUpdater.updateUser = sinon.stub().callsArg(2)
 			@AuthenticationController._recordSuccessfulLogin(@user._id, @callback)
 			@AuthenticationController._recordSuccessfulLogin(@user._id, @callback)
-			
+
 		it "should increment the user.login.success metric", ->
 		it "should increment the user.login.success metric", ->
 			@Metrics.inc
 			@Metrics.inc
 				.calledWith("user.login.success")
 				.calledWith("user.login.success")
@@ -349,7 +353,7 @@ describe "AuthenticationController", ->
 	describe "_recordFailedLogin", ->
 	describe "_recordFailedLogin", ->
 		beforeEach ->
 		beforeEach ->
 			@AuthenticationController._recordFailedLogin(@callback)
 			@AuthenticationController._recordFailedLogin(@callback)
-			
+
 		it "should increment the user.login.failed metric", ->
 		it "should increment the user.login.failed metric", ->
 			@Metrics.inc
 			@Metrics.inc
 				.calledWith("user.login.failed")
 				.calledWith("user.login.failed")
@@ -374,13 +378,12 @@ describe "AuthenticationController", ->
 			@req.session.user.last_name.should.equal @user.last_name
 			@req.session.user.last_name.should.equal @user.last_name
 			@req.session.user.referal_id.should.equal @user.referal_id
 			@req.session.user.referal_id.should.equal @user.referal_id
 			@req.session.user.isAdmin.should.equal @user.isAdmin
 			@req.session.user.isAdmin.should.equal @user.isAdmin
-			
+
 		it "should destroy the session", ->
 		it "should destroy the session", ->
 			@req.session.destroy.called.should.equal true
 			@req.session.destroy.called.should.equal true
-			
+
 		it "should regenerate the session to protect against session fixation", ->
 		it "should regenerate the session to protect against session fixation", ->
 			@req.sessionStore.generate.calledWith(@req).should.equal true
 			@req.sessionStore.generate.calledWith(@req).should.equal true
 
 
 		it "should return the callback", ->
 		it "should return the callback", ->
 			@callback.called.should.equal true
 			@callback.called.should.equal true
-

+ 13 - 15
services/web/test/UnitTests/coffee/User/UserControllerTests.coffee

@@ -19,9 +19,9 @@ describe "UserController", ->
 			save:sinon.stub().callsArgWith(0)
 			save:sinon.stub().callsArgWith(0)
 			ace:{}
 			ace:{}
 
 
-		@UserDeleter = 
+		@UserDeleter =
 			deleteUser: sinon.stub().callsArgWith(1)
 			deleteUser: sinon.stub().callsArgWith(1)
-		@UserLocator = 
+		@UserLocator =
 			findById: sinon.stub().callsArgWith(1, null, @user)
 			findById: sinon.stub().callsArgWith(1, null, @user)
 		@User =
 		@User =
 			findById: sinon.stub().callsArgWith(1, null, @user)
 			findById: sinon.stub().callsArgWith(1, null, @user)
@@ -36,18 +36,18 @@ describe "UserController", ->
 			setUserPassword: sinon.stub()
 			setUserPassword: sinon.stub()
 		@ReferalAllocator =
 		@ReferalAllocator =
 			allocate:sinon.stub()
 			allocate:sinon.stub()
-		@SubscriptionDomainHandler = 
+		@SubscriptionDomainHandler =
 			autoAllocate:sinon.stub()
 			autoAllocate:sinon.stub()
 		@UserUpdater =
 		@UserUpdater =
 			changeEmailAddress:sinon.stub()
 			changeEmailAddress:sinon.stub()
 		@settings =
 		@settings =
 			siteUrl: "sharelatex.example.com"
 			siteUrl: "sharelatex.example.com"
-		@UserHandler = 
+		@UserHandler =
 			populateGroupLicenceInvite:sinon.stub().callsArgWith(1)
 			populateGroupLicenceInvite:sinon.stub().callsArgWith(1)
 		@UserSessionsManager =
 		@UserSessionsManager =
-			onLogin: sinon.stub()
-			onLogout: sinon.stub()
-			revokeAllSessions: sinon.stub().callsArgWith(1, null)
+			trackSession: sinon.stub()
+			untrackSession: sinon.stub()
+			revokeAllUserSessions: sinon.stub().callsArgWith(1, null)
 		@UserController = SandboxedModule.require modulePath, requires:
 		@UserController = SandboxedModule.require modulePath, requires:
 			"./UserLocator": @UserLocator
 			"./UserLocator": @UserLocator
 			"./UserDeleter": @UserDeleter
 			"./UserDeleter": @UserDeleter
@@ -62,13 +62,13 @@ describe "UserController", ->
 			"./UserHandler":@UserHandler
 			"./UserHandler":@UserHandler
 			"./UserSessionsManager": @UserSessionsManager
 			"./UserSessionsManager": @UserSessionsManager
 			"settings-sharelatex": @settings
 			"settings-sharelatex": @settings
-			"logger-sharelatex": 
+			"logger-sharelatex":
 				log:->
 				log:->
 				err:->
 				err:->
 			"../../infrastructure/Metrics": inc:->
 			"../../infrastructure/Metrics": inc:->
 
 
-		@req = 
-			session: 
+		@req =
+			session:
 				destroy:->
 				destroy:->
 				user :
 				user :
 					_id : @user_id
 					_id : @user_id
@@ -203,12 +203,12 @@ describe "UserController", ->
 			@UserRegistrationHandler.registerNewUserAndSendActivationEmail = sinon.stub().callsArgWith(1, null, @user, @url = "mock/url")
 			@UserRegistrationHandler.registerNewUserAndSendActivationEmail = sinon.stub().callsArgWith(1, null, @user, @url = "mock/url")
 			@req.body.email = @user.email = @email = "email@example.com"
 			@req.body.email = @user.email = @email = "email@example.com"
 			@UserController.register @req, @res
 			@UserController.register @req, @res
-		
+
 		it "should register the user and send them an email", ->
 		it "should register the user and send them an email", ->
 			@UserRegistrationHandler.registerNewUserAndSendActivationEmail
 			@UserRegistrationHandler.registerNewUserAndSendActivationEmail
 				.calledWith(@email)
 				.calledWith(@email)
 				.should.equal true
 				.should.equal true
-		
+
 		it "should return the user and activation url", ->
 		it "should return the user and activation url", ->
 			@res.json
 			@res.json
 				.calledWith({
 				.calledWith({
@@ -238,7 +238,7 @@ describe "UserController", ->
 			@res.send = =>
 			@res.send = =>
 				@AuthenticationManager.setUserPassword.called.should.equal false
 				@AuthenticationManager.setUserPassword.called.should.equal false
 				done()
 				done()
-			@UserController.changePassword @req, @res			
+			@UserController.changePassword @req, @res
 
 
 		it "should set the new password if they do match", (done)->
 		it "should set the new password if they do match", (done)->
 			@AuthenticationManager.authenticate.callsArgWith(2, null, @user)
 			@AuthenticationManager.authenticate.callsArgWith(2, null, @user)
@@ -250,5 +250,3 @@ describe "UserController", ->
 				@AuthenticationManager.setUserPassword.calledWith(@user._id, "newpass").should.equal true
 				@AuthenticationManager.setUserPassword.calledWith(@user._id, "newpass").should.equal true
 				done()
 				done()
 			@UserController.changePassword @req, @res
 			@UserController.changePassword @req, @res
-
-