Page MenuHomePhabricator

AuthManager shouldn't allow the browser to show suggestions for input fields (autocomplete)
Closed, ResolvedPublic

Description

OATHAuth has a field, which, because of it's function, requires the user to insert different, "random" numbers. This can result in the following, if the browser supports the autocomplete function:

This isn't really the expected result, as the browser should not suggest anything for the field. I'm not sure, if we should make this by default in AuthManager, or if we should implement an option for getFieldInfo for an AuthenticationRequest to specify some HTMLForm options (and OATHAuth could add autocomplete => off).

Event Timeline

Florian created this task.Jul 31 2016, 7:58 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 31 2016, 7:58 PM

Change 308353 had a related patch set uploaded (by Florianschmidtwelzow):
Set autocomplete='off' to AuthManager forms

https://gerrit.wikimedia.org/r/308353

See discussion at https://bugzilla.mozilla.org/show_bug.cgi?id=956906 for some reasons that autocomplete=off can be undesirable. In general it is the website deciding that they are smarter than the user and attempting to override their browser configuration.

autocomplete=off can be undesirable

That. Basically all web browsers [citation needed] nowadays ignore the "autocomplete" attribute.

I haven't read the complete discussion in the task linked by @bd808, but, it seems that it doesn't cover what I request :) The browser or any password manager should still be possible to fill in or save saved passwords, I don't see any risk in this, as long as the user actively wants this. However, providing a list of previously used (and mostly now incorrect) one-time passwords to the user when clicking into the field seems wrong and misleading. That's why I would want to not show this list at all.

What I see now is, that the HTMLForm autocomplete attribute is set to the form, which also feels wrong, as the better solution would be to turn the autocomplete off field-based, not form-based. Would this be ok from you all? :)

What I see now is, that the HTMLForm autocomplete attribute is set to the form, which also feels wrong, as the better solution would be to turn the autocomplete off field-based, not form-based. Would this be ok from you all? :)

Per-field control is a better idea than blanket disable of autocomplete. I really don't see the harm in the behavior you are showing in the screenshot for a user who has explicitly configured their browser to save form inputs.

I think that in the real world you will still find that support/enforcement is going to vary widely based on the particular browser brand and version in use. WHATWG is trying to encourage a more semantic system than the on/off flag for autocomplete and Chromium is recommending using a semantic but unrecognized value to discourage autofill.

Hmm, based on the problem that HTMLForm currently does not allow to set the autocomplete bit on a per-field basis, this would be a bigger change in both mediawiki/core and at least OATHAuth. Based on the comments and the links I feel, that this is uselss work, as:

  • the benefit for the user is realtively small (totally agree!)
  • "the web" is trying to find better solutions instead of simply on/off switch (with which I agree, too)

So, in fact, I'm not sure if a bigger change for enabling a feature in HTMLForm, which seems to be not needed anywhere else at least there's no-one asking for it, is really the best way here. So, maybe this sounds more like a "Declined", in Bugzilla words, I would use "WONTFIX" :D?

Change 322139 had a related patch set uploaded (by Huji):
Disable autocompletion of one-time tokens

https://gerrit.wikimedia.org/r/322139

Change 308353 abandoned by Florianschmidtwelzow:
Set autocomplete='off' to AuthManager forms

Reason:
I'll take a look on Ib75b078cf9b844fcde21758fcbf7ab6a1cd36187 later, test it and probably merge it. It's a way better solution as this one (like Bryan said, setting the whole form to autocomplete=off is probably not the best idea, just because one field should not be autocompleted) :) Thanks for the feedback from all of you! :)

https://gerrit.wikimedia.org/r/308353

Florian closed this task as Resolved.Nov 18 2016, 6:31 PM
Florian added a subscriber: Tgr.

*yay* Thanks @Tgr!

Change 322139 merged by jenkins-bot:
Disable autocompletion of one-time tokens

https://gerrit.wikimedia.org/r/322139

Tgr added a comment.Nov 18 2016, 10:20 PM

FWIW this is also a (very minor) security issue as autocomplete="off" also stops the browser from storing the submitted value in browser history and repopulating when the user navigates back. OATH tokens are blacklisted on use and only valid for a short time, and the secret seed should be unguessable from any reasonably small amount of one-time codes, so the ability of an attacker with physical access to the machine to recover tokens should not matter much, but still it's nice that it's denied.