Skip to content

Commit

Permalink
Account Protection: Add more structure surrounding password rules
Browse files Browse the repository at this point in the history
  • Loading branch information
nateweller committed Feb 21, 2025
1 parent e6ee25f commit 994ea25
Show file tree
Hide file tree
Showing 14 changed files with 636 additions and 184 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ protected function register_hooks(): void {
*/
protected function register_runtime_hooks(): void {
// Validate password after successful login
add_action( 'wp_authenticate_user', array( $this->password_detection, 'login_form_password_detection' ), 10, 2 );
add_filter( 'wp_authenticate_user', array( $this->password_detection, 'login_form_password_detection' ), 10, 2 );

// Handle password detection login failure
add_action( 'wp_login_failed', array( $this->password_detection, 'handle_password_detection_validation_error' ), 10, 2 );
Expand All @@ -117,7 +117,7 @@ protected function register_runtime_hooks(): void {
add_action( 'wp_enqueue_scripts', array( $this->password_detection, 'enqueue_styles' ) );

// Add password validation
add_action( 'user_profile_update_errors', array( $this->password_manager, 'validate_profile_update' ), 10, 3 );
add_filter( 'user_profile_update_errors', array( $this->password_manager, 'filter_user_profile_update_errors' ), 10, 3 );
add_action( 'validate_password_reset', array( $this->password_manager, 'validate_password_reset' ), 10, 2 );

// Update recent passwords list
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public function login_form_password_detection( $user, string $password ) {
return $user;
}

if ( $this->validation_service->is_weak_password( $password ) ) {
if ( $this->validation_service->is_compromised_password( $password ) ) {
$transient = $this->generate_and_store_transient_data( $user->ID );

$email_sent = $this->email_service->api_send_auth_email( $user, $transient['auth_code'] );
Expand Down
28 changes: 14 additions & 14 deletions projects/packages/account-protection/src/class-password-manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,17 @@ public function __construct( ?Validation_Service $validation_service = null ) {
}

/**
* Validate the profile update.
* Inject additional password validation errors on profile update.
*
* @param \WP_Error $errors The error object.
* @param bool $update Whether the user is being updated.
* @param \stdClass $user A copy of the new user object.
* @see https://developer.wordpress.org/reference/hooks/user_profile_update_errors/
*
* @param \WP_Error $errors WP_Error object (passed by reference).
* @param bool $update Whether this is a user update.
* @param \stdClass $user User object (passed by reference).
*
* @return void
*/
public function validate_profile_update( \WP_Error $errors, bool $update, \stdClass $user ): void {
public function filter_user_profile_update_errors( \WP_Error $errors, bool $update, \stdClass $user ): void {
if ( empty( $user->user_pass ) ) {
return;
}
Expand All @@ -47,11 +49,10 @@ public function validate_profile_update( \WP_Error $errors, bool $update, \stdCl
return;
}

$error = $this->validation_service->get_first_validation_error( $user->user_pass, true, $user );
$validation_errors = $this->validation_service->get_validation_errors( $user->user_pass, (array) $user );

if ( ! empty( $error ) ) {
$errors->add( 'password_error', $error );
return;
if ( ! empty( $validation_errors ) ) {
$errors->add( 'password_error', $validation_errors[0] );
}
}

Expand Down Expand Up @@ -80,11 +81,10 @@ public function validate_password_reset( \WP_Error $errors, $user ): void {
}

// phpcs:ignore WordPress.Security.NonceVerification
$password = sanitize_text_field( wp_unslash( $_POST['pass1'] ) );
$error = $this->validation_service->get_first_validation_error( $password );
if ( ! empty( $error ) ) {
$errors->add( 'password_error', $error );
return;
$password = sanitize_text_field( wp_unslash( $_POST['pass1'] ) );
$validation_errors = $this->validation_service->get_validation_errors( $password, (array) $user );
if ( ! empty( $validation_errors ) ) {
$errors->add( 'password_error', $validation_errors[0] );
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ public function validate_password_ajax(): void {
wp_send_json_error( array( 'message' => __( 'Invalid nonce.', 'jetpack-account-protection' ) ) );
}

$user_specific = false;
$userdata = null;
if ( isset( $_POST['user_specific'] ) ) {
$user_specific = filter_var( sanitize_text_field( wp_unslash( $_POST['user_specific'] ) ), FILTER_VALIDATE_BOOLEAN );
$userdata = get_userdata( get_current_user_id() );
}

$password = sanitize_text_field( wp_unslash( $_POST['password'] ) );
$state = $this->validation_service->get_validation_state( $password, $user_specific );
$password = sanitize_text_field( wp_unslash( $_POST['password'] ) );
$validation_errors = $this->validation_service->get_validation_errors( $password, $userdata );

Check failure on line 50 in projects/packages/account-protection/src/class-password-strength-meter.php

View workflow job for this annotation

GitHub Actions / Static analysis

TypeError PhanTypeMismatchArgument Argument 2 ($userdata) is $userdata of type ?\WP_User|?false but \Automattic\Jetpack\Account_Protection\Validation_Service::get_validation_errors() takes ?array defined at src/class-validation-service.php:74 FAQ on Phan issues: pdWQjU-Jb-p2

wp_send_json_success( array( 'state' => $state ) );
wp_send_json_success( array( 'errors' => $validation_errors ) );
}

/**
Expand Down Expand Up @@ -98,15 +98,14 @@ public function localize_jetpack_data( bool $user_specific = false ): void {
'jetpack-password-strength-meter',
'jetpackData',
array(
'ajaxurl' => admin_url( 'admin-ajax.php' ),
'nonce' => wp_create_nonce( 'validate_password_nonce' ),
'userSpecific' => $user_specific,
'logo' => plugin_dir_url( __FILE__ ) . 'assets/jetpack-logo.svg',
'infoIcon' => plugin_dir_url( __FILE__ ) . 'assets/info.svg',
'checkIcon' => plugin_dir_url( __FILE__ ) . 'assets/check.svg',
'crossIcon' => plugin_dir_url( __FILE__ ) . 'assets/cross.svg',
'loadingIcon' => plugin_dir_url( __FILE__ ) . 'assets/loading.svg',
'validationInitialState' => $this->validation_service->get_validation_initial_state( $user_specific ),
'ajaxurl' => admin_url( 'admin-ajax.php' ),
'nonce' => wp_create_nonce( 'validate_password_nonce' ),
'userSpecific' => $user_specific,
'logo' => plugin_dir_url( __FILE__ ) . 'assets/jetpack-logo.svg',
'infoIcon' => plugin_dir_url( __FILE__ ) . 'assets/info.svg',
'checkIcon' => plugin_dir_url( __FILE__ ) . 'assets/check.svg',
'crossIcon' => plugin_dir_url( __FILE__ ) . 'assets/cross.svg',
'loadingIcon' => plugin_dir_url( __FILE__ ) . 'assets/loading.svg',
)
);
}
Expand Down
212 changes: 71 additions & 141 deletions projects/packages/account-protection/src/class-validation-service.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,20 @@ class Validation_Service {
*/
private $connection_manager;

/**
* Validation rules.
*
* @var array
*/
private $rules = array(
Min_Length_Rule::class,
Not_Empty_Validation_Rule::class,

Check failure on line 32 in projects/packages/account-protection/src/class-validation-service.php

View workflow job for this annotation

GitHub Actions / Static analysis

UndefError PhanUndeclaredClassReference Reference to undeclared class \Automattic\Jetpack\Account_Protection\Not_Empty_Validation_Rule FAQ on Phan issues: pdWQjU-Jb-p2
No_Backslash_Rule::class,
Not_Recent_Rule::class,
Not_Compromised_Rule::class,
No_Userdata_Rule::class,
);

/**
* Constructor for dependency injection.
*
Expand Down Expand Up @@ -50,125 +64,42 @@ protected function request_suffixes( string $password_prefix ) {
}

/**
* Return validation initial state.
*
* @param bool $user_specific Whether or not to include user specific checks.
*
* @return array An array of all validation statuses and messages.
*/
public function get_validation_initial_state( $user_specific ): array {
$base_conditions = array(
'core' => array(
'status' => null,
'message' => __( 'Strong password', 'jetpack-account-protection' ),
'info' => __( 'Passwords should meet WordPress core security requirements to enhance account protection.', 'jetpack-account-protection' ),
),
'contains_backslash' => array(
'status' => null,
'message' => __( "Doesn't contain a backslash (\\) character", 'jetpack-account-protection' ),
'info' => null,
),
'invalid_length' => array(
'status' => null,
'message' => __( 'Between 6 and 150 characters', 'jetpack-account-protection' ),
'info' => null,
),
'weak' => array(
'status' => null,
'message' => __( 'Not a leaked password', 'jetpack-account-protection' ),
'info' => __( 'If found in a public breach, this password may already be known to attackers.', 'jetpack-account-protection' ),
),
);

if ( ! $user_specific ) {
return $base_conditions;
}

$user_specific_conditions = array(
'matches_user_data' => array(
'status' => null,
'message' => __( "Doesn't match existing user data", 'jetpack-account-protection' ),
'info' => __( 'Using a password similar to your username or email makes it easier to guess.', 'jetpack-account-protection' ),
),
'recent' => array(
'status' => null,
'message' => __( 'Not used recently', 'jetpack-account-protection' ),
'info' => __( 'Reusing old passwords may increase security risks. A fresh password improves protection.', 'jetpack-account-protection' ),
),
);

return array_merge( $base_conditions, $user_specific_conditions );
}

/**
* Return validation state - client-side.
* Get the validation errors.
*
* @param string $password The password to check.
* @param bool $user_specific Whether or not to run user specific checks.
* @param array $userdata The user data to check against, or null if not provided.

Check failure on line 70 in projects/packages/account-protection/src/class-validation-service.php

View workflow job for this annotation

GitHub Actions / Static analysis

TypeError PhanTypeMismatchDeclaredParamNullable Doc-block of $userdata in get_validation_errors is phpdoc param type array which is not a permitted replacement of the nullable param type ?array declared in the signature ('?T' should be documented as 'T|null' or '?T') FAQ on Phan issues: pdWQjU-Jb-p2
*
* @return array An array of the status of each check.
* @return string[] List of broken rule IDs.
*/
public function get_validation_state( string $password, $user_specific ): array {
$validation_state = $this->get_validation_initial_state( $user_specific );

$validation_state['contains_backslash']['status'] = $this->contains_backslash( $password );
$validation_state['invalid_length']['status'] = $this->is_invalid_length( $password );
$validation_state['weak']['status'] = $this->is_weak_password( $password );

if ( ! $user_specific ) {
return $validation_state;
}

// Run checks on existing user data
$user = wp_get_current_user();
$validation_state['matches_user_data']['status'] = $this->matches_user_data( $user, $password );
$validation_state['recent']['status'] = $this->is_recent_password( $user, $password );

return $validation_state;
}

/**
* Return first validation error - server-side.
*
* @param string $password The password to check.
* @param bool $user_specific Whether or not to run user specific checks.
* @param \stdClass|null $user The user data or null.
*
* @return string The first validation errors (if any).
*/
public function get_first_validation_error( string $password, $user_specific = false, $user = null ): string {
// Update and create-user forms include backlash validation
if ( ! $user_specific ) {
if ( $this->contains_backslash( $password ) ) {
return __( '<strong>Error:</strong> The password cannot contain a backslash (\\) character.', 'jetpack-account-protection' );
public function get_validation_errors( string $password, array $userdata = null ) {

Check failure on line 74 in projects/packages/account-protection/src/class-validation-service.php

View workflow job for this annotation

GitHub Actions / Static analysis

DeprecatedError PhanDeprecatedImplicitNullableParam Implicit nullable parameters (array $userdata = null) have been deprecated in PHP 8.4 FAQ on Phan issues: pdWQjU-Jb-p2

Check warning on line 74 in projects/packages/account-protection/src/class-validation-service.php

View workflow job for this annotation

GitHub Actions / dev branch for PHP 8.0

Implicitly marking a parameter as nullable is deprecated since PHP 8.4. Update the type to be explicitly nullable instead. Found implicitly nullable parameter: $userdata. (PHPCompatibility.FunctionDeclarations.RemovedImplicitlyNullableParam.Deprecated)
$broken_rules = array();

foreach ( $this->rules as $rule ) {
$rule = new $rule( $this );

// Get the number of parameters the callback accepts;
$param_names = array_map(
function ( $param ) {
return $param->getName();
},
( new \ReflectionMethod( $rule, 'callback' ) )->getParameters()
);

// Assemble the arguments required by the callback.
$args = array( $password );
if ( in_array( 'userdata', $param_names, true ) ) {
$args[] = $userdata;
}
}

if ( $this->is_invalid_length( $password ) ) {
return __( '<strong>Error:</strong> The password must be between 6 and 150 characters.', 'jetpack-account-protection' );
}
// Call the callback.
$validation_callback_result = call_user_func_array( array( $rule, 'callback' ), $args );

if ( $this->is_weak_password( $password ) ) {
return __( '<strong>Error:</strong> The password was found in a public leak.', 'jetpack-account-protection' );
}

// Skip user-specific checks during password reset
if ( $user_specific ) {
// Reset form includes empty validation
if ( empty( $password ) ) {
return __( '<strong>Error:</strong> The password cannot be a space or all spaces.', 'jetpack-account-protection' );
}

// Run checks on new user data
if ( $this->matches_user_data( $user, $password ) ) {
return __( '<strong>Error:</strong> The password matches new user data.', 'jetpack-account-protection' );
}
if ( $this->is_recent_password( $user, $password ) ) {
return __( '<strong>Error:</strong> The password was used recently.', 'jetpack-account-protection' );
if ( ! $validation_callback_result ) {
$broken_rules[] = $rule->id;
}
}

return '';
return $broken_rules;
}

/**
Expand Down Expand Up @@ -197,41 +128,37 @@ public function is_invalid_length( string $password ): bool {
/**
* Check if the password matches any user data.
*
* @param \WP_User|\stdClass $user The user.
* @param string $password The password to check.
* @param string $password The password to check.
* @param array $userdata The user data.
*
* @return bool True if the password matches any user data, false otherwise.
*/
public function matches_user_data( $user, string $password ): bool {
if ( ! $user ) {
return false;
}

$email_parts = explode( '@', $user->user_email ); // [email protected]
$email_username = $email_parts[0]; // 'test'
$email_domain = $email_parts[1]; // 'example.com'
$email_provider = explode( '.', $email_domain )[0]; // 'example'

$user_data = array(
$user->user_login ?? '',
$user->display_name ?? '',
$user->first_name ?? '',
$user->last_name ?? '',
$user->user_email ?? '',
$email_username ?? '',
$email_provider ?? '',
$user->nickname ?? '',
public function matches_user_data( string $password, array $userdata ): bool {
$data_to_match = array(
$userdata['user_login'] ?? '',
$userdata['display_name'] ?? '',
$userdata['first_name'] ?? '',
$userdata['last_name'] ?? '',
$userdata['user_email'] ?? '',
$userdata['nickname'] ?? '',
);

$password_lower = strtolower( $password );
if ( $userdata['user_email'] ) {
$email_parts = explode( '@', $userdata['user_email'] ); // [email protected]
$email_username = $email_parts[0]; // 'test'
$email_domain = $email_parts[1]; // 'example.com'
$email_provider = explode( '.', $email_domain )[0]; // 'example'

foreach ( $user_data as $data ) {
// Skip if $data is 3 characters or less.
$data_to_match[] = $email_username;
$data_to_match[] = $email_provider;
}

foreach ( $data_to_match as $data ) {
if ( strlen( $data ) <= 3 ) {
continue;
}

if ( ! empty( $data ) && strpos( $password_lower, strtolower( $data ) ) !== false ) {
if ( strpos( strtolower( $password ), strtolower( $data ) ) !== false ) {
return true;
}
}
Expand All @@ -246,7 +173,7 @@ public function matches_user_data( $user, string $password ): bool {
*
* @return bool True if the password is in the list of compromised/common passwords, false otherwise.
*/
public function is_weak_password( string $password ): bool {
public function is_compromised_password( string $password ): bool {
if ( ! $this->connection_manager->is_connected() ) {
return false;
}
Expand Down Expand Up @@ -296,18 +223,21 @@ public function is_current_password( int $user_id, string $password ): bool {
/**
* Check if the password has been used recently by the user.
*
* @param \WP_User|\stdClass $user The user data.
* @param string $password The password to check.
* @param string $password The password to check.
* @param array $userdata The user data.
*
* @return bool True if the password was recently used, false otherwise.
*/
public function is_recent_password( $user, string $password ): bool {
$user_data = $user instanceof \WP_User ? $user : get_userdata( $user->ID );
if ( $this->is_current_password( $user_data->ID, $password ) ) {
public function is_recent_password( string $password, array $userdata ): bool {
if ( ! array_key_exists( 'ID', $userdata ) ) {
return false;
}

if ( $this->is_current_password( $userdata['ID'], $password ) ) {
return true;
}

$recent_passwords = get_user_meta( $user->ID, Config::PASSWORD_MANAGER_RECENT_PASSWORD_HASHES_USER_META_KEY, true );
$recent_passwords = get_user_meta( $userdata['ID'], Config::PASSWORD_MANAGER_RECENT_PASSWORD_HASHES_USER_META_KEY, true );
if ( empty( $recent_passwords ) || ! is_array( $recent_passwords ) ) {
return false;
}
Expand Down
Loading

0 comments on commit 994ea25

Please sign in to comment.