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

VERIFIED FIXED in 0.7

Status

Cloud Services
Firefox Sync: Backend
P1
normal
VERIFIED FIXED
9 years ago
5 years ago

People

(Reporter: shaver, Assigned: Mardak)

Tracking

unspecified
x86
Windows NT
Points:
---
Dependency tree / graph
Bug Flags:
blocking-weave1.0 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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!
Flags: blocking-weave1.0?

Updated

9 years ago
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 482906
(Assignee)

Comment 2

9 years ago
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.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

Comment 3

9 years ago
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).
(Assignee)

Comment 4

9 years ago
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.
Target Milestone: --- → 0.7
(Assignee)

Comment 5

9 years ago
Created attachment 397419 [details] [diff] [review]
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.
Assignee: nobody → edilee
Status: REOPENED → ASSIGNED
Attachment #397419 - Flags: review?(thunder)
(Assignee)

Updated

9 years ago
Blocks: 506790, 506792

Updated

9 years ago
Priority: -- → P1
(Assignee)

Updated

9 years ago
Flags: blocking-weave1.0? → blocking-weave1.0+
(Assignee)

Updated

9 years ago
Blocks: 482906

Updated

9 years ago
Priority: P1 → P2

Updated

9 years ago
Priority: P2 → P1

Updated

9 years ago
Duplicate of this bug: 514358
(Assignee)

Updated

9 years ago
Blocks: 514499

Comment 7

9 years ago
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.
Attachment #397419 - Flags: review?(thunder) → review+
(Assignee)

Comment 8

9 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Blocks: 514609
(Assignee)

Updated

9 years ago
Duplicate of this bug: 518512

Updated

8 years ago
Status: RESOLVED → VERIFIED
Flags: in-litmus?
removing in-litmus flag, it no longer exists
Flags: in-litmus?
You need to log in before you can comment on or make changes to this bug.