On Mon, Aug 14, 2017 at 12:48 PM, David Woodhouse <dwmw2 at infradead.org> wrote: > On Mon, 2017-08-14 at 12:17 -0700, Daniel Lenski wrote: >> Fair point. However, I think there are cases where the other UIs could >> benefit. GlobalProtect OTP puts its one-time-tokens into a field that >> NM-plugin couldn't distinguish from a fixed password field. I thought >> it'd be better to provide general-purpose hints to the GUI, rather >> than trying to special-case this into each of them. > > Hm. Maybe we should have exposed OC_FORM_OPT_TOKEN to the GUI? Maybe > too late to change that now but we could add a flag... Yeah, I wondered about that as well. Ideally, OC_FORM_OPT_FILL_{USERNAME,PASSWORD,TOKEN} would all be exposed to the GUI so they can do as little non-standard diving of the meaning of the form fields as possible. >> > Besides... if I look in your auth-globalprotect.c it looks like you're >> > generating the field names out of thin air anyway. If you had used >> > "username" and "password" instead of "user" and "passwd" then this >> > wouldn't have been necessary at all, would it? :) >> I did that at first, but then I had to special-case-rename the fields >> to user and passwd when submitted them as an HTML form. >> >> I rewrote it with the hints because I thought it'd be more useful to >> have this general-purpose mechanism for recognizing the purpose of the >> fields independently from their names. > > Yeah, I do see that point. I'm really wary of making promises we can't > keep though. We have to make lots of assumptions and special cases in > the command-line code ? especially in Juniper auth where I'd *really* > like the GUIs to just use a WebView instead. But for the GUI side I was > trying not to make so many. Hmm, yes. I hadn't considered the Juniper mess at all, and how you'd probably like to get OpenConnect out of the HTML-parsing business. I see what you mean. > How about we stick with strncmp() on the first four letters for now, > and we can work something better out later. Sounds good! My updated [PATCH v2 2/8] does just that. > I'm leaning towards fixing > the general case of form fields in main.c by allowing something like > '--form-opt main:username=dwmw2', and then the --user and --passwd > special cases just become appropriate invocations of the same 'add > stored option' function, with the name of the form option being > protocol-specific. So instead of *:username we'd match main:username > for Cisco, and whatever:username for Juniper. And whatever:user for > GPST. Sounds like a plan! I just might resurrect my 2015 patch to concatenate the password and token code when that happens :-D (http://lists.infradead.org/pipermail/openconnect-devel/2015-December/003325.html) -Dan