Bug 177738 – [SOUP] Default kerberos authentication credentials are used in ephemeral (private) mode

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Tomas Popela]

If the kerberos credentials are set and there is a valid ticket, then it is used even in the ephemeral (private) mode. It's not expected as we should show an authentication dialog instead. Firefox was doing the same until https://bugzilla.mozilla.org/show_bug.cgi?id=1248564 was fixed. Chromium still needs to be fixed.

[Tomas Popela]

Created attachment 322364 [details]
Patch

[Michael Catanzaro]

Comment on attachment 322364 [details]
Patch

It looks good to me, but ask Carlos Garcia to review this, because since it's possible to have an ephemeral web view with a non-ephemeral web context, I'm not certain if ephemeral web views actually use ephemeral sessions IDs. Hopefully they do.

[Carlos Garcia Campos]

Comment on attachment 322364 [details]
Patch

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

> Source/WebCore/platform/network/soup/NetworkStorageSessionSoup.cpp:86
> +    globalSessionMap().add(sessionID, std::make_unique<NetworkStorageSession>(sessionID, std::make_unique<SoupNetworkSession>(nullptr, sessionID)));

I think we could change the order of the construct parameters t avoid this nullptr, since when providing cookies we always provide a session id too.

> Source/WebCore/platform/network/soup/SoupNetworkSession.h:51
> -    explicit SoupNetworkSession(SoupCookieJar* = nullptr);
> +    explicit SoupNetworkSession(SoupCookieJar* = nullptr, PAL::SessionID = PAL::SessionID::emptySessionID());

why is empty session the default and not default session ID? what's the difference? I guess i doesn't mater as long as is ephemeral is false in both cases.

[Tomas Popela]

(In reply to Carlos Garcia Campos from comment #3)
> > Source/WebCore/platform/network/soup/NetworkStorageSessionSoup.cpp:86
> > +    globalSessionMap().add(sessionID, std::make_unique<NetworkStorageSession>(sessionID, std::make_unique<SoupNetworkSession>(nullptr, sessionID)));
> 
> I think we could change the order of the construct parameters t avoid this
> nullptr, since when providing cookies we always provide a session id too.

Yes, that would be definitely better.
 
> > Source/WebCore/platform/network/soup/SoupNetworkSession.h:51
> > -    explicit SoupNetworkSession(SoupCookieJar* = nullptr);
> > +    explicit SoupNetworkSession(SoupCookieJar* = nullptr, PAL::SessionID = PAL::SessionID::emptySessionID());
> 
> why is empty session the default and not default session ID? what's the
> difference? I guess i doesn't mater as long as is ephemeral is false in both
> cases.

Yeah, both values are not ephemeral. But I will change it to the default session ID as that sounds some appropriate (and is used more across the codebase).

[Tomas Popela]

Committed r222706: <http://trac.webkit.org/changeset/222706>