Bug 1337244 - Delete previous device registration when setting-up a new FxA user. r=markh

MozReview-Commit-ID: wp4OJ1nwKH

--HG--
extra : rebase_source : 31cc6e6c40c9f0b0e6efa76032a5ae5c75281306

services/fxaccounts/FxAccounts.jsm

@@ -69,6 +69,7 @@ var publicProperties = [
   "setSignedInUser",
   "signOut",
   "updateDeviceRegistration",
+  "deleteDeviceRegistration",
   "updateUserAccountData",
   "whenVerified",
 ];
@@ -546,7 +547,13 @@ FxAccountsInternal.prototype = {
    */
   setSignedInUser: function setSignedInUser(credentials) {
     log.debug("setSignedInUser - aborting any existing flows");
-    return this.abortExistingFlow().then(() => {
+    return this.getSignedInUser().then(signedInUser => {
+      if (signedInUser) {
+        return this.deleteDeviceRegistration(signedInUser.sessionToken, signedInUser.deviceId);
+      }
+    }).then(() =>
+      this.abortExistingFlow()
+    ).then(() => {
       let currentAccountState = this.currentAccountState = this.newAccountState(
         Cu.cloneInto(credentials, {}) // Pass a clone of the credentials object.
       );
@@ -567,7 +574,7 @@ FxAccountsInternal.prototype = {
       }).then(() => {
         return currentAccountState.resolve();
       });
-    })
+    });
   },
 
   /**
@@ -823,8 +830,8 @@ FxAccountsInternal.prototype = {
     const options = { service: "sync" };
 
     if (deviceId) {
-      log.debug("destroying device and session");
-      return this.fxAccountsClient.signOutAndDestroyDevice(sessionToken, deviceId, options);
+      log.debug("destroying device, session and unsubscribing from FxA push");
+      return this.deleteDeviceRegistration(sessionToken, deviceId);
     }
 
     log.debug("destroying session");
@@ -1536,6 +1543,31 @@ FxAccountsInternal.prototype = {
     }).catch(error => this._logErrorAndResetDeviceRegistrationVersion(error));
   },
 
+  // Delete the Push Subscription and the device registration on the auth server.
+  // Returns a promise that always resolves, never rejects.
+  async deleteDeviceRegistration(sessionToken, deviceId) {
+    try {
+      // Allow tests to skip device registration because it makes remote requests to the auth server.
+      if (Services.prefs.getBoolPref("identity.fxaccounts.skipDeviceRegistration")) {
+        return Promise.resolve();
+      }
+    } catch (ignore) {}
+
+    try {
+      await this.fxaPushService.unsubscribe();
+      if (sessionToken && deviceId) {
+        await this.fxAccountsClient.signOutAndDestroyDevice(sessionToken, deviceId);
+      }
+      this.currentAccountState.updateUserAccountData({
+        deviceId: null,
+        deviceRegistrationVersion: null
+      });
+    } catch (err) {
+      log.error("Could not delete the device registration", err);
+    }
+    return Promise.resolve();
+  },
+
   handleDeviceDisconnection(deviceId) {
     return this.currentAccountState.getUserAccountData()
       .then(data => data ? data.deviceId : null)

services/fxaccounts/tests/xpcshell/test_accounts.js

@@ -70,6 +70,9 @@ MockStorageManager.prototype = {
   },
 
   updateAccountData(updatedFields) {
+    if (!this.accountData) {
+      return Promise.resolve();
+    }
     for (let [name, value] of Object.entries(updatedFields)) {
       if (value == null) {
         delete this.accountData[name];
@@ -256,6 +259,30 @@ add_task(function* test_get_signed_in_user_initially_unset() {
   do_check_eq(result, null);
 });
 
+add_task(function* test_set_signed_in_user_deletes_previous_device() {
+  _("Check setSignedInUser tries to delete a previous registered device");
+  let account = MakeFxAccounts();
+  let deleteDeviceRegistrationCalled = false;
+  let credentials = {
+    email: "foo@example.com",
+    uid: "1234@lcip.org",
+    assertion: "foobar",
+    sessionToken: "dead",
+    kA: "beef",
+    kB: "cafe",
+    verified: true
+  };
+  yield account.setSignedInUser(credentials);
+
+  account.internal.deleteDeviceRegistration = () => {
+    deleteDeviceRegistrationCalled = true;
+    return Promise.resolve(true);
+  }
+
+  yield account.setSignedInUser(credentials);
+  do_check_true(deleteDeviceRegistrationCalled);
+});
+
 add_task(function* test_update_account_data() {
   _("Check updateUserAccountData does the right thing.");
   let account = MakeFxAccounts();
@@ -961,16 +988,16 @@ add_task(function* test_sign_out_with_device() {
 
   const spy = {
     signOut: { count: 0 },
-    signOutAndDeviceDestroy: { count: 0, args: [] }
+    deleteDeviceRegistration: { count: 0, args: [] }
   };
   const client = fxa.internal.fxAccountsClient;
   client.signOut = function() {
     spy.signOut.count += 1;
     return Promise.resolve();
   };
-  client.signOutAndDestroyDevice = function() {
-    spy.signOutAndDeviceDestroy.count += 1;
-    spy.signOutAndDeviceDestroy.args.push(arguments);
+  fxa.internal.deleteDeviceRegistration = function() {
+    spy.deleteDeviceRegistration.count += 1;
+    spy.deleteDeviceRegistration.args.push(arguments);
     return Promise.resolve();
   };
 
@@ -981,12 +1008,10 @@ add_task(function* test_sign_out_with_device() {
       fxa.internal.getUserAccountData().then(user2 => {
         do_check_eq(user2, null);
         do_check_eq(spy.signOut.count, 0);
-        do_check_eq(spy.signOutAndDeviceDestroy.count, 1);
-        do_check_eq(spy.signOutAndDeviceDestroy.args[0].length, 3);
-        do_check_eq(spy.signOutAndDeviceDestroy.args[0][0], credentials.sessionToken);
-        do_check_eq(spy.signOutAndDeviceDestroy.args[0][1], credentials.deviceId);
-        do_check_true(spy.signOutAndDeviceDestroy.args[0][2]);
-        do_check_eq(spy.signOutAndDeviceDestroy.args[0][2].service, "sync");
+        do_check_eq(spy.deleteDeviceRegistration.count, 1);
+        do_check_eq(spy.deleteDeviceRegistration.args[0].length, 2);
+        do_check_eq(spy.deleteDeviceRegistration.args[0][0], credentials.sessionToken);
+        do_check_eq(spy.deleteDeviceRegistration.args[0][1], credentials.deviceId);
         resolve();
       });
     });
@@ -1008,7 +1033,7 @@ add_task(function* test_sign_out_without_device() {
 
   const spy = {
     signOut: { count: 0, args: [] },
-    signOutAndDeviceDestroy: { count: 0 }
+    deleteDeviceRegistration: { count: 0 }
   };
   const client = fxa.internal.fxAccountsClient;
   client.signOut = function() {
@@ -1016,8 +1041,8 @@ add_task(function* test_sign_out_without_device() {
     spy.signOut.args.push(arguments);
     return Promise.resolve();
   };
-  client.signOutAndDestroyDevice = function() {
-    spy.signOutAndDeviceDestroy.count += 1;
+  fxa.internal.deleteDeviceRegistration = function() {
+    spy.deleteDeviceRegistration.count += 1;
     return Promise.resolve();
   };
 
@@ -1032,7 +1057,7 @@ add_task(function* test_sign_out_without_device() {
         do_check_eq(spy.signOut.args[0][0], credentials.sessionToken);
         do_check_true(spy.signOut.args[0][1]);
         do_check_eq(spy.signOut.args[0][1].service, "sync");
-        do_check_eq(spy.signOutAndDeviceDestroy.count, 0);
+        do_check_eq(spy.deleteDeviceRegistration.count, 0);
         resolve();
       });
     });
@@ -1045,10 +1070,9 @@ add_task(function* test_sign_out_without_device() {
 
 add_task(function* test_sign_out_with_remote_error() {
   let fxa = new MockFxAccounts();
-  let client = fxa.internal.fxAccountsClient;
   let remoteSignOutCalled = false;
   // Force remote sign out to trigger an error
-  client.signOutAndDestroyDevice = function() { remoteSignOutCalled = true; throw "Remote sign out error"; };
+  fxa.internal.deleteDeviceRegistration = function() { remoteSignOutCalled = true; throw "Remote sign out error"; };
   let promiseLogout = new Promise(resolve => {
     makeObserver(ONLOGOUT_NOTIFICATION, function() {
       log.debug("test_sign_out_with_remote_error observed onlogout");

services/fxaccounts/tests/xpcshell/test_accounts_device_registration.js

@@ -117,6 +117,9 @@ function MockFxAccounts(device = {}) {
           });
         });
       },
+      unsubscribe() {
+        return Promise.resolve();
+      }
     },
     DEVICE_REGISTRATION_VERSION
   });
@@ -291,6 +294,38 @@ add_task(function* test_updateDeviceRegistration_with_unknown_device_error() {
   do_check_eq(data.deviceRegistrationVersion, DEVICE_REGISTRATION_VERSION);
 });
 
+add_task(function* test_deleteDeviceRegistration() {
+  const credentials = getTestUser("pb");
+  const fxa = new MockFxAccounts({ name: "my device" });
+  yield fxa.internal.setSignedInUser(credentials);
+
+  const state = fxa.internal.currentAccountState;
+  let data = yield state.getUserAccountData();
+  do_check_eq(data.deviceId, credentials.deviceId);
+  do_check_eq(data.deviceRegistrationVersion, DEVICE_REGISTRATION_VERSION);
+
+  const spy = {
+    signOutAndDestroyDevice: { count: 0, args: [] }
+  };
+  const client = fxa.internal.fxAccountsClient;
+  client.signOutAndDestroyDevice = function() {
+    spy.signOutAndDestroyDevice.count += 1;
+    spy.signOutAndDestroyDevice.args.push(arguments);
+    return Promise.resolve({});
+  };
+  yield fxa.deleteDeviceRegistration(credentials.sessionToken, credentials.deviceId);
+
+  do_check_eq(spy.signOutAndDestroyDevice.count, 1);
+  do_check_eq(spy.signOutAndDestroyDevice.args[0].length, 2);
+  do_check_eq(spy.signOutAndDestroyDevice.args[0][0], credentials.sessionToken);
+  do_check_eq(spy.signOutAndDestroyDevice.args[0][1], credentials.deviceId);
+
+  data = yield state.getUserAccountData();
+
+  do_check_false(data.deviceId);
+  do_check_false(data.deviceRegistrationVersion);
+});
+
 add_task(function* test_updateDeviceRegistration_with_device_session_conflict_error() {
   const deviceName = "foo";
   const deviceType = "bar";

services/sync/tps/extensions/tps/resource/auth/fxaccounts.jsm

@@ -111,7 +111,7 @@ var Authentication = {
       let { sessionToken, deviceId } = user;
       if (deviceId) {
         Logger.logInfo("Destroying device " + deviceId);
-        Async.promiseSpinningly(fxc.signOutAndDestroyDevice(sessionToken, deviceId, { service: "sync" }));
+        Async.promiseSpinningly(fxAccounts.deleteDeviceRegistration(sessionToken, deviceId));
       } else {
         Logger.logError("No device found.");
         Async.promiseSpinningly(fxc.signOut(sessionToken, { service: "sync" }));