Add autocomplete="off" to bp-login widget password field

[Prometheus Fire]

This one came from one of my corporate clients.

The password input field in the BP Login Widget throws a security warning in IBM Security AppScan autocomplete is not disabled for password field.

This location in /buddypress/bp-core/bp-core-widgets/ on line 85.

Change from:

<input type="password" name="pwd" id="bp-login-widget-user-pass" class="input" value="" />

Change to:

<input type="password" name="pwd" id="bp-login-widget-user-pass" class="input" value="" autocomplete="off" />

This is a small and fairly simple fix, maybe in the next update?

[boonebgorges]

It's my understanding that browsers are dropping support for 'autocomplete=off' on password fields. See eg ​http://stackoverflow.com/questions/3868299/is-autocomplete-off-compatible-with-all-modern-browsers/21348793#21348793 and ​http://security.stackexchange.com/questions/49326/should-websites-be-allowed-to-disable-autocomplete-on-forms-or-fields. I believe that the only practical effect of setting autocomplete=off on password fields is to disable password managers. But, according to those links, modern password managers and browsers ignore that setting anyway. So I'm wondering if maybe the IBM Security AppScan ruleset is in the wrong in this case.

On the other hand, wp-login.php does use autocomplete=off for the password field (and the entire form, in fact). See ​https://core.trac.wordpress.org/changeset/15710 and ​https://core.trac.wordpress.org/ticket/24364

Do others have thoughts about best practices here?

[netweb]

Via one of the SO link above:

​http://googlechromereleases.blogspot.ro/2014/04/stable-channel-update.html
Google Chrome's Daniel Xie writes:

As we’ve previously discussed, Chrome will now offer to remember and fill password fields in the presence of autocomplete=off. This gives more power to users in spirit of the priority of constituencies, and it encourages the use of the Chrome password manager so users can have more complex passwords. This change does not affect non-password fields.

I like that statement and agree wholeheartedly with it.

That said, I think AppScan is "doing it right" and BuddyPress should set autocomplete=off

Another IBM application update, different app but same thoughts:

When the AUTOCOMPLETE attribute is not disabled, passwords and user names can be transparently stored by the browser, potentially exposing them to other users of the same workstation environment.

An attacker would require local access to the user’s browser in order to exploit this vulnerability. The exposure of this issue was rated as High since users could access the application from shared public Internet terminals (such as an Internet café). Should access to the application be restricted to only authorized and secured workstations, then the exposure would be rated as Low.

Resolution: Disable the AUTOCOMPLETE attribute on the form. For example:
<FORM AUTOCOMPLETE = “off”></FORM>

Summarising, in the context of BP by setting autocomplete=off this allows IBM AppScan to pass as valid, it will also be ignored by Google Chrome and any other browser or password manager that ignores this.

If you run an internet cafe and your allowing your customers to save passwords using a browsers password manager and sharing this with any and all customers using the same terminal then we cannot do anything about except weep if internet cafes are not using chrome://flags to disable autocomplete for their terminals.

Secondly, if patched may have to also implement some of the changes from ​#WP24364 re:

When the user wants to change a setting on the Profile screen, the first password field is auto-filled. That results in error on submitting the form: "ERROR: You entered your new password only once...".

[hnla]

​https://bugzilla.mozilla.org/show_bug.cgi?id=956906

The summary from a very long thread from a Mozi discussion on the subject:

Summary of the change, so people don't have to wade through a long discussion:

• This change makes it so that autocomplete=off does not stop the Password Manager >from working. Normal form autofill can be disabled as usual.
• The password manager *always* prompts if it wants to save a password. Passwords are >not saved without permission from the user.
• We are the third browser to implement this change, after IE and Chrome.
• This can be undone locally by flipping the signon.storeWhenAutocompleteOff pref >(from about:config) off.
• The rationale behind this change was the widespread abuse of the autocomplete >attribute to prevent password saving where no prevention is required. This change gives >users full control over password saving, without compromising on security (again, the user >is always prompted).

Seems overall that autocomplete=off should be implemented, the main concern in doing so being that preventing browsers auto saving to password managers would be a very bad thing possibly resulting in people using weak passwords where they might have been using very strong ones in the knowledge that a browser action by user would have the password inserted to field.

It seems that Mozi here acknowledge that they are the last to implement a fix for autocomplete disabling their password saving thus all major browsers are safe in this respect and my 20 char passwords will be automagically inserted regardless of autocomplete set.

[boonebgorges]

This is enough to convince me. Thanks, netweb and hnla.

[boonebgorges]

In 9607:

Add 'autocomplete="off"' to password field on login widget.

Props Prometheus Fire, netweb, hnla.
Fixes #6269.

[boonebgorges]

If there are further issues with this implementation - like maybe browser-specific problems that require setting properties on the whole form (​https://core.trac.wordpress.org//intertrac/ticket%3A24364) - let's handle it in a separate ticket.

[r-a-y]

Might we want to use the bp_form_field_attributes( 'password' ) instead?

[boonebgorges]

Oh right :)

[boonebgorges]

In 9612:

Use bp_form_field_attributes( 'password' ) to print autocomplete=off attribute in login widget.

This is better than hardcoding. See #5914.

Props r-a-y.
Fixes #6269.