dominic-ks / bdvs-password-reset

WordPress - Allow users to reset their password using a random code via the REST API
GNU General Public License v3.0
9 stars 3 forks source link

Roots/Bedrock compatibility #22

Open quentint opened 5 months ago

quentint commented 5 months ago

Hi, I think I discovered an incompatibility with this nice plugin and the Roots/Bedrock ecosystem.

They override the wp_set_password function but handle the REST and non-REST cases differently, which breaks bdvs-password-reset. See this implementation: https://github.com/roots/wp-password-bcrypt/blob/5b8542e71578f4540b8dd69c39ae9f156d29c200/wp-password-bcrypt.php#L86

I fixed/hacked this with these lines (because I couldn't find anything better for the moment):

<?php
// my-theme/functions.php

// Hack to fix "bdvs-password-reset" plugin on Roots/Bedrock
add_action('application_password_is_api_request', function(bool $isRest) {
    return false;
}, 10, 2);

I hope this helps and could be fixed 😉

dominic-ks commented 5 months ago

Hello @quentint, thanks very much for raising the issue. I've never actually used Bedrock before, nor tested this plugin with it. I'd need to get some time to set up a test site using it to explore this issue and test your proposed change to accommodate it. With limited knowledge on the setup, I wouldn't want to be too quick to make the change without understanding its implications.

What may be good in the meantime is that we could update the docs to show that your suggested code may be required to use the plugin in this instance. As a side note, seems like it would be better to add this in a local custom plugin rather than the theme's functions.php file.

quentint commented 5 months ago

With limited knowledge on the setup, I wouldn't want to be too quick to make the change without understanding its implications

Same thing for me, this is why I consider my add_action a hack. This is also why I'm afraid mentioning it in the docs might be a bad idea, for now. I'll try to find a better understanding of the issue and propose a better fix.

As a side note, seems like it would be better to add this in a local custom plugin rather than the theme's functions.php file.

Correct!