WordPress.org

Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 9 months ago

#8938 closed defect (bug) (fixed)

Use new filter for extending WordPress authentication

Reported by: wnorris Owned by: ryan
Milestone: 2.8 Priority: normal
Severity: normal Version:
Component: Security Keywords: has-patch dev-reviewed
Focuses: Cc:
PR Number:

Description

One of the key problems with WordPress authentication as it exists today is that it is very username/password centric. The wp_authenticate() function immediately fails if either of those two things are missing. This becomes particularly problematic when using non-traditional authentication methods like OpenID and OAuth. While wp_authenticate() can be replaced by a plugin, this causes other problems described in ticket #8833.

The best compromise (proposed by Peter Westwood) is to use a new filter for authentication. wp_authenticate() would be responsible for basic sanitizing of username and password, and then call the filter. Two implementations of this filter would be added in core:

  • wp_authenticate_username_password - does normal username/password authentication against the WP user table
  • wp_authenticate_cookie - attempts authentication using the auth cookie. By default, this would only be used when doing traditional login (via wp-login.php).

Plugins can simply add a new function which uses this filter and authenticate the user by whatever means they choose. Both of the above functions will immediately return if they see that the user has already been authenticated.

A few notes on backward compatibility... I propose leaving wp_authenticate in pluggable.php for the time being so plugins that replace it don't break. I would however like to deprecate that in favor of the new authenticate filter, and eventually move wp_authenticate to user.php. I've left the wp_authenticate action in wp_signon() for the time being, but I think it can also be deprecated. I'm currently in the process of doing a very thorough analysis of all plugins in the WP.org directory to see which ones are even touching the authentication code. I've still got a bit more to do, but I think we're only talking about 20-30 plugins max.

There are a couple of small hacks in the included patch:

  • one small one that I'm not too worried about that prevents the "empty username" error from displaying when you first load wp-login.php
  • a bit uglier one used to pass the $secure_cookie boolean from wp_signon to wp_authenticate_cookie. Ugly just in the fact that it throws it into a global. Since $secure_cookie is actually set in wp-login.php in global scope, I think we might just be able to use that, and not worry about passing it in to wp_signon as a parameter.

I'm very open to changes to what I've got... I'm less concerned with how we actually achieve this, so long as we get the desired result.

Attachments (2)

wp-auth.diff (5.8 KB) - added by wnorris 11 years ago.
wp-auth-2.diff (1.8 KB) - added by wnorris 11 years ago.
fix error displayed on initial wp-login.php load

Download all attachments as: .zip

Change History (12)

@wnorris
11 years ago

#1 @ryan
11 years ago

Looks good to me.

#2 @westi
11 years ago

  • Keywords dev-reviewed added

+1

Time to stew in trunk me thinks

#3 @josephscott
11 years ago

I'm trying this in a test install of -trunk and seems to be working fine.

#4 @westi
11 years ago

This is working fine for me too.

Time to commit and get some more users.

#5 @westi
11 years ago

(In [10437]) Make authentication more pluggable than ever before. See #8938 props wnorris.

#6 @westi
11 years ago

(In [10455]) Don't show the error on normal login page loads. See #8938.

@wnorris
11 years ago

fix error displayed on initial wp-login.php load

#7 @wnorris
11 years ago

new patch to handle error displayed on initial wp-login.php load. We let wp_authenticate_username_password do it's normal thing, returning an error if username and password are empty. Then in wp_signon (which is only ever called fro wp-login.php), we check for those errors, and wipe them out if present. Solves the same problem, but puts the logic where it really should be... in wp_signon().

#8 @westi
11 years ago

(In [10463]) Much better handling of wp-login.php page load. See #8938 props wnorris.

#9 @ryan
11 years ago

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

#10 @afercia
9 months ago

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.