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

David Woodhouse dwmw2 at infradead.org
Mon Aug 14 12:48:21 PDT 2017


On Mon, 2017-08-14 at 12:17 -0700, Daniel Lenski wrote:
> > They can remember the user's answers for *all* form fields.
>
> In my experience the GUIs remember some fields *incorrectly*, as when
> they try to auto-fill a saved password into what is actually an OTP
> field with the same name.

Yeah, they currently use the form name and the field name; if both of
those are repeated then they'll get a false match.

There is separately a feature request for NM-openconnect to allow it to
save *some* password fields but not others, which would be useful.

> 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... 

> > 
> > 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.

How about we stick with strncmp() on the first four letters for now,
and we can work something better out later. 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.

But strncmp() will do for now. I've made you jump through enough "just
fix this other thing..." hoops already :)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 4938 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/openconnect-devel/attachments/20170814/e0eaa50a/attachment-0001.bin>


More information about the openconnect-devel mailing list