Make WordPress Core

Opened 6 years ago

Closed 20 months ago

Last modified 20 months ago

#41136 closed defect (bug) (fixed)

Login forms lacking autocomplete attributes

Reported by: johnjamesjacoby's profile johnjamesjacoby Owned by: joedolson's profile joedolson
Milestone: 6.0 Priority: normal
Severity: normal Version:
Component: Login and Registration Keywords: has-patch commit
Focuses: accessibility, administration Cc:

Description (last modified by ocean90)

The security team received a report via HackerOne related to autocomplete attributes being omitted from various form fields in wp-login.php. Since there is no direct security issue (and we've handled this type of improvement publicly previously) I'm creating a new ticket here to continue that.

In my research, form fields in wp_login_form(), show_user_form(), and show_blog_form() need similar scrutiny and improvements.

Related: #24364

Attachments (2)

41136.patch (2.4 KB) - added by dhanendran 6 years ago.
autocomplete attribute added
41136.2.diff (6.1 KB) - added by joedolson 20 months ago.
Sets autocomplete values for login/register

Download all attachments as: .zip

Change History (26)

#1 @netweb
6 years ago

Related: #buddypress6269 "Add autocomplete="off" to bp-login widget password field"

There's some useful prior research links in the above ticket

#2 @ocean90
6 years ago

  • Description modified (diff)

@dhanendran
6 years ago

autocomplete attribute added

#3 @afercia
6 years ago

  • Component changed from Users to Login and Registration

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


5 years ago

#5 @melchoyce
5 years ago

What UX feedback is needed here?

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


5 years ago

#7 @melchoyce
5 years ago

  • Keywords ux-feedback removed

Removing ux-feedback for now. Feel free to re-add if the ticket picks back up.

#8 @rianrietveld
2 years ago

  • Focuses accessibility administration added

Hey all, I'd like to give this ticket some attention and priority.

WCAG on autocomplete
For WCAG 2.1 AA autocomplete values are required in login forms.
That's success criterion 1.3.5 Identify Input Purpose
https://www.w3.org/WAI/WCAG21/quickref/?showtechniques=131%2C412#identify-input-purpose.

In the new WCAG 2.2 A an additional success criterion wil be added to help users remember their login data, and using autocomplete is one of the techniques for that.
That's success criterion 3.3.7 Accessible authentication https://www.w3.org/WAI/standards-guidelines/wcag/new-in-22/#337-accessible-authentication-a

The release of 2.2 is planned for June 2022. Both success criteria are added to help people with a cognitive disability.

So instead of adding autocomplete="off", as the patch suggests the values should be:

<input type="text" name="log" id="user_login" autocomplete="username" class="input" value="" size="20" autocapitalize="off" >

and

<input type="password" name="pwd" id="user_pass" autocomplete="current-password" class="input password-input" value="" size="20" >

All autocomplete values are listed on MDN: [The HTML autocomplete attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/autocomplete)

Discussion:
As the autocomplete values are stored in the browser, this may result in security issues when users share their computers or use a computer in a public place like a library.

One way or another, we have to make a decision about adding autocomplete values. This ticket could serve as reference and documentation about what we decide.

Last edited 2 years ago by rianrietveld (previous) (diff)

This ticket was mentioned in Slack in #accessibility by rianrietveld. View the logs.


2 years ago

#10 @sabernhardt
2 years ago

  • Milestone changed from Awaiting Review to 6.0

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


21 months ago

#12 follow-up: @joedolson
21 months ago

  • Owner set to joedolson
  • Status changed from new to accepted

From a technical standpoint, using any autocomplete on any field anywhere introduces the same security issue, so I'm not sure to what degree we should take responsibility for that. Any software can be used in both a private and a shared environment.

There's a relevant conversation here: https://github.com/w3c/wcag/issues/2110

One thing I think this will need is to provide a filter so that autocomplete can be disabled; sites with higher security needs might need an easy way to disable this feature.

Overall, however, I think this is more of a benefit than a potential for harm. Would benefit from some additional voices on the security side, however.

#13 in reply to: ↑ 12 @peterwilsoncc
20 months ago

Replying to joedolson:

One thing I think this will need is to provide a filter so that autocomplete can be disabled; sites with higher security needs might need an easy way to disable this feature.

According to caniuse, browsers ignore off for the following fields

  • Chrome, Edge: intentionally ignores autocomplete="off" when the user uses the browser's autofill functionality.
  • Firefox: ignores autocomplete="off" for login forms
  • Safari: ignores the off value for username, email and password fields

It's also worth noting that since comment#3 several years ago, support for autocomplete has become universal with the exceptions noted above.

Overall, however, I think this is more of a benefit than a potential for harm. Would benefit from some additional voices on the security side, however.

From a security perspective, I think it's fine to include the autocomplete values. I don't think adding filters is necessary given the browser manufacturers' decisions documented above.

#14 @joedolson
20 months ago

Thanks for the second opinion, @peterwilsoncc! That matches what I was thinking, so sounds great.

@joedolson
20 months ago

Sets autocomplete values for login/register

#15 @joedolson
20 months ago

  • Keywords needs-testing added

New patch sets autocomplete values on:

  • on login username
  • on login password
  • on register username
  • on register email
  • added autocomplete=off to show_blog_form() blog title and blog name fields.

In the login username field in wp-login.php, I used autocomplete='username', though the form accepts either username or email. Perhaps this should be filterable? To the best of my knowledge, autocomplete="username" is the appropriate value for a username field that accepts an email address.

I also omitted autocomplete on the 'lost password' form, which also accepts either username or email. For the lost password, I'm less certain what would be ideal.

Testing:

  • verify that autocomplete values are present and match data type in any log-in or register form.
  • verify whether using autocomplete="username" will produce an email address if the user has used their email address as a log-in value.

#16 @peterwilsoncc
20 months ago

I also omitted autocomplete on the 'lost password' form, which also accepts either username or email. For the lost password, I'm less certain what would be ideal.

My instinct is username but if you could ask for advice is accessibility circles that would be lovely.

I did some quick testing and it seems to be working as expected.

#43886 is related but for fields that need the new-password autocomplete value.

#17 @rianrietveld
20 months ago

Thanks so much for going ahead with this.

Especially because the rules in the new WCAG 2.2, that is still in draft, will probably be even be more stricter [3.3.6. Accessible authentication](https://www.w3.org/WAI/standards-guidelines/wcag/new-in-22/#337-accessible-authentication-a]. (Don't shoot the messenger)

The values for autocomplete can be found on
[MDN web docs The HTML autocomplete attribute]
(https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/autocomplete)

To comply to WCAG 2.1 AA for the login form we need:
username on "Username or Email Address"
current-password on "Password"

And possibly if we want to take this further on more login/create account forms:

For retrieving a forgotten password:
username on "Username or Email Address"

For creating a new password (so the browser doen't fill out the existing password)
new-password

This ticket was mentioned in PR #2480 on WordPress/wordpress-develop by peterwilsoncc.


20 months ago
#18

  • Keywords has-patch added; needs-patch removed

#19 @peterwilsoncc
20 months ago

Thanks for following up @rianrietveld.

In the linked pull request, I've added the autocomplete="username" attribute to the forgotten password page.

To keep this ticket focused on current usernames and passwords, let's focus on new-password in #43886. It's also on the 6.0 milestone but is slightly more nuanced.

@joedolson If you're happy with my addition on the PR, feel free to commit at your convienience. Your changes tested well during manual testing.

#20 @joedolson
20 months ago

  • Keywords commit added

That works for me.

#21 @joedolson
20 months ago

  • Keywords needs-testing removed

#22 @joedolson
20 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 53041:

Login and Registration: Add autocomplete attributes.

Set valid autocomplete attributes for registration and login fields for username, email, and current password. Autocomplete values are required in WCAG 2.1 at level AA for login forms.

Props johnjamesjacoby, dhanendran, rianrietveld, joedolson, peterwilsoncc.
Fixes #41136.

#23 @rianrietveld
20 months ago

Thank you!

Note: See TracTickets for help on using tickets.