redirect users to /register when coming from templates or share url

redirect to /login when going anywhere else (/project /project/1234)

services/web/app/coffee/Features/Authentication/AuthenticationController.coffee

@@ -71,18 +71,33 @@ module.exports = AuthenticationController =
 			if load_from_db
 				AuthenticationController.getLoggedInUser req, { allow_auth_token: options.allow_auth_token }, (error, user) ->
 					return next(error) if error?
-					return AuthenticationController._redirectToRegisterPage(req, res) if !user?
+					return AuthenticationController._redirectToLoginOrRegisterPage(req, res) if !user?
 					req.user = user
 					return next()
 			else
 				if !req.session.user?
-					return AuthenticationController._redirectToRegisterPage(req, res)
+					AuthenticationController._redirectToLoginOrRegisterPage(req, res) 
 				else
 					req.user = req.session.user
 					return next()
 
 		return doRequest
 
+
+	_redirectToLoginOrRegisterPage: (req, res)->
+		if req.query.zipUrl? or req.query.project_name?
+			return AuthenticationController._redirectToRegisterPage(req, res) 
+		else
+			AuthenticationController._redirectToLoginPage(req, res) 
+
+
+	_redirectToLoginPage: (req, res) ->
+		logger.log url: req.url, "user not logged in so redirecting to login page"
+		req.query.redir = req.path
+		url = "/login?#{querystring.stringify(req.query)}"
+		res.redirect url
+		Metrics.inc "security.login-redirect"
+
 	_redirectToRegisterPage: (req, res) ->
 		logger.log url: req.url, "user not logged in so redirecting to register page"
 		req.query.redir = req.path

services/web/app/coffee/managers/SecurityManager.coffee

@@ -54,10 +54,7 @@ module.exports = SecurityManager =
 						res.redirect('/restricted')
 					else
 						logger.log "user not logged in and trying to access #{req.url}, being redirected to login"
-						req.query.redir = req._parsedUrl.pathname
+						AuthenticationController._redirectToLoginOrRegisterPage(req, res)
-						url = "/register?#{querystring.stringify(req.query)}"
-						res.redirect url
-						email = "not logged in user"
 		if arguments.length > 1
 			options =
 				allow_auth_token: false

services/web/app/views/user/register.jade

@@ -7,12 +7,13 @@ block content
 				.registration_message
 					if sharedProjectData.user_first_name !== undefined
 						h1  #{translate("user_wants_you_to_see_project", {username:sharedProjectData.user_first_name, projectname:sharedProjectData.project_name})}
-						div #{translate("join_sl_to_view_project")}
+						div #{translate("join_sl_to_view_project")}.
+							a(href="/login")  #{translate("login_here")}
 					else if newTemplateData.templateName !== undefined
 						h1 #{translate("register_to_edit_template", {templateName:newTemplateData.templateName})}
 
 						div #{translate("already_have_sl_account")}
-							a(href="/login") #{translate("login_here")}
+							a(href="/login")  #{translate("login_here")}
 
 			.row
 				.col-md-6.col-md-offset-3.col-lg-4.col-lg-offset-4

services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee

@@ -214,9 +214,7 @@ describe "AuthenticationController", ->
 					@middleware(@req, @res, @next)
 
 				it "should call getLoggedInUser with the passed options", ->
-					@AuthenticationController.getLoggedInUser
+					@AuthenticationController.getLoggedInUser.calledWith(@req, { allow_auth_token: true }).should.equal true
-						.calledWith(@req, { allow_auth_token: true })
-						.should.equal true
 
 				it "should set the user property on the request", ->
 					@req.user.should.deep.equal @user
@@ -226,14 +224,12 @@ describe "AuthenticationController", ->
 
 			describe "when the user is not logged in", ->
 				beforeEach ->
-					@AuthenticationController._redirectToRegisterPage = sinon.stub()
+					@AuthenticationController._redirectToLoginOrRegisterPage = sinon.stub()
 					@AuthenticationController.getLoggedInUser = sinon.stub().callsArgWith(2, null, null)
 					@middleware(@req, @res, @next)
 
 				it "should redirect to the register page", ->
-					@AuthenticationController._redirectToRegisterPage
+					@AuthenticationController._redirectToLoginOrRegisterPage.calledWith(@req, @res).should.equal true
-						.calledWith(@req, @res)
-						.should.equal true
 
 		describe "when not loading from the database", ->
 			beforeEach ->
@@ -257,13 +253,12 @@ describe "AuthenticationController", ->
 			describe "when the user is not logged in", ->
 				beforeEach ->
 					@req.session = {}
-					@AuthenticationController._redirectToRegisterPage = sinon.stub()
+					@AuthenticationController._redirectToLoginOrRegisterPage = sinon.stub()
+					@req.query = {}
 					@middleware(@req, @res, @next)
 
-				it "should redirect to the register page", ->
+				it "should redirect to the register or login page", ->
-					@AuthenticationController._redirectToRegisterPage
+					@AuthenticationController._redirectToLoginOrRegisterPage.calledWith(@req, @res).should.equal true
-						.calledWith(@req, @res)
-						.should.equal true
 
 		describe "when not loading from the database but an auth_token is provided", ->
 			beforeEach ->
@@ -277,6 +272,49 @@ describe "AuthenticationController", ->
 					.calledWith(@req, {allow_auth_token: true})
 					.should.equal true
 
+
+
+		describe "_redirectToLoginOrRegisterPage", ->
+
+			beforeEach ->
+				@middleware = @AuthenticationController.requireLogin(@options = { load_from_db: false })
+				@req.session = {}
+				@AuthenticationController._redirectToRegisterPage = sinon.stub()
+				@AuthenticationController._redirectToLoginPage = sinon.stub()
+				@req.query = {}
+
+			describe "they have come directly to the url", ->
+				beforeEach -> 
+					@req.query = {}
+					@middleware(@req, @res, @next)
+
+				it "should redirect to the login page", ->
+					@AuthenticationController._redirectToRegisterPage.calledWith(@req, @res).should.equal false
+					@AuthenticationController._redirectToLoginPage.calledWith(@req, @res).should.equal true
+
+			describe "they have come via a templates link", ->
+
+				beforeEach -> 
+					@req.query.zipUrl = "something"
+					@middleware(@req, @res, @next)
+
+				it "should redirect to the register page", ->
+					@AuthenticationController._redirectToRegisterPage.calledWith(@req, @res).should.equal true
+					@AuthenticationController._redirectToLoginPage.calledWith(@req, @res).should.equal false
+
+			describe "they have been invited to a project", ->
+				
+				beforeEach -> 
+					@req.query.project_name = "something"
+					@middleware(@req, @res, @next)
+
+				it "should redirect to the register page", ->
+					@AuthenticationController._redirectToRegisterPage.calledWith(@req, @res).should.equal true
+					@AuthenticationController._redirectToLoginPage.calledWith(@req, @res).should.equal false
+
+
+
+
 	describe "_redirectToRegisterPage", ->
 		beforeEach ->
 			@req.path = "/target/url"
@@ -293,9 +331,16 @@ describe "AuthenticationController", ->
 				.calledWith(url: @url, "user not logged in so redirecting to register page")
 				.should.equal true
 
-		it "should increase the security.login-redirect metric", ->
+	describe "_redirectToLoginPage", ->
-			@Metrics.inc.calledWith("security.login-redirect").should.equal true
+		beforeEach ->
-				
+			@req.path = "/target/url"
+			@req.query =
+				extra_query: "foo"
+			@AuthenticationController._redirectToLoginPage(@req, @res)
+
+		it "should redirect to the register page with a query string attached", ->
+			@res.redirectedTo.should.equal "/login?extra_query=foo&redir=%2Ftarget%2Furl"
+
 
 	describe "_recordSuccessfulLogin", ->
 		beforeEach ->