Bug 167584 - Allow to use stored credentials also for downloads started by DownloadManager::startDownload
Summary: Allow to use stored credentials also for downloads started by DownloadManager...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2017-01-30 05:27 PST by Carlos Garcia Campos
Modified: 2017-02-14 05:10 PST (History)
4 users (show)

See Also:


Attachments
Patch (2.13 KB, patch)
2017-01-30 05:39 PST, Carlos Garcia Campos
mcatanzaro: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2017-01-30 05:27:40 PST
I don't know why this is the default in other ports, but I don't think it's what we want in GTK+.
Comment 1 Carlos Garcia Campos 2017-01-30 05:39:24 PST
Created attachment 300100 [details]
Patch
Comment 2 Michael Catanzaro 2017-01-30 06:24:30 PST
Comment on attachment 300100 [details]
Patch

r=me but please ask Alex Christensen before landing if you can do this cross-platform. Either this must be broken on Mac too, or he might have a different suggestion.
Comment 3 Carlos Garcia Campos 2017-01-30 22:04:42 PST
(In reply to comment #2)
> Comment on attachment 300100 [details]
> Patch
> 
> r=me but please ask Alex Christensen before landing if you can do this
> cross-platform. Either this must be broken on Mac too, or he might have a
> different suggestion.

Alex? :-)
Comment 4 Alex Christensen 2017-02-01 12:49:43 PST
Comment on attachment 300100 [details]
Patch

This should definitely have a test.  If we allow stored credentials in an ephemeral session, won't it just use the credentials in the CredentialStorage for that session?
Comment 5 Carlos Garcia Campos 2017-02-01 22:24:44 PST
(In reply to comment #4)
> Comment on attachment 300100 [details]
> Patch
> 
> This should definitely have a test.  If we allow stored credentials in an
> ephemeral session, won't it just use the credentials in the
> CredentialStorage for that session?

But we don't allow it. The patch allows stored credentials only for the default session, the problem is that PendingDownload is created with the default parameter for allow stored credentials unconditionally, which is to no allow them.
Comment 6 Michael Catanzaro 2017-02-07 08:01:12 PST
So this is good then...?
Comment 7 Alex Christensen 2017-02-13 23:55:29 PST
Comment on attachment 300100 [details]
Patch

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

> Source/WebKit2/NetworkProcess/Downloads/DownloadManager.cpp:64
> +#if PLATFORM(GTK)
> +    // The GTK+ port wants to allow using stored credentials also for pending downloads.
> +    parameters.allowStoredCredentials = sessionID.isEphemeral() ? DoNotAllowStoredCredentials : AllowStoredCredentials;
> +#endif

Let's make this the case on all ports.
Comment 8 Carlos Garcia Campos 2017-02-14 05:10:17 PST
Committed r212291: <http://trac.webkit.org/changeset/212291>