Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check haveibeenpwned API during password reset and account creation #170

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

jamesmorrison
Copy link
Member

Description of the Change

Verify new password against the Have I Been Pwned API during password change (on profile or via reset process)

Closes #77

How to test the Change

Changing password through profile

  1. Login to WP and access you profile
  2. Scroll down to "Set New Password" and enter something in a breach (e.g. london123)
  3. Scroll down to the "Update Profile" button
  4. If need be, remove the disabled attribute on the Update Profile button - this is added through JS when a "weak" password is identified
  5. Save and confirm you see the error: "The password entered may have been included in a data breach and is not considered safe to use. Please choose another." - note you may also see another error stating "Password must be medium strength or greater." (ignore this, but be aware of correlation between weak passwords and data breaches, in many cases both errors will show

Reset password process

  1. Navigate to WP login and run through the password reset process
  2. Open the link in the email
  3. In the new password field enter something in a breach (e.g. london123)
  4. If need be, remove the disabled attribute on the Save Password button - this is added through JS when a "weak" password is identified
  5. Save and confirm you see the error: "The password entered may have been included in a data breach and is not considered safe to use. Please choose another." - note you may also see another error stating "Password must be medium strength or greater." (ignore this, but be aware of correlation between weak passwords and data breaches, in many cases both errors will show

Changelog Entry

Added - Verify new password against the Have I Been Pwned API during password change (on profile or via reset process)

Credits

Props @jamesmorrison

Checklist:

includes/classes/Authentication/Passwords.php Outdated Show resolved Hide resolved
includes/classes/Authentication/Passwords.php Show resolved Hide resolved
@@ -307,6 +312,11 @@ public function validate_strong_password( $errors, $user_data ) {
return $errors;
}

// Validate the password against the Have I Been Pwned API.
if ( ! $this->is_password_secure( $password ) && is_wp_error( $errors ) ) {
$errors->add( 'password_reset_error', __( '<strong>ERROR:</strong> The password entered may have been included in a data breach and is not considered safe to use. Please choose another.', 'tenup' ) );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought (non-blocking): Do we want to trigger an error here, or should we add a warning to the screen pre-update? Alternatively, should there be a checkbox, like the weak password box, that lets them bypass this if they really want to use that password?

Copy link
Member Author

@jamesmorrison jamesmorrison Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "tell, don't ask" and force an error here; as you've noted elsewhere I'll add a filter to disable the functionality.

includes/classes/Authentication/Passwords.php Outdated Show resolved Hide resolved
includes/classes/Authentication/Passwords.php Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check haveibeenpwned API during password reset and account creation
2 participants