Merge pull request #677 from sharelatex/ja-store-tokens-in-mongo

Store OneTimeTokens in mongo rather than redis

services/web/app/coffee/Features/Security/OneTimeTokenHandler.coffee

@@ -1,34 +1,50 @@
 Settings = require('settings-sharelatex')
-RedisWrapper = require("../../infrastructure/RedisWrapper")
-rclient = RedisWrapper.client("one_time_token")
 crypto = require("crypto")
 logger = require("logger-sharelatex")
+{db} = require "../../infrastructure/mongojs"
+Errors = require "../Errors/Errors"
 
 ONE_HOUR_IN_S = 60 * 60
 
-buildKey = (use, token)-> return "#{use}_token:#{token}"
-
 module.exports =
-
+	getNewToken: (use, data, options = {}, callback = (error, data) ->)->
-	getNewToken: (use, value, options = {}, callback)->
 		# options is optional
 		if typeof options == "function"
 			callback = options
 			options = {}
 		expiresIn = options.expiresIn or ONE_HOUR_IN_S
+		createdAt = new Date()
+		expiresAt = new Date(createdAt.getTime() + expiresIn * 1000)
 		token = crypto.randomBytes(32).toString("hex")
-		logger.log {value, expiresIn, token_start: token.slice(0,8)}, "generating token for #{use}"
+		logger.log {data, expiresIn, token_start: token.slice(0,8)}, "generating token for #{use}"
-		multi = rclient.multi()
+		db.tokens.insert {
-		multi.set buildKey(use, token), value
+			use: use
-		multi.expire buildKey(use, token), expiresIn
+			token: token,
-		multi.exec (err)->
+			data: data,
-			callback(err, token)
+			createdAt: createdAt,
+			expiresAt: expiresAt
+		}, (error) ->
+			return callback(error) if error?
+			callback null, token
 
-	getValueFromTokenAndExpire: (use, token, callback)->
+	getValueFromTokenAndExpire: (use, token, callback = (error, data) ->)->
-		logger.log token_start: token.slice(0,8), "getting value from #{use} token"
+		logger.log token_start: token.slice(0,8), "getting data from #{use} token"
-		multi = rclient.multi()
+		now = new Date()
-		multi.get buildKey(use, token)
+		db.tokens.findAndModify {
-		multi.del buildKey(use, token)
+			query: {
-		multi.exec (err, results)->
+				use: use,
-			callback err, results?[0]
+				token: token,
+				expiresAt: { $gt: now },
+				usedAt: { $exists: false }
+			},
+			update: {
+				$set: {
+					usedAt: now
+				}
+			}
+		}, (error, token) ->
+			return callback(error) if error?
+			if !token?
+				return callback(new Errors.NotFoundError('no token found'))
+			return callback null, token.data
 

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

@@ -9,20 +9,14 @@ UserUpdater = require "./UserUpdater"
 ONE_YEAR_IN_S = 365 * 24 * 60 * 60
 
 module.exports = UserEmailsConfirmationHandler =
-	serializeData: (user_id, email) ->
-		JSON.stringify({user_id, email})
-
-	deserializeData: (data) ->
-		JSON.parse(data)
-
 	sendConfirmationEmail: (user_id, email, emailTemplate, callback = (error) ->) ->
 		if arguments.length == 3
 			callback = emailTemplate
 			emailTemplate = 'confirmEmail'
 		email = EmailHelper.parseEmail(email)
 		return callback(new Error('invalid email')) if !email?
-		value = UserEmailsConfirmationHandler.serializeData(user_id, email)
+		data = {user_id, email}
-		OneTimeTokenHandler.getNewToken 'email_confirmation', value, {expiresIn: ONE_YEAR_IN_S}, (err, token)->
+		OneTimeTokenHandler.getNewToken 'email_confirmation', data, {expiresIn: ONE_YEAR_IN_S}, (err, token)->
 			return callback(err) if err?
 			emailOptions =
 				to: email
@@ -35,7 +29,7 @@ module.exports = UserEmailsConfirmationHandler =
 			return callback(error) if error?
 			if !data?
 				return callback(new Errors.NotFoundError('no token found'))
-			{user_id, email} = UserEmailsConfirmationHandler.deserializeData(data)
+			{user_id, email} = data
 			logger.log {data, user_id, email, token_start: token.slice(0,8)}, 'found data for email confirmation'
 			if !user_id? or email != EmailHelper.parseEmail(email)
 				return callback(new Errors.NotFoundError('invalid data'))

services/web/app/coffee/infrastructure/mongojs.coffee

@@ -1,6 +1,6 @@
 Settings = require "settings-sharelatex"
 mongojs = require "mongojs"
-db = mongojs(Settings.mongo.url, ["projects", "users", "userstubs"])
+db = mongojs(Settings.mongo.url, ["projects", "users", "userstubs", "tokens"])
 module.exports =
 	db: db
 	ObjectId: mongojs.ObjectId

services/web/test/acceptance/coffee/UserEmailsTests.coffee

@@ -3,7 +3,7 @@ async = require("async")
 User = require "./helpers/User"
 request = require "./helpers/request"
 settings = require "settings-sharelatex"
-rclient = require("redis-sharelatex").createClient(settings.redis.web)
+{db, ObjectId} = require("../../../app/js/infrastructure/mongojs")
 
 describe "UserEmails", ->
 	beforeEach (done) ->
@@ -32,10 +32,15 @@ describe "UserEmails", ->
 						expect(body[1].confirmedAt).to.not.exist
 						cb()
 				(cb) =>
-					rclient.keys 'email_confirmation_token:*', (error, keys) ->
+					db.tokens.find {
+						use: 'email_confirmation',
+						usedAt: { $exists: false }
+					}, (error, tokens) =>
 						# There should only be one confirmation token at the moment
-						expect(keys.length).to.equal 1
+						expect(tokens.length).to.equal 1
-						token = keys[0].split(':')[1]
+						expect(tokens[0].data.email).to.equal 'newly-added-email@example.com'
+						expect(tokens[0].data.user_id).to.equal @user._id
+						token = tokens[0].token
 						cb()
 				(cb) =>
 					@user.request { 
@@ -54,9 +59,12 @@ describe "UserEmails", ->
 						expect(body[1].confirmedAt).to.exist
 						cb()
 				(cb) =>
-					rclient.keys 'email_confirmation_token:*', (error, keys) ->
+					db.tokens.find {
+						use: 'email_confirmation',
+						usedAt: { $exists: false }
+					}, (error, tokens) =>
 						# Token should be deleted after use
-						expect(keys.length).to.equal 0
+						expect(tokens.length).to.equal 0
 						cb()
 			], done
 
@@ -75,11 +83,15 @@ describe "UserEmails", ->
 						json: {@email}
 					}, cb
 				(cb) =>
-					rclient.keys 'email_confirmation_token:*', (error, keys) ->
+					db.tokens.find {
-						# There should only be one confirmation token at the moment,
+						use: 'email_confirmation',
-						# for the first user
+						usedAt: { $exists: false }
-						expect(keys.length).to.equal 1
+					}, (error, tokens) =>
-						token1 = keys[0].split(':')[1]
+						# There should only be one confirmation token at the moment
+						expect(tokens.length).to.equal 1
+						expect(tokens[0].data.email).to.equal @email
+						expect(tokens[0].data.user_id).to.equal @user._id
+						token1 = tokens[0].token
 						cb()
 				(cb) =>
 					# Delete the email from the first user
@@ -107,14 +119,19 @@ describe "UserEmails", ->
 						expect(response.statusCode).to.equal 404
 						cb()
 				(cb) =>
-					rclient.keys 'email_confirmation_token:*', (error, keys) ->
+					db.tokens.find {
+						use: 'email_confirmation',
+						usedAt: { $exists: false }
+					}, (error, tokens) =>
 						# The first token has been used, so this should be token2 now
-						expect(keys.length).to.equal 1
+						expect(tokens.length).to.equal 1
-						token2 = keys[0].split(':')[1]
+						expect(tokens[0].data.email).to.equal @email
+						expect(tokens[0].data.user_id).to.equal @user2._id
+						token2 = tokens[0].token
 						cb()
 				(cb) =>
 					# Second user should be able to confirm the email
-					@user2.request { 
+					@user2.request {
 						method: 'POST',
 						url: '/user/emails/confirm',
 						json:
@@ -130,3 +147,48 @@ describe "UserEmails", ->
 						expect(body[1].confirmedAt).to.exist
 						cb()
 			], done
+
+	describe "with an expired token", ->
+		it 'should not confirm the email', (done) ->
+			token = null
+			async.series [
+				(cb) =>
+					@user.request { 
+						method: 'POST',
+						url: '/user/emails',
+						json:
+							email: @email = 'expired-token-email@example.com'
+					}, (error, response, body) =>
+						return done(error) if error?
+						expect(response.statusCode).to.equal 204
+						cb()
+				(cb) =>
+					db.tokens.find {
+						use: 'email_confirmation',
+						usedAt: { $exists: false }
+					}, (error, tokens) =>
+						# There should only be one confirmation token at the moment
+						expect(tokens.length).to.equal 1
+						expect(tokens[0].data.email).to.equal @email
+						expect(tokens[0].data.user_id).to.equal @user._id
+						token = tokens[0].token
+						cb()
+				(cb) =>
+					db.tokens.update {
+						token: token
+					}, {
+						$set: { 
+							expiresAt: new Date(Date.now() - 1000000)
+						}
+					}, cb
+				(cb) =>
+					@user.request { 
+						method: 'POST',
+						url: '/user/emails/confirm',
+						json:
+							token: token
+					}, (error, response, body) =>
+						return done(error) if error?
+						expect(response.statusCode).to.equal 404
+						cb()
+			], done

services/web/test/unit/coffee/Security/OneTimeTokenHandlerTests.coffee

@@ -5,64 +5,99 @@ path = require('path')
 sinon = require('sinon')
 modulePath = path.join __dirname, "../../../../app/js/Features/Security/OneTimeTokenHandler"
 expect = require("chai").expect
+Errors = require "../../../../app/js/Features/Errors/Errors"
+tk = require("timekeeper")
 
 describe "OneTimeTokenHandler", ->
-
 	beforeEach ->
-		@value = "user id here"
+		tk.freeze Date.now() # freeze the time for these tests
-		@stubbedToken = require("crypto").randomBytes(32)
+		@stubbedToken = "mock-token"
-
+		@callback = sinon.stub()
-		@settings = 
-			redis:
-				web:{}
-		@redisMulti =
-			set:sinon.stub()
-			get:sinon.stub()
-			del:sinon.stub()
-			expire:sinon.stub()
-			exec:sinon.stub()
-		self = @
 		@OneTimeTokenHandler = SandboxedModule.require modulePath, requires:
-			"../../infrastructure/RedisWrapper" :
-				client: =>
-					auth:->
-					multi: -> return self.redisMulti
-
 			"settings-sharelatex":@settings
 			"logger-sharelatex": log:->
 			"crypto": randomBytes: () => @stubbedToken
+			"../Errors/Errors": Errors
+			"../../infrastructure/mongojs": db: @db = tokens: {}
 
+	afterEach ->
+		tk.reset()
 
 	describe "getNewToken", ->
+		beforeEach ->
+			@db.tokens.insert = sinon.stub().yields()
 
-		it "should set a new token into redis with a ttl", (done)->
+		describe 'normally', ->
-			@redisMulti.exec.callsArgWith(0) 
+			beforeEach ->
-			@OneTimeTokenHandler.getNewToken 'password', @value, (err, token) =>
+				@OneTimeTokenHandler.getNewToken 'password', 'mock-data-to-store', @callback
-				@redisMulti.set.calledWith("password_token:#{@stubbedToken.toString("hex")}", @value).should.equal true
-				@redisMulti.expire.calledWith("password_token:#{@stubbedToken.toString("hex")}", 60 * 60).should.equal true
-				done()
 
-		it "should return if there was an error", (done)->
+			it "should insert a generated token with a 1 hour expiry", ->
-			@redisMulti.exec.callsArgWith(0, "error")
+				@db.tokens.insert
-			@OneTimeTokenHandler.getNewToken 'password', @value, (err, token)=>
+					.calledWith({
-				err.should.exist
+						use: 'password'
-				done()
+						token: @stubbedToken,
+						createdAt: new Date(),
+						expiresAt: new Date(Date.now() + 60 * 60 * 1000)
+						data: 'mock-data-to-store'
+					})
+					.should.equal true
 
-		it "should allow the expiry time to be overridden", (done) ->
+			it 'should call the callback with the token', ->
-			@redisMulti.exec.callsArgWith(0) 
+				@callback.calledWith(null, @stubbedToken).should.equal true
-			@ttl = 42
+
-			@OneTimeTokenHandler.getNewToken 'password', @value, {expiresIn: @ttl}, (err, token) =>
+		describe 'with an optional expiresIn parameter', ->
-				@redisMulti.expire.calledWith("password_token:#{@stubbedToken.toString("hex")}", @ttl).should.equal true
+			beforeEach ->
-				done()
+				@OneTimeTokenHandler.getNewToken 'password', 'mock-data-to-store', { expiresIn: 42 }, @callback
+
+			it "should insert a generated token with a custom expiry", ->
+				@db.tokens.insert
+					.calledWith({
+						use: 'password'
+						token: @stubbedToken,
+						createdAt: new Date(),
+						expiresAt: new Date(Date.now() + 42 * 1000)
+						data: 'mock-data-to-store'
+					})
+					.should.equal true
+
+			it 'should call the callback with the token', ->
+				@callback.calledWith(null, @stubbedToken).should.equal true
 
 	describe "getValueFromTokenAndExpire", ->
+		describe 'successfully', ->
+			beforeEach ->
+				@db.tokens.findAndModify = sinon.stub().yields(null, { data: 'mock-data' })
+				@OneTimeTokenHandler.getValueFromTokenAndExpire 'password', 'mock-token', @callback
+
+			it 'should expire the token', ->
+				@db.tokens.findAndModify
+					.calledWith({
+						query: {
+							use: 'password'
+							token: 'mock-token',
+							expiresAt: { $gt: new Date() },
+							usedAt: { $exists: false }
+						},
+						update: {
+							$set: { usedAt: new Date() }
+						}
+					})
+					.should.equal true
+
+			it 'should return the data', ->
+				@callback.calledWith(null, 'mock-data').should.equal true
+
+		describe 'when a valid token is not found', ->
+			beforeEach ->
+				@db.tokens.findAndModify = sinon.stub().yields(null, null)
+				@OneTimeTokenHandler.getValueFromTokenAndExpire 'password', 'mock-token', @callback
+
+			it 'should return a NotFoundError', ->
+				@callback
+					.calledWith(sinon.match.instanceOf(Errors.NotFoundError))
+					.should.equal true
+
+
 
-		it "should get and delete the token", (done)->
-			@redisMulti.exec.callsArgWith(0, null, [@value]) 
-			@OneTimeTokenHandler.getValueFromTokenAndExpire 'password', @stubbedToken, (err, value)=>
-				value.should.equal @value
-				@redisMulti.get.calledWith("password_token:#{@stubbedToken}").should.equal true
-				@redisMulti.del.calledWith("password_token:#{@stubbedToken}").should.equal true
-				done()
 
 

services/web/test/unit/coffee/User/UserEmailsConfirmationHandlerTests.coffee

@@ -36,7 +36,7 @@ describe "UserEmailsConfirmationHandler", ->
 				@OneTimeTokenHandler.getNewToken
 					.calledWith(
 						'email_confirmation',
-						JSON.stringify({@user_id, @email}),
+						{@user_id, @email},
 						{ expiresIn: 365 * 24 * 60 * 60 }
 					)
 					.should.equal true
@@ -72,7 +72,7 @@ describe "UserEmailsConfirmationHandler", ->
 		beforeEach ->
 			@OneTimeTokenHandler.getValueFromTokenAndExpire = sinon.stub().yields(
 				null, 
-				JSON.stringify({@user_id, @email})
+				{@user_id, @email}
 			)
 			@UserUpdater.confirmEmail = sinon.stub().yields()
 
@@ -105,7 +105,7 @@ describe "UserEmailsConfirmationHandler", ->
 			beforeEach ->
 				@OneTimeTokenHandler.getValueFromTokenAndExpire = sinon.stub().yields(
 					null, 
-					JSON.stringify({@email})
+					{@email}
 				)
 				@UserEmailsConfirmationHandler.confirmEmailFromToken @token = 'mock-token', @callback
 
@@ -116,7 +116,7 @@ describe "UserEmailsConfirmationHandler", ->
 			beforeEach ->
 				@OneTimeTokenHandler.getValueFromTokenAndExpire = sinon.stub().yields(
 					null, 
-					JSON.stringify({@user_id})
+					{@user_id}
 				)
 				@UserEmailsConfirmationHandler.confirmEmailFromToken @token = 'mock-token', @callback