clients: some cleanup of requesting VPN secrets

6 files changed, 179 insertions, 169 deletions

diff --git a/clients/cli/agent.c b/clients/cli/agent.c
index 656a58511d..2088648aa0 100644
--- a/clients/cli/agent.c
+++ b/clients/cli/agent.c
@@ -104,7 +104,7 @@ get_secrets_from_user (const char *request_id,
 			rl_startup_hook = set_deftext;
 			pre_input_deftext = g_strdup (secret->value);
 		}
-		pwd = nmc_readline ("%s (%s): ", secret->name, secret->prop_name);
+		pwd = nmc_readline ("%s (%s): ", secret->pretty_name, secret->entry_id);
 
 		/* No password provided, cancel the secrets. */
 		if (!pwd)
diff --git a/clients/cli/common.c b/clients/cli/common.c
index 4f369f458b..f785e2c568 100644
--- a/clients/cli/common.c
+++ b/clients/cli/common.c
@@ -531,10 +531,10 @@ vpn_openconnect_get_secrets (NMConnection *connection, GPtrArray *secrets)
 {
 	GError *error = NULL;
 	NMSettingVpn *s_vpn;
-	const char *vpn_type, *gw, *port;
-	char *cookie = NULL;
-	char *gateway = NULL;
-	char *gwcert = NULL;
+	const char *gw, *port;
+	gs_free char *cookie = NULL;
+	gs_free char *gateway = NULL;
+	gs_free char *gwcert = NULL;
 	int status = 0;
 	int i;
 	gboolean ret;
@@ -546,8 +546,7 @@ vpn_openconnect_get_secrets (NMConnection *connection, GPtrArray *secrets)
 		return FALSE;
 
 	s_vpn = nm_connection_get_setting_vpn (connection);
-	vpn_type = nm_setting_vpn_get_service_type (s_vpn);
-	if (g_strcmp0 (vpn_type, NM_DBUS_INTERFACE ".openconnect"))
+	if (!nm_streq0 (nm_setting_vpn_get_service_type (s_vpn), NM_SECRET_AGENT_VPN_TYPE_OPENCONNECT))
 		return FALSE;
 
 	/* Get gateway and port */
@@ -570,34 +569,31 @@ vpn_openconnect_get_secrets (NMConnection *connection, GPtrArray *secrets)
 
 	/* Append port to the host value */
 	if (gateway && port) {
-		char *tmp = gateway;
-		gateway = g_strdup_printf ("%s%s", gateway, port);
-		g_free (tmp);
+		gs_free char *tmp = gateway;
+
+		gateway = g_strdup_printf ("%s%s", tmp, port);
 	}
 
 	/* Fill secrets to the array */
 	for (i = 0; i < secrets->len; i++) {
 		NMSecretAgentSimpleSecret *secret = secrets->pdata[i];
 
-		if (!g_strcmp0 (secret->vpn_type, vpn_type)) {
-			if (!g_strcmp0 (secret->vpn_property, "cookie")) {
-				g_free (secret->value);
-				secret->value = cookie;
-				cookie = NULL;
-			} else if (!g_strcmp0 (secret->vpn_property, "gateway")) {
-				g_free (secret->value);
-				secret->value = gateway;
-				gateway = NULL;
-			} else if (!g_strcmp0 (secret->vpn_property, "gwcert")) {
-				g_free (secret->value);
-				secret->value = gwcert;
-				gwcert = NULL;
-			}
+		if (secret->secret_type != NM_SECRET_AGENT_SECRET_TYPE_VPN_SECRET)
+			continue;
+		if (!nm_streq0 (secret->vpn_type, NM_SECRET_AGENT_VPN_TYPE_OPENCONNECT))
+			continue;
+
+		if (nm_streq0 (secret->entry_id, NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRET "cookie")) {
+			g_free (secret->value);
+			secret->value = g_steal_pointer (&cookie);
+		} else if (nm_streq0 (secret->entry_id, NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRET "gateway")) {
+			g_free (secret->value);
+			secret->value = g_steal_pointer (&gateway);
+		} else if (nm_streq0 (secret->entry_id, NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRET "gwcert")) {
+			g_free (secret->value);
+			secret->value = g_steal_pointer (&gwcert);
 		}
 	}
-	g_free (cookie);
-	g_free (gateway);
-	g_free (gwcert);
 
 	return TRUE;
 }
@@ -624,7 +620,7 @@ get_secrets_from_user (const char *request_id,
 
 		/* First try to find the password in provided passwords file,
 		 * then ask user. */
-		if (pwds_hash && (pwd = g_hash_table_lookup (pwds_hash, secret->prop_name))) {
+		if (pwds_hash && (pwd = g_hash_table_lookup (pwds_hash, secret->entry_id))) {
 			pwd = g_strdup (pwd);
 		} else {
 			if (ask) {
@@ -640,8 +636,10 @@ get_secrets_from_user (const char *request_id,
 				}
 				if (msg)
 					g_print ("%s\n", msg);
-				pwd = nmc_readline_echo (secret->password ? echo_on : TRUE,
-				                         "%s (%s): ", secret->name, secret->prop_name);
+				pwd = nmc_readline_echo (secret->is_secret
+				                         ? echo_on
+				                         : TRUE,
+				                         "%s (%s): ", secret->pretty_name, secret->entry_id);
 				if (!pwd)
 					pwd = g_strdup ("");
 			} else {
@@ -649,7 +647,7 @@ get_secrets_from_user (const char *request_id,
 					g_print ("%s\n", msg);
 				g_printerr (_("Warning: password for '%s' not given in 'passwd-file' "
 				              "and nmcli cannot ask without '--ask' option.\n"),
-				            secret->prop_name);
+				            secret->entry_id);
 			}
 		}
 		/* No password provided, cancel the secrets. */
diff --git a/clients/common/nm-secret-agent-simple.c b/clients/common/nm-secret-agent-simple.c
index 21aaf99540..ecd4e75db2 100644
--- a/clients/common/nm-secret-agent-simple.c
+++ b/clients/common/nm-secret-agent-simple.c
@@ -147,7 +147,6 @@ strv_has (gchar **haystack,
 
 typedef struct {
 	NMSecretAgentSimpleSecret base;
-
 	NMSetting *setting;
 	char *property;
 } NMSecretAgentSimpleSecretReal;
@@ -157,11 +156,10 @@ nm_secret_agent_simple_secret_free (NMSecretAgentSimpleSecret *secret)
 {
 	NMSecretAgentSimpleSecretReal *real = (NMSecretAgentSimpleSecretReal *)secret;
 
-	g_free (secret->name);
-	g_free (secret->prop_name);
+	g_free ((char *) secret->pretty_name);
+	g_free ((char *) secret->entry_id);
 	g_free (secret->value);
-	g_free (secret->vpn_property);
-	g_free (secret->vpn_type);
+	g_free ((char *) secret->vpn_type);
 	g_free (real->property);
 	g_clear_object (&real->setting);
 
@@ -169,33 +167,45 @@ nm_secret_agent_simple_secret_free (NMSecretAgentSimpleSecret *secret)
 }
 
 static NMSecretAgentSimpleSecret *
-nm_secret_agent_simple_secret_new (const char *name,
+nm_secret_agent_simple_secret_new (NMSecretAgentSecretType secret_type,
+                                   const char *pretty_name,
                                    NMSetting  *setting,
                                    const char *property,
-                                   const char *vpn_property,
-                                   const char *vpn_type,
-                                   gboolean    password)
+                                   const char *vpn_type)
 {
 	NMSecretAgentSimpleSecretReal *real;
+	const char *vpn_prefix;
+	const char *value;
+
+	nm_assert (property);
+	nm_assert (NM_IS_SETTING (setting));
 
 	real = g_slice_new0 (NMSecretAgentSimpleSecretReal);
-	real->base.name = g_strdup (name);
-	real->base.prop_name = vpn_property ?
-	                         g_strdup_printf ("%s.%s.%s", nm_setting_get_name (setting), property, vpn_property) :
-	                         g_strdup_printf ("%s.%s", nm_setting_get_name (setting), property);
-	real->base.vpn_property = g_strdup (vpn_property);
-	real->base.vpn_type = g_strdup (vpn_type);
-	real->base.password = password;
-
-	if (setting) {
-		real->setting = g_object_ref (setting);
-		real->property = g_strdup (property);
-
-		if (vpn_property)
-			real->base.value = g_strdup (nm_setting_vpn_get_secret (NM_SETTING_VPN (setting), vpn_property));
-		else
-			g_object_get (setting, property, &real->base.value, NULL);
+	*((NMSecretAgentSecretType *) &real->base.secret_type) = secret_type;
+	real->setting = g_object_ref (setting);
+	real->base.pretty_name = g_strdup (pretty_name);
+	real->property = g_strdup (property);
+	switch (secret_type) {
+	case NM_SECRET_AGENT_SECRET_TYPE_PROPERTY:
+	case NM_SECRET_AGENT_SECRET_TYPE_SECRET:
+		nm_assert (!vpn_type);
+		nm_assert (g_object_class_find_property (G_OBJECT_GET_CLASS (setting), property));
+		nm_assert ((secret_type == NM_SECRET_AGENT_SECRET_TYPE_SECRET) == nm_setting_get_secret_flags (setting, property, NULL, NULL));
+		real->base.entry_id = g_strdup_printf ("%s.%s", nm_setting_get_name (setting), property);
+		g_object_get (setting, property, &real->base.value, NULL);
+		real->base.is_secret = (secret_type != NM_SECRET_AGENT_SECRET_TYPE_PROPERTY);
+		break;
+	case NM_SECRET_AGENT_SECRET_TYPE_VPN_SECRET:
+		vpn_prefix = NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRET;
+		value = nm_setting_vpn_get_secret (NM_SETTING_VPN (setting), property);
+		real->base.entry_id = g_strdup_printf ("%s%s", vpn_prefix, property);
+		nm_assert (vpn_type);
+		real->base.vpn_type = g_strdup (vpn_type);
+		real->base.value = g_strdup (value);
+		real->base.is_secret = TRUE;
+		break;
 	}
+	nm_assert (real->base.entry_id);
 
 	return &real->base;
 }
@@ -220,37 +230,33 @@ add_8021x_secrets (NMSecretAgentSimpleRequest *request,
 		 * is not visible here since we only care about phase2 authentication
 		 * (and don't even care of which one)
 		 */
-		secret = nm_secret_agent_simple_secret_new (_("Username"),
+		secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_PROPERTY,
+		                                            _("Username"),
 		                                            NM_SETTING (s_8021x),
 		                                            NM_SETTING_802_1X_IDENTITY,
-		                                            NULL,
-		                                            NULL,
-		                                            FALSE);
+		                                            NULL);
 		g_ptr_array_add (secrets, secret);
-		secret = nm_secret_agent_simple_secret_new (_("Password"),
+		secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET,
+		                                            _("Password"),
 		                                            NM_SETTING (s_8021x),
 		                                            NM_SETTING_802_1X_PASSWORD,
-		                                            NULL,
-		                                            NULL,
-		                                            TRUE);
+		                                            NULL);
 		g_ptr_array_add (secrets, secret);
 		return TRUE;
 	}
 
 	if (!strcmp (eap_method, "tls")) {
-		secret = nm_secret_agent_simple_secret_new (_("Identity"),
+		secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_PROPERTY,
+		                                            _("Identity"),
 		                                            NM_SETTING (s_8021x),
 		                                            NM_SETTING_802_1X_IDENTITY,
-		                                            NULL,
-		                                            NULL,
-		                                            FALSE);
+		                                            NULL);
 		g_ptr_array_add (secrets, secret);
-		secret = nm_secret_agent_simple_secret_new (_("Private key password"),
+		secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET,
+		                                            _("Private key password"),
 		                                            NM_SETTING (s_8021x),
 		                                            NM_SETTING_802_1X_PRIVATE_KEY_PASSWORD,
-		                                            NULL,
-		                                            NULL,
-		                                            TRUE);
+		                                            NULL);
 		g_ptr_array_add (secrets, secret);
 		return TRUE;
 	}
@@ -270,12 +276,11 @@ add_wireless_secrets (NMSecretAgentSimpleRequest *request,
 		return FALSE;
 
 	if (!strcmp (key_mgmt, "wpa-none") || !strcmp (key_mgmt, "wpa-psk")) {
-		secret = nm_secret_agent_simple_secret_new (_("Password"),
+		secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET,
+		                                            _("Password"),
 		                                            NM_SETTING (s_wsec),
 		                                            NM_SETTING_WIRELESS_SECURITY_PSK,
-		                                            NULL,
-		                                            NULL,
-		                                            TRUE);
+		                                            NULL);
 		g_ptr_array_add (secrets, secret);
 		return TRUE;
 	}
@@ -286,12 +291,11 @@ add_wireless_secrets (NMSecretAgentSimpleRequest *request,
 
 		index = nm_setting_wireless_security_get_wep_tx_keyidx (s_wsec);
 		key = g_strdup_printf ("wep-key%d", index);
-		secret = nm_secret_agent_simple_secret_new (_("Key"),
+		secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET,
+		                                            _("Key"),
 		                                            NM_SETTING (s_wsec),
 		                                            key,
-		                                            NULL,
-		                                            NULL,
-		                                            TRUE);
+		                                            NULL);
 		g_free (key);
 
 		g_ptr_array_add (secrets, secret);
@@ -300,12 +304,11 @@ add_wireless_secrets (NMSecretAgentSimpleRequest *request,
 
 	if (!strcmp (key_mgmt, "iee8021x")) {
 		if (!g_strcmp0 (nm_setting_wireless_security_get_auth_alg (s_wsec), "leap")) {
-			secret = nm_secret_agent_simple_secret_new (_("Password"),
+			secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET,
+			                                            _("Password"),
 			                                            NM_SETTING (s_wsec),
 			                                            NM_SETTING_WIRELESS_SECURITY_LEAP_PASSWORD,
-			                                            NULL,
-			                                            NULL,
-			                                            TRUE);
+			                                            NULL);
 			g_ptr_array_add (secrets, secret);
 			return TRUE;
 		} else
@@ -325,26 +328,23 @@ add_pppoe_secrets (NMSecretAgentSimpleRequest *request,
 	NMSettingPppoe *s_pppoe = nm_connection_get_setting_pppoe (request->connection);
 	NMSecretAgentSimpleSecret *secret;
 
-	secret = nm_secret_agent_simple_secret_new (_("Username"),
+	secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_PROPERTY,
+	                                            _("Username"),
 	                                            NM_SETTING (s_pppoe),
 	                                            NM_SETTING_PPPOE_USERNAME,
-	                                            NULL,
-	                                            NULL,
-	                                            FALSE);
+	                                            NULL);
 	g_ptr_array_add (secrets, secret);
-	secret = nm_secret_agent_simple_secret_new (_("Service"),
+	secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_PROPERTY,
+	                                            _("Service"),
 	                                            NM_SETTING (s_pppoe),
 	                                            NM_SETTING_PPPOE_SERVICE,
-	                                            NULL,
-	                                            NULL,
-	                                            FALSE);
+	                                            NULL);
 	g_ptr_array_add (secrets, secret);
-	secret = nm_secret_agent_simple_secret_new (_("Password"),
+	secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET,
+	                                            _("Password"),
 	                                            NM_SETTING (s_pppoe),
 	                                            NM_SETTING_PPPOE_PASSWORD,
-	                                            NULL,
-	                                            NULL,
-	                                            TRUE);
+	                                            NULL);
 	g_ptr_array_add (secrets, secret);
 	return TRUE;
 }
@@ -369,23 +369,27 @@ add_vpn_secret_helper (GPtrArray *secrets, NMSettingVpn *s_vpn, const char *name
 	NMSettingSecretFlags flags;
 	int i;
 
-	/* Check for duplicates */
-	for (i = 0; i < secrets->len; i++) {
-		secret = secrets->pdata[i];
-
-		if (g_strcmp0 (secret->vpn_property, name) == 0)
-			return;
-	}
-
 	flags = get_vpn_secret_flags (s_vpn, name);
 	if (   flags & NM_SETTING_SECRET_FLAG_AGENT_OWNED
 	    || flags & NM_SETTING_SECRET_FLAG_NOT_SAVED) {
-		secret = nm_secret_agent_simple_secret_new (ui_name,
+		secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_VPN_SECRET,
+		                                            ui_name,
 		                                            NM_SETTING (s_vpn),
-		                                            NM_SETTING_VPN_SECRETS,
 		                                            name,
-		                                            nm_setting_vpn_get_service_type (s_vpn),
-		                                            TRUE);
+		                                            nm_setting_vpn_get_service_type (s_vpn));
+
+		/* Check for duplicates */
+		for (i = 0; i < secrets->len; i++) {
+			NMSecretAgentSimpleSecret *s = secrets->pdata[i];
+
+			if (   s->secret_type == secret->secret_type
+			    && nm_streq0 (s->vpn_type, secret->vpn_type)
+			    && nm_streq0 (s->entry_id, secret->entry_id)) {
+				nm_secret_agent_simple_secret_free (secret);
+				return;
+			}
+		}
+
 		g_ptr_array_add (secrets, secret);
 	}
 }
@@ -417,6 +421,7 @@ add_vpn_secrets (NMSecretAgentSimpleRequest *request,
 	/* Now add what client thinks might be required, because hints may be empty or incomplete */
 	p = secret_names = nm_vpn_get_secret_names (nm_setting_vpn_get_service_type (s_vpn));
 	while (p && p->name) {
+		g_print (">>> request %s = %s\n", p->name, p->ui_name);
 		add_vpn_secret_helper (secrets, s_vpn, p->name, _(p->ui_name));
 		p++;
 	}
@@ -489,24 +494,22 @@ request_secrets_from_ui (NMSecretAgentSimpleRequest *request)
 			title = _("PIN code required");
 			msg = g_strdup (_("PIN code is needed for the mobile broadband device"));
 
-			secret = nm_secret_agent_simple_secret_new (_("PIN"),
+			secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_PROPERTY,
+			                                            _("PIN"),
 			                                            NM_SETTING (s_gsm),
 			                                            NM_SETTING_GSM_PIN,
-			                                            NULL,
-			                                            NULL,
-			                                            FALSE);
+			                                            NULL);
 			g_ptr_array_add (secrets, secret);
 		} else {
 			title = _("Mobile broadband network password");
 			msg = g_strdup_printf (_("A password is required to connect to '%s'."),
 			                       nm_connection_get_id (request->connection));
 
-			secret = nm_secret_agent_simple_secret_new (_("Password"),
+			secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET,
+			                                            _("Password"),
 			                                            NM_SETTING (s_gsm),
 			                                            NM_SETTING_GSM_PASSWORD,
-			                                            NULL,
-			                                            NULL,
-			                                            TRUE);
+			                                            NULL);
 			g_ptr_array_add (secrets, secret);
 		}
 	} else if (nm_connection_is_type (request->connection, NM_SETTING_MACSEC_SETTING_NAME)) {
@@ -517,12 +520,11 @@ request_secrets_from_ui (NMSecretAgentSimpleRequest *request)
 
 		if (nm_setting_macsec_get_mode (s_macsec) == NM_SETTING_MACSEC_MODE_PSK) {
 			title = _("MACsec PSK authentication");
-			secret = nm_secret_agent_simple_secret_new (_("MKA CAK"),
+			secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET,
+			                                            _("MKA CAK"),
 			                                            NM_SETTING (s_macsec),
 			                                            NM_SETTING_MACSEC_MKA_CAK,
-			                                            NULL,
-			                                            NULL,
-			                                            TRUE);
+			                                            NULL);
 			g_ptr_array_add (secrets, secret);
 		} else {
 			title = _("MACsec EAP authentication");
@@ -535,12 +537,11 @@ request_secrets_from_ui (NMSecretAgentSimpleRequest *request)
 		msg = g_strdup_printf (_("A password is required to connect to '%s'."),
 		                       nm_connection_get_id (request->connection));
 
-		secret = nm_secret_agent_simple_secret_new (_("Password"),
+		secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET,
+		                                            _("Password"),
 		                                            NM_SETTING (s_cdma),
 		                                            NM_SETTING_CDMA_PASSWORD,
-		                                            NULL,
-		                                            NULL,
-		                                            TRUE);
+		                                            NULL);
 		g_ptr_array_add (secrets, secret);
 	} else if (nm_connection_is_type (request->connection, NM_SETTING_BLUETOOTH_SETTING_NAME)) {
 		NMSetting *setting = NULL;
@@ -558,12 +559,11 @@ request_secrets_from_ui (NMSecretAgentSimpleRequest *request)
 			msg = g_strdup_printf (_("A password is required to connect to '%s'."),
 			                       nm_connection_get_id (request->connection));
 
-			secret = nm_secret_agent_simple_secret_new (_("Password"),
+			secret = nm_secret_agent_simple_secret_new (NM_SECRET_AGENT_SECRET_TYPE_SECRET,
+			                                            _("Password"),
 			                                            setting,
 			                                            "password",
-			                                            NULL,
-			                                            NULL,
-			                                            TRUE);
+			                                            NULL);
 			g_ptr_array_add (secrets, secret);
 		} else
 			ok = FALSE;
@@ -690,9 +690,7 @@ nm_secret_agent_simple_response (NMSecretAgentSimple *self,
 		GHashTable *settings;
 		GHashTableIter iter;
 		const char *name;
-		const char *vpn_secrets_base_name = NULL;
-
-		g_variant_builder_init (&vpn_secrets_builder, G_VARIANT_TYPE ("a{ss}"));
+		gboolean has_vpn = FALSE;
 
 		settings = g_hash_table_new (nm_str_hash, g_str_equal);
 		for (i = 0; i < secrets->len; i++) {
@@ -705,22 +703,27 @@ nm_secret_agent_simple_response (NMSecretAgentSimple *self,
 				                     setting_builder);
 			}
 
-			if (secret->base.vpn_property) {
-				/* VPN secrets need slightly different treatment.
-				 * "secrets" property is actually a hash table of secrets. */
-				vpn_secrets_base_name = secret->property;
-				g_variant_builder_add (&vpn_secrets_builder, "{ss}",
-				                       secret->base.vpn_property, secret->base.value);
-			} else {
+			switch (secret->base.secret_type) {
+			case NM_SECRET_AGENT_SECRET_TYPE_PROPERTY:
+			case NM_SECRET_AGENT_SECRET_TYPE_SECRET:
 				g_variant_builder_add (setting_builder, "{sv}",
 				                       secret->property,
 				                       g_variant_new_string (secret->base.value));
+				break;
+			case NM_SECRET_AGENT_SECRET_TYPE_VPN_SECRET:
+				if (!has_vpn) {
+					g_variant_builder_init (&vpn_secrets_builder, G_VARIANT_TYPE ("a{ss}"));
+					has_vpn = TRUE;
+				}
+				g_variant_builder_add (&vpn_secrets_builder, "{ss}",
+				                       secret->property, secret->base.value);
+				break;
 			}
 		}
 
-		if (vpn_secrets_base_name) {
+		if (has_vpn) {
 			g_variant_builder_add (setting_builder, "{sv}",
-			                       vpn_secrets_base_name,
+			                       "secrets",
 			                       g_variant_builder_end (&vpn_secrets_builder));
 		}
 
diff --git a/clients/common/nm-secret-agent-simple.h b/clients/common/nm-secret-agent-simple.h
index 2989723dbf..c0f6befcb4 100644
--- a/clients/common/nm-secret-agent-simple.h
+++ b/clients/common/nm-secret-agent-simple.h
@@ -42,13 +42,25 @@ typedef struct {
 
 } NMSecretAgentSimpleClass;
 
+typedef enum {
+	NM_SECRET_AGENT_SECRET_TYPE_PROPERTY,
+	NM_SECRET_AGENT_SECRET_TYPE_SECRET,
+	NM_SECRET_AGENT_SECRET_TYPE_VPN_SECRET,
+} NMSecretAgentSecretType;
+
 typedef struct {
-	char *name, *prop_name, *value;
-	char *vpn_property;
-	char *vpn_type;
-	gboolean password;
+	const NMSecretAgentSecretType secret_type;
+	const char *pretty_name;
+	const char *entry_id;
+	char *value;
+	const char *vpn_type;
+	gboolean is_secret;
 } NMSecretAgentSimpleSecret;
 
+#define NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRET "vpn.secret."
+
+#define NM_SECRET_AGENT_VPN_TYPE_OPENCONNECT  NM_DBUS_INTERFACE".openconnect"
+
 GType nm_secret_agent_simple_get_type (void);
 
 NMSecretAgentOld *nm_secret_agent_simple_new                 (const char          *name);
diff --git a/clients/tui/nmt-password-dialog.c b/clients/tui/nmt-password-dialog.c
index fb9567e691..16920fcf39 100644
--- a/clients/tui/nmt-password-dialog.c
+++ b/clients/tui/nmt-password-dialog.c
@@ -144,12 +144,12 @@ nmt_password_dialog_constructed (GObject *object)
 		NMSecretAgentSimpleSecret *secret = priv->secrets->pdata[i];
 		NmtNewtEntryFlags flags;
 
-		widget = nmt_newt_label_new (secret->name);
+		widget = nmt_newt_label_new (secret->pretty_name);
 		nmt_newt_grid_add (secret_grid, widget, 0, i);
 		nmt_newt_widget_set_padding (widget, 4, 0, 1, 0);
 
 		flags = NMT_NEWT_ENTRY_NONEMPTY;
-		if (secret->password)
+		if (secret->is_secret)
 			flags |= NMT_NEWT_ENTRY_PASSWORD;
 		widget = nmt_newt_entry_new (30, flags);
 		if (secret->value)
diff --git a/clients/tui/nmtui-connect.c b/clients/tui/nmtui-connect.c
index 086e4bd3fb..2a954fb8cb 100644
--- a/clients/tui/nmtui-connect.c
+++ b/clients/tui/nmtui-connect.c
@@ -100,41 +100,38 @@ secrets_requested (NMSecretAgentSimple *agent,
 {
 	NmtNewtForm *form;
 	NMConnection *connection = NM_CONNECTION (user_data);
-	char *cookie = NULL;
-	char *gateway = NULL;
-	char *gwcert = NULL;
 	int i;
 
 	/* Get secrets for OpenConnect VPN */
-	if (connection && nm_connection_is_type (connection, NM_SETTING_VPN_SETTING_NAME)) {
+	if (   connection
+	    && nm_connection_is_type (connection, NM_SETTING_VPN_SETTING_NAME)) {
 		NMSettingVpn *s_vpn = nm_connection_get_setting_vpn (connection);
-		const char *vpn_type = nm_setting_vpn_get_service_type (s_vpn);
 
-		if (!g_strcmp0 (vpn_type, NM_DBUS_INTERFACE ".openconnect")) {
+		if (nm_streq0 (nm_setting_vpn_get_service_type (s_vpn), NM_SECRET_AGENT_VPN_TYPE_OPENCONNECT)) {
+			gs_free char *cookie = NULL;
+			gs_free char *gateway = NULL;
+			gs_free char *gwcert = NULL;
+
 			openconnect_authenticate (connection, &cookie, &gateway, &gwcert);
 
 			for (i = 0; i < secrets->len; i++) {
 				NMSecretAgentSimpleSecret *secret = secrets->pdata[i];
 
-				if (!g_strcmp0 (secret->vpn_type, NM_DBUS_INTERFACE ".openconnect")) {
-					if (!g_strcmp0 (secret->vpn_property, "cookie")) {
-						g_free (secret->value);
-						secret->value = cookie;
-						cookie = NULL;
-					} else if (!g_strcmp0 (secret->vpn_property, "gateway")) {
-						g_free (secret->value);
-						secret->value = gateway;
-						gateway = NULL;
-					} else if (!g_strcmp0 (secret->vpn_property, "gwcert")) {
-						g_free (secret->value);
-						secret->value = gwcert;
-						gwcert = NULL;
-					}
+				if (secret->secret_type != NM_SECRET_AGENT_SECRET_TYPE_VPN_SECRET)
+					continue;
+				if (!nm_streq0 (secret->vpn_type, NM_SECRET_AGENT_VPN_TYPE_OPENCONNECT))
+					continue;
+				if (nm_streq0 (secret->entry_id, NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRET "cookie")) {
+					g_free (secret->value);
+					secret->value = g_steal_pointer (&cookie);
+				} else if (nm_streq0 (secret->entry_id, NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRET "gateway")) {
+					g_free (secret->value);
+					secret->value = g_steal_pointer (&gateway);
+				} else if (nm_streq0 (secret->entry_id, NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRET "gwcert")) {
+					g_free (secret->value);
+					secret->value = g_steal_pointer (&gwcert);
 				}
 			}
-			g_free (cookie);
-			g_free (gateway);
-			g_free (gwcert);
 		}
 	}