From 375238940ea88380beb1dad977623dfaf7df275e Mon Sep 17 00:00:00 2001 From: Philipp von Weitershausen Date: Tue, 5 Oct 2010 20:32:56 +0200 Subject: [PATCH] Bug 601973 - SyncEngine._testDecrypt() yields wrong result [r=mconnor] Fix a 'this' scoping error in SyncEngine._testDecrypt(). Rename this method to canDecrypt() since it's clearly public API. Provide tests for SyncEngine.canDecrypt() as well as Service.wipeClient(). --- services/sync/modules/engines.js | 5 +- services/sync/modules/service.js | 2 +- .../tests/unit/test_service_wipeClient.js | 64 +++++++++++++++++++ .../sync/tests/unit/test_syncengine_sync.js | 64 +++++++++++++++++++ 4 files changed, 132 insertions(+), 3 deletions(-) create mode 100644 services/sync/tests/unit/test_service_wipeClient.js diff --git a/services/sync/modules/engines.js b/services/sync/modules/engines.js index 0ed525d46e4..9ac3061c0b6 100644 --- a/services/sync/modules/engines.js +++ b/services/sync/modules/engines.js @@ -759,7 +759,7 @@ SyncEngine.prototype = { } }, - _testDecrypt: function _testDecrypt() { + canDecrypt: function canDecrypt() { // Report failure even if there's nothing to decrypt let canDecrypt = false; @@ -768,8 +768,9 @@ SyncEngine.prototype = { test.limit = 1; test.sort = "newest"; test.full = true; + let self = this; test.recordHandler = function(record) { - record.decrypt(ID.get("WeaveCryptoID"), this.cryptoMetaURL); + record.decrypt(ID.get("WeaveCryptoID"), self.cryptoMetaURL); canDecrypt = true; }; diff --git a/services/sync/modules/service.js b/services/sync/modules/service.js index 361ebd9e379..cc0a0ea3ee0 100644 --- a/services/sync/modules/service.js +++ b/services/sync/modules/service.js @@ -1681,7 +1681,7 @@ WeaveSvc.prototype = { // Fully wipe each engine if it's able to decrypt data for each (let engine in engines) - if (engine._testDecrypt()) + if (engine.canDecrypt()) engine.wipeClient(); // Save the password/passphrase just in-case they aren't restored by sync diff --git a/services/sync/tests/unit/test_service_wipeClient.js b/services/sync/tests/unit/test_service_wipeClient.js new file mode 100644 index 00000000000..0859f380ebf --- /dev/null +++ b/services/sync/tests/unit/test_service_wipeClient.js @@ -0,0 +1,64 @@ +Cu.import("resource://services-sync/service.js"); +Cu.import("resource://services-sync/engines.js"); + + +function CanDecryptEngine() { + SyncEngine.call(this, "CanDecrypt"); +} +CanDecryptEngine.prototype = { + __proto__: SyncEngine.prototype, + + // Override these methods with mocks for the test + canDecrypt: function canDecrypt() { + return true; + }, + + wasWiped: false, + wipeClient: function wipeClient() { + this.wasWiped = true; + } +}; +Engines.register(CanDecryptEngine); + + +function CannotDecryptEngine() { + SyncEngine.call(this, "CannotDecrypt"); +} +CannotDecryptEngine.prototype = { + __proto__: SyncEngine.prototype, + + // Override these methods with mocks for the test + canDecrypt: function canDecrypt() { + return false; + }, + + wasWiped: false, + wipeClient: function wipeClient() { + this.wasWiped = true; + } +}; +Engines.register(CannotDecryptEngine); + + +function test_withEngineList() { + try { + _("Ensure initial scenario."); + do_check_false(Engines.get("candecrypt").wasWiped); + do_check_false(Engines.get("cannotdecrypt").wasWiped); + + _("Wipe local engine data."); + Service.wipeClient(["candecrypt", "cannotdecrypt"]); + + _("Ensure only the engine that can decrypt was wiped."); + do_check_true(Engines.get("candecrypt").wasWiped); + do_check_false(Engines.get("cannotdecrypt").wasWiped); + } finally { + Engines.get("candecrypt").wasWiped = false; + Engines.get("cannotdecrypt").wasWiped = false; + Service.startOver(); + } +} + +function run_test() { + test_withEngineList(); +} diff --git a/services/sync/tests/unit/test_syncengine_sync.js b/services/sync/tests/unit/test_syncengine_sync.js index 29327b3fb10..257ddee3772 100644 --- a/services/sync/tests/unit/test_syncengine_sync.js +++ b/services/sync/tests/unit/test_syncengine_sync.js @@ -1087,6 +1087,68 @@ function test_syncFinish_deleteLotsInBatches() { } } +function test_canDecrypt_noCryptoMeta() { + _("SyncEngine.canDecrypt returns false if the engine fails to decrypt items on the server, e.g. due to a missing crypto key."); + Svc.Prefs.set("clusterURL", "http://localhost:8080/"); + Svc.Prefs.set("username", "foo"); + + let collection = new ServerCollection(); + collection.wbos.flying = new ServerWBO( + 'flying', encryptPayload({id: 'flying', + denomination: "LNER Class A3 4472"})); + + let server = sync_httpd_setup({ + "/1.0/foo/storage/steam": collection.handler() + }); + do_test_pending(); + createAndUploadKeypair(); + + let engine = makeSteamEngine(); + try { + + do_check_false(engine.canDecrypt()); + + } finally { + server.stop(do_test_finished); + Svc.Prefs.resetBranch(""); + Records.clearCache(); + syncTesting = new SyncTestingInfrastructure(makeSteamEngine); + } +} + +function test_canDecrypt_true() { + _("SyncEngine.canDecrypt returns true if the engine can decrypt the items on the server."); + Svc.Prefs.set("clusterURL", "http://localhost:8080/"); + Svc.Prefs.set("username", "foo"); + + let crypto_steam = new ServerWBO('steam'); + let collection = new ServerCollection(); + collection.wbos.flying = new ServerWBO( + 'flying', encryptPayload({id: 'flying', + denomination: "LNER Class A3 4472"})); + + let server = sync_httpd_setup({ + "/1.0/foo/storage/crypto/steam": crypto_steam.handler(), + "/1.0/foo/storage/steam": collection.handler() + }); + do_test_pending(); + createAndUploadKeypair(); + createAndUploadSymKey("http://localhost:8080/1.0/foo/storage/crypto/steam"); + + let engine = makeSteamEngine(); + try { + + do_check_true(engine.canDecrypt()); + + } finally { + server.stop(do_test_finished); + Svc.Prefs.resetBranch(""); + Records.clearCache(); + syncTesting = new SyncTestingInfrastructure(makeSteamEngine); + } +} + + function run_test() { test_syncStartup_emptyOrOutdatedGlobalsResetsSync(); test_syncStartup_metaGet404(); @@ -1104,4 +1166,6 @@ function run_test() { test_syncFinish_noDelete(); test_syncFinish_deleteByIds(); test_syncFinish_deleteLotsInBatches(); + test_canDecrypt_noCryptoMeta(); + test_canDecrypt_true(); }