WordPress.org

Make WordPress Core

Opened 20 months ago

Closed 5 months ago

#42985 closed defect (bug) (fixed)

No error message returned by empty wp-login.php form

Reported by: zergling81 Owned by: audrasjb
Milestone: 5.2 Priority: normal
Severity: normal Version: 2.5
Component: Login and Registration Keywords: has-screenshots has-patch
Focuses: ui, accessibility Cc:

Description

If the wp-login.php login form is submitted empty, the page refreshes and no error message is displayed. Expected behavior would be a shake and an appropriate error message.

(This is also needed to satisfy WACG 3.3.1 if we care about that... https://www.w3.org/TR/UNDERSTANDING-WCAG20/minimize-error-identified.html)

Attachments (7)

empty_username_password_errors.JPG (23.1 KB) - added by birgire 20 months ago.
42985.diff (1.0 KB) - added by birgire 20 months ago.
42985.1.diff (516 bytes) - added by audrasjb 6 months ago.
Patch refresh: display errors in wp-login.php when email and/or password fields are empty.
error-both-fields.png (39.8 KB) - added by audrasjb 6 months ago.
Error both fields are empty
error-password-field.png (38.6 KB) - added by audrasjb 6 months ago.
Error password field
error-user-name-field.png (38.7 KB) - added by audrasjb 6 months ago.
Error username field empty
42985.2.diff (992 bytes) - added by audrasjb 5 months ago.
Re-add wp-login errors

Download all attachments as: .zip

Change History (29)

#1 @ajayghaghretiya1
20 months ago

  • Keywords needs-patch added

#2 @birgire
20 months ago

Within wp_signon() we have this part:

$user = wp_authenticate( $credentials['user_login'], $credentials['user_password'] );

if ( is_wp_error( $user ) ) {
    if ( $user->get_error_codes() == array( 'empty_username', 'empty_password' ) ) {
        $user = new WP_Error( '', '' );
    }

    return $user;
}

Source

If the username and password fields are both empty, then the corresponding error is overridden with $user = new WP_Error( '', '' );. That means no errors shows up for the login form.

If that would not be the case then we would get the login form shaking and the corresponding error messages. See empty_username_password_errors.JPG

Version 1, edited 20 months ago by birgire (previous) (next) (diff)

#3 @afercia
20 months ago

  • Keywords has-screenshots added
  • Version changed from 4.9.1 to 2.5

For some background, see a relevant ticket and related changes:
https://core.trac.wordpress.org/ticket/8938
https://core.trac.wordpress.org/ticket/8938#comment:6

https://core.trac.wordpress.org/changeset/10455
https://core.trac.wordpress.org/changeset/10463

Where the reasoning seems to be the will to avoid to show the error on normal login page loads. However, seems to me there should be one previous relevant change. I've noticed that in WordPress 2.3 (!) this scenario was covered and two error messages were displayed when both fields were empty:

https://cldup.com/YKNmGIGrds.png

Then, in WordPress 2.5 these 2 messages disappeared. Still have to find the relevant change.

#4 @birgire
20 months ago

I wonder if the wp_signon(), should be allowed to return the empty username and password error and handle it in the wp-login.php, for better flexibility.

I checked version 2.3 mentioned by @afercia and there it seems to be handled in wp-login.php with:

if ( $_POST && empty( $user_login ) )
    $errors['user_login'] = __('<strong>ERROR</strong>: The username field is empty.');        
if ( $_POST && empty( $user_pass ) )
    $errors['user_pass'] = __('<strong>ERROR</strong>: The password field is empty.');

before the login_header(__('Login'));

See source.

Last edited 20 months ago by birgire (previous) (diff)

#5 @afercia
20 months ago

Seems the original change was the login refactoring in [6643], see #5405. Before this change, the last switch case (login/default) displayed both errors and checked also for $_POST.

#6 @birgire
20 months ago

Currently there seems to be no unit tests for e.g. wp_signon().

There's already a ticket #36476 - The login form is not covered by a test

#7 @danieltj
20 months ago

Checking that both fields are empty and the request is a POST request would handle this nicely, as @afercia mentioned above in WordPress 2.3 doing it this way.

@birgire
20 months ago

#8 @birgire
20 months ago

  • Keywords has-patch added; needs-patch removed

If wp_signon() is allowed to return the empty username and password error, then it could be handled instead in wp-login.php, to avoid adding extra $_POST check into the wp_signon() function.

This is the approach in 42985.diff.

Another way would be to only modify wp-login.php and compensate for the removal of the empty_username && empty_password errors within wp_signon(). But then in that case, we can ask what would be the purpose of the current empty_username && empty_password errors removal within wp_signon() ;-)

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


20 months ago

#10 @afercia
20 months ago

To clarify (before it gets asked) why WordPress doesn't hide error messages related to the username and that's not considered "information disclosure", see https://core.trac.wordpress.org/ticket/3708#comment:3 and all the following tickets closed as wontfix, for example #4290.

#11 @afercia
20 months ago

  • Milestone changed from Awaiting Review to 5.0

#12 @afercia
17 months ago

Related: #43037.

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


11 months ago

#14 @pento
11 months ago

  • Milestone changed from 5.0 to 5.1

#15 @afercia
8 months ago

  • Keywords needs-refresh added
  • Milestone changed from 5.1 to 5.2

Unfortunately we're running out of time for 5.1. This ticket does have a patch (probably needs a refresh). Needs to be tested in depth and reviewed. Punting to 5.2.

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


7 months ago

#17 @audrasjb
7 months ago

  • Owner set to audrasjb
  • Status changed from new to assigned

@audrasjb
6 months ago

Patch refresh: display errors in wp-login.php when email and/or password fields are empty.

@audrasjb
6 months ago

Error both fields are empty

@audrasjb
6 months ago

Error password field

@audrasjb
6 months ago

Error username field empty

#18 @audrasjb
6 months ago

  • Keywords needs-refresh removed

Hi,
I refreshed the patch in 42985.1.diff.
Tested on my side, works fine.
We don't need the wp-login.php change anymore to make it working.

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


5 months ago

#20 @afercia
5 months ago

  • Keywords needs-refresh added

I think the change in wp-login.php is necessary to make the "interim login" work correctly. Otherwise, users will see the error messages instead of the message "Your session has expired...". @birgire, correct?

http://cldup.com/4U1plPIHsu.png

@audrasjb
5 months ago

Re-add wp-login errors

#21 @audrasjb
5 months ago

  • Keywords needs-refresh removed

Thanks for the heads up @afercia
Patch refreshed in 42985.2.diff

#22 @afercia
5 months ago

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

In 44918:

Accessibility: Login: Display error messages when both the username and password fields are empty.

For accessibility and usability, if an input error is detected, the item that is in error needs to be identified and the error needs to be described to the user in text (WCAG Success Criterion 3.3.1). The login form displays an error when the username field is empty or when the password field is empty. It omits to do so when both fields are empty.

This change restores the login form behavior to the one that used to work in WordPress 2.3 (!) and displays the related error messages also when both fields are empty.

Props birgire, audrasjb.
See #8938, #5405, #3708.
Fixes #42985.

Note: See TracTickets for help on using tickets.