From 42a7c412d9d7b6ff5b829e8057e37ca01e14656f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez?= Date: Mon, 6 Oct 2014 11:30:54 +0200 Subject: [PATCH] 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 | 20 +++++++++++----- .../tests/xpcshell/test_mobileid_manager.js | 24 ++++++++----------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/services/mobileid/MobileIdentityManager.jsm b/services/mobileid/MobileIdentityManager.jsm index 3c3556fcc2b..dcf5dc3f1cf 100644 --- a/services/mobileid/MobileIdentityManager.jsm +++ b/services/mobileid/MobileIdentityManager.jsm @@ -280,8 +280,7 @@ this.MobileIdentityManager = { .then( (creds) => { if (creds) { - this.iccInfo[aServiceId].credentials = creds; - return; + return creds; } return this.credStore.getByMsisdn(this.iccInfo[aServiceId].msisdn); } @@ -306,10 +305,12 @@ this.MobileIdentityManager = { ) .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 = @@ -675,14 +676,16 @@ this.MobileIdentityManager = { 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; @@ -972,7 +975,12 @@ this.MobileIdentityManager = { // 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; } diff --git a/services/mobileid/tests/xpcshell/test_mobileid_manager.js b/services/mobileid/tests/xpcshell/test_mobileid_manager.js index aa2fdba7e37..920ec8bc58b 100644 --- a/services/mobileid/tests/xpcshell/test_mobileid_manager.js +++ b/services/mobileid/tests/xpcshell/test_mobileid_manager.js @@ -457,7 +457,7 @@ add_test(function() { 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. @@ -960,10 +960,10 @@ add_test(function() { 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); @@ -1054,7 +1054,7 @@ add_test(function() { 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); @@ -1139,7 +1139,7 @@ add_test(function() { 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); @@ -1229,7 +1229,7 @@ add_test(function() { 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); @@ -1278,7 +1278,7 @@ add_test(function() { sessionToken: _sessionToken, msisdn: PHONE_NUMBER, origin: ORIGIN, - deviceIccIds: null + deviceIccIds: [] }; let ui = new MockUi({ @@ -1323,7 +1323,7 @@ add_test(function() { 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); @@ -1383,12 +1383,8 @@ add_test(function() { 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]; } };