From 44f6b5d48ebb8f5cd70d17449dfbe5ac44b4bb85 Mon Sep 17 00:00:00 2001 From: Mark Banner Date: Wed, 22 Apr 2015 19:17:15 +0100 Subject: [PATCH] Bug 1153788 - Part 1 Opportunistically obtain encryption keys for FxA for Loop conversations context. r=mikedeboer --- .../content/test/general/browser_fxa_oauth.js | 4 +-- .../general/browser_fxa_oauth_with_keys.html | 4 ++- browser/components/loop/LoopRooms.jsm | 28 ++++++++++++++----- browser/components/loop/MozLoopService.jsm | 18 ++++++++++-- .../loop/test/mochitest/browser.ini | 2 +- .../loop/test/mochitest/loop_fxa.sjs | 2 +- .../test_loopservice_encryptionkey.js | 20 +++++++++++-- 7 files changed, 60 insertions(+), 18 deletions(-) diff --git a/browser/base/content/test/general/browser_fxa_oauth.js b/browser/base/content/test/general/browser_fxa_oauth.js index 575f6b29fdda..f30d358e56c3 100644 --- a/browser/base/content/test/general/browser_fxa_oauth.js +++ b/browser/base/content/test/general/browser_fxa_oauth.js @@ -147,8 +147,8 @@ let gTests = [ Assert.ok(tabOpened); Assert.equal(tokenData.code, "code1"); Assert.equal(tokenData.state, "state"); - Assert.equal(keys.kAr, "kAr"); - Assert.equal(keys.kBr, "kBr"); + Assert.deepEqual(keys.kAr, {k: "kAr"}); + Assert.deepEqual(keys.kBr, {k: "kBr"}); resolve(); }; diff --git a/browser/base/content/test/general/browser_fxa_oauth_with_keys.html b/browser/base/content/test/general/browser_fxa_oauth_with_keys.html index c47b2f4c2a6c..2a83a4622868 100644 --- a/browser/base/content/test/general/browser_fxa_oauth_with_keys.html +++ b/browser/base/content/test/general/browser_fxa_oauth_with_keys.html @@ -16,7 +16,9 @@ state: "state", code: "code1", closeWindow: "signin", - keys: { kAr: 'kAr', kBr: 'kBr' }, + // Keys normally contain more information, but this is enough + // to keep Loop's tests happy. + keys: { kAr: { k: 'kAr' }, kBr: { k: 'kBr' }}, }, }, }, diff --git a/browser/components/loop/LoopRooms.jsm b/browser/components/loop/LoopRooms.jsm index 80aa6e153a46..cff23aeb0794 100644 --- a/browser/components/loop/LoopRooms.jsm +++ b/browser/components/loop/LoopRooms.jsm @@ -202,10 +202,10 @@ let LoopRoomsInternal = { * information. */ promiseEncryptRoomData: Task.async(function* (roomData) { - // For now, disable encryption/context if context is disabled, or if - // FxA is turned on. - if (!MozLoopService.getLoopPref("contextInConverations.enabled") || - this.sessionType == LOOP_SESSION_TYPE.FXA) { + // XXX We should only return unencrypted data whilst we're still working + // on context. Once bug 1115340 is fixed, this function should no longer be + // here. + function getUnencryptedData() { var serverRoomData = extend({}, roomData); delete serverRoomData.decryptedContext; @@ -218,6 +218,11 @@ let LoopRoomsInternal = { }; } + // For now, disable encryption/context if context is disabled + if (!MozLoopService.getLoopPref("contextInConverations.enabled")) { + return getUnencryptedData(); + } + var newRoomData = extend({}, roomData); if (!newRoomData.context) { @@ -227,7 +232,17 @@ let LoopRoomsInternal = { // First get the room key. let key = yield this.promiseGetOrCreateRoomKey(newRoomData); - newRoomData.context.wrappedKey = yield this.promiseEncryptedRoomKey(key); + try { + newRoomData.context.wrappedKey = yield this.promiseEncryptedRoomKey(key); + } + catch (ex) { + // XXX Bug 1153788 should remove this, then we can remove the whole + // try/catch. + if (ex.message == "FxA re-register not implemented") { + return getUnencryptedData(); + } + return Promise.reject(ex); + } // Now encrypt the actual data. newRoomData.context.value = yield loopCrypto.encryptBytes(key, @@ -654,8 +669,7 @@ let LoopRoomsInternal = { }; // If we're not encrypting currently, then only send the roomName. - if (!Services.prefs.getBoolPref("loop.contextInConverations.enabled") || - this.sessionType == LOOP_SESSION_TYPE.FXA) { + if (!Services.prefs.getBoolPref("loop.contextInConverations.enabled")) { sendData = { roomName: newRoomName }; diff --git a/browser/components/loop/MozLoopService.jsm b/browser/components/loop/MozLoopService.jsm index 690aec477938..df69a5e97272 100644 --- a/browser/components/loop/MozLoopService.jsm +++ b/browser/components/loop/MozLoopService.jsm @@ -959,6 +959,9 @@ let MozLoopServiceInternal = { gFxAOAuthClientPromise = this.promiseFxAOAuthParameters().then( parameters => { + // Add the fact that we want keys to the parameters. + parameters.keys = true; + try { gFxAOAuthClient = new FxAccountsOAuthClient({ parameters: parameters, @@ -1031,7 +1034,10 @@ let MozLoopServiceInternal = { * @param {Deferred} deferred used to resolve the gFxAOAuthClientPromise * @param {Object} result (with code and state) */ - _fxAOAuthComplete: function(deferred, result) { + _fxAOAuthComplete: function(deferred, result, keys) { + if (keys.kBr) { + Services.prefs.setCharPref("loop.key.fxa", keys.kBr.k); + } gFxAOAuthClientPromise = null; // Note: The state was already verified in FxAccountsOAuthClient. deferred.resolve(result); @@ -1331,8 +1337,14 @@ this.MozLoopService = { return new Promise((resolve, reject) => { if (this.userProfile) { // We're an FxA user. - // XXX Bug 1153788 will implement this for FxA. - reject(new Error("unimplemented")); + if (Services.prefs.prefHasUserValue("loop.key.fxa")) { + resolve(MozLoopService.getLoopPref("key.fxa")); + return; + } + + // XXX If we don't have a key for FxA yet, then simply reject for now. + // We'll create some sign-in/sign-out UX in bug 1153788. + reject(new Error("FxA re-register not implemented")); return; } diff --git a/browser/components/loop/test/mochitest/browser.ini b/browser/components/loop/test/mochitest/browser.ini index a7f75f5e1427..39ea034c39ef 100644 --- a/browser/components/loop/test/mochitest/browser.ini +++ b/browser/components/loop/test/mochitest/browser.ini @@ -7,7 +7,7 @@ support-files = google_service.sjs head.js loop_fxa.sjs - ../../../../base/content/test/general/browser_fxa_oauth.html + ../../../../base/content/test/general/browser_fxa_oauth_with_keys.html [browser_CardDavImporter.js] [browser_fxa_login.js] diff --git a/browser/components/loop/test/mochitest/loop_fxa.sjs b/browser/components/loop/test/mochitest/loop_fxa.sjs index 1facf00b0d4f..c427a915c959 100644 --- a/browser/components/loop/test/mochitest/loop_fxa.sjs +++ b/browser/components/loop/test/mochitest/loop_fxa.sjs @@ -134,7 +134,7 @@ function params(request, response) { */ function oauth_authorization(request, response) { response.setStatusLine(request.httpVersion, 302, "Found"); - response.setHeader("Location", "browser_fxa_oauth.html"); + response.setHeader("Location", "browser_fxa_oauth_with_keys.html"); } /** diff --git a/browser/components/loop/test/xpcshell/test_loopservice_encryptionkey.js b/browser/components/loop/test/xpcshell/test_loopservice_encryptionkey.js index 1dca70125305..775b3cf47552 100644 --- a/browser/components/loop/test/xpcshell/test_loopservice_encryptionkey.js +++ b/browser/components/loop/test/xpcshell/test_loopservice_encryptionkey.js @@ -3,6 +3,7 @@ /* global Services, Assert */ const kGuestKeyPref = "loop.key"; +const kFxAKeyPref = "loop.key.fxa"; do_register_cleanup(function() { Services.prefs.clearUserPref(kGuestKeyPref); @@ -33,12 +34,25 @@ add_task(function* test_guestGetKey() { Assert.equal(key, kFakeKey, "should return existing key"); }); -add_task(function* test_fxaGetKey() { - // Set the userProfile to look like we're logged into FxA. +add_task(function* test_fxaGetKnownKey() { + const kFakeKey = "75312468"; + // Set the userProfile to look like we're logged into FxA with a key stored. MozLoopServiceInternal.fxAOAuthTokenData = { token_type: "bearer" }; MozLoopServiceInternal.fxAOAuthProfile = { email: "fake@invalid.com" }; + Services.prefs.setCharPref(kFxAKeyPref, kFakeKey); + + let key = yield MozLoopService.promiseProfileEncryptionKey(); + + Assert.equal(key, kFakeKey, "should return existing key"); +}); + +add_task(function* test_fxaGetKey() { + // Set the userProfile to look like we're logged into FxA without a key stored. + MozLoopServiceInternal.fxAOAuthTokenData = { token_type: "bearer" }; + MozLoopServiceInternal.fxAOAuthProfile = { email: "fake@invalid.com" }; + Services.prefs.clearUserPref(kFxAKeyPref); // Currently unimplemented, add a test when we implement the code. yield Assert.rejects(MozLoopService.promiseProfileEncryptionKey(), - /unimplemented/, "should reject as unimplemented"); + /not implemented/, "should reject as unimplemented"); });