41843 - Single Signon practically unusable for website logins

Status: VERIFIED FIXED

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

Description

[Stephen P. Morse]

Dan Veditz discovered the following problem with single singon:

1. Log onto nytimes to see a particular story
2. Have single singon save the logon
3. Exit and reenter browser
4. Return to nytimes and log on for a different story
5. Login does not get prefilled

Problem is that the url contains a query string that identifies the story.  
Single singon used to strip off the query string before processing the url.  
Warren recently removed all the strip code and so the full url is being used for 
the match.  Fix is to put back some of the strip code, at least for the query 
string.

Comment 1

[Stephen P. Morse]

This is a regression that just occured.  It greatly reduces the 
usefuleness of the single signon feature. 

Nominating for nsbeta2.

Comment 2

[Stephen P. Morse]

Seth, if I put back the stripping code and have it simply strip off the query 
string, will that break anything for mailnews?

Comment 3

[Stephen P. Morse]

The problem is worse than just the query string.  If you go to www.vanguard.com 
and do a logon, the URL that gets captured is:

https://majestic2.vanguard.com/PRFL/DA/0.1.InitialFrameSet/145681965508102319?

In this case the numbers at the end are different each time you return to the 
site and so single signon will fail here.  Prior to removing the stripping code, 
this worked because we removed the path from the url and kept just the host.

Bottom line: the recent change of removing the stripping code has broken 
nytimes.com, vanguard.com, and probably many other sites.  In other words, 
single signon is now unusable!

Comment 4

[Stephen P. Morse]

Wait, I can fix this without breaking mailnews.  It's not single signon that 
needs to do the stripping but rather the caller (that was warren's intent).  The 
caller in the case of website logins is unique -- happens to be in wallet.cpp at 
line that reads:

   SINGSIGN_RememberSignonData (URLName, signonData);

So if I do adequate stripping there I can fix the new problem that just started 
occuring for website logins and not break anything that seth did for mailnews.

Comment 5

[Stephen P. Morse]

Updating summary line to adequately reflect the severity of this bug.

Comment 6

[(not reading, please use [e-mail] instead)]

morse:  yes, the correct fix is to change the caller.

Comment 7

[leger]

Putting on [nsbeta2+] radar for beta2 fix. 

Comment 8

[(not reading, please use [e-mail] instead)]

I just talked to morse on the phone. 

his changes should not hork mailnews.

but I'd like to follow warren's original plan, and not do any parsing in the
single signon code.

can you put your stripping code in one place, and just make callers (wallet.cpp
and nsWalletService.cpp) call it first?

that should not be hard to do.

Comment 9

[Stephen P. Morse]

Fix checked in.

Comment 10

[sairuh (rarely reading bugmail)]

works now. [need to remember to clear out any cookies from nytimes.com, as well
as turn off cookies, in order to test this case.] vrfy 2000.06.14.08-m17
commercial on all/all.