Bug 84697 - [BlackBerry] Auth credentials set in private mode are reused in public mode.
Summary: [BlackBerry] Auth credentials set in private mode are reused in public mode.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-24 03:23 PDT by Jason Liu
Modified: 2012-05-08 20:34 PDT (History)
5 users (show)

See Also:


Attachments
Patch (6.62 KB, patch)
2012-04-25 01:48 PDT, Jason Liu
no flags Details | Formatted Diff | Diff
Patch (6.80 KB, patch)
2012-05-06 22:37 PDT, Jason Liu
no flags Details | Formatted Diff | Diff
Patch (6.79 KB, patch)
2012-05-08 02:09 PDT, Jason Liu
no flags Details | Formatted Diff | Diff
Patch (6.78 KB, patch)
2012-05-08 19:17 PDT, Jason Liu
no flags Details | Formatted Diff | Diff
Patch (6.78 KB, patch)
2012-05-08 19:22 PDT, Jason Liu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.