Bug 623689: don't misbehave on master password cancel. r=philiKON

This commit is contained in:
Richard Newman 2011-01-10 21:51:29 -08:00
parent 3f4146ba5f
commit 218c03dd3e
5 changed files with 193 additions and 33 deletions

View File

@ -81,6 +81,9 @@ PARTIAL_DATA_SYNC: 60 * 1000, // 1 minute
// and the mobile sync interval.
HMAC_EVENT_INTERVAL: 600000,
// How long to wait between sync attempts if the Master Password is locked.
MASTER_PASSWORD_LOCKED_RETRY_INTERVAL: 15 * 60 * 1000, // 15 minutes
// 50 is hardcoded here because of URL length restrictions.
// (GUIDs can be up to 64 chars long)
MOBILE_BATCH_SIZE: 50,
@ -116,6 +119,7 @@ LOGIN_FAILED: "error.login.failed",
SYNC_FAILED_PARTIAL: "error.sync.failed_partial",
CLIENT_NOT_CONFIGURED: "service.client_not_configured",
STATUS_DISABLED: "service.disabled",
MASTER_PASSWORD_LOCKED: "service.master_password_locked",
// success states
LOGIN_SUCCEEDED: "success.login",
@ -161,6 +165,7 @@ JPAKE_ERROR_KEYMISMATCH: "jpake.error.keymismatch",
JPAKE_ERROR_WRONGMESSAGE: "jpake.error.wrongmessage",
// Ways that a sync can be disabled (messages only to be printed in debug log)
kSyncMasterPasswordLocked: "User elected to leave Master Password locked",
kSyncWeaveDisabled: "Weave is disabled",
kSyncNotLoggedIn: "User is not logged in",
kSyncNetworkOffline: "Network is offline",

View File

@ -83,7 +83,6 @@ function WeaveSvc() {
WeaveSvc.prototype = {
_lock: Utils.lock,
_catch: Utils.catch,
_locked: false,
_loggedIn: false,
@ -220,6 +219,20 @@ WeaveSvc.prototype = {
this._locked = false;
},
// A specialized variant of Utils.catch.
// This provides a more informative error message when we're already syncing:
// see Bug 616568.
_catch: function _catch(func) {
function lockExceptions(ex) {
if (Utils.isLockException(ex)) {
// This only happens if we're syncing already.
this._log.info("Cannot start sync: already syncing?");
}
}
return Utils.catch.call(this, func, lockExceptions);
},
_updateCachedURLs: function _updateCachedURLs() {
// Nothing to cache yet if we don't have the building blocks
if (this.clusterURL == "" || this.username == "")
@ -825,9 +838,24 @@ WeaveSvc.prototype = {
return false;
}
// Unlock master password, or return.
try {
// Fetch collection info on every startup.
// Attaching auth credentials to a request requires access to
// passwords, which means that Resource.get can throw MP-related
// exceptions!
// Try to fetch the passphrase first, while we still have control.
try {
this.passphrase;
} catch (ex) {
this._log.debug("Fetching passphrase threw " + ex +
"; assuming master password locked.");
Status.login = MASTER_PASSWORD_LOCKED;
return false;
}
let test = new Resource(this.infoURL).get();
switch (test.status) {
case 200:
// The user is authenticated.
@ -1065,6 +1093,11 @@ WeaveSvc.prototype = {
this._log.info("Logging in user " + this.username);
if (!this.verifyLogin()) {
if (Status.login == MASTER_PASSWORD_LOCKED) {
// Drat.
this._log.debug("Login failed: " + Status.login);
return false;
}
// verifyLogin sets the failure states here.
throw "Login failed: " + Status.login;
}
@ -1361,6 +1394,9 @@ WeaveSvc.prototype = {
reason = kSyncNetworkOffline;
else if (Status.minimumNextSync > Date.now())
reason = kSyncBackoffNotMet;
else if ((Status.login == MASTER_PASSWORD_LOCKED) &&
Utils.mpLocked())
reason = kSyncMasterPasswordLocked;
else if (!this._loggedIn)
reason = kSyncNotLoggedIn;
else if (Svc.Prefs.get("firstSync") == "notReady")
@ -1376,6 +1412,8 @@ WeaveSvc.prototype = {
* Remove any timers/observers that might trigger a sync
*/
_clearSyncTriggers: function _clearSyncTriggers() {
this._log.debug("Clearing sync triggers.");
// Clear out any scheduled syncs
if (this._syncTimer)
this._syncTimer.clear();
@ -1400,8 +1438,10 @@ WeaveSvc.prototype = {
// We're ready to sync even if we're not logged in... so long as the
// master password isn't locked.
if (Utils.mpLocked())
if (Utils.mpLocked()) {
ignore.push(kSyncNotLoggedIn);
ignore.push(kSyncMasterPasswordLocked);
}
let skip = this._checkSync(ignore);
this._log.trace("_checkSync returned \"" + skip + "\".");
@ -1425,9 +1465,19 @@ WeaveSvc.prototype = {
* delay is optional
*/
syncOnIdle: function WeaveSvc_syncOnIdle(delay) {
// No need to add a duplicate idle observer
// No need to add a duplicate idle observer, and no point if we got kicked
// out by the master password dialog.
if (Status.login == MASTER_PASSWORD_LOCKED &&
Utils.mpLocked()) {
this._log.debug("Not syncing on idle: Login status is " + Status.login);
// If we're not syncing now, we need to schedule the next one.
this._scheduleAtInterval(MASTER_PASSWORD_LOCKED_RETRY_INTERVAL);
return false;
}
if (this._idleTime)
return;
return false;
this._idleTime = delay || IDLE_TIME;
this._log.debug("Idle timer created for sync, will sync after " +
@ -1451,8 +1501,8 @@ WeaveSvc.prototype = {
// Start the sync right away if we're already late
if (interval <= 0) {
if (this.syncOnIdle())
this._log.debug("Syncing as soon as we're idle.");
this.syncOnIdle();
return;
}
@ -1536,6 +1586,22 @@ WeaveSvc.prototype = {
Utils.delay(function() this._doHeartbeat(), interval, this, "_heartbeatTimer");
},
/**
* Incorporates the backoff/retry logic used in error handling and elective
* non-syncing.
*/
_scheduleAtInterval: function _scheduleAtInterval(minimumInterval) {
const MINIMUM_BACKOFF_INTERVAL = 15 * 60 * 1000; // 15 minutes
let interval = this._calculateBackoff(this._syncErrors, MINIMUM_BACKOFF_INTERVAL);
if (minimumInterval)
interval = Math.max(minimumInterval, interval);
let d = new Date(Date.now() + interval);
this._log.config("Starting backoff, next sync at:" + d.toString());
this._scheduleNextSync(interval);
},
_syncErrors: 0,
/**
* Deal with sync errors appropriately
@ -1552,36 +1618,37 @@ WeaveSvc.prototype = {
Status.enforceBackoff = true;
}
const MINIMUM_BACKOFF_INTERVAL = 15 * 60 * 1000; // 15 minutes
let interval = this._calculateBackoff(this._syncErrors, MINIMUM_BACKOFF_INTERVAL);
this._scheduleNextSync(interval);
let d = new Date(Date.now() + interval);
this._log.config("Starting backoff, next sync at:" + d.toString());
this._scheduleAtInterval();
},
_skipScheduledRetry: function _skipScheduledRetry() {
return [LOGIN_FAILED_INVALID_PASSPHRASE,
LOGIN_FAILED_LOGIN_REJECTED].indexOf(Status.login) == -1;
},
// Call _lockedSync with a specialized variant of Utils.catch.
// This provides a more informative error message when we're already syncing:
// see Bug 616568.
sync: function sync() {
try {
this._log.debug("In wrapping sync().");
this._catch(function () {
// Make sure we're logged in.
if (this._shouldLogin()) {
this._log.trace("In sync: should login.");
this.login();
this._log.debug("In sync: should login.");
if (!this.login()) {
this._log.debug("Not syncing: login returned false.");
this._clearSyncTriggers(); // No more pending syncs, please.
// Try again later, just as if we threw an error... only without the
// error count.
if (!this._skipScheduledRetry())
this._scheduleAtInterval(MASTER_PASSWORD_LOCKED_RETRY_INTERVAL);
return;
}
}
else {
this._log.trace("In sync: no need to login.");
}
return this._lockedSync.apply(this, arguments);
} catch (ex) {
this._log.debug("Exception: " + Utils.exceptionStr(ex));
if (Utils.isLockException(ex)) {
// This only happens if we're syncing already.
this._log.info("Cannot start sync: already syncing?");
}
}
})();
},
/**

View File

@ -89,8 +89,11 @@ let Utils = {
*
* @usage MyObj._catch = Utils.catch;
* MyObj.foo = function() { this._catch(func)(); }
*
* Optionally pass a function which will be called if an
* exception occurs.
*/
catch: function Utils_catch(func) {
catch: function Utils_catch(func, exceptionCallback) {
let thisArg = this;
return function WrappedCatch() {
try {
@ -98,6 +101,10 @@ let Utils = {
}
catch(ex) {
thisArg._log.debug("Exception: " + Utils.exceptionStr(ex));
if (exceptionCallback) {
return exceptionCallback.call(thisArg, ex);
}
return null;
}
};
},
@ -125,7 +132,7 @@ let Utils = {
},
isLockException: function isLockException(ex) {
return ex && (ex.indexOf("Could not acquire lock.") == 0);
return ex && ex.indexOf && ex.indexOf("Could not acquire lock.") == 0;
},
/**

View File

@ -174,6 +174,43 @@ function run_test() {
// TODO: need better tests around master password prompting. See Bug 620583.
mpLocked = true;
// Testing exception handling if master password dialog is canceled.
// Do this by stubbing out Service.passphrase.
let oldPP = Service.__lookupGetter__("passphrase");
_("Old passphrase function is " + oldPP);
Service.__defineGetter__("passphrase",
function() {
throw "User canceled Master Password entry";
});
let oldClearSyncTriggers = Service._clearSyncTriggers;
let oldLockedSync = Service._lockedSync;
let cSTCalled = false;
let lockedSyncCalled = false;
Service._clearSyncTriggers = function() { cSTCalled = true; };
Service._lockedSync = function() { lockedSyncCalled = true; };
_("If master password is canceled, login fails and we report lockage.");
do_check_false(!!Service.login());
do_check_eq(Status.login, MASTER_PASSWORD_LOCKED);
do_check_eq(Status.service, LOGIN_FAILED);
_("Locked? " + Utils.mpLocked());
_("checkSync reports the correct term.");
do_check_eq(Service._checkSync(), kSyncMasterPasswordLocked);
_("Sync doesn't proceed and clears triggers if MP is still locked.");
Service.sync();
do_check_true(cSTCalled);
do_check_false(lockedSyncCalled);
// N.B., a bunch of methods are stubbed at this point. Be careful putting
// new tests after this point!
} finally {
Svc.Prefs.resetBranch("");
server.stop(do_test_finished);

View File

@ -1,13 +1,17 @@
_("Make sure catch when copied to an object will correctly catch stuff");
Cu.import("resource://services-sync/util.js");
Cu.import("resource://services-sync/service.js");
function run_test() {
let ret, rightThis, didCall, didThrow;
_("Make sure catch when copied to an object will correctly catch stuff");
let ret, rightThis, didCall, didThrow, wasTen, wasLocked;
let obj = {
catch: Utils.catch,
_log: {
debug: function(str) {
didThrow = str.search(/^Exception: /) == 0;
},
info: function(str) {
wasLocked = str.indexOf("Cannot start sync: already syncing?") == 0;
}
},
@ -21,22 +25,62 @@ function run_test() {
rightThis = this == obj;
didCall = true;
throw 10;
})(),
callbacky: function() this.catch(function() {
rightThis = this == obj;
didCall = true;
throw 10;
}, function(ex) {
wasTen = (ex == 10)
})(),
lockedy: function() this.catch(function() {
rightThis = this == obj;
didCall = true;
throw("Could not acquire lock.");
})()
};
_("Make sure a normal call will call and return");
rightThis = didCall = didThrow = false;
rightThis = didCall = didThrow = wasLocked = false;
ret = obj.func();
do_check_eq(ret, 5);
do_check_true(rightThis);
do_check_true(didCall);
do_check_false(didThrow);
do_check_eq(wasTen, undefined);
do_check_false(wasLocked);
_("Make sure catch/throw results in debug call and caller doesn't need to handle exception");
rightThis = didCall = didThrow = false;
rightThis = didCall = didThrow = wasLocked = false;
ret = obj.throwy();
do_check_eq(ret, undefined);
do_check_true(rightThis);
do_check_true(didCall);
do_check_true(didThrow);
do_check_eq(wasTen, undefined);
do_check_false(wasLocked);
_("Test callback for exception testing.");
rightThis = didCall = didThrow = wasLocked = false;
ret = obj.callbacky();
do_check_eq(ret, undefined);
do_check_true(rightThis);
do_check_true(didCall);
do_check_true(didThrow);
do_check_true(wasTen);
do_check_false(wasLocked);
_("Test the lock-aware catch that Service uses.");
obj.catch = Service._catch;
rightThis = didCall = didThrow = wasLocked = false;
wasTen = undefined;
ret = obj.lockedy();
do_check_eq(ret, undefined);
do_check_true(rightThis);
do_check_true(didCall);
do_check_true(didThrow);
do_check_eq(wasTen, undefined);
do_check_true(wasLocked);
}