I have two reasons for wanting the OTP fields to be of the type 'password'.

It is a one time _password_. A user doesn't get any smarter from this -- at best, more confused -- hence this should be hidden.

In the case that one uses username+password+Yubikey for login and the user types a wrong password, then the OTP field isn't cleared. But as the OTP has been used, it it no longer valid and hence the field should be cleared for a new OTP. (Alternatively, the validation should occur in a different order, such that the OTP isn't used in the case user/pass combination is wrong.)

CommentFileSizeAuthor
#3 otp-autocomplete.diff1.32 KBrasmuslp
otp-field-password.patch1.3 KBrasmuslp

Comments

toddejohnson’s picture

toddejohnson commented

Thanks for the patch! They are always appreciated and motivate me to get them included.

I feel it would be wise to stay with the standard way YubiKeys are used elsewhere for the default. I only see mediawiki that uses the password style. In my quick look I couldn't find a design guide. Possibly set this up to be a option for the administrator?

I personally agree that passwords would be preferred as then the computer wouldn't keep a history of them and they can be confusing. However would that cause issues for sites where passwords might be memorized by browsers and users who use that feature? I have done some research on this in the past and many suggest using the sessionid as a part of the form field to make the field unique.

rasmuslp’s picture

rasmuslp commented

I found a document for Module Development and Design Guidelines, but I can't find anything in it describing the type of field that shold be used for the OTP. I have only used Yubikeys for Lastpass -- and they also use a password field for the OTP -- this was my incentive for changing this.

Again, as I am a Lastpass user, I actually have no idea how browsers builtin password managers would interpret this. But as it is now, if the browser saves the username and password field, then why doesn't also save the OTP field ? It's the same type as the username field. I will try to research this.

rasmuslp’s picture

rasmuslp commented
StatusFileSize
new otp-autocomplete.diff1.32 KB

I have tested with multiple password fields, but browser behaviour (FF 12 and Chrome 18) was not consistant. If I set 'autocomplete=off' on the OTP field, it actually works in Chrome.

However, setting the 'autocomplete=off' attribute on the OTP fields will at least remove the autocomplete part of the problem. Diff attatched.

toddejohnson’s picture

toddejohnson commented
Title: OTP fields as password fields instead of textfield(s) » OTP fields autocomplete
Assigned: Unassigned » toddejohnson

I recived a response from Yubico:

We have experience
with both variants, and both will work, but our experience has made us
prefer text fields. Password fields interact badly with password
manager and browser "save password" features. It is also more obvious
to the user what happens when they can see the OTP in the input field.
About the only time we use password fields is when the password and OTP
is combined in the same field, but we try to stay out of that as well,
for similar reasons.

For this reason I'm going to prefer text fields. I changed the title to represent this.

rasmuslp’s picture

rasmuslp commented

I can live with that. I will, however, still recommend my patch from above to disable autocomplete on the OTP field. Unless you can come up with a better solution of course. Do you have anything particular in mind ?

toddejohnson’s picture

toddejohnson commented
Status: Active » Fixed

I've applied your patch. I'm also back porting it to 6.x-2.x. I don't want to expose the sessionid by using that to randomize the field name as that might be a security risk.

Just a note this tag is not valid HTML although it is supported by every browser but Opera from my reading.

rasmuslp’s picture

rasmuslp commented

Thanks for the update.

It was my impression that it would be included in the HTML5 standard, therefore i would guess that it is only a matter of time before Opera supports it too.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.