roots / wp-password-bcrypt Goto Github PK

• I've read the guidelines for Contributing to Roots Projects
• This request isn't a duplicate of an existing issue
• I've read the docs and followed them (if applicable)
• This is not a personal support request that should be posted on the Roots Discourse community

Description

We're seeing timeouts related to wp_check_password().

Since we're running on Heroku, we can't increase the timeout of 30s.

We're also not seeing a stacktrace, just this:

Sentry\Exception\FatalErrorException: Error: Maximum execution time of 30 seconds exceeded
#0 /vendor/roots/wp-password-bcrypt/wp-password-bcrypt.php(42): null

Login and everything else works fine. We don't know what's actually failing.

What if I remove the plugin later? The passwords would not convert into md5, so users wouldn't be able to log in, right?!

Fatal error: Cannot redeclare wp_check_password() (previously declared in C:\Users\johh-smith\Documents\Websites\test.dev\wp-includes\pluggable.php:2087) in C:\Users\john-smith\Documents\Websites\test.dev\wp-content\plugins\wp-password-bcrypt\wp-password-bcrypt.php on line 44

Just tried activating it on a local dev site using Desktop Server.

Terms

• I have read the guidelines for Contributing to Roots Projects
• This request is not a duplicate of an existing issue
• This is not a personal support request that should be posted on the Roots Discourse community

Summary

It is currently not possible to use phpstan with phpstan-wordpress and the wordpress-stubs packages when installing roots/wp-password-bcrypt via composer.json without resorting to some very hacky (and unreliable/repetitive/frustrating) workarounds.

The full issue is described in szepeviktor/phpstan-wordpress#41.

Motivation

Why are we doing this?

To reduce the frustration of working with WordPress, to improve compatibility, and to facilitate reproducible development tasks in local and CI workflows.

Please read:

• szepeviktor/phpstan-wordpress#41
• php-stubs/wordpress-stubs#70

I am aware this has been mentioned indirectly in the past, but it appears that there has been little consideration given to making a change in this plugin despite the numerous accounts over the years. I am referring specifically to #11, but there are other examples.

It would be great if roots/wp-password-bcrypt could find a way to improve compatibility with these other libraries, because both this plugin and those libraries have been positive improvements to the WordPress ecosystem that unfortunately do not mix well.

What use cases does it support?

• Using the widely-used phpstan static analysis tooling with WordPress integration via https://github.com/szepeviktor/phpstan-wordpress
• Using this plugin via Composer installation

What is the expected outcome?

Developers and CI pipelines should be able to run phpstan analyse when this plugin is installed via composer.json and the WordPress stubs are loaded without any conflict.

I encourage the authors of this plugin to listen to the feedback from other developers who have encountered such conflicts and have given specific suggestions as to a fix.

For example:

Check if function exists in wordpress-stubs.php · Issue #70 · php-stubs/wordpress-stubs

I've given a couple of workshops on WordPress+PHPStan and both times this exact problem was the first to arise. We determined that the most correct solution is for the Bedrock bcrypt library to wrap its functions in function_exists() checks because it has no way to ensure that another definition of one of those functions isn't loaded by another plugin. I'm not sure why that is not the case.

Potential conflicts / foreseeable issues

Unknown. I am only a user of both tools reporting my experience and, more importantly, linking the two GitHub issues together because it looks like there's only been isolated conversations in each repo about this issue.

Additional Context

No response

Terms

• I have read the guidelines for Contributing to Roots Projects
• This request is not a duplicate of an existing issue
• This is not a personal support request that should be posted on the Roots Discourse community

Summary

Instead of only being able to install the plugin as a MU Plugin manually it would be nice to be able to install it that was via Composer since the WordPress Composer installers allow for this. I have my own working example of doing this here: https://github.com/ndigitals/wp-local-media-proxy/blob/main/composer.json

Specifically adding

"type": "wordpress-muplugin",

and leveraging the boxuk/wp-muplugin-loader Composer package works. Though I believe you may have your own MU Plugin loader.

Motivation

Why are we doing this?

Allow for installing as a MU Plugin but also keep it managed as a package dependence so that Composer sites can still update the package.

What use cases does it support?

There is an aspect where this package breaks PHPStan checks when used along with WordPress stubs. This is due to this package conflicting with Core functions and autoloading. However, the additional use case is to also be able to clearly see when review the Site Health tools in the Dashboard that this MU Plugin is actually running on the site. With the current Composer installation it's hidden and not obvious to general site support.

What is the expected outcome?

This can be installed as a MU Plugin and loaded, doesn't conflict with PHPStan execution with WordPress Stubs, is accurately reflected in the WordPress Dashboard Site Health.

Potential conflicts / foreseeable issues

Which MU plugin autoloader to use it a question. There is a generally available Composer package that can be used but there also seems to be a non-Composer based one that Bedroock/Roots uses for it's own stuff. This doesn't work well for non-Bedrock sites.

Additional Context

No response

It may be a good practice to implement password_needs_rehash by default.

Since we're using PASSWORD_DEFAULT and relying on the default cost option, PHP can update these in the future. If that happens and the plugin doesn't change, hashing would be broken. This is a very long term potential problem, but it could happen.

If we always check for password_needs_rehash then we can future proof against this.

Other options:

• Hardcode PASSWORD_BCRYPT to avoid this
• Hardcode the default cost option?
• Leave this up to the users and document how they could implement password_needs_rehash

I saw that #7 has already come and gone but I have just experienced this in a different situation.

I have a dev box running PHP Version 5.5.9-1ubuntu4.11 according to phpinfo().

I installed the plugin by downloading the master.zip from github and extracting to the correct plugins folder.

When I activated it I saw this error:

Fatal error: Cannot redeclare wp_check_password() (previously declared in /srv/www/basetemplate/htdocs/wp-includes/pluggable.php:2087) in /srv/www/basetemplate/htdocs/wp-content/plugins/wp-password-bcrypt/wp-password-bcrypt.php on line 24

I'm not using the mu stuff, I didn't use composer, I did renamed the folder to remove the -master, I am using the latest Sage as the theme.

In order to use this plugin with composer, the type needs to be added to the composer.json file:

{
    "type": "wordpress-plugin"
}

I have a local installation of WP with Bedrock and I can't reset my password.

1. My local server can't send emails (so I can't reset my password in a normal way);
2. I've tried modifying my password from PhpMyAdmin using MD5 checksum, but that didn't work (I couldn't log in with the new password);
3. I've tried modifying my password from PhpMyAdmin by entering already encrypted password using this tool: https://passwordhashing.com/BCrypt?plainText=123456 and that didn't work either.

I am basically locked out of my website. What can I do to reset my password?

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?

Fatal error: Cannot redeclare wp_check_password() (../wp-password-bcrypt/wp-password-bcrypt.php on line 29

Cant get it to work, I have had no problems either adding it in Code Snippets, or as a regular plugin.

I have built myself a solution to prevent deleting, and deactivating selected plugins so I dont need to have this as a mu-plugin. This let me update the plugin at scale through wp managing solution.

Still doesnt work though

I took the wp-password-bcrypt.php file and pack it down to a zip file and uploaded as a plugin, and it wont activate due to the error above.

It is simply not fun to update client site one by one in case it updates in the future. Uploading a plugin for bulk update across all sites is such a time saver.

And yes, I've seen that the problem have occured before in this Github rep and in the forum, but isn't there a solution available?

Thank you for reading!

Steps To Reproduce

See earlier field.

Expected Behavior

See earlier field.

Actual Behavior

See earlier field.

Relevant Log Output

No response

Versions

Latest on everything (PHP, bcrypt and wp)

Bug report

Please provide steps to reproduce, including full log output:

Occurs each time I attempt to go to my plugins page. I am unable to access my plugins page. Each time I do I receive this in my logs:

PHP Fatal error: Cannot redeclare wp_check_password() (previously declared in /xxxxx/xxxxx/xxxxx/wp-includes/pluggable.php:2126) in /xxxxx/xxxxx/xxxxx/wp-content/mu-plugins/wp-password-bcrypt.php on line 24

Please describe your local environment:

wp-password-bcrypt version: 1.0

WordPress version: 4.8.2

Where did the bug happen? Development or remote servers?

Remote servers

This is a fantastic plugin but I worry that it will be criminally under used because it's not in the main WordPress plugins directory:

As far as I can tell this is because in the roots ecosystem you consider doing everything via Composer as the normal / best way to do it, plus your concerns about someone disabling the plugin:

As explained above, you don't want to disable this plugin once you've enabled it. Installing this in plugins (as a "normal" plugin) instead of in mu-plugins (as a must-use plugin) makes it possible for an admin user to accidentally disable it.

If you add it to the WordPress plugins directory then:

1. It makes it much easier for people to install - the vast majority of WordPress users don't use Composer
2. Any security updates you make for this will be automatically highlighted to users

Surely the point of this plugin is to help all WordPress users improve their security? The risk that some admins will disable the plugin (which would be odd when it's likely that they would have been the ones to install it), seems relatively smaller than the benefits.

Perhaps there's some other restrictions on the WordPress plugins directory that I don't know about.

You mention other plugin in the FAQ. It would be nice to know if the wp-bcrypt plugin can be swapped with this one without affecting current user logins. I've been running the wp-bcrypt plugin plugin for at least a few years now and have thousands of users on my site so It would be great to switch to a plugin that is supported.

Installing this code give an error when installing on WP 4.9.6

Fatal error: Cannot redeclare wp_check_password() (previously declared in .../public_html/wp-includes/pluggable.php:2230) in .../public_html/wp-content/plugins/8020plugins/wp-password-bcrypt..php on line 44

The code on the WP directory works just fine - pls update github (or else delete it).
https://wordpress.org/plugins/password-bcrypt/

• [x ] I've read the guidelines for Contributing to Roots Projects
• [x ] This request isn't a duplicate of an existing request
• [x ] This is not a personal support request that should be posted on the Roots Discourse community

Summary

You've removed the dependency already in composer.json. If you make a new release then a person could use composer 2.0+ to install it without adopting dev-master as their install target.

When installing this plugin the passwords of existing users are still stored as an MD5 hash. When the user logs in, there is a check to see if they have a legacy MD5 hash, and if so (upon successful authentication) the provided value will be used to 'upgrade' it to a bcrypt hash.

This on it's own doesn't do much to increase security, as until the user logs in their password will still be stored as an MD5 hash. Depending on how often users log in, it may be a very long time (think membership / community sites) until a secure bcrypt hash is stored for every user. If the database is compromised, you will still be leaking MD5 hashes which for simple passwords are trivial to reverse.

A more secure option would be to run all the existing MD5 hashes through password_hash when the plugin is installed, so in affect you are storing bcrypt(md5(password)) - no more MD5. A legacy flag can be added as a prefix in the hash field, and then when authenticating you can check that, to see if it's a legacy password that the should be MD5'ed before being passed to password_verify. At that point, once you have authenticated the user, you can generate a plain bcrypt hash so just bcrypt(password) is stored.

This is explained further, with example PHP code, in this Reddit post (not mine):

https://www.reddit.com/r/PHP/comments/3lwxlw/hash_and_verify_passwords_in_php_the_right_way/cva6y6p

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

1. 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 );
		},
	]);
});

3. Call aforementioned REST API route
4. 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

Bug Report

• I've read the guidelines for Contributing to Roots Projects
• This is a feature request
• This is a bug report
• This request isn't a duplicate of an existing issue
• I've read the docs and followed them (if applicable)

Replace any X with your information.

What is the current behavior?

X When using this package paired with https://github.com/georgestephanis/application-passwords a users password (hash) is reset after every request using the app password.

What is the expected or desired behavior?

X A users password is not reset when making a basic auth request w/ the app password.

Bug report

(delete this section if not applicable)

Please provide steps to reproduce, including full log output:

• Install the Application Password plugin.
• Add an app password to your user (via your profile).
• Make an API request with no-auth and notice no change.
• Make an API request with your username and app password to any endpoint, and notice you're logged out of the WP admin, and your password hash has updated.

Please describe your local environment:

wp-password-bcrypt version: 1.0.0

WordPress version: 4.9.8

Active theme: twentyseventeen

Active plugins: application password

Where did the bug happen? Development or remote servers?

local & remote.

Please provide a repository or your wordpress_sites config (if possible):

N/A

Is there a related Discourse thread or were any utilized (please link them)?

No