firefox asks more than once for stored passwords on startup

Bug #398128 reported by Joan Tur
46
This bug affects 8 people
Affects Status Importance Assigned to Milestone
Mozilla Firefox
Fix Released
Medium
firefox (Ubuntu)
Fix Released
Medium
Unassigned
firefox-3.0 (Ubuntu)
Fix Released
Medium
Unassigned
firefox-3.5 (Ubuntu)
Fix Released
Medium
Unassigned

Bug Description

Binary package hint: firefox-3.0

I've set firefox to start up with the tabs that were open when closed. After starting firefox again, the master password is asked as many times as tabs requiring stored passwords.

In my opinion, it should ask the master password only once, like kwalletmanager does when opening konqueror.

Thanks!

Data applied:

Description: Ubuntu 8.10
Release: 8.10

firefox:
  Instal·lat: 3.0.11+build2+nobinonly-0ubuntu0.8.10.1
  Candidat: 3.0.11+build2+nobinonly-0ubuntu0.8.10.1
  Taula de versió:
 *** 3.0.11+build2+nobinonly-0ubuntu0.8.10.1 0
        500 http://ftp.gui.uva.es intrepid-updates/main Packages
        500 http://ftp.gui.uva.es intrepid-security/main Packages
        100 /var/lib/dpkg/status
     3.0.3+nobinonly-0ubuntu2 0
        500 http://ftp.gui.uva.es intrepid/main Packages

Expected to happen: it should ask for the master password only once; now it asks for the master password as many times as opened tabs requiring a stored password.

ProblemType: Bug
Architecture: amd64
DistroRelease: Ubuntu 8.10
NonfreeKernelModules: fglrx
Package: firefox-3.0 3.0.11+build2+nobinonly-0ubuntu0.8.10.1
ProcEnviron:
 PATH=/home/username/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
 LANG=ca_ES.UTF-8
 SHELL=/bin/bash
SourcePackage: firefox-3.0
Uname: Linux 2.6.28-11-generic x86_64

Tags: apport-bug
Revision history for this message
In , Dolske (dolske) wrote :

*** Bug 488637 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Matti-mversen (matti-mversen) wrote :

isn't this a dupe of bug 356097 ?

Revision history for this message
In , Dolske (dolske) wrote :

No. Bug 356097 is about proxy auth, and will be fixed by bug 475053.

Revision history for this message
Joan Tur (joantur) wrote :

Binary package hint: firefox-3.0

I've set firefox to start up with the tabs that were open when closed. After starting firefox again, the master password is asked as many times as tabs requiring stored passwords.

In my opinion, it should ask the master password only once, like kwalletmanager does when opening konqueror.

Thanks!

Data applied:

Description: Ubuntu 8.10
Release: 8.10

firefox:
  Instal·lat: 3.0.11+build2+nobinonly-0ubuntu0.8.10.1
  Candidat: 3.0.11+build2+nobinonly-0ubuntu0.8.10.1
  Taula de versió:
 *** 3.0.11+build2+nobinonly-0ubuntu0.8.10.1 0
        500 http://ftp.gui.uva.es intrepid-updates/main Packages
        500 http://ftp.gui.uva.es intrepid-security/main Packages
        100 /var/lib/dpkg/status
     3.0.3+nobinonly-0ubuntu2 0
        500 http://ftp.gui.uva.es intrepid/main Packages

Expected to happen: it should ask for the master password only once; now it asks for the master password as many times as opened tabs requiring a stored password.

ProblemType: Bug
Architecture: amd64
DistroRelease: Ubuntu 8.10
NonfreeKernelModules: fglrx
Package: firefox-3.0 3.0.11+build2+nobinonly-0ubuntu0.8.10.1
ProcEnviron:
 PATH=/home/username/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
 LANG=ca_ES.UTF-8
 SHELL=/bin/bash
SourcePackage: firefox-3.0
Uname: Linux 2.6.28-11-generic x86_64

Revision history for this message
Joan Tur (joantur) wrote :
Revision history for this message
In , Mozilla-bugs-micahscomputing (mozilla-bugs-micahscomputing) wrote :

Ubuntu Bug:
https://bugs.launchpad.net/bugs/398128

User was seeing this in 3.0.11 and I'm seeing this in 3.5.

Revision history for this message
Micah Gersten (micahg) wrote :

Thank you for reporting this to Ubuntu. I can confirm this behavior.

Changed in firefox-3.0 (Ubuntu):
importance: Undecided → Medium
status: New → Confirmed
Changed in firefox-3.5 (Ubuntu):
importance: Undecided → Medium
Revision history for this message
Micah Gersten (micahg) wrote :

Thank you for your bug report. This bug has been reported to the developers of the software. You can track it and make comments at: https://bugzilla.mozilla.org/show_bug.cgi?id=499233

Changed in firefox-3.0 (Ubuntu):
status: Confirmed → Triaged
Changed in firefox-3.5 (Ubuntu):
status: New → Triaged
Changed in firefox:
status: Unknown → Confirmed
Revision history for this message
In , Randy-steer (randy-steer) wrote :

THIS is what I've been voting for in multiple other bugs (from 3.0 to 3.5). I don't understand why it's not all considered one bug. A tab/process/thread opens a URL and -- for whatever reason -- a login is required and a call is made to the master password dialog. For future maintenance alone, I would think that all of those circumstances would be using the same call and subroutines for master password. Can't the call for master password check, and if necessary set, a flag, and then put each process/thread in a queue until the MP is entered?

Revision history for this message
In , Jonny (dioni21) wrote :

Bug 369963 is marked as solved, but indeed is a duplicated of this, and therefore, unsolved...

It is a very old and annoying bug for those who keep many open tabs between browser restarts.

Revision history for this message
In , Rnicoletto (rnicoletto) wrote :

(In reply to comment #6)
> Bug 369963 is marked as solved, but indeed is a duplicated of this, and
> therefore, unsolved...
>
> It is a very old and annoying bug for those who keep many open tabs between
> browser restarts.

bug 475053 has been resolved and landed on trunk. If you're experiencing this bug try reproducing with a clean profile on latest trunk version (http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/).

Revision history for this message
In , Jasonb-dante (jasonb-dante) wrote :

Bug 475053 is not the same thing and it did not fix this. (Already tested with nightlies after it was checked in.)

Revision history for this message
In , Askmike2009 (askmike2009) wrote :

Same problem (I think it was not the case in Firefox 2): I have several tabs opening at startup with identification sessions for which my credentials are saved. It triggers a corresponding number of Master Password requests instead of one request for all.

Reproducible: Always

Steps to Reproduce:
1. Have as startup page with two (or more) tabs with identification sessions (for instance your email account and another account)
2. Have the Master Password remember your credentials for the said tabs
3. Restart Firefox

Actual Results:
You will have two (or more) prompts for Master Password; filling in the first prompt will not make the other prompts disappear.

Expected Results:
Have one prompt for all.

Uneducated guess:
Recognition of a website with credentials saved in Master Password should not trigger immediate prompt for Master Password but a CHECK for existence of untreated (i.e. not filled in correctly and not canceled) Master Password prompt.

Revision history for this message
In , Bugzilla (bugzilla) wrote :

I can confirm this. It annoys me every day when I start up Firefox. It's been the case for all of Firefox 3 and Firefox 2 as well, for as long as I can remember.

Entering the master password correctly on each individual prompt causes the login credentials for the respective tab (whichever it is) to fill in, and for those you cancel, they are not, so it's obviously every process for itself.

I don't know how it's implemented at the moment, but maybe it should be set up in a callback format, a little like AJAX requests. Password manager remembers whether a prompt is open, and while there's no response from the user, it queues the requests and finally calls the callback functions for each tab that was asking for credentials.

Revision history for this message
In , Nickolay Ponomarev (asqueella) wrote :

*** Bug 490331 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Paul-oshannessy (paul-oshannessy) wrote :

*** Bug 516051 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Dolske (dolske) wrote :

Created an attachment (id=400653)
Patch v.1 (WIP)

Somewhat rough work-in-progress. Still need to handle a few edge cases, clean things up, and add tests. But the basic concept works with this.

Revision history for this message
In , Dolske (dolske) wrote :

*** Bug 437557 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Dolske (dolske) wrote :

Created an attachment (id=403604)
Patch v.2 (WIP)

Cleaned up, still a few todos and needs tests, but I'll toss it to zpao for a prereview.

Revision history for this message
In , Paul-oshannessy (paul-oshannessy) wrote :

(From update of attachment 403604)
>+ // XXX Can we detect if this document has died in the time since
>+ // we deferted it?

My question too. You can write a test for it :) (or at least make sure we have a litmus test)

>+ // XXX do we need be able to defer this too?

Presumably yes? You know the code better, but if we haven't decrypted anything for the form yet, I would think this would have to be deferred.

All-in-all it looks good. You know the parts you need to finish. My only concern (and one you say you know the answer to) is how this affects 3rd party consumers. If I'm understanding this right, it shouldn't actually affect them at all since they mostly aren't trying to fill forms.

Revision history for this message
In , Paul-oshannessy (paul-oshannessy) wrote :

*** Bug 522228 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Tomlevels (tomlevels) wrote :

I used to have an extension which solved this problem, but since a few days it does not work anymore. Any progress on the fix?

Revision history for this message
In , Bugzilla-henrik (bugzilla-henrik) wrote :

I also get multiple master password prompts upon startup with multiple tabs. I'm using Namaroka 3.6b2pre (mozilla-1.9.2/rev/7347a2572f1d) running on MacOSX 10.6.

The StartupMaster add-on (https://addons.mozilla.org/en-US/firefox/addon/9808) can be used as a workaround, but it's definately not perfect.

Revision history for this message
In , Mitchs-bugzilla (mitchs-bugzilla) wrote :

This hasta make it in to 3.6, this bug is old.

Many have suggested putting the master password prompt first, before any windows open, this (would) have solved both bugs. Is this what the patch does?

Revision history for this message
In , Honzab-moz (honzab-moz) wrote :

Works perfectly! The memory problem seems to be a bug in Apoint.exe but I'll keep an eye on it. Thanks for fixing this.

Revision history for this message
In , Honzab-moz (honzab-moz) wrote :

(In reply to comment #21)
> Works perfectly! The memory problem seems to be a bug in Apoint.exe but I'll
> keep an eye on it. Thanks for fixing this.

Sorry, wrong bug.

Revision history for this message
In , Paul-oshannessy (paul-oshannessy) wrote :

(In reply to comment #20)
> This hasta make it in to 3.6, this bug is old.

Unless our plans drastically change, this won't happen - plan is to freeze for RC next week, and this (and the changes it depends on) haven't even made it onto trunk yet.

> Many have suggested putting the master password prompt first, before any
> windows open, this (would) have solved both bugs. Is this what the patch does?

No, this patch keeps track of whether or not we're showing the master password UI, if you enter it once, the other requests for it get notified so they don't also popup the dialog.

Revision history for this message
In , Srcmax (srcmax) wrote :

Any progress on the fix?

The bug is always present in 3.6b4 and it's very irritating ... I hope it will be fix in 3.6 final.

Revision history for this message
In , Gavin Sharp (gavin-sharp) wrote :

*** Bug 533145 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Georges (georgeskesseler) wrote :

I have done some testing with firefox 3.6b5 and this bug is not fixed.
Which surprises me as the proxy auth bug which is similar IS fixed.
Now here is a race condition. If you have mutitple tabs with password forms and an authenticating proxy it depends what comes first.
If the proxy kicks in first, you get a single master password prompt and no prompt at all for the forms in the tabs.
If the proxy is late, then you get a master password for each tab and one for the proxy.

What I don't get, as many other people, is why this has not been fixed all together instead of one for proxy auth, one for basic auth, one for form auth and so on?

Result: firefox is still as "unusable" as before and even got a more erratic behaviour inside enterprises: sometimes it works sometimes not.

Also why has this new bug appeared whereas form based multiple prompts have already been discussed in other bugs for years? Looks to me as if people want to evade the issue.

Revision history for this message
In , Digulla-hepe (digulla-hepe) wrote :

In one of the many comments in all these bugs, someone suggested the one and only correct solution: Create a global lock for *all and any kind of dialog*.

Even if you pop up just a single dialog per proxy, master password, site auth, ...., that still means that I can get a second dialog while I'm typing in the first. I can type text blind but not passwords. So I'll always press RETURN for the wrong dialog.

Therefore, FF must never open a second dialog, no matter what.

A great optimization would be to wait for asking for anything until I open a tab which actually needs the password.

Revision history for this message
In , Digulla-hepe (digulla-hepe) wrote :

Along the same lines, I don't see how this bug depends on bug 513534. I would say it's the other way around: When the dialogs have become sequential, we can start on the optimization.

Revision history for this message
In , Kairo-kairo (kairo-kairo) wrote :

I wonder if we should have a generic framework that enables us never to ask for the master password twice at the same time in a more general way - Standard8 has been working on something for Thunderbird to make things better there, not sure how much it would fit that...

Revision history for this message
In , Chelmite (steve-kelem) wrote :

When I start up Firefox (3.5.5) I have lots of tabs.

I get SIXTEEN password popups! I type my password into the first one that lets me type and hit Enter. The popup doesn't disappear. A new one pops up on top of it. Sometimes it pops up while I'm typing in the first one, grabbing some of the newly-typed characters.

When there are lots of password popups, ONLY ONE of them will take input or honor its buttons. This turns "entering your password" into a game of whack-a-mole, trying to guess which one of the sixteen popups will let me hit Cancel! I'm not sure what else is going on (probably the page loads for each of the tabs), but it's REALLY SLOW, so it takes several minutes before a real firefox window appears!

Revision history for this message
In , Dolske (dolske) wrote :

I'm updating the patch for this bug, and am curious what people want to happen for one specific situation:

Suppose you are restoring a session in which there are multiple HTTP Auth requests for different servers. One master password prompt will be displayed while the others wait. Now, suppose for some reason you click Cancel instead of entering the master password. What should happen to the pending auth requests, and why (what's your use case)?

1) Cancel all of them. [No further MP or HTTP prompts displayed while restoring]

2) Cancel only the prompts for which you have stored logins. [ie, no further MP prompts.]

3) Don't cancel any of them. As each is processed, ask again for the Master Password if there are stored logins for the site.

4) Don't cancel any of them, but don't prompt again for the Master Password while restoring [thus, each prompt will be empty, with no login prefilled].

I'm planning on implementing #1, on the theory that if you're restoring a large number of tabs but click Cancel for the MP, you don't want bothered with any more prompts. The user might even just be borrowing someone else's computer, and has no interest in authenticating to anything (eg, they just want to check gmail). You can, of course, reload any tab to trigger the MP/HTTP auth again.

#3 is the easiest, but I don't think it's very useful (it just leads to clicking Cancel over and over). #4 may be rather hard to do due to implementation details.

Revision history for this message
In , Brendan-webafrica (brendan-webafrica) wrote :

#4 seems is an interesting option however I don't think it would be appropriate without also making the other options available to the user. Perhaps these four cases can be implemented as an option in a separate feature request.

Personally I prefer #1.

Does the above logic apply when the multiple prompts are not for a restore, such as when we click a bookmark with multiple tabs or quickly open multiple links in new tabs?

Revision history for this message
In , Joshprogramming (joshprogramming) wrote :

If the master password prompt only came up when you clicked the tab with the logins then I'd definitely say #3. However, based on my usage the master password comes up for all tabs at once, so #1 makes the most sense. It doesn't make sense to me to cancel it once and then want to enter it directly after.

Revision history for this message
In , Jjfelten (jjfelten) wrote :

Any are an improvement over the current situation, but my suggestion is to give the user the choice; add a Cancel All button.

Revision history for this message
In , Jonathan Pritchard (jonathanr-pritchard+bugzilla) wrote :

A Cancel All button sounds the most transparent and in keeping with current methodologies. I thought you could always utilise the hang down (probably not the right name for it) that comes up across the top of a web page to ask if you want to save a password and instead give an explanation of the Cancel All, something like that.

Revision history for this message
In , Digulla-hepe (digulla-hepe) wrote :

Implement *anything* and do it *before* 3.6 goes bad! The cancel button is an interesting piece of UI ambiguity but if it delays this patch by even a single hour, drop the work on it and open a new bug to fix it later.

To solve the ambiguity, you'd need a way to tell the user "There are N other pages which also need the master password", otherwise she can't make an educated decision. If you can't provide this data, then "Cancel" must be equals "Cancel All" (otherwise, it would be pointless to even have a Cancel button).

If "Cancel" isn't always "Cancel All", then what does a single cancel mean? Why would the user cancel a single MP request? Is there a situation where this would make sense at all?

As I see it, there is never a situation where I want to provide the MP for some connections but not for others. So when I press cancel, I just want to get rid of the damn dialog, period. That's why I think it's always "Cancel All". If you can, then set a flag not to ask again for the MP until all tabs have loaded and close the dialog.

If there is no simple way to implement this, then just open a new bug and concentrate on fixing the issue at hand.

Revision history for this message
In , Mstamat (mstamat) wrote :

I largely agree with Aaron. It is imperative to improve on the current situation before the next FF release. So, solution #1 would be the best to pursue in the short term.

In the long-term (i.e. *after* the next FF release), I would opt for a refinement of #2. From my use of FF, it is quite common to only want to enter the master password (e.g. to quickly access your email) and skip any other password prompts for later. So, a more flexible solution would be the following:
   * "Cancel" for master password prompt would cancel all prompts for which you have stored logins. This prompt should appear before any other prompts for which you have no credentials stored.
   * For the remaining prompts that have no stored logins there will be an additional "Cancel All" button. The number of prompts that will be canceled with this button should also be displayed (as Aaron proposes).

Unless the user wants to enter passwords for only some of the non-master password prompts, this solution is almost* optimal in terms of required user effort. Here is a table of usage scenarios and the associated user effort:

    | master password
    |----------------------------------------------------------------
    | | enter | do not enter
---------------------------------------------------------------------
    | enter | 1 master passwd | 1 click
    | all | + | +
    | | 1 passwd per other prompt | 1 passwd per other prompt
  p | | (minimum effort) | (minimum effort)
  a |----------------------------------------------------------------
o s | enter | 1 master passwd | 1 click
t s | some | + | +
h w | | 1 click or passwd | 1 click or passwd
e o | | per other prompt | per orther prompt
r r |----------------------------------------------------------------
  d | enter | 1 master passwd | 2 clicks
  s | none | + | (minimum effort + 1 click)
    | | 1 click |
    | | (minimum effort) |

*The "almost" is because of the extra click in the last cell of the table.

Revision history for this message
In , Kairo-kairo (kairo-kairo) wrote :

Dolske, as long as we have popup password prompts for HTTP Auth, #1 is best, I think. If we (ever) switch to in-tab-prompts, then #4 seems to be best, but I think it should be up to the bug doing the different prompting to care about that.

Revision history for this message
In , Digulla-hepe (digulla-hepe) wrote :

That said, I'd really prefer when the password prompts (normal and master) would be deferred until I actually click on the tab which needs them. But again, this is another story. Let's just get the "single password prompt" working. This bug is several years old and it's such a pain that I'm willing to spend money on the fix. Justin, do you have a PayPal account?

Revision history for this message
In , Faaborg (faaborg) wrote :

solution #1 is fine for the short term. Ultimately I think we are going to want to switch to a model where the master password is presented in manner more analogous to OS user accounts (and is only entered once to log into the browser).

Revision history for this message
In , Hskupin (hskupin) wrote :

*** Bug 538253 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Ulrich-windl (ulrich-windl) wrote :

(In reply to comment #31)
IMHO those tabs requiring access to secrets protected by the master password would "queue" a request for the master password. Once the master password is entered, all those requests would be fulfilled. What should happen if the user presses "Cancel" for the master password? As the user doesn't know the order of requests in the queue, the obvious solution IMHO would be to annotate the master password dialog with the number of requests (e.g. "Master password (7 requests)"). Then, if cancelling that, it should be obvious that those 7 requests cannot continue until a master password is entered. Also the reasonable solution IMHO would be either to cancel all requests in the queue if the master password isn't entered, or to keep all requests in the queue, and re-open the prompt for the master password (while there are requests in the request queue). Obviously the second solution is not what the user might want.
Independent from that is the effect that most tabs will still want to access the secrets, so they might re-queue requests and thus re-open the prompt for the master password.

Revision history for this message
In , Beltzner (beltzner) wrote :

Let's go with option 1, no need to add a "Cancel All" button since adding a string will make it impossible to back-port this to 1.9.2 when it's done.

Revision history for this message
In , Catlee (catlee) wrote :

(In reply to comment #31)

#1 is fine. The only reason I hit cancel right now is so that I can dismiss the 4 or 5 dialogs that pop up for me on session restore. Assuming I only got one prompt, the only reasons I would hit cancel would be because I'm offline, or know that I don't want any of the tabs from session restore.

Revision history for this message
In , Dolske (dolske) wrote :

Created an attachment (id=421176)
Patch v.3 (WIP)

Revision history for this message
In , Dolske (dolske) wrote :

Patch v.3 happens to be applied on top of a minor security patch in my queue, so I've marked it as private, just to be extra cautious. I expect to be able to unmark it shortly.

Revision history for this message
In , Etcoproc (etcoproc) wrote :

My suggestion is: when Firefox starts and each tab begins to load, some of them ask for password. FF should ask user for master password (showing only one window). If the user clicked cancel, FF should stop asking. But when user enters some page that depends on password FF should ask ONCE more. If user cancels again, FF should quit asking (until FF restarts).

Revision history for this message
In , Rnicoletto (rnicoletto) wrote :

(In reply to comment #31)
> I'm updating the patch for this bug, and am curious what people want to happen
> for one specific situation:

I vote for #1 too.

Revision history for this message
In , Abiedron (abiedron) wrote :

I haven't any problem with thunderbird 2.23 - now I update TB to version 3.0 and I have been asked for:
1. Master password
2. 1st Password to google calendar in lightning for 1st google calendar
3. 2nd Password to google calendar in lightning for 2nd google calendar

I have 3-rd google calendar yet (totally i have 3 calendars), but for 2 of them I use the same google login - so Thunderbird prompts me about passwords for callendars ony 2 times.

I think I should be prompt only once to type master password.

Revision history for this message
In , Dolske (dolske) wrote :

Thanks for the feedback everyone, case #1 from comment 31 is what I'm implementing here. Any further refinements should be requested in separate bugs.

Revision history for this message
In , Paul-oshannessy (paul-oshannessy) wrote :

*** Bug 542321 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Mats Palmgren (matspal) wrote :

*** Bug 540796 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Matti-mversen (matti-mversen) wrote :

*** Bug 543319 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Matti-mversen (matti-mversen) wrote :

*** Bug 546184 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Ria-klaassen (ria-klaassen) wrote :

*** Bug 547166 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Kevin Brosnan (kbrosnan) wrote :

*** Bug 550112 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Joe-drew (joe-drew) wrote :

Can the patch on this bug be unhidden now?

Revision history for this message
In , Dolske (dolske) wrote :

(From update of attachment 421176)
Indeed, the other bug has already shipped.

Revision history for this message
In , Matti-mversen (matti-mversen) wrote :

*** Bug 555672 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Mozilla-bugs-micahscomputing (mozilla-bugs-micahscomputing) wrote :

*** Bug 557142 has been marked as a duplicate of this bug. ***

Revision history for this message
In , P (p92) wrote :

hello, in what firefox release will it be included ?

Revision history for this message
In , Kevin Brosnan (kbrosnan) wrote :

You are on the cc list so you should get an email when someone attaches an updated patch. Once the patch is approved and checked in there will be a comment in this bug with the mecurial mozilla-central revision that includes the fix for this bug.

Revision history for this message
In , Dolske (dolske) wrote :

Created an attachment (id=439149)
Patch v.4

I need to write some tests for this. But I verified it works with manual testing, so it's good for a review pass.

Revision history for this message
In , Bugzilla-standard8 (bugzilla-standard8) wrote :

(In reply to comment #63)
> Created an attachment (id=439149) [details]
> Patch v.4

Justin, I've just taken a quick glance at this - would this change also fix the case where code wants to get a login direct from the PW manager without being form fill or going through the prompt mechanism? i.e. calling findLogins direct?

Its just that being able to get a login without using the prompt code but having master password prompts not stack up is one of the last remaining pieces that mailnews is currently having to work around. Don't let me extend the scope of this bug, but I just want to make you aware of it as I haven't got around to filing a separate bug yet.

Revision history for this message
In , Dolske (dolske) wrote :

(In reply to comment #64)

> Justin, I've just taken a quick glance at this - would this change also fix the
> case where code wants to get a login direct from the PW manager without being
> form fill or going through the prompt mechanism? i.e. calling findLogins
> direct?

No. For other code (and extensions) using the password manager API, to avoid triggering another MP prompt you'll need to add code to check nsILoginManager.uiBusy (and if true, defer your work until an observer sees that the master password has been entered).

Unfortunately, it's not possible to fix this in a generic way without breaking the API. Once NSS has invoked the PSM callbacks to show the prompt the first time, logging in from that prompt won't work unless we unwind the stack all the way back to the original caller. So if a second request comes along while the prompt is up, the only choices are to show a second prompt or return an error.

The long term fix here is to either change how the token login / prompting is implemented, or make the login manager interfaces async (ie, return results through callbacks). Fodder for another bug, though!

Revision history for this message
In , Paul-oshannessy (paul-oshannessy) wrote :
Download full text (4.8 KiB)

(From update of attachment 439149)
>+ get isLoggedIn() {
>+ let status = this._sdrSlot.status;
>+ this.log("SDR slot status is " + status);
>+ if (status == Ci.nsIPKCS11Slot.SLOT_READY ||
>+ status == Ci.nsIPKCS11Slot.SLOT_LOGGED_IN)
>+ return true;
>+ if (status == Ci.nsIPKCS11Slot.SLOT_NOT_LOGGED_IN)
>+ return false;
>+ throw "uhh unexpected slot status?";

uhh expected better error message?

>+ // Don't trigger a 2nd master password prompt (it's
>+ // unlikely we'll get here, because we normally don't
>+ // attach this listener until after a MP entry).
>+ if (this._pwmgr.uiBusy)
>+ return;
>+

So how would we get to this case? I'm fine with it there if we can actually hit it, and it would be good to have a test to make sure that we're actually preventing this case.

> autoCompleteSearch : function (aSearchString, aPreviousResult, aElement) {
> // aPreviousResult & aResult are nsIAutoCompleteResult,
> // aElement is nsIDOMHTMLInputElement
>
> if (!this._remember)
>- return false;
>+ return null;

What's this change for?

>+ // If we're currently displaying a master password prompt, defer
>+ // processing this document until the user handles the prompt.
>+ if (this.uiBusy) {
>+ this.log("deferring fillDoc for " + doc.documentURI);
>+ let self = this;
>+ let observer = {
>+ QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver, Ci.nsISupportsWeakReference]),
>+
>+ observe: function (subject, topic, data) {
>+ self.log("Got deferred fillDoc notification: " + topic);
>+ // Only run observer once.
>+ Services.obs.removeObserver(this, "passwordmgr-crypto-login");
>+ Services.obs.removeObserver(this, "passwordmgr-crypto-loginCanceled");
>+ if (topic == "passwordmgr-crypto-loginCanceled")
>+ return;
>+ self._fillDocument(doc);
>+ },
>+ handleEvent : function (event) {
>+ // Not expected to be called
>+ }
>+ };
>+ // Trickyness follows: We want an observer, but don't want it to
>+ // cause leaks. So add the observer with a weak reference, and use
>+ // a dummy event listener (a strong reference) to keep it alive
>+ // until the document is destroyed.
>+ Services.obs.addObserver(observer, "passwordmgr-crypto-login", true);
>+ Services.obs.addObserver(observer, "passwordmgr-crypto-loginCanceled", true);
>+ doc.addEventListener("mozCleverClosureHack", observer, false);
>+ return;
>+ }
>+

Trickyness indeed. Is this something we do elsewhere in the code? Is mozCleverClosureHack an event name that's standard or can we make that more password managery? Or even something super clever & ghostbustery?

>@@ -103,34 +102,47 @@ LoginManagerP...

Read more...

Revision history for this message
In , Blacknova-myrealbox (blacknova-myrealbox) wrote :

Hello, thanks everyone for contributing to the resolution to this bug!

A few questions:
1) Does Justin's patch also address multiple modal password dialogs raised by multiple tabs requesting *HTTPS* authentication? or is this patch specific to HTTP prompts only?
2) Any expectations as to when this will hit trunk or any other maintenance branches of 3.5.5?

Cheers!

Revision history for this message
In , Jonny (dioni21) wrote :

I could be wrong, but I think this same bug is affecting Thunderbird.

Since I started using extensions that need master password access, I have to enter it once for each extension, and once for Thunderbird itself.

For example, the lightning extension, when configured to use a Google calendar shows this behavior. Is this the same bug, or just an extension bug?

Revision history for this message
In , Mozilla-bugs-micahscomputing (mozilla-bugs-micahscomputing) wrote :

(In reply to comment #68)
> I could be wrong, but I think this same bug is affecting Thunderbird.
>
> Since I started using extensions that need master password access, I have to
> enter it once for each extension, and once for Thunderbird itself.
>
> For example, the lightning extension, when configured to use a Google calendar
> shows this behavior. Is this the same bug, or just an extension bug?

No, that's a separate issue, see bug 349641.

Revision history for this message
In , Mail-ptt-grafikdesign (mail-ptt-grafikdesign) wrote :

Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.1.9) Gecko/20100315 Firefox/3.5.9 GTB7.0 (.NET CLR 3.5.30729)

Since Firefox 3.5.x it's annoying me when restart old sessions, but after having typed in once I Cancel all Request's and reload all Tab's new. Then it'll work.

Revision history for this message
In , Anders-rickardsson (anders-rickardsson) wrote :

Please solve this! This bug is so annoying! PLEASE!

Revision history for this message
In , Blacknova-myrealbox (blacknova-myrealbox) wrote :

On a related note, can someone with Bugzilla privileges please confirm bug 524909 & offer some suggestions? 524909 is a bit more annoying as you are unable to type anything at all in the modal Master Password input dialog! (note: the browser doesn't appear to be hung, it just seems to refuse any sort of text entry in the input field).

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5

Revision history for this message
In , Dolske (dolske) wrote :

(In reply to comment #66)

> >+ // Don't trigger a 2nd master password prompt (it's
> >+ // unlikely we'll get here, because we normally don't
> >+ // attach this listener until after a MP entry).
>
> So how would we get to this case?

Hmm, I guess we can't. I was thinking some mix of loading a login page, canceling the resulting MP prompt, and then later revisiting the page to type in a login... But the MP cancelation would throw, and fillDoc() would thus bail out before attaching an autocomplete listener. So, not just edge-casy but shouldn't be possible. Removed. Ditto for the other case of this.

> > if (!this._remember)
> >- return false;
> >+ return null;
>
> What's this change for?

Noticed while checking code that autoCompleteSearch is supposed to return an nsIAutoCompleteResult (or null), not a bool.

> >+ doc.addEventListener("mozCleverClosureHack", observer, false);
>
> Trickyness indeed. Is this something we do elsewhere in the code?

Yeah, I first used it over in browser.js for some plugin stuff.

> Didn't we already do the hasLogins check when adding the prompt? Isn't it just
> |prompt.hasLogins| & we can avoid this additional DB hit?

Hmm, good catch. I think that's deadwood from an early version of the patch. I've removed the |prompt.hasLogins| setting, since it wasn't used. Better to check it right when needed (otherwise we could miss a recently-saved login).

Revision history for this message
In , Dolske (dolske) wrote :

Created an attachment (id=445640)
Patch v.5

Fixed nits, added a test. The test passes for me, but seems to trigger failures in other later tests. Looks like we're running into bug 437904. :( Someone's going to need to debug that problem before this can safely land.

Also, painful test-debugging note to self: avoid global "var crypto" because that can lead to accidentally using window.crypto elsewhere, giving unfortunate results.

Revision history for this message
In , Paul-oshannessy (paul-oshannessy) wrote :

(From update of attachment 445640)
Looks great. Just 2 comments on the tests. r=zpao with them addressed and figuring out what's triggering the other failures (nothing jumped out at me while looking at them

>+++ b/toolkit/components/passwordmgr/test/test_master_password.html
>+ // XXX check that there's 1 MP window open

For both of these XXX, I realize that counting would be a more exact science, but you can you at least check |pwcrypt.uiBusy|. None of the tests are actually checking it. It would be nice to have explicit checks for uiBusy up the chain, but they all end up forwarding pwcrypt's value, so that's ok.

>+ // XXX do a test5ABC with clicking cancel?

Yea, probably should. Part of the expected behavior is that cancelling should cancel all pending ones.

Revision history for this message
In , Paul-oshannessy (paul-oshannessy) wrote :

*** Bug 566835 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Jo-hermans (jo-hermans) wrote :

*** Bug 571187 has been marked as a duplicate of this bug. ***

Revision history for this message
Micah Gersten (micahg) wrote :

Adding a task for the unversioned Firefox for Lucid+

Changed in firefox (Ubuntu):
importance: Undecided → Medium
status: New → Triaged
Revision history for this message
In , Dao (dao) wrote :

*** Bug 574633 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Kai Engert (kaie) wrote :

I tried to implement a more generic solution.
Instead of the patch from this bug, could you please try bug 177175 attachment 454051 and give feedback?

Revision history for this message
In , Matti-mversen (matti-mversen) wrote :

*** Bug 575584 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Jo-hermans (jo-hermans) wrote :

*** Bug 577895 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Dolske (dolske) wrote :

Created attachment 460738
Patch v.6

Apply cleanly to trunk.

Revision history for this message
In , Shawn Wilsher (sdwilsh) wrote :

Comment on attachment 460738
Patch v.6

a=sdwilsh, but the first sign of trouble on the tree because of this should mean this gets backed out stat.

Revision history for this message
In , Dolske (dolske) wrote :
Changed in firefox:
status: Confirmed → Fix Released
Revision history for this message
In , Wildmyron (wildmyron) wrote :

*** Bug 584003 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Matti-mversen (matti-mversen) wrote :

*** Bug 587113 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Wildmyron (wildmyron) wrote :

*** Bug 590020 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Solin-nick (solin-nick) wrote :

What version is this fix planned for? Because I started Firefox today and lone behold the password prompt spammed me 10 times.

Revision history for this message
In , Ryanvm (ryanvm) wrote :

Firefox 4.0. You can confirm it with the current 4.0b4 release or the upcoming 4.0b5 release.

Micah Gersten (micahg)
Changed in firefox:
milestone: none → 4.0
Revision history for this message
In , Matti-mversen (matti-mversen) wrote :

*** Bug 593292 has been marked as a duplicate of this bug. ***

Changed in firefox:
importance: Unknown → Medium
Revision history for this message
In , Jo-hermans (jo-hermans) wrote :

*** Bug 600549 has been marked as a duplicate of this bug. ***

Revision history for this message
Micah Gersten (micahg) wrote :

This is fixed in Natty now with Firefox 4.0b7. Please report any other issues you may find.

Changed in firefox (Ubuntu):
status: Triaged → Fix Released
Revision history for this message
In , P (p92) wrote :

hum now that this master password popup storm bug has been solved - it would be also time to solve the cookie confirmation popup storm also before firefox 4 release : big #515521

Revision history for this message
In , Matti-mversen (matti-mversen) wrote :

*** Bug 614184 has been marked as a duplicate of this bug. ***

P (p92)
Changed in firefox-3.0 (Ubuntu):
status: Triaged → Fix Released
Changed in firefox-3.5 (Ubuntu):
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Related questions

Remote bug watches

Bug watches keep track of this bug in other bug trackers.