Bug 1033238 - Cannot revoke Mobile ID permission. r=jedp
authorFernando Jiménez <ferjmoreno@gmail.com>
Fri, 11 Jul 2014 14:56:57 +0200
changeset 193455 387dff702fab9b1097423f374cb5559b688ab93b
parent 193454 960c711f29132d36570cc18808fe82f8840a63f4
child 193456 610aa015d7eae120940e031b7ad742268d8161c9
push id27121
push userryanvm@gmail.com
push dateFri, 11 Jul 2014 19:59:41 +0000
treeherdermozilla-central@c11ea2f54a6a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjedp
bugs1033238
milestone33.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 1033238 - Cannot revoke Mobile ID permission. r=jedp
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: {}