Bug 1075070 - [MobileID] First time an app requests a MobileID assertion for an already verified phone fails if it has no previous permission. r=spenrose
authorFernando Jiménez <ferjmoreno@gmail.com>
Mon, 06 Oct 2014 11:30:54 +0200
changeset 232171 2f99196c505e869be276b1488044c8d12751519a
parent 232170 09ccd185571b6547f52bfc48c96169b173a6de82
child 232172 44ef6f3261f66b18328ba89d6d9898f65876de93
push id4187
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:29:12 +0000
treeherdermozilla-beta@f23cc6a30c11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersspenrose
bugs1075070
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1075070 - [MobileID] First time an app requests a MobileID assertion for an already verified phone fails if it has no previous permission. r=spenrose
services/mobileid/MobileIdentityManager.jsm
services/mobileid/tests/xpcshell/test_mobileid_manager.js
--- a/services/mobileid/MobileIdentityManager.jsm
+++ b/services/mobileid/MobileIdentityManager.jsm
@@ -275,18 +275,17 @@ this.MobileIdentityManager = {
     // First of all we need to check if we already have existing credentials
     // for the given SIM information (ICC ID or MSISDN). If we have no valid
     // credentials, we have to check with the server which options do we have
     // to verify the associated phone number.
     return this.credStore.getByIccId(this.iccInfo[aServiceId].iccId)
     .then(
       (creds) => {
         if (creds) {
-          this.iccInfo[aServiceId].credentials = creds;
-          return;
+          return creds;
         }
         return this.credStore.getByMsisdn(this.iccInfo[aServiceId].msisdn);
       }
     )
     .then(
       (creds) => {
         if (creds) {
           this.iccInfo[aServiceId].credentials = creds;
@@ -301,20 +300,22 @@ this.MobileIdentityManager = {
         return this.client.discover(this.iccInfo[aServiceId].msisdn,
                                     this.iccInfo[aServiceId].mcc,
                                     this.iccInfo[aServiceId].mnc,
                                     this.iccInfo[aServiceId].roaming);
       }
     )
     .then(
       (result) => {
-        log.debug("Discover result ${}", result);
+        // If we already have credentials for this ICC and no discover request
+        // is done, we just bail out.
         if (!result || !result.verificationMethods) {
           return;
         }
+        log.debug("Discover result ${}", result);
         this.iccInfo[aServiceId].verificationMethods = result.verificationMethods;
         this.iccInfo[aServiceId].verificationDetails = result.verificationDetails;
         this.iccInfo[aServiceId].canDoSilentVerification =
           (result.verificationMethods.indexOf(SMS_MO_MT) != -1);
         return;
       }
     );
   },
@@ -670,24 +671,26 @@ this.MobileIdentityManager = {
             (!result.phoneNumber && (result.serviceId === undefined))) {
           return Promise.reject(ERROR_INTERNAL_INVALID_PROMPT_RESULT);
         }
 
         let msisdn;
         let mcc;
 
         // If the user selected one of the existing SIM cards we have to check
-        // that we either have the MSISDN for that SIM or we can do a silent
-        // verification that does not require us to have the MSISDN in advance.
+        // that we either have the MSISDN for that SIM, we have already existing
+        // credentials or we can do a silent verification that does not require
+        // us to have the MSISDN in advance.
         // result.serviceId can be "0".
         if (result.serviceId !== undefined &&
             result.serviceId !== null) {
           let icc = this.iccInfo[result.serviceId];
           log.debug("icc ${}", icc);
-          if (!icc || !icc.msisdn && !icc.canDoSilentVerification) {
+          if (!icc || !icc.msisdn && !icc.canDoSilentVerification &&
+              !icc.credentials) {
             return Promise.reject(ERROR_INTERNAL_CANNOT_VERIFY_SELECTION);
           }
           msisdn = icc.msisdn;
           mcc = icc.mcc;
         } else {
           msisdn = result.prefix ? result.prefix + result.phoneNumber
                                  : result.phoneNumber;
           mcc = result.mcc;
@@ -967,17 +970,22 @@ this.MobileIdentityManager = {
     .then(
       (creds) => {
         // Even if we have credentails it is possible that the user has
         // removed the permission to share its mobile id with this origin, so
         // we check the permission and if it is not granted, we ask the user
         // before generating and sharing the assertion.
         // If we've just prompted the user in the previous step, the permission
         // is already granted and stored so we just progress the credentials.
+        // But we have to refresh the cached permission before checking.
         if (creds) {
+          permission = permissionManager.testPermissionFromPrincipal(
+            principal,
+            MOBILEID_PERM
+          );
           if (permission == Ci.nsIPermissionManager.ALLOW_ACTION) {
             return creds;
           }
           return this.promptAndVerify(principal, manifestURL, creds);
         }
         return this.promptAndVerify(principal, manifestURL);
       }
     )
--- a/services/mobileid/tests/xpcshell/test_mobileid_manager.js
+++ b/services/mobileid/tests/xpcshell/test_mobileid_manager.js
@@ -452,17 +452,17 @@ add_test(function() {
       credStore._("getByOrigin").call(1).arg(1, ORIGIN);
       credStore._("getByMsisdn").callsLength(1);
       credStore._("getByMsisdn").call(1).arg(1, PHONE_NUMBER);
       credStore._("add").callsLength(1);
       credStore._("add").call(1).arg(1, undefined);
       credStore._("add").call(1).arg(2, PHONE_NUMBER);
       credStore._("add").call(1).arg(3, ORIGIN);
       credStore._("add").call(1).arg(4, SESSION_TOKEN);
-      credStore._("add").call(1).arg(5, null);
+      credStore._("add").call(1).arg(5, []);
 
 
       // MockUI.
       ui._("startFlow").callsLength(1);
       ui._("startFlow").call(1).arg(1, "");
       ui._("startFlow").call(1).arg(2, []);
       ui._("verifyCodePrompt").callsLength(1);
       ui._("verifyCodePrompt").call(1).arg(1, 3);
@@ -955,20 +955,20 @@ add_test(function() {
       // MockCredStore.
       credStore._("getByOrigin").callsLength(1);
       credStore._("getByOrigin").call(1).arg(1, ORIGIN);
       credStore._("add").callsLength(1);
       credStore._("add").call(1).arg(1, undefined);
       credStore._("add").call(1).arg(2, PHONE_NUMBER);
       credStore._("add").call(1).arg(3, ORIGIN);
       credStore._("add").call(1).arg(4, SESSION_TOKEN);
-      credStore._("add").call(1).arg(5, null);
+      credStore._("add").call(1).arg(5, []);
       credStore._("setDeviceIccIds").callsLength(1);
       credStore._("setDeviceIccIds").call(1).arg(1, PHONE_NUMBER);
-      credStore._("setDeviceIccIds").call(1).arg(2, null);
+      credStore._("setDeviceIccIds").call(1).arg(2, []);
 
       // MockUI.
       ui._("startFlow").callsLength(1);
       ui._("verifyCodePrompt").callsLength(0);
       ui._("verify").callsLength(0);
 
       // MockClient.
       client._("discover").callsLength(0);
@@ -1049,17 +1049,17 @@ add_test(function() {
       credStore._("getByOrigin").call(1).arg(1, ORIGIN);
       credStore._("getByMsisdn").callsLength(1);
       credStore._("getByMsisdn").call(1).arg(1, ANOTHER_PHONE_NUMBER);
       credStore._("add").callsLength(1);
       credStore._("add").call(1).arg(1, undefined);
       credStore._("add").call(1).arg(2, ANOTHER_PHONE_NUMBER);
       credStore._("add").call(1).arg(3, ORIGIN);
       credStore._("add").call(1).arg(4, _sessionToken);
-      credStore._("add").call(1).arg(5, null);
+      credStore._("add").call(1).arg(5, []);
       credStore._("setDeviceIccIds").callsLength(0);
       credStore._("removeOrigin").callsLength(1);
       credStore._("removeOrigin").call(1).arg(1, PHONE_NUMBER);
       credStore._("removeOrigin").call(1).arg(2, ORIGIN);
 
       // MockUI.
       ui._("startFlow").callsLength(1);
       ui._("verifyCodePrompt").callsLength(1);
@@ -1134,17 +1134,17 @@ add_test(function() {
       credStore._("getByOrigin").callsLength(1);
       credStore._("getByOrigin").call(1).arg(1, ORIGIN);
       credStore._("getByMsisdn").callsLength(0);
       credStore._("add").callsLength(1);
       credStore._("add").call(1).arg(1, undefined);
       credStore._("add").call(1).arg(2, PHONE_NUMBER);
       credStore._("add").call(1).arg(3, ORIGIN);
       credStore._("add").call(1).arg(4, _sessionToken);
-      credStore._("add").call(1).arg(5, null);
+      credStore._("add").call(1).arg(5, []);
       credStore._("setDeviceIccIds").callsLength(1);
       credStore._("removeOrigin").callsLength(0);
 
       // MockUI.
       ui._("startFlow").callsLength(1);
       ui._("verifyCodePrompt").callsLength(0);
       ui._("verify").callsLength(0);
 
@@ -1224,17 +1224,17 @@ add_test(function() {
       credStore._("getByOrigin").call(1).arg(1, ORIGIN);
       credStore._("getByMsisdn").callsLength(1);
       credStore._("getByMsisdn").call(1).arg(1, ANOTHER_PHONE_NUMBER);
       credStore._("add").callsLength(1);
       credStore._("add").call(1).arg(1, undefined);
       credStore._("add").call(1).arg(2, ANOTHER_PHONE_NUMBER);
       credStore._("add").call(1).arg(3, ORIGIN);
       credStore._("add").call(1).arg(4, _sessionToken);
-      credStore._("add").call(1).arg(5, null);
+      credStore._("add").call(1).arg(5, []);
       credStore._("setDeviceIccIds").callsLength(0);
       credStore._("removeOrigin").callsLength(1);
       credStore._("removeOrigin").call(1).arg(1, PHONE_NUMBER);
       credStore._("removeOrigin").call(1).arg(2, ORIGIN);
 
       // MockUI.
       ui._("startFlow").callsLength(1);
       ui._("verifyCodePrompt").callsLength(1);
@@ -1273,17 +1273,17 @@ add_test(function() {
   do_test_pending();
 
   let _sessionToken = Date.now();
 
   let existingCredentials = {
     sessionToken: _sessionToken,
     msisdn: PHONE_NUMBER,
     origin: ORIGIN,
-    deviceIccIds: null
+    deviceIccIds: []
   };
 
   let ui = new MockUi({
     startFlowResult: {
       phoneNumber: PHONE_NUMBER
     }
   });
   MobileIdentityManager.ui = ui;
@@ -1318,17 +1318,17 @@ add_test(function() {
       credStore._("getByOrigin").call(2).arg(1, ORIGIN);
       credStore._("getByMsisdn").callsLength(1);
       credStore._("getByMsisdn").call(1).arg(1, PHONE_NUMBER);
       credStore._("add").callsLength(1);
       credStore._("add").call(1).arg(1, undefined);
       credStore._("add").call(1).arg(2, PHONE_NUMBER);
       credStore._("add").call(1).arg(3, ORIGIN);
       credStore._("add").call(1).arg(4, SESSION_TOKEN);
-      credStore._("add").call(1).arg(5, null);
+      credStore._("add").call(1).arg(5, []);
       credStore._("setDeviceIccIds").callsLength(0);
       credStore._("delete").callsLength(1);
       credStore._("delete").call(1).arg(1, PHONE_NUMBER);
 
       // MockUI.
       ui._("startFlow").callsLength(1);
       ui._("verifyCodePrompt").callsLength(1);
       ui._("verify").callsLength(1);
@@ -1378,22 +1378,18 @@ add_test(function() {
 
     getRadioInterface: function(aIndex) {
       return this._interfaces[aIndex];
     }
   };
 
   MobileIdentityManager._mobileConnectionService = {
     _interfaces: [RADIO_INTERFACE, ANOTHER_RADIO_INTERFACE],
-    getVoiceConnectionInfo: function(aIndex) {
-      return this._interfaces[aIndex].voice;
-    },
-
-    getDataConnectionInfo: function(aIndex) {
-      return this._interfaces[aIndex].data;
+    getItemByServiceId: function(aIndex) {
+      return this._interfaces[aIndex];
     }
   };
 
   MobileIdentityManager._iccProvider = {
     _listeners: [],
     registerIccMsg: function(aClientId, aIccListener) {
       this._listeners.push(aIccListener);
     },