Просмотр исходного кода

Keep password reset token in session, and strip it from reset page url.

This fixes an issue where the reset token was leaked in the referrer header
when navigating away from the password reset page to an external site.

Now we get the token from the query string, store it in the session,
then redirect to the bare url of the password reset page, which then
uses the stored token to render the reset form.
Shane Kilkelly 11 лет назад
Родитель
Сommit
0aaeb6671e

+ 11 - 5
services/web/app/coffee/Features/PasswordReset/PasswordResetController.coffee

@@ -6,12 +6,12 @@ module.exports =
 
 
 	renderRequestResetForm: (req, res)->
 	renderRequestResetForm: (req, res)->
 		logger.log "rendering request reset form"
 		logger.log "rendering request reset form"
-		res.render "user/passwordReset", 
+		res.render "user/passwordReset",
 			title:"reset_password"
 			title:"reset_password"
 
 
 	requestReset: (req, res)->
 	requestReset: (req, res)->
 		email = req.body.email.trim().toLowerCase()
 		email = req.body.email.trim().toLowerCase()
-		opts = 
+		opts =
 			endpointName: "password_reset_rate_limit"
 			endpointName: "password_reset_rate_limit"
 			timeInterval: 60
 			timeInterval: 60
 			subjectName: req.ip
 			subjectName: req.ip
@@ -28,17 +28,23 @@ module.exports =
 					res.send 404, {message: req.i18n.translate("cant_find_email")}
 					res.send 404, {message: req.i18n.translate("cant_find_email")}
 
 
 	renderSetPasswordForm: (req, res)->
 	renderSetPasswordForm: (req, res)->
-		res.render "user/setPassword", 
+		if req.query.passwordResetToken?
+			req.session.resetToken = req.query.passwordResetToken
+			return res.redirect('/user/password/set')
+		if !req.session.resetToken?
+			return res.redirect('/user/password/reset')
+		res.render "user/setPassword",
 			title:"set_password"
 			title:"set_password"
-			passwordResetToken:req.query.passwordResetToken
+			passwordResetToken: req.session.resetToken
 
 
 	setNewUserPassword: (req, res)->
 	setNewUserPassword: (req, res)->
 		{passwordResetToken, password} = req.body
 		{passwordResetToken, password} = req.body
 		if !password? or password.length == 0 or !passwordResetToken? or passwordResetToken.length == 0
 		if !password? or password.length == 0 or !passwordResetToken? or passwordResetToken.length == 0
 			return res.sendStatus 400
 			return res.sendStatus 400
+		delete req.session.resetToken
 		PasswordResetHandler.setNewUserPassword passwordResetToken?.trim(), password?.trim(), (err, found) ->
 		PasswordResetHandler.setNewUserPassword passwordResetToken?.trim(), password?.trim(), (err, found) ->
 			return next(err) if err?
 			return next(err) if err?
 			if found
 			if found
 				res.sendStatus 200
 				res.sendStatus 200
 			else
 			else
-				res.send 404, {message: req.i18n.translate("password_reset_token_expired")}
+				res.send 404, {message: req.i18n.translate("password_reset_token_expired")}

+ 46 - 1
services/web/test/UnitTests/coffee/PasswordReset/PasswordResetControllerTests.coffee

@@ -14,7 +14,7 @@ describe "PasswordResetController", ->
 		@PasswordResetHandler =
 		@PasswordResetHandler =
 			generateAndEmailResetToken:sinon.stub()
 			generateAndEmailResetToken:sinon.stub()
 			setNewUserPassword:sinon.stub()
 			setNewUserPassword:sinon.stub()
-		@RateLimiter = 
+		@RateLimiter =
 			addCount: sinon.stub()
 			addCount: sinon.stub()
 		@PasswordResetController = SandboxedModule.require modulePath, requires:
 		@PasswordResetController = SandboxedModule.require modulePath, requires:
 			"settings-sharelatex":@settings
 			"settings-sharelatex":@settings
@@ -32,6 +32,8 @@ describe "PasswordResetController", ->
 				password:@password
 				password:@password
 			i18n:
 			i18n:
 				translate:->
 				translate:->
+			session: {}
+			query: {}
 
 
 		@res = {}
 		@res = {}
 
 
@@ -86,6 +88,9 @@ describe "PasswordResetController", ->
 
 
 	describe "setNewUserPassword", ->
 	describe "setNewUserPassword", ->
 
 
+		beforeEach ->
+			@req.session.resetToken = @token
+
 		it "should tell the user handler to reset the password", (done)->
 		it "should tell the user handler to reset the password", (done)->
 			@PasswordResetHandler.setNewUserPassword.callsArgWith(2, null, true)
 			@PasswordResetHandler.setNewUserPassword.callsArgWith(2, null, true)
 			@res.sendStatus = (code)=>
 			@res.sendStatus = (code)=>
@@ -119,5 +124,45 @@ describe "PasswordResetController", ->
 				done()
 				done()
 			@PasswordResetController.setNewUserPassword @req, @res
 			@PasswordResetController.setNewUserPassword @req, @res
 
 
+		it "should clear the session.resetToken", (done) ->
+			@PasswordResetHandler.setNewUserPassword.callsArgWith(2, null, true)
+			@res.sendStatus = (code)=>
+				code.should.equal 200
+				@req.session.should.not.have.property 'resetToken'
+				done()
+			@PasswordResetController.setNewUserPassword @req, @res
+
+	describe "renderSetPasswordForm", ->
+
+		describe "with token in query-string", ->
+			beforeEach ->
+				@req.query.passwordResetToken = @token
+
+			it "should set session.resetToken and redirect", (done) ->
+				@req.session.should.not.have.property 'resetToken'
+				@res.redirect = (path) =>
+					path.should.equal '/user/password/set'
+					@req.session.resetToken.should.equal @token
+					done()
+				@PasswordResetController.renderSetPasswordForm(@req, @res)
+
+		describe "without a token in query-string", ->
+
+			describe "with token in session", ->
+				beforeEach ->
+					@req.session.resetToken = @token
+
+				it "should render the page, passing the reset token", (done) ->
+					@res.render = (template_path, options) =>
+						options.passwordResetToken.should.equal @req.session.resetToken
+						done()
+					@PasswordResetController.renderSetPasswordForm(@req, @res)
 
 
+			describe "without a token in session", ->
 
 
+				it "should redirect to the reset request page", (done) ->
+					@res.redirect = (path) =>
+						path.should.equal "/user/password/reset"
+						@req.session.should.not.have.property 'resetToken'
+						done()
+					@PasswordResetController.renderSetPasswordForm(@req, @res)