fix(db): ensure that devices get deleted with session tokens

docs/API.md

@@ -341,6 +341,13 @@ These fields are represented as
 ### .deleteAccountResetToken(tokenId) ###
 
 Will delete the token of the correct type designated by the given `tokenId`.
+For `sessionTokens`,
+associated records in `devices` and `unverifiedTokens`
+will also be deleted.
+For `keyFetchTokens`,
+associated records in `unverifiedTokens`
+will also be deleted.
+
 
 ## .updateSessionToken(tokenId, token) ##
 

fxa-auth-db-server/docs/API.md

@@ -962,6 +962,7 @@ Content-Length: 285
 
 This operation is idempotent. If you delete a `tokenId` twice, the same result occurs. In fact, if you delete a
 `tokenId` that doesn't exist, it also returns the same `200 OK` result (since it is already not there).
+Also deletes any device records associated with the session.
 
 ### Example
 

fxa-auth-db-server/test/backend/db_tests.js

@@ -219,7 +219,7 @@ module.exports = function(config, DB) {
         test(
           'session token handling',
           function (t) {
-            t.plan(111)
+            t.plan(113)
 
             var VERIFIED_SESSION_TOKEN_ID = hex32()
             var UNVERIFIED_SESSION_TOKEN_ID = hex32()
@@ -235,6 +235,7 @@ module.exports = function(config, DB) {
               mustVerify: false,
               tokenVerificationId: hex16()
             }
+            var DEVICE_ID = newUuid()
 
             // Fetch all of the sessions tokens for the account
             return db.sessions(ACCOUNT.uid)
@@ -462,6 +463,18 @@ module.exports = function(config, DB) {
                 return db.createSessionToken(UNVERIFIED_SESSION_TOKEN_ID, UNVERIFIED_SESSION_TOKEN)
               })
               .then(function(results) {
+                // Create a device
+                return db.createDevice(ACCOUNT.uid, DEVICE_ID, {
+                  sessionTokenId: SESSION_TOKEN_ID
+                })
+              })
+              .then(function() {
+                // Fetch devices for the account
+                return db.accountDevices(ACCOUNT.uid)
+              })
+              .then(function(results) {
+                t.equal(results.length, 1, 'Account has one device')
+
                 // Delete all three session tokens
                 return P.all([
                   db.deleteSessionToken(SESSION_TOKEN_ID),
@@ -475,6 +488,12 @@ module.exports = function(config, DB) {
                   t.deepEqual(result, {}, 'Returned an empty object on forgot session token deletion')
                 })
 
+                // Fetch devices for the account
+                return db.accountDevices(ACCOUNT.uid)
+              })
+              .then(function(results) {
+                t.equal(results.length, 0, 'Account has no devices')
+
                 // Attempt to verify deleted unverified session token
                 return db.verifyTokens(UNVERIFIED_SESSION_TOKEN.tokenVerificationId, { uid: ACCOUNT.uid })
               })

fxa-auth-db-server/test/backend/remote.js

@@ -195,7 +195,7 @@ module.exports = function(cfg, server) {
   test(
     'session token handling',
     function (t) {
-      t.plan(133)
+      t.plan(141)
       var user = fake.newUserDataHex()
       var verifiedUser = fake.newUserDataHex()
       delete verifiedUser.sessionToken.tokenVerificationId
@@ -455,6 +455,19 @@ module.exports = function(cfg, server) {
           t.equal(token.uaDeviceType, 'different device type', 'uaDeviceType was updated')
           t.equal(token.lastAccessTime, 42, 'lastAccessTime was updated')
 
+          // Create a device
+          return client.putThen('/account/' + user.accountId + '/device/' + user.deviceId, user.device)
+        })
+        .then(function(r) {
+          respOk(t, r)
+
+          // Fetch devices for the account
+          return client.getThen('/account/' + user.accountId + '/devices')
+        })
+        .then(function(r) {
+          respOk(t, r)
+          t.equal(r.obj.length, 1, 'account has one device')
+
           // Delete both session tokens
           return P.all([
             client.delThen('/sessionToken/' + user.sessionTokenId),
@@ -467,6 +480,13 @@ module.exports = function(cfg, server) {
             respOk(t, result)
           })
 
+          // Fetch devices for the account
+          return client.getThen('/account/' + user.accountId + '/devices')
+        })
+        .then(function(r) {
+          respOk(t, r)
+          t.equal(r.obj.length, 0, 'account has no devices')
+
           // Fetch all of the session tokens for the account
           return client.getThen('/account/' + user.accountId + '/sessions')
         })

lib/db/mem.js

@@ -271,11 +271,31 @@ module.exports = function (log, error) {
 
   Memory.prototype.deleteSessionToken = function (tokenId) {
     tokenId = tokenId.toString('hex')
+    var sessionToken = sessionTokens[tokenId]
 
-    delete unverifiedTokens[tokenId]
-    delete sessionTokens[tokenId]
+    return P.resolve()
+      .then(function () {
+        if (sessionToken) {
+          return getAccountByUid(sessionToken.uid)
+            .then(function (account) {
+              var devices = account.devices
 
-    return P.resolve({})
+              Object.keys(devices).forEach(function (key) {
+                var sessionTokenId = devices[key].sessionTokenId
+
+                if (sessionTokenId && sessionTokenId.toString('hex') === tokenId) {
+                  delete devices[key]
+                }
+              })
+            })
+        }
+      })
+      .then(function () {
+        delete unverifiedTokens[tokenId]
+        delete sessionTokens[tokenId]
+
+        return {}
+      })
   }
 
   Memory.prototype.deleteKeyFetchToken = function (tokenId) {

lib/db/mysql.js

@@ -498,9 +498,9 @@ module.exports = function (log, error) {
     return this.write(DELETE_ACCOUNT, [uid])
   }
 
-  // Delete : sessionTokens, unverifiedTokens
+  // Delete : sessionTokens, unverifiedTokens, devices
   // Where  : tokenId = $1
-  var DELETE_SESSION_TOKEN = 'CALL deleteSessionToken_2(?)'
+  var DELETE_SESSION_TOKEN = 'CALL deleteSessionToken_3(?)'
 
   MySql.prototype.deleteSessionToken = function (tokenId) {
     return this.write(DELETE_SESSION_TOKEN, [tokenId])

lib/db/patch.js

@@ -4,4 +4,4 @@
 
 // The expected patch level of the database. Update if you add a new
 // patch in the schema/ directory.
-module.exports.level = 29
+module.exports.level = 30

lib/db/schema/patch-029-30.sql

@@ -0,0 +1,32 @@
+-- Update deleteSessionToken stored procedure to delete from devices.
+CREATE PROCEDURE `deleteSessionToken_3` (
+  IN `tokenIdArg` BINARY(32)
+)
+BEGIN
+  DECLARE EXIT HANDLER FOR SQLEXCEPTION
+  BEGIN
+    ROLLBACK;
+    RESIGNAL;
+  END;
+
+  START TRANSACTION;
+
+  DELETE FROM sessionTokens WHERE tokenId = tokenIdArg;
+  DELETE FROM unverifiedTokens WHERE tokenId = tokenIdArg;
+  DELETE FROM devices WHERE sessionTokenId = tokenIdArg;
+
+  COMMIT;
+END;
+
+-- Prior to this migration, it was possible to delete a session token
+-- without deleting the associated device (if a device record exists).
+-- That's fixed by deleteSessionTokens_3, but we should also purge any
+-- historical devices that have reached this state.
+DELETE FROM devices
+  USING devices LEFT JOIN sessionTokens
+    ON devices.sessionTokenId = sessionTokens.tokenId
+  WHERE sessionTokens.tokenId IS NULL;
+
+-- Update the patch level.
+UPDATE dbMetadata SET value = '30' WHERE name = 'schema-patch-level';
+

lib/db/schema/patch-030-029.sql

@@ -0,0 +1,4 @@
+-- DROP PROCEDURE `deleteSessionToken_3`;
+
+-- UPDATE dbMetadata SET value = '29' WHERE name = 'schema-patch-level';
+