From dd14e517130888b3335086a669f6bb1d6bac1682 Mon Sep 17 00:00:00 2001
From: Shane Kilkelly <shane@kilkelly.me>
Date: Fri, 23 Sep 2016 16:53:07 +0100
Subject: [PATCH] Handle null, undefined and false in `isUserLoggedIn`

---
 .../AuthenticationController.coffee           |  2 +-
 .../AuthenticationControllerTests.coffee      | 20 ++++++++++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee
index a72d31943e..e53ee477b4 100644
--- a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee
+++ b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee
@@ -94,7 +94,7 @@ module.exports = AuthenticationController =
 
 	isUserLoggedIn: (req) ->
 		user_id = AuthenticationController.getLoggedInUserId(req)
-		return user_id != null
+		return (user_id not in [null, undefined, false])
 
 	# TODO: perhaps should produce an error if the current user is not present
 	getLoggedInUserId: (req) ->
diff --git a/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee b/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee
index 002c3968e7..3a697441cb 100644
--- a/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee
+++ b/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee
@@ -44,6 +44,24 @@ describe "AuthenticationController", ->
 	afterEach ->
 		tk.reset()
 
+	describe 'isUserLoggedIn', () ->
+
+		beforeEach ->
+			@stub = sinon.stub(@AuthenticationController, 'getLoggedInUserId')
+
+		afterEach ->
+			@stub.restore()
+
+		it 'should do the right thing in all cases', () ->
+			@AuthenticationController.getLoggedInUserId.returns('some_id')
+			expect(@AuthenticationController.isUserLoggedIn(@req)).to.equal true
+			@AuthenticationController.getLoggedInUserId.returns(null)
+			expect(@AuthenticationController.isUserLoggedIn(@req)).to.equal false
+			@AuthenticationController.getLoggedInUserId.returns(false)
+			expect(@AuthenticationController.isUserLoggedIn(@req)).to.equal false
+			@AuthenticationController.getLoggedInUserId.returns(undefined)
+			expect(@AuthenticationController.isUserLoggedIn(@req)).to.equal false
+
 	describe 'setInSessionUser', () ->
 
 		beforeEach ->
@@ -361,7 +379,7 @@ describe "AuthenticationController", ->
 		describe "with a user session", ->
 			beforeEach ->
 				@req.session =
-					user: {"mock": "user"}
+					user: {"mock": "user", "_id": "some_id"}
 				@AuthenticationController.requireGlobalLogin @req, @res, @next
 
 			it "should call next() directly", ->