nmcli, nmtui: reduce duplication around openconnect auth helper

Pull a bunch of stuff into nm_vpn_openconnect_authenticate_helper() that
both callers were doing for themselves, and make its API a bit simpler.
It's given the NMSettingVpn and the GPtrArray of secrets, and it simply
succeeds or fails.

src/libnmc-base/nm-vpn-helpers.c

@@ -16,6 +16,7 @@
 #include <net/if.h>
 
 #include "nm-client-utils.h"
+#include "nm-secret-agent-simple.h"
 #include "nm-utils.h"
 #include "libnm-glib-aux/nm-io-utils.h"
 #include "libnm-glib-aux/nm-secret-utils.h"
@@ -233,18 +234,16 @@ struct {
 #define OC_ARGS_MAX        (12 + 2 * NR_OC_STRING_PROPS)
 
 gboolean
-nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn,
-                                       char        **cookie,
-                                       char        **gateway,
-                                       char        **gwcert,
-                                       char        **resolve,
-                                       int          *status,
-                                       GError      **error)
+nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, GPtrArray *secrets, GError **error)
 {
     gs_free char        *output      = NULL;
     gs_free char        *legacy_host = NULL;
     gs_free char        *connect_url = NULL;
+    gs_free char        *cookie      = NULL;
+    gs_free char        *gwcert      = NULL;
+    gs_free char        *resolve     = NULL;
     gs_free const char **output_v    = NULL;
+    int                  status      = 0;
     const char *const   *iter;
     const char          *path;
     const char          *opt;
@@ -333,10 +332,27 @@ nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn,
                       NULL,
                       &output,
                       NULL,
-                      status,
+                      &status,
                       error))
         return FALSE;
 
+    if (WIFEXITED(status) && WEXITSTATUS(status) != 0) {
+        /* The caller will prepend "Error: openconnect failed: " to this */
+        g_set_error(error,
+                    NM_VPN_PLUGIN_ERROR,
+                    NM_VPN_PLUGIN_ERROR_FAILED,
+                    _("exited with status %d"),
+                    WEXITSTATUS(status));
+        return FALSE;
+    } else if (WIFSIGNALED(status)) {
+        g_set_error(error,
+                    NM_VPN_PLUGIN_ERROR,
+                    NM_VPN_PLUGIN_ERROR_FAILED,
+                    _("exited on signal %d"),
+                    WTERMSIG(status));
+        return FALSE;
+    }
+
     /* Parse output and set cookie, gateway and gwcert
      * output example:
      * COOKIE='loremipsum'
@@ -352,27 +368,49 @@ nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn,
     for (iter = output_v; iter && *iter; iter++) {
         char *s_mutable = (char *) *iter;
 
-        _extract_variable_value(s_mutable, "COOKIE=", cookie);
+        _extract_variable_value(s_mutable, "COOKIE=", &cookie);
         _extract_variable_value(s_mutable, "CONNECT_URL=", &connect_url);
         _extract_variable_value(s_mutable, "HOST=", &legacy_host);
-        _extract_variable_value(s_mutable, "FINGERPRINT=", gwcert);
-        _extract_variable_value(s_mutable, "RESOLVE=", resolve);
+        _extract_variable_value(s_mutable, "FINGERPRINT=", &gwcert);
+        _extract_variable_value(s_mutable, "RESOLVE=", &resolve);
     }
 
-    if (connect_url) {
-        *gateway = g_steal_pointer(&connect_url);
-    } else {
-        if (!legacy_host) {
-            g_set_error(error,
-                        NM_VPN_PLUGIN_ERROR,
-                        NM_VPN_PLUGIN_ERROR_FAILED,
-                        _("OpenConnect failed to return gateway URL"));
-            return FALSE;
+    if (!cookie || !gwcert || (!legacy_host && !connect_url)) {
+        g_set_error(error,
+                    NM_VPN_PLUGIN_ERROR,
+                    NM_VPN_PLUGIN_ERROR_FAILED,
+                    _("insufficent secrets returned"));
+        return FALSE;
+    }
+
+    for (i = 0; i < secrets->len; i++) {
+        NMSecretAgentSimpleSecret *secret = secrets->pdata[i];
+
+        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_SECRETS "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_SECRETS "gateway")) {
+            g_free(secret->value);
+            if (connect_url)
+                secret->value = g_steal_pointer(&connect_url);
+            else if (port)
+                secret->value = g_strdup_printf("%s%s", legacy_host, port);
+            else
+                secret->value = g_steal_pointer(&legacy_host);
+        } else if (nm_streq0(secret->entry_id,
+                             NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "gwcert")) {
+            g_free(secret->value);
+            secret->value = g_steal_pointer(&gwcert);
+        } else if (nm_streq0(secret->entry_id,
+                             NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "resolve")) {
+            g_free(secret->value);
+            secret->value = g_steal_pointer(&resolve);
         }
-        if (port)
-            *gateway = g_strdup_printf("%s%s", legacy_host, port);
-        else
-            *gateway = g_steal_pointer(&legacy_host);
     }
 
     return TRUE;

src/libnmc-base/nm-vpn-helpers.h

@@ -19,12 +19,7 @@ gboolean nm_vpn_supports_ipv6(NMConnection *connection);
 
 const NmcVpnPasswordName *nm_vpn_get_secret_names(const char *service_type);
 
-gboolean nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn,
-                                                char        **cookie,
-                                                char        **gateway,
-                                                char        **gwcert,
-                                                char        **resolve,
-                                                int          *status,
-                                                GError      **error);
+gboolean
+nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, GPtrArray *secrets, GError **error);
 
 #endif /* __NM_VPN_HELPERS_H__ */

src/nmcli/common.c

@@ -635,12 +635,6 @@ vpn_openconnect_get_secrets(NMConnection *connection, GPtrArray *secrets)
 {
     GError       *error = NULL;
     NMSettingVpn *s_vpn;
-    gs_free char *cookie  = NULL;
-    gs_free char *gateway = NULL;
-    gs_free char *gwcert  = NULL;
-    gs_free char *resolve = NULL;
-    int           status  = 0;
-    int           i;
     gboolean      ret;
 
     if (!connection)
@@ -654,52 +648,14 @@ vpn_openconnect_get_secrets(NMConnection *connection, GPtrArray *secrets)
         return FALSE;
 
     /* Interactively authenticate to OpenConnect server and get secrets */
-    ret = nm_vpn_openconnect_authenticate_helper(s_vpn,
-                                                 &cookie,
-                                                 &gateway,
-                                                 &gwcert,
-                                                 &resolve,
-                                                 &status,
-                                                 &error);
+    ret = nm_vpn_openconnect_authenticate_helper(s_vpn, secrets, &error);
+
     if (!ret) {
         nmc_printerr(_("Error: openconnect failed: %s\n"), error->message);
         g_clear_error(&error);
         return FALSE;
     }
 
-    if (WIFEXITED(status)) {
-        if (WEXITSTATUS(status) != 0)
-            nmc_printerr(_("Error: openconnect failed with status %d\n"), WEXITSTATUS(status));
-    } else if (WIFSIGNALED(status))
-        nmc_printerr(_("Error: openconnect failed with signal %d\n"), WTERMSIG(status));
-
-    /* Fill secrets to the array */
-    for (i = 0; i < secrets->len; i++) {
-        NMSecretAgentSimpleSecret *secret = secrets->pdata[i];
-
-        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_SECRETS "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_SECRETS "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_SECRETS "gwcert")) {
-            g_free(secret->value);
-            secret->value = g_steal_pointer(&gwcert);
-        } else if (nm_streq0(secret->entry_id,
-                             NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "resolve")) {
-            g_free(secret->value);
-            secret->value = g_steal_pointer(&resolve);
-        }
-    }
-
     return TRUE;
 }
 

src/nmtui/nmtui-connect.c

@@ -31,16 +31,11 @@
  * before starting the command and restored after it returns.
  */
 static gboolean
-openconnect_authenticate(NMConnection *connection,
-                         char        **cookie,
-                         char        **gateway,
-                         char        **gwcert,
-                         char        **resolve)
+openconnect_authenticate(NMConnection *connection, GPtrArray *secrets)
 {
     GError       *error = NULL;
     NMSettingVpn *s_vpn;
     gboolean      ret;
-    int           status = 0;
 
     nmt_newt_message_dialog(
         _("openconnect will be run to authenticate.\nIt will return to nmtui when completed."));
@@ -50,13 +45,7 @@ openconnect_authenticate(NMConnection *connection,
 
     newtSuspend();
 
-    ret = nm_vpn_openconnect_authenticate_helper(s_vpn,
-                                                 cookie,
-                                                 gateway,
-                                                 gwcert,
-                                                 resolve,
-                                                 &status,
-                                                 &error);
+    ret = nm_vpn_openconnect_authenticate_helper(s_vpn, secrets, &error);
 
     newtResume();
 
@@ -66,16 +55,6 @@ openconnect_authenticate(NMConnection *connection,
         return FALSE;
     }
 
-    if (WIFEXITED(status)) {
-        if (WEXITSTATUS(status) != 0) {
-            nmt_newt_message_dialog(_("openconnect failed with status %d"), WEXITSTATUS(status));
-            return FALSE;
-        }
-    } else if (WIFSIGNALED(status)) {
-        nmt_newt_message_dialog(_("openconnect failed with signal %d"), WTERMSIG(status));
-        return FALSE;
-    }
-
     return TRUE;
 }
 
@@ -89,7 +68,6 @@ secrets_requested(NMSecretAgentSimple *agent,
 {
     NmtNewtForm  *form;
     NMConnection *connection = NM_CONNECTION(user_data);
-    int           i;
 
     /* Get secrets for OpenConnect VPN */
     if (connection && nm_connection_is_type(connection, NM_SETTING_VPN_SETTING_NAME)) {
@@ -97,38 +75,7 @@ secrets_requested(NMSecretAgentSimple *agent,
 
         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;
-            gs_free char *resolve = NULL;
-
-            openconnect_authenticate(connection, &cookie, &gateway, &gwcert, &resolve);
-
-            for (i = 0; i < secrets->len; i++) {
-                NMSecretAgentSimpleSecret *secret = secrets->pdata[i];
-
-                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_SECRETS "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_SECRETS "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_SECRETS "gwcert")) {
-                    g_free(secret->value);
-                    secret->value = g_steal_pointer(&gwcert);
-                } else if (nm_streq0(secret->entry_id,
-                                     NM_SECRET_AGENT_ENTRY_ID_PREFX_VPN_SECRETS "resolve")) {
-                    g_free(secret->value);
-                    secret->value = g_steal_pointer(&resolve);
-                }
-            }
+            openconnect_authenticate(connection, secrets);
         }
     }