applet/vpn-request: fix external UI mode

When a connection is opened for editing in nm-c-e, secrets are asked
to NM. NM in turn asks them to registered agents, including the applet
if available; the applet now supports an external-ui-mode in which it
spawns a VPN-specific authentication dialog binary that doesn't have a
graphical component and contains the knowledge about which secrets are
missing, how to retrieve them from keyrings and how to ask them to
users; the binary then returns through stdout a keyfile that describes
the known secrets, their values returned from keyrings and the labels
to present to users. The applet uses this information to show a GTK
dialog (only when necessary) and returns the secrets to NM.

Currently when using the external-ui-mode and the auth-dialog binary
returns secrets with the ShouldAsk key set to false, the applet
doesn't show a GTK dialog and returns an empty response. This breaks
when the auth-dialog returns secrets from keyrings as they are
dropped.

This commit changes the handling of secrets in external-ui-mode: now
we first build a list of known secrets and, if necessary, populate the
dialog with the ones that must be asked. After the dialog quits we
update the secret values in the list from the dialog and return all
secrets, including ones not asked.

https://gitlab.gnome.org/GNOME/network-manager-applet/issues/59
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/193

Fixes: bce8d0a75c1d ("applet/vpn-request: add external UI mode")

1 files changed, 134 insertions, 95 deletions

diff --git a/src/applet-vpn-request.c b/src/applet-vpn-request.c
index 04c46dd9..07a71fce 100644
--- a/src/applet-vpn-request.c
+++ b/src/applet-vpn-request.c
@@ -26,6 +26,14 @@
 /*****************************************************************************/
 
 typedef struct {
+	char *name;
+	char *label;
+	char *value;
+	gboolean is_secret;
+	gboolean should_ask;
+} EuiSecret;
+
+typedef struct {
 	char *uuid;
 	char *id;
 	char *service_type;
@@ -41,8 +49,7 @@ typedef struct {
 	gboolean external_ui_mode;
 
 	/* These are just for the external UI mode */
-	char *secret_names[3];
-	int no_passwords;
+	EuiSecret *eui_secrets;
 } RequestData;
 
 typedef struct {
@@ -65,55 +72,23 @@ applet_vpn_request_get_secrets_size (void)
 
 /*****************************************************************************/
 
-static gboolean
-external_ui_add_password (NMAVpnPasswordDialog *dialog,
-                          int passwords,
-                          GKeyFile *keyfile,
-                          const char *group,
-                          GError **error)
+static void
+external_ui_add_secrets (VpnSecretsInfo *info)
 {
-	gs_free char *label = NULL;
-	gs_free char *value = NULL;
-	gboolean is_secret;
-	gboolean should_ask;
-
-	is_secret = g_key_file_get_boolean (keyfile, group, "IsSecret", NULL);
-	should_ask = g_key_file_get_boolean (keyfile, group, "ShouldAsk", NULL);
-	if (!should_ask || !is_secret)
-		return FALSE;
-
-	label = g_key_file_get_string (keyfile, group, "Label", error);
-	if (!label)
-		return FALSE;
-
-	value = g_key_file_get_string (keyfile, group, "Value", NULL);
-
-	switch (passwords) {
-	case 0:
-		nma_vpn_password_dialog_set_show_password (dialog, TRUE);
-		nma_vpn_password_dialog_set_password_label (dialog, label);
-		if (value)
-			nma_vpn_password_dialog_set_password (dialog, value);
-		break;
-	case 1:
-		nma_vpn_password_dialog_set_show_password_secondary (dialog, TRUE);
-		nma_vpn_password_dialog_set_password_secondary_label (dialog, label);
-		if (value)
-			nma_vpn_password_dialog_set_password_secondary (dialog, value);
-		break;
-	case 2:
-		nma_vpn_password_dialog_set_show_password_ternary (dialog, TRUE);
-		nma_vpn_password_dialog_set_password_ternary_label (dialog, label);
-		if (value)
-			nma_vpn_password_dialog_set_password_ternary (dialog, value);
-		break;
-	default:
-		g_set_error_literal (error, NM_SECRET_AGENT_ERROR, NM_SECRET_AGENT_ERROR_FAILED,
-		                     "More than 3 passwords are not supported.");
-		return FALSE;
+	RequestData *req_data = info->req_data;
+	EuiSecret *secret;
+	guint i;
+
+	for (i = 0; req_data->eui_secrets[i].name; i++) {
+		secret = &req_data->eui_secrets[i];
+		if (   secret->is_secret
+		    && secret->value
+		    && secret->value[0]) {
+			g_variant_builder_add (&req_data->secrets_builder, "{ss}",
+			                       secret->name,
+			                       secret->value);
+		}
 	}
-
-	return TRUE;
 }
 
 static void
@@ -122,24 +97,35 @@ external_ui_dialog_response (GtkDialog *dialog, int response_id, gpointer user_d
 	VpnSecretsInfo *info = user_data;
 	RequestData *req_data = info->req_data;
 	NMAVpnPasswordDialog *vpn_dialog = NMA_VPN_PASSWORD_DIALOG (dialog);
-
-	g_variant_builder_add (&req_data->secrets_builder, "{ss}",
-	                       req_data->secret_names[0],
-	                       nma_vpn_password_dialog_get_password (vpn_dialog));
-
-	if (req_data->no_passwords > 1) {
-		g_variant_builder_add (&req_data->secrets_builder, "{ss}",
-		                       req_data->secret_names[1],
-		                       nma_vpn_password_dialog_get_password_secondary (vpn_dialog));
-	}
-
-	if (req_data->no_passwords > 2) {
-		g_variant_builder_add (&req_data->secrets_builder, "{ss}",
-		                       req_data->secret_names[2],
-		                       nma_vpn_password_dialog_get_password_ternary (vpn_dialog));
+	EuiSecret *secret;
+	const char *value;
+	guint i_secret, i_pw;
+
+	for (i_secret = 0, i_pw = 0; req_data->eui_secrets[i_secret].name; i_secret++) {
+		secret = &req_data->eui_secrets[i_secret];
+		if (   secret->is_secret
+		    && secret->should_ask) {
+			switch (i_pw) {
+			case 0:
+				value = nma_vpn_password_dialog_get_password (vpn_dialog);
+				break;
+			case 1:
+				value = nma_vpn_password_dialog_get_password_secondary (vpn_dialog);
+				break;
+			case 2:
+				value = nma_vpn_password_dialog_get_password_ternary (vpn_dialog);
+				break;
+			default:
+				continue;
+			}
+			g_free (secret->value);
+			secret->value = g_strdup (value);
+			i_pw++;
+		}
 	}
 
 	gtk_widget_destroy (GTK_WIDGET (dialog));
+	external_ui_add_secrets (info);
 	complete_request (info);
 }
 
@@ -149,13 +135,15 @@ external_ui_from_child_response (VpnSecretsInfo *info, GError **error)
 	RequestData *req_data = info->req_data;
 	gs_unref_keyfile GKeyFile *keyfile = NULL;
 	gs_strfreev char **groups = NULL;
-	GtkWidget *dialog = NULL;
+	NMAVpnPasswordDialog *dialog = NULL;
 	gs_free char *version = NULL;
 	gs_free char *title = NULL;
 	gs_free char *message = NULL;
-	gs_free char *value = NULL;
-	int i;
+	gsize num_groups;
+	guint num_ask = 0;
+	guint i_group, i_secret, i_pw;
 
+	/* Parse response key file */
 	keyfile = g_key_file_new ();
 
 	if (!g_key_file_load_from_data (keyfile,
@@ -166,7 +154,7 @@ external_ui_from_child_response (VpnSecretsInfo *info, GError **error)
 		return FALSE;
 	}
 
-	groups = g_key_file_get_groups (keyfile, NULL);
+	groups = g_key_file_get_groups (keyfile, &num_groups);
 	if (g_strcmp0 (groups[0], "VPN Plugin UI") != 0) {
 		g_set_error_literal (error,
 		                     NM_SECRET_AGENT_ERROR,
@@ -194,39 +182,83 @@ external_ui_from_child_response (VpnSecretsInfo *info, GError **error)
 	if (!message)
 		return FALSE;
 
-	dialog = nma_vpn_password_dialog_new (title, message, NULL);
-	nma_vpn_password_dialog_set_show_password (NMA_VPN_PASSWORD_DIALOG (dialog), FALSE);
-	nma_vpn_password_dialog_set_show_password_secondary (NMA_VPN_PASSWORD_DIALOG (dialog), FALSE);
-	nma_vpn_password_dialog_set_show_password_ternary (NMA_VPN_PASSWORD_DIALOG (dialog), FALSE);
-
-	for (i = 1; groups[i] != NULL; i++) {
-		GError *local = NULL;
-
-		if (external_ui_add_password (NMA_VPN_PASSWORD_DIALOG (dialog),
-		                              req_data->no_passwords,
-		                              keyfile,
-		                              groups[i],
-		                              &local)) {
-			req_data->secret_names[req_data->no_passwords++] = g_strdup (groups[i]);
-		} else if (local) {
-			g_warning ("Skipping entry: %s\n", local->message);
-			g_error_free (local);
+	/* Create a secret instance for each group */
+	req_data->eui_secrets = g_new0 (EuiSecret, num_groups);
+	for (i_group = 1, i_secret = 0; i_group < num_groups; i_group++) {
+		EuiSecret *secret = &req_data->eui_secrets[i_secret];
+		const char *group = groups[i_group];
+		char *label;
+
+		label = g_key_file_get_string (keyfile, group, "Label", NULL);
+		if (!label) {
+			g_warning ("Skipping entry: no label\n");
+			continue;
 		}
+
+		secret->name = g_strdup (group);
+		secret->label = label;
+		secret->value = g_key_file_get_string (keyfile, group, "Value", NULL);
+		secret->is_secret = g_key_file_get_boolean (keyfile, group, "IsSecret", NULL);
+		secret->should_ask = g_key_file_get_boolean (keyfile, group, "ShouldAsk", NULL);
+
+		i_secret++;
+
+		if (secret->is_secret && secret->should_ask)
+			num_ask++;
 	}
 
-	g_object_ref_sink (dialog);
-	if (req_data->no_passwords > 0 ) {
+	/* If there are any secrets that must be asked to user,
+	 * create a dialog and display it. */
+	if (num_ask > 0) {
+		dialog = (NMAVpnPasswordDialog *) nma_vpn_password_dialog_new (title, message, NULL);
+		g_object_ref_sink (dialog);
+
+		nma_vpn_password_dialog_set_show_password (dialog, FALSE);
+		nma_vpn_password_dialog_set_show_password_secondary (dialog, FALSE);
+		nma_vpn_password_dialog_set_show_password_ternary (dialog, FALSE);
+
+		for (i_secret = 0, i_pw = 0; req_data->eui_secrets[i_secret].name; i_secret++) {
+			EuiSecret *secret = &req_data->eui_secrets[i_secret];
+
+			if (   secret->is_secret
+			    && secret->should_ask) {
+				switch (i_pw) {
+				case 0:
+					nma_vpn_password_dialog_set_show_password (dialog, TRUE);
+					nma_vpn_password_dialog_set_password_label (dialog, secret->label);
+					if (secret->value)
+						nma_vpn_password_dialog_set_password (dialog, secret->value);
+					break;
+				case 1:
+					nma_vpn_password_dialog_set_show_password_secondary (dialog, TRUE);
+					nma_vpn_password_dialog_set_password_secondary_label (dialog, secret->label);
+					if (secret->value)
+						nma_vpn_password_dialog_set_password_secondary (dialog, secret->value);
+					break;
+				case 2:
+					nma_vpn_password_dialog_set_show_password_ternary (dialog, TRUE);
+					nma_vpn_password_dialog_set_password_ternary_label (dialog, secret->label);
+					if (secret->value)
+						nma_vpn_password_dialog_set_password_ternary (dialog, secret->value);
+					break;
+				default:
+					g_warning ("Skipping entry: more than 3 passwords not supported\n");
+					continue;
+				}
+				i_pw++;
+			}
+		}
 		g_signal_connect (dialog,
 		                  "response",
 		                  G_CALLBACK (external_ui_dialog_response),
 		                  info);
-		gtk_widget_show (dialog);
-	} else {
-		/* No secrets, return right away an empty response. */
-		g_object_unref (dialog);
-		complete_request (info);
+		gtk_widget_show (GTK_WIDGET (dialog));
+		return TRUE;
 	}
 
+	/* Nothing to ask, return known secrets */
+	external_ui_add_secrets (info);
+	complete_request (info);
 	return TRUE;
 }
 
@@ -661,6 +693,8 @@ ensure_killed (gpointer data)
 static void
 request_data_free (RequestData *req_data)
 {
+	guint i;
+
 	if (!req_data)
 		return;
 
@@ -690,9 +724,14 @@ request_data_free (RequestData *req_data)
 
 	g_variant_builder_clear (&req_data->secrets_builder);
 
-	g_free (req_data->secret_names[0]);
-	g_free (req_data->secret_names[1]);
-	g_free (req_data->secret_names[2]);
+	if (req_data->eui_secrets) {
+		for (i = 0; req_data->eui_secrets[i].name; i++) {
+			g_free (req_data->eui_secrets[i].name);
+			g_free (req_data->eui_secrets[i].label);
+			g_free (req_data->eui_secrets[i].value);
+		}
+		g_free (req_data->eui_secrets);
+	}
 
 	g_slice_free (RequestData, req_data);
 }