I recently upgraded from 1.3.1 to 1.7 RC2. There's a new behavior that is really terrible. Now, when I try to read my POP email, it prompts me for my master password. But I have NOT stored the password for this POP server, so I bonk cancel. I have to do that 5 times before it finally stops asking me for my master password and gets around to asking me for my POP server password. Really irritating. This bug is similar to, but not a dup of, bug 236510 I'm happy to answer any questions you have about this. I have 4 POP email accounts and 5 news server accounts configured in the profile. I use the password manager to store the passwords for 2 of the 4 POP mail accounts, but NOT to store the password for this particular account. I think that it should not prompt me for the master password for this account at all. If you think it really must do so, for some reason, then it should do so at most once. When I bonk cancel, it should take the hint.

Can you verify that it exists in 1.7rc3 Other than that, moving "branch" off `1.4` to `other branch` (1.7) Bumping severity up a notch, and adding regression keyword. Should this exist in 1.7rc3 I'll request blocking, if it does not exist, I'll peek for a dupe, and if I find none, I'll resolved.

(In reply to comment #1) > Can you verify that it exists in 1.7rc3 ? If it does, please check with a new profile too. Thanks.

Nelson: are you seeing this behavior when you explicitly check the account for new mail or when you open mail (perhaps it's trying to check all your accounts)? With linux trunk 2004061006, I get a master password dialog even though I didn't store the password, but I don't think the password manager even knows what is stored until it has the master password (if you try to manage passwords without typing the master password, the list is empty).

Yes, it exists in RC3 too. In fact, it's worse. It's 7 prompts in RC3. In answer to comment 3, here are the exact steps to reproduce. 1. fire up mozilla. 2. Window->Mail and Newsgroups 3. Right click on this email account, and in the pop-up menu, choose "Get Messages for Account". 4. observe and cancel (in sequence) *7* master password prompts. 5. Finally get regular mail server password prompt.

Would it be possible to test this with a fresh profile (where you add these mail accounts to your new profile) and then set passwords same as you have on your current profile. Could be a mal-set pref of some sort causing this, and if its reproduceable the way I just described, may help the mailnews devs track it down faster.

Here's another idea. Review the changes between RC2 and RC3 that could have affected the wallet. Back 'em out. The look for other changes in that area that happened recently, back 'em out too.

(In reply to comment #6) > Here's another idea. It could be interesting to try with a current Trunk (v1.8a1 or after) build too. You should be able to install both and share your profile(s).

> In answer to comment 3, here are the exact steps to reproduce. Using these steps, I get only one master password dialog. (In reply to comment #6) > Here's another idea. Review the changes between RC2 and RC3 that could > have affected the wallet. Back 'em out. The look for other changes in that > area that happened recently, back 'em out too. http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_7_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=05%2F14%2F2004&maxdate=06%2F08%2F2004&cvsroot=%2Fcvsroot

Perhaps this info is relevant: I have 7 saved passwords. 2 are passwords used with composer, to upload composed web pages, 2 are for POP email accounts 3 are for logins to web sites I'd *guess* that I'm getting one prompt for each of those, and I'd guess that somewhere between RC2 and RC3, some code as changed so that I now get prompted for all types of passwords, whereas before, I was only prompted for POP and web site logins.

Two additional discoveries: 1, I added an eighth password, one for another web site. Sure enough, now when I try to read email on my POP account, I get prompted *8 times* for my master password. 2, I also get prompted 8 times now by doing the following: a. Edit->preferences. b. Privacy & Security-> passwords c. click Manage Passwords. d. when it prompts for the master password, click cancel. e. repeat step d until it stops asking for the master password. Clearly, somewhere there is some logic that walks down the content of the .s file, and for each password entry, it makes some call that asks for the master password if it wasn't recently given. It should give up and STOP when any of those requests is cancelled.

I can reproduce the behavior in comment 10. It regressed between linux trunk 2004021208 and 2004021308, apparently bug 186237. but the original bug is more complicated than that. With 2 passwords and checking mail on an account without a password stored: 1.4: no master password prompt 1.5-1.6: 1 master password prompt 1.7rc3-trunk: 2 master password prompts 1 -> 2 prompts regressed between 2004021208 and 2004021308

This looks like it's rev 1.191 in nsMsgIncomingServer.cpp to fix bug 219976 which added a call to nsIPasswordManagerInternal::FindPasswordEntry() checked in Sept 2003 http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/mailnews/base/util&command=DIFF_FRAMESET&file=nsMsgIncomingServer.cpp&rev1=1.190&rev2=1.191&root=/cvsroot However, when I do a bonsai blame on the file it looks like the call has been there since at least rev 1.152

Nope, Andrew is right, bug 186237 caused it to keep enumerating after the first failure, so it enumerates through each saved host (number of unique host entries, not passwords) asking for the master password to unencrypt each one. The more sites for which you use password manager the worse it gets (I get 101 prompts), but probably the more likely you are to just enter the password. Even if FindPasswordEntry were working correctly you'd get one master password prompt. That's OK in this bug since it is asking for the POP password, but the comments in the Biff case seem to imply they don't want the prompt, so fixing this bug only makes bug 236510 bearable but doesn't fix it.

The .s file has the name of the site/account unencrypted and unencoded. So, it should be possible to traverse the list, and not even ATTEMPT to decrypt or decode anything UNTIL a match is found on the site/account name. Since I have no password stored for this account, I should get zero prompts to the master password to access this account. It did work that way in 1.3 (and also 1.4 according to comment 11 above). So, apparently there have been several regressions. This bug should be marked fixed when it works like 1.3/1.4 again.

the problem is here: http://lxr.mozilla.org/seamonkey/source/extensions/wallet/src/nsPasswordManager.cpp#207 This code enumerates all entries. The enumerator decrypts everything. Only after that, it checks if the urls match. Fixing it would require a sane way to access the singsign database. (this code is ugly... real ugly)

Can we use a distinguished XPCOM failure result code to indicate that the user has canceled the master password dialog, and break out of the (or all such) loops? /be

or just back up to pre-1.4 behavior with some patch back outs?

This reverts a couple lines made during the change to bug 186237. Result is that if you cancel out of the master password prompt on a mail account with no password you go directly to the mail password prompt. For an account that *does* have a stored password you have to cancel three times before getting the mail password prompt. This does not appear to regress bug 186237, in fact I'm not sure why these lines were changed in that bug since nothing in there uses nsPasswordManager::FindPasswordEntry.

Comment on attachment 150853 [details] [diff] [review] backs out mysterious extra bit of bug 186237 should I get mcalmus to review as well since this affects part of his original patch? Is he still active?

Comment on attachment 150853 [details] [diff] [review] backs out mysterious extra bit of bug 186237 With the NS_SUCCEEDED(rv) test removed, as discussed over IRC. /be

Comment on attachment 150856 [details] [diff] [review] same but remove extraneous NS_SUCCEEDED test marking implied sr= from comment 21

Comment on attachment 150856 [details] [diff] [review] same but remove extraneous NS_SUCCEEDED test a=leaf on behalf of drivers

Fix checked into trunk and 1.7 branch (couple hours ago, forgot to update bug).

Used latest Mozilla 1.7 branch 06-17 build: Bug verification is as following: So far, after click "Cancel" from the Master Password Dialog. It displays 3 times for those stored password POP accounts. And it displays 1 time for those non-store password POP accounts. For this bug itself, Master password does not prompt per saved host anymore.. The rest issues will be tracked on bug 236510 & bug 247417... Changing Keywords from "fixed1.7" to "verified1.7" Will leave bug status "as is" until this bug be verified on trunk.

I filed bug 247623 for the fact that mail prompts for the master password at all when accessing an account without a password stored.

(I marked this fixed-aviary1.0 when I ported the patch to the branch, but the code actually isn't used there -- it's forked, and substantially different. Did anyone test for the presence of this bug in Thunderbird?)

The fix for Bug #186237 had a similar effect on the password manager dialog. I'm not sure whether it's completely appropriate to put it here, but in any event I have a patch to fix it for that situation as well.

This patch allows the code to propagate more specific failure reasons to the top layer so the exception can deal with different situations as apporpriate. I didn't know and couldn't find a way to parse out the exception other than a String test so if there's a better way, I'd love to use it.

Comment on attachment 151330 [details] [diff] [review] Patch to fix similar issue with Passowrd Manager dialog >} catch(e) { >+ /* The user cancelled the master password dialog */ >+ if (e.indexOf("NS_ERROR_NOT_AVAILABLE") != -1) { Use: e.result==Components.results.NS_ERROR_NOT_AVAILABLE

Changed to reflect Comment #30.

Comment on attachment 151353 [details] [diff] [review] Same patch using correct exception syntax sr=dveditz

Comment on attachment 151353 [details] [diff] [review] Same patch using correct exception syntax Is someone able to check in the patch now that it has been fully r and sr'd?

(In reply to comment #33) > (From update of attachment 151353 [details] [diff] [review]) > Is someone able to check in the patch now that it has been fully r and sr'd? checked in on trunk

Bug 247417 seems to have added more fixes to this