From 669c7d3e7a45aabc3035f8f538bbd36e23dbd3d8 Mon Sep 17 00:00:00 2001 From: Daniel Lenski Date: Mon, 8 Oct 2018 10:39:54 -0700 Subject: [PATCH] Fix issue causing front-ends/GUIs to be insensitive to changes in the Juniper realm dropdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This has been a persistent, puzzling issue (http://lists.infradead.org/pipermail/openconnect-devel/2018-July/004926.html, http://lists.infradead.org/pipermail/openconnect-devel/2017-November/004558.html, etc.). When connecting to a Juniper VPN from a front-end (e.g. NM-OpenConnect, OpenConnect-GUI for Windows, OpenConnect for Android), changing the selected realm/`authgroup` in the drop-down box causes the form to immediately reload *without* saving the change. This turned out to be a rather subtle issue… The meaning and usage of `vpninfo->authgroup` differs across the different protocols, which made this hard to isolate. * With AnyConnect, changing the authgroup value in the form is supposed to trigger an immediate reload of the form, since the form contents can differ from one authgroup to another. Hence a `process_auth_form` callback should immediately return `OC_FORM_RESULT_NEWGROUP` when the form value changes. * With Juniper, the authgroup dropdown don't *actually* need to trigger a reloading of the form, since the form won't change if the authgroup field changes. (At least not on any Juniper VPN I have access to.) However, it doesn't hurt anything either, and setting the dropdown as `form->authgroup_opt` allows CLI users to specify the desired setting with `--authgroup`, which is very convenient. * With GlobalProtect, the authgroup has been repurposed to represent the desired *gateway* to connect to, in the cases where the user is connecting via the *portal* interface. The authgroup selection always appears in a form by itself, currently. This similarly allows CLI users to pick the desired gateway with `--authgroup`. Long story short, the problem here was that `form->authgroup_selection` needed to be set to a specific index (within `form->authgroup_opt->choices[]`) of the currently selected value, in order for the GUI to show the right value as selected. If this wasn't set, then every time the selection was changed (causing the form handler to return `OC_FORM_RESULT_NEWGROUP`), the selected index would revert to `0` on the next iteration of the form. For AnyConnect, the `form->authgroup_selection` was already set correctly; for Juniper and GlobalProtect, it wasn't. It seems to me that the most robust fix here is to ensure that `process_auth_form` itself always sets `form->authgroup_selection` to the index of the value matching `vpninfo->authgroup` _before_ handing the form off `process_auth_form_cb`. Tested that this change makes Juniper realm dropdowns work correctly in the NM-OpenConnect and Android front-ends. Signed-off-by: Daniel Lenski --- library.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library.c b/library.c index 5ee2dbc8..303278ee 100644 --- a/library.c +++ b/library.c @@ -1107,9 +1107,9 @@ int process_auth_form(struct openconnect_info *vpninfo, struct oc_auth_form *for retry: auth_choice = NULL; - if (grp && grp->nr_choices && !vpninfo->xmlpost) { + if (grp && grp->nr_choices) { + /* Set group selection from authgroup */ if (vpninfo->authgroup) { - /* For non-XML-POST, the server doesn't tell us which group is selected */ int i; for (i = 0; i < grp->nr_choices; i++) if (!strcmp(grp->choices[i]->name, vpninfo->authgroup))