XMLRPC Authentication

[PhoenixRises]

Currently, XMLRPC uses the functions login_pass_ok and user_pass_ok in ~/xmlrpc.php and ~/wp-includes/user.php respectively.

Using this method causes XML-RPC to use the WordPress default login method but does not take into account any other method that may be used.

This can be modified by removing the user_pass_ok function from user.php entirely and replacing the call in login_pass_ok with the pluggable wp_login (see diff file).

I have classed this as an optimzation as it does not in any way affect the functionality of XML-RPC.

Thanks.

[brianlayman]

That's funny... It's my day off so I was actually looking at this too. I have a slightly different take on the issue.

In my mind, the bug is that this routine WAS actually moved to XMLRPC as intended, however it was never actually removed from USER when this occured. And because of the include structure (user.php is required into settings.php before pluggable.php is and settings.php is included into xmlrpc.php before user_pass_ok is defined), that function cannot be overridden.

I think the correct fix is simply to delete the extra copy of user_pass_ok from user.php. The function's code in xmlrpc.php is identical to what is in user.php. The only change the userbase would see is that those of us who plugged that function would suddenly have their new routines called.

[brianlayman]

BTW commenting the code out in user.php DOES indeed allow the plugged version to be used. This change is required to allow third party tools like LiveWriter and BlogDesk to be used on sites which have custom login systems.

[josephscott]

The problem with removing user_pass_ok() is that plugins make use of it. Changing it to use wp_login() would save plugin authors some grief.

[brianlayman]

Yeah, I know the stuff at b5media uses it and the stuff I wrote for blogs.borland.com uses it too IIRC. So I think we should just remove the extra copy that isn't wrapped with an "if ( !function_exists('user_pass_ok') )"

[josephscott]

brianlayman - can you put together a new diff and lets see how it goes from there.

[brianlayman]

Easy enough to do... btw YOU may call me Brian....

[josephscott]

Ok, I must be missing something, I was thinking of making it look like:

function user_pass_ok($user_login,$user_pass) {
    return wp_login( $user_login, $user_pass );
}

[brianlayman]

See this patch fixes actually allows you to do that. It keeps XMLRPC working the way it does now and provides full backwards compatiblity with the current WordPress but it also allows you to put user_pass_ok into the my-hacks.php or whereever you keep your customizations.

If you want to put a routine in that calls wp_login that's fine but there's a lot of stuff in wp_login that may not be appropriate to an xml connection. Is the "password field is empty" error appropriate? How much cookie stuff is dragged into the mix that isn't needed? I suspect wp_login is way heavier than is actually needed by a blogging too and that that's why it wasn't originally used.

But as I said, with this patch, you can now write code that allows you to make that call... if you want to.

[brianlayman]

Oh I see now that the original patch DID include that removal, it just also added the functionality you were suggesting. However, as I implied, I think keeping the xmlrpc interface thin should remain part of the goal and allowing the developer to include more if and only if they have the need should be their option.

[josephscott]

Ok, let me take a step back here and look at the idea behind this ticket.

The point seems to be that the auth mechanism in xmlrpc isn't pluggable. But that doesn't seem to be the case, since it all funnels down to get_userdatabylogin() and wp_check_password(), both of which are pluggable.

Which leads me to believe that this ticket is a non-issue.

[brianlayman]

Look at it this way: user_pass_ok is supposed to be pluggable. It is written that way in xmlrpc.php, but there's a copy of the legacy function abandoned in user.php. It blocks the pluggablity.

We shouldn't have two copies of the same code and functions that are pluggable should function that way. So this needs to be resolved.

Right now the code in xmlrpc.php can never execute but it is what is meant to be the active pluggable function.

[josephscott]

I'm a bit lost here, there is no user_pass_ok() function defined in xmlrpc.php. There is a login_pass_ok() function, that makes use of user_pass_ok(). The only place that user_pass_ok() is defined in WP is in wp-includes/user.php.

To make user_pass_ok() pluggable why don't we move it to pluggable.php and wrap it with a function_exists check?

[brianlayman]

Move user_pass_ok from user.php to xmlrpc.php

[brianlayman]

I see what you mean. The xmlrpc I was working with already had the user_pass_ok added to it by Mark J. I hadn't realized it was edited. I've modified my patch so that you will see exactly what I am seeing.

I don't have a problem with it being in pluggable.php. The include structure should allow it, and I don't think it is used anywhere other than xmlrpc.php, but I haven't searched for it this morning to check...

[ryan]

I was thinking of adding wp_authenticate() to pluggable functions. This would be a cleaned up version of wp_login() that returned WP_Error for error conditions. login_pass_ok() could use this and pass any WP_Error message back in its IXR_Error.

[ryan]

user_pass_ok() now calls the pluggable wp_authenticate function. Should be good enough.