513191 - cloud -> client sync wipes auth cache, including weave credentials

Status: VERIFIED FIXED

(Firefox :: Sync, defect, P1)

Description

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

If you pick a full client wipe, it seems to lose the weave credentials just input by the user, and then gets a 401 trying to get data from the server. Points for thoroughness, though!

Comment 2

[Ed Lee :Mardak]

Not quite a dupe. Bug 482906 is about restoring passwords if sync didn't re-create them after syncing passwords (i.e., there was no weave password encrypted in the cloud).

I've been able to wipe local and sync from cloud successfully before, but only recently has it started to cause 401s, so it's not even able to GET/PUT data on the server to even start syncing.

Comment 3

[Dan Mills [:thunder]]

The same solution might apply to both, though: just skip our two chrome://weave entries when wiping, or re-add them manually right after wiping (unless 'save password' pref is off).

Comment 4

[Ed Lee :Mardak]

There seems to be some issues with about:weave login compared to login.xul. I reverted to an earlier version with both available, and login.xul wipe local works.

Components.utils.import("resource://weave/service.js"); Weave.Service.wipeClient(Weave.Engines.getEnabled().map(function(i) i.name)); Weave.Utils.openStatus();

I believe logging in and restarting -> autologin, wipe local works. I'll keep debugging.

Comment 5

[Ed Lee :Mardak]

Attachment: v1

The problem was that we were doing a login then doing Weave.Service.password = .. which is a smart setter that clears the temp password. By the time we get to sync after a local wipe, the cached temp password is gone, and the persisted login is also gone.

It was working before in the old login dialog because we persisted the login first (extensions.weave.rememberpassword) then did the login that loaded the password into the cache, and it stayed around even after a local wipe.

Comment 7

[Dan Mills [:thunder]]

Comment on attachment 397419 [details] [diff] [review]
v1

>+const WEAVE_HOST = "chrome://weave";

Put this in constants.js.in, and change the name to make it clearer what it's for?

Otherwise, good patch, should remove a bunch of corner cases/odd behavior.

One thing it doesn't handle is:

1) I have a username/password/passphrase saved.
2) I manually attempt a login, and type new values.
3) login will still set those values, although it will not persist them.  verifyPassword/Passphrase don't take any args, so you'd have to set those too.
4) If login fails, the wrong stuff will be in memory (but not persisted).
5) Restarting Firefox would bring the persisted values back.

We could work around the above scenario, although I'm not sure how bad it really is.  Might want to file a follow-up bug for it.

Comment 8

[Ed Lee :Mardak]

http://hg.mozilla.org/labs/weave/rev/ad259e13e4c1
Don't aggressively persist the password with smart (dumb?) setters that clear out the temp password; and provide a separate function to persist the login. This effectively makes setting password/passphrase always temporary until persisted, which will check if the value is different from the one already stored on disk. A number of verify/cluster functions are privitized to not need to take user/pass/passph as arguments so that the default authenticator will work, and verifyPassphrase will use the stored passphrase to correctly handle auto-login.

Comment 10

[Tracy Walker [:tracy]]

removing in-litmus flag, it no longer exists