From 0c06ef5684b916aff1a3af0a2a6f60584788e1e6 Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Wed, 10 Apr 2024 10:31:55 +0200 Subject: [PATCH 1/7] fix(cli): membership dedupe command Tweaks to changes from #84 --- includes/cli/class-membership-dedupe.php | 127 +++++++++++++++-------- 1 file changed, 85 insertions(+), 42 deletions(-) diff --git a/includes/cli/class-membership-dedupe.php b/includes/cli/class-membership-dedupe.php index 1b9a3398..9d209e2d 100644 --- a/includes/cli/class-membership-dedupe.php +++ b/includes/cli/class-membership-dedupe.php @@ -39,7 +39,7 @@ public static function register_commands() { [ 'type' => 'assoc', 'name' => 'plan-id', - 'optional' => false, + 'optional' => true, ], [ 'type' => 'flag', @@ -64,62 +64,97 @@ public static function register_commands() { * * wp newspack-network clean-up-duplicate-memberships --plan-id=1234 * + * ## OPTIONS + * + * [--plan-id] + * : Membership plan ID to clean up. If not set, all synchronized plans will be cleaned up. + * + * [--live] + * : Run the command in live mode, updating the users. + * + * [--csv] + * : Output CSV. + * * @param array $args Positional args. * @param array $assoc_args Associative args and flags. */ public static function clean_duplicate_memberships( $args, $assoc_args ) { WP_CLI::line( '' ); - $live = isset( $assoc_args['live'] ); - $csv = isset( $assoc_args['csv'] ); - - $plan_id = $assoc_args['plan-id']; - if ( ! is_numeric( $plan_id ) ) { - WP_CLI::error( 'Membership plan ID must be numeric' ); - } - $plan_id = (int) $plan_id; + $live = isset( $assoc_args['live'] ); if ( ! $live ) { WP_CLI::line( 'Running in dry-run mode. Use --live flag to run in live mode.' ); WP_CLI::line( '' ); } - $user_ids = self::get_users_with_duplicate_membership( $plan_id ); - WP_CLI::line( sprintf( '%d users found with duplicate memberships', count( $user_ids ) ) ); + $csv = isset( $assoc_args['csv'] ); - $duplicates = []; - foreach ( $user_ids as $user_id ) { - $memberships = get_posts( + $plan_id_from_args = isset( $assoc_args['plan-id'] ) ? $assoc_args['plan-id'] : null; + $plan_ids = []; + if ( $plan_id_from_args ) { + if ( ! is_numeric( $plan_id_from_args ) ) { + WP_CLI::error( 'Membership plan ID must be numeric' ); + } + $plan_ids = [ (int) $plan_id_from_args ]; + } else { + // Get all network-sync'd membership plans. + $plan_ids = get_posts( [ - 'author' => $user_id, - 'post_type' => 'wc_user_membership', - 'post_status' => 'any', - 'post_parent' => $plan_id, + 'post_type' => 'wc_membership_plan', + 'fields' => 'ids', + 'meta_query' => [ // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_query + [ + 'key' => \Newspack_Network\Woocommerce_Memberships\Admin::NETWORK_ID_META_KEY, + 'compare' => 'EXISTS', + ], + ], ] ); + } - foreach ( $memberships as $membership ) { - $user = get_user_by( 'id', $membership->post_author ); - $duplicates[] = [ - 'user' => $membership->post_author, - 'email' => $user->user_email, - 'membership' => $membership->ID, - 'subscription' => get_post_meta( $membership->ID, '_subscription_id', true ), - 'status' => $membership->post_status, - 'remote' => get_post_meta( $membership->ID, '_remote_site_url', true ), - ]; + $user_ids = []; + foreach ( $plan_ids as $plan_id ) { + WP_CLI::line( sprintf( 'Checking plan #%d', $plan_id ) ); + + $user_ids = array_merge( $user_ids, self::get_users_with_duplicate_membership( $plan_id ) ); + WP_CLI::line( sprintf( '%d users found with duplicate memberships', count( $user_ids ) ) ); + + $duplicates = []; + foreach ( $user_ids as $user_id ) { + $memberships = get_posts( + [ + 'author' => $user_id, + 'post_type' => 'wc_user_membership', + 'post_status' => 'any', + 'post_parent' => $plan_id, + ] + ); + + foreach ( $memberships as $membership ) { + $user = get_user_by( 'id', $membership->post_author ); + if ( $user === false ) { + continue; + } + $duplicates[] = [ + 'user' => $membership->post_author, + 'email' => $user->user_email, + 'membership' => $membership->ID, + 'subscription' => get_post_meta( $membership->ID, '_subscription_id', true ), + 'status' => $membership->post_status, + 'remote' => get_post_meta( $membership->ID, '_remote_site_url', true ), + ]; + } } - } - if ( $csv && ! empty( $duplicates ) ) { - WP_CLI::line( 'COPY AND PASTE THIS CSV: ' ); - WP_CLI::line(); - WP_CLI\Utils\format_items( 'csv', $duplicates, array_keys( $duplicates[0] ) ); - WP_CLI::line(); - } + if ( $csv && ! empty( $duplicates ) ) { + WP_CLI::line( 'COPY AND PASTE THIS CSV: ' ); + WP_CLI::line(); + WP_CLI\Utils\format_items( 'csv', $duplicates, array_keys( $duplicates[0] ) ); + WP_CLI::line(); + } - if ( $live ) { - WP_CLI::line( 'Deleting duplicates' ); - self::deduplicate_memberships( $duplicates ); + self::deduplicate_memberships( $duplicates, $live ); + WP_CLI::line( '' ); } WP_CLI::success( 'Done' ); @@ -151,8 +186,12 @@ private static function get_users_with_duplicate_membership( $plan_id ) { * De-duplicate memberships so that users only have one membership of a plan. * * @param array $duplicates Analyzed data from ::clean_duplicate_memberships. + * @param bool $live Whether to actually delete the duplicates. */ - private static function deduplicate_memberships( $duplicates ) { + private static function deduplicate_memberships( $duplicates, $live ) { + if ( $live ) { + WP_CLI::line( 'Deleting duplicates' ); + } $userdata = []; foreach ( $duplicates as $duplicate ) { @@ -166,13 +205,17 @@ private static function deduplicate_memberships( $duplicates ) { foreach ( $userdata as $email => $duplicates ) { WP_CLI::line( sprintf( 'Processing %s', $email ) ); if ( count( $duplicates ) < 2 ) { - WP_CLI::line( ' - User does not have too many memberships' ); + WP_CLI::line( ' - User has multiple memberships, but no duplicates' ); } $memberships_to_delete = array_slice( $duplicates, 1 ); foreach ( $memberships_to_delete as $duplicate ) { - wp_delete_post( $duplicate['membership'], true ); - WP_CLI::line( sprintf( ' - Deleted extra membership %d', $duplicate['membership'] ) ); + if ( $live ) { + wp_delete_post( $duplicate['membership'], true ); + WP_CLI::line( sprintf( ' - Deleted extra membership %d', $duplicate['membership'] ) ); + } else { + WP_CLI::line( sprintf( ' - Would have deleted extra membership %d', $duplicate['membership'] ) ); + } } } } From 2ae49146f1c140eab3b0a84b8b21cd3629e2a54d Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Wed, 10 Apr 2024 10:31:55 +0200 Subject: [PATCH 2/7] refactor: event log page slug constant --- includes/constants.php | 4 +++- includes/hub/admin/class-event-log.php | 9 ++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/includes/constants.php b/includes/constants.php index f0bc43b9..b3e96cd7 100644 --- a/includes/constants.php +++ b/includes/constants.php @@ -1,7 +1,7 @@ 'Invalid Signature.', 'INVALID_DATA' => 'Bad request. Invalid Data.', ]; + +const EVENT_LOG_PAGE_SLUG = 'newspack-network-event-log'; diff --git a/includes/hub/admin/class-event-log.php b/includes/hub/admin/class-event-log.php index f7170018..cc68ff7c 100644 --- a/includes/hub/admin/class-event-log.php +++ b/includes/hub/admin/class-event-log.php @@ -8,14 +8,13 @@ namespace Newspack_Network\Hub\Admin; use Newspack_Network\Admin as Network_Admin; +use const Newspack_Network\constants\EVENT_LOG_PAGE_SLUG; /** * Class to handle the Event log admin page */ class Event_Log { - const PAGE_SLUG = 'newspack-network-event-log'; - /** * Runs the initialization. */ @@ -30,7 +29,7 @@ public static function init() { * @return void */ public static function add_admin_menu() { - Network_Admin::add_submenu_page( __( 'Event Log', 'newspack-network' ), self::PAGE_SLUG, [ __CLASS__, 'render_page' ] ); + Network_Admin::add_submenu_page( __( 'Event Log', 'newspack-network' ), EVENT_LOG_PAGE_SLUG, [ __CLASS__, 'render_page' ] ); } /** @@ -39,7 +38,7 @@ public static function add_admin_menu() { * @return void */ public static function admin_enqueue_scripts() { - $page_slug = Network_Admin::PAGE_SLUG . '_page_' . self::PAGE_SLUG; + $page_slug = Network_Admin::PAGE_SLUG . '_page_' . EVENT_LOG_PAGE_SLUG; if ( get_current_screen()->id !== $page_slug ) { return; } @@ -62,7 +61,7 @@ public static function render_page() { echo '

', esc_html( __( 'Event Log', 'newspack-network' ) ), '

'; echo '
'; - echo ''; + echo ''; $table->prepare_items(); From 7a585f1c2e678d81c5d2e3f201448c2e0ddf555b Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Wed, 10 Apr 2024 10:31:55 +0200 Subject: [PATCH 3/7] refactor: request to hub --- includes/hub/class-pull-endpoint.php | 34 +++-------- includes/node/class-pulling.php | 40 ++----------- includes/utils/class-requests.php | 88 ++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 63 deletions(-) create mode 100644 includes/utils/class-requests.php diff --git a/includes/hub/class-pull-endpoint.php b/includes/hub/class-pull-endpoint.php index f6f91d68..cb45b645 100644 --- a/includes/hub/class-pull-endpoint.php +++ b/includes/hub/class-pull-endpoint.php @@ -61,42 +61,22 @@ public static function get_pull_limit() { * @return WP_REST_Response */ public static function handle_pull( $request ) { + $request_error = \Newspack_Network\Utils\Requests::get_request_to_hub_errors( $request ); + if ( \is_wp_error( $request_error ) ) { + return new WP_REST_Response( [ 'error' => $request_error->get_error_message() ], 403 ); + } + $site = $request['site']; $last_processed_id = $request['last_processed_id']; $actions = $request['actions']; - $signature = $request['signature']; - $nonce = $request['nonce']; - Debugger::log( 'Pull request received' ); - Debugger::log( $site ); - Debugger::log( $last_processed_id ); - Debugger::log( $actions ); + Debugger::log( sprintf( 'Pull request received from site %s, with last processed ID %d, for actions: %s.', $site, $last_processed_id, implode( ', ', $actions ) ) ); - if ( empty( $site ) || - empty( $actions ) || - empty( $nonce ) || - empty( $signature ) - ) { + if ( empty( $actions ) ) { return new WP_REST_Response( array( 'error' => 'Bad request.' ), 400 ); } $node = Nodes::get_node_by_url( $site ); - - if ( ! $node ) { - Debugger::log( 'Node not found.' ); - return new WP_REST_Response( array( 'error' => 'Bad request. Site not registered in this Hub.' ), 403 ); - } - - $verified = $node->decrypt_message( $signature, $nonce ); - $verified_message = json_decode( $verified ); - - if ( ! $verified || ! is_object( $verified_message ) || (int) $last_processed_id !== (int) $verified_message->last_processed_id ) { - Debugger::log( 'Signature check failed' ); - return new WP_REST_Response( array( 'error' => 'INVALID_SIGNATURE' ), 403 ); - } - - Debugger::log( 'Successfully verified request' ); - $query_args = [ 'excluded_node_id' => $node->get_id(), 'id_greater_than' => $last_processed_id, diff --git a/includes/node/class-pulling.php b/includes/node/class-pulling.php index 9f748624..edbf10ab 100644 --- a/includes/node/class-pulling.php +++ b/includes/node/class-pulling.php @@ -132,49 +132,17 @@ public static function set_last_processed_id( $id ) { } /** - * Gets the request parameters for the pull request + * Makes a request to the Hub to pull data * - * @return array + * @return array|\WP_Error */ - public static function get_request_params() { + public static function make_request() { $params = [ 'last_processed_id' => self::get_last_processed_id(), 'actions' => Accepted_Actions::ACTIONS_THAT_NODES_PULL, 'site' => get_bloginfo( 'url' ), ]; - return self::sign_params( $params ); - } - - /** - * Signs the request parameters with the Node's secret key - * - * @param array $params The request parameters. - * @return array The params array with an additional signature key. - */ - public static function sign_params( $params ) { - $message = wp_json_encode( $params ); - $secret_key = Settings::get_secret_key(); - $nonce = Crypto::generate_nonce(); - $signature = Crypto::encrypt_message( $message, $secret_key, $nonce ); - $params['signature'] = $signature; - $params['nonce'] = $nonce; - return $params; - } - - /** - * Makes a request to the Hub to pull data - * - * @return array|\WP_Error - */ - public static function make_request() { - $url = trailingslashit( Settings::get_hub_url() ) . 'wp-json/newspack-network/v1/pull'; - $params = self::get_request_params(); - $response = wp_remote_post( - $url, - [ - 'body' => $params, - ] - ); + $response = \Newspack_Network\Utils\Requests::request_to_hub( 'wp-json/newspack-network/v1/pull', $params ); if ( is_wp_error( $response ) ) { return $response; } diff --git a/includes/utils/class-requests.php b/includes/utils/class-requests.php new file mode 100644 index 00000000..52b47e33 --- /dev/null +++ b/includes/utils/class-requests.php @@ -0,0 +1,88 @@ + $method, + 'body' => self::sign_params( $params ), + 'timeout' => 60, // phpcs:ignore WordPressVIPMinimum.Performance.RemoteRequestTimeout.timeout_timeout + ] + ); + } + + /** + * Signs the request parameters with the Node's secret key + * + * @param array $params The request parameters. + * @return array The params array with an additional signature key. + */ + public static function sign_params( $params ) { + $message = wp_json_encode( $params ); + $secret_key = Settings::get_secret_key(); + $nonce = Crypto::generate_nonce(); + $signature = Crypto::encrypt_message( $message, $secret_key, $nonce ); + $params['signature'] = $signature; + $params['nonce'] = $nonce; + return $params; + } + + /** + * Validate a request. + * + * @param WP_REST_Request $request Full data about the request. + * @return bool|WP_Error True if the request is valid, WP_Error otherwise. + */ + public static function get_request_to_hub_errors( $request ) { + $site = $request['site']; + $signature = $request['signature']; + $nonce = $request['nonce']; + + if ( empty( $site ) || + empty( $nonce ) || + empty( $signature ) + ) { + return new WP_Error( 'newspack_network_bad_request', __( 'Bad request.', 'newspack-network' ) ); + } + + $node = \Newspack_Network\Hub\Nodes::get_node_by_url( $site ); + + if ( ! $node ) { + \Newspack_Network\Debugger::log( 'Node not found.' ); + return new WP_Error( 'newspack_network_bad_request_node_not_found', __( 'Bad request. Site not registered in this Hub', 'newspack-network' ) ); + } + + $verified = $node->decrypt_message( $signature, $nonce ); + $verified_message = json_decode( $verified ); + if ( ! $verified || ! is_object( $verified_message ) ) { + \Newspack_Network\Debugger::log( 'Signature check failed' ); + return new WP_Error( 'newspack_network_bad_request_signature', __( 'Bad request. Invalid signature.', 'newspack-network' ) ); + } + + return true; + } +} From e90b96f7372fb6bdb7a919263121beebdc74aa27 Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Wed, 10 Apr 2024 10:31:55 +0200 Subject: [PATCH 4/7] chore: comment typo --- includes/hub/database/class-orders.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/hub/database/class-orders.php b/includes/hub/database/class-orders.php index 5cc02b1c..5a073546 100644 --- a/includes/hub/database/class-orders.php +++ b/includes/hub/database/class-orders.php @@ -12,7 +12,7 @@ use Newspack_Network\Debugger; /** - * Class to handle the ubscriptions post type registration + * Class to handle the Orders post type registration */ class Orders { From aee431687478117c40727bf43ce6220d5e470d4f Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Wed, 10 Apr 2024 10:31:55 +0200 Subject: [PATCH 5/7] fix: username generation --- includes/utils/class-users.php | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/includes/utils/class-users.php b/includes/utils/class-users.php index fe6d5128..41aa3c58 100644 --- a/includes/utils/class-users.php +++ b/includes/utils/class-users.php @@ -26,7 +26,6 @@ class Users { * @return WP_User|WP_Error */ public static function get_or_create_user_by_email( $email, $remote_site_url, $remote_id, $insert_array = [] ) { - $existing_user = get_user_by( 'email', $email ); if ( $existing_user ) { @@ -40,10 +39,13 @@ public static function get_or_create_user_by_email( $email, $remote_site_url, $r return $existing_user; } + $nicename = self::generate_user_nicename( $email ); + $user_array = [ - 'user_login' => $email, + 'user_login' => substr( $email, 0, 60 ), 'user_email' => $email, - 'user_nicename' => $email, + 'user_nicename' => $nicename, + 'display_name' => $nicename, 'user_pass' => wp_generate_password(), 'role' => NEWSPACK_NETWORK_READER_ROLE, ]; @@ -76,6 +78,29 @@ public static function get_or_create_user_by_email( $email, $remote_site_url, $r return $new_user; } + /** + * Generate a URL-sanitized version of the given string for a new reader account. + * + * @param string $name User's display name, or email if not available. + * @return string + */ + public static function generate_user_nicename( $name ) { + $name = self::strip_email_domain( $name ); // If an email address, strip the domain. + + return substr( \sanitize_title( \sanitize_user( $name, true ) ), 0, 50 ); + } + + /** + * Strip the domain part of an email address string. + * If not an email address, just return the string. + * + * @param string $str String to check. + * @return string + */ + public static function strip_email_domain( $str ) { + return trim( explode( '@', $str, 2 )[0] ); + } + /** * Looks for avatar information and tries to sideload it to the user * From 04bf2545be2ec65e4e2fee62765ef2b51add91b3 Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Wed, 10 Apr 2024 10:31:55 +0200 Subject: [PATCH 6/7] feat(event-reader-registered): handle existing users by adding a role --- includes/incoming-events/class-reader-registered.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/includes/incoming-events/class-reader-registered.php b/includes/incoming-events/class-reader-registered.php index 5f35a17c..89f881ef 100644 --- a/includes/incoming-events/class-reader-registered.php +++ b/includes/incoming-events/class-reader-registered.php @@ -50,6 +50,15 @@ public function maybe_create_user() { User_Update_Watcher::$enabled = false; - $user = User_Utils::get_or_create_user_by_email( $email, $this->get_site(), $this->data->user_id ?? '' ); + // If a user exists, but has a non-synchronizable role, add a synchronizable role. + $existing_user = get_user_by( 'email', $email ); + if ( $existing_user ) { + $synced_roles = \Newspack_Network\Utils\Users::get_synced_user_roles(); + if ( ! array_intersect( $existing_user->roles, $synced_roles ) ) { + $existing_user->add_role( $synced_roles[0] ); + } + } else { + $user = User_Utils::get_or_create_user_by_email( $email, $this->get_site(), $this->data->user_id ?? '', (array) $this->data ); + } } } From 8f91df9d45cebd74ce4f9ea3082386cc0f99ed0f Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Thu, 16 May 2024 14:24:17 +0200 Subject: [PATCH 7/7] fix: constant import --- includes/class-users.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/includes/class-users.php b/includes/class-users.php index d3d48d3b..16fa3507 100644 --- a/includes/class-users.php +++ b/includes/class-users.php @@ -7,6 +7,8 @@ namespace Newspack_Network; +use const Newspack_Network\constants\EVENT_LOG_PAGE_SLUG; + /** * Class to handle the Users admin page */ @@ -75,7 +77,7 @@ public static function manage_users_custom_column( $value, $column_name, $user_i $summary = $last_activity->get_summary(); $event_log_url = add_query_arg( [ - 'page' => \Newspack_Network\Hub\Admin\Event_Log::PAGE_SLUG, + 'page' => EVENT_LOG_PAGE_SLUG, 'email' => $user->user_email, ], admin_url( 'admin.php' )