merge: branch 'bg/external-ui-mode-fix'

https://gitlab.gnome.org/GNOME/network-manager-applet/issues/59
https://gitlab.gnome.org/GNOME/network-manager-applet/merge_requests/53

2 files changed, 143 insertions, 105 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);
 }
diff --git a/src/libnma/nma-vpn-password-dialog.c b/src/libnma/nma-vpn-password-dialog.c
index a6738146..adc67971 100644
--- a/src/libnma/nma-vpn-password-dialog.c
+++ b/src/libnma/nma-vpn-password-dialog.c
@@ -101,19 +101,18 @@ dialog_show_callback (GtkWidget *widget, gpointer callback_data)
 	NMAVpnPasswordDialogPrivate *priv = NMA_VPN_PASSWORD_DIALOG_GET_PRIVATE (dialog);
 	GtkWidget *to_focus = NULL;
 
-	if (   gtk_widget_get_visible (priv->password_entry_tertiary)
-	    && gtk_entry_get_text_length (GTK_ENTRY (priv->password_entry_tertiary)) == 0) {
-		to_focus = priv->password_entry_tertiary;
-	}
 
-	if (   gtk_widget_get_visible (priv->password_entry_secondary)
-	           && gtk_entry_get_text_length (GTK_ENTRY (priv->password_entry_secondary)) == 0) {
+	if (   gtk_widget_get_visible (priv->password_entry)
+	    && gtk_entry_get_text_length (GTK_ENTRY (priv->password_entry)) == 0)
+		to_focus = priv->password_entry;
+	else if (   gtk_widget_get_visible (priv->password_entry_secondary)
+	         && gtk_entry_get_text_length (GTK_ENTRY (priv->password_entry_secondary)) == 0)
 		to_focus = priv->password_entry_secondary;
-	}
+	else if (   gtk_widget_get_visible (priv->password_entry_tertiary)
+	         && gtk_entry_get_text_length (GTK_ENTRY (priv->password_entry_tertiary)) == 0)
+		to_focus = priv->password_entry_tertiary;
 
-	if (to_focus == NULL)
-		to_focus = priv->password_entry;
-	gtk_widget_grab_focus (to_focus);
+	gtk_widget_grab_focus (to_focus ?: priv->password_entry);
 }
 
 static void