GNOME.org
 

[network-manager-openvpn] properties: remove dead code and simplify password widget setup

commit 3f37d63cbdeb22383ce4dfff40e2feb617760e77
Author: Dan Williams <dcbw redhat com>
Date:   Wed Jul 13 15:01:15 2011 -0500

    properties: remove dead code and simplify password widget setup
    
    Since the secret agent (ie nm-applet) takes care of getting secrets
    for us (via the auth dialog), saving updated secrets, and deleting
    old ones, the VPN plugin doesn't need to do that itself anymore.
    So we get to delete a bunch of code that extracted and/or deleted
    secrets from the keyring since the agent handles all of that.  Only
    the auth dialog needs to talk to gnome-keyring anymore.

 properties/Makefile.am    |    7 +-
 properties/auth-helpers.c |  177 +++++----------------------------------------
 properties/auth-helpers.h |   10 +--
 properties/nm-openvpn.c   |   63 +---------------
 4 files changed, 26 insertions(+), 231 deletions(-)
---
diff --git a/properties/Makefile.am b/properties/Makefile.am
index fdb50a3..d1caf79 100644
--- a/properties/Makefile.am
+++ b/properties/Makefile.am
@@ -28,10 +28,9 @@ libnm_openvpn_properties_la_CFLAGS =                    \
         -DGNOMELOCALEDIR=\"$(datadir)/locale\"          \
         -DVERSION=\"$(VERSION)\"
 
-libnm_openvpn_properties_la_LIBADD =    \
-        $(GTK_LIBS)                     \
-        $(NM_LIBS)         \
-        $(top_builddir)/common-gnome/libnm-openvpn-common-gnome.la \
+libnm_openvpn_properties_la_LIBADD = \
+        $(GTK_LIBS) \
+        $(NM_LIBS) \
         $(top_builddir)/common/libnm-openvpn-common.la
 
 libnm_openvpn_properties_la_LDFLAGS =   \
diff --git a/properties/auth-helpers.c b/properties/auth-helpers.c
index 8bb6502..a52e727 100644
--- a/properties/auth-helpers.c
+++ b/properties/auth-helpers.c
@@ -34,14 +34,12 @@
 #include <errno.h>
 
 #include <glib/gi18n-lib.h>
-#include <gnome-keyring-memory.h>
 #include <nm-setting-connection.h>
 #include <nm-setting-8021x.h>
 
 #include "auth-helpers.h"
 #include "nm-openvpn.h"
 #include "src/nm-openvpn-service.h"
-#include "common-gnome/keyring-helpers.h"
 #include "common/utils.h"
 
 static void
@@ -51,17 +49,15 @@ show_password (GtkToggleButton *togglebutton, GtkEntry *password_entry)
 }
 
 static GtkWidget *
-fill_password (GtkBuilder *builder,
-               const char *widget_name,
-               NMConnection *connection,
-               gboolean priv_key_password)
+setup_secret_widget (GtkBuilder *builder,
+                     const char *widget_name,
+                     NMSettingVPN *s_vpn,
+                     const char *secret_key)
 {
-	NMSettingVPN *s_vpn;
+	NMSettingSecretFlags pw_flags = NM_SETTING_SECRET_FLAG_NONE;
 	GtkWidget *widget;
 	GtkWidget *show_passwords;
 	const char *tmp;
-	char *keyring_pw;
-	gboolean unused;
 
 	widget = GTK_WIDGET (gtk_builder_get_object (builder, widget_name));
 	g_assert (widget);
@@ -69,26 +65,13 @@ fill_password (GtkBuilder *builder,
 	show_passwords = GTK_WIDGET (gtk_builder_get_object (builder, "show_passwords"));
 	g_signal_connect (show_passwords, "toggled", G_CALLBACK (show_password), widget);
 
-	if (!connection)
-		return widget;
-
-	/* Grab from the connection first */
-	s_vpn = nm_connection_get_setting_vpn (connection);
 	if (s_vpn) {
-		tmp = nm_setting_vpn_get_secret (s_vpn, priv_key_password ? NM_OPENVPN_KEY_CERTPASS : NM_OPENVPN_KEY_PASSWORD);
-		if (tmp) {
+		tmp = nm_setting_vpn_get_secret (s_vpn, secret_key);
+		if (tmp)
 			gtk_entry_set_text (GTK_ENTRY (widget), tmp);
-			return widget;
-		}
-	}
 
-	/* If not the connection then from the keyring */
-	keyring_pw = keyring_helpers_lookup_secret (nm_connection_get_uuid (connection),
-	                                            priv_key_password ? NM_OPENVPN_KEY_CERTPASS : NM_OPENVPN_KEY_PASSWORD,
-	                                            &unused);
-	if (keyring_pw) {
-		gtk_entry_set_text (GTK_ENTRY (widget), keyring_pw);
-		gnome_keyring_memory_free (keyring_pw);
+		nm_setting_get_secret_flags (NM_SETTING (s_vpn), secret_key, &pw_flags, NULL);
+		g_object_set_data (G_OBJECT (widget), "flags", GUINT_TO_POINTER (pw_flags));
 	}
 
 	return widget;
@@ -138,14 +121,12 @@ tls_cert_changed_cb (GtkWidget *widget, GtkWidget *next_widget)
 static void
 tls_setup (GtkBuilder *builder,
            GtkSizeGroup *group,
-           NMConnection *connection,
+           NMSettingVPN *s_vpn,
            const char *prefix,
            GtkWidget *ca_chooser,
            ChangedCallback changed_cb,
            gpointer user_data)
 {
-	NMSettingVPN *s_vpn;
-	NMSettingSecretFlags pw_flags = NM_SETTING_SECRET_FLAG_NONE;
 	GtkWidget *widget, *cert, *key;
 	const char *value;
 	char *tmp;
@@ -163,7 +144,6 @@ tls_setup (GtkBuilder *builder,
 	                                   _("Choose your personal certificate..."));
 	g_signal_connect (G_OBJECT (cert), "selection-changed", G_CALLBACK (changed_cb), user_data);
 
-	s_vpn = nm_connection_get_setting_vpn (connection);
 	if (s_vpn) {
 		value = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_CERT);
 		if (value && strlen (value))
@@ -195,27 +175,20 @@ tls_setup (GtkBuilder *builder,
 
 	/* Fill in the private key password */
 	tmp = g_strdup_printf ("%s_private_key_password_entry", prefix);
-	widget = fill_password (builder, tmp, connection, TRUE);
+	widget = setup_secret_widget (builder, tmp, s_vpn, NM_OPENVPN_KEY_CERTPASS);
 	g_free (tmp);
 	gtk_size_group_add_widget (group, widget);
 	g_signal_connect (widget, "changed", G_CALLBACK (changed_cb), user_data);
-
-	if (s_vpn) {
-		nm_setting_get_secret_flags (NM_SETTING (s_vpn), NM_OPENVPN_KEY_CERTPASS, &pw_flags, NULL);
-		g_object_set_data (G_OBJECT (widget), "flags", GUINT_TO_POINTER (pw_flags));
-	}
 }
 
 static void
 pw_setup (GtkBuilder *builder,
           GtkSizeGroup *group, 
-          NMConnection *connection,
+          NMSettingVPN *s_vpn,
           const char *prefix,
           ChangedCallback changed_cb,
           gpointer user_data)
 {
-	NMSettingVPN *s_vpn;
-	NMSettingSecretFlags pw_flags = NM_SETTING_SECRET_FLAG_NONE;
 	GtkWidget *widget;
 	const char *value;
 	char *tmp;
@@ -225,7 +198,6 @@ pw_setup (GtkBuilder *builder,
 	g_free (tmp);
 	gtk_size_group_add_widget (group, widget);
 
-	s_vpn = nm_connection_get_setting_vpn (connection);
 	if (s_vpn) {
 		value = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_USERNAME);
 		if (value && strlen (value))
@@ -235,27 +207,21 @@ pw_setup (GtkBuilder *builder,
 
 	/* Fill in the user password */
 	tmp = g_strdup_printf ("%s_password_entry", prefix);
-	widget = fill_password (builder, tmp, connection, FALSE);
+	widget = setup_secret_widget (builder, tmp, s_vpn, NM_OPENVPN_KEY_PASSWORD);
 	g_free (tmp);
 	gtk_size_group_add_widget (group, widget);
 	g_signal_connect (widget, "changed", G_CALLBACK (changed_cb), user_data);
-
-	if (s_vpn) {
-		nm_setting_get_secret_flags (NM_SETTING (s_vpn), NM_OPENVPN_KEY_PASSWORD, &pw_flags, NULL);
-		g_object_set_data (G_OBJECT (widget), "flags", GUINT_TO_POINTER (pw_flags));
-	}
 }
 
 void
 tls_pw_init_auth_widget (GtkBuilder *builder,
                          GtkSizeGroup *group,
-                         NMConnection *connection,
+                         NMSettingVPN *s_vpn,
                          const char *contype,
                          const char *prefix,
                          ChangedCallback changed_cb,
                          gpointer user_data)
 {
-	NMSettingVPN *s_vpn;
 	GtkWidget *ca;
 	const char *value;
 	char *tmp;
@@ -288,7 +254,6 @@ tls_pw_init_auth_widget (GtkBuilder *builder,
 	                                   _("Choose a Certificate Authority certificate..."));
 	g_signal_connect (G_OBJECT (ca), "selection-changed", G_CALLBACK (changed_cb), user_data);
 
-	s_vpn = nm_connection_get_setting_vpn (connection);
 	if (s_vpn) {
 		value = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_CA);
 		if (value && strlen (value))
@@ -297,9 +262,9 @@ tls_pw_init_auth_widget (GtkBuilder *builder,
 
 	/* Set up the rest of the options */
 	if (tls)
-		tls_setup (builder, group, connection, prefix, ca, changed_cb, user_data);
+		tls_setup (builder, group, s_vpn, prefix, ca, changed_cb, user_data);
 	if (pw)
-		pw_setup (builder, group, connection, prefix, changed_cb, user_data);
+		pw_setup (builder, group, s_vpn, prefix, changed_cb, user_data);
 }
 
 #define SK_DIR_COL_NAME 0
@@ -308,11 +273,10 @@ tls_pw_init_auth_widget (GtkBuilder *builder,
 void
 sk_init_auth_widget (GtkBuilder *builder,
                      GtkSizeGroup *group,
-                     NMConnection *connection,
+                     NMSettingVPN *s_vpn,
                      ChangedCallback changed_cb,
                      gpointer user_data)
 {
-	NMSettingVPN *s_vpn;
 	GtkWidget *widget;
 	const char *value = NULL;
 	GtkListStore *store;
@@ -334,7 +298,6 @@ sk_init_auth_widget (GtkBuilder *builder,
 	                                   _("Choose an OpenVPN static key..."));
 	g_signal_connect (G_OBJECT (widget), "selection-changed", G_CALLBACK (changed_cb), user_data);
 
-	s_vpn = nm_connection_get_setting_vpn (connection);
 	if (s_vpn) {
 		value = nm_setting_vpn_get_data_item (s_vpn, NM_OPENVPN_KEY_STATIC_KEY);
 		if (value && strlen (value))
@@ -690,81 +653,6 @@ auth_widget_update_connection (GtkBuilder *builder,
 	return TRUE;
 }
 
-static gboolean
-save_secret (GtkBuilder *builder,
-			 const char *widget_name,
-			 const char *vpn_uuid,
-			 const char *vpn_name,
-			 const char *secret_name)
-{
-	GtkWidget *w;
-	const char *secret;
-	GnomeKeyringResult result;
-	gboolean ret;
-
-	w = GTK_WIDGET (gtk_builder_get_object (builder, widget_name));
-	g_assert (w);
-	secret = gtk_entry_get_text (GTK_ENTRY (w));
-	if (secret && strlen (secret)) {
-		result = keyring_helpers_save_secret (vpn_uuid, vpn_name, NULL, secret_name, secret);
-		ret = result == GNOME_KEYRING_RESULT_OK;
-		if (!ret)
-			g_warning ("%s: failed to save user password to keyring.", __func__);
-	} else
-		ret = keyring_helpers_delete_secret (vpn_uuid, secret_name);
-
-	return ret;
-}
-
-gboolean
-auth_widget_save_secrets (GtkBuilder *builder,
-						  const char *contype,
-						  NMConnection *connection)
-{
-	NMSetting *s_vpn;
-	gboolean ret = TRUE;
-	const char *uuid, *name;
-	NMSettingSecretFlags pw_flags = NM_SETTING_SECRET_FLAG_NONE;
-	NMSettingSecretFlags certpw_flags = NM_SETTING_SECRET_FLAG_NONE;
-
-	uuid = nm_connection_get_uuid (connection);
-	name = nm_connection_get_id (connection);
-
-	s_vpn = nm_connection_get_setting (connection, NM_TYPE_SETTING_VPN);
-	if (!s_vpn)
-		return FALSE;
-
-	nm_setting_get_secret_flags (s_vpn, NM_OPENVPN_KEY_PASSWORD, &pw_flags, NULL);
-	nm_setting_get_secret_flags (s_vpn, NM_OPENVPN_KEY_CERTPASS, &certpw_flags, NULL);
-
-	if (!strcmp (contype, NM_OPENVPN_CONTYPE_TLS)) {
-		if (certpw_flags & NM_SETTING_SECRET_FLAG_AGENT_OWNED)
-			ret = save_secret (builder, "tls_private_key_password_entry", uuid, name, NM_OPENVPN_KEY_CERTPASS);
-		else
-			keyring_helpers_delete_secret (uuid, NM_OPENVPN_KEY_CERTPASS);
-	} else if (!strcmp (contype, NM_OPENVPN_CONTYPE_PASSWORD)) {
-		if (pw_flags & NM_SETTING_SECRET_FLAG_AGENT_OWNED)
-			ret = save_secret (builder, "pw_password_entry", uuid, name, NM_OPENVPN_KEY_PASSWORD);
-		else
-			keyring_helpers_delete_secret (uuid, NM_OPENVPN_KEY_PASSWORD);
-	} else if (!strcmp (contype, NM_OPENVPN_CONTYPE_PASSWORD_TLS)) {
-		if (pw_flags & NM_SETTING_SECRET_FLAG_AGENT_OWNED)
-			ret = save_secret (builder, "pw_tls_password_entry", uuid, name, NM_OPENVPN_KEY_PASSWORD);
-		else
-			keyring_helpers_delete_secret (uuid, NM_OPENVPN_KEY_PASSWORD);
-
-		if (certpw_flags & NM_SETTING_SECRET_FLAG_AGENT_OWNED)
-			ret = save_secret (builder, "pw_tls_private_key_password_entry", uuid, name, NM_OPENVPN_KEY_CERTPASS);
-		else
-			keyring_helpers_delete_secret (uuid, NM_OPENVPN_KEY_CERTPASS);
-	} else if (!strcmp (contype, NM_OPENVPN_CONTYPE_STATIC_KEY)) {
-		/* No secrets here */
-	} else
-		g_assert_not_reached ();
-
-	return ret;
-}
-
 static const char *
 find_tag (const char *tag, const char *buf, gsize len)
 {
@@ -1800,34 +1688,3 @@ advanced_dialog_new_hash_from_dialog (GtkWidget *dialog, GError **error)
 	return hash;
 }
 
-gboolean
-advanced_save_secrets (GHashTable *advanced, NMConnection *connection)
-{
-	NMSetting *s_vpn;
-	const char *secret, *uuid, *name;
-	GnomeKeyringResult result;
-	gboolean ret = FALSE;
-	NMSettingSecretFlags flags = NM_SETTING_SECRET_FLAG_NONE;
-
-	s_vpn = nm_connection_get_setting (connection, NM_TYPE_SETTING_VPN);
-	if (!s_vpn)
-		return FALSE;
-	nm_setting_get_secret_flags (s_vpn, NM_OPENVPN_KEY_HTTP_PROXY_PASSWORD, &flags, NULL);
-
-	uuid = nm_connection_get_uuid (connection);
-	name = nm_connection_get_id (connection);
-
-	secret = g_hash_table_lookup (advanced, NM_OPENVPN_KEY_HTTP_PROXY_PASSWORD);
-	if (secret && strlen (secret) && (flags & NM_SETTING_SECRET_FLAG_AGENT_OWNED)) {
-		/* Only save the proxy password if it's agent-owned */
-		result = keyring_helpers_save_secret (uuid, name, NULL, NM_OPENVPN_KEY_HTTP_PROXY_PASSWORD, secret);
-		if (result == GNOME_KEYRING_RESULT_OK)
-			ret = TRUE;
-		else
-			g_warning ("%s: failed to save HTTP proxy password to keyring.", __func__);
-	} else
-		ret = keyring_helpers_delete_secret (uuid, NM_OPENVPN_KEY_HTTP_PROXY_PASSWORD);
-
-	return ret;
-}
-
diff --git a/properties/auth-helpers.h b/properties/auth-helpers.h
index 32f4df3..1c298e0 100644
--- a/properties/auth-helpers.h
+++ b/properties/auth-helpers.h
@@ -32,7 +32,7 @@ typedef void (*ChangedCallback) (GtkWidget *widget, gpointer user_data);
 
 void tls_pw_init_auth_widget (GtkBuilder *builder,
                               GtkSizeGroup *group,
-                              NMConnection *connection,
+                              NMSettingVPN *s_vpn,
                               const char *contype,
                               const char *prefix,
                               ChangedCallback changed_cb,
@@ -40,7 +40,7 @@ void tls_pw_init_auth_widget (GtkBuilder *builder,
 
 void sk_init_auth_widget (GtkBuilder *builder,
                           GtkSizeGroup *group,
-                          NMConnection *connection,
+                          NMSettingVPN *s_vpn,
                           ChangedCallback changed_cb,
                           gpointer user_data);
 
@@ -50,12 +50,6 @@ gboolean auth_widget_update_connection (GtkBuilder *builder,
                                         const char *contype,
                                         NMSettingVPN *s_vpn);
 
-gboolean auth_widget_save_secrets (GtkBuilder *builder,
-								   const char *contype,
-								   NMConnection *connection);
-
-gboolean advanced_save_secrets (GHashTable *advanced, NMConnection *connection);
-
 GtkFileFilter *tls_file_chooser_filter_new (gboolean pkcs_allowed);
 
 GtkFileFilter *sk_file_chooser_filter_new (void);
diff --git a/properties/nm-openvpn.c b/properties/nm-openvpn.c
index 86572a9..13f0431 100644
--- a/properties/nm-openvpn.c
+++ b/properties/nm-openvpn.c
@@ -44,7 +44,6 @@
 #include <nm-setting-connection.h>
 #include <nm-setting-ip4-config.h>
 
-#include "common-gnome/keyring-helpers.h"
 #include "src/nm-openvpn-service.h"
 #include "nm-openvpn.h"
 #include "auth-helpers.h"
@@ -310,7 +309,7 @@ init_plugin_ui (OpenvpnPluginUiWidget *self, NMConnection *connection, GError **
 	}
 
 	/* TLS auth widget */
-	tls_pw_init_auth_widget (priv->builder, priv->group, connection,
+	tls_pw_init_auth_widget (priv->builder, priv->group, s_vpn,
 	                         NM_OPENVPN_CONTYPE_TLS, "tls",
 	                         stuff_changed_cb, self);
 	gtk_list_store_append (store, &iter);
@@ -321,7 +320,7 @@ init_plugin_ui (OpenvpnPluginUiWidget *self, NMConnection *connection, GError **
 	                    -1);
 
 	/* Password auth widget */
-	tls_pw_init_auth_widget (priv->builder, priv->group, connection,
+	tls_pw_init_auth_widget (priv->builder, priv->group, s_vpn,
 	                         NM_OPENVPN_CONTYPE_PASSWORD, "pw",
 	                         stuff_changed_cb, self);
 	gtk_list_store_append (store, &iter);
@@ -334,7 +333,7 @@ init_plugin_ui (OpenvpnPluginUiWidget *self, NMConnection *connection, GError **
 		active = 1;
 
 	/* Password+TLS auth widget */
-	tls_pw_init_auth_widget (priv->builder, priv->group, connection,
+	tls_pw_init_auth_widget (priv->builder, priv->group, s_vpn,
 	                         NM_OPENVPN_CONTYPE_PASSWORD_TLS, "pw_tls",
 	                         stuff_changed_cb, self);
 	gtk_list_store_append (store, &iter);
@@ -347,7 +346,7 @@ init_plugin_ui (OpenvpnPluginUiWidget *self, NMConnection *connection, GError **
 		active = 2;
 
 	/* Static key auth widget */
-	sk_init_auth_widget (priv->builder, priv->group, connection, stuff_changed_cb, self);
+	sk_init_auth_widget (priv->builder, priv->group, s_vpn, stuff_changed_cb, self);
 
 	gtk_list_store_append (store, &iter);
 	gtk_list_store_set (store, &iter,
@@ -476,33 +475,6 @@ update_connection (NMVpnPluginUiWidgetInterface *iface,
 	return valid;
 }
 
-static gboolean
-save_secrets (NMVpnPluginUiWidgetInterface *iface,
-              NMConnection *connection,
-              GError **error)
-{
-	OpenvpnPluginUiWidgetPrivate *priv = OPENVPN_PLUGIN_UI_WIDGET_GET_PRIVATE (iface);
-	char *auth_type;
-	gboolean ret = FALSE;
-
-	auth_type = get_auth_type (priv->builder);
-	if (auth_type) {
-		ret = auth_widget_save_secrets (priv->builder, auth_type, connection);
-		g_free (auth_type);
-	}
-
-	if (ret)
-		ret = advanced_save_secrets (priv->advanced, connection);
-
-	if (!ret) {
-		g_set_error (error, OPENVPN_PLUGIN_UI_ERROR,
-		             OPENVPN_PLUGIN_UI_ERROR_UNKNOWN,
-		             "%s", "Saving secrets to gnome keyring failed.");
-	}
-
-	return ret;
-}
-
 static void
 is_new_func (const char *key, const char *value, gpointer user_data)
 {
@@ -624,7 +596,6 @@ openvpn_plugin_ui_widget_interface_init (NMVpnPluginUiWidgetInterface *iface_cla
 	/* interface implementation */
 	iface_class->get_widget = get_widget;
 	iface_class->update_connection = update_connection;
-	iface_class->save_secrets = save_secrets;
 }
 
 static NMConnection *
@@ -724,31 +695,6 @@ get_capabilities (NMVpnPluginUiInterface *iface)
 	return (NM_VPN_PLUGIN_UI_CAPABILITY_IMPORT | NM_VPN_PLUGIN_UI_CAPABILITY_EXPORT);
 }
 
-static gboolean
-delete_connection (NMVpnPluginUiInterface *iface,
-                   NMConnection *connection,
-                   GError **error)
-{
-	NMSettingConnection *s_con;
-	const char *uuid;
-
-	/* Remove any secrets in the keyring associated with this connection's UUID */
-	s_con = (NMSettingConnection *) nm_connection_get_setting (connection, NM_TYPE_SETTING_CONNECTION);
-	if (!s_con) {
-		g_set_error (error,
-		             OPENVPN_PLUGIN_UI_ERROR,
-		             OPENVPN_PLUGIN_UI_ERROR_INVALID_CONNECTION,
-		             "missing 'connection' setting");
-		return FALSE;
-	}
-
-	uuid = nm_setting_connection_get_uuid (s_con);
-	keyring_helpers_delete_secret (uuid, NM_OPENVPN_KEY_PASSWORD);
-	keyring_helpers_delete_secret (uuid, NM_OPENVPN_KEY_CERTPASS);
-
-	return TRUE;
-}
-
 static NMVpnPluginUiWidgetInterface *
 ui_factory (NMVpnPluginUiInterface *iface, NMConnection *connection, GError **error)
 {
@@ -809,7 +755,6 @@ openvpn_plugin_ui_interface_init (NMVpnPluginUiInterface *iface_class)
 	iface_class->import_from_file = import;
 	iface_class->export_to_file = export;
 	iface_class->get_suggested_name = get_suggested_name;
-	iface_class->delete_connection = delete_connection;
 }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]