[PATCH 2/8] add OC_FORM_OPT_FILL_{USERNAME, PASSWORD} flags to hint at purpose of a form field without requiring a specific name

Daniel Lenski dlenski at gmail.com
Mon Aug 14 21:42:20 PDT 2017


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



More information about the openconnect-devel mailing list