soc.2008.masterpassword: 7819ad38: Implemented a password caching system to...

scrouaf at soc.pidgin.im scrouaf at soc.pidgin.im
Sat Aug 16 02:10:49 EDT 2008


-----------------------------------------------------------------
Revision: 7819ad38545e6988a5f8d898ab797c6521cd8965
Ancestor: c679068b3d34ff6ccb44f7902636b39dea2336ee
Author: scrouaf at soc.pidgin.im
Date: 2008-08-16T06:07:22
Branch: im.pidgin.soc.2008.masterpassword
URL: http://d.pidgin.im/viewmtn/revision/info/7819ad38545e6988a5f8d898ab797c6521cd8965

Modified files:
        libpurple/account.c libpurple/account.h libpurple/keyring.c
        libpurple/plugins/keyrings/gnomekeyring.c
        libpurple/plugins/keyrings/internalkeyring.c
        pidgin/gtkaccount.c

ChangeLog: 

Implemented a password caching system to limit problems linked to the
async nature of keyrings.
Some work on connection (which seems to work alright) and on password 
dialogs and account preferences (especially the remember password checkbox)
which are still partly broken.

-------------- next part --------------
============================================================
--- libpurple/account.c	f728af63918e67cceb0657cb4dfb63bac4f0e92d
+++ libpurple/account.c	cdc8bc81e795ab250749fdd8a85a0176efdac36a
@@ -84,6 +84,12 @@ typedef struct
 	PurpleAccountRequestAuthorizationCb deny_cb;
 } PurpleAccountRequestInfo;
 
+typedef struct
+{
+	gpointer cb;
+	gpointer data;
+} CbInfo;
+
 static PurpleAccountUiOps *account_ui_ops = NULL;
 
 static GList   *accounts = NULL;
@@ -106,6 +112,9 @@ static void request_password_ok_cb_conti
        gchar * password, GError * error, gpointer data);
 static void request_password_ok_cb_continue(const PurpleAccount * account, 
 	GError * error,	gpointer data);
+
+static void purple_account_get_password_async_finish(PurpleAccount * account,
+	char * password, GError * error, gpointer data);
 /*********************************************************************
  * Writing to disk                                                   *
  *********************************************************************/
@@ -396,8 +405,10 @@ account_to_xmlnode(PurpleAccount *accoun
 
 	if (purple_account_get_remember_password(account))
 	{
-		purple_debug_info("accounts", "Exporting password for account %s.\n",
-			purple_account_get_username(account));
+		purple_debug_info("accounts", "Exporting password for account %s (%s).\n",
+			purple_account_get_username(account),
+			purple_account_get_protocol_id(account));
+
 		purple_keyring_export_password(account, &keyring_id, 
 			&mode, &data, &error, &destroy);
 
@@ -409,7 +420,6 @@ account_to_xmlnode(PurpleAccount *accoun
 				error->message);
 
 		} else {
-
 			child = xmlnode_new_child(node, "password");
 			xmlnode_set_attrib(child, "keyring_id", keyring_id);
 			xmlnode_set_attrib(child, "mode", mode);
@@ -1166,18 +1176,14 @@ request_password_ok_cb(PurpleAccount *ac
 		return;
 	}
 
-	if(remember)
-		purple_account_set_remember_password(account, TRUE);
-	else
-		purple_account_set_remember_password(account, FALSE);
+	if(!remember)
+		purple_keyring_set_password_async(account, NULL, NULL, NULL, NULL);
 
-#if 0
-	purple_account_set_password_async(account, g_strdup(entry), g_free,
-			request_password_ok_cb_continue, g_strdup(entry));
-#else
+	purple_account_set_remember_password(account, remember);
+
 	purple_account_set_password(account, entry);
 	purple_connection_new(account, FALSE, entry);
-#endif
+
 }
 
 static void
@@ -1628,18 +1634,17 @@ purple_account_set_password(PurpleAccoun
 void
 purple_account_set_password(PurpleAccount *account, const char *password)
 {
+	schedule_accounts_save();
+
 	g_return_if_fail(account != NULL);
 
 	if (account->password != NULL)
 		g_free(account->password);
 
-	if (purple_account_get_remember_password(account) == FALSE)
-		account->password = g_strdup(password);
+	account->password = g_strdup(password);
 
-	else
+	if (purple_account_get_remember_password(account) == TRUE)
 		purple_keyring_set_password_sync(account, password);
-
-	schedule_accounts_save();
 }
 
 void 
@@ -1655,6 +1660,10 @@ purple_account_set_password_async(Purple
 	 */
 	if(account != NULL) {
 
+		if (account->password != NULL)
+			g_free(account->password);
+		account->password = g_strdup(password);
+
 		if (purple_account_get_remember_password(account) == FALSE) {
 
 			account->password = g_strdup(password);
@@ -1750,18 +1759,8 @@ purple_account_set_remember_password(Pur
 {
 	g_return_if_fail(account != NULL);
 
-	if (account->remember_pass != value) {
-		purple_debug_info("accounts",
-			"Setting remember_password for account %s to %s.\n",
-			purple_account_get_username(account),
-			(value)?"TRUE":"FALSE");
+	account->remember_pass = value;
 
-		account->remember_pass = value;
-
-		if (value == FALSE)
-			purple_keyring_set_password_sync(account, NULL);
-
-	}
 	schedule_accounts_save();
 }
 
@@ -2077,20 +2076,26 @@ const char *
 
 /* XXX will be replaced by the async code in 3.0 */
 const char *
-purple_account_get_password(const PurpleAccount *account)
+purple_account_get_password(PurpleAccount *account)
 {
 	g_return_val_if_fail(account != NULL, NULL);
 
-	if (account->password != NULL) {
-	
-		purple_debug_info("keyring", "password was read from stored\n");
-		return account->password;
-	
+	if (account->password == NULL) {
+		purple_debug_info("accounts",
+			"Reading password for account %s (%s) from sync keyring.\n",
+			purple_account_get_username(account),
+			purple_account_get_protocol_id(account));
 
+		account->password = g_strdup(purple_keyring_get_password_sync(account));
+
 	} else {
-		purple_debug_info("keyring", "reading password from keyring\n");	
-		return purple_keyring_get_password_sync(account);
+		purple_debug_info("accounts",
+			"Reading password for account %s (%s) from cached.\n",
+			purple_account_get_username(account),
+			purple_account_get_protocol_id(account));
 	}
+
+	return account->password;
 }
 
 void
@@ -2098,11 +2103,57 @@ purple_account_get_password_async(Purple
 				  PurpleKeyringReadCallback cb,
 				  gpointer data)
 {
-	purple_keyring_get_password_async(account, cb, data);
+	char * password;
+
+	CbInfo * info = g_new0(CbInfo,1);
+	info->cb = cb;
+	info->data = data;
+
+	if (account->password != NULL) {
+		purple_debug_info("accounts",
+			"Reading password for account %s (%s) from cached (async).\n",
+			purple_account_get_username(account),
+			purple_account_get_protocol_id(account));
+		cb(account, account->password, NULL, data);
+
+	} else {
+		purple_debug_info("accounts",
+			"Reading password for account %s (%s) from async keyring.\n",
+			purple_account_get_username(account),
+			purple_account_get_protocol_id(account));
+		purple_keyring_get_password_async(account, 
+			purple_account_get_password_async_finish, data);
+	}
 }
 
+static void
+purple_account_get_password_async_finish(PurpleAccount * account,
+					 char * password,
+					 GError * error,
+					 gpointer data)
+{
+	CbInfo * info;
+	PurpleKeyringReadCallback cb;
 
+	purple_debug_info("accounts",
+		"Read password for account %s (%s) from async keyring.\n",
+		purple_account_get_username(account),
+		purple_account_get_protocol_id(account));
 
+	info = data;
+
+	g_return_if_fail(account != NULL);
+	g_return_if_fail(info != NULL);
+
+	account->password = g_strdup(password);
+
+	cb = info->cb;
+	if (cb != NULL)
+		cb(account, password, error, info->data);
+
+	g_free(data);
+}
+
 const char *
 purple_account_get_alias(const PurpleAccount *account)
 {
============================================================
--- libpurple/account.h	dbc81839d1ff72a772d742057890390cf188de03
+++ libpurple/account.h	3ab4e92017f078e6e951d1f86d8c4f0f89a6ec72
@@ -572,7 +572,7 @@ const char *purple_account_get_username(
  *
  * @return The password.
  */
-const char *purple_account_get_password(const PurpleAccount *account)  __attribute__ ((deprecated));
+const char *purple_account_get_password(PurpleAccount *account)  __attribute__ ((deprecated));
 
 /**
  * Reads the password for the account and passes it to the callback
============================================================
--- libpurple/keyring.c	6715ee72ed55de37ee680d83c0406786c60a1eee
+++ libpurple/keyring.c	8895b9a5a71036218c0582262962d3b9baea9ff7
@@ -1046,31 +1046,25 @@ purple_keyring_get_password_sync(const P
 	PurpleKeyringReadSync read;
 	const PurpleKeyring * inuse;
 
-	if (account == NULL) {
-		return NULL;
+	g_return_val_if_fail(account != NULL, NULL);
 
-	} else {
+	purple_debug_info("keyring (sync)",
+		"Reading password for account %s (%s)",
+		purple_account_get_username(account),
+		purple_account_get_protocol_id(account));
 
-		inuse = purple_keyring_get_inuse();
+	inuse = purple_keyring_get_inuse();
 
-		if (inuse == NULL) {
+	if (inuse == NULL) {
+		return NULL;
 
-			return NULL;
+	} else {
+		read = purple_keyring_get_read_sync(inuse);
 
-		} else {
-
-			read = purple_keyring_get_read_sync(inuse);
-
-			if (read == NULL){
-
-				return NULL;
-
-			} else {
-
-				return read(account);
-
-			}
-		}
+		if (read == NULL)
+			return NULL;
+		else
+			return read(account);
 	}
 }
 
@@ -1084,22 +1078,24 @@ purple_keyring_set_password_sync(PurpleA
 	PurpleKeyringSaveSync save;
 	const PurpleKeyring * inuse;
 
-	if (account != NULL) {
+	purple_debug_info("keyring (sync)",
+		"Setting password for account %s (%s)",
+		purple_account_get_username(account),
+		purple_account_get_protocol_id(account));
 
-		inuse = purple_keyring_get_inuse();
+	g_return_if_fail(account != NULL);
 
-		if (inuse != NULL) {
+	inuse = purple_keyring_get_inuse();
+	if (inuse != NULL) {
+		save = purple_keyring_get_save_sync(inuse);
 
-			save = purple_keyring_get_save_sync(inuse);
+		if (save != NULL)
+			return save(account, password);
+	}
 
-			if (save != NULL){
+	/* schedule account save */
+	purple_account_set_password(NULL, NULL);
 
-				return save(account, password);
-
-			}
-		}
-	}
-
 	return;
 }
 
============================================================
--- libpurple/plugins/keyrings/gnomekeyring.c	86a221688e2f9cd8701d39223940d97031a74d5d
+++ libpurple/plugins/keyrings/gnomekeyring.c	65ab054c4682c4ed72dd91be818ecf2dfea73b96
@@ -191,7 +191,7 @@ gkp_save(PurpleAccount * account,
 	storage->cb = cb;
 	storage->user_data = data;
 
-	if(password != NULL || *password != '\O') {
+	if(password != NULL && *password != '\O') {
 
 		purple_debug_info("Gnome keyring plugin",
 			"Updating password for account %s (%s).\n",
@@ -330,7 +330,7 @@ gkp_save_sync(PurpleAccount * account,
 {
 	const char * name;
 
-	if(password != NULL || *password != '\O') {
+	if(password != NULL && *password != '\O') {
 
 		name =g_strdup_printf("pidgin-%s", purple_account_get_username(account)),
 
============================================================
--- libpurple/plugins/keyrings/internalkeyring.c	e2de871d9e1a339a0d0fe14ba80b277c5e80bde0
+++ libpurple/plugins/keyrings/internalkeyring.c	54987c1ac094525a5a1bc5276041c1c625416855
@@ -176,7 +176,7 @@ internal_keyring_save_sync(PurpleAccount
 
 	ACTIVATE();
 
-	if (password == NULL || *password != '\O') {
+	if (password == NULL || *password == '\O') {
 		g_hash_table_remove(internal_keyring_passwords, account);
 		purple_debug_info("Internal Keyring (sync)", 
 			"Password for %s (%s) was deleted.\n",
@@ -256,7 +256,13 @@ internal_keyring_export_password(PurpleA
 	purple_debug_info("Internal keyring",
 		"Exporting password");
 
-	password = GET_PASSWORD(account);
+	/* we're using this rather than GET_PASSWORD(),
+	 * because on account creation, the account might be
+	 * exported before the password is known. This would
+	 * lead to exporting uninitialised data, which 
+	 * we obviously don't want.
+	 */
+	password = purple_account_get_password(account);
 
 	if (password == NULL) {
 		return FALSE;
============================================================
--- pidgin/gtkaccount.c	96ae00e327f7edd1349b84e12e948304ce1e36e8
+++ pidgin/gtkaccount.c	8c71060d98085bec815d7e9478c4186abdfcf2ac
@@ -144,8 +144,6 @@ static void set_account(GtkListStore *st
 static void add_account_to_liststore(PurpleAccount *account, gpointer user_data);
 static void set_account(GtkListStore *store, GtkTreeIter *iter,
 						  PurpleAccount *account, GdkPixbuf *global_buddyicon);
-static void add_login_options_continue(PurpleAccount * account, 
-	gchar * password, GError * error, gpointer user_data);
 
 /**************************************************************************
  * Add/Modify Account dialog
@@ -392,33 +390,9 @@ update_editable(PurpleConnection *gc, Ac
 		gtk_widget_set_sensitive((GtkWidget *)l->data, set);
 }
 
-typedef struct _AddLoginOptionsCalbackData {
-	AccountPrefsDialog *dialog;
-	GtkWidget *parent;
-} AddLoginOptionsCallbackData;
-
 static void
 add_login_options(AccountPrefsDialog *dialog, GtkWidget *parent)
 {
-
-	AddLoginOptionsCallbackData *data;
-	data = g_new(AddLoginOptionsCallbackData, 1);
-
-	data->dialog = dialog;
-	data->parent = parent;
-
-	purple_account_get_password_async(dialog->account, add_login_options_continue, data);
-}
-
-static void
-add_login_options_continue(PurpleAccount * account,
-			   gchar * password,
-			   GError * error,
-			   gpointer user_data)
-{
-	AddLoginOptionsCallbackData * data = user_data;
-	AccountPrefsDialog *dialog = data->dialog;
-	GtkWidget *parent = data->parent;
 	GtkWidget *frame;
 	GtkWidget *hbox;
 	GtkWidget *vbox;
@@ -587,10 +561,10 @@ add_login_options_continue(PurpleAccount
 
 	/* Set the fields. */
 	if (dialog->account != NULL) {
+		if (purple_account_get_password(dialog->account))
+			gtk_entry_set_text(GTK_ENTRY(dialog->password_entry),
+							   purple_account_get_password(dialog->account));
 
-		if (password)
-			gtk_entry_set_text(GTK_ENTRY(dialog->password_entry), password);
-
 		gtk_toggle_button_set_active(
 				GTK_TOGGLE_BUTTON(dialog->remember_pass_check),
 				purple_account_get_remember_password(dialog->account));
@@ -1212,6 +1186,7 @@ ok_account_prefs_cb(GtkWidget *w, Accoun
 	char *tmp;
 	gboolean new_acct = FALSE, icon_change = FALSE;
 	PurpleAccount *account;
+	gboolean remember;
 
 	/* Build the username string. */
 	username = g_strdup(gtk_entry_get_text(GTK_ENTRY(dialog->screenname_entry)));
@@ -1317,10 +1292,12 @@ ok_account_prefs_cb(GtkWidget *w, Accoun
 
 
 	/* Remember Password */
-	purple_account_set_remember_password(account,
-			gtk_toggle_button_get_active(
-					GTK_TOGGLE_BUTTON(dialog->remember_pass_check)));
+	remember = gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(dialog->remember_pass_check));
+	if(!remember)
+		purple_keyring_set_password_async(account, NULL, NULL, NULL, NULL);
 
+	purple_account_set_remember_password(account, remember);
+
 	/* Check Mail */
 	if (dialog->prpl_info && dialog->prpl_info->options & OPT_PROTO_MAIL_CHECK)
 		purple_account_set_check_mail(account,
@@ -1331,15 +1308,17 @@ ok_account_prefs_cb(GtkWidget *w, Accoun
 	value = gtk_entry_get_text(GTK_ENTRY(dialog->password_entry));
 
 	/*
-	 * The function most likely needs to be split in two here.
+	 * We set the password if this is a new account because new accounts
+	 * will be set to online, and if the user has entered a password into
+	 * the account editor (but has not checked the 'save' box), then we
+	 * don't want to prompt them.
 	 */
 	if ((purple_account_get_remember_password(account) || new_acct) && (*value != '\0'))
-/* 		purple_account_set_password_async(account, g_strdup(value), g_free, NULL, NULL); */
 		purple_account_set_password(account, value);
 	else
-/*		purple_account_set_password_async(account, NULL, NULL, NULL,NULL); */
 		purple_account_set_password(account, NULL);
 
+
 	purple_account_set_username(account, username);
 	g_free(username);
 


More information about the Commits mailing list