Bug 1013064 (part 5) - stop disabling the password engine when MP enabled. r=ttaubert

From 07aa9cc1fcd5479976effe29f6adf5ad5ba7a8f8 Mon Sep 17 00:00:00 2001
This commit is contained in:
Mark Hammond 2014-06-12 18:19:49 +10:00
parent d59b09a390
commit 6a1e989c2d
14 changed files with 7 additions and 275 deletions

View File

@ -4,20 +4,6 @@
"use strict";
const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
Cu.import("resource://gre/modules/Services.jsm");
let service = Cc["@mozilla.org/weave/service;1"]
.getService(Ci.nsISupports)
.wrappedJSObject;
if (!service.allowPasswordsEngine) {
let checkbox = document.getElementById("fxa-pweng-chk");
checkbox.checked = false;
checkbox.disabled = true;
}
addEventListener("dialogaccept", function () {
let pane = document.getElementById("sync-customize-pane");
pane.writePreferences(true);

View File

@ -45,8 +45,7 @@
<checkbox label="&engine.bookmarks.label;"
accesskey="&engine.bookmarks.accesskey;"
preference="engine.bookmarks"/>
<checkbox id="fxa-pweng-chk"
label="&engine.passwords.label;"
<checkbox label="&engine.passwords.label;"
accesskey="&engine.passwords.accesskey;"
preference="engine.passwords"/>
<checkbox label="&engine.history.label;"

View File

@ -87,12 +87,6 @@ let gSyncUtils = {
this._openLink(Weave.Svc.Prefs.get(root + "privacyURL"));
},
openMPInfoPage: function (event) {
event.stopPropagation();
let baseURL = Services.urlFormatter.formatURLPref("app.support.baseURL");
this._openLink(baseURL + "sync-master-password");
},
openFirstSyncProgressPage: function () {
this._openLink("about:sync-progress");
},

View File

@ -154,17 +154,6 @@ let gSyncPane = {
for (let checkbox of engines.querySelectorAll("checkbox")) {
checkbox.disabled = enginesListDisabled;
}
let checkbox = document.getElementById("fxa-pweng-chk");
let help = document.getElementById("fxa-pweng-help");
let allowPasswordsEngine = service.allowPasswordsEngine;
if (!allowPasswordsEngine) {
checkbox.checked = false;
}
checkbox.disabled = !allowPasswordsEngine || enginesListDisabled;
help.hidden = allowPasswordsEngine || enginesListDisabled;
});
// If fxAccountEnabled is false and we are in a "not configured" state,
// then fxAccounts is probably fully disabled rather than just unconfigured,

View File

@ -276,20 +276,9 @@
<checkbox label="&engine.bookmarks.label;"
accesskey="&engine.bookmarks.accesskey;"
preference="engine.bookmarks"/>
<hbox>
<checkbox id="fxa-pweng-chk"
label="&engine.passwords.label;"
accesskey="&engine.passwords.accesskey;"
preference="engine.passwords"/>
<vbox id="fxa-pweng-help">
<spacer flex="1"/>
<hbox id="fxa-pweng-help-link">
<image onclick="gSyncUtils.openMPInfoPage(event);" />
</hbox>
<spacer flex="1"/>
</vbox>
</hbox>
<checkbox label="&engine.passwords.label;"
accesskey="&engine.passwords.accesskey;"
preference="engine.passwords"/>
<checkbox label="&engine.history.label;"
accesskey="&engine.history.accesskey;"
preference="engine.history"/>

View File

@ -154,17 +154,6 @@ let gSyncPane = {
for (let checkbox of engines.querySelectorAll("checkbox")) {
checkbox.disabled = enginesListDisabled;
}
let checkbox = document.getElementById("fxa-pweng-chk");
let help = document.getElementById("fxa-pweng-help");
let allowPasswordsEngine = service.allowPasswordsEngine;
if (!allowPasswordsEngine) {
checkbox.checked = false;
}
checkbox.disabled = !allowPasswordsEngine || enginesListDisabled;
help.hidden = allowPasswordsEngine || enginesListDisabled;
});
// If fxAccountEnabled is false and we are in a "not configured" state,
// then fxAccounts is probably fully disabled rather than just unconfigured,

View File

@ -265,20 +265,9 @@
<checkbox label="&engine.bookmarks.label;"
accesskey="&engine.bookmarks.accesskey;"
preference="engine.bookmarks"/>
<hbox>
<checkbox id="fxa-pweng-chk"
label="&engine.passwords.label;"
accesskey="&engine.passwords.accesskey;"
preference="engine.passwords"/>
<vbox id="fxa-pweng-help">
<spacer flex="1"/>
<hbox id="fxa-pweng-help-link">
<image onclick="gSyncUtils.openMPInfoPage(event);" />
</hbox>
<spacer flex="1"/>
</vbox>
</hbox>
<checkbox label="&engine.passwords.label;"
accesskey="&engine.passwords.accesskey;"
preference="engine.passwords"/>
<checkbox label="&engine.history.label;"
accesskey="&engine.history.accesskey;"
preference="engine.history"/>

View File

@ -171,12 +171,4 @@ label.small {
margin-bottom: 0.6em;
}
#fxa-pweng-help-link > label {
margin: 0;
}
#fxa-pweng-help-link > image {
list-style-image: url("chrome://global/skin/icons/question-16.png");
}
%endif

View File

@ -233,20 +233,4 @@ html|a.inline-link:-moz-focusring {
margin-bottom: 0.6em;
}
#fxa-pweng-help-link > label {
margin: 0;
}
#fxa-pweng-help-link > image {
width: 16px;
height: 16px;
list-style-image: url("chrome://global/skin/icons/question-16.png");
}
@media (min-resolution: 2dppx) {
#fxa-pweng-help-link > image {
list-style-image: url("chrome://global/skin/icons/question-32.png");
}
}
%endif

View File

@ -161,12 +161,4 @@ label.small {
margin-bottom: 0.6em;
}
#fxa-pweng-help-link > label {
margin: 0;
}
#fxa-pweng-help-link > image {
list-style-image: url("chrome://global/skin/icons/question-16.png");
}
%endif

View File

@ -108,16 +108,6 @@ WeaveService.prototype = {
}
},
/**
* Returns whether the password engine is allowed. We explicitly disallow
* the password engine when a master password is used to ensure those can't
* be accessed without the master key.
*/
get allowPasswordsEngine() {
// This doesn't apply to old-style sync, it's only an issue for FxA.
return !this.fxAccountsEnabled || !Utils.mpEnabled();
},
/**
* Whether Sync appears to be enabled.
*

View File

@ -36,28 +36,6 @@ PasswordEngine.prototype = {
_recordObj: LoginRec,
applyIncomingBatchSize: PASSWORDS_STORE_BATCH_SIZE,
get isAllowed() {
return Cc["@mozilla.org/weave/service;1"]
.getService(Ci.nsISupports)
.wrappedJSObject
.allowPasswordsEngine;
},
get enabled() {
// If we are disabled due to !isAllowed(), we must take care to ensure the
// engine has actually had the enabled setter called which reflects this state.
let prefVal = SyncEngine.prototype.__lookupGetter__("enabled").call(this);
let newVal = this.isAllowed && prefVal;
if (newVal != prefVal) {
this.enabled = newVal;
}
return newVal;
},
set enabled(val) {
SyncEngine.prototype.__lookupSetter__("enabled").call(this, this.isAllowed && val);
},
_syncFinish: function _syncFinish() {
SyncEngine.prototype._syncFinish.call(this);

View File

@ -1,137 +0,0 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
Cu.import("resource://gre/modules/Log.jsm");
Cu.import("resource://services-sync/stages/enginesync.js");
Cu.import("resource://services-sync/util.js");
Cu.import("resource://services-sync/engines/passwords.js");
Cu.import("resource://services-sync/service.js");
Cu.import("resource://testing-common/services/sync/utils.js");
function run_test() {
initTestLogging("Trace");
run_next_test();
}
add_test(function test_simple() {
ensureLegacyIdentityManager();
// Stub fxAccountsEnabled
let xpcs = Cc["@mozilla.org/weave/service;1"]
.getService(Components.interfaces.nsISupports)
.wrappedJSObject;
let fxaEnabledGetter = xpcs.__lookupGetter__("fxAccountsEnabled");
xpcs.__defineGetter__("fxAccountsEnabled", () => true);
// Stub mpEnabled.
let mpEnabledF = Utils.mpEnabled;
let mpEnabled = false;
Utils.mpEnabled = function() mpEnabled;
let manager = Service.engineManager;
Service.engineManager.register(PasswordEngine);
let engine = Service.engineManager.get("passwords");
let wipeCount = 0;
let engineWipeServerF = engine.wipeServer;
engine.wipeServer = function() {
++wipeCount;
}
// A server for the metadata.
let server = new SyncServer();
let johndoe = server.registerUser("johndoe", "password");
johndoe.createContents({
meta: {global: {engines: {passwords: {version: engine.version,
syncID: engine.syncID}}}},
crypto: {},
clients: {}
});
server.start();
setBasicCredentials("johndoe", "password", "abcdeabcdeabcdeabcdeabcdea");
Service.serverURL = server.baseURI;
Service.clusterURL = server.baseURI;
let engineSync = new EngineSynchronizer(Service);
engineSync._log.level = Log.Level.Trace;
function assertEnabled(expected, message) {
Assert.strictEqual(engine.enabled, expected, message);
// The preference *must* reflect the actual state.
Assert.strictEqual(Svc.Prefs.get("engine." + engine.prefName), expected,
message + " (pref should match enabled state)");
}
try {
assertEnabled(true, "password engine should be enabled by default")
let engineMeta = Service.recordManager.get(engine.metaURL);
// This engine should be in the meta/global
Assert.notStrictEqual(engineMeta.payload.engines[engine.name], undefined,
"The engine should appear in the metadata");
Assert.ok(!engineMeta.changed, "the metadata for the password engine hasn't changed");
// (pretend to) enable a master-password
mpEnabled = true;
// The password engine should be locally disabled...
assertEnabled(false, "if mp is locked the engine should be disabled");
// ...but not declined.
Assert.ok(!manager.isDeclined("passwords"), "password engine is not declined");
// Next time a sync would happen, we call _updateEnabledEngines(), which
// would remove the engine from the metadata - call that now.
engineSync._updateEnabledEngines();
// The global meta should no longer list the engine.
engineMeta = Service.recordManager.get(engine.metaURL);
Assert.strictEqual(engineMeta.payload.engines[engine.name], undefined,
"The engine should have vanished");
// And we should have wiped the server data.
Assert.strictEqual(wipeCount, 1, "wipeServer should have been called");
// Now simulate an incoming meta/global indicating the engine should be
// enabled. We should fail to actually enable it - the pref should remain
// false and we wipe the server for anything another device might have
// stored.
let meta = {
payload: {
engines: {
"passwords": {"version":1,"syncID":"yfBi2v7PpFO2"},
},
},
};
engineSync._updateEnabledFromMeta(meta, 3, manager);
Assert.strictEqual(wipeCount, 2, "wipeServer should have been called");
Assert.ok(!manager.isDeclined("passwords"), "password engine is not declined");
assertEnabled(false, "engine still not enabled locally");
// Let's turn the MP off - but *not* re-enable it locally.
mpEnabled = false;
// Just disabling the MP isn't enough to force it back to enabled.
assertEnabled(false, "engine still not enabled locally");
// Another incoming metadata record with the engine enabled should cause
// it to be enabled locally.
meta = {
payload: {
engines: {
"passwords": 1,
},
},
};
engineSync._updateEnabledFromMeta(meta, 3, manager);
Assert.strictEqual(wipeCount, 2, "wipeServer should *not* have been called again");
Assert.ok(!manager.isDeclined("passwords"), "password engine is not declined");
// It should be enabled locally.
assertEnabled(true, "engine now enabled locally");
// Next time a sync starts it should magically re-appear in our meta/global
engine._syncStartup();
//engineSync._updateEnabledEngines();
engineMeta = Service.recordManager.get(engine.metaURL);
Assert.equal(engineMeta.payload.engines[engine.name].version, engine.version,
"The engine should re-appear in the metadata");
} finally {
// restore the damage we did above...
engine.wipeServer = engineWipeServerF;
engine._store.wipe();
// Un-stub mpEnabled and fxAccountsEnabled
Utils.mpEnabled = mpEnabledF;
xpcs.__defineGetter__("fxAccountsEnabled", fxaEnabledGetter);
server.stop(run_next_test);
}
});

View File

@ -169,5 +169,3 @@ skip-if = debug
[test_healthreport.js]
skip-if = ! healthreport
[test_password_mpenabled.js]