Bug 1448165 p3 - Migrate deviceId and deviceRegistrationVersion under device. r?markh draft
authorEdouard Oger <eoger@fastmail.com>
Thu, 22 Mar 2018 14:36:49 -0400
changeset 774429 f029cb58f86254b5fe3c1a76e4c7599cd7eb7975
parent 774428 4cdad8fd11bcc68505890249b0d7c70ab1d65b17
child 774430 21d7dc1ed5e9f4f55daddb8f12e4810bd2edf852
push id104396
push userbmo:eoger@fastmail.com
push dateWed, 28 Mar 2018 21:42:47 +0000
reviewersmarkh
bugs1448165
milestone61.0a1
Bug 1448165 p3 - Migrate deviceId and deviceRegistrationVersion under device. r?markh MozReview-Commit-ID: 5uWOyLGdCC1
services/fxaccounts/FxAccounts.jsm
services/fxaccounts/FxAccountsCommon.js
services/fxaccounts/FxAccountsWebChannel.jsm
services/fxaccounts/tests/xpcshell/test_accounts.js
services/fxaccounts/tests/xpcshell/test_accounts_device_registration.js
services/fxaccounts/tests/xpcshell/test_oauth_grant_client.js
services/fxaccounts/tests/xpcshell/test_push_service.js
services/fxaccounts/tests/xpcshell/test_storage_manager.js
services/fxaccounts/tests/xpcshell/test_web_channel.js
--- a/services/fxaccounts/FxAccounts.jsm
+++ b/services/fxaccounts/FxAccounts.jsm
@@ -655,35 +655,45 @@ FxAccountsInternal.prototype = {
   /**
    * Invalidate the FxA certificate, so that it will be refreshed from the server
    * the next time it is needed.
    */
   invalidateCertificate() {
     return this.currentAccountState.updateUserAccountData({ cert: null });
   },
 
-  getDeviceId() {
-    return this.currentAccountState.getUserAccountData()
-      .then(data => {
-        if (data) {
-          if (!data.deviceId || !data.deviceRegistrationVersion ||
-              data.deviceRegistrationVersion < this.DEVICE_REGISTRATION_VERSION) {
-            // There is no device id or the device registration is outdated.
-            // Either way, we should register the device with FxA
-            // before returning the id to the caller.
-            return this._registerOrUpdateDevice(data);
-          }
-
-          // Return the device id that we already registered with the server.
-          return data.deviceId;
+  async getDeviceId() {
+    let data = await this.currentAccountState.getUserAccountData();
+    if (!data) {
+      // Without a signed-in user, there can be no device id.
+      return null;
+    }
+    // Try migrating first. Remove this in Firefox 65+.
+    if (data.deviceId) {
+      log.info("Migrating from deviceId to device.");
+      await this.currentAccountState.updateUserAccountData({
+        deviceId: null,
+        deviceRegistrationVersion: null,
+        device: {
+          id: data.deviceId,
+          registrationVersion: data.deviceRegistrationVersion
         }
-
-        // Without a signed-in user, there can be no device id.
-        return null;
       });
+      data = await this.currentAccountState.getUserAccountData();
+    }
+    const {device} = data;
+    if (!device || !device.registrationVersion ||
+        device.registrationVersion < this.DEVICE_REGISTRATION_VERSION) {
+      // There is no device registered or the device registration is outdated.
+      // Either way, we should register the device with FxA
+      // before returning the id to the caller.
+      return this._registerOrUpdateDevice(data);
+    }
+    // Return the device id that we already registered with the server.
+    return device.id;
   },
 
   async getDeviceList() {
     let accountData = await this._getVerifiedAccountOrReject();
     return this.fxAccountsClient.getDeviceList(accountData.sessionToken);
   },
 
   /**
@@ -1543,35 +1553,40 @@ FxAccountsInternal.prototype = {
         return currentState.reject(error);
       }
     ).catch(err => Promise.reject(this._errorToErrorClass(err)));
   },
 
   // Attempt to update the auth server with whatever device details are stored
   // in the account data. Returns a promise that always resolves, never rejects.
   // If the promise resolves to a value, that value is the device id.
-  updateDeviceRegistration() {
-    return this.getSignedInUser().then(signedInUser => {
+  async updateDeviceRegistration() {
+    try {
+      const signedInUser = await this.getSignedInUser();
       if (signedInUser) {
-        return this._registerOrUpdateDevice(signedInUser);
+        await this._registerOrUpdateDevice(signedInUser);
       }
-      return null;
-    }).catch(error => this._logErrorAndResetDeviceRegistrationVersion(error));
+    } catch (error) {
+      await this._logErrorAndResetDeviceRegistrationVersion(error);
+    }
   },
 
   async handleDeviceDisconnection(deviceId) {
     const accountData = await this.currentAccountState.getUserAccountData();
-    const localDeviceId = accountData ? accountData.deviceId : null;
+    if (!accountData || !accountData.device) {
+      // Nothing we can do here.
+      return;
+    }
+    const localDeviceId = accountData.device.id;
     const isLocalDevice = (deviceId == localDeviceId);
     if (isLocalDevice) {
       this.signOut(true);
     }
     const data = JSON.stringify({ isLocalDevice });
     await this.notifyObservers(ON_DEVICE_DISCONNECTED_NOTIFICATION, data);
-    return null;
   },
 
   handleEmailUpdated(newEmail) {
     Services.prefs.setStringPref(PREF_LAST_FXA_USER, CryptoUtils.sha256Base64(newEmail));
     return this.currentAccountState.updateUserAccountData({ email: newEmail });
   },
 
   async handleAccountDestroyed(uid) {
@@ -1623,62 +1638,65 @@ FxAccountsInternal.prototype = {
     return this.currentAccountState.updateUserAccountData({
       profileCache
     });
   },
 
   // If you change what we send to the FxA servers during device registration,
   // you'll have to bump the DEVICE_REGISTRATION_VERSION number to force older
   // devices to re-register when Firefox updates
-  _registerOrUpdateDevice(signedInUser) {
-    try {
-      // Allow tests to skip device registration because:
-      //   1. It makes remote requests to the auth server.
-      //   2. _getDeviceName does not work from xpcshell.
-      //   3. The B2G tests fail when attempting to import services-sync/util.js.
-      if (Services.prefs.getBoolPref("identity.fxaccounts.skipDeviceRegistration")) {
-        return Promise.resolve();
-      }
-    } catch (ignore) {}
-
-    if (!signedInUser.sessionToken) {
-      return Promise.reject(new Error(
-        "_registerOrUpdateDevice called without a session token"));
+  async _registerOrUpdateDevice(signedInUser) {
+    // Allow tests to skip device registration because it makes remote requests
+    // to the auth server and _getDeviceName does not work from xpcshell.
+    if (Services.prefs.getBoolPref("identity.fxaccounts.skipDeviceRegistration", false)) {
+      return null;
     }
 
-    return this.fxaPushService.registerPushEndpoint().then(subscription => {
+    const {sessionToken, device: currentDevice} = signedInUser;
+    if (!sessionToken) {
+      throw new Error("_registerOrUpdateDevice called without a session token");
+    }
+
+    try {
+      const subscription = await this.fxaPushService.registerPushEndpoint();
       const deviceName = this._getDeviceName();
       let deviceOptions = {};
 
       // if we were able to obtain a subscription
       if (subscription && subscription.endpoint) {
         deviceOptions.pushCallback = subscription.endpoint;
         let publicKey = subscription.getKey("p256dh");
         let authKey = subscription.getKey("auth");
         if (publicKey && authKey) {
           deviceOptions.pushPublicKey = urlsafeBase64Encode(publicKey);
           deviceOptions.pushAuthKey = urlsafeBase64Encode(authKey);
         }
       }
 
-      if (signedInUser.deviceId) {
+      let device;
+      if (currentDevice && currentDevice.id) {
         log.debug("updating existing device details");
-        return this.fxAccountsClient.updateDevice(
-          signedInUser.sessionToken, signedInUser.deviceId, deviceName, deviceOptions);
+        device = await this.fxAccountsClient.updateDevice(
+          sessionToken, currentDevice.id, deviceName, deviceOptions);
+      } else {
+        log.debug("registering new device details");
+        device = await this.fxAccountsClient.registerDevice(
+          sessionToken, deviceName, this._getDeviceType(), deviceOptions);
       }
 
-      log.debug("registering new device details");
-      return this.fxAccountsClient.registerDevice(
-        signedInUser.sessionToken, deviceName, this._getDeviceType(), deviceOptions);
-    }).then(device =>
-      this.currentAccountState.updateUserAccountData({
-        deviceId: device.id,
-        deviceRegistrationVersion: this.DEVICE_REGISTRATION_VERSION
-      }).then(() => device.id)
-    ).catch(error => this._handleDeviceError(error, signedInUser.sessionToken));
+      await this.currentAccountState.updateUserAccountData({
+        device: {
+          id: device.id,
+          registrationVersion: this.DEVICE_REGISTRATION_VERSION
+        }
+      });
+      return device.id;
+    } catch (error) {
+      return this._handleDeviceError(error, sessionToken);
+    }
   },
 
   _getDeviceName() {
     return Utils.getDeviceName();
   },
 
   _getDeviceType() {
     return Utils.getDeviceType();
@@ -1698,69 +1716,78 @@ FxAccountsInternal.prototype = {
 
       // `_handleTokenError` re-throws the error.
       return this._handleTokenError(error);
     }).catch(error =>
       this._logErrorAndResetDeviceRegistrationVersion(error)
     ).catch(() => {});
   },
 
-  _recoverFromUnknownDevice() {
+  async _recoverFromUnknownDevice() {
     // FxA did not recognise the device id. Handle it by clearing the device
     // id on the account data. At next sync or next sign-in, registration is
     // retried and should succeed.
     log.warn("unknown device id, clearing the local device data");
-    return this.currentAccountState.updateUserAccountData({ deviceId: null })
-      .catch(error => this._logErrorAndResetDeviceRegistrationVersion(error));
+    try {
+      await this.currentAccountState.updateUserAccountData({device: null});
+    } catch (error) {
+      await this._logErrorAndResetDeviceRegistrationVersion(error);
+    }
   },
 
-  _recoverFromDeviceSessionConflict(error, sessionToken) {
+  async _recoverFromDeviceSessionConflict(error, sessionToken) {
     // FxA has already associated this session with a different device id.
     // Perhaps we were beaten in a race to register. Handle the conflict:
     //   1. Fetch the list of devices for the current user from FxA.
     //   2. Look for ourselves in the list.
     //   3. If we find a match, set the correct device id and device registration
     //      version on the account data and return the correct device id. At next
     //      sync or next sign-in, registration is retried and should succeed.
     //   4. If we don't find a match, log the original error.
     log.warn("device session conflict, attempting to ascertain the correct device id");
-    return this.fxAccountsClient.getDeviceList(sessionToken)
-      .then(devices => {
-        const matchingDevices = devices.filter(device => device.isCurrentDevice);
-        const length = matchingDevices.length;
-        if (length === 1) {
-          const deviceId = matchingDevices[0].id;
-          return this.currentAccountState.updateUserAccountData({
-            deviceId,
-            deviceRegistrationVersion: null
-          }).then(() => deviceId);
-        }
-        if (length > 1) {
-          log.error("insane server state, " + length + " devices for this session");
-        }
-        return this._logErrorAndResetDeviceRegistrationVersion(error);
-      }).catch(secondError => {
-        log.error("failed to recover from device-session conflict", secondError);
-        this._logErrorAndResetDeviceRegistrationVersion(error);
-      });
+    try {
+      const devices = await this.fxAccountsClient.getDeviceList(sessionToken);
+      const matchingDevices = devices.filter(device => device.isCurrentDevice);
+      const length = matchingDevices.length;
+      if (length === 1) {
+        const deviceId = matchingDevices[0].id;
+        await this.currentAccountState.updateUserAccountData({
+          device: {
+            id: deviceId,
+            registrationVersion: null
+          }
+        });
+        return deviceId;
+      }
+      if (length > 1) {
+        log.error("insane server state, " + length + " devices for this session");
+      }
+      await this._logErrorAndResetDeviceRegistrationVersion(error);
+    } catch (secondError) {
+      log.error("failed to recover from device-session conflict", secondError);
+      await this._logErrorAndResetDeviceRegistrationVersion(error);
+    }
+    return null;
   },
 
-  _logErrorAndResetDeviceRegistrationVersion(error) {
+  async _logErrorAndResetDeviceRegistrationVersion(error) {
     // Device registration should never cause other operations to fail.
     // If we've reached this point, just log the error and reset the device
-    // registration version on the account data. At next sync or next sign-in,
+    // on the account data. At next sync or next sign-in,
     // registration will be retried.
     log.error("device registration failed", error);
-    return this.currentAccountState.updateUserAccountData({
-      deviceRegistrationVersion: null
-    }).catch(secondError => {
+    try {
+      this.currentAccountState.updateUserAccountData({
+        device: null
+      });
+    } catch (secondError) {
       log.error(
         "failed to reset the device registration version, device registration won't be retried",
         secondError);
-    }).then(() => {});
+    }
   },
 
   _handleTokenError(err) {
     if (!err || err.code != 401 || err.errno != ERRNO_INVALID_AUTH_TOKEN) {
       throw err;
     }
     log.warn("recovering from invalid token error", err);
     return this.accountStatus().then(exists => {
--- a/services/fxaccounts/FxAccountsCommon.js
+++ b/services/fxaccounts/FxAccountsCommon.js
@@ -208,30 +208,30 @@ exports.DERIVED_KEYS_NAMES = ["kSync", "
 // JSON file in the profile dir and in the login manager.
 // In order to prevent new fields accidentally ending up in the "wrong" place,
 // all fields stored are listed here.
 
 // The fields we save in the plaintext JSON.
 // See bug 1013064 comments 23-25 for why the sessionToken is "safe"
 exports.FXA_PWDMGR_PLAINTEXT_FIELDS = new Set(
   ["email", "verified", "authAt", "sessionToken", "uid", "oauthTokens", "profile",
-  "deviceId", "deviceRegistrationVersion", "profileCache"]);
+  "device", "profileCache"]);
 
 // Fields we store in secure storage if it exists.
 exports.FXA_PWDMGR_SECURE_FIELDS = new Set(
   [...exports.DERIVED_KEYS_NAMES, "keyFetchToken", "unwrapBKey", "assertion"]);
 
 // Fields we keep in memory and don't persist anywhere.
 exports.FXA_PWDMGR_MEMORY_FIELDS = new Set(
   ["cert", "keyPair"]);
 
 // A whitelist of fields that remain in storage when the user needs to
 // reauthenticate. All other fields will be removed.
 exports.FXA_PWDMGR_REAUTH_WHITELIST = new Set(
-  ["email", "uid", "profile", "deviceId", "deviceRegistrationVersion", "verified"]);
+  ["email", "uid", "profile", "device", "verified"]);
 
 // The pseudo-host we use in the login manager
 exports.FXA_PWDMGR_HOST = "chrome://FirefoxAccounts";
 // The realm we use in the login manager.
 exports.FXA_PWDMGR_REALM = "Firefox Accounts credentials";
 
 // Error matching.
 exports.SERVER_ERRNO_TO_ERROR = {};
--- a/services/fxaccounts/FxAccountsWebChannel.jsm
+++ b/services/fxaccounts/FxAccountsWebChannel.jsm
@@ -422,17 +422,17 @@ this.FxAccountsWebChannelHelpers.prototy
     // that hard-codes field names.
     // However, in this case the field names aren't really in our control.
     // We *could* still insist the server know what fields names are valid,
     // but that makes life difficult for the server when Firefox adds new
     // features (ie, new fields) - forcing the server to track a map of
     // versions to supported field names doesn't buy us much.
     // So we just remove field names we know aren't handled.
     let newCredentials = {
-      deviceId: null
+      device: null // Force a brand new device registration.
     };
     for (let name of Object.keys(credentials)) {
       if (name == "email" || name == "uid" || FxAccountsStorageManagerCanStoreField(name)) {
         newCredentials[name] = credentials[name];
       } else {
         log.info("changePassword ignoring unsupported field", name);
       }
     }
--- a/services/fxaccounts/tests/xpcshell/test_accounts.js
+++ b/services/fxaccounts/tests/xpcshell/test_accounts.js
@@ -1474,17 +1474,16 @@ function expandHex(two_hex) {
 function expandBytes(two_hex) {
   return CommonUtils.hexToBytes(expandHex(two_hex));
 }
 
 function getTestUser(name) {
   return {
     email: name + "@example.com",
     uid: "1ad7f502-4cc7-4ec1-a209-071fd2fae348",
-    deviceId: name + "'s device id",
     sessionToken: name + "'s session token",
     keyFetchToken: name + "'s keyfetch token",
     unwrapBKey: expandHex("44"),
     verified: false
   };
 }
 
 function makeObserver(aObserveTopic, aObserveFunc) {
--- a/services/fxaccounts/tests/xpcshell/test_accounts_device_registration.js
+++ b/services/fxaccounts/tests/xpcshell/test_accounts_device_registration.js
@@ -3,16 +3,17 @@
 
 "use strict";
 
 ChromeUtils.import("resource://services-common/utils.js");
 ChromeUtils.import("resource://gre/modules/FxAccounts.jsm");
 ChromeUtils.import("resource://gre/modules/FxAccountsClient.jsm");
 ChromeUtils.import("resource://gre/modules/FxAccountsCommon.js");
 ChromeUtils.import("resource://gre/modules/Log.jsm");
+var {AccountState} = ChromeUtils.import("resource://gre/modules/FxAccounts.jsm", {});
 
 initTestLogging("Trace");
 
 var log = Log.repository.getLogger("Services.FxAccounts.test");
 log.level = Log.Level.Debug;
 
 const BOGUS_PUBLICKEY = "BBXOKjUb84pzws1wionFpfCBjDuCh4-s_1b52WA46K5wYL2gCWEOmFKWn_NkS5nmJwTBuO8qxxdjAIDtNeklvQc";
 const BOGUS_AUTHKEY = "GSsIiaD2Mr83iPqwFNK4rw";
@@ -87,16 +88,22 @@ function MockFxAccountsClient(device) {
   FxAccountsClient.apply(this);
 }
 MockFxAccountsClient.prototype = {
   __proto__: FxAccountsClient.prototype
 };
 
 function MockFxAccounts(device = {}) {
   return new FxAccounts({
+    newAccountState(credentials) {
+      // we use a real accountState but mocked storage.
+      let storage = new MockStorageManager();
+      storage.initialize(credentials);
+      return new AccountState(storage);
+    },
     _getDeviceName() {
       return device.name || "mock device name";
     },
     fxAccountsClient: new MockFxAccountsClient(device),
     fxaPushService: {
       registerPushEndpoint() {
         return new Promise((resolve) => {
           resolve({
@@ -117,19 +124,20 @@ function MockFxAccounts(device = {}) {
   });
 }
 
 add_task(async function test_updateDeviceRegistration_with_new_device() {
   const deviceName = "foo";
   const deviceType = "bar";
 
   const credentials = getTestUser("baz");
-  delete credentials.deviceId;
   const fxa = new MockFxAccounts({ name: deviceName });
   await fxa.internal.setSignedInUser(credentials);
+  // Remove the current device registration (setSIgnedInUser does one!).
+  await fxa.updateUserAccountData({uid: credentials.uid, device: null});
 
   const spy = {
     registerDevice: { count: 0, args: [] },
     updateDevice: { count: 0, args: [] },
     getDeviceList: { count: 0, args: [] }
   };
   const client = fxa.internal.fxAccountsClient;
   client.registerDevice = function() {
@@ -148,96 +156,104 @@ add_task(async function test_updateDevic
     return Promise.resolve({});
   };
   client.getDeviceList = function() {
     spy.getDeviceList.count += 1;
     spy.getDeviceList.args.push(arguments);
     return Promise.resolve([]);
   };
 
-  const result = await fxa.updateDeviceRegistration();
+  await fxa.updateDeviceRegistration();
 
-  Assert.equal(result, "newly-generated device id");
   Assert.equal(spy.updateDevice.count, 0);
   Assert.equal(spy.getDeviceList.count, 0);
   Assert.equal(spy.registerDevice.count, 1);
   Assert.equal(spy.registerDevice.args[0].length, 4);
   Assert.equal(spy.registerDevice.args[0][0], credentials.sessionToken);
   Assert.equal(spy.registerDevice.args[0][1], deviceName);
   Assert.equal(spy.registerDevice.args[0][2], "desktop");
   Assert.equal(spy.registerDevice.args[0][3].pushCallback, "http://mochi.test:8888");
   Assert.equal(spy.registerDevice.args[0][3].pushPublicKey, BOGUS_PUBLICKEY);
   Assert.equal(spy.registerDevice.args[0][3].pushAuthKey, BOGUS_AUTHKEY);
 
   const state = fxa.internal.currentAccountState;
   const data = await state.getUserAccountData();
 
-  Assert.equal(data.deviceId, "newly-generated device id");
-  Assert.equal(data.deviceRegistrationVersion, DEVICE_REGISTRATION_VERSION);
+  Assert.equal(data.device.id, "newly-generated device id");
+  Assert.equal(data.device.registrationVersion, DEVICE_REGISTRATION_VERSION);
 });
 
 add_task(async function test_updateDeviceRegistration_with_existing_device() {
+  const deviceId = "my device id";
   const deviceName = "phil's device";
 
   const credentials = getTestUser("pb");
   const fxa = new MockFxAccounts({ name: deviceName });
   await fxa.internal.setSignedInUser(credentials);
+  await fxa.updateUserAccountData({uid: credentials.uid, device: {
+    id: deviceId,
+    registrationVersion: 1 // < 42
+  }});
 
   const spy = {
     registerDevice: { count: 0, args: [] },
     updateDevice: { count: 0, args: [] },
     getDeviceList: { count: 0, args: [] }
   };
   const client = fxa.internal.fxAccountsClient;
   client.registerDevice = function() {
     spy.registerDevice.count += 1;
     spy.registerDevice.args.push(arguments);
     return Promise.resolve({});
   };
   client.updateDevice = function() {
     spy.updateDevice.count += 1;
     spy.updateDevice.args.push(arguments);
     return Promise.resolve({
-      id: credentials.deviceId,
+      id: deviceId,
       name: deviceName
     });
   };
   client.getDeviceList = function() {
     spy.getDeviceList.count += 1;
     spy.getDeviceList.args.push(arguments);
     return Promise.resolve([]);
   };
-  const result = await fxa.updateDeviceRegistration();
+  await fxa.updateDeviceRegistration();
 
-  Assert.equal(result, credentials.deviceId);
   Assert.equal(spy.registerDevice.count, 0);
   Assert.equal(spy.getDeviceList.count, 0);
   Assert.equal(spy.updateDevice.count, 1);
   Assert.equal(spy.updateDevice.args[0].length, 4);
   Assert.equal(spy.updateDevice.args[0][0], credentials.sessionToken);
-  Assert.equal(spy.updateDevice.args[0][1], credentials.deviceId);
+  Assert.equal(spy.updateDevice.args[0][1], deviceId);
   Assert.equal(spy.updateDevice.args[0][2], deviceName);
   Assert.equal(spy.updateDevice.args[0][3].pushCallback, "http://mochi.test:8888");
   Assert.equal(spy.updateDevice.args[0][3].pushPublicKey, BOGUS_PUBLICKEY);
   Assert.equal(spy.updateDevice.args[0][3].pushAuthKey, BOGUS_AUTHKEY);
 
   const state = fxa.internal.currentAccountState;
   const data = await state.getUserAccountData();
 
-  Assert.equal(data.deviceId, credentials.deviceId);
-  Assert.equal(data.deviceRegistrationVersion, DEVICE_REGISTRATION_VERSION);
+  Assert.equal(data.device.id, deviceId);
+  Assert.equal(data.device.registrationVersion, DEVICE_REGISTRATION_VERSION);
 });
 
 add_task(async function test_updateDeviceRegistration_with_unknown_device_error() {
   const deviceName = "foo";
   const deviceType = "bar";
+  const currentDeviceId = "my device id";
 
   const credentials = getTestUser("baz");
   const fxa = new MockFxAccounts({ name: deviceName });
   await fxa.internal.setSignedInUser(credentials);
+  await fxa.updateUserAccountData({uid: credentials.uid, device: {
+    id: currentDeviceId,
+    registrationVersion: 1 // < 42
+  }});
 
   const spy = {
     registerDevice: { count: 0, args: [] },
     updateDevice: { count: 0, args: [] },
     getDeviceList: { count: 0, args: [] }
   };
   const client = fxa.internal.fxAccountsClient;
   client.registerDevice = function() {
@@ -259,45 +275,49 @@ add_task(async function test_updateDevic
     });
   };
   client.getDeviceList = function() {
     spy.getDeviceList.count += 1;
     spy.getDeviceList.args.push(arguments);
     return Promise.resolve([]);
   };
 
-  const result = await fxa.updateDeviceRegistration();
+  await fxa.updateDeviceRegistration();
 
-  Assert.equal(null, result);
   Assert.equal(spy.getDeviceList.count, 0);
   Assert.equal(spy.registerDevice.count, 0);
   Assert.equal(spy.updateDevice.count, 1);
   Assert.equal(spy.updateDevice.args[0].length, 4);
   Assert.equal(spy.updateDevice.args[0][0], credentials.sessionToken);
-  Assert.equal(spy.updateDevice.args[0][1], credentials.deviceId);
+  Assert.equal(spy.updateDevice.args[0][1], currentDeviceId);
   Assert.equal(spy.updateDevice.args[0][2], deviceName);
   Assert.equal(spy.updateDevice.args[0][3].pushCallback, "http://mochi.test:8888");
   Assert.equal(spy.updateDevice.args[0][3].pushPublicKey, BOGUS_PUBLICKEY);
   Assert.equal(spy.updateDevice.args[0][3].pushAuthKey, BOGUS_AUTHKEY);
 
 
   const state = fxa.internal.currentAccountState;
   const data = await state.getUserAccountData();
 
-  Assert.equal(null, data.deviceId);
-  Assert.equal(data.deviceRegistrationVersion, DEVICE_REGISTRATION_VERSION);
+  Assert.equal(null, data.device);
 });
 
 add_task(async function test_updateDeviceRegistration_with_device_session_conflict_error() {
   const deviceName = "foo";
   const deviceType = "bar";
+  const currentDeviceId = "my device id";
+  const conflictingDeviceId = "conflicting device id";
 
   const credentials = getTestUser("baz");
   const fxa = new MockFxAccounts({ name: deviceName });
   await fxa.internal.setSignedInUser(credentials);
+  await fxa.updateUserAccountData({uid: credentials.uid, device: {
+    id: currentDeviceId,
+    registrationVersion: 1 // < 42
+  }});
 
   const spy = {
     registerDevice: { count: 0, args: [] },
     updateDevice: { count: 0, args: [], times: [] },
     getDeviceList: { count: 0, args: [] }
   };
   const client = fxa.internal.fxAccountsClient;
   client.registerDevice = function() {
@@ -311,61 +331,60 @@ add_task(async function test_updateDevic
     spy.updateDevice.time = Date.now();
     if (spy.updateDevice.count === 1) {
       return Promise.reject({
         code: 400,
         errno: ERRNO_DEVICE_SESSION_CONFLICT
       });
     }
     return Promise.resolve({
-      id: credentials.deviceId,
+      id: conflictingDeviceId,
       name: deviceName
     });
   };
   client.getDeviceList = function() {
     spy.getDeviceList.count += 1;
     spy.getDeviceList.args.push(arguments);
     spy.getDeviceList.time = Date.now();
     return Promise.resolve([
       { id: "ignore", name: "ignore", type: "ignore", isCurrentDevice: false },
-      { id: credentials.deviceId, name: deviceName, type: deviceType, isCurrentDevice: true }
+      { id: conflictingDeviceId, name: deviceName, type: deviceType, isCurrentDevice: true }
     ]);
   };
 
-  const result = await fxa.updateDeviceRegistration();
+  await fxa.updateDeviceRegistration();
 
-  Assert.equal(result, credentials.deviceId);
   Assert.equal(spy.registerDevice.count, 0);
   Assert.equal(spy.updateDevice.count, 1);
   Assert.equal(spy.updateDevice.args[0].length, 4);
   Assert.equal(spy.updateDevice.args[0][0], credentials.sessionToken);
-  Assert.equal(spy.updateDevice.args[0][1], credentials.deviceId);
+  Assert.equal(spy.updateDevice.args[0][1], currentDeviceId);
   Assert.equal(spy.updateDevice.args[0][2], deviceName);
   Assert.equal(spy.updateDevice.args[0][3].pushCallback, "http://mochi.test:8888");
   Assert.equal(spy.updateDevice.args[0][3].pushPublicKey, BOGUS_PUBLICKEY);
   Assert.equal(spy.updateDevice.args[0][3].pushAuthKey, BOGUS_AUTHKEY);
   Assert.equal(spy.getDeviceList.count, 1);
   Assert.equal(spy.getDeviceList.args[0].length, 1);
   Assert.equal(spy.getDeviceList.args[0][0], credentials.sessionToken);
   Assert.ok(spy.getDeviceList.time >= spy.updateDevice.time);
 
   const state = fxa.internal.currentAccountState;
   const data = await state.getUserAccountData();
 
-  Assert.equal(data.deviceId, credentials.deviceId);
-  Assert.equal(data.deviceRegistrationVersion, null);
+  Assert.equal(data.device.id, conflictingDeviceId);
+  Assert.equal(data.device.registrationVersion, null);
 });
 
 add_task(async function test_updateDeviceRegistration_with_unrecoverable_error() {
   const deviceName = "foo";
 
   const credentials = getTestUser("baz");
-  delete credentials.deviceId;
   const fxa = new MockFxAccounts({ name: deviceName });
   await fxa.internal.setSignedInUser(credentials);
+  await fxa.updateUserAccountData({uid: credentials.uid, device: null});
 
   const spy = {
     registerDevice: { count: 0, args: [] },
     updateDevice: { count: 0, args: [] },
     getDeviceList: { count: 0, args: [] }
   };
   const client = fxa.internal.fxAccountsClient;
   client.registerDevice = function() {
@@ -382,88 +401,87 @@ add_task(async function test_updateDevic
     return Promise.resolve({});
   };
   client.getDeviceList = function() {
     spy.getDeviceList.count += 1;
     spy.getDeviceList.args.push(arguments);
     return Promise.resolve([]);
   };
 
-  const result = await fxa.updateDeviceRegistration();
+  await fxa.updateDeviceRegistration();
 
-  Assert.equal(null, result);
   Assert.equal(spy.getDeviceList.count, 0);
   Assert.equal(spy.updateDevice.count, 0);
   Assert.equal(spy.registerDevice.count, 1);
   Assert.equal(spy.registerDevice.args[0].length, 4);
 
   const state = fxa.internal.currentAccountState;
   const data = await state.getUserAccountData();
 
-  Assert.equal(null, data.deviceId);
+  Assert.equal(null, data.device);
 });
 
 add_task(async function test_getDeviceId_with_no_device_id_invokes_device_registration() {
   const credentials = getTestUser("foo");
   credentials.verified = true;
-  delete credentials.deviceId;
   const fxa = new MockFxAccounts();
   await fxa.internal.setSignedInUser(credentials);
+  await fxa.updateUserAccountData({uid: credentials.uid, device: null});
 
   const spy = { count: 0, args: [] };
   fxa.internal.currentAccountState.getUserAccountData =
     () => Promise.resolve({ email: credentials.email,
-                            deviceRegistrationVersion: DEVICE_REGISTRATION_VERSION });
+                            registrationVersion: DEVICE_REGISTRATION_VERSION });
   fxa.internal._registerOrUpdateDevice = function() {
     spy.count += 1;
     spy.args.push(arguments);
     return Promise.resolve("bar");
   };
 
   const result = await fxa.internal.getDeviceId();
 
   Assert.equal(spy.count, 1);
   Assert.equal(spy.args[0].length, 1);
   Assert.equal(spy.args[0][0].email, credentials.email);
-  Assert.equal(null, spy.args[0][0].deviceId);
+  Assert.equal(null, spy.args[0][0].device);
   Assert.equal(result, "bar");
 });
 
 add_task(async function test_getDeviceId_with_registration_version_outdated_invokes_device_registration() {
   const credentials = getTestUser("foo");
   credentials.verified = true;
   const fxa = new MockFxAccounts();
   await fxa.internal.setSignedInUser(credentials);
 
   const spy = { count: 0, args: [] };
   fxa.internal.currentAccountState.getUserAccountData =
-    () => Promise.resolve({ deviceId: credentials.deviceId, deviceRegistrationVersion: 0 });
+    () => Promise.resolve({ device: {id: "my id", registrationVersion: 0}});
   fxa.internal._registerOrUpdateDevice = function() {
     spy.count += 1;
     spy.args.push(arguments);
     return Promise.resolve("wibble");
   };
 
   const result = await fxa.internal.getDeviceId();
 
   Assert.equal(spy.count, 1);
   Assert.equal(spy.args[0].length, 1);
-  Assert.equal(spy.args[0][0].deviceId, credentials.deviceId);
+  Assert.equal(spy.args[0][0].device.id, "my id");
   Assert.equal(result, "wibble");
 });
 
 add_task(async function test_getDeviceId_with_device_id_and_uptodate_registration_version_doesnt_invoke_device_registration() {
   const credentials = getTestUser("foo");
   credentials.verified = true;
   const fxa = new MockFxAccounts();
   await fxa.internal.setSignedInUser(credentials);
 
   const spy = { count: 0 };
   fxa.internal.currentAccountState.getUserAccountData =
-    () => Promise.resolve({ deviceId: credentials.deviceId, deviceRegistrationVersion: DEVICE_REGISTRATION_VERSION });
+    () => Promise.resolve({ device: {id: "foo's device id", registrationVersion: DEVICE_REGISTRATION_VERSION}});
   fxa.internal._registerOrUpdateDevice = function() {
     spy.count += 1;
     return Promise.resolve("bar");
   };
 
   const result = await fxa.internal.getDeviceId();
 
   Assert.equal(spy.count, 0);
@@ -473,46 +491,66 @@ add_task(async function test_getDeviceId
 add_task(async function test_getDeviceId_with_device_id_and_with_no_registration_version_invokes_device_registration() {
   const credentials = getTestUser("foo");
   credentials.verified = true;
   const fxa = new MockFxAccounts();
   await fxa.internal.setSignedInUser(credentials);
 
   const spy = { count: 0, args: [] };
   fxa.internal.currentAccountState.getUserAccountData =
-    () => Promise.resolve({ deviceId: credentials.deviceId });
+    () => Promise.resolve({ device: {id: "wibble"}});
   fxa.internal._registerOrUpdateDevice = function() {
     spy.count += 1;
     spy.args.push(arguments);
     return Promise.resolve("wibble");
   };
 
   const result = await fxa.internal.getDeviceId();
 
   Assert.equal(spy.count, 1);
   Assert.equal(spy.args[0].length, 1);
-  Assert.equal(spy.args[0][0].deviceId, credentials.deviceId);
+  Assert.equal(spy.args[0][0].device.id, "wibble");
   Assert.equal(result, "wibble");
 });
 
+add_task(async function test_migration_toplevel_deviceId_to_device() {
+  const credentials = getTestUser("foo");
+  credentials.verified = true;
+  const fxa = new MockFxAccounts();
+  await fxa.internal.setSignedInUser(credentials);
+  await fxa.updateUserAccountData({uid: credentials.uid, device: null});
+  // Can't use updateUserAccountData here since it won't accept deprecated fields!
+  const accountData = fxa.internal.currentAccountState.storageManager.accountData;
+  accountData.deviceId = "mydeviceid";
+  accountData.deviceRegistrationVersion = DEVICE_REGISTRATION_VERSION;
+
+  const result = await fxa.internal.getDeviceId();
+  Assert.equal(result, "mydeviceid");
+
+  const state = fxa.internal.currentAccountState;
+  const data = await state.getUserAccountData();
+  Assert.deepEqual(data.device, {id: "mydeviceid", registrationVersion: DEVICE_REGISTRATION_VERSION});
+  Assert.ok(!data.deviceId);
+  Assert.ok(!data.deviceRegistrationVersion);
+});
+
 function expandHex(two_hex) {
   // Return a 64-character hex string, encoding 32 identical bytes.
   let eight_hex = two_hex + two_hex + two_hex + two_hex;
   let thirtytwo_hex = eight_hex + eight_hex + eight_hex + eight_hex;
   return thirtytwo_hex + thirtytwo_hex;
 }
 
 function expandBytes(two_hex) {
   return CommonUtils.hexToBytes(expandHex(two_hex));
 }
 
 function getTestUser(name) {
   return {
     email: name + "@example.com",
     uid: "1ad7f502-4cc7-4ec1-a209-071fd2fae348",
-    deviceId: name + "'s device id",
     sessionToken: name + "'s session token",
     keyFetchToken: name + "'s keyfetch token",
     unwrapBKey: expandHex("44"),
     verified: false
   };
 }
 
--- a/services/fxaccounts/tests/xpcshell/test_oauth_grant_client.js
+++ b/services/fxaccounts/tests/xpcshell/test_oauth_grant_client.js
@@ -65,17 +65,18 @@ add_test(function missingParams() {
   run_next_test();
 });
 
 add_test(function successfulResponse() {
   let client = new FxAccountsOAuthGrantClient(CLIENT_OPTIONS);
   let response = {
     success: true,
     status: STATUS_SUCCESS,
-    body: "{\"access_token\":\"http://example.com/image.jpeg\",\"id\":\"0d5c1a89b8c54580b8e3e8adadae864a\"}",
+    body: JSON.stringify({access_token: "http://example.com/image.jpeg",
+                          id: "0d5c1a89b8c54580b8e3e8adadae864a"})
   };
 
   client._Request = new mockResponse(response);
   client.getTokenFromAssertion("assertion", "scope")
     .then(
       function(result) {
         Assert.equal(result.access_token, "http://example.com/image.jpeg");
         run_next_test();
@@ -83,17 +84,17 @@ add_test(function successfulResponse() {
     );
 });
 
 add_test(function successfulDestroy() {
   let client = new FxAccountsOAuthGrantClient(CLIENT_OPTIONS);
   let response = {
     success: true,
     status: STATUS_SUCCESS,
-    body: "{}",
+    body: JSON.stringify({}),
   };
 
   client._Request = new mockResponse(response);
   client.destroyToken("deadbeef").then(run_next_test);
 });
 
 add_test(function parseErrorResponse() {
   let client = new FxAccountsOAuthGrantClient(CLIENT_OPTIONS);
@@ -114,17 +115,18 @@ add_test(function parseErrorResponse() {
       run_next_test();
     });
 });
 
 add_task(async function serverErrorResponse() {
   let client = new FxAccountsOAuthGrantClient(CLIENT_OPTIONS);
   let response = {
     status: 400,
-    body: "{ \"code\": 400, \"errno\": 104, \"error\": \"Bad Request\", \"message\": \"Unauthorized\", \"reason\": \"Invalid fxa assertion\" }",
+    body: JSON.stringify({code: 400, errno: 104, error: "Bad Request",
+                          message: "Unauthorized", reason: "Invalid fxa assertion"}),
   };
 
   client._Request = new mockResponse(response);
   client.getTokenFromAssertion("blah", "scope")
     .catch(function(e) {
       Assert.equal(e.name, "FxAccountsOAuthGrantClientError");
       Assert.equal(e.code, 400);
       Assert.equal(e.errno, ERRNO_INVALID_FXA_ASSERTION);
@@ -178,17 +180,18 @@ add_test(function onCompleteRequestError
       run_next_test();
     });
 });
 
 add_test(function incorrectErrno() {
   let client = new FxAccountsOAuthGrantClient(CLIENT_OPTIONS);
   let response = {
     status: 400,
-    body: "{ \"code\": 400, \"errno\": \"bad errno\", \"error\": \"Bad Request\", \"message\": \"Unauthorized\", \"reason\": \"Invalid fxa assertion\" }",
+    body: JSON.stringify({code: 400, errno: "bad errno", error: "Bad Request",
+                          message: "Unauthorized", reason: "Invalid fxa assertion"}),
   };
 
   client._Request = new mockResponse(response);
   client.getTokenFromAssertion("blah", "scope")
     .catch(function(e) {
       Assert.equal(e.name, "FxAccountsOAuthGrantClientError");
       Assert.equal(e.code, 400);
       Assert.equal(e.errno, ERRNO_UNKNOWN_ERROR);
--- a/services/fxaccounts/tests/xpcshell/test_push_service.js
+++ b/services/fxaccounts/tests/xpcshell/test_push_service.js
@@ -188,17 +188,17 @@ add_task(async function observePushTopic
   };
 
   let signoutCalled = false;
   let { FxAccounts } = ChromeUtils.import("resource://gre/modules/FxAccounts.jsm", {});
   const fxAccountsMock = new FxAccounts({
     newAccountState() {
       return {
         async getUserAccountData() {
-          return { deviceId };
+          return {device: {id: deviceId}};
         }
       };
     },
     signOut() {
       signoutCalled = true;
     }
   });
 
@@ -238,17 +238,17 @@ add_task(async function observePushTopic
   };
 
   let signoutCalled = false;
   let { FxAccounts } = ChromeUtils.import("resource://gre/modules/FxAccounts.jsm", {});
   const fxAccountsMock = new FxAccounts({
     newAccountState() {
       return {
         async getUserAccountData() {
-          return { deviceId: "thelocaldevice" };
+          return {device: {id: "thelocaldevice"}};
         }
       };
     },
     signOut() {
       signoutCalled = true;
     }
   });
 
--- a/services/fxaccounts/tests/xpcshell/test_storage_manager.js
+++ b/services/fxaccounts/tests/xpcshell/test_storage_manager.js
@@ -97,82 +97,85 @@ add_storage_task(async function checkIni
 
 // Initialized with account data (ie, simulating a new user being logged in).
 // Should reflect the initial data and be written to storage.
 add_storage_task(async function checkNewUser(sm) {
   let initialAccountData = {
     uid: "uid",
     email: "someone@somewhere.com",
     kXCS: "kXCS",
-    deviceId: "device id"
+    device: {
+      id: "device id"
+    }
   };
   sm.plainStorage = new MockedPlainStorage();
   if (sm.secureStorage) {
     sm.secureStorage = new MockedSecureStorage(null);
   }
   await sm.initialize(initialAccountData);
   let accountData = await sm.getAccountData();
   Assert.equal(accountData.uid, initialAccountData.uid);
   Assert.equal(accountData.email, initialAccountData.email);
   Assert.equal(accountData.kXCS, initialAccountData.kXCS);
-  Assert.equal(accountData.deviceId, initialAccountData.deviceId);
+  Assert.deepEqual(accountData.device, initialAccountData.device);
 
   // and it should have been written to storage.
   Assert.equal(sm.plainStorage.data.accountData.uid, initialAccountData.uid);
   Assert.equal(sm.plainStorage.data.accountData.email, initialAccountData.email);
-  Assert.equal(sm.plainStorage.data.accountData.deviceId, initialAccountData.deviceId);
+  Assert.deepEqual(sm.plainStorage.data.accountData.device, initialAccountData.device);
   // check secure
   if (sm.secureStorage) {
     Assert.equal(sm.secureStorage.data.accountData.kXCS, initialAccountData.kXCS);
   } else {
     Assert.equal(sm.plainStorage.data.accountData.kXCS, initialAccountData.kXCS);
   }
 });
 
 // Initialized without account data but storage has it available.
 add_storage_task(async function checkEverythingRead(sm) {
   sm.plainStorage = new MockedPlainStorage({
     uid: "uid",
     email: "someone@somewhere.com",
-    deviceId: "wibble",
-    deviceRegistrationVersion: null
+    device: {
+      id: "wibble",
+      registrationVersion: null
+    }
   });
   if (sm.secureStorage) {
     sm.secureStorage = new MockedSecureStorage(null);
   }
   await sm.initialize();
   let accountData = await sm.getAccountData();
   Assert.ok(accountData, "read account data");
   Assert.equal(accountData.uid, "uid");
   Assert.equal(accountData.email, "someone@somewhere.com");
-  Assert.equal(accountData.deviceId, "wibble");
-  Assert.equal(accountData.deviceRegistrationVersion, null);
+  Assert.deepEqual(accountData.device, {id: "wibble", registrationVersion: null});
   // Update the data - we should be able to fetch it back and it should appear
   // in our storage.
   await sm.updateAccountData({
     verified: true,
     kSync: "kSync",
     kXCS: "kXCS",
     kExtSync: "kExtSync",
     kExtKbHash: "kExtKbHash",
-    deviceRegistrationVersion: DEVICE_REGISTRATION_VERSION
+    device: {
+      id: "wibble",
+      registrationVersion: DEVICE_REGISTRATION_VERSION
+    }
   });
   accountData = await sm.getAccountData();
   Assert.equal(accountData.kSync, "kSync");
   Assert.equal(accountData.kXCS, "kXCS");
   Assert.equal(accountData.kExtSync, "kExtSync");
   Assert.equal(accountData.kExtKbHash, "kExtKbHash");
-  Assert.equal(accountData.deviceId, "wibble");
-  Assert.equal(accountData.deviceRegistrationVersion, DEVICE_REGISTRATION_VERSION);
+  Assert.deepEqual(accountData.device, {id: "wibble", registrationVersion: DEVICE_REGISTRATION_VERSION});
   // Check the new value was written to storage.
   await sm._promiseStorageComplete; // storage is written in the background.
-  // "verified", "deviceId" and "deviceRegistrationVersion" are plain-text fields.
   Assert.equal(sm.plainStorage.data.accountData.verified, true);
-  Assert.equal(sm.plainStorage.data.accountData.deviceId, "wibble");
-  Assert.equal(sm.plainStorage.data.accountData.deviceRegistrationVersion, DEVICE_REGISTRATION_VERSION);
+  Assert.deepEqual(sm.plainStorage.data.accountData.device, {id: "wibble", registrationVersion: DEVICE_REGISTRATION_VERSION});
   // derive keys are secure
   if (sm.secureStorage) {
     Assert.equal(sm.secureStorage.data.accountData.kExtKbHash, "kExtKbHash");
     Assert.equal(sm.secureStorage.data.accountData.kExtSync, "kExtSync");
     Assert.equal(sm.secureStorage.data.accountData.kXCS, "kXCS");
     Assert.equal(sm.secureStorage.data.accountData.kSync, "kSync");
   } else {
     Assert.equal(sm.plainStorage.data.accountData.kExtKbHash, "kExtKbHash");
--- a/services/fxaccounts/tests/xpcshell/test_web_channel.js
+++ b/services/fxaccounts/tests/xpcshell/test_web_channel.js
@@ -771,18 +771,18 @@ add_task(async function test_helpers_cha
   };
   let helpers = new FxAccountsWebChannelHelpers({
     fxAccounts: {
       updateUserAccountData(credentials) {
         return new Promise(resolve => {
           Assert.ok(credentials.hasOwnProperty("email"));
           Assert.ok(credentials.hasOwnProperty("uid"));
           Assert.ok(credentials.hasOwnProperty("unwrapBKey"));
-          Assert.ok(credentials.hasOwnProperty("deviceId"));
-          Assert.equal(null, credentials.deviceId);
+          Assert.ok(credentials.hasOwnProperty("device"));
+          Assert.equal(null, credentials.device);
           // "foo" isn't a field known by storage, so should be dropped.
           Assert.ok(!credentials.hasOwnProperty("foo"));
           wasCalled.updateUserAccountData = true;
 
           resolve();
         });
       },