From 98ee18d888df477c35a8f2ce258ac2daf1099b26 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 29 Nov 2017 19:46:00 +0100
Subject: [PATCH] all: add new D-Bus API
 org.freedesktop.NetworkManager.Settings.Connection.Update2()

We already have Update(), UpdateUnsaved() and Save(), which serve
similar purposes. We will need a form of update with another argument.

Most notably, to block autoconnect while doing the update.

Other use cases could be to prevent reapplying connection.zone and
connection.metered, to to reapply all changes.

Instead of adding a specific update function that only serves that
new use-case, add a extensible Update2() function. It can be extended
to cope with future variants of update.
---
 ...top.NetworkManager.Settings.Connection.xml |  34 +++++
 libnm-core/nm-dbus-interface.h                |  14 ++
 libnm-core/nm-errors.h                        |   2 +
 libnm/libnm.ver                               |   1 +
 libnm/nm-remote-connection.c                  |  61 ++++----
 src/settings/nm-settings-connection.c         | 134 ++++++++++++++----
 6 files changed, 188 insertions(+), 58 deletions(-)

diff --git a/introspection/org.freedesktop.NetworkManager.Settings.Connection.xml b/introspection/org.freedesktop.NetworkManager.Settings.Connection.xml
index 61bd26ddc6..6e518677e9 100644
--- a/introspection/org.freedesktop.NetworkManager.Settings.Connection.xml
+++ b/introspection/org.freedesktop.NetworkManager.Settings.Connection.xml
@@ -101,6 +101,40 @@
     -->
     <signal name="Updated"/>
 
+    <!--
+        Update2:
+        @settings: New connection settings, properties, and (optionally) secrets.
+          Provide an empty array, to use the current settings.
+        @flags: optional flags argument. Currently supported flags are:
+          "0x1" (to-disk),
+          "0x2" (in-memory).
+          Unknown flags cause the call to fail.
+        @args: optional arguments dictionary, for extensibility. Currently no
+          arguments are accepted. Specifying unknown keys causes the call
+          to fail.
+        @result: output argument, currently no results are returned.
+
+        Update the connection with new settings and properties (replacing all
+        previous settings and properties). If the flag 0x1 is present,
+        the connection is persisted to disk. If the flag 0x2 is present,
+        the change is only made in memory (without touching an eventual
+        profile on disk). If neither 0x1 nor 0x2 is set, the change is made
+        in memory only, if the connection is already in memory only.
+        Secrets may be part of the update request, and will be either stored in persistent
+        storage or sent to a Secret Agent for storage, depending on the flags
+        associated with each secret.
+
+        Update2 is a extensible alternative to Update, UpdateUnsaved and Save.
+
+        Since: 1.12
+    -->
+    <method name="Update2">
+      <arg name="settings" type="a{sa{sv}}" direction="in"/>
+      <arg name="flags" type="u" direction="in"/>
+      <arg name="args" type="a{sv}" direction="in"/>
+      <arg name="result" type="a{sv}" direction="out"/>
+    </method>
+
     <!--
         Removed:
 
diff --git a/libnm-core/nm-dbus-interface.h b/libnm-core/nm-dbus-interface.h
index 6a0d3def2a..0b86fbee0e 100644
--- a/libnm-core/nm-dbus-interface.h
+++ b/libnm-core/nm-dbus-interface.h
@@ -896,4 +896,18 @@ typedef enum { /*< flags >*/
 	NM_ACTIVATION_STATE_FLAG_MASTER_HAS_SLAVES          = (1LL <<  5),
 } NMActivationStateFlags;
 
+/**
+ * NMSettingsUpdate2Flags:
+ * @NM_SETTINGS_UPDATE2_FLAG_NONE: an alias for numeric zero, no flags set.
+ * @NM_SETTINGS_UPDATE2_FLAG_TO_DISK: to persist the connection to disk.
+ * @NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY: to make the connection in-memory only.
+ *
+ * Since: 1.12
+ */
+typedef enum { /*< flags >*/
+	NM_SETTINGS_UPDATE2_FLAG_NONE                       = 0,
+	NM_SETTINGS_UPDATE2_FLAG_TO_DISK                    = (1LL <<  0),
+	NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY                  = (1LL <<  1),
+} NMSettingsUpdate2Flags;
+
 #endif /* __NM_DBUS_INTERFACE_H__ */
diff --git a/libnm-core/nm-errors.h b/libnm-core/nm-errors.h
index 6bacc8da56..bdd6c2234f 100644
--- a/libnm-core/nm-errors.h
+++ b/libnm-core/nm-errors.h
@@ -261,6 +261,7 @@ GQuark nm_secret_agent_error_quark (void);
  * @NM_SETTINGS_ERROR_READ_ONLY_CONNECTION: attempted to modify a read-only connection
  * @NM_SETTINGS_ERROR_UUID_EXISTS: a connection with that UUID already exists
  * @NM_SETTINGS_ERROR_INVALID_HOSTNAME: attempted to set an invalid hostname
+ * @NM_SETTINGS_ERROR_INVALID_ARGUMENTS: invalid arguments
  *
  * Errors related to the settings/persistent configuration interface of
  * NetworkManager.
@@ -277,6 +278,7 @@ typedef enum {
 	NM_SETTINGS_ERROR_READ_ONLY_CONNECTION, /*< nick=ReadOnlyConnection >*/
 	NM_SETTINGS_ERROR_UUID_EXISTS,          /*< nick=UuidExists >*/
 	NM_SETTINGS_ERROR_INVALID_HOSTNAME,     /*< nick=InvalidHostname >*/
+	NM_SETTINGS_ERROR_INVALID_ARGUMENTS,    /*< nick=InvalidArguments >*/
 } NMSettingsError;
 
 GQuark nm_settings_error_quark (void);
diff --git a/libnm/libnm.ver b/libnm/libnm.ver
index aad581698b..a57d336561 100644
--- a/libnm/libnm.ver
+++ b/libnm/libnm.ver
@@ -1260,4 +1260,5 @@ global:
 	nm_setting_team_remove_runner_tx_hash_by_value;
 	nm_setting_vpn_get_data_keys;
 	nm_setting_vpn_get_secret_keys;
+	nm_settings_update2_flags_get_type;
 } libnm_1_10_0;
diff --git a/libnm/nm-remote-connection.c b/libnm/nm-remote-connection.c
index 5cd42555f5..01496a75e7 100644
--- a/libnm/nm-remote-connection.c
+++ b/libnm/nm-remote-connection.c
@@ -87,23 +87,25 @@ nm_remote_connection_commit_changes (NMRemoteConnection *connection,
                                      GError **error)
 {
 	NMRemoteConnectionPrivate *priv;
-	GVariant *settings;
+	gs_unref_variant GVariant *result = NULL;
 	gboolean ret;
+	GVariantBuilder args;
 
 	g_return_val_if_fail (NM_IS_REMOTE_CONNECTION (connection), FALSE);
 
 	priv = NM_REMOTE_CONNECTION_GET_PRIVATE (connection);
 
-	settings = nm_connection_to_dbus (NM_CONNECTION (connection), NM_CONNECTION_SERIALIZE_ALL);
-	if (save_to_disk) {
-		ret = nmdbus_settings_connection_call_update_sync (priv->proxy,
-		                                                   settings,
-		                                                   cancellable, error);
-	} else {
-		ret = nmdbus_settings_connection_call_update_unsaved_sync (priv->proxy,
-		                                                           settings,
-		                                                           cancellable, error);
-	}
+	g_variant_builder_init (&args, G_VARIANT_TYPE ("a{sv}"));
+	ret = nmdbus_settings_connection_call_update2_sync (priv->proxy,
+	                                                    nm_connection_to_dbus (NM_CONNECTION (connection),
+	                                                                           NM_CONNECTION_SERIALIZE_ALL),
+	                                                    save_to_disk
+	                                                      ? NM_SETTINGS_UPDATE2_FLAG_TO_DISK
+	                                                      : NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY,
+	                                                    g_variant_builder_end (&args),
+	                                                    &result,
+	                                                    cancellable,
+	                                                    error);
 	if (error && *error)
 		g_dbus_error_strip_remote_error (*error);
 	return ret;
@@ -113,11 +115,13 @@ static void
 update_cb (GObject *proxy, GAsyncResult *result, gpointer user_data)
 {
 	GSimpleAsyncResult *simple = user_data;
-	gboolean (*finish_func) (NMDBusSettingsConnection *, GAsyncResult *, GError **);
 	GError *error = NULL;
+	gs_unref_variant GVariant *v = NULL;
 
-	finish_func = g_object_get_data (G_OBJECT (simple), "finish_func");
-	if (finish_func (NMDBUS_SETTINGS_CONNECTION (proxy), result, &error))
+	if (nmdbus_settings_connection_call_update2_finish (NMDBUS_SETTINGS_CONNECTION (proxy),
+	                                                    &v,
+	                                                    result,
+	                                                    &error))
 		g_simple_async_result_set_op_res_gboolean (simple, TRUE);
 	else {
 		g_dbus_error_strip_remote_error (error);
@@ -149,7 +153,7 @@ nm_remote_connection_commit_changes_async (NMRemoteConnection *connection,
 {
 	NMRemoteConnectionPrivate *priv;
 	GSimpleAsyncResult *simple;
-	GVariant *settings;
+	GVariantBuilder args;
 
 	g_return_if_fail (NM_IS_REMOTE_CONNECTION (connection));
 
@@ -158,22 +162,17 @@ nm_remote_connection_commit_changes_async (NMRemoteConnection *connection,
 	simple = g_simple_async_result_new (G_OBJECT (connection), callback, user_data,
 	                                    nm_remote_connection_commit_changes_async);
 
-	settings = nm_connection_to_dbus (NM_CONNECTION (connection), NM_CONNECTION_SERIALIZE_ALL);
-	if (save_to_disk) {
-		g_object_set_data (G_OBJECT (simple), "finish_func",
-		                   nmdbus_settings_connection_call_update_finish);
-		nmdbus_settings_connection_call_update (priv->proxy,
-		                                        settings,
-		                                        cancellable,
-		                                        update_cb, simple);
-	} else {
-		g_object_set_data (G_OBJECT (simple), "finish_func",
-		                   nmdbus_settings_connection_call_update_unsaved_finish);
-		nmdbus_settings_connection_call_update_unsaved (priv->proxy,
-		                                                settings,
-		                                                cancellable,
-		                                                update_cb, simple);
-	}
+	g_variant_builder_init (&args, G_VARIANT_TYPE ("a{sv}"));
+	nmdbus_settings_connection_call_update2 (priv->proxy,
+	                                         nm_connection_to_dbus (NM_CONNECTION (connection),
+	                                                                NM_CONNECTION_SERIALIZE_ALL),
+	                                         save_to_disk
+	                                           ? NM_SETTINGS_UPDATE2_FLAG_TO_DISK
+	                                           : NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY,
+	                                         g_variant_builder_end (&args),
+	                                         cancellable,
+	                                         update_cb,
+	                                         simple);
 }
 
 /**
diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c
index 22c389c872..62a8ca3073 100644
--- a/src/settings/nm-settings-connection.c
+++ b/src/settings/nm-settings-connection.c
@@ -1589,8 +1589,9 @@ typedef struct {
 	NMAgentManager *agent_mgr;
 	NMAuthSubject *subject;
 	NMConnection *new_settings;
-	gboolean save_to_disk;
+	NMSettingsUpdate2Flags flags;
 	char *audit_args;
+	bool is_update2:1;
 } UpdateInfo;
 
 static void
@@ -1654,7 +1655,13 @@ update_complete (NMSettingsConnection *self,
 {
 	if (error)
 		g_dbus_method_invocation_return_gerror (info->context, error);
-	else
+	else if (info->is_update2) {
+		GVariantBuilder result;
+
+		g_variant_builder_init (&result, G_VARIANT_TYPE ("a{sv}"));
+		g_dbus_method_invocation_return_value (info->context,
+		                                       g_variant_new ("(@a{sv})", g_variant_builder_end (&result)));
+	} else
 		g_dbus_method_invocation_return_value (info->context, NULL);
 
 	nm_audit_log_connection_op (NM_AUDIT_OP_CONN_UPDATE, self, !error, info->audit_args,
@@ -1677,6 +1684,8 @@ update_auth_cb (NMSettingsConnection *self,
 	UpdateInfo *info = data;
 	NMSettingsConnectionCommitReason commit_reason;
 	gs_free_error GError *local = NULL;
+	NMSettingsConnectionPersistMode persist_mode;
+	const char *log_diff_name;
 
 	if (error) {
 		update_complete (self, info, error);
@@ -1719,19 +1728,25 @@ update_auth_cb (NMSettingsConnection *self,
 	                   nm_connection_get_id (info->new_settings)))
 		commit_reason |= NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED;
 
+	if (NM_FLAGS_HAS (info->flags, NM_SETTINGS_UPDATE2_FLAG_TO_DISK))
+		persist_mode = NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK;
+	else if (NM_FLAGS_HAS (info->flags, NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY))
+		persist_mode = NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY;
+	else
+		persist_mode = NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP;
+
+	if (   persist_mode == NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY
+	    || (   persist_mode == NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP
+	        && nm_settings_connection_get_unsaved (self)))
+		log_diff_name = info->new_settings ? "update-unsaved" : "make-unsaved";
+	else
+		log_diff_name = info->new_settings ? "update-settings" : "write-out-to-disk";
+
 	_update (self,
 	         info->new_settings,
-	         info->save_to_disk
-	           ? NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK
-	           : NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY,
+	         persist_mode,
 	         commit_reason,
-	         !info->save_to_disk
-	           ? (info->new_settings
-	                ? "update-unsaved"
-	                : "make-unsaved")
-	           : (info->new_settings
-	                ? "update-settings"
-	                : "write-out-to-disk"),
+	         log_diff_name,
 	         &local);
 
 	if (!local) {
@@ -1782,10 +1797,11 @@ get_update_modify_permission (NMConnection *old, NMConnection *new)
 }
 
 static void
-settings_connection_update_helper (NMSettingsConnection *self,
-                                   GDBusMethodInvocation *context,
-                                   GVariant *new_settings,
-                                   gboolean save_to_disk)
+settings_connection_update (NMSettingsConnection *self,
+                            gboolean is_update2,
+                            GDBusMethodInvocation *context,
+                            GVariant *new_settings,
+                            NMSettingsUpdate2Flags flags)
 {
 	NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self);
 	NMAuthSubject *subject = NULL;
@@ -1795,8 +1811,6 @@ settings_connection_update_helper (NMSettingsConnection *self,
 	const char *permission;
 	char *error_desc = NULL;
 
-	g_assert (new_settings != NULL || save_to_disk == TRUE);
-
 	/* If the connection is read-only, that has to be changed at the source of
 	 * the problem (ex a system settings plugin that can't write connections out)
 	 * instead of over D-Bus.
@@ -1806,12 +1820,22 @@ settings_connection_update_helper (NMSettingsConnection *self,
 
 	/* Check if the settings are valid first */
 	if (new_settings) {
-		tmp = _nm_simple_connection_new_from_dbus (new_settings,
-		                                             NM_SETTING_PARSE_FLAGS_STRICT
-		                                           | NM_SETTING_PARSE_FLAGS_NORMALIZE,
-		                                           &error);
-		if (!tmp)
+		if (!g_variant_is_of_type (new_settings, NM_VARIANT_TYPE_CONNECTION)) {
+			g_set_error_literal (&error,
+			                     NM_SETTINGS_ERROR,
+			                     NM_SETTINGS_ERROR_INVALID_ARGUMENTS,
+			                     "settings is of invalid type");
 			goto error;
+		}
+
+		if (g_variant_n_children (new_settings) > 0) {
+			tmp = _nm_simple_connection_new_from_dbus (new_settings,
+			                                             NM_SETTING_PARSE_FLAGS_STRICT
+			                                           | NM_SETTING_PARSE_FLAGS_NORMALIZE,
+			                                           &error);
+			if (!tmp)
+				goto error;
+		}
 	}
 
 	subject = _new_auth_subject (context, &error);
@@ -1833,10 +1857,11 @@ settings_connection_update_helper (NMSettingsConnection *self,
 	}
 
 	info = g_slice_new0 (UpdateInfo);
+	info->is_update2 = is_update2;
 	info->context = context;
 	info->agent_mgr = g_object_ref (priv->agent_mgr);
 	info->subject = subject;
-	info->save_to_disk = save_to_disk;
+	info->flags = flags;
 	info->new_settings = tmp;
 
 	permission = get_update_modify_permission (NM_CONNECTION (self),
@@ -1859,7 +1884,7 @@ impl_settings_connection_update (NMSettingsConnection *self,
                                  GDBusMethodInvocation *context,
                                  GVariant *new_settings)
 {
-	settings_connection_update_helper (self, context, new_settings, TRUE);
+	settings_connection_update (self, FALSE, context, new_settings, NM_SETTINGS_UPDATE2_FLAG_TO_DISK);
 }
 
 static void
@@ -1867,14 +1892,68 @@ impl_settings_connection_update_unsaved (NMSettingsConnection *self,
                                          GDBusMethodInvocation *context,
                                          GVariant *new_settings)
 {
-	settings_connection_update_helper (self, context, new_settings, FALSE);
+	settings_connection_update (self, FALSE, context, new_settings, NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY);
 }
 
 static void
 impl_settings_connection_save (NMSettingsConnection *self,
                                GDBusMethodInvocation *context)
 {
-	settings_connection_update_helper (self, context, NULL, TRUE);
+	settings_connection_update (self, FALSE, context, NULL, NM_SETTINGS_UPDATE2_FLAG_TO_DISK);
+}
+
+static void
+impl_settings_connection_update2 (NMSettingsConnection *self,
+                                  GDBusMethodInvocation *context,
+                                  GVariant *settings,
+                                  guint32 flags_u,
+                                  GVariant *args)
+{
+	GError *error = NULL;
+	GVariantIter iter;
+	const char *args_name;
+	const NMSettingsUpdate2Flags flags = (NMSettingsUpdate2Flags) flags_u;
+
+	if (NM_FLAGS_ANY (flags_u, ~((guint32) (NM_SETTINGS_UPDATE2_FLAG_TO_DISK |
+	                                        NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY)))) {
+		error = g_error_new_literal (NM_SETTINGS_ERROR,
+		                             NM_SETTINGS_ERROR_INVALID_ARGUMENTS,
+		                             "Unknown flags");
+		g_dbus_method_invocation_take_error (context, error);
+		return;
+	}
+
+	if (NM_FLAGS_ALL (flags, NM_SETTINGS_UPDATE2_FLAG_TO_DISK |
+	                         NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY)) {
+		error = g_error_new_literal (NM_SETTINGS_ERROR,
+		                             NM_SETTINGS_ERROR_INVALID_ARGUMENTS,
+		                             "Conflicting flags");
+		g_dbus_method_invocation_take_error (context, error);
+		return;
+	}
+
+	if (!g_variant_is_of_type (args, G_VARIANT_TYPE ("a{sv}"))) {
+		error = g_error_new_literal (NM_SETTINGS_ERROR,
+		                             NM_SETTINGS_ERROR_INVALID_ARGUMENTS,
+		                             "args is of invalid type");
+		g_dbus_method_invocation_take_error (context, error);
+		return;
+	}
+
+	g_variant_iter_init (&iter, args);
+	while (g_variant_iter_next (&iter, "{&sv}", &args_name, NULL)) {
+		error = g_error_new (NM_SETTINGS_ERROR,
+		                     NM_SETTINGS_ERROR_INVALID_ARGUMENTS,
+		                     "Unsupported argument '%s'", args_name);
+		g_dbus_method_invocation_take_error (context, error);
+		return;
+	}
+
+	settings_connection_update (self,
+	                            TRUE,
+	                            context,
+	                            settings,
+	                            flags);
 }
 
 static void
@@ -3005,6 +3084,7 @@ nm_settings_connection_class_init (NMSettingsConnectionClass *class)
 	                                        "GetSecrets", impl_settings_connection_get_secrets,
 	                                        "ClearSecrets", impl_settings_connection_clear_secrets,
 	                                        "Save", impl_settings_connection_save,
+	                                        "Update2", impl_settings_connection_update2,
 	                                        NULL);
 }