]> www.infradead.org Git - users/dwmw2/openconnect.git/commitdiff
Fix issue causing front-ends/GUIs to be insensitive to changes in the Juniper realm...
authorDaniel Lenski <dlenski@gmail.com>
Mon, 8 Oct 2018 17:39:54 +0000 (10:39 -0700)
committerDaniel Lenski <dlenski@gmail.com>
Mon, 8 Oct 2018 17:39:54 +0000 (10:39 -0700)
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 <dlenski@gmail.com>
library.c

index 5ee2dbc85ea1d350b4684db15ae9bea43d51e617..303278ee40a091c3ac2a35bbceee9f2db67190af 100644 (file)
--- 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))