45455 - Deleting a blank username from SingSignon deletes a valid username

Status: VERIFIED FIXED

(SeaMonkey :: Passwords & Permissions, defect, P3)

Description

[John Morrison]

Overview Description:

  Deleting a blank username from SingSignon deletes a valid username

Steps to Reproduce:

1) First, follow the 'Steps to Reproduce' bug #45454
2) Now bring up 'Tasks->Privacy...->Password Manager->View Stored Passwords'
3) Delete the entry with the username value of null.
4) Return to the sign-on page -- notice that the prefill value is the 
   blank username
5) Return to 'View Stored Passwords' -- notice that the username that
   was *actually* deleted was the valid username, not the blank one.

Bonus points: try it with two valid usernames and a blank one :-]

Actual Results:  
  The wrong username is deleted.

Expected Results: 
  a) shouldn't have a blank username in the database to begin with (IMHO).
  b) if one exists, then need to correctly allow the user to delete it 
     from the database. 

Reproducibility: 100% mac/linux/win32

Build Date & Platform Bug Found:
  2000071308 win95
  2000071308 mac os 9.0
  2000071308 linux rh 6.1

Comment 1

[Stephen P. Morse]

Problem was in the si_RemoveUser routine in singsign.cpp.  Turns out that that 
routine was designed to delete the first user if the username passed to it was 
blank.  That was obviously a poor design.

So the fix is simple.  I'll add another parameter to that routine to determine 
if you want the first user deleted or if you want to delete the user whose name 
is passed in as a parameter.  I'll attach the patch.  John Morrison, can you 
please code review this for me.  Thanks.

Comment 2

[Stephen P. Morse]

Attachment: Patch to fix the problem

Comment 3

[Stephen P. Morse]

Fix checked in.  File modifies is extensions/wallet/src/singsign.cpp

Comment 4

[sairuh (rarely reading bugmail)]

steve, did you check this into the trunk (m18) or beta2 branch (m17) tree? i
still encounter this problem using the opt commercial beta2 bits on linux,
2000.07.31.04-m17. jrgm, d'you see this as well (either trunk or branch)?

if this has been fixed only for the trunk, and will *not* go into the beta2
branch, i'll add the relnote2 kw. otherwise, if this is still a problem on the
trunk, this should be reopened.

Comment 5

[John Morrison]

This was only on the trunk. (I'm not sure if I'd even release note this for 
beta2 -- in principle, we could release note every open bug, but then no one
would ever read the release notes. This is an edge case that needed to get 
fixed, but not really for beta2; the user can't easily fall into this trap).

Comment 6

[Stephen P. Morse]

Sorry for not responding sooner but I was out of town.  John's absolutely 
correct -- I agree with everything he just said.  He took the words out of my 
mouth.

Comment 7

[sairuh (rarely reading bugmail)]

feel free to verify these bugs if i don't get to 'em beforehand. if you do, pls
remove the verifyme keyword when you do verify such a bug. thx!

Comment 8

[sairuh (rarely reading bugmail)]

yep, i'm able to delete an empty singon (ie, blank username, non-blank passwd)
w/o affecting other signons. vrfy using 2000.08.22.08 commercial bits on the Big
3.