settings: preserve agent secrets over Update operation

The core problem was that the Update would trigger a write to
disk to save the connection's new settings, which called
nm_settings_connection_replace_settings().  Which saved existing
transient (agent/unsaved) secrets, replaced settings with the
new ones from Update(), then copied back the old transient
secrets.  This was to ensure that changes triggered from getting
agent secrets during activation (which might write the connection
out to disk if new system secrets were provided, which triggered
an inotify read-back of the connection, which blew away the
transient secrets just returned from the agent) didn't blow away
transient secrets.  Unfortunately that fix was too general.

As a quick hack for now, copy the new secrets and re-apply them
after nm_connection_replace_settings() has run.  We'll do the
actual fix later, but it's more involved and needs more testing
so we don't want to apply it this close to release.

3 files changed, 132 insertions, 31 deletions

diff --git a/libnm-util/nm-connection.c b/libnm-util/nm-connection.c
index 6e9f92b59c..91fc3afac4 100644
--- a/libnm-util/nm-connection.c
+++ b/libnm-util/nm-connection.c
@@ -99,6 +99,7 @@ nm_connection_error_get_type (void)
 			ENUM_ENTRY (NM_CONNECTION_ERROR_UNKNOWN, "UnknownError"),
 			ENUM_ENTRY (NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND, "ConnectionSettingNotFound"),
 			ENUM_ENTRY (NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID, "ConnectionTypeInvalid"),
+			ENUM_ENTRY (NM_CONNECTION_ERROR_SETTING_NOT_FOUND, "SettingNotFound"),
 			{ 0, 0, 0 }
 		};
 		etype = g_enum_register_static ("NMConnectionError", values);
@@ -790,60 +791,76 @@ nm_connection_verify (NMConnection *connection, GError **error)
  * nm_connection_update_secrets:
  * @connection: the #NMConnection
  * @setting_name: the setting object name to which the secrets apply
- * @setting_secrets: (element-type utf8 GObject.Value): a #GHashTable mapping
+ * @secrets: (element-type utf8 GObject.Value): a #GHashTable mapping
  * string:#GValue of setting property names and secrets of the given @setting_name
  * @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).  Will also
  * extract the given setting's secrets hash if given a hash of hashes, as would
- * be returned from nm_connection_to_hash().
+ * be returned from nm_connection_to_hash().  If @setting_name is %NULL, expects
+ * a fully serialized #NMConnection as returned by nm_connection_to_hash() and
+ * will update all secrets from all settings contained in @secrets.
  *
- * 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
+ * Returns: %TRUE if the secrets were successfully updated, %FALSE if the update
+ * failed (tried to update secrets for a setting that doesn't exist, etc)
  **/
 gboolean
 nm_connection_update_secrets (NMConnection *connection,
                               const char *setting_name,
-                              GHashTable *setting_secrets,
+                              GHashTable *secrets,
                               GError **error)
 {
 	NMSetting *setting;
 	gboolean success;
 	GHashTable *tmp;
-	GType setting_type;
 
 	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 (setting_secrets != NULL, FALSE);
+	g_return_val_if_fail (secrets != NULL, FALSE);
 	if (error)
 		g_return_val_if_fail (*error == NULL, FALSE);
 
-	setting_type = nm_connection_lookup_setting_type (setting_name);
-	if (!setting_type) {
-		g_set_error_literal (error,
-		                     NM_CONNECTION_ERROR,
-		                     NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND,
-		                     setting_name);
-		return FALSE;
-	}
+	if (setting_name) {
+		/* Update just one setting */
+		setting = nm_connection_get_setting_by_name (connection, setting_name);
+		if (!setting) {
+			g_set_error_literal (error,
+				                 NM_CONNECTION_ERROR,
+				                 NM_CONNECTION_ERROR_SETTING_NOT_FOUND,
+				                 setting_name);
+			return FALSE;
+		}
 
-	setting = nm_connection_get_setting (connection, setting_type);
-	if (!setting) {
-		g_set_error_literal (error,
-		                     NM_CONNECTION_ERROR,
-		                     NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND,
-		                     setting_name);
-		return FALSE;
-	}
+		/* Check if this is a hash of hashes, ie a full deserialized connection,
+		 * not just a single hashed setting.
+		 */
+		tmp = g_hash_table_lookup (secrets, setting_name);
+		success = nm_setting_update_secrets (setting, tmp ? tmp : secrets, error);
+	} else {
+		GHashTableIter iter;
+		const char *name;
 
-	/* Check if this is a hash of hashes, ie a full deserialized connection,
-	 * not just a single hashed setting.
-	 */
-	tmp = g_hash_table_lookup (setting_secrets, setting_name);
+		success = TRUE; /* Just in case 'secrets' has no elements */
+
+		/* Try as a serialized connection (GHashTable of GHashTables) */
+		g_hash_table_iter_init (&iter, secrets);
+		while (g_hash_table_iter_next (&iter, (gpointer) &name, (gpointer) &tmp)) {
+			setting = nm_connection_get_setting_by_name (connection, name);
+			if (!setting) {
+				g_set_error_literal (error,
+						             NM_CONNECTION_ERROR,
+						             NM_CONNECTION_ERROR_SETTING_NOT_FOUND,
+						             name);
+				return FALSE;
+			}
 
-	success = nm_setting_update_secrets (setting, tmp ? tmp : setting_secrets, error);
+			/* Update the secrets for this setting */
+			success = nm_setting_update_secrets (setting, tmp, error);
+			if (success == FALSE)
+				break;
+		}
+	}
 	if (success)
 		g_signal_emit (connection, signals[SECRETS_UPDATED], 0, setting_name);
 	return success;
diff --git a/libnm-util/nm-connection.h b/libnm-util/nm-connection.h
index 87b053c101..a7e0721cc1 100644
--- a/libnm-util/nm-connection.h
+++ b/libnm-util/nm-connection.h
@@ -66,6 +66,8 @@ G_BEGIN_DECLS
  *   'connection' setting did not point to a valid connection base type; ie
  *   it was not a hardware-related setting like #NMSettingWired or
  *   #NMSettingWireless.
+ * @NM_CONNECTION_ERROR_SETTING_NOT_FOUND: the #NMConnection object
+ *   did not contain the specified #NMSetting object
  *
  * Describes errors that may result from operations involving a #NMConnection.
  *
@@ -74,7 +76,8 @@ typedef enum
 {
 	NM_CONNECTION_ERROR_UNKNOWN = 0,
 	NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND,
-	NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID
+	NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID,
+	NM_CONNECTION_ERROR_SETTING_NOT_FOUND
 } NMConnectionError;
 
 #define NM_TYPE_CONNECTION_ERROR (nm_connection_error_get_type ()) 
diff --git a/libnm-util/tests/test-secrets.c b/libnm-util/tests/test-secrets.c
index f1d105fcd2..1fe3c43823 100644
--- a/libnm-util/tests/test-secrets.c
+++ b/libnm-util/tests/test-secrets.c
@@ -591,7 +591,84 @@ test_update_secrets_wifi_bad_setting_name (void)
 	                                        "asdfasdfasdfasf",
 	                                        secrets,
 	                                        &error);
-	g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND);
+	g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_SETTING_NOT_FOUND);
+	g_assert (success == FALSE);
+
+	g_object_unref (connection);
+}
+
+static void
+test_update_secrets_whole_connection (void)
+{
+	NMConnection *connection;
+	NMSettingWirelessSecurity *s_wsec;
+	GHashTable *secrets, *wsec_hash;
+	GError *error = NULL;
+	gboolean success;
+	const char *wepkey = "11111111111111111111111111";
+
+	connection = wifi_connection_new ();
+
+	/* Build up the secrets hash */
+	secrets = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_ALL);
+	wsec_hash = g_hash_table_lookup (secrets, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME);
+	g_assert (wsec_hash);
+	g_hash_table_insert (wsec_hash, NM_SETTING_WIRELESS_SECURITY_WEP_KEY0, string_to_gvalue (wepkey));
+
+	success = nm_connection_update_secrets (connection, NULL, secrets, &error);
+	g_assert_no_error (error);
+	g_assert (success == TRUE);
+
+	s_wsec = nm_connection_get_setting_wireless_security (connection);
+	g_assert (s_wsec);
+	g_assert_cmpstr (nm_setting_wireless_security_get_wep_key (s_wsec, 0), ==, wepkey);
+
+	g_object_unref (connection);
+}
+
+static void
+test_update_secrets_whole_connection_empty_hash (void)
+{
+	NMConnection *connection;
+	GHashTable *secrets;
+	GError *error = NULL;
+	gboolean success;
+
+	connection = wifi_connection_new ();
+	secrets = g_hash_table_new (g_str_hash, g_str_equal);
+	success = nm_connection_update_secrets (connection, NULL, secrets, &error);
+	g_assert_no_error (error);
+	g_assert (success == TRUE);
+	g_object_unref (connection);
+}
+
+static void
+test_update_secrets_whole_connection_bad_setting (void)
+{
+	NMConnection *connection;
+	GHashTable *secrets, *wsec_hash;
+	GError *error = NULL;
+	gboolean success;
+	const char *wepkey = "11111111111111111111111111";
+
+	connection = wifi_connection_new ();
+
+	/* Build up the secrets hash */
+	secrets = nm_connection_to_hash (connection, NM_SETTING_HASH_FLAG_ALL);
+	wsec_hash = g_hash_table_lookup (secrets, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME);
+	g_assert (wsec_hash);
+	g_hash_table_insert (wsec_hash, NM_SETTING_WIRELESS_SECURITY_WEP_KEY0, string_to_gvalue (wepkey));
+
+	/* Steal the wsec setting hash so it's not deallocated, and stuff it back
+	 * in with a different name so we ensure libnm-util is returning the right
+	 * error when it finds an entry in the connection hash that doesn't match
+	 * any setting in the connection.
+	 */
+	g_hash_table_steal (secrets, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME);
+	g_hash_table_insert (secrets, "asdfasdfasdfasdf", wsec_hash);
+
+	success = nm_connection_update_secrets (connection, NULL, secrets, &error);
+	g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_SETTING_NOT_FOUND);
 	g_assert (success == FALSE);
 
 	g_object_unref (connection);
@@ -617,6 +694,10 @@ int main (int argc, char **argv)
 	test_update_secrets_wifi_full_hash ();
 	test_update_secrets_wifi_bad_setting_name ();
 
+	test_update_secrets_whole_connection ();
+	test_update_secrets_whole_connection_empty_hash ();
+	test_update_secrets_whole_connection_bad_setting ();
+
 	base = g_path_get_basename (argv[0]);
 	fprintf (stdout, "%s: SUCCESS\n", base);
 	g_free (base);