Merge branch 'master' into pr-style-announcements

services/web/app/coffee/Features/Analytics/AnalyticsManager.coffee

@@ -18,7 +18,7 @@ module.exports =
 
 
 	recordEvent: (user_id, event, segmentation = {}, callback = (error) ->) ->
-		if user_id == settings.smokeTest?.userId
+		if user_id+"" == settings.smokeTest?.userId+""
 			return callback()
 		opts =
 			body:

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

@@ -62,17 +62,18 @@ module.exports = AuthenticationController =
 			if err?
 				return next(err)
 			if user # `user` is either a user object or false
+				redir = AuthenticationController._getRedirectFromSession(req) || "/project"
 				AuthenticationController.afterLoginSessionSetup req, user, (err) ->
 					if err?
 						return next(err)
-					res.json {redir: req._redir}
+					AuthenticationController._clearRedirectFromSession(req)
+					res.json {redir: redir}
 			else
 				res.json message: info
 		)(req, res, next)
 
 	doPassportLogin: (req, username, password, done) ->
 		email = username.toLowerCase()
-		redir = Url.parse(req?.body?.redir or "/project").path
 		LoginRateLimiter.processLoginRequest email, (err, isAllowed)->
 			return done(err) if err?
 			if !isAllowed
@@ -90,7 +91,6 @@ module.exports = AuthenticationController =
 					req.session.justLoggedIn = true
 					# capture the request ip for use when creating the session
 					user._login_req_ip = req.ip
-					req._redir = redir
 					return done(null, user)
 				else
 					AuthenticationController._recordFailedLogin()
@@ -157,21 +157,23 @@ module.exports = AuthenticationController =
 		return isValid
 
 	_redirectToLoginOrRegisterPage: (req, res)->
-		if req.query.zipUrl? or req.query.project_name?
+		if (req.query.zipUrl? or
+			  req.query.project_name? or
+			  req.path == '/user/subscription/new')
 			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
+		AuthenticationController._setRedirectInSession(req)
 		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
+		AuthenticationController._setRedirectInSession(req)
 		url = "/register?#{querystring.stringify(req.query)}"
 		res.redirect url
 		Metrics.inc "security.login-redirect"
@@ -188,3 +190,16 @@ module.exports = AuthenticationController =
 	_recordFailedLogin: (callback = (error) ->) ->
 		Metrics.inc "user.login.failed"
 		callback()
+
+	_setRedirectInSession: (req, value) ->
+		if !value?
+			value = if Object.keys(req.query).length > 0 then "#{req.path}?#{querystring.stringify(req.query)}" else req.path
+		if req.session?
+			req.session.postLoginRedirect = value
+
+	_getRedirectFromSession: (req) ->
+		return req?.session?.postLoginRedirect || null
+
+	_clearRedirectFromSession: (req) ->
+		if req.session?
+			delete req.session.postLoginRedirect

services/web/app/coffee/Features/Authorization/AuthorizationMiddlewear.coffee

@@ -108,5 +108,5 @@ module.exports = AuthorizationMiddlewear =
 			logger.log {from: from}, "redirecting to login"
 			redirect_to = "/login"
 			if from?
-				redirect_to += "?redir=#{encodeURIComponent(from)}"
+				AuthenticationController._setRedirectInSession(req, from)
 			res.redirect redirect_to

services/web/app/coffee/Features/Docstore/DocstoreManager.coffee

@@ -30,7 +30,7 @@ module.exports = DocstoreManager =
 				logger.error err: error, project_id: project_id, "error getting all docs from docstore"
 				callback(error)
 
-	getDoc: (project_id, doc_id, options = {}, callback = (error, lines, rev) ->) ->
+	getDoc: (project_id, doc_id, options = {}, callback = (error, lines, rev, version) ->) ->
 		if typeof(options) == "function"
 			callback = options
 			options = {}
@@ -45,19 +45,20 @@ module.exports = DocstoreManager =
 			return callback(error) if error?
 			if 200 <= res.statusCode < 300
 				logger.log doc_id: doc_id, project_id: project_id, version: doc.version, rev: doc.rev, "got doc from docstore api"
-				callback(null, doc.lines, doc.rev)
+				callback(null, doc.lines, doc.rev, doc.version)
 			else
 				error = new Error("docstore api responded with non-success code: #{res.statusCode}")
 				logger.error err: error, project_id: project_id, doc_id: doc_id, "error getting doc from docstore"
 				callback(error)
 
-	updateDoc: (project_id, doc_id, lines, callback = (error, modified, rev) ->) ->
+	updateDoc: (project_id, doc_id, lines, version, callback = (error, modified, rev) ->) ->
 		logger.log project_id: project_id, doc_id: doc_id, "updating doc in docstore api"
 		url = "#{settings.apis.docstore.url}/project/#{project_id}/doc/#{doc_id}"
 		request.post {
 			url: url
 			json:
 				lines: lines
+				version: version
 		}, (error, res, result) ->
 			return callback(error) if error?
 			if 200 <= res.statusCode < 300

services/web/app/coffee/Features/Documents/DocumentController.coffee

@@ -7,7 +7,7 @@ module.exports =
 		doc_id = req.params.doc_id
 		plain = req?.query?.plain == 'true'
 		logger.log doc_id:doc_id, project_id:project_id, "receiving get document request from api (docupdater)"
-		ProjectEntityHandler.getDoc project_id, doc_id, (error, lines, rev) ->
+		ProjectEntityHandler.getDoc project_id, doc_id, (error, lines, rev, version) ->
 			if error?
 				logger.err err:error, doc_id:doc_id, project_id:project_id, "error finding element for getDocument"
 				return next(error)
@@ -18,14 +18,15 @@ module.exports =
 				res.type "json"
 				res.send JSON.stringify {
 					lines: lines
+					version: version
 				}
 
 	setDocument: (req, res, next = (error) ->) ->
 		project_id = req.params.Project_id
 		doc_id = req.params.doc_id
-		lines = req.body.lines
+		{lines, version} = req.body
 		logger.log doc_id:doc_id, project_id:project_id, "receiving set document request from api (docupdater)"
-		ProjectEntityHandler.updateDocLines project_id, doc_id, lines, (error) ->
+		ProjectEntityHandler.updateDocLines project_id, doc_id, lines, version, (error) ->
 			if error?
 				logger.err err:error, doc_id:doc_id, project_id:project_id, "error finding element for getDocument"
 				return next(error)

services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee

@@ -126,7 +126,7 @@ module.exports = ProjectEntityHandler =
 			doc = new Doc name: docName
 			# Put doc in docstore first, so that if it errors, we don't have a doc_id in the project
 			# which hasn't been created in docstore.
-			DocstoreManager.updateDoc project_id.toString(), doc._id.toString(), docLines, (err, modified, rev) ->
+			DocstoreManager.updateDoc project_id.toString(), doc._id.toString(), docLines, 0, (err, modified, rev) ->
 				return callback(err) if err? 
 
 				ProjectEntityHandler._putElement project, folder_id, doc, "doc", (err, result)=>
@@ -292,7 +292,7 @@ module.exports = ProjectEntityHandler =
 					return callback(err)
 				callback(err, folder, parentFolder_id)
 
-	updateDocLines : (project_id, doc_id, lines, callback = (error) ->)->
+	updateDocLines : (project_id, doc_id, lines, version, callback = (error) ->)->
 		ProjectGetter.getProjectWithoutDocLines project_id, (err, project)->
 			return callback(err) if err?
 			return callback(new Errors.NotFoundError("project not found")) if !project?
@@ -307,7 +307,7 @@ module.exports = ProjectEntityHandler =
 					return callback(error)
 
 				logger.log project_id: project_id, doc_id: doc_id, "telling docstore manager to update doc"
-				DocstoreManager.updateDoc project_id, doc_id, lines, (err, modified, rev) ->
+				DocstoreManager.updateDoc project_id, doc_id, lines, version, (err, modified, rev) ->
 					if err?
 						logger.error err: err, doc_id: doc_id, project_id:project_id, lines: lines, "error sending doc to docstore"
 						return callback(err)

services/web/app/coffee/Features/Subscription/RecurlyWrapper.coffee

@@ -418,7 +418,15 @@ module.exports = RecurlyWrapper =
 			url: "subscriptions/#{subscriptionId}/cancel",
 			method: "put"
 		}, (error, response, body) ->
-			callback(error)
+			if error?
+				RecurlyWrapper._parseXml body, (_err, parsed) ->
+					if parsed?.error?.description == "A canceled subscription can't transition to canceled"
+						logger.log {subscriptionId, error, body}, "subscription already cancelled, not really an error, proceeding"
+						callback(null)
+					else
+						callback(error)
+			else
+				callback(null)
 		)
 
 	reactivateSubscription: (subscriptionId, callback) ->

services/web/app/coffee/Features/Subscription/SubscriptionController.coffee

@@ -14,10 +14,6 @@ module.exports = SubscriptionController =
 
 	plansPage: (req, res, next) ->
 		plans = SubscriptionViewModelBuilder.buildViewModel()
-		if AuthenticationController.isUserLoggedIn(req)
-			baseUrl = ""
-		else
-			baseUrl = "/register?redir="
 		viewName = "subscriptions/plans"
 		if req.query.v?
 			viewName = "#{viewName}_#{req.query.v}"
@@ -29,7 +25,6 @@ module.exports = SubscriptionController =
 				res.render viewName,
 					title: "plans_and_pricing"
 					plans: plans
-					baseUrl: baseUrl
 					gaExperiments: Settings.gaExperiments.plansPage
 					recomendedCurrency:recomendedCurrency
 					shouldABTestPlans: currentUser == null or (currentUser?.signUpDate? and currentUser.signUpDate >= (new Date('2016-10-27')))

services/web/app/coffee/Features/User/UserController.coffee

@@ -33,8 +33,14 @@ module.exports = UserController =
 				if err?
 					logger.err {user_id}, "error while deleting user account"
 					return next(err)
-				req.session?.destroy()
-				res.sendStatus(200)
+				sessionId = req.sessionID
+				req.logout?()
+				req.session.destroy (err) ->
+					if err?
+						logger.err err: err, 'error destorying session'
+						return next(err)
+					UserSessionsManager.untrackSession(user, sessionId)
+					res.sendStatus(200)
 
 	unsubscribe: (req, res)->
 		user_id = AuthenticationController.getLoggedInUserId(req)

services/web/app/coffee/Features/User/UserPagesController.coffee

@@ -20,7 +20,6 @@ module.exports =
 
 		res.render 'user/register',
 			title: 'register'
-			redir: req.query.redir
 			sharedProjectData: sharedProjectData
 			newTemplateData: newTemplateData
 			new_email:req.query.new_email || ""
@@ -49,19 +48,25 @@ module.exports =
 					token: req.query.token
 
 	loginPage : (req, res)->
+		# if user is being sent to /login with explicit redirect (redir=/foo),
+		# such as being sent from the editor to /login, then set the redirect explicitly
+		if req.query.redir? and !AuthenticationController._getRedirectFromSession(req)?
+			logger.log {redir: req.query.redir}, "setting explicit redirect from login page"
+			AuthenticationController._setRedirectInSession(req, req.query.redir)
 		res.render 'user/login',
 			title: 'login',
-			redir: req.query.redir,
 			email: req.query.email
 
 	settingsPage : (req, res, next)->
 		user_id = AuthenticationController.getLoggedInUserId(req)
 		logger.log user: user_id, "loading settings page"
+		shouldAllowEditingDetails = !(Settings?.ldap?.updateUserDetailsOnLogin) and !(Settings?.saml?.updateUserDetailsOnLogin)
 		UserLocator.findById user_id, (err, user)->
 			return next(err) if err?
 			res.render 'user/settings',
 				title:'account_settings'
 				user: user,
+				shouldAllowEditingDetails: shouldAllowEditingDetails
 				languages: Settings.languages,
 				accountSettingsTabActive: true
 

services/web/app/coffee/infrastructure/Server.coffee

@@ -90,6 +90,7 @@ webRouter.use session
 		secure: Settings.secureCookie
 	store: sessionStore
 	key: Settings.cookieName
+	rolling: true
 
 # passport
 webRouter.use passport.initialize()

services/web/app/views/layout/navbar.jade

@@ -35,6 +35,9 @@ nav.navbar.navbar-default
 									each child in item.dropdown
 										if child.divider
 											li.divider
+										else if child.user_email
+											li
+												div.subdued #{getUserEmail()}
 										else
 											li
 												if child.url

services/web/app/views/sentry.jade

@@ -70,7 +70,7 @@
 			// whitelistUrls: ['example.com/scripts/']
 			}).install();
 		}
-	- if (typeof(user) != "undefined" && typeof (user.email) != "undefined")
+	- if (user && typeof(user) != "undefined" && typeof (user.email) != "undefined")
 		script(type="text/javascript").
 			if (typeof(Raven) != "undefined" && Raven.setUserContext) {
 				Raven.setUserContext({email: '#{user.email}'});

services/web/app/views/subscriptions/plans.jade

@@ -100,7 +100,7 @@ block content
 												li
 													br
 													a.btn.btn-info(
-														ng-href="#{baseUrl}/user/subscription/new?planCode={{ getCollaboratorPlanCode() }}&currency={{currencyCode}}", ng-click="signUpNowClicked('collaborator')"
+														ng-href="/user/subscription/new?planCode={{ getCollaboratorPlanCode() }}&currency={{currencyCode}}", ng-click="signUpNowClicked('collaborator')"
 													)
 														span(ng-show="ui.view != 'annual'") #{translate("start_free_trial")}
 														span(ng-show="ui.view == 'annual'") #{translate("buy_now")}
@@ -124,7 +124,7 @@ block content
 												li
 													br
 													a.btn.btn-info(
-														ng-href="#{baseUrl}/user/subscription/new?planCode=professional{{ ui.view == 'annual' && '-annual' || planQueryString}}&currency={{currencyCode}}", ng-click="signUpNowClicked('professional')"
+														ng-href="/user/subscription/new?planCode=professional{{ ui.view == 'annual' && '-annual' || planQueryString}}&currency={{currencyCode}}", ng-click="signUpNowClicked('professional')"
 													)
 														span(ng-show="ui.view != 'annual'") #{translate("start_free_trial")}
 														span(ng-show="ui.view == 'annual'") #{translate("buy_now")}
@@ -166,7 +166,7 @@ block content
 												li
 													br
 													a.btn.btn-info(
-														ng-href="#{baseUrl}/user/subscription/new?planCode=student{{ plansVariant == 'default' ? planQueryString : '_'+plansVariant }}&currency={{currencyCode}}",
+														ng-href="/user/subscription/new?planCode=student{{ plansVariant == 'default' ? planQueryString : '_'+plansVariant }}&currency={{currencyCode}}",
 														ng-click="signUpNowClicked('student')"
 													) #{translate("start_free_trial")}
 
@@ -189,7 +189,7 @@ block content
 												li
 													br
 													a.btn.btn-info(
-														ng-href="#{baseUrl}/user/subscription/new?planCode=student-annual{{ plansVariant == 'default' ? '' : '_'+plansVariant }}&currency={{currencyCode}}",
+														ng-href="/user/subscription/new?planCode=student-annual{{ plansVariant == 'default' ? '' : '_'+plansVariant }}&currency={{currencyCode}}",
 														ng-click="signUpNowClicked('student')"
 													) #{translate("buy_now")}
 

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

@@ -10,7 +10,6 @@ block content
 							h1 #{translate("log_in")}
 						form(async-form="login", name="loginForm", action='/login', method="POST", ng-cloak)
 							input(name='_csrf', type='hidden', value=csrfToken)
-							input(name='redir', type='hidden', value=redir)
 							form-messages(for="loginForm")
 							.form-group
 								input.form-control(

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

@@ -12,7 +12,7 @@ block content
 							| #{translate("join_sl_to_view_project")}.
 						div
 							| #{translate("if_you_are_registered")},
-							a(href="/login?redir=#{getReqQueryParam('redir')}")  #{translate("login_here")}
+							a(href="/login")  #{translate("login_here")}
 					else if newTemplateData.templateName !== undefined
 						h1 #{translate("register_to_edit_template", {templateName:newTemplateData.templateName})}
 

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

@@ -39,25 +39,34 @@ block content
 													label.control-label #{translate("email")}
 													div.form-control(readonly="true") #{user.email}
 
-											.form-group
-												label(for='firstName').control-label #{translate("first_name")}
-												input.form-control(
-													type='text',
-													name='first_name',
-													value=user.first_name
-												)
-											.form-group
-												label(for='lastName').control-label #{translate("last_name")}
-												input.form-control(
-													type='text',
-													name='last_name',
-													value=user.last_name
-												)
-											.actions
-												button.btn.btn-primary(
-													type='submit',
-													ng-disabled="settingsForm.$invalid"
-												)  #{translate("update")}
+											if shouldAllowEditingDetails
+												.form-group
+													label(for='firstName').control-label #{translate("first_name")}
+													input.form-control(
+														type='text',
+														name='first_name',
+														value=user.first_name
+													)
+												.form-group
+													label(for='lastName').control-label #{translate("last_name")}
+													input.form-control(
+														type='text',
+														name='last_name',
+														value=user.last_name
+													)
+												.actions
+													button.btn.btn-primary(
+														type='submit',
+														ng-disabled="settingsForm.$invalid"
+													)  #{translate("update")}
+											else
+												.form-group
+													label.control-label #{translate("first_name")}
+													div.form-control(readonly="true") #{user.first_name}
+												.form-group
+													label.control-label #{translate("last_name")}
+													div.form-control(readonly="true") #{user.last_name}
+
 									if !externalAuthenticationSystemUsed()
 										.col-md-5.col-md-offset-1
 											h3  #{translate("change_password")}

services/web/config/settings.defaults.coffee

@@ -347,6 +347,10 @@ module.exports = settings =
 			text: "Account"
 			only_when_logged_in: true
 			dropdown: [{
+				user_email: true
+			},{
+				divider: true
+			}, {
 				text: "Account Settings"
 				url: "/user/settings"
 			}, {

services/web/public/coffee/main/account-settings.coffee

@@ -64,7 +64,11 @@ define [
 						$scope.state.inflight = false
 						$scope.state.error = false
 						$scope.state.invalidCredentials = false
-						window.location = "/"
+						setTimeout(
+							() ->
+								window.location = "/login"
+							, 1000
+						)
 					.error (data, status) ->
 						$scope.state.inflight = false
 						if status == 403

services/web/public/stylesheets/app/editor.less

@@ -138,6 +138,10 @@
 	.spelling-highlight {
 		position: absolute;
 		background-image: url(/img/spellcheck-underline.png);
+		@media (-webkit-min-device-pixel-ratio: 2), (min-resolution: 192dpi) {
+			background-image: url(/img/spellcheck-underline@2x.png);
+			background-size: 5px 4px;
+		}
 		background-repeat: repeat-x;
 		background-position: bottom left;
 	}

services/web/public/stylesheets/components/dropdowns.less

@@ -58,8 +58,8 @@
     .nav-divider(@dropdown-divider-bg);
   }
 
-  // Links within the dropdown menu
-  > li > a {
+  // Links and other items within the dropdown menu
+  > li > a,div {
     display: block;
     padding: 3px 20px;
     clear: both;
@@ -67,8 +67,11 @@
     line-height: @line-height-base;
     color: @dropdown-link-color;
     white-space: nowrap; // prevent links from randomly breaking onto new lines
+    &.subdued {
+      color: #7a7a7a
+    }
     .subdued {
-        color: #7a7a7a
+      color: #7a7a7a
     }
   }
 }

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

@@ -91,7 +91,10 @@ describe "AuthenticationController", ->
 			@info = null
 			@req.login = sinon.stub().callsArgWith(1, null)
 			@res.json = sinon.stub()
-			@req.session = @session = {passport: {user: @user}}
+			@req.session = @session = {
+				passport: {user: @user},
+				postLoginRedirect: "/path/to/redir/to"
+			}
 			@req.session.destroy = sinon.stub().callsArgWith(0, null)
 			@req.session.save = sinon.stub().callsArgWith(0, null)
 			@req.sessionStore = {generate: sinon.stub()}
@@ -114,11 +117,11 @@ describe "AuthenticationController", ->
 		describe 'when authenticate produces a user', ->
 
 			beforeEach ->
-				@req._redir = 'some_redirect'
+				@req.session.postLoginRedirect = 'some_redirect'
 				@passport.authenticate.callsArgWith(1, null, @user, @info)
 
 			afterEach ->
-				delete @req._redir
+				delete @req.session.postLoginRedirect
 
 			it 'should call req.login', () ->
 				@AuthenticationController.passportLogin @req, @res, @next
@@ -128,7 +131,7 @@ describe "AuthenticationController", ->
 			it 'should send a json response with redirect', () ->
 				@AuthenticationController.passportLogin @req, @res, @next
 				@res.json.callCount.should.equal 1
-				@res.json.calledWith({redir: @req._redir}).should.equal true
+				@res.json.calledWith({redir: 'some_redirect'}).should.equal true
 
 			describe 'when session.save produces an error', () ->
 				beforeEach ->
@@ -152,10 +155,11 @@ describe "AuthenticationController", ->
 				@AuthenticationController.passportLogin @req, @res, @next
 				@req.login.callCount.should.equal 0
 
-			it 'should send a json response with redirect', () ->
+			it 'should not send a json response with redirect', () ->
 				@AuthenticationController.passportLogin @req, @res, @next
 				@res.json.callCount.should.equal 1
 				@res.json.calledWith({message: @info}).should.equal true
+				expect(@res.json.lastCall.args[0].redir?).to.equal false
 
 	describe 'afterLoginSessionSetup', ->
 
@@ -230,7 +234,8 @@ describe "AuthenticationController", ->
 			@req.body =
 				email: @email
 				password: @password
-				redir: @redir = "/path/to/redir/to"
+				session:
+					postLoginRedirect: "/path/to/redir/to"
 			@cb = sinon.stub()
 
 		describe "when the users rate limit", ->
@@ -265,9 +270,6 @@ describe "AuthenticationController", ->
 			it "should set res.session.justLoggedIn", ->
 				@req.session.justLoggedIn.should.equal true
 
-			it "should redirect the user to the specified location", ->
-				expect(@req._redir).to.deep.equal @redir
-
 			it "should record the successful login", ->
 				@AuthenticationController._recordSuccessfulLogin
 					.calledWith(@user._id)
@@ -313,17 +315,6 @@ describe "AuthenticationController", ->
 					.calledWith(email: @email.toLowerCase(), "failed log in")
 					.should.equal true
 
-		describe "with a URL to a different domain", ->
-			beforeEach ->
-				@LoginRateLimiter.processLoginRequest.callsArgWith(1, null, true)
-				@req.body.redir = "http://www.facebook.com/test"
-				@AuthenticationManager.authenticate = sinon.stub().callsArgWith(2, null, @user)
-				@cb = sinon.stub()
-				@AuthenticationController.doPassportLogin(@req, @req.body.email, @req.body.password, @cb)
-
-			it "should only redirect to the local path", ->
-				expect(@req._redir).to.equal "/test"
-
 	describe "getLoggedInUserId", ->
 
 		beforeEach ->
@@ -488,8 +479,8 @@ describe "AuthenticationController", ->
 			@AuthenticationController._redirectToRegisterPage(@req, @res)
 
 		it "should redirect to the register page with a query string attached", ->
-			@res.redirectedTo
-				.should.equal "/register?extra_query=foo&redir=%2Ftarget%2Furl"
+			@req.session.postLoginRedirect.should.equal '/target/url?extra_query=foo'
+			@res.redirectedTo.should.equal "/register?extra_query=foo"
 
 		it "should log out a message", ->
 			@logger.log
@@ -504,7 +495,8 @@ describe "AuthenticationController", ->
 			@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"
+			@req.session.postLoginRedirect.should.equal '/target/url?extra_query=foo'
+			@res.redirectedTo.should.equal "/login?extra_query=foo"
 
 
 	describe "_recordSuccessfulLogin", ->
@@ -535,3 +527,34 @@ describe "AuthenticationController", ->
 
 		it "should call the callback", ->
 			@callback.called.should.equal true
+
+
+	describe '_setRedirectInSession', ->
+		beforeEach ->
+			@req = {session: {}}
+			@req.path = "/somewhere"
+			@req.query = {one: "1"}
+
+		it 'should set redirect property on session', ->
+			@AuthenticationController._setRedirectInSession(@req)
+			expect(@req.session.postLoginRedirect).to.equal "/somewhere?one=1"
+
+		it 'should set the supplied value', ->
+			@AuthenticationController._setRedirectInSession(@req, '/somewhere/specific')
+			expect(@req.session.postLoginRedirect).to.equal "/somewhere/specific"
+
+	describe '_getRedirectFromSession', ->
+		beforeEach ->
+			@req = {session: {postLoginRedirect: "/a?b=c"}}
+
+		it 'should get redirect property from session', ->
+			expect(@AuthenticationController._getRedirectFromSession(@req)).to.equal "/a?b=c"
+
+	describe '_clearRedirectFromSession', ->
+		beforeEach ->
+			@req = {session: {postLoginRedirect: "/a?b=c"}}
+
+		it 'should remove the redirect property from session', ->
+			@AuthenticationController._clearRedirectFromSession(@req)
+			expect(@req.session.postLoginRedirect).to.equal undefined
+

services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee

@@ -56,12 +56,13 @@ describe "DocstoreManager", ->
 		beforeEach ->
 			@lines = ["mock", "doc", "lines"]
 			@rev = 5
+			@version = 42
 			@modified = true
 
 		describe "with a successful response code", ->
 			beforeEach ->
 				@request.post = sinon.stub().callsArgWith(1, null, statusCode: 204, { modified: @modified, rev: @rev })
-				@DocstoreManager.updateDoc @project_id, @doc_id, @lines, @callback
+				@DocstoreManager.updateDoc @project_id, @doc_id, @lines, @version, @callback
 
 			it "should update the doc in the docstore api", ->
 				@request.post
@@ -69,6 +70,7 @@ describe "DocstoreManager", ->
 						url: "#{@settings.apis.docstore.url}/project/#{@project_id}/doc/#{@doc_id}"
 						json:
 							lines: @lines
+							version: @version
 					})
 					.should.equal true
 
@@ -78,7 +80,7 @@ describe "DocstoreManager", ->
 		describe "with a failed response code", ->
 			beforeEach ->
 				@request.post = sinon.stub().callsArgWith(1, null, statusCode: 500, "")
-				@DocstoreManager.updateDoc @project_id, @doc_id, @lines, @callback
+				@DocstoreManager.updateDoc @project_id, @doc_id, @lines, @version, @callback
 
 			it "should call the callback with an error", ->
 				@callback.calledWith(new Error("docstore api responded with non-success code: 500")).should.equal true
@@ -97,6 +99,7 @@ describe "DocstoreManager", ->
 			@doc =
 				lines:   @lines = ["mock", "doc", "lines"]
 				rev:     @rev = 5
+				version: @version = 42
 
 		describe "with a successful response code", ->
 			beforeEach ->
@@ -112,7 +115,7 @@ describe "DocstoreManager", ->
 					.should.equal true
 
 			it "should call the callback with the lines, version and rev", ->
-				@callback.calledWith(null, @lines, @rev).should.equal true
+				@callback.calledWith(null, @lines, @rev, @version).should.equal true
 
 		describe "with a failed response code", ->
 			beforeEach ->
@@ -145,7 +148,7 @@ describe "DocstoreManager", ->
 					.should.equal true
 
 			it "should call the callback with the lines, version and rev", ->
-				@callback.calledWith(null, @lines, @rev).should.equal true
+				@callback.calledWith(null, @lines, @rev, @version).should.equal true
 
 	describe "getAllDocs", ->
 		describe "with a successful response code", ->

services/web/test/UnitTests/coffee/Documents/DocumentControllerTests.coffee

@@ -33,7 +33,7 @@ describe "DocumentController", ->
 
 		describe "when the document exists", ->
 			beforeEach ->
-				@ProjectEntityHandler.getDoc = sinon.stub().callsArgWith(2, null, @doc_lines, @rev)
+				@ProjectEntityHandler.getDoc = sinon.stub().callsArgWith(2, null, @doc_lines, @rev, @version)
 				@DocumentController.getDocument(@req, @res, @next)
 
 			it "should get the document from Mongo", ->
@@ -45,6 +45,7 @@ describe "DocumentController", ->
 				@res.type.should.equal "json"
 				@res.body.should.equal JSON.stringify
 					lines: @doc_lines
+					version: @version
 
 		describe "when the document doesn't exist", ->
 			beforeEach ->
@@ -63,14 +64,15 @@ describe "DocumentController", ->
 
 		describe "when the document exists", ->
 			beforeEach ->
-				@ProjectEntityHandler.updateDocLines = sinon.stub().callsArg(3)
+				@ProjectEntityHandler.updateDocLines = sinon.stub().yields()
 				@req.body =
 					lines: @doc_lines
+					version: @version
 				@DocumentController.setDocument(@req, @res, @next)
 
 			it "should update the document in Mongo", ->
 				@ProjectEntityHandler.updateDocLines
-					.calledWith(@project_id, @doc_id, @doc_lines)
+					.calledWith(@project_id, @doc_id, @doc_lines, @version)
 					.should.equal true
 
 			it "should return a successful response", ->
@@ -78,7 +80,7 @@ describe "DocumentController", ->
 
 		describe "when the document doesn't exist", ->
 			beforeEach ->
-				@ProjectEntityHandler.updateDocLines = sinon.stub().callsArgWith(3, new Errors.NotFoundError("document does not exist"))
+				@ProjectEntityHandler.updateDocLines = sinon.stub().yields(new Errors.NotFoundError("document does not exist"))
 				@req.body =
 					lines: @doc_lines
 				@DocumentController.setDocument(@req, @res, @next)

services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee

@@ -402,7 +402,7 @@ describe 'ProjectEntityHandler', ->
 			@ProjectEntityHandler._putElement = sinon.stub().callsArgWith(4, null, {path:{fileSystem:@path}})
 			@callback = sinon.stub()
 			@tpdsUpdateSender.addDoc = sinon.stub().callsArg(1)
-			@DocstoreManager.updateDoc = sinon.stub().callsArgWith(3, null, true, 0)
+			@DocstoreManager.updateDoc = sinon.stub().yields(null, true, 0)
 
 			@ProjectEntityHandler.addDoc project_id, folder_id, @name, @lines, @callback
 
@@ -589,6 +589,7 @@ describe 'ProjectEntityHandler', ->
 			@doc = {
 				_id: doc_id
 			}
+			@version = 42
 			@ProjectGetter.getProjectWithoutDocLines = sinon.stub().callsArgWith(1, null, @project)
 			@projectLocator.findElement = sinon.stub().callsArgWith(1, null, @doc, {fileSystem: @path})
 			@tpdsUpdateSender.addDoc = sinon.stub().callsArg(1)
@@ -597,8 +598,8 @@ describe 'ProjectEntityHandler', ->
 
 		describe "when the doc has been modified", ->
 			beforeEach ->
-				@DocstoreManager.updateDoc = sinon.stub().callsArgWith(3, null, true, @rev = 5)
-				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @callback
+				@DocstoreManager.updateDoc = sinon.stub().yields(null, true, @rev = 5)
+				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @callback
 
 			it "should get the project without doc lines", ->
 				@ProjectGetter.getProjectWithoutDocLines
@@ -616,7 +617,7 @@ describe 'ProjectEntityHandler', ->
 
 			it "should update the doc in the docstore", ->
 				@DocstoreManager.updateDoc
-					.calledWith(project_id, doc_id, @lines)
+					.calledWith(project_id, doc_id, @lines, @version)
 					.should.equal true
 
 			it "should mark the project as updated", ->
@@ -640,8 +641,8 @@ describe 'ProjectEntityHandler', ->
 
 		describe "when the doc has not been modified", ->
 			beforeEach ->
-				@DocstoreManager.updateDoc = sinon.stub().callsArgWith(3, null, false, @rev = 5)
-				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @callback
+				@DocstoreManager.updateDoc = sinon.stub().yields(null, false, @rev = 5)
+				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @callback
 
 			it "should not mark the project as updated", ->
 				@projectUpdater.markAsUpdated.called.should.equal false
@@ -655,7 +656,7 @@ describe 'ProjectEntityHandler', ->
 		describe "when the project is not found", ->
 			beforeEach ->
 				@ProjectGetter.getProjectWithoutDocLines = sinon.stub().callsArgWith(1, null, null)
-				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @callback
+				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @callback
 
 			it "should return a not found error", ->
 				@callback.calledWith(new Errors.NotFoundError()).should.equal true
@@ -663,7 +664,7 @@ describe 'ProjectEntityHandler', ->
 		describe "when the doc is not found", ->
 			beforeEach ->
 				@projectLocator.findElement = sinon.stub().callsArgWith(1, null, null, null)
-				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @callback
+				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @callback
 
 			it "should log out the error", ->
 				@logger.error

services/web/test/UnitTests/coffee/Subscription/RecurlyWrapperTests.coffee

@@ -314,6 +314,19 @@ describe "RecurlyWrapper", ->
 		it "should send a cancel request to the API", ->
 			@apiRequest.called.should.equal true
 
+		describe 'when the subscription is already cancelled', ->
+
+			beforeEach ->
+				@RecurlyWrapper.apiRequest.restore()
+				@recurlySubscriptionId = "subscription-id-123"
+				@apiRequest = sinon.stub @RecurlyWrapper, "apiRequest", (options, callback) =>
+					callback(new Error('woops'), {}, "<error><description>A canceled subscription can't transition to canceled</description></error>")
+
+			it 'should not produce an error', (done) ->
+				@RecurlyWrapper.cancelSubscription @recurlySubscriptionId, (err) =>
+					expect(err).to.equal null
+					done()
+
 	describe "reactivateSubscription", ->
 		beforeEach (done) ->
 			@recurlySubscriptionId = "subscription-id-123"

services/web/test/UnitTests/coffee/User/UserControllerTests.coffee

@@ -89,6 +89,8 @@ describe "UserController", ->
 
 		beforeEach ->
 			@req.body.password = 'wat'
+			@req.logout = sinon.stub()
+			@req.session.destroy = sinon.stub().callsArgWith(0, null)
 			@AuthenticationController.getLoggedInUserId = sinon.stub().returns(@user._id)
 			@AuthenticationManager.authenticate = sinon.stub().callsArgWith(2, null, @user)
 			@UserDeleter.deleteUser = sinon.stub().callsArgWith(1, null)
@@ -159,6 +161,17 @@ describe "UserController", ->
 					done()
 				@UserController.tryDeleteUser @req, @res, @next
 
+		describe 'when session.destroy produces an error', ->
+
+			beforeEach ->
+				@req.session.destroy = sinon.stub().callsArgWith(0, new Error('woops'))
+
+			it 'should call next with an error', (done) ->
+				@next = (err) =>
+					expect(err).to.not.equal null
+					expect(err).to.be.instanceof Error
+					done()
+				@UserController.tryDeleteUser @req, @res, @next
 
 	describe "unsubscribe", ->
 

services/web/test/UnitTests/coffee/User/UserPagesControllerTests.coffee

@@ -30,8 +30,10 @@ describe "UserPagesController", ->
 		@AuthenticationController =
 			getLoggedInUserId: sinon.stub().returns(@user._id)
 			getSessionUser: sinon.stub().returns(@user)
+			_getRedirectFromSession: sinon.stub()
+			_setRedirectInSession: sinon.stub()
 		@UserPagesController = SandboxedModule.require modulePath, requires:
-			"settings-sharelatex":@settings
+			"settings-sharelatex": @settings
 			"logger-sharelatex":
 				log:->
 				err:->
@@ -56,14 +58,6 @@ describe "UserPagesController", ->
 				done()
 			@UserPagesController.registerPage @req, @res
 
-		it "should set the redirect", (done)->
-			redirect = "/go/here/please"
-			@req.query.redir = redirect
-			@res.render = (page, opts)=>
-				opts.redir.should.equal redirect
-				done()
-			@UserPagesController.registerPage @req, @res
-
 		it "should set sharedProjectData", (done)->
 			@req.query.project_name = "myProject"
 			@req.query.user_first_name = "user_first_name_here"
@@ -98,13 +92,19 @@ describe "UserPagesController", ->
 				done()
 			@UserPagesController.loginPage @req, @res
 
-		it "should set the redirect", (done)->
-			redirect = "/go/here/please"
-			@req.query.redir = redirect
-			@res.render = (page, opts)=>
-				opts.redir.should.equal redirect
-				done()
-			@UserPagesController.loginPage @req, @res
+		describe 'when an explicit redirect is set via query string', ->
+
+			beforeEach ->
+				@AuthenticationController._getRedirectFromSession = sinon.stub().returns(null)
+				@AuthenticationController._setRedirectInSession = sinon.stub()
+				@req.query.redir = '/somewhere/in/particular'
+
+			it 'should set a redirect', (done) ->
+				@res.render = (page) =>
+					@AuthenticationController._setRedirectInSession.callCount.should.equal 1
+					expect(@AuthenticationController._setRedirectInSession.lastCall.args[1]).to.equal @req.query.redir
+					done()
+				@UserPagesController.loginPage @req, @res
 
 	describe 'sessionsPage', ->
 
@@ -149,6 +149,40 @@ describe "UserPagesController", ->
 				done()
 			@UserPagesController.settingsPage @req, @res
 
+		it "should set 'shouldAllowEditingDetails' to true", (done)->
+			@res.render = (page, opts)=>
+				opts.shouldAllowEditingDetails.should.equal true
+				done()
+			@UserPagesController.settingsPage @req, @res
+
+		describe 'when ldap.updateUserDetailsOnLogin is true', ->
+
+			beforeEach ->
+				@settings.ldap = {updateUserDetailsOnLogin: true}
+
+			afterEach ->
+				delete @settings.ldap
+
+			it 'should set "shouldAllowEditingDetails" to false', (done) ->
+				@res.render = (page, opts)=>
+					opts.shouldAllowEditingDetails.should.equal false
+					done()
+				@UserPagesController.settingsPage @req, @res
+
+		describe 'when saml.updateUserDetailsOnLogin is true', ->
+
+			beforeEach ->
+				@settings.saml = {updateUserDetailsOnLogin: true}
+
+			afterEach ->
+				delete @settings.saml
+
+			it 'should set "shouldAllowEditingDetails" to false', (done) ->
+				@res.render = (page, opts)=>
+					opts.shouldAllowEditingDetails.should.equal false
+					done()
+				@UserPagesController.settingsPage @req, @res
+
 	describe "activateAccountPage", ->
 		beforeEach ->
 			@req.query.user_id = @user_id

services/web/test/acceptance/coffee/ProjectInviteTests.coffee

@@ -60,7 +60,7 @@ tryAcceptInvite = (user, invite, callback=(err, response, body)->) ->
 			token: invite.token
 	}, callback
 
-tryRegisterUser = (user, email, redir, callback=(err, response, body)->) ->
+tryRegisterUser = (user, email, callback=(err, response, body)->) ->
 	user.getCsrfToken (error) =>
 		return callback(error) if error?
 		user.request.post {
@@ -68,7 +68,6 @@ tryRegisterUser = (user, email, redir, callback=(err, response, body)->) ->
 			json:
 				email: email
 				password: "some_weird_password"
-				redir: redir
 		}, callback
 
 tryFollowLoginLink = (user, loginLink, callback=(err, response, body)->) ->
@@ -76,7 +75,7 @@ tryFollowLoginLink = (user, loginLink, callback=(err, response, body)->) ->
 		return callback(error) if error?
 		user.request.get loginLink, callback
 
-tryLoginUser = (user, redir, callback=(err, response, body)->) ->
+tryLoginUser = (user, callback=(err, response, body)->) ->
 	user.getCsrfToken (error) =>
 		return callback(error) if error?
 		user.request.post {
@@ -84,7 +83,6 @@ tryLoginUser = (user, redir, callback=(err, response, body)->) ->
 			json:
 				email: user.email
 				password: user.password
-				redir: redir
 		}, callback
 
 tryGetInviteList = (user, projectId, callback=(err, response, body)->) ->
@@ -143,35 +141,28 @@ expectInviteRedirectToRegister = (user, link, callback=(err,result)->) ->
 	tryFollowInviteLink user, link, (err, response, body) ->
 		expect(err).to.be.oneOf [null, undefined]
 		expect(response.statusCode).to.equal 302
-		expect(response.headers.location).to.match new RegExp("^/register\?.*redir=.*$")
+		expect(response.headers.location).to.match new RegExp("^/register.*$")
 		# follow redirect to register page and extract the redirectUrl from form
 		user.request.get response.headers.location, (err, response, body) ->
-			redirectUrl = body.match(/input name="redir" type="hidden" value="([^"]*)"/m)?[1]
-			loginUrl = body.match(/href="([^"]*)">\s*Login here/m)?[1]
-			expect(redirectUrl).to.not.be.oneOf [null, undefined]
-			expect(loginUrl).to.not.be.oneOf [null, undefined]
-			callback(null, redirectUrl, loginUrl)
+			callback(null)
 
-expectLoginPage = (user, loginLink, callback=(err, result)->) ->
-	tryFollowLoginLink user, loginLink, (err, response, body) ->
+expectLoginPage = (user, callback=(err, result)->) ->
+	tryFollowLoginLink user, "/login", (err, response, body) ->
 		expect(err).to.be.oneOf [null, undefined]
 		expect(response.statusCode).to.equal 200
 		expect(body).to.match new RegExp("<title>Login - .*</title>")
-		redirectUrl = body.match(/input name="redir" type="hidden" value="([^"]*)"/m)?[1]
-		callback(null, redirectUrl)
+		callback(null)
 
-expectLoginRedirectToInvite = (user, redir, link, callback=(err, result)->) ->
-	tryLoginUser user, redir, (err, response, body) ->
+expectLoginRedirectToInvite = (user, link, callback=(err, result)->) ->
+	tryLoginUser user, (err, response, body) ->
 		expect(err).to.be.oneOf [null, undefined]
 		expect(response.statusCode).to.equal 200
-		expect(link).to.match new RegExp("^.*#{body.redir}\?.*$")
 		callback(null, null)
 
-expectRegistrationRedirectToInvite = (user, email, redir, link, callback=(err, result)->) ->
-	tryRegisterUser user, email, redir, (err, response, body) ->
+expectRegistrationRedirectToInvite = (user, email, link, callback=(err, result)->) ->
+	tryRegisterUser user, email, (err, response, body) ->
 		expect(err).to.be.oneOf [null, undefined]
 		expect(response.statusCode).to.equal 200
-		expect(link).to.match new RegExp("^.*#{body.redir}\?.*$")
 		callback(null, null)
 
 expectInviteRedirectToProject = (user, link, invite, callback=(err,result)->) ->
@@ -433,11 +424,8 @@ describe "ProjectInviteTests", ->
 
 				it 'should allow user to accept the invite if the user registers a new account', (done) ->
 					Async.series [
-						(cb) =>
-							expectInviteRedirectToRegister @user, @link, (err, redirectUrl) =>
-								@_redir = redirectUrl
-								cb()
-						(cb) => expectRegistrationRedirectToInvite @user, "some_email@example.com", @_redir, @link, cb
+						(cb) => expectInviteRedirectToRegister @user, @link, cb
+						(cb) => expectRegistrationRedirectToInvite @user, "some_email@example.com", @link, cb
 						(cb) => expectInvitePage @user, @link, cb
 						(cb) => expectAcceptInviteAndRedirect @user, @invite, cb
 						(cb) => expectProjectAccess @user, @invite.projectId, cb
@@ -457,11 +445,8 @@ describe "ProjectInviteTests", ->
 				it 'should display invalid-invite if the user registers a new account', (done) ->
 					badLink = @link.replace(@invite.token, 'not_a_real_token')
 					Async.series [
-						(cb) =>
-							expectInviteRedirectToRegister @user, badLink, (err, redirectUrl) =>
-								@_redir = redirectUrl
-								cb()
-						(cb) => expectRegistrationRedirectToInvite @user, "some_email@example.com", @_redir, badLink, cb
+						(cb) => expectInviteRedirectToRegister @user, badLink, cb
+						(cb) => expectRegistrationRedirectToInvite @user, "some_email@example.com", badLink, cb
 						(cb) => expectInvalidInvitePage @user, badLink, cb
 						(cb) => expectNoProjectAccess @user, @invite.projectId, cb
 					], done
@@ -479,16 +464,9 @@ describe "ProjectInviteTests", ->
 
 				it 'should allow the user to login to view the invite', (done) ->
 					Async.series [
-						(cb) =>
-							expectInviteRedirectToRegister @user, @link, (err, redirectUrl, loginUrl) =>
-								@_redir = redirectUrl
-								@_loginLink = loginUrl
-								cb()
-						(cb) =>
-							expectLoginPage @user, @_loginLink, (err, redirectUrl) =>
-								expect(@_redir).to.equal redirectUrl
-								cb()
-						(cb) => expectLoginRedirectToInvite @user, @_redir, @link, cb
+						(cb) => expectInviteRedirectToRegister @user, @link, cb
+						(cb) => expectLoginPage @user, cb
+						(cb) => expectLoginRedirectToInvite @user, @link, cb
 						(cb) => expectInvitePage @user, @link, cb
 						(cb) => expectNoProjectAccess @user, @invite.projectId, cb
 					], done
@@ -515,15 +493,10 @@ describe "ProjectInviteTests", ->
 					badLink = @link.replace(@invite.token, 'not_a_real_token')
 					Async.series [
 						(cb) =>
-							expectInviteRedirectToRegister @user, badLink, (err, redirectUrl, loginUrl) =>
-								@_redir = redirectUrl
-								@_loginLink = loginUrl
-								cb()
+							expectInviteRedirectToRegister @user, badLink, cb
 						(cb) =>
-							expectLoginPage @user, @_loginLink, (err, redirectUrl) =>
-								expect(@_redir).to.equal redirectUrl
-								cb()
-						(cb) => expectLoginRedirectToInvite @user, @_redir, badLink, cb
+							expectLoginPage @user, cb
+						(cb) => expectLoginRedirectToInvite @user, badLink, cb
 						(cb) => expectInvalidInvitePage @user, badLink, cb
 						(cb) => expectNoProjectAccess @user, @invite.projectId, cb
 					], done