Closed Bug 543784 Opened 15 years ago Closed 14 years ago

Login/sync even if the master password is locked

Categories

(Firefox :: Sync, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Future
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: Mardak, Assigned: rnewman)

References

Details

Attachments

(2 files, 1 obsolete file)

Some users only has weave credentials stored in the password manager, but they do use a master password. Perhaps we should force a prompt after some number of autologin attempts or when we know we should sync (threshold or timer).
Depends on: 524221
Please force a login on firefox startup.
Corner case -> Future.
Target Milestone: --- → Future
(In reply to comment #2) > Corner case -> Future. You must be kidding big time when you say using a master password is a corner case?! Could you explain your statement to prevent misunderstanding? TIA Max
Not kidding at all. The vast majority of users don't use it. Those that do typically have more than just weave credentials stored in the pwmgr, and thus get unlocked in finite time. The specific case outlined in comment 0 (only weave creds in pwmgr, but mp enabled) is very much a corner case.
Hi Mike, thanks for the clarification, I see your point and actually I wasn't aware that Weave automatically signs in as soon as I've unlocked the password DB entering the master password. But take a look at this use case: I have my laptop at work connected to the Weave service, also saving the open tabs. When I get home I boot up my private laptop and expect to get the open tabs from one hour ago when I left work. This forces me to manually click on "Sign In". BTW: "The vast majority of users don't use it." Is that proven? Cheers! Max
Proving absence is hard. We could integrate that into a future Test Pilot study to get harder numbers, though it's not a representative sample. That said, we can reason fairly well about it. When you start with the general stats around frequency of preference changes (at least at one time, 75% of users never opened the Options dialog, so you're already down to 1/4 of of users!) and then look at the relative hassle (adding more modal dialogs is generally rare) and even the most aggressive I can imagine is 5%.
(In reply to comment #4) > [...] Those that do > typically have more than just weave credentials stored in the pwmgr, and thus > get unlocked in finite time. [...]. Finite time yes, but in my case often a matter of days or weeks. I've about 40 passwords stored; however, the biggest part of sites I use on a regular basis uses cookies to keep me loged in, e.g. bugzilla. The ones which don't use cookies are mainly onlinestores which I don't visit frequently. In fact I always unlocked the password manually by login into Weave. Now with version 1.4 this has become a real pain.
Please note that there was a bug (bug 577608) which caused a race condition, with e.g. User Agent Switcher being installed. So, this might have improved/been fixed with 1.4.1b3. For what it's worth, I'm now using the following in my vimperatorrc, and this enables connecting+sync at login, and additionally syncing on browser exit (not sure about the latter though): autocmd VimperatorEnter .* emenu Tools.Sync.Connect autocmd VimperatorEnter .* emenu Tools.Sync.Sync Now autocmd VimperatorLeave .* emenu Tools.Sync.Sync Now
We're going to get rid of login-on-startup and just prompt when a sync is due. Depends on bug 590763
Assignee: nobody → mconnor
Depends on: 590763
No longer depends on: 524221
I am sure that during a session I have entered a master password only to find later on during that session that Sync still did not connect. This was with beta6 or earlier of Firefox and realize that Sync has since then been overhauled.
This should block Firefox 4.
blocking2.0: --- → ?
blocking2.0: ? → betaN+
I also have lots of passwords stored, but I can get around using them for up to a week. And I actually have seen running two Firefox installations un-synced for a week. For example, if you're using one installation just for browsing news pages, you never need a password. Up to 4.0b6, there was at least the 'Connect...' reminding me to log in, but now there's no UI left. Maybe its possible to (optionally) allow to store the sync password outside of the password manager, so syncing is possible without master password?
(In reply to comment #15) > Maybe its possible to (optionally) allow to store the sync password outside of > the password manager, so syncing is possible without master password? That's definitely not the answer here. We should just make Sync prompt for the master password, as this bug says.
blocking2.0: betaN+ → beta9+
Attached patch First draft. v1 (obsolete) (deleted) — Splinter Review
This patch causes the following behavior: * Prompts for MP during setup (of course -- we need to connect) * Does not prompt on startup * Autoconnect still tries to connect in the background (useful behavior: for non-MP users and unlocked MPs, this will detect version expiry earlier), but quietly backs off * Manual sync causes a password prompt * Canceling prompt causes a yellow "can't connect to the server" box. Do we want to prevent this? * Repeated such attempts (canceled prompt) cause repeated requests, which is desired. * Attempting to view sync key prompts for MP * Generating a new sync key bizarrely causes the immediately successive sync to fail with "not logged in"... still looking into this.
Assignee: mconnor → rnewman
Status: NEW → ASSIGNED
Also need to verify that timed syncs still work, of course. Still a WIP...
Attached patch First draft. v2 (deleted) — Splinter Review
This is altogether more thorough, and I tested the timed sync behavior. Tests and CrossWeave pass. Non-MP clients will see no noticeable change in behavior. MP clients will now be prompted for MP at first sync -- which is sometimes ten seconds after launching the browser (if a sync was already scheduled), but usually (for single-client test setups) much longer: Service.Main DEBUG Next sync in 83362 sec.
Attachment #497553 - Attachment is obsolete: true
Attachment #497836 - Flags: review?(mconnor)
Comment on attachment 497836 [details] [diff] [review] First draft. v2 Looking good! > case "weave:service:sync:error": > this._handleSyncError(); > if (Status.sync == CREDENTIALS_CHANGED) { > this.logout(); >- Utils.delay(function() this.login(), 0, this); > } > break; Please make sure this doesn't break UI code in any way (e.g. when one machine has changed the Sync Key or password and the other now wants to Sync.) r=me otherwise (taking over from mconnor who's out)
Attachment #497836 - Flags: review?(mconnor) → review+
Comment on attachment 497836 [details] [diff] [review] First draft. v2 Oh erm tests?
(In reply to comment #20) > Please make sure this doesn't break UI code in any way (e.g. when one machine > has changed the Sync Key or password and the other now wants to Sync.) Verified that two clients (one with MP enabled) will detect and display Wrong Sync Key after beginning a sync. The MP client needs you to enter your master password first, of course. On the subject of tests... indeed! I wanted to verify the approach and have any problems spotted before I extended the test suite. Can't write tests for problems I didn't spot.
Attached patch Rudimentary tests. v1 (deleted) — Splinter Review
Basic tests for: * Do we call login() at the start of a sync? * Do we consider the MP being locked when deciding whether to sync? See also Bug 614875, which tracks us reaching a state of not sucking in this area.
Attachment #498924 - Flags: review?(philipp)
(In reply to comment #23) > See also Bug 614875, which tracks us reaching a state of not sucking in this > area. ... by which I mean Bug 620583, of course. *sigh*
Comment on attachment 498924 [details] [diff] [review] Rudimentary tests. v1 >+ _("... and not if not."); >+ Service._checkSyncStatus(); >+ do_check_false(scheduleCalled); >+ Service._scheduleNextSync = scheduleNextSyncF; This confused me for a moment, but it's because we're not logged in yet. Can you mention that in the comment? r=me with that
Attachment #498924 - Flags: review?(philipp) → review+
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if you believe it MUST be in the next beta (ie: trunk is in an unshippable state without this)
No longer blocks: 621194
blocking2.0: beta9+ → betaN+
No longer depends on: 590763
Undoing some damage from LegNeato's script.
Blocks: 621194
Blocks: 590763
No longer depends on: 590763
I'm seeing that Sync requests the master password every time it tries to sync if a master password is set. I'm not sure whether to file a new bug or if there's a regression on this?
(In reply to comment #31) > I'm seeing that Sync requests the master password every time it tries to > sync if a master password is set. I'm not sure whether to file a new bug or > if there's a regression on this? Always file a new bug please. Thanks.
Component: Firefox Sync: UI → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: