Bug 27560 - [Gtk] Password is saved into gnome-keyring even if auth. fails
: [Gtk] Password is saved into gnome-keyring even if auth. fails
Status: RESOLVED FIXED
: WebKit
WebKit Gtk
: 528+ (Nightly build)
: PC Linux
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2009-07-22 13:40 PST by
Modified: 2009-07-24 07:36 PST (History)


Attachments
webkit-bug-27560-fix-authentication.patch (6.12 KB, patch)
2009-07-24 03:31 PST, Priit Laes (IRC: plaes)
xan.lopez: review-
Review Patch | Details | Formatted Diff | Diff
webkit-bug-27560-fix-authentication-v2.patch (5.50 KB, patch)
2009-07-24 04:34 PST, Priit Laes (IRC: plaes)
xan.lopez: review+
Review Patch | Details | Formatted Diff | Diff
webkit-bug-27560-fix-authentication-v3.patch (5.55 KB, patch)
2009-07-24 05:15 PST, Priit Laes (IRC: plaes)
xan.lopez: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-07-22 13:40:25 PST
When choosing to save http authentication into gnome-keyring, it is always saved even if authentication fails.
This also means that it can overwrite previously saved password with wrong pass.
------- Comment #1 From 2009-07-24 03:31:04 PST -------
Created an attachment (id=33422) [details]
webkit-bug-27560-fix-authentication.patch
------- Comment #2 From 2009-07-24 03:45:41 PST -------
(From update of attachment 33422 [details])
>  2009-07-23  Jan Michael Alonzo  <jmalonzo@webkit.org>
>  
>          Reviewed by Eric Seidel.
> diff --git a/WebKit/gtk/webkit/webkitsoupauthdialog.c b/WebKit/gtk/webkit/webkitsoupauthdialog.c
> index d5ca79c..eeaaa63 100644
> --- a/WebKit/gtk/webkit/webkitsoupauthdialog.c
> +++ b/WebKit/gtk/webkit/webkitsoupauthdialog.c
> @@ -90,12 +90,16 @@ typedef struct _WebKitAuthData {
>      GtkWidget* passwordEntry;
>  #if USE(GNOMEKEYRING)
>      GtkWidget* checkButton;
> +    char *username;
> +    char *password;
>  #endif
>  } WebKitAuthData;
>  
>  static void free_authData(WebKitAuthData* authData)
>  {
>      g_object_unref(authData->msg);
> +    g_free(authData->username);
> +    g_free(authData->password);

Should be protected by #if USE(GNOMEKEYRING)

>      g_slice_free(WebKitAuthData, authData);
>  }
>  
> @@ -104,47 +108,52 @@ static void set_password_callback(GnomeKeyringResult result, guint32 val, gpoint
>  {
>      /* Dummy callback, gnome_keyring_set_network_password does not accept a NULL one */
>  }
> -#endif
>  
> -static void response_callback(GtkDialog* dialog, gint response_id, WebKitAuthData* authData)
> +static void save_password_callback(SoupMessage* msg, WebKitAuthData* authData)
>  {
> -    const char* login;
> -    const char* password;
> -#if USE(GNOMEKEYRING)
>      SoupURI* uri;
> -    gboolean storePassword;
> +
> +    if (msg->status_code >= 400)
> +        return;
> +

Mmm, are you sure anything < 400 means we should save the password?

>  
> @@ -278,7 +287,7 @@ static void show_auth_dialog(WebKitAuthData* authData, const char* login, const
>      gtk_box_pack_start (GTK_BOX (vbox), rememberBox,
>                          FALSE, FALSE, 0);
>  
> -    checkButton = gtk_check_button_new_with_label(_("_Remember password"));
> +    checkButton = gtk_check_button_new_with_mnemonic(_("_Remember password"));
>      if (login && password)
>          gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(checkButton), TRUE);
>      gtk_label_set_line_wrap(GTK_LABEL(gtk_bin_get_child(GTK_BIN(checkButton))), TRUE);

This seems unrelated to this bug (although it looks OK, please open a new bug with it).

Marking r- for now, waiting for the updated patch :)
------- Comment #3 From 2009-07-24 04:34:41 PST -------
Created an attachment (id=33426) [details]
webkit-bug-27560-fix-authentication-v2.patch
------- Comment #4 From 2009-07-24 04:38:52 PST -------
(From update of attachment 33426 [details])
authData is leaked when the authentication fails, but other than that looks good to me.
------- Comment #5 From 2009-07-24 05:15:36 PST -------
Created an attachment (id=33428) [details]
webkit-bug-27560-fix-authentication-v3.patch
------- Comment #6 From 2009-07-24 05:26:56 PST -------
(From update of attachment 33428 [details])
OK.
------- Comment #7 From 2009-07-24 07:36:19 PST -------
Landed in r46350, closing.