fix(tokens): Do not override the createdAt field on existing tokens.

Previously this code could overwrite `createdAt` to the current timestamp
when reading an existing token from the database.

https://github.com/mozilla/fxa-auth-server-private/pull/45

r=philbooth

Author: rfk

lib/tokens/session_token.js

@@ -23,12 +23,7 @@ module.exports = (log, inherits, Token, config) => {
     this.verifierSetAt = details.verifierSetAt
     this.locale = details.locale || null
     this.mustVerify = !!details.mustVerify || false
-
-    if (details.createdAt > 0) {
-      this.accountCreatedAt = details.createdAt
-    } else {
-      this.accountCreatedAt = null
-    }
+    this.accountCreatedAt = details.accountCreatedAt || null
 
     // Tokens are considered verified if no tokenVerificationId exists
     this.tokenVerificationId = details.tokenVerificationId || null
@@ -39,8 +34,9 @@ module.exports = (log, inherits, Token, config) => {
   SessionToken.tokenTypeID = 'sessionToken'
 
   SessionToken.create = function (details) {
-    log.trace({ op: 'SessionToken.create', uid: details && details.uid })
-    return Token.createNewToken(SessionToken, details || {})
+    details = details || {}
+    log.trace({ op: 'SessionToken.create', uid: details.uid })
+    return Token.createNewToken(SessionToken, details)
   }
 
   SessionToken.fromHex = function (string, details) {

lib/tokens/token.js

@@ -40,16 +40,22 @@ module.exports = function (log, random, P, hkdf, Bundle, error) {
     this.algorithm = 'sha256'
     this.uid = details.uid || null
     this.lifetime = details.lifetime || Infinity
-    this.createdAt = optionallyOverrideCreatedAt(details.createdAt)
+    this.createdAt = details.createdAt || 0
   }
 
   function optionallyOverrideCreatedAt (timestamp) {
     var now = Date.now()
 
-    if (! config.isProduction && timestamp >= 0 && timestamp < now) {
-      // In the wild, all tokens should have a fresh createdAt timestamp.
-      // For testing purposes only, allow createdAt to be overridden.
-      return timestamp
+    // In the wild, all tokens should have a fresh createdAt timestamp.
+    // For testing purposes only, allow createdAt to be overridden.
+    if (timestamp !== undefined) {
+      if (config.isProduction) {
+        throw new Error('unexpected value for createdAt')
+      }
+
+      if (timestamp >= 0 && timestamp <= now) {
+        return timestamp
+      }
     }
 
     return now
@@ -61,28 +67,21 @@ module.exports = function (log, random, P, hkdf, Bundle, error) {
   Token.createNewToken = function(TokenType, details) {
     return random(32)
       .then(bytes => Token.deriveTokenKeys(TokenType, bytes))
-      .then(keys => new TokenType(keys, details || {}))
+      .then(keys => {
+        details = details || {}
+        details.createdAt = optionallyOverrideCreatedAt(details.createdAt)
+        return new TokenType(keys, details)
+      })
   }
 
 
   // Re-create an existing token of the given type.
   // This uses known seed data to derive the keys.
   //
   Token.createTokenFromHexData = function(TokenType, hexData, details) {
-    var d = P.defer()
     var data = Buffer(hexData, 'hex')
-    Token.deriveTokenKeys(TokenType, data)
-      .then(
-        function (keys) {
-          d.resolve(new TokenType(keys, details || {}))
-        }
-      )
-      .catch(
-        function (err) {
-          d.reject(err)
-        }
-      )
-    return d.promise
+    return Token.deriveTokenKeys(TokenType, data)
+      .then(keys => new TokenType(keys, details || {}))
   }
 
 

test/local/session_token_tests.js

@@ -17,20 +17,22 @@ var SessionToken = tokens.SessionToken
 
 var TOKEN_FRESHNESS_THRESHOLD = require('../../lib/tokens/session_token').TOKEN_FRESHNESS_THREADHOLD
 
-var ACCOUNT = {
+var TOKEN = {
   createdAt: Date.now(),
+  accountCreatedAt: Date.now(),
   uid: 'xxx',
   email: Buffer('test@example.com').toString('hex'),
   emailCode: '123456',
   emailVerified: true,
   tokenVerificationId: crypto.randomBytes(16)
 }
 
+
 describe('SessionToken', () => {
   it(
     'interface is correct',
     () => {
-      return SessionToken.create(ACCOUNT)
+      return SessionToken.create(TOKEN)
         .then(function (token) {
           assert.equal(typeof token.lastAuthAt, 'function', 'lastAuthAt method is defined')
           assert.equal(typeof token.update, 'function', 'update method is defined')
@@ -44,16 +46,16 @@ describe('SessionToken', () => {
     're-creation from tokenData works',
     () => {
       var token = null
-      return SessionToken.create(ACCOUNT)
+      return SessionToken.create(TOKEN)
         .then(
           function (x) {
             token = x
-            assert.equal(token.accountCreatedAt, ACCOUNT.createdAt)
+            assert.equal(token.accountCreatedAt, TOKEN.accountCreatedAt)
           }
         )
         .then(
           function () {
-            return SessionToken.fromHex(token.data, ACCOUNT)
+            return SessionToken.fromHex(token.data, TOKEN)
           }
         )
         .then(
@@ -96,7 +98,7 @@ describe('SessionToken', () => {
     () => {
       var token = null
       var tokenData = 'a0a1a2a3a4a5a6a7a8a9aaabacadaeafb0b1b2b3b4b5b6b7b8b9babbbcbdbebf'
-      return SessionToken.fromHex(tokenData, ACCOUNT)
+      return SessionToken.fromHex(tokenData, TOKEN)
         .then(
           function (x) {
             token = x
@@ -111,7 +113,7 @@ describe('SessionToken', () => {
   it(
     'SessionToken.setUserAgentInfo',
     () => {
-      return SessionToken.create(ACCOUNT)
+      return SessionToken.create(TOKEN)
         .then(function (token) {
           token.setUserAgentInfo({
             data: 'foo',

test/local/token_tests.js

@@ -5,7 +5,7 @@
 'use strict'
 
 const assert = require('insist')
-var crypto = require('crypto')
+var random = require('../../lib/crypto/random')
 var hkdf = require('../../lib/crypto/hkdf')
 var mocks = require('../mocks')
 var P = require('../../lib/promise')
@@ -26,7 +26,7 @@ describe('Token', () => {
       delete require.cache[require.resolve(modulePath)]
       delete require.cache[require.resolve('../../config')]
       process.env.NODE_ENV = 'dev'
-      Token = require(modulePath)(log, crypto, P, hkdf, Bundle, null)
+      Token = require(modulePath)(log, random, P, hkdf, Bundle, null)
     })
 
     it('Token constructor was exported', () => {
@@ -35,25 +35,61 @@ describe('Token', () => {
       assert.equal(Token.length, 2, 'function expects two arguments')
     })
 
+    it('Token constructor has expected factory methods', () => {
+      assert.equal(typeof Token.createNewToken, 'function', 'Token.createNewToken is function')
+      assert.equal(Token.createNewToken.length, 2, 'function expects two arguments')
+      assert.equal(typeof Token.createTokenFromHexData, 'function', 'Token.createTokenFromHexData is function')
+      assert.equal(Token.createTokenFromHexData.length, 3, 'function expects three arguments')
+    })
+
     it('Token constructor sets createdAt', () => {
       var now = Date.now() - 1
       var token = new Token({}, { createdAt: now })
 
       assert.equal(token.createdAt, now, 'token.createdAt is correct')
     })
 
-    it('Token constructor does not set createdAt if it is negative', () => {
-      var notNow = -Date.now()
-      var token = new Token({}, { createdAt: notNow })
+    it('Token.createNewToken defaults createdAt to the current time', () => {
+      var now = Date.now()
+      return Token.createNewToken(Token, {}).then(token => {
+        assert.ok(token.createdAt >= now && token.createdAt <= Date.now(), 'token.createdAt seems correct')
+      })
+    })
+
+    it('Token.createNewToken accepts an override for createdAt', () => {
+      var now = Date.now() - 1
+      return Token.createNewToken(Token, { createdAt: now }).then(token => {
+        assert.equal(token.createdAt, now, 'token.createdAt is correct')
+      })
+    })
 
-      assert.ok(token.createdAt > 0, 'token.createdAt seems correct')
+    it('Token.createNewToken ignores a negative value for createdAt', () => {
+      var now = Date.now()
+      var notNow = -now
+      return Token.createNewToken(Token, { createdAt: notNow }).then(token => {
+        assert.ok(token.createdAt >= now && token.createdAt <= Date.now(), 'token.createdAt seems correct')
+      })
     })
 
-    it('Token constructor does not set createdAt if it is in the future', () => {
+    it('Token.createNewToken ignores a createdAt timestamp in the future', () => {
+      var now = Date.now()
       var notNow = Date.now() + 1000
-      var token = new Token({}, { createdAt: notNow })
+      return Token.createNewToken(Token, { createdAt: notNow }).then(token => {
+        assert.ok(token.createdAt >= now && token.createdAt <= Date.now(), 'token.createdAt seems correct')
+      })
+    })
 
-      assert.ok(token.createdAt > 0 && token.createdAt < notNow, 'token.createdAt seems correct')
+    it('Token.createTokenFromHexData accepts a value for createdAt', () => {
+      var now = Date.now() - 20
+      return Token.createTokenFromHexData(Token, 'ABCD', { createdAt: now }).then(token => {
+        assert.equal(token.createdAt, now, 'token.createdAt is correct')
+      })
+    })
+
+    it('Token.createTokenFromHexData fails if not given a value for createdAt', () => {
+      return Token.createTokenFromHexData(Token, 'ABCD', { other: 'data' }).then(token => {
+        assert.equal(token.createdAt, 0, 'token.createdAt is correct')
+      })
     })
   })
 
@@ -63,14 +99,35 @@ describe('Token', () => {
       delete require.cache[require.resolve(modulePath)]
       delete require.cache[require.resolve('../../config')]
       process.env.NODE_ENV = 'prod'
-      Token = require(modulePath)(log, crypto, P, hkdf, Bundle, null)
+      Token = require(modulePath)(log, random, P, hkdf, Bundle, null)
+    })
+
+    it('Token.createNewToken defaults createdAt to the current time', () => {
+      var now = Date.now()
+      return Token.createNewToken(Token, {}).then(token => {
+        assert.ok(token.createdAt >= now && token.createdAt <= Date.now(), 'token.createdAt seems correct')
+      })
     })
 
-    it('Token constructor does not set createdAt', () => {
-      var notNow = Date.now() - 1
-      var token = new Token({}, { createdAt: notNow })
+    it('Token.createNewToken does not accept an override for createdAt', () => {
+      var now = Date.now() - 1
+      return Token.createNewToken(Token, { createdAt: now }).then(
+        () => assert.fail('should have thrown'),
+        (err) => assert.equal(err.message, 'unexpected value for createdAt')
+      )
+    })
+
+    it('Token.createTokenFromHexData accepts a value for createdAt', () => {
+      var now = Date.now() - 20
+      return Token.createTokenFromHexData(Token, 'ABCD', { createdAt: now }).then(token => {
+        assert.equal(token.createdAt, now, 'token.createdAt is correct')
+      })
+    })
 
-      assert.ok(token.createdAt > notNow, 'token.createdAt seems correct')
+    it('Token.createTokenFromHexData defaults to zero if not given a value for createdAt', () => {
+      return Token.createTokenFromHexData(Token, 'ABCD', { other: 'data' }).then(token => {
+        assert.equal(token.createdAt, 0, 'token.createdAt is correct')
+      })
     })
   })