Bug: unexpected behavior when calling wp_set_password() via REST API
teppokoivula opened this issue · 7 comments
Terms
- I have read the guidelines for Contributing to Roots Projects
- This request is not a duplicate of an existing issue
- I have read the docs and followed them (if applicable)
- I have seached the Roots Discourse for answers and followed them (if applicable)
- This is not a personal support request that should be posted on the Roots Discourse community
Description
What's wrong?
Trying to change user password within a custom REST API route by calling wp_set_password()
doesn't seem to do anything at the moment.
What have you tried?
Among other things tried disabling wp-password-bcrypt, after which things started working again.
What insights have you gained?
If I disable "application_password_is_api_request" check by filter (add_filter('application_password_is_api_request', '__return_false')
) before calling wp_set_password()
in my REST route, changing password seems to work as expected.
Possible solutions
Not familiar enough with the implementation to make any real suggestions, except that if this can't be reliably fixed, then perhaps it should be documented?
Temporary workarounds
Calling add_filter('application_password_is_api_request', '__return_false')
before wp_set_password()
in REST route.
Steps To Reproduce
- Create a REST API route and add call to
wp_set_password('some-password-string', 1)
:
add_action( 'rest_api_init', function () {
register_rest_route( 'my-namespace', '/pass-test', [
'methods' => 'GET',
'permission_callback' => '__return_true',
'callback' => function( \WP_REST_Request $request ) {
// add_filter( 'application_password_is_api_request', '__return_false' );
wp_set_password( 'new-pass-here', 1 );
},
]);
});
- Call aforementioned REST API route
- Try to log in with new password
Expected Behavior
New password should work.
Actual Behavior
New password doesn't work, but old password still works. If I uncomment the filter in the action hook above, things now work as expected.
Relevant Log Output
No response
Versions
1.1.0
I haven't dug into this yet, but I just wanted to say thanks for providing an informative bug report with along with workarounds that you've tried, research that you've done into this, and steps to reproduce. I appreciate that. 🙏
Looking at that function, there's an early return for API requests that would prevent setting the password:
wp-password-bcrypt/wp-password-bcrypt.php
Lines 108 to 113 in 9ad6c27
I'm assuming WP_Application_Passwords
exists (but you could verify that), which leaves the other condition as the likely culprit.
empty($passwords = WP_Application_Passwords::get_user_application_passwords($user_id))
So if there are no existing application passwords for the user, that function will just return without doing anything 🤔
Would you be able to debug and see which condition is causing the short-circuit? You could copy the plugin source and modify it.
Just had a chance to dig in. First of all, the reason this method currently does nothing is indeed latter rule; there are no application passwords defined, so WP_Application_Passwords::get_user_application_passwords($user_id)
returns an empty array.
Just to be extra clear: I'm not trying to set an application password, but rather change the user's "normal" password via REST API. (I hope that was obvious already, but one can never be too sure.)
Looking at #31 I have a rough idea why the method works the way it does, but it does also seem to prevent what I'm trying to do here, and differs in this regard from how the core version works. Just wondering if hooking into wp_update_application_password and wp_create_application_password could be an alternative approach with fewer side effects?
Edit: Strike that, that wouldn't help with the problem mentioned in issue #22. But it doesn't seem quite right to assume that API requests are always about applications passwords either :)
wp_set_password should never update application passwords. That's a straight violation of the implicit API of that function which leads to this bug.
Regarding application passwords in wp_check_password, you have to decide what to update based on what type of hash is checked.
If the stored hash (second Argument) matches the user's password hash (needs to be fetched using passed ID) the password is updated.
If any of the application passwords (need to be fetched) match the passed hash the application passwords are updated.
If none of the above is true nothing is updated.
The are several plugins that call wp_check_password on strings that are in fact not the user's Login password.
Those will all reset the real password.
To add to my comments,
I don't think bcrypt should be used for application passwords at all.
Application passwords are randomly generated and have roughly 140 bits of entropy. A "simple" hash function is more than enough.
I have the same issue!
Trying to reset the users password via rest api over AJAX, exits without any error because empty($passwords = WP_Application_Passwords::get_user_application_passwords($user_id))
is true.
At least an entry should be added to log in such case