Bug 27560

Summary: [Gtk] Password is saved into gnome-keyring even if auth. fails
Product: WebKit Reporter: Priit Laes (IRC: plaes) <plaes>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
webkit-bug-27560-fix-authentication.patch
xan.lopez: review-
webkit-bug-27560-fix-authentication-v2.patch
xan.lopez: review+
webkit-bug-27560-fix-authentication-v3.patch xan.lopez: review+

Priit Laes (IRC: plaes)
Reported 2009-07-22 13:40:25 PDT
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.
Attachments
webkit-bug-27560-fix-authentication.patch (6.12 KB, patch)
2009-07-24 03:31 PDT, Priit Laes (IRC: plaes)
xan.lopez: review-
webkit-bug-27560-fix-authentication-v2.patch (5.50 KB, patch)
2009-07-24 04:34 PDT, Priit Laes (IRC: plaes)
xan.lopez: review+
webkit-bug-27560-fix-authentication-v3.patch (5.55 KB, patch)
2009-07-24 05:15 PDT, Priit Laes (IRC: plaes)
xan.lopez: review+
Priit Laes (IRC: plaes)
Comment 1 2009-07-24 03:31:04 PDT
Created attachment 33422 [details] webkit-bug-27560-fix-authentication.patch
Xan Lopez
Comment 2 2009-07-24 03:45:41 PDT
Comment on attachment 33422 [details] webkit-bug-27560-fix-authentication.patch > 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 :)
Priit Laes (IRC: plaes)
Comment 3 2009-07-24 04:34:41 PDT
Created attachment 33426 [details] webkit-bug-27560-fix-authentication-v2.patch
Xan Lopez
Comment 4 2009-07-24 04:38:52 PDT
Comment on attachment 33426 [details] webkit-bug-27560-fix-authentication-v2.patch authData is leaked when the authentication fails, but other than that looks good to me.
Priit Laes (IRC: plaes)
Comment 5 2009-07-24 05:15:36 PDT
Created attachment 33428 [details] webkit-bug-27560-fix-authentication-v3.patch
Xan Lopez
Comment 6 2009-07-24 05:26:56 PDT
Comment on attachment 33428 [details] webkit-bug-27560-fix-authentication-v3.patch OK.
Xan Lopez
Comment 7 2009-07-24 07:36:19 PDT
Landed in r46350, closing.
Note You need to log in before you can comment on or make changes to this bug.