Bug 1033238 - Cannot revoke Mobile ID permission. r=jedp, a=2.0+
authorFernando Jiménez <ferjmoreno@gmail.com>
Fri, 11 Jul 2014 14:56:57 +0200
changeset 207900 a8b32e56b4f0346b4a63218350f582682e1f32ac
parent 207899 0061c1fe7e28f94a15dc9373dd2914ddc8db5b1a
child 207901 e100e6b0efc18af6db4dd9a08f7d74127f617051
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjedp, 2
bugs1033238
milestone32.0a2
Bug 1033238 - Cannot revoke Mobile ID permission. r=jedp, a=2.0+
services/mobileid/MobileIdentityManager.jsm
services/mobileid/tests/xpcshell/test_mobileid_manager.js
--- a/services/mobileid/MobileIdentityManager.jsm
+++ b/services/mobileid/MobileIdentityManager.jsm
@@ -355,16 +355,35 @@ this.MobileIdentityManager = {
     log.debug("UI resend code");
     if (!this.activeVerificationFlow) {
       return;
     }
     this.doVerification();
   },
 
   /*********************************************************
+   * Result helpers
+   *********************************************************/
+  success: function(aPromiseId, aResult) {
+    let mm = this.messageManagers[aPromiseId];
+    mm.sendAsyncMessage("MobileId:GetAssertion:Return:OK", {
+      promiseId: aPromiseId,
+      result: aResult
+    });
+  },
+
+  error: function(aPromiseId, aError) {
+    let mm = this.messageManagers[aPromiseId];
+    mm.sendAsyncMessage("MobileId:GetAssertion:Return:KO", {
+      promiseId: aPromiseId,
+      error: aError
+    });
+  },
+
+  /*********************************************************
    * Permissions helper
    ********************************************************/
 
   addPermission: function(aPrincipal) {
     permissionManager.addFromPrincipal(aPrincipal, MOBILEID_PERM,
                                        Ci.nsIPermissionManager.ALLOW_ACTION);
   },
 
@@ -774,19 +793,30 @@ this.MobileIdentityManager = {
     return deferred.promise;
   },
 
   getMobileIdAssertion: function(aPrincipal, aPromiseId, aOptions) {
     log.debug("getMobileIdAssertion ${}", aPrincipal);
 
     let uri = Services.io.newURI(aPrincipal.origin, null, null);
     let principal = securityManager.getAppCodebasePrincipal(
-      uri, aPrincipal.appid, aPrincipal.isInBrowserElement);
+      uri, aPrincipal.appId, aPrincipal.isInBrowserElement);
     let manifestURL = appsService.getManifestURLByLocalId(aPrincipal.appId);
 
+    let permission = permissionManager.testPermissionFromPrincipal(
+      principal,
+      MOBILEID_PERM
+    );
+
+    if (permission == Ci.nsIPermissionManager.DENY_ACTION ||
+        permission == Ci.nsIPermissionManager.UNKNOWN_ACTION) {
+      this.error(aPromiseId, ERROR_PERMISSION_DENIED);
+      return;
+    }
+
     let _creds;
 
     // First of all we look if we already have credentials for this origin.
     // If we don't have credentials it means that it is the first time that
     // the caller requested an assertion.
     this.credStore.getByOrigin(aPrincipal.origin)
     .then(
       (creds) => {
@@ -859,25 +889,18 @@ this.MobileIdentityManager = {
       (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.
         if (creds) {
-          let permission = permissionManager.testPermissionFromPrincipal(
-            principal,
-            MOBILEID_PERM
-          );
           if (permission == Ci.nsIPermissionManager.ALLOW_ACTION) {
             return creds;
-          } else if (permission == Ci.nsIPermissionManager.DENY_ACTION ||
-                     permission == Ci.nsIPermissionManager.UNKNOWN_ACTION) {
-            return Promise.reject(ERROR_PERMISSION_DENIED);
           }
           return this.promptAndVerify(principal, manifestURL, creds);
         }
         return this.promptAndVerify(principal, manifestURL);
       }
     )
     .then(
       (creds) => {
@@ -905,21 +928,17 @@ this.MobileIdentityManager = {
                                                         .replace(/_/g, '/')));
 
         if (!decodedPayload || !decodedPayload.verifiedMSISDN) {
           return Promise.reject(ERROR_INVALID_ASSERTION);
         }
 
         this.ui.verified(decodedPayload.verifiedMSISDN);
 
-        let mm = this.messageManagers[aPromiseId];
-        mm.sendAsyncMessage("MobileId:GetAssertion:Return:OK", {
-          promiseId: aPromiseId,
-          result: assertion
-        });
+        this.success(aPromiseId, assertion);
       }
     )
     .then(
       null,
       (error) => {
         log.error("getMobileIdAssertion rejected with ${}", error);
 
         // If we got an invalid token error means that the credentials that
@@ -933,20 +952,16 @@ this.MobileIdentityManager = {
           _creds && this.credStore.delete(_creds.msisdn);
           this.getMobileIdAssertion(aPrincipal, aPromiseId, aOptions);
           return;
         }
 
         // Notify the error to the UI.
         this.ui.error(error);
 
-        let mm = this.messageManagers[aPromiseId];
-        mm.sendAsyncMessage("MobileId:GetAssertion:Return:KO", {
-          promiseId: aPromiseId,
-          error: error
-        });
+        this.error(aPromiseId, error);
       }
     );
   },
 
 };
 
 MobileIdentityManager.init();
--- a/services/mobileid/tests/xpcshell/test_mobileid_manager.js
+++ b/services/mobileid/tests/xpcshell/test_mobileid_manager.js
@@ -24,51 +24,52 @@ const DEBUG = false;
 
 const GET_ASSERTION_IPC_MSG = "MobileId:GetAssertion";
 const GET_ASSERTION_RETURN_OK = "MobileId:GetAssertion:Return:OK";
 const GET_ASSERTION_RETURN_KO = "MobileId:GetAssertion:Return:KO";
 
 // === Globals ===
 
 const ORIGIN = "app://afakeorigin";
+const APP_ID = 1;
 const PRINCIPAL = {
   origin: ORIGIN,
-  appId: "123"
+  appId: APP_ID
 };
 const PHONE_NUMBER = "+34666555444";
 const ANOTHER_PHONE_NUMBER = "+44123123123";
 const VERIFICATION_CODE = "123456";
 const SESSION_TOKEN = "aSessionToken";
 const ICC_ID = "aIccId";
 const MNC = "aMnc";
 const MCC = 214;
 const OPERATOR = "aOperator";
 const CERTIFICATE = "eyJhbGciOiJEUzI1NiJ9.eyJsYXN0QXV0aEF0IjoxNDA0NDY5NzkyODc3LCJ2ZXJpZmllZE1TSVNETiI6IiszMTYxNzgxNTc1OCIsInB1YmxpYy1rZXkiOnsiYWxnb3JpdGhtIjoiRFMiLCJ5IjoiNGE5YzkzNDY3MWZhNzQ3YmM2ZjMyNjE0YTg1MzUyZjY5NDcwMDdhNTRkMDAxMDY4OWU5ZjJjZjc0ZGUwYTEwZTRlYjlmNDk1ZGFmZTA0NGVjZmVlNDlkN2YwOGU4ODQyMDJiOTE5OGRhNWZhZWE5MGUzZjRmNzE1YzZjNGY4Yjc3MGYxZTU4YWZhNDM0NzVhYmFiN2VlZGE1MmUyNjk2YzFmNTljNzMzYjFlYzBhNGNkOTM1YWIxYzkyNzAxYjNiYTA5ZDRhM2E2MzNjNTJmZjE2NGYxMWY3OTg1YzlmZjY3ZThmZDFlYzA2NDU3MTdkMjBiNDE4YmM5M2YzYzVkNCIsInAiOiJmZjYwMDQ4M2RiNmFiZmM1YjQ1ZWFiNzg1OTRiMzUzM2Q1NTBkOWYxYmYyYTk5MmE3YThkYWE2ZGMzNGY4MDQ1YWQ0ZTZlMGM0MjlkMzM0ZWVlYWFlZmQ3ZTIzZDQ4MTBiZTAwZTRjYzE0OTJjYmEzMjViYTgxZmYyZDVhNWIzMDVhOGQxN2ViM2JmNGEwNmEzNDlkMzkyZTAwZDMyOTc0NGE1MTc5MzgwMzQ0ZTgyYTE4YzQ3OTMzNDM4Zjg5MWUyMmFlZWY4MTJkNjljOGY3NWUzMjZjYjcwZWEwMDBjM2Y3NzZkZmRiZDYwNDYzOGMyZWY3MTdmYzI2ZDAyZTE3IiwicSI6ImUyMWUwNGY5MTFkMWVkNzk5MTAwOGVjYWFiM2JmNzc1OTg0MzA5YzMiLCJnIjoiYzUyYTRhMGZmM2I3ZTYxZmRmMTg2N2NlODQxMzgzNjlhNjE1NGY0YWZhOTI5NjZlM2M4MjdlMjVjZmE2Y2Y1MDhiOTBlNWRlNDE5ZTEzMzdlMDdhMmU5ZTJhM2NkNWRlYTcwNGQxNzVmOGViZjZhZjM5N2Q2OWUxMTBiOTZhZmIxN2M3YTAzMjU5MzI5ZTQ4MjliMGQwM2JiYzc4OTZiMTViNGFkZTUzZTEzMDg1OGNjMzRkOTYyNjlhYTg5MDQxZjQwOTEzNmM3MjQyYTM4ODk1YzlkNWJjY2FkNGYzODlhZjFkN2E0YmQxMzk4YmQwNzJkZmZhODk2MjMzMzk3YSJ9LCJwcmluY2lwYWwiOiIwMzgxOTgyYS0xZTgzLTI1NjYtNjgzZS05MDRmNDA0NGM1MGRAbXNpc2RuLWRldi5zdGFnZS5tb3phd3MubmV0IiwiaWF0IjoxNDA0NDY5NzgyODc3LCJleHAiOjE0MDQ0OTEzOTI4NzcsImlzcyI6Im1zaXNkbi1kZXYuc3RhZ2UubW96YXdzLm5ldCJ9."
 
 // === Helpers ===
 
-function addPermission(aOrigin, aAction) {
+function addPermission(aAction) {
   let uri = Cc["@mozilla.org/network/io-service;1"]
               .getService(Ci.nsIIOService)
-              .newURI(aOrigin, null, null);
+              .newURI(ORIGIN, null, null);
   let _principal = Cc["@mozilla.org/scriptsecuritymanager;1"]
                      .getService(Ci.nsIScriptSecurityManager)
-                     .getNoAppCodebasePrincipal(uri);
+                     .getAppCodebasePrincipal(uri, APP_ID, false);
   let pm = Cc["@mozilla.org/permissionmanager;1"]
              .getService(Ci.nsIPermissionManager);
   pm.addFromPrincipal(_principal, MOBILEID_PERM, aAction);
 }
 
-function removePermission(aOrigin) {
+function removePermission() {
   let uri = Cc["@mozilla.org/network/io-service;1"]
               .getService(Ci.nsIIOService)
-              .newURI(aOrigin, null, null);
+              .newURI(ORIGIN, null, null);
   let _principal = Cc["@mozilla.org/scriptsecuritymanager;1"]
                      .getService(Ci.nsIScriptSecurityManager)
-                     .getNoAppCodebasePrincipal(uri);
+                     .getAppCodebasePrincipal(uri, APP_ID, false);
   let pm = Cc["@mozilla.org/permissionmanager;1"]
              .getService(Ci.nsIPermissionManager);
   pm.removeFromPrincipal(_principal, MOBILEID_PERM);
 }
 
 // === Mocks ===
 
 let Mock = function(aOptions) {
@@ -357,16 +358,17 @@ let (XULAppInfo = {
 
 // === Global cleanup ===
 
 function cleanup() {
   MobileIdentityManager.credStore = kMobileIdentityCredStore;
   MobileIdentityManager.client = kMobileIdentityClient;
   MobileIdentityManager.ui = null;
   MobileIdentityManager.iccInfo = null;
+  removePermission(ORIGIN);
 }
 
 // Unregister mocks and restore original code.
 do_register_cleanup(cleanup);
 
 // === Tests ===
 function run_test() {
   run_next_test();
@@ -442,16 +444,18 @@ add_test(function() {
       client._("sign").call(1).arg(1, SESSION_TOKEN);
       client._("sign").call(1).arg(2, CERTIFICATE_LIFETIME);
 
       do_test_finished();
       run_next_test();
     }
   };
 
+  addPermission(Ci.nsIPermissionManager.ALLOW_ACTION);
+
   MobileIdentityManager.receiveMessage({
     name: GET_ASSERTION_IPC_MSG,
     principal: PRINCIPAL,
     target: mm,
     json: {
       promiseId: promiseId
     }
   });
@@ -730,17 +734,17 @@ add_test(function() {
 
       removePermission(ORIGIN);
 
       do_test_finished();
       run_next_test();
     }
   };
 
-  addPermission(ORIGIN, Ci.nsIPermissionManager.ALLOW_ACTION);
+  addPermission(Ci.nsIPermissionManager.ALLOW_ACTION);
 
   MobileIdentityManager.receiveMessage({
     name: GET_ASSERTION_IPC_MSG,
     principal: PRINCIPAL,
     target: mm,
     json: {
       promiseId: promiseId,
       options: {}
@@ -790,17 +794,17 @@ add_test(function() {
 
       removePermission(ORIGIN);
 
       do_test_finished();
       run_next_test();
     }
   };
 
-  addPermission(ORIGIN, Ci.nsIPermissionManager.PROMPT_ACTION);
+  addPermission(Ci.nsIPermissionManager.PROMPT_ACTION);
 
   MobileIdentityManager.receiveMessage({
     name: GET_ASSERTION_IPC_MSG,
     principal: PRINCIPAL,
     target: mm,
     json: {
       promiseId: promiseId,
       options: {}
@@ -838,23 +842,21 @@ add_test(function() {
       do_check_eq(aMsg, GET_ASSERTION_RETURN_KO);
       do_check_eq(typeof aData, "object");
       do_check_eq(aData.promiseId, promiseId);
       do_check_eq(aData.error, ERROR_PERMISSION_DENIED);
 
       // Check spied calls.
 
       // MockCredStore.
-      credStore._("getByOrigin").callsLength(1);
-      credStore._("getByOrigin").call(1).arg(1, ORIGIN);
+      credStore._("getByOrigin").callsLength(0);
 
       // MockUI.
       ui._("startFlow").callsLength(0);
-      ui._("error").callsLength(1);
-      ui._("error").call(1).arg(1, ERROR_PERMISSION_DENIED);
+      ui._("error").callsLength(0);
 
       do_test_finished();
       run_next_test();
     }
   };
 
   removePermission(ORIGIN);
 
@@ -928,16 +930,18 @@ add_test(function() {
       client._("verifyCode").callsLength(0);
       client._("sign").callsLength(0);
 
       do_test_finished();
       run_next_test();
     }
   };
 
+  addPermission(Ci.nsIPermissionManager.ALLOW_ACTION);
+
   MobileIdentityManager.receiveMessage({
     name: GET_ASSERTION_IPC_MSG,
     principal: PRINCIPAL,
     target: mm,
     json: {
       promiseId: promiseId,
       options: {}
     }
@@ -1287,17 +1291,17 @@ add_test(function() {
       client._("verifyCode").callsLength(1);
       client._("sign").callsLength(1);
 
       do_test_finished();
       run_next_test();
     }
   };
 
-  addPermission(ORIGIN, Ci.nsIPermissionManager.ALLOW_ACTION);
+  addPermission(Ci.nsIPermissionManager.ALLOW_ACTION);
 
   MobileIdentityManager.receiveMessage({
     name: GET_ASSERTION_IPC_MSG,
     principal: PRINCIPAL,
     target: mm,
     json: {
       promiseId: promiseId,
       options: {}