Bug 84697

Summary: [BlackBerry] Auth credentials set in private mode are reused in public mode.
Product: WebKit Reporter: Jason Liu <jasonliuwebkit>
Component: WebKit BlackBerryAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: joenotcharles, rwlbuis, staikos, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Jason Liu 2012-04-24 03:23:04 PDT
1. Turn on private browsing mode
2. Go to a site that uses Basic, Digest, or NTLM auth
3. Log in
4. Close the tab and turn off private browsing mode
5. Go back to the same site

Expected:
5. Will need to type username and password again

Observed:
5. Logs into site automatically
Comment 1 Jason Liu 2012-04-25 01:48:07 PDT
Created attachment 138761 [details]
Patch
Comment 2 Joe Mason 2012-04-25 07:03:10 PDT
Looks good to me.
Comment 3 Jason Liu 2012-04-27 22:58:10 PDT
r?
Comment 4 Antonio Gomes 2012-04-28 06:39:33 PDT
Comment on attachment 138761 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=138761&action=review

> Source/WebCore/ChangeLog:11
> +        Add setPrivateMode function for CredentialStorage.
> +
> +        We have to change Private Browsering to test, so have to write a manual test case.
> +        Test: ManualTests/blackberry/http-auth-private-mode-changed.html

you could explain better why the change here

> Source/WebCore/platform/network/CredentialStorage.cpp:166
> +void CredentialStorage::setPrivateMode(const bool mode)
> +{
> +    if (!mode)
> +        protectionSpaceToCredentialMap().clear();
> +}

so here you delete all credentials you have, even the ones before entering private mode?
Comment 5 Antonio Gomes 2012-04-28 06:40:29 PDT
Comment on attachment 138761 [details]
Patch

Also, how do other browsers behavior? do they need this code?

r- due to the poor changelog description, and the two open questions.
Comment 6 Jason Liu 2012-05-06 20:37:15 PDT
(In reply to comment #4)
> (From update of attachment 138761 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=138761&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        Add setPrivateMode function for CredentialStorage.
> > +
> > +        We have to change Private Browsering to test, so have to write a manual test case.
> > +        Test: ManualTests/blackberry/http-auth-private-mode-changed.html
> 
> you could explain better why the change here
> 
I will add comments.
> > Source/WebCore/platform/network/CredentialStorage.cpp:166
> > +void CredentialStorage::setPrivateMode(const bool mode)
> > +{
> > +    if (!mode)
> > +        protectionSpaceToCredentialMap().clear();
> > +}
> 
> so here you delete all credentials you have, even the ones before entering private mode?

Yes. It is like cookies.
And FireFox does like this, too. Can't find chrome's private mode.
Comment 7 Antonio Gomes 2012-05-06 21:07:02 PDT
> > > Source/WebCore/platform/network/CredentialStorage.cpp:166
> > > +void CredentialStorage::setPrivateMode(const bool mode)
> > > +{
> > > +    if (!mode)
> > > +        protectionSpaceToCredentialMap().clear();
> > > +}
> > 
> > so here you delete all credentials you have, even the ones before entering private mode?
> 
> Yes. It is like cookies.
> And FireFox does like this, too. Can't find chrome's private mode.

ctrl+shitf+n :)
Comment 8 Jason Liu 2012-05-06 21:12:34 PDT
(In reply to comment #7)
> > > > Source/WebCore/platform/network/CredentialStorage.cpp:166
> > > > +void CredentialStorage::setPrivateMode(const bool mode)
> > > > +{
> > > > +    if (!mode)
> > > > +        protectionSpaceToCredentialMap().clear();
> > > > +}
> > > 
> > > so here you delete all credentials you have, even the ones before entering private mode?
> > 
> > Yes. It is like cookies.
> > And FireFox does like this, too. Can't find chrome's private mode.
> 
> ctrl+shitf+n :)

I will try this. Thank you :)
Comment 9 Jason Liu 2012-05-06 22:24:39 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > > > > Source/WebCore/platform/network/CredentialStorage.cpp:166
> > > > > +void CredentialStorage::setPrivateMode(const bool mode)
> > > > > +{
> > > > > +    if (!mode)
> > > > > +        protectionSpaceToCredentialMap().clear();
> > > > > +}
> > > > 
> > > > so here you delete all credentials you have, even the ones before entering private mode?
> > > 
> > > Yes. It is like cookies.
> > > And FireFox does like this, too. Can't find chrome's private mode.
> > 
> > ctrl+shitf+n :)
> 
> I will try this. Thank you :)

Google's Chrome pop a new window for private mode. It is different from our browser.
Comment 10 Jason Liu 2012-05-06 22:37:21 PDT
Created attachment 140478 [details]
Patch
Comment 11 Rob Buis 2012-05-07 08:27:50 PDT
Comment on attachment 140478 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=140478&action=review

Still some stuff to clean up.

> Source/WebCore/ChangeLog:10
> +        Now, we only save credentials in memery and CredentialBackingStore isn't enabled.

Typo memery -> memory

> Source/WebCore/ChangeLog:13
> +        We have to change Private Browsering to test, so have to write a manual test case.

Private Browsing.

> Source/WebCore/platform/network/CredentialStorage.cpp:162
> +void CredentialStorage::setPrivateMode(const bool mode)

No need for const bool.
Comment 12 Jason Liu 2012-05-08 02:09:08 PDT
Created attachment 140694 [details]
Patch
Comment 13 Rob Buis 2012-05-08 06:53:32 PDT
Comment on attachment 140694 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=140694&action=review

Looks good.

> Source/WebKit/blackberry/ChangeLog:10
> +        We have to change Private Browsering to test, so have to write a manual test case.

Browsing :)
Comment 14 Jason Liu 2012-05-08 19:17:55 PDT
Created attachment 140852 [details]
Patch
Comment 15 Jason Liu 2012-05-08 19:22:02 PDT
Created attachment 140853 [details]
Patch
Comment 16 Rob Buis 2012-05-08 19:23:11 PDT
Comment on attachment 140853 [details]
Patch

Looks good.
Comment 17 Jason Liu 2012-05-08 19:25:59 PDT
(In reply to comment #16)
> (From update of attachment 140853 [details])
> Looks good.

Thank you. :)
Comment 18 WebKit Review Bot 2012-05-08 20:34:30 PDT
Comment on attachment 140853 [details]
Patch

Clearing flags on attachment: 140853

Committed r116488: <http://trac.webkit.org/changeset/116488>
Comment 19 WebKit Review Bot 2012-05-08 20:34:36 PDT
All reviewed patches have been landed.  Closing bug.