210433 - Master Password is reasked many times even if Cancel is pressed

Status: RESOLVED DUPLICATE of bug 237610

(SeaMonkey :: Passwords & Permissions, defect)

Description

[Webmaster33]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4a) Gecko/20030401
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4a) Gecko/20030401

Master Password is reasked the same number of times, how much field you have on
the webpage, even if Cancel is pressed.

Once Cancel should be enough for a webpage to cancel the Master Password dialog.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.



Expected Results:  
Only 1 Cancel for a webpage to cancel the Master Password dialog.

Comment 1

[u97006]

Is this a dupe of bug 134593?

Comment 2

[Webmaster33]

Matthias Eisner: 
Well, the summary if this bug ("Master Password is reasked many times even if
Cancel is pressed") describes much clearly the problem, unlike the "Master
password prompt is displayed twice for each login form" of bug 134593.
But I think both bugs are about the same problem.

Comment 3

[John G. Myers]

*** This bug has been marked as a duplicate of 134593 ***

Comment 4

[John G. Myers]

Actually, this might not be a duplicate after all, as bug 134593 only addresses
a reask upon form submit.  If this is asking about a prompt per field upon load,
it is a separate bug.  If so, please reopen and submit a URL that exhibits this bug.

Comment 5

[Webmaster33]

Here is a good example:
http://bugzilla.mozilla.org/query.cgi?GoAheadAndLogIn=1

Master password dialog is reasked 2 times, right when the page is loaded.
Therefore not dupe of bug 134593. Reopening.

On pages with X fields, the dialog is reasked X times, after pressing Cancel!

Comment 6

[John G. Myers]

Mass reassign ssaux bugs to nobody

Comment 7

[John G. Myers]

-> Password Manager

Comment 8

[Sean Aitken]

This is a real pain. My wife uses Moz during the day, and when I get home, I
click the mail icon, which at that time, I realize that it's her profile.  I
have to literally click the 'cancel' button 12 times. Please fix!

Comment 9

[Eric Lecht]

I emailed NY Times, figuring it MAY be a problem at their website and NOY
completely a Mozilla pproblem. It has gotten so bad i can't even read articles
anymore. NY Times does all SORTS of funky stuff. I may be off-base here, but i
sent the following to their General Help email, will pass on what they say..:

"Gentle NY Time staffer,

Please read my problem; likely i'm not the only one suffering from it. 
Recently it has become increasingly difficult to get at articles. I'm a Mac
user, use Mozilla (security concerns relegate IE as unusable), i'm savvy to
cookies and permissions, and NY Times and all the cookies I need permitted to
register are permitted. 

Recently i'm getting spurious results accessing articles. I'll launch Mozilla,
and of course click on 2 or 3 articles to load in tabs in the background. I
couple of weeks ago I noticed this no longer workls, as in I must launch the 1st
article, AND MANUALLY LOGIN where before I was effectively automatically logged in.
Worse, if I forget and accidentally launch two or three, of course each stops at
the registration page. I'll go to the 1st, log in, kill the extras, go BACK to
the front-page tab and re-click the articles. They bring meagain (and massively
irritatingly) to a registration page whereupon no matter WHAT I enter in the
registration fields (like my username and password) i'll stay at the
registration page. No article

This is maddening.
I tried enabling ALL cookies (for the sake of arguement). Same result. 
I closed and re-opened the browser. Same result.

Look --i'm not going to start using that awful, awful insecure 'promiscuous'
browser IE (Internet Explorer) just to read the NY Times, nor should you demand
this, considering HOW BAD security is on the thing (and everybody knows it or
ought to).

NY Times has a problem it would seem, a knotty one that's going to take some
work to solve. To blame it on Mozilla is not going to be productive; we users
know enough to permit certain cookies in order to access the NY Times, we are
more or less OK with that. But the situation is getting SO BAD we cannot even
access articles in any way, shape or form no matter WHAT we do. I'd rather be
reading articles here at 6am, but it has gotten SO BAD I must write this letter
to you. After all, NY Times is my lifeline here in Boise ID.

Ideas? Notions? Solutions? A set of guidelines for users who exert more than
passing lip service to responsible browsing security, and pay attention to
cookies? I'll copy Mozilla's Bugzilla on this; provide me w/ a contact to pass
on what I learn from them if you wish. I know cookies and advertising are the
lifeblood of a paper, and frankly i have no quibble with it, but this only goes
so far...

warm regards,

vinylone

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

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

Comment 11

[john fang]

Attachment: patch for the latest mozilla,1.8a+

this patch can resolve the problem 'click cancle, prompt again and again when
Master Password Prompt.'

Comment 12

[john fang]

Comment on attachment 163552 [details] [diff] [review]
patch for the latest mozilla,1.8a+

mvl, can you help me to review the patch? Thanks!

Comment 13

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 163552 [details] [diff] [review]
patch for the latest mozilla,1.8a+

>-        /* not first data node for any saved user, so simply pick first user */
>-        if (url->chosen_user) {
>+        /* Not first data node for any saved user, so simply pick first user.
>+	 * rv=NS_ERROR_NOT_AVAILABLE means user click 'cancel' button or other errors*/
>+        if ((url->chosen_user)&&(rv!=NS_ERROR_NOT_AVAILABLE)) {

I tried to figure out hwo this patch works. I would say that place where the
master password is asked is at the  si_Decrypt call (line 1200). Or maybe not.
This code is confusing...

Can you explain it some more?

Comment 14

[:Gavin Sharp [email: [e-mail]]]

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

Comment 15

[Vidar Haarr (not reading bugmail)]

(In reply to comment #13)
> (From update of attachment 163552 [details] [diff] [review])
> I tried to figure out hwo this patch works. I would say that place where the
> master password is asked is at the  si_Decrypt call (line 1200). Or maybe not.
> This code is confusing...
> 
> Can you explain it some more?

The callstack is DecryptString calls nsSecretDecoderRing::Decrypt which forces a
PK11_Authenticate since the master login is always reset. If this login fails
for some reason, Decrypt returns NS_ERROR_NOT_AVAILABLE. If it does, we will
stop trying to decode the already stored usernames for this form, and user_count
will be 0.

Afterwards there is a if statement that checks if user_count is 0, 1 or >1, and
if it's 0, it simply picks the first user assigned to that realm.

The callers of si_GetUser, if it returns a valid user struct (!null), will try
to get the password (which in turn requires a login), and so will prompt for the
master password again.

So what the patch does is that it simply saves the return value from Decrypt and
then checks before picking the first user assigned to the realm and return that
from si_GetUser, if we got NS_ERROR_NOT_AVAILABLE it will return null instead,
which will suppress any subsequent password fillings and therefore any master
password prompts.

That's how I understand it, at least. This code is horrible; I'm amazed it works
at all.

In any case the patch is bitrottet and should be updated.

Comment 16

[John G. Myers]

See patch attached to bug 132323.  I tried to clean up this mess, but got review
withheld without reason.

Comment 17

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 163552 [details] [diff] [review]
patch for the latest mozilla,1.8a+

Sorry, I don't have time for wallet reviews. (and wallet should just, with callers switching to the alternative in toolkit)

Comment 18

[Matěj Cepl]

isn't this dup of bug 237610?

Comment 19

[Bill McGonigle (not currently reading bugmail; please contact directly)]

Going with Jens' duping logic to favor the Core bug.