From d468f662acbd10237079df7d5cbbd4f8aef6ea57 Mon Sep 17 00:00:00 2001
From: Brian Gough <bjg@network-theory.co.uk>
Date: Thu, 23 Feb 2017 12:04:36 +0000
Subject: [PATCH] handle disconnects of unauthenticated users

---
 .../app/coffee/WebsocketController.coffee     | 10 ++++
 .../coffee/WebsocketControllerTests.coffee    | 51 ++++++++++++++++++-
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/services/real-time/app/coffee/WebsocketController.coffee b/services/real-time/app/coffee/WebsocketController.coffee
index 0d9708dd5e..5c6f8eab53 100644
--- a/services/real-time/app/coffee/WebsocketController.coffee
+++ b/services/real-time/app/coffee/WebsocketController.coffee
@@ -52,6 +52,16 @@ module.exports = WebsocketController =
 		metrics.inc "editor.leave-project"
 		Utils.getClientAttributes client, ["project_id", "user_id"], (error, {project_id, user_id}) ->
 			return callback(error) if error?
+			# bail out if the client had not managed to authenticate or join
+			# the project.  Prevents downstream errors in docupdater from
+			# flushProjectToMongoAndDelete with null project_id.
+			if not user_id?
+				logger.log {client_id: client.id}, "client leaving, unknown user"
+				return callback()
+			if not project_id?
+				logger.log {user_id: user_id, client_id: client.id}, "client leaving, not in project"
+				return callback()
+
 			logger.log {project_id, user_id, client_id: client.id}, "client leaving project"
 			WebsocketLoadBalancer.emitToRoom project_id, "clientTracking.clientDisconnected", client.id
 		
diff --git a/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee b/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee
index 7777327282..33da6f2ba1 100644
--- a/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee
+++ b/services/real-time/test/unit/coffee/WebsocketControllerTests.coffee
@@ -129,6 +129,7 @@ describe 'WebsocketController', ->
 							throw "expected room_id to be project_id"
 						return @clientsInRoom
 			@client.params.project_id = @project_id
+			@client.params.user_id = @user_id
 			@WebsocketController.FLUSH_IF_EMPTY_DELAY = 0
 			tk.reset() # Allow setTimeout to work.
 			
@@ -163,7 +164,55 @@ describe 'WebsocketController', ->
 			it "should not flush the project in the document updater", ->
 				@DocumentUpdaterManager.flushProjectToMongoAndDelete
 					.called.should.equal false
-						
+
+		describe "when client has not authenticated", ->
+			beforeEach (done) ->
+				@client.params.user_id = null
+				@client.params.project_id = null
+				@WebsocketController.leaveProject @io, @client, done
+
+			it "should not end clientTracking.clientDisconnected to the project room", ->
+				@WebsocketLoadBalancer.emitToRoom
+					.calledWith(@project_id, "clientTracking.clientDisconnected", @client.id)
+					.should.equal false
+
+			it "should not mark the user as disconnected", ->
+				@ConnectedUsersManager.markUserAsDisconnected
+					.calledWith(@project_id, @client.id)
+					.should.equal false
+
+			it "should not flush the project in the document updater", ->
+				@DocumentUpdaterManager.flushProjectToMongoAndDelete
+					.calledWith(@project_id)
+					.should.equal false
+
+			it "should increment the leave-project metric", ->
+				@metrics.inc.calledWith("editor.leave-project").should.equal true
+
+		describe "when client has not joined a project", ->
+			beforeEach (done) ->
+				@client.params.user_id = @user_id
+				@client.params.project_id = null
+				@WebsocketController.leaveProject @io, @client, done
+
+			it "should not end clientTracking.clientDisconnected to the project room", ->
+				@WebsocketLoadBalancer.emitToRoom
+					.calledWith(@project_id, "clientTracking.clientDisconnected", @client.id)
+					.should.equal false
+
+			it "should not mark the user as disconnected", ->
+				@ConnectedUsersManager.markUserAsDisconnected
+					.calledWith(@project_id, @client.id)
+					.should.equal false
+
+			it "should not flush the project in the document updater", ->
+				@DocumentUpdaterManager.flushProjectToMongoAndDelete
+					.calledWith(@project_id)
+					.should.equal false
+
+			it "should increment the leave-project metric", ->
+				@metrics.inc.calledWith("editor.leave-project").should.equal true
+
 	describe "joinDoc", ->
 		beforeEach ->
 			@doc_id = "doc-id-123"