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, a=bajaj
authorFernando Jiménez <ferjmoreno@gmail.com>
Mon, 06 Oct 2014 11:30:54 +0200
changeset 204106 2da5a884c70b9ced113d3c45bf2cabd7c8db5eb7
parent 204105 f92e54a91038697d7c29a109d9bd3deaeef89528
child 204107 9a3dcfcfbd24f0f2629f6b0560965011ce02fdc7
push id425
push userryanvm@gmail.com
push dateWed, 22 Oct 2014 15:00:39 +0000
treeherdermozilla-b2g32_v2_0@9a3dcfcfbd24 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersspenrose, bajaj
bugs1075070
milestone32.0
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, a=bajaj
services/mobileid/MobileIdentityManager.jsm
services/mobileid/tests/xpcshell/test_mobileid_manager.js
--- a/services/mobileid/MobileIdentityManager.jsm
+++ b/services/mobileid/MobileIdentityManager.jsm
@@ -267,18 +267,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;
@@ -293,20 +292,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;
       }
     );
   },
@@ -662,24 +663,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;
@@ -959,17 +962,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);
@@ -953,20 +953,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);
@@ -1045,17 +1045,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);
@@ -1130,17 +1130,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);
 
@@ -1220,17 +1220,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);
@@ -1269,17 +1269,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;
@@ -1314,17 +1314,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);
@@ -1374,22 +1374,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);
     },