From a330aee9b79a19a5157f8e0ef367bd53deae2435 Mon Sep 17 00:00:00 2001 From: dkoo Date: Fri, 10 May 2024 08:25:39 -0600 Subject: [PATCH 1/5] feat: start new custom table for oauth transients data --- includes/class-newspack.php | 1 + includes/oauth/class-oauth-transients.php | 156 ++++++++++++++++++++++ 2 files changed, 157 insertions(+) create mode 100644 includes/oauth/class-oauth-transients.php diff --git a/includes/class-newspack.php b/includes/class-newspack.php index 06e38ffdc1..a811c960c2 100644 --- a/includes/class-newspack.php +++ b/includes/class-newspack.php @@ -101,6 +101,7 @@ private function includes() { include_once NEWSPACK_ABSPATH . 'includes/reader-revenue/my-account/class-woocommerce-my-account.php'; include_once NEWSPACK_ABSPATH . 'includes/reader-revenue/class-reader-revenue-emails.php'; include_once NEWSPACK_ABSPATH . 'includes/oauth/class-oauth.php'; + include_once NEWSPACK_ABSPATH . 'includes/oauth/class-oauth-transients.php'; include_once NEWSPACK_ABSPATH . 'includes/oauth/class-google-oauth.php'; include_once NEWSPACK_ABSPATH . 'includes/oauth/class-google-services-connection.php'; include_once NEWSPACK_ABSPATH . 'includes/oauth/class-mailchimp-api.php'; diff --git a/includes/oauth/class-oauth-transients.php b/includes/oauth/class-oauth-transients.php new file mode 100644 index 0000000000..1413b8e8d6 --- /dev/null +++ b/includes/oauth/class-oauth-transients.php @@ -0,0 +1,156 @@ +prefix . self::TABLE_NAME; + } + + /** + * Checks if the custom table has been created and is up-to-date. + * If not, run the create_custom_table method. + * See: https://codex.wordpress.org/Creating_Tables_with_Plugins + */ + public static function check_update_version() { + $current_version = \get_option( self::TABLE_VERSION_OPTION, false ); + + if ( self::TABLE_VERSION !== $current_version ) { + self::create_custom_table(); + \update_option( self::TABLE_VERSION_OPTION, self::TABLE_VERSION ); + } + } + + /** + * Create a custom DB table to store transient data needed for OAuth. + * Avoids the use of slow post meta for query sorting purposes. + * Only create the table if it doesn't already exist. + */ + public static function create_custom_table() { + global $wpdb; + $table_name = self::get_table_name(); + + if ( $wpdb->get_var( $wpdb->prepare( 'SHOW TABLES LIKE %s', $table_name ) ) != $table_name ) { // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + $charset_collate = $wpdb->get_charset_collate(); + $sql = "CREATE TABLE $table_name ( + -- Reader's unique ID. + id varchar(100) NOT NULL, + -- Scope of the data. + scope varchar(30) NOT NULL, + -- Value of the data. + value varchar(100) NOT NULL, + -- Timestamp when data was created. + created_at datetime NOT NULL, + PRIMARY KEY (id, scope), + KEY (scope) + ) $charset_collate;"; + + require_once ABSPATH . 'wp-admin/includes/upgrade.php'; + dbDelta( $sql ); + } + } + + /** + * Get a value from the database. + * + * @param string $id The reader's unique ID. + * @param string $scope The scope of the data to get. + * @param string $field_to_get The column to get. Defaults to 'value'. + * + * @return mixed The value of the data, or false if not found. + */ + public static function get( $id, $scope, $field_to_get = 'value' ) { + global $wpdb; + + $table_name = self::get_table_name(); + $value = $wpdb->get_var( // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + $wpdb->prepare( + 'SELECT %1$s FROM %2$s WHERE id = %3$d AND scope = %4$s', // phpcs:ignore WordPress.DB.PreparedSQLPlaceholders.UnquotedComplexPlaceholder + $field_to_get, + $table_name, + $id, + $scope + ) + ); + + return $value ?? false; + } + + /** + * Set a value in the database. + * + * @param string $id The reader's unique ID. + * @param string $scope The scope of the data to set. + * @param string $value The value of the data to set. + * + * @return bool True if the value was set, false otherwise. + */ + public static function set( $id, $scope, $value ) { + global $wpdb; + + $table_name = self::get_table_name(); + $result = $wpdb->insert( // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + $table_name, + [ + 'id' => $id, + 'scope' => $scope, + 'value' => $value, + 'created_at' => \current_time( 'mysql' ), + ], + [ + '%s', + '%s', + '%s', + '%s', + ] + ); + + return $result; + } +} + +OAuth_Transients::init(); From 5ad805b4e98799dbb919642c69bd56c787eb3669 Mon Sep 17 00:00:00 2001 From: dkoo Date: Fri, 10 May 2024 08:27:34 -0600 Subject: [PATCH 2/5] fix: copy/paste error --- includes/oauth/class-oauth-transients.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/oauth/class-oauth-transients.php b/includes/oauth/class-oauth-transients.php index 1413b8e8d6..c68beba65c 100644 --- a/includes/oauth/class-oauth-transients.php +++ b/includes/oauth/class-oauth-transients.php @@ -39,7 +39,7 @@ class OAuth_Transients { * Initialize hooks. */ public static function init() { - register_activation_hook( NEWSPACK_LISTINGS_FILE, [ __CLASS__, 'create_custom_table' ] ); + register_activation_hook( NEWSPACK_PLUGIN_FILE, [ __CLASS__, 'create_custom_table' ] ); add_action( 'init', [ __CLASS__, 'check_update_version' ] ); } From 0c94e3ec16cc6f4464cfb262d60ad639fbf8cd42 Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Sun, 12 May 2024 16:08:03 +0200 Subject: [PATCH 3/5] fix(google-oauth): use custom table for transients --- includes/oauth/class-google-login.php | 9 +++-- includes/oauth/class-oauth-transients.php | 43 ++++++++++++++++++++--- includes/oauth/class-oauth.php | 15 ++++---- 3 files changed, 50 insertions(+), 17 deletions(-) diff --git a/includes/oauth/class-google-login.php b/includes/oauth/class-google-login.php index 326fdd5423..56a609e5b1 100644 --- a/includes/oauth/class-google-login.php +++ b/includes/oauth/class-google-login.php @@ -144,10 +144,9 @@ public static function oauth_callback() { Logger::log( 'Got user email from Google: ' . $user_email ); // Associate the email address with the a unique ID for later retrieval. - $transient_expiration_time = 60 * 5; // 5 minutes. - $has_set_transient = set_transient( self::EMAIL_TRANSIENT_PREFIX . OAuth::get_unique_id(), $user_email, $transient_expiration_time ); + $set_transient_result = OAuth_Transients::set( OAuth::get_unique_id(), 'email', $user_email ); // If transient setting failed, the email address will not be available for the registration endpoint. - if ( ! $has_set_transient ) { + if ( $set_transient_result === false ) { self::handle_error( __( 'Failed setting transient.', 'newspack-plugin' ) ); \wp_die( \esc_html__( 'Authentication failed.', 'newspack-plugin' ) ); } @@ -178,8 +177,8 @@ private static function handle_error( $message ) { public static function api_google_login_register( $request ) { $uid = OAuth::get_unique_id(); // Retrieve the email address associated with the unique ID when the user was authenticated. - $email = get_transient( self::EMAIL_TRANSIENT_PREFIX . $uid ); - delete_transient( self::EMAIL_TRANSIENT_PREFIX . $uid ); // Burn after reading. + $email = OAuth_Transients::get( $uid, 'email' ); + OAuth_Transients::delete( $uid, 'email' ); $metadata = []; if ( $request->get_param( 'metadata' ) ) { try { diff --git a/includes/oauth/class-oauth-transients.php b/includes/oauth/class-oauth-transients.php index c68beba65c..2aa5eb78ca 100644 --- a/includes/oauth/class-oauth-transients.php +++ b/includes/oauth/class-oauth-transients.php @@ -105,11 +105,11 @@ public static function create_custom_table() { */ public static function get( $id, $scope, $field_to_get = 'value' ) { global $wpdb; - $table_name = self::get_table_name(); - $value = $wpdb->get_var( // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + + $value = $wpdb->get_var( // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching $wpdb->prepare( - 'SELECT %1$s FROM %2$s WHERE id = %3$d AND scope = %4$s', // phpcs:ignore WordPress.DB.PreparedSQLPlaceholders.UnquotedComplexPlaceholder + 'SELECT %1$s FROM %2$s WHERE id = "%3$s" AND scope = "%4$s"', // phpcs:ignore WordPress.DB.PreparedSQLPlaceholders.UnquotedComplexPlaceholder $field_to_get, $table_name, $id, @@ -120,6 +120,30 @@ public static function get( $id, $scope, $field_to_get = 'value' ) { return $value ?? false; } + /** + * Delete a row from the database. + * + * @param string $id The reader's unique ID. + * @param string $scope The scope of the data to get. + * + * @return boolean True if the row was deleted. + */ + public static function delete( $id, $scope ) { + global $wpdb; + $table_name = self::get_table_name(); + + $result = $wpdb->delete( // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + $table_name, + [ + 'id' => $id, + 'scope' => $scope, + ], + [ '%s', '%s' ] + ); + + return boolval( $result ); + } + /** * Set a value in the database. * @@ -127,11 +151,16 @@ public static function get( $id, $scope, $field_to_get = 'value' ) { * @param string $scope The scope of the data to set. * @param string $value The value of the data to set. * - * @return bool True if the value was set, false otherwise. + * @return mixed The value if it was set, false otherwise. */ public static function set( $id, $scope, $value ) { global $wpdb; + $existing = self::get( $id, $scope ); + if ( $existing ) { + return $existing; + } + $table_name = self::get_table_name(); $result = $wpdb->insert( // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching $table_name, @@ -149,7 +178,11 @@ public static function set( $id, $scope, $value ) { ] ); - return $result; + if ( $result === false ) { + return false; + } + + return $value; } } diff --git a/includes/oauth/class-oauth.php b/includes/oauth/class-oauth.php index 937df3dbe9..6ee8794917 100644 --- a/includes/oauth/class-oauth.php +++ b/includes/oauth/class-oauth.php @@ -13,7 +13,7 @@ * Main class. */ class OAuth { - const CSRF_TOKEN_TRANSIENT_NAME_BASE = '_newspack_google_oauth_csrf_'; + const CSRF_TOKEN_TRANSIENT_SCOPE_PREFIX = 'csrf_'; /** * Get API key for proxies. @@ -46,10 +46,9 @@ public static function get_unique_id() { * @return string CSRF token. */ public static function generate_csrf_token( $namespace ) { - $csrf_token = sha1( openssl_random_pseudo_bytes( 1024 ) ); - $transient_name = self::CSRF_TOKEN_TRANSIENT_NAME_BASE . $namespace . self::get_unique_id(); - set_transient( $transient_name, $csrf_token, 5 * MINUTE_IN_SECONDS ); - return $csrf_token; + $csrf_token = sha1( openssl_random_pseudo_bytes( 1024 ) ); + $transient_scope = self::CSRF_TOKEN_TRANSIENT_SCOPE_PREFIX . $namespace; + return OAuth_Transients::set( self::get_unique_id(), $transient_scope, $csrf_token ); } /** @@ -59,8 +58,10 @@ public static function generate_csrf_token( $namespace ) { * @return string CSRF token. */ public static function retrieve_csrf_token( $namespace ) { - $csrf_token_transient_name = self::CSRF_TOKEN_TRANSIENT_NAME_BASE . $namespace . self::get_unique_id(); - return get_transient( $csrf_token_transient_name ); + $transient_scope = self::CSRF_TOKEN_TRANSIENT_SCOPE_PREFIX . $namespace; + $value = OAuth_Transients::get( self::get_unique_id(), $transient_scope ); + OAuth_Transients::delete( self::get_unique_id(), $transient_scope ); + return $value; } /** From 92f3a60ca5f319664f6c908efc7f7223826ad3eb Mon Sep 17 00:00:00 2001 From: Adam Cassis Date: Sun, 12 May 2024 16:34:23 +0200 Subject: [PATCH 4/5] feat: cleanup old transients --- includes/oauth/class-oauth-transients.php | 46 ++++++++++++++--------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/includes/oauth/class-oauth-transients.php b/includes/oauth/class-oauth-transients.php index 2aa5eb78ca..eafffac465 100644 --- a/includes/oauth/class-oauth-transients.php +++ b/includes/oauth/class-oauth-transients.php @@ -13,34 +13,37 @@ * Main class. */ class OAuth_Transients { - /** - * Name of the custom table. To be prefixed with WPDB prefix. - */ const TABLE_NAME = 'newspack_oauth_transients'; - - /** - * Installed version number of the custom table. - */ const TABLE_VERSION = '1.0'; + const TABLE_VERSION_OPTION = '_newspack_oauth_transients_version'; + const CRON_HOOK = 'np_oauth_transients_cleanup'; /** - * Option name for the installed version number of the custom table. + * Initialize hooks. */ - const TABLE_VERSION_OPTION = '_newspack_oauth_transients_version'; + public static function init() { + register_activation_hook( NEWSPACK_PLUGIN_FILE, [ __CLASS__, 'create_custom_table' ] ); + add_action( 'init', [ __CLASS__, 'check_update_version' ] ); + add_action( 'init', [ __CLASS__, 'cron_init' ] ); + add_action( self::CRON_HOOK, [ __CLASS__, 'cleanup' ] ); + } /** - * How long to keep the data in the table. Any data older than this will be purged. - * - * @var int + * Schedule cron job to prune unused transients. If the OAuth process is interrupted, + * a transient might never be deleted. */ - private $expiration = 30 * MINUTE_IN_SECONDS; + public static function cron_init() { + \register_deactivation_hook( NEWSPACK_PLUGIN_FILE, [ __CLASS__, 'cron_deactivate' ] ); + if ( ! wp_next_scheduled( self::CRON_HOOK ) ) { + \wp_schedule_event( time(), 'weekly', self::CRON_HOOK ); + } + } /** - * Initialize hooks. + * Deactivate the cron job. */ - public static function init() { - register_activation_hook( NEWSPACK_PLUGIN_FILE, [ __CLASS__, 'create_custom_table' ] ); - add_action( 'init', [ __CLASS__, 'check_update_version' ] ); + public static function cron_deactivate() { + \wp_clear_scheduled_hook( self::CRON_HOOK ); } /** @@ -184,6 +187,15 @@ public static function set( $id, $scope, $value ) { return $value; } + + /** + * Cleanup old transients. + */ + public static function cleanup() { + global $wpdb; + $table_name = self::get_table_name(); + $wpdb->query( "DELETE FROM $table_name WHERE created_at < now() - interval 1 HOUR" ); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.PreparedSQL.InterpolatedNotPrepared + } } OAuth_Transients::init(); From 4239475cce456bdf6fde03f84603543567d08a8f Mon Sep 17 00:00:00 2001 From: dkoo Date: Mon, 13 May 2024 14:11:35 -0600 Subject: [PATCH 5/5] fix: clean up on the fly too, and limit to deleting 1000 at a time max --- includes/oauth/class-oauth-transients.php | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/includes/oauth/class-oauth-transients.php b/includes/oauth/class-oauth-transients.php index eafffac465..93cdec1a97 100644 --- a/includes/oauth/class-oauth-transients.php +++ b/includes/oauth/class-oauth-transients.php @@ -100,13 +100,14 @@ public static function create_custom_table() { /** * Get a value from the database. * - * @param string $id The reader's unique ID. - * @param string $scope The scope of the data to get. - * @param string $field_to_get The column to get. Defaults to 'value'. + * @param string $id The reader's unique ID. + * @param string $scope The scope of the data to get. + * @param string $field_to_get The column to get. Defaults to 'value'. + * @param boolean $cleanup If true, clean up old transients while getting. * * @return mixed The value of the data, or false if not found. */ - public static function get( $id, $scope, $field_to_get = 'value' ) { + public static function get( $id, $scope, $field_to_get = 'value', $cleanup = true ) { global $wpdb; $table_name = self::get_table_name(); @@ -120,6 +121,11 @@ public static function get( $id, $scope, $field_to_get = 'value' ) { ) ); + // Prune old transients. + if ( $cleanup ) { + self::cleanup(); + } + return $value ?? false; } @@ -171,7 +177,7 @@ public static function set( $id, $scope, $value ) { 'id' => $id, 'scope' => $scope, 'value' => $value, - 'created_at' => \current_time( 'mysql' ), + 'created_at' => \current_time( 'mysql', true ), // GMT time. ], [ '%s', @@ -189,12 +195,12 @@ public static function set( $id, $scope, $value ) { } /** - * Cleanup old transients. + * Cleanup old transients. Limit to max 1000 at a time, just in case. */ public static function cleanup() { global $wpdb; $table_name = self::get_table_name(); - $wpdb->query( "DELETE FROM $table_name WHERE created_at < now() - interval 1 HOUR" ); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.PreparedSQL.InterpolatedNotPrepared + $wpdb->query( "DELETE FROM $table_name WHERE created_at < now() - interval 30 MINUTE LIMIT 1000" ); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.PreparedSQL.InterpolatedNotPrepared } }