Allow custom errors on login when using XML-RPC

[koke]

Currently, the code for login in XML-RPC does this:

$user = wp_authenticate($username, $password);

if (is_wp_error($user)) {
        $this->error = new IXR_Error(403, __('Bad login/pass combination.'));
        return false;
}

So any custom error returned by wp_authenticate is thrown away and replaced by a generic error

Our use case: we recently added Two Step authentication to WordPress.com. The XML-RPC api needs to use an application specific password, since there's no support for entering the auth code. When a user logs in with the right username/password we need to return a specific error message asking the user to login with an application specific password.

Other custom login systems may require specific error messages too

[westi]

These sounds like a great quick win for 3.5.

Would someone like to create a patch?

[JustinSainton]

I'm not really familiar with XML-RPC, but would love to learn more. If I understand the issue properly, perhaps this simple patch would suffice? Filter the user object to return a WP_Error object and then filter the error message. Let me know if this is even a proper direction.

[JustinSainton]

Whoops, intended to add the $user var to the message filter on the first pass, so you could actually access whatever was passed there.

[koke]

Actually, the only reason I find not to return the errors directly, is that some errors from wp_authenticate_username_password return html instead of text only. We could, replace invalud_username, invalid_password, and authentication_failed by the current generic error, and convert everything else to an IXR_Error

[JustinSainton]

Actually, unless I'm mis-reading something, all of the errors returned by wp_authenticate_username_password contain HTML (at least the <strong> ERROR: </strong>, if not also a link). All of them except for blog_suspended, at least.

Any thoughts on approach? Could preg_replace or strip_tags - but both would end up hacky. We could map the error codes to non-HTML versions. Or we could just keep it as-is.

FWIW, the reason I used the filter in my initial approach rather than Sergey's approach was because my understanding was that you needed to add a custom error message for an application key, rather than return the message directly from the WP_Error object.

[nacin]

This will probably need to be a filter on the error object. If we want to do more in the future, we can.

To be honest, I'm a little weary of returning more than just the generic error by default, lest XML-RPC becomes a quieter way to slam a site for identifying valid usernames.

[JustinSainton]

Maybe like 21907.1.diff sans the xmlrpc_userlogin_error filter?

[koke]

Replying to nacin:

To be honest, I'm a little weary of returning more than just the generic error by default, lest XML-RPC becomes a quieter way to slam a site for identifying valid usernames.

I'm totally ok with returning the same generic error, unless a plugin asks otherwise

[nacin]

In [21910]:

XML-RPC: Have the deprecated login_pass_ok() method wrap login(). Move it below login() so the proper method is found first. see #21907.

[nacin]

In [21911]:

Deprecate user_pass_ok() in favor of wp_authenticate(). see #21907.

[nacin]

In [21912]:

Introduce the xmlrpc_login_error filter, applied to the IXR_Error being returned by the server when login() fails. props JustinSainton, fixes #21907.

[westi]

Replying to nacin:

This will probably need to be a filter on the error object. If we want to do more in the future, we can.

To be honest, I'm a little weary of returning more than just the generic error by default, lest XML-RPC becomes a quieter way to slam a site for identifying valid usernames.

Why is this any easier/quieter than scripting the wp-login.php POST?