WordPress.org

Make WordPress Core

Opened 6 months ago

Last modified 3 months ago

#42985 new defect (bug)

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

Reported by: zergling81 Owned by:
Milestone: 5.0 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 (2)

empty_username_password_errors.JPG (23.1 KB) - added by birgire 6 months ago.
42985.diff (1.0 KB) - added by birgire 6 months ago.

Download all attachments as: .zip

Change History (14)

#1 @ajayghaghretiya1
6 months ago

  • Keywords needs-patch added

#2 @birgire
6 months ago

Within the wp_signon() function, 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 show 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

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

#3 @afercia
6 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
6 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 6 months ago by birgire (previous) (diff)

#5 @afercia
6 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
6 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
6 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
6 months ago

#8 @birgire
6 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.


5 months ago

#10 @afercia
5 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
5 months ago

  • Milestone changed from Awaiting Review to 5.0

#12 @afercia
3 months ago

Related: #43037.

Note: See TracTickets for help on using tickets.