Bug 177738

Summary: [SOUP] Default kerberos authentication credentials are used in ephemeral (private) mode
Product: WebKit Reporter: Tomas Popela <tpopela>
Component: WebKitGTKAssignee: Tomas Popela <tpopela>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, bugs-noreply, buildbot, cgarcia, danw, gustavo, mcatanzaro
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch cgarcia: review+

Description Tomas Popela 2017-10-02 02:19:29 PDT
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.
Comment 1 Tomas Popela 2017-10-02 02:25:32 PDT
Created attachment 322364 [details]
Patch
Comment 2 Michael Catanzaro 2017-10-02 04:47:25 PDT
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.
Comment 3 Carlos Garcia Campos 2017-10-02 04:55:57 PDT
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.
Comment 4 Tomas Popela 2017-10-02 05:46:28 PDT
(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).
Comment 5 Tomas Popela 2017-10-02 05:55:32 PDT
Committed r222706: <http://trac.webkit.org/changeset/222706>