2008-11-20 Dan Williams <dcbw@redhat.com>

	* libnm-util/libnm-util.ver
	  libnm-util/nm-setting.c
	  libnm-util/nm-setting.h
		- Add NMSetting errors
		- (nm_setting_update_secrets): return errors

	* libnm-util/nm-connection.c
	  libnm-util/nm-connection.h
		- (nm_connection_update_secrets): return errors

	* libnm-util/nm-setting-vpn.c
	  src/nm-activation-request.c
	  src/vpn-manager/nm-vpn-connection.c
		- Handle update secrets errors



git-svn-id: http://svn-archive.gnome.org/svn/NetworkManager/trunk@4314 4912f4e0-d625-0410-9fb7-b9a5a253dbdc

9 files changed, 187 insertions, 38 deletions

diff --git a/ChangeLog b/ChangeLog
index b150a409be..c72e575108 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,22 @@
 2008-11-20  Dan Williams  <dcbw@redhat.com>
 
+	* libnm-util/libnm-util.ver
+	  libnm-util/nm-setting.c
+	  libnm-util/nm-setting.h
+		- Add NMSetting errors
+		- (nm_setting_update_secrets): return errors
+
+	* libnm-util/nm-connection.c
+	  libnm-util/nm-connection.h
+		- (nm_connection_update_secrets): return errors
+
+	* libnm-util/nm-setting-vpn.c
+	  src/nm-activation-request.c
+	  src/vpn-manager/nm-vpn-connection.c
+		- Handle update secrets errors
+
+2008-11-20  Dan Williams  <dcbw@redhat.com>
+
 	* libnm-util/nm-setting.c
 	  libnm-util/nm-setting.h
 	  libnm-util/libnm-util.ver
diff --git a/libnm-util/libnm-util.ver b/libnm-util/libnm-util.ver
index 3444ef7895..836dc045a0 100644
--- a/libnm-util/libnm-util.ver
+++ b/libnm-util/libnm-util.ver
@@ -84,6 +84,8 @@ global:
 	nm_setting_connection_get_read_only;
 	nm_setting_duplicate;
 	nm_setting_enumerate_values;
+	nm_setting_error_get_type;
+	nm_setting_error_quark;
 	nm_setting_new_from_hash;
 	nm_setting_get_name;
 	nm_setting_get_type;
diff --git a/libnm-util/nm-connection.c b/libnm-util/nm-connection.c
index d83057c786..81d4ecb870 100644
--- a/libnm-util/nm-connection.c
+++ b/libnm-util/nm-connection.c
@@ -597,6 +597,7 @@ nm_connection_verify (NMConnection *connection, GError **error)
  * @setting_name: the setting object name to which the secrets apply
  * @secrets: a #GHashTable mapping string:#GValue of setting property names and
  * secrets
+ * @error: location to store error, or %NULL
  *
  * Update the specified setting's secrets, given a hash table of secrets
  * intended for that setting (deserialized from D-Bus for example).
@@ -607,24 +608,31 @@ nm_connection_verify (NMConnection *connection, GError **error)
 gboolean
 nm_connection_update_secrets (NMConnection *connection,
                               const char *setting_name,
-                              GHashTable *secrets)
+                              GHashTable *secrets,
+                              GError **error)
 {
 	NMSetting *setting;
+	gboolean success;
 
 	g_return_val_if_fail (connection != NULL, FALSE);
 	g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE);
 	g_return_val_if_fail (setting_name != NULL, FALSE);
 	g_return_val_if_fail (secrets != NULL, FALSE);
+	if (error)
+		g_return_val_if_fail (*error == NULL, FALSE);
 
 	setting = nm_connection_get_setting (connection, nm_connection_lookup_setting_type (setting_name));
 	if (!setting) {
-		g_warning ("Unhandled settings object for secrets update.");
+		g_set_error (error, NM_CONNECTION_ERROR,
+		             NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND,
+		             "%s", setting_name);
 		return FALSE;
 	}
 
-	nm_setting_update_secrets (setting, secrets);
-	g_signal_emit (connection, signals[SECRETS_UPDATED], 0, setting_name);
-	return TRUE;
+	success = nm_setting_update_secrets (setting, secrets, error);
+	if (success)
+		g_signal_emit (connection, signals[SECRETS_UPDATED], 0, setting_name);
+	return success;
 }
 
 static gint
diff --git a/libnm-util/nm-connection.h b/libnm-util/nm-connection.h
index ec2becc83c..cbb17fbf66 100644
--- a/libnm-util/nm-connection.h
+++ b/libnm-util/nm-connection.h
@@ -115,7 +115,8 @@ void          nm_connection_clear_secrets (NMConnection *connection);
 
 gboolean      nm_connection_update_secrets (NMConnection *connection,
                                             const char *setting_name,
-                                            GHashTable *secrets);
+                                            GHashTable *secrets,
+                                            GError **error);
 
 void             nm_connection_set_scope (NMConnection *connection,
                                                  NMConnectionScope scope);
diff --git a/libnm-util/nm-setting-vpn.c b/libnm-util/nm-setting-vpn.c
index 549b5beae0..def7432216 100644
--- a/libnm-util/nm-setting-vpn.c
+++ b/libnm-util/nm-setting-vpn.c
@@ -1,5 +1,4 @@
 /* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
-
 /*
  * Dan Williams <dcbw@redhat.com>
  * Tambet Ingo <tambet@gmail.com>
@@ -237,16 +236,23 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 	return TRUE;
 }
 
-static void
-update_one_secret (NMSetting *setting, const char *key, GValue *value)
+static gboolean
+update_one_secret (NMSetting *setting, const char *key, GValue *value, GError **error)
 {
 	NMSettingVPNPrivate *priv = NM_SETTING_VPN_GET_PRIVATE (setting);
 
-	g_return_if_fail (key != NULL);
-	g_return_if_fail (value != NULL);
-	g_return_if_fail (G_VALUE_HOLDS_STRING (value));
+	g_return_val_if_fail (key != NULL, FALSE);
+	g_return_val_if_fail (value != NULL, FALSE);
+
+	if (!G_VALUE_HOLDS_STRING (value)) {
+		g_set_error (error, NM_SETTING_ERROR,
+		             NM_SETTING_ERROR_PROPERTY_TYPE_MISMATCH,
+		             "%s", key);
+		return FALSE;
+	}
 
 	g_hash_table_insert (priv->secrets, g_strdup (key), g_value_dup_string (value));
+	return FALSE;
 }
 
 static void
diff --git a/libnm-util/nm-setting.c b/libnm-util/nm-setting.c
index 47bb034c62..1241e5d033 100644
--- a/libnm-util/nm-setting.c
+++ b/libnm-util/nm-setting.c
@@ -29,6 +29,48 @@
 #include "nm-setting-connection.h"
 #include "nm-utils.h"
 
+/**
+ * nm_setting_error_quark:
+ *
+ * Registers an error quark for #NMSetting if necessary.
+ *
+ * Returns: the error quark used for NMSetting errors.
+ **/
+GQuark
+nm_setting_error_quark (void)
+{
+	static GQuark quark;
+
+	if (G_UNLIKELY (!quark))
+		quark = g_quark_from_static_string ("nm-setting-error-quark");
+	return quark;
+}
+
+/* This should really be standard. */
+#define ENUM_ENTRY(NAME, DESC) { NAME, "" #NAME "", DESC }
+
+GType
+nm_setting_error_get_type (void)
+{
+	static GType etype = 0;
+
+	if (etype == 0) {
+		static const GEnumValue values[] = {
+			/* Unknown error. */
+			ENUM_ENTRY (NM_SETTING_ERROR_UNKNOWN, "UnknownError"),
+			/* The property was not found. */
+			ENUM_ENTRY (NM_SETTING_ERROR_PROPERTY_NOT_FOUND, "PropertyNotFound"),
+			/* The property was not a secret. */
+			ENUM_ENTRY (NM_SETTING_ERROR_PROPERTY_NOT_SECRET, "PropertyNotSecret"),
+			/* The property type didn't match the required property type. */
+			ENUM_ENTRY (NM_SETTING_ERROR_PROPERTY_TYPE_MISMATCH, "PropertyTypeMismatch"),
+			{ 0, 0, 0 }
+		};
+		etype = g_enum_register_static ("NMSettingError", values);
+	}
+	return etype;
+}
+
 G_DEFINE_ABSTRACT_TYPE (NMSetting, nm_setting, G_TYPE_OBJECT)
 
 #define NM_SETTING_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_SETTING, NMSettingPrivate))
@@ -435,51 +477,93 @@ nm_setting_need_secrets (NMSetting *setting)
 	return secrets;
 }
 
-static void
-update_one_secret (NMSetting *setting, const char *key, GValue *value)
+typedef struct {
+	NMSetting *setting;
+	GError **error;
+} UpdateSecretsInfo;
+
+static gboolean
+update_one_secret (NMSetting *setting, const char *key, GValue *value, GError **error)
 {
 	GParamSpec *prop_spec;
 	GValue transformed_value = { 0 };
+	gboolean success = FALSE;
 
 	prop_spec = g_object_class_find_property (G_OBJECT_GET_CLASS (setting), key);
 	if (!prop_spec) {
-		nm_warning ("Ignoring invalid secret '%s'.", key);
-		return;
+		g_set_error (error,
+		             NM_SETTING_ERROR,
+		             NM_SETTING_ERROR_PROPERTY_NOT_FOUND,
+		             "%s", key);
+		return FALSE;
 	}
 
 	if (!(prop_spec->flags & NM_SETTING_PARAM_SECRET)) {
-		nm_warning ("Ignoring secret '%s' as it's not marked as a secret.", key);
-		return;
+		g_set_error (error,
+		             NM_SETTING_ERROR,
+		             NM_SETTING_ERROR_PROPERTY_NOT_SECRET,
+		             "%s", key);
+		return FALSE;
 	}
 
-	if (g_value_type_compatible (G_VALUE_TYPE (value), G_PARAM_SPEC_VALUE_TYPE (prop_spec)))
+	if (g_value_type_compatible (G_VALUE_TYPE (value), G_PARAM_SPEC_VALUE_TYPE (prop_spec))) {
 		g_object_set_property (G_OBJECT (setting), prop_spec->name, value);
-	else if (g_value_transform (value, &transformed_value)) {
+		success = TRUE;
+	} else if (g_value_transform (value, &transformed_value)) {
 		g_object_set_property (G_OBJECT (setting), prop_spec->name, &transformed_value);
 		g_value_unset (&transformed_value);
+		success = TRUE;
 	} else {
-		nm_warning ("Ignoring secret property '%s' with invalid type (%s)",
-		            key, G_VALUE_TYPE_NAME (value));
+		g_set_error (error,
+		             NM_SETTING_ERROR,
+		             NM_SETTING_ERROR_PROPERTY_TYPE_MISMATCH,
+		             "%s", key);
 	}
+	return success;
 }
 
 static void
 update_one_cb (gpointer key, gpointer val, gpointer user_data)
 {
-	NMSetting *setting = (NMSetting *) user_data;
+	UpdateSecretsInfo *info = user_data;
 	const char *secret_key = (const char *) key;
 	GValue *secret_value = (GValue *) val;
 
-	NM_SETTING_GET_CLASS (setting)->update_one_secret (setting, secret_key, secret_value);
+	if (*(info->error) == NULL)
+		NM_SETTING_GET_CLASS (info->setting)->update_one_secret (info->setting, secret_key, secret_value, info->error);
 }
 
-void
-nm_setting_update_secrets (NMSetting *setting, GHashTable *secrets)
+/**
+ * nm_setting_update_secrets:
+ * @setting: the #NMSetting
+ * @secrets: a #GHashTable mapping string:#GValue of setting property names and
+ * secrets
+ * @error: location to store error, or %NULL
+ *
+ * Update the setting's secrets, given a hash table of secrets intended for that
+ * setting (deserialized from D-Bus for example).
+ * 
+ * Returns: TRUE if the secrets were successfully updated and the connection
+ * is valid, FALSE on failure or if the setting was never added to the connection
+ **/
+gboolean
+nm_setting_update_secrets (NMSetting *setting, GHashTable *secrets, GError **error)
 {
-	g_return_if_fail (NM_IS_SETTING (setting));
-	g_return_if_fail (secrets != NULL);
+	UpdateSecretsInfo *info;
+
+	g_return_val_if_fail (setting != NULL, FALSE);
+	g_return_val_if_fail (NM_IS_SETTING (setting), FALSE);
+	g_return_val_if_fail (secrets != NULL, FALSE);
+	if (error)
+		g_return_val_if_fail (*error == NULL, FALSE);
+
+	info = g_malloc0 (sizeof (UpdateSecretsInfo));
+	info->setting = setting;
+	info->error = error;
+	g_hash_table_foreach (secrets, update_one_cb, info);
+	g_free (info);
 
-	g_hash_table_foreach (secrets, update_one_cb, setting);
+	return *(info->error) ? FALSE : TRUE;
 }
 
 char *
diff --git a/libnm-util/nm-setting.h b/libnm-util/nm-setting.h
index 5eb92b3b77..8e4af24783 100644
--- a/libnm-util/nm-setting.h
+++ b/libnm-util/nm-setting.h
@@ -38,6 +38,20 @@ G_BEGIN_DECLS
 #define NM_IS_SETTING_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((obj), NM_TYPE_SETTING))
 #define NM_SETTING_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_SETTING, NMSettingClass))
 
+typedef enum
+{
+	NM_SETTING_ERROR_UNKNOWN = 0,
+	NM_SETTING_ERROR_PROPERTY_NOT_FOUND,
+	NM_SETTING_ERROR_PROPERTY_NOT_SECRET,
+	NM_SETTING_ERROR_PROPERTY_TYPE_MISMATCH
+} NMSettingError;
+
+#define NM_TYPE_SETTING_ERROR (nm_setting_error_get_type ()) 
+GType nm_setting_error_get_type (void);
+
+#define NM_SETTING_ERROR nm_setting_error_quark ()
+GQuark nm_setting_error_quark (void);
+
 #define NM_SETTING_PARAM_SERIALIZE    (1 << (0 + G_PARAM_USER_SHIFT))
 #define NM_SETTING_PARAM_REQUIRED     (1 << (1 + G_PARAM_USER_SHIFT))
 #define NM_SETTING_PARAM_SECRET       (1 << (2 + G_PARAM_USER_SHIFT))
@@ -59,9 +73,10 @@ typedef struct {
 
 	GPtrArray  *(*need_secrets)      (NMSetting  *setting);
 
-	void        (*update_one_secret) (NMSetting  *setting,
+	gboolean    (*update_one_secret) (NMSetting  *setting,
 	                                  const char *key,
-	                                  GValue     *value);
+	                                  GValue     *value,
+	                                  GError    **error);
 } NMSettingClass;
 
 typedef void (*NMSettingValueIterFn) (NMSetting *setting,
@@ -90,13 +105,15 @@ typedef enum {
 	/* Match all attributes exactly */
 	NM_SETTING_COMPARE_FLAG_EXACT = 0x00000000,
 
-	/* Match only important attributes, like SSID, type, security settings, etc */
+	/* Match only important attributes, like SSID, type, security settings, etc;
+	 * For example, does not match connection ID or UUID.
+	 */
 	NM_SETTING_COMPARE_FLAG_FUZZY = 0x00000001,
 
 	/* Ignore the connection ID */
 	NM_SETTING_COMPARE_FLAG_IGNORE_ID = 0x00000002,
 
-	/* Ignore secrets */
+	/* Ignore connection secrets */
 	NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS = 0x00000004
 } NMSettingCompareFlags;
 
@@ -114,8 +131,9 @@ char       *nm_setting_to_string      (NMSetting *setting);
 /* Secrets */
 void        nm_setting_clear_secrets  (NMSetting *setting);
 GPtrArray  *nm_setting_need_secrets   (NMSetting *setting);
-void        nm_setting_update_secrets (NMSetting *setting,
-							    GHashTable *secrets);
+gboolean    nm_setting_update_secrets (NMSetting *setting,
+                                       GHashTable *secrets,
+                                       GError **error);
 
 G_END_DECLS
 
diff --git a/src/nm-activation-request.c b/src/nm-activation-request.c
index 47df80e36f..5680a6d9d7 100644
--- a/src/nm-activation-request.c
+++ b/src/nm-activation-request.c
@@ -409,6 +409,7 @@ update_one_setting (const char* key,
 {
 	GType type;
 	NMSetting *setting = NULL;
+	GError *error = NULL;
 
 	/* Check whether a complete & valid NMSetting object was returned.  If
 	 * yes, replace the setting object in the connection.  If not, just try
@@ -441,8 +442,14 @@ update_one_setting (const char* key,
 
 	if (setting)
 		nm_connection_add_setting (connection, setting);
-	else
-		nm_connection_update_secrets (connection, key, setting_hash);
+	else {
+		if (!nm_connection_update_secrets (connection, key, setting_hash, &error)) {
+			nm_warning ("Failed to update connection secrets: %d %s",
+			            error ? error->code : -1,
+			            error && error->message ? error->message : "(none)");
+			g_clear_error (&error);
+		}
+	}
 
 	*updated = g_slist_append (*updated, (gpointer) key);
 }
diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c
index c29f48d1f9..6f3e2592e5 100644
--- a/src/vpn-manager/nm-vpn-connection.c
+++ b/src/vpn-manager/nm-vpn-connection.c
@@ -780,11 +780,17 @@ update_vpn_properties_secrets (gpointer key, gpointer data, gpointer user_data)
 {
 	NMConnection *connection = NM_CONNECTION (user_data);
 	GHashTable *secrets = (GHashTable *) data;
+	GError *error = NULL;
 
 	if (strcmp (key, NM_SETTING_VPN_SETTING_NAME))
 		return;
 
-	nm_connection_update_secrets (connection, NM_SETTING_VPN_SETTING_NAME, secrets);
+	if (!nm_connection_update_secrets (connection, NM_SETTING_VPN_SETTING_NAME, secrets, &error)) {
+		nm_warning ("Failed to update VPN secrets: %d %s",
+		            error ? error->code : -1,
+		            error && error->message ? error->message : "(none)");
+		g_clear_error (&error);
+	}
 }
 
 static void