Bug 84697 – [BlackBerry] Auth credentials set in private mode are reused in public mode.

• Component: WebKit BlackBerry
• Status: RESOLVED FIXED

[Jason Liu]

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

[Jason Liu]

Created attachment 138761 [details]
Patch

[Joe Mason]

Looks good to me.

[Jason Liu]

r?

[Antonio Gomes]

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?

[Antonio Gomes]

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.

[Jason Liu]

(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.

[Antonio Gomes]

> > > 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 :)

[Jason Liu]

(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 :)

[Jason Liu]

(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.

[Jason Liu]

Created attachment 140478 [details]
Patch

[Rob Buis]

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.

[Jason Liu]

Created attachment 140694 [details]
Patch

[Rob Buis]

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 :)

[Jason Liu]

Created attachment 140852 [details]
Patch

[Jason Liu]

Created attachment 140853 [details]
Patch

[Rob Buis]

Comment on attachment 140853 [details]
Patch

Looks good.

[Jason Liu]

(In reply to comment #16)
> (From update of attachment 140853 [details])
> Looks good.

Thank you. :)

[WebKit Review Bot]

Comment on attachment 140853 [details]
Patch

Clearing flags on attachment: 140853

Committed r116488: <http://trac.webkit.org/changeset/116488>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.