Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix issue causing front-ends/GUIs to be insensitive to changes in the…
… Juniper realm dropdown

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>
  • Loading branch information
dlenski committed Oct 8, 2018
1 parent 6d72123 commit 669c7d3
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions library.c
Expand Up @@ -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))
Expand Down

0 comments on commit 669c7d3

Please sign in to comment.