From b38e622a8418ffede314431e5d4efae104e42f92 Mon Sep 17 00:00:00 2001 From: Jed Parsons Date: Tue, 11 Mar 2014 17:49:26 -0700 Subject: [PATCH] Bug 978896 - FxA: watch() should get silent assertion if possible. r=ferjm --- dom/identity/DOMIdentity.jsm | 1 + dom/identity/nsDOMIdentity.js | 2 +- .../tests/mochitest/file_declareAudience.html | 6 +- .../tests/mochitest/test_declareAudience.html | 11 +- .../tests/mochitest/test_syntheticEvents.html | 5 +- services/fxaccounts/FxAccountsManager.jsm | 20 ++- toolkit/identity/FirefoxAccounts.jsm | 30 +++- toolkit/identity/tests/unit/head_identity.js | 30 ++-- .../tests/unit/test_firefox_accounts.js | 145 ++++++++++++------ 9 files changed, 172 insertions(+), 78 deletions(-) diff --git a/dom/identity/DOMIdentity.jsm b/dom/identity/DOMIdentity.jsm index 3bdf6a09db7..c30d5d03b77 100644 --- a/dom/identity/DOMIdentity.jsm +++ b/dom/identity/DOMIdentity.jsm @@ -335,6 +335,7 @@ this.DOMIdentity = { }, _unwatch: function DOMIdentity_unwatch(message, targetMM) { + log("DOMIDentity__unwatch: " + message.id); this.getService(message).RP.unwatch(message.id, targetMM); }, diff --git a/dom/identity/nsDOMIdentity.js b/dom/identity/nsDOMIdentity.js index aeaf247d16a..af75473f40f 100644 --- a/dom/identity/nsDOMIdentity.js +++ b/dom/identity/nsDOMIdentity.js @@ -670,7 +670,7 @@ nsDOMIdentity.prototype = { }, uninit: function DOMIdentity_uninit() { - this._log("nsDOMIdentity uninit()"); + this._log("nsDOMIdentity uninit() " + this._id); this._identityInternal._mm.sendAsyncMessage( "Identity:RP:Unwatch", { id: this._id } diff --git a/dom/identity/tests/mochitest/file_declareAudience.html b/dom/identity/tests/mochitest/file_declareAudience.html index d96ab6b4327..3313ca3ca83 100644 --- a/dom/identity/tests/mochitest/file_declareAudience.html +++ b/dom/identity/tests/mochitest/file_declareAudience.html @@ -41,7 +41,11 @@ onready: onready, onlogin: onlogin, onerror: onerror, - onlogout: function() {}, + + // onlogout will actually be called every time watch() is invoked, + // because fxa will find no signed-in user and so trigger logout. + // For this test, though, we don't care and just ignore logout. + onlogout: function () {}, }); }; diff --git a/dom/identity/tests/mochitest/test_declareAudience.html b/dom/identity/tests/mochitest/test_declareAudience.html index 59e49b013be..4ef9daf5f4c 100644 --- a/dom/identity/tests/mochitest/test_declareAudience.html +++ b/dom/identity/tests/mochitest/test_declareAudience.html @@ -34,7 +34,13 @@ is("appStatus" in document.nodePrincipal, true, function MockFXAManager() {} MockFXAManager.prototype = { - getAssertion: function(audience) { + getAssertion: function(audience, options) { + // Always reject a request for a silent assertion, simulating the + // scenario in which there is no signed-in user to begin with. + if (options.silent) { + return Promise.resolve(null); + } + let deferred = Promise.defer(); jwcrypto.generateKeyPair("DS160", (err, kp) => { if (err) { @@ -137,7 +143,7 @@ function receiveMessage(event) { let expected = app.expected; is(result.success, expected.success, - "Assertion request " + (expected.success ? "succeeds" : "fails")); + "Assertion request succeeds"); if (expected.success) { // Confirm that the assertion audience and origin are as expected @@ -180,7 +186,6 @@ window.addEventListener("message", receiveMessage, false, true); function runTest() { for (let app of apps) { dump("** Testing " + app.title + "\n"); - // Set up state for message handler expectedErrors = 0; receivedErrors = []; diff --git a/dom/identity/tests/mochitest/test_syntheticEvents.html b/dom/identity/tests/mochitest/test_syntheticEvents.html index 194c8533587..2e01deac6bd 100644 --- a/dom/identity/tests/mochitest/test_syntheticEvents.html +++ b/dom/identity/tests/mochitest/test_syntheticEvents.html @@ -31,7 +31,10 @@ Components.utils.import("resource://gre/modules/identity/FirefoxAccounts.jsm"); // plumbing. function MockFXAManager() {} MockFXAManager.prototype = { - getAssertion: function() { + getAssertion: function(audience, options) { + if (options.silent) { + return Promise.resolve(null); + } return Promise.resolve("here~you.go.dude"); } }; diff --git a/services/fxaccounts/FxAccountsManager.jsm b/services/fxaccounts/FxAccountsManager.jsm index 81cca5f8b5c..a293b412e11 100644 --- a/services/fxaccounts/FxAccountsManager.jsm +++ b/services/fxaccounts/FxAccountsManager.jsm @@ -356,8 +356,18 @@ this.FxAccountsManager = { ); }, + /* + * Try to get an assertion for the given audience. + * + * aOptions can include: + * + * refreshAuthentication - (bool) Force re-auth. + * + * silent - (bool) Prevent any UI interaction. + * I.e., try to get an automatic assertion. + * + */ getAssertion: function(aAudience, aOptions) { - log.debug("getAssertion " + aAudience + JSON.stringify(aOptions)); if (!aAudience) { return this._error(ERROR_INVALID_AUDIENCE); } @@ -390,6 +400,9 @@ this.FxAccountsManager = { // will return the assertion. Otherwise, we will return an error. return this._signOut().then( () => { + if (aOptions.silent) { + return Promise.resolve(null); + } return this._uiRequest(UI_REQUEST_REFRESH_AUTH, aAudience, user.accountId); } @@ -401,6 +414,11 @@ this.FxAccountsManager = { } log.debug("No signed in user"); + + if (aOptions.silent) { + return Promise.resolve(null); + } + // If there is no currently signed in user, we trigger the signIn UI // flow. return this._uiRequest(UI_REQUEST_SIGN_IN_FLOW, aAudience); diff --git a/toolkit/identity/FirefoxAccounts.jsm b/toolkit/identity/FirefoxAccounts.jsm index 2e9a0600f60..e12d80e44f6 100644 --- a/toolkit/identity/FirefoxAccounts.jsm +++ b/toolkit/identity/FirefoxAccounts.jsm @@ -87,20 +87,38 @@ FxAccountsService.prototype = { */ watch: function watch(aRpCaller) { this._rpFlows.set(aRpCaller.id, aRpCaller); + log.debug("watch: " + aRpCaller.id); log.debug("Current rp flows: " + this._rpFlows.size); - // Nothing to do but call ready() + // Log the user in, if possible, and then call ready(). let runnable = { run: () => { - this.doReady(aRpCaller.id); + this.fxAccountsManager.getAssertion(aRpCaller.audience, {silent:true}).then( + data => { + if (data) { + this.doLogin(aRpCaller.id, data); + } else { + this.doLogout(aRpCaller.id); + } + this.doReady(aRpCaller.id); + }, + error => { + log.error("get silent assertion failed: " + JSON.stringify(error)); + this.doError(aRpCaller.id, error.toString()); + } + ); } }; Services.tm.currentThread.dispatch(runnable, Ci.nsIThread.DISPATCH_NORMAL); }, - unwatch: function(aRpCaller, aTargetMM) { - // nothing to do + /** + * Delete the flow when the screen is unloaded + */ + unwatch: function(aRpCallerId, aTargetMM) { + log.debug("unwatching: " + aRpCallerId); + this._rpFlows.delete(aRpCallerId); }, /** @@ -160,7 +178,9 @@ FxAccountsService.prototype = { // Call logout() on the next tick let runnable = { run: () => { - this.doLogout(aRpCallerId); + this.fxAccountsManager.signOut().then(() => { + this.doLogout(aRpCallerId); + }); } }; Services.tm.currentThread.dispatch(runnable, diff --git a/toolkit/identity/tests/unit/head_identity.js b/toolkit/identity/tests/unit/head_identity.js index 70fa5809394..20917c794f1 100644 --- a/toolkit/identity/tests/unit/head_identity.js +++ b/toolkit/identity/tests/unit/head_identity.js @@ -9,7 +9,7 @@ const Cr = Components.results; Cu.import("resource://testing-common/httpd.js"); // XXX until bug 937114 is fixed -Cu.importGlobalProperties(['atob']); +Cu.importGlobalProperties(["atob"]); // The following boilerplate makes sure that XPCom calls // that use the profile directory work. @@ -43,7 +43,7 @@ const TEST_URL2 = "https://myfavoritebaconinacan.com"; const TEST_USER = "user@mozilla.com"; const TEST_PRIVKEY = "fake-privkey"; const TEST_CERT = "fake-cert"; -const TEST_ASSERTION = "face-assertion"; +const TEST_ASSERTION = "fake-assertion"; const TEST_IDPPARAMS = { domain: "myfavoriteflan.com", authentication: "/foo/authenticate.html", @@ -72,8 +72,8 @@ function uuid() { } function base64UrlDecode(s) { - s = s.replace(/-/g, '+'); - s = s.replace(/_/g, '/'); + s = s.replace(/-/g, "+"); + s = s.replace(/_/g, "/"); // Replace padding if it was stripped by the sender. // See http://tools.ietf.org/html/rfc4648#section-4 @@ -101,15 +101,15 @@ function mock_doc(aIdentity, aOrigin, aDoFunc) { mockedDoc.id = uuid(); mockedDoc.loggedInUser = aIdentity; mockedDoc.origin = aOrigin; - mockedDoc['do'] = aDoFunc; + mockedDoc["do"] = aDoFunc; mockedDoc._mm = TEST_MESSAGE_MANAGER; - mockedDoc.doReady = partial(aDoFunc, 'ready'); - mockedDoc.doLogin = partial(aDoFunc, 'login'); - mockedDoc.doLogout = partial(aDoFunc, 'logout'); - mockedDoc.doError = partial(aDoFunc, 'error'); - mockedDoc.doCancel = partial(aDoFunc, 'cancel'); - mockedDoc.doCoffee = partial(aDoFunc, 'coffee'); - mockedDoc.childProcessShutdown = partial(aDoFunc, 'child-process-shutdown'); + mockedDoc.doReady = partial(aDoFunc, "ready"); + mockedDoc.doLogin = partial(aDoFunc, "login"); + mockedDoc.doLogout = partial(aDoFunc, "logout"); + mockedDoc.doError = partial(aDoFunc, "error"); + mockedDoc.doCancel = partial(aDoFunc, "cancel"); + mockedDoc.doCoffee = partial(aDoFunc, "coffee"); + mockedDoc.childProcessShutdown = partial(aDoFunc, "child-process-shutdown"); mockedDoc.RP = mockedDoc; @@ -127,9 +127,9 @@ function mock_fxa_rp(aIdentity, aOrigin, aDoFunc) { mockedDoc.doReady = partial(aDoFunc, "ready"); mockedDoc.doLogin = partial(aDoFunc, "login"); mockedDoc.doLogout = partial(aDoFunc, "logout"); - mockedDoc.doError = partial(aDoFunc, 'error'); - mockedDoc.doCancel = partial(aDoFunc, 'cancel'); - mockedDoc.childProcessShutdown = partial(aDoFunc, 'child-process-shutdown'); + mockedDoc.doError = partial(aDoFunc, "error"); + mockedDoc.doCancel = partial(aDoFunc, "cancel"); + mockedDoc.childProcessShutdown = partial(aDoFunc, "child-process-shutdown"); mockedDoc.RP = mockedDoc; diff --git a/toolkit/identity/tests/unit/test_firefox_accounts.js b/toolkit/identity/tests/unit/test_firefox_accounts.js index 7434740a5fe..698f2af3b45 100644 --- a/toolkit/identity/tests/unit/test_firefox_accounts.js +++ b/toolkit/identity/tests/unit/test_firefox_accounts.js @@ -14,13 +14,19 @@ XPCOMUtils.defineLazyModuleGetter(this, "FirefoxAccounts", // data. do_get_profile(); -function MockFXAManager() {} +function MockFXAManager() { + this.signedIn = true; +} MockFXAManager.prototype = { getAssertion: function(audience) { - let deferred = Promise.defer(); - deferred.resolve(TEST_ASSERTION); - return deferred.promise; - } + let result = this.signedIn ? TEST_ASSERTION : null; + return Promise.resolve(result); + }, + + signOut: function() { + this.signedIn = false; + return Promise.resolve(null); + }, } let originalManager = FirefoxAccounts.fxAccountsManager; @@ -45,13 +51,49 @@ function test_mock() { }); } -function test_watch() { +function test_watch_signed_in() { do_test_pending(); + let received = []; + + let mockedRP = mock_fxa_rp(null, TEST_URL, function(method, data) { + received.push([method, data]); + + if (method == "ready") { + // confirm that we were signed in and then ready was called + do_check_eq(received.length, 2); + do_check_eq(received[0][0], "login"); + do_check_eq(received[0][1], TEST_ASSERTION); + do_check_eq(received[1][0], "ready"); + do_test_finished(); + run_next_test(); + } + }); + + FirefoxAccounts.RP.watch(mockedRP); +} + +function test_watch_signed_out() { + do_test_pending(); + + let received = []; + let signedInState = FirefoxAccounts.fxAccountsManager.signedIn; + FirefoxAccounts.fxAccountsManager.signedIn = false; + let mockedRP = mock_fxa_rp(null, TEST_URL, function(method) { - do_check_eq(method, "ready"); - do_test_finished(); - run_next_test(); + received.push(method); + + if (method == "ready") { + // confirm that we were signed out and then ready was called + do_check_eq(received.length, 2); + do_check_eq(received[0], "logout"); + do_check_eq(received[1], "ready"); + + // restore initial state + FirefoxAccounts.fxAccountsManager.signedIn = signedInState; + do_test_finished(); + run_next_test(); + } }); FirefoxAccounts.RP.watch(mockedRP); @@ -62,21 +104,31 @@ function test_request() { let received = []; - let mockedRP = mock_fxa_rp(null, TEST_URL, function(method) { - // We will received "ready" as a result of watch(), then "login" - // as a result of request() - received.push(method); + // initially signed out + let signedInState = FirefoxAccounts.fxAccountsManager.signedIn; + FirefoxAccounts.fxAccountsManager.signedIn = false; - if (received.length == 2) { - do_check_eq(received[0], "ready"); - do_check_eq(received[1], "login"); - do_test_finished(); - run_next_test(); + let mockedRP = mock_fxa_rp(null, TEST_URL, function(method, data) { + received.push([method, data]); + + // On watch(), we are signed out. Then we call request(). + if (received.length === 2) { + do_check_eq(received[0][0], "logout"); + do_check_eq(received[1][0], "ready"); + + // Pretend request() showed ux and the user signed in + FirefoxAccounts.fxAccountsManager.signedIn = true; + FirefoxAccounts.RP.request(mockedRP.id); } - // Second, call request() - if (method == "ready") { - FirefoxAccounts.RP.request(mockedRP.id); + if (received.length === 3) { + do_check_eq(received[2][0], "login"); + do_check_eq(received[2][1], TEST_ASSERTION); + + // restore initial state + FirefoxAccounts.fxAccountsManager.signedIn = signedInState; + do_test_finished(); + run_next_test(); } }); @@ -90,20 +142,20 @@ function test_logout() { let received = []; let mockedRP = mock_fxa_rp(null, TEST_URL, function(method) { - // We will receive "ready" as a result of watch(), and "logout" - // as a result of logout() received.push(method); - if (received.length == 2) { - do_check_eq(received[0], "ready"); - do_check_eq(received[1], "logout"); - do_test_finished(); - run_next_test(); + // At first, watch() signs us in automatically. Then we sign out. + if (received.length === 2) { + do_check_eq(received[0], "login"); + do_check_eq(received[1], "ready"); + + FirefoxAccounts.RP.logout(mockedRP.id); } - if (method == "ready") { - // Second, call logout() - FirefoxAccounts.RP.logout(mockedRP.id); + if (received.length === 3) { + do_check_eq(received[2], "logout"); + do_test_finished(); + run_next_test(); } }); @@ -122,31 +174,21 @@ function test_error() { let originalManager = FirefoxAccounts.fxAccountsManager; FirefoxAccounts.RP.fxAccountsManager = { getAssertion: function(audience) { - return Promise.reject("barf!"); + return Promise.reject(new Error("barf!")); } }; let mockedRP = mock_fxa_rp(null, TEST_URL, function(method, message) { - // We will receive "ready" as a result of watch(), and "logout" - // as a result of logout() - received.push([method, message]); + // We will immediately receive an error, due to watch()'s attempt + // to getAssertion(). + do_check_eq(method, "error"); + do_check_true(/barf/.test(message)); - if (received.length == 2) { - do_check_eq(received[0][0], "ready"); + // Put things back the way they were + FirefoxAccounts.fxAccountsManager = originalManager; - do_check_eq(received[1][0], "error"); - do_check_eq(received[1][1], "barf!"); - - // Put things back the way they were - FirefoxAccounts.fxAccountsManager = originalManager; - - do_test_finished(); - run_next_test(); - } - - if (method == "ready") { - FirefoxAccounts.RP.request(mockedRP.id); - } + do_test_finished(); + run_next_test(); }); // First, call watch() @@ -197,7 +239,8 @@ function test_child_process_shutdown() { let TESTS = [ test_overall, test_mock, - test_watch, + test_watch_signed_in, + test_watch_signed_out, test_request, test_logout, test_error,