From c035cdaf3e6a07b5cf28cfa42fdc7da3318f29f5 Mon Sep 17 00:00:00 2001 From: Pedro Silva Date: Mon, 31 Jul 2023 18:20:39 +0100 Subject: [PATCH] Add previous token validation for OnboardingUrl Add retries on listen_for_merchant_id() to wait for client_id and signature --- modules/ppcp-onboarding/services.php | 6 +- .../src/Helper/OnboardingUrl.php | 100 ++++++++++++++++-- .../src/Render/OnboardingRenderer.php | 19 +++- modules/ppcp-wc-gateway/services.php | 4 +- .../src/Settings/SettingsListener.php | 69 +++++++++++- psalm-baseline.xml | 3 +- 6 files changed, 184 insertions(+), 17 deletions(-) diff --git a/modules/ppcp-onboarding/services.php b/modules/ppcp-onboarding/services.php index 61c231f94..06f2d27a7 100644 --- a/modules/ppcp-onboarding/services.php +++ b/modules/ppcp-onboarding/services.php @@ -229,13 +229,15 @@ return array( $partner_referrals_sandbox = $container->get( 'api.endpoint.partner-referrals-sandbox' ); $partner_referrals_data = $container->get( 'api.repository.partner-referrals-data' ); $settings = $container->get( 'wcgateway.settings' ); - $signup_link_cache = $container->get( 'onboarding.signup-link-cache' ); + $signup_link_cache = $container->get( 'onboarding.signup-link-cache' ); + $logger = $container->get( 'woocommerce.logger.woocommerce' ); return new OnboardingRenderer( $settings, $partner_referrals, $partner_referrals_sandbox, $partner_referrals_data, - $signup_link_cache + $signup_link_cache, + $logger ); }, 'onboarding.render-options' => static function ( ContainerInterface $container ) : OnboardingOptionsRenderer { diff --git a/modules/ppcp-onboarding/src/Helper/OnboardingUrl.php b/modules/ppcp-onboarding/src/Helper/OnboardingUrl.php index 3365882ad..881d4295f 100644 --- a/modules/ppcp-onboarding/src/Helper/OnboardingUrl.php +++ b/modules/ppcp-onboarding/src/Helper/OnboardingUrl.php @@ -66,6 +66,13 @@ class OnboardingUrl { */ private $cache_ttl = 3 * MONTH_IN_SECONDS; + /** + * The TTL for the previous token cache. + * + * @var int + */ + private $previous_cache_ttl = 60; + /** * The constructor * @@ -84,20 +91,19 @@ class OnboardingUrl { } /** - * Validates the token, if it's valid then delete it. - * If it's invalid don't delete it, to prevent malicious requests from invalidating the token. + * Instances the object with a $token. * * @param Cache $cache The cache object where the URL is stored. - * @param string $onboarding_token The token to validate. + * @param string $token The token to validate. * @param int $user_id User ID to associate the link with. - * @return bool + * @return false|self */ - public static function validate_token_and_delete( Cache $cache, string $onboarding_token, int $user_id ): bool { - if ( ! $onboarding_token ) { + public static function make_from_token( Cache $cache, string $token, int $user_id ) { + if ( ! $token ) { return false; } - $token_data = json_decode( UrlHelper::url_safe_base64_decode( $onboarding_token ) ?: '', true ); + $token_data = json_decode( UrlHelper::url_safe_base64_decode( $token ) ?: '', true ); if ( ! $token_data ) { return false; @@ -111,20 +117,57 @@ class OnboardingUrl { return false; } - $onboarding_url = new self( $cache, $token_data['k'], $token_data['u'] ); + return new self( $cache, $token_data['k'], $token_data['u'] ); + } + + /** + * Validates the token, if it's valid then delete it. + * If it's invalid don't delete it, to prevent malicious requests from invalidating the token. + * + * @param Cache $cache The cache object where the URL is stored. + * @param string $token The token to validate. + * @param int $user_id User ID to associate the link with. + * @return bool + */ + public static function validate_token_and_delete( Cache $cache, string $token, int $user_id ): bool { + $onboarding_url = self::make_from_token( $cache, $token, $user_id ); + + if ( $onboarding_url === false ) { + return false; + } if ( ! $onboarding_url->load() ) { return false; } - if ( ( $onboarding_url->token() ?: '' ) !== $onboarding_token ) { + if ( ( $onboarding_url->token() ?: '' ) !== $token ) { return false; } + $onboarding_url->replace_previous_token( $token ); $onboarding_url->delete(); return true; } + /** + * Validates the token against the previous token. + * Useful to don't throw errors on burst calls to endpoints. + * + * @param Cache $cache The cache object where the URL is stored. + * @param string $token The token to validate. + * @param int $user_id User ID to associate the link with. + * @return bool + */ + public static function validate_previous_token( Cache $cache, string $token, int $user_id ): bool { + $onboarding_url = self::make_from_token( $cache, $token, $user_id ); + + if ( $onboarding_url === false ) { + return false; + } + + return $onboarding_url->check_previous_token( $token ); + } + /** * Load cached data if is valid and initialize object. * @@ -315,4 +358,43 @@ class OnboardingUrl { return implode( '_', array( $this->cache_key_prefix, $this->user_id ) ); } + /** + * Returns the compiled cache key of the previous token + * + * @return string + */ + private function previous_cache_key(): string { + return $this->cache_key() . '_previous'; + } + + /** + * Checks it the previous token matches the token provided. + * + * @param string $previous_token The previous token. + * @return bool + */ + private function check_previous_token( string $previous_token ): bool { + if ( ! $this->cache->has( $this->previous_cache_key() ) ) { + return false; + } + + $cached_token = $this->cache->get( $this->previous_cache_key() ); + + return $cached_token === $previous_token; + } + + /** + * Replaces the previous token. + * + * @param string $previous_token The previous token. + * @return void + */ + private function replace_previous_token( string $previous_token ): void { + $this->cache->set( + $this->previous_cache_key(), + $previous_token, + $this->previous_cache_ttl + ); + } + } diff --git a/modules/ppcp-onboarding/src/Render/OnboardingRenderer.php b/modules/ppcp-onboarding/src/Render/OnboardingRenderer.php index 34215a2a0..081f1599b 100644 --- a/modules/ppcp-onboarding/src/Render/OnboardingRenderer.php +++ b/modules/ppcp-onboarding/src/Render/OnboardingRenderer.php @@ -9,12 +9,14 @@ declare(strict_types=1); namespace WooCommerce\PayPalCommerce\Onboarding\Render; +use Psr\Log\LoggerInterface; use WooCommerce\PayPalCommerce\ApiClient\Endpoint\PartnerReferrals; use WooCommerce\PayPalCommerce\ApiClient\Exception\RuntimeException; use WooCommerce\PayPalCommerce\ApiClient\Helper\Cache; use WooCommerce\PayPalCommerce\ApiClient\Repository\PartnerReferralsData; use WooCommerce\PayPalCommerce\Onboarding\Helper\OnboardingUrl; use WooCommerce\PayPalCommerce\WcGateway\Settings\Settings; +use WooCommerce\WooCommerce\Logging\Logger\NullLogger; /** * Class OnboardingRenderer @@ -56,6 +58,13 @@ class OnboardingRenderer { */ protected $cache; + /** + * The logger + * + * @var LoggerInterface + */ + private $logger; + /** * OnboardingRenderer constructor. * @@ -64,19 +73,22 @@ class OnboardingRenderer { * @param PartnerReferrals $sandbox_partner_referrals The PartnerReferrals for sandbox. * @param PartnerReferralsData $partner_referrals_data The default partner referrals data. * @param Cache $cache The cache. + * @param ?LoggerInterface $logger The logger. */ public function __construct( Settings $settings, PartnerReferrals $production_partner_referrals, PartnerReferrals $sandbox_partner_referrals, PartnerReferralsData $partner_referrals_data, - Cache $cache + Cache $cache, + LoggerInterface $logger = null ) { $this->settings = $settings; $this->production_partner_referrals = $production_partner_referrals; $this->sandbox_partner_referrals = $sandbox_partner_referrals; $this->partner_referrals_data = $partner_referrals_data; $this->cache = $cache; + $this->logger = $logger ?: new NullLogger(); } /** @@ -102,9 +114,12 @@ class OnboardingRenderer { $onboarding_url = new OnboardingUrl( $this->cache, $cache_key, get_current_user_id() ); if ( $onboarding_url->load() ) { + $this->logger->debug( 'Loaded onbording URL from cache: ' . $cache_key ); return $onboarding_url->get() ?: ''; } + $this->logger->info( 'Generating onboarding URL for: ' . $cache_key ); + $onboarding_url->init(); $data = $this->partner_referrals_data @@ -116,6 +131,8 @@ class OnboardingRenderer { $onboarding_url->set( $url ); $onboarding_url->persist(); + $this->logger->info( 'Persisted onboarding URL for: ' . $cache_key ); + return $url; } diff --git a/modules/ppcp-wc-gateway/services.php b/modules/ppcp-wc-gateway/services.php index 26d866066..6be410b8d 100644 --- a/modules/ppcp-wc-gateway/services.php +++ b/modules/ppcp-wc-gateway/services.php @@ -292,6 +292,7 @@ return array( $signup_link_ids = $container->get( 'onboarding.signup-link-ids' ); $pui_status_cache = $container->get( 'pui.status-cache' ); $dcc_status_cache = $container->get( 'dcc.status-cache' ); + $logger = $container->get( 'woocommerce.logger.woocommerce' ); return new SettingsListener( $settings, $fields, @@ -304,7 +305,8 @@ return array( $signup_link_ids, $pui_status_cache, $dcc_status_cache, - $container->get( 'http.redirector' ) + $container->get( 'http.redirector' ), + $logger ); }, 'wcgateway.order-processor' => static function ( ContainerInterface $container ): OrderProcessor { diff --git a/modules/ppcp-wc-gateway/src/Settings/SettingsListener.php b/modules/ppcp-wc-gateway/src/Settings/SettingsListener.php index 15110b650..60d07be4c 100644 --- a/modules/ppcp-wc-gateway/src/Settings/SettingsListener.php +++ b/modules/ppcp-wc-gateway/src/Settings/SettingsListener.php @@ -9,6 +9,7 @@ declare(strict_types=1); namespace WooCommerce\PayPalCommerce\WcGateway\Settings; +use Psr\Log\LoggerInterface; use WooCommerce\PayPalCommerce\AdminNotices\Entity\Message; use WooCommerce\PayPalCommerce\AdminNotices\Repository\Repository; use WooCommerce\PayPalCommerce\ApiClient\Authentication\Bearer; @@ -23,6 +24,7 @@ use WooCommerce\PayPalCommerce\WcGateway\Helper\DCCProductStatus; use WooCommerce\PayPalCommerce\WcGateway\Helper\PayUponInvoiceProductStatus; use WooCommerce\PayPalCommerce\Webhooks\WebhookRegistrar; use WooCommerce\PayPalCommerce\WcGateway\Exception\NotFoundException; +use WooCommerce\WooCommerce\Logging\Logger\NullLogger; /** * Class SettingsListener @@ -122,6 +124,27 @@ class SettingsListener { */ protected $redirector; + /** + * The logger. + * + * @var LoggerInterface + */ + private $logger; + + /** + * Max onboarding URL retries. + * + * @var int + */ + private $onboarding_max_retries = 3; + + /** + * Delay between onboarding URL retries. + * + * @var int + */ + private $onboarding_retry_delay = 2; + /** * SettingsListener constructor. * @@ -137,6 +160,7 @@ class SettingsListener { * @param Cache $pui_status_cache The PUI status cache. * @param Cache $dcc_status_cache The DCC status cache. * @param RedirectorInterface $redirector The HTTP redirector. + * @param ?LoggerInterface $logger The logger. */ public function __construct( Settings $settings, @@ -150,7 +174,8 @@ class SettingsListener { array $signup_link_ids, Cache $pui_status_cache, Cache $dcc_status_cache, - RedirectorInterface $redirector + RedirectorInterface $redirector, + LoggerInterface $logger = null ) { $this->settings = $settings; @@ -165,6 +190,7 @@ class SettingsListener { $this->pui_status_cache = $pui_status_cache; $this->dcc_status_cache = $dcc_status_cache; $this->redirector = $redirector; + $this->logger = $logger ?: new NullLogger(); } /** @@ -187,16 +213,48 @@ class SettingsListener { $merchant_id = sanitize_text_field( wp_unslash( $_GET['merchantIdInPayPal'] ) ); $merchant_email = sanitize_text_field( wp_unslash( $_GET['merchantId'] ) ); $onboarding_token = sanitize_text_field( wp_unslash( $_GET['ppcpToken'] ) ); + $retry_count = isset( $_GET['ppcpRetry'] ) ? ( (int) sanitize_text_field( wp_unslash( $_GET['ppcpRetry'] ) ) ) : 0; // phpcs:enable WordPress.Security.NonceVerification.Missing // phpcs:enable WordPress.Security.NonceVerification.Recommended $this->settings->set( 'merchant_id', $merchant_id ); $this->settings->set( 'merchant_email', $merchant_email ); - if ( ! OnboardingUrl::validate_token_and_delete( $this->signup_link_cache, $onboarding_token, get_current_user_id() ) ) { - $this->onboarding_redirect( false ); + // If no client_id is present we will try to wait for PayPal to invoke LoginSellerEndpoint. + if ( ! $this->settings->has( 'client_id' ) || ! $this->settings->get( 'client_id' ) ) { + + // Try at most {onboarding_max_retries} times ({onboarding_retry_delay} seconds delay). Then give up and just fill the merchant fields like before. + if ( $retry_count < $this->onboarding_max_retries ) { + + if ( $this->onboarding_retry_delay > 0 ) { + sleep( $this->onboarding_retry_delay ); + } + + $retry_count++; + $this->logger->info( 'Retrying onboarding return URL, retry nr: ' . ( (string) $retry_count ) ); + $redirect_url = add_query_arg( 'ppcpRetry', $retry_count ); + $this->redirector->redirect( $redirect_url ); + } } + // Process token validation. + $onboarding_token_sample = ( (string) substr( $onboarding_token, 0, 4 ) ) . '...' . ( (string) substr( $onboarding_token, -4 ) ); + $this->logger->debug( 'Validating onboarding ppcpToken: ' . $onboarding_token_sample ); + + if ( ! OnboardingUrl::validate_token_and_delete( $this->signup_link_cache, $onboarding_token, get_current_user_id() ) ) { + if ( OnboardingUrl::validate_previous_token( $this->signup_link_cache, $onboarding_token, get_current_user_id() ) ) { + // It's a valid token used previously, don't do anything but silently redirect. + $this->logger->info( 'Validated previous token, silently redirecting: ' . $onboarding_token_sample ); + $this->onboarding_redirect(); + } else { + $this->logger->error( 'Failed to validate onboarding ppcpToken: ' . $onboarding_token_sample ); + $this->onboarding_redirect( false ); + } + } + + $this->logger->info( 'Validated onboarding ppcpToken: ' . $onboarding_token_sample ); + + // Save the merchant data. $is_sandbox = $this->settings->has( 'sandbox_on' ) && $this->settings->get( 'sandbox_on' ); if ( $is_sandbox ) { $this->settings->set( 'merchant_id_sandbox', $merchant_id ); @@ -212,6 +270,7 @@ class SettingsListener { */ do_action( 'woocommerce_paypal_payments_onboarding_before_redirect' ); + // If after all the retry redirects there still isn't a valid client_id then just send an error. if ( ! $this->settings->has( 'client_id' ) || ! $this->settings->get( 'client_id' ) ) { $this->onboarding_redirect( false ); } @@ -230,6 +289,10 @@ class SettingsListener { if ( ! $success ) { $redirect_url = add_query_arg( 'ppcp-onboarding-error', '1', $redirect_url ); + $this->logger->info( 'Redirect ERROR: ' . $redirect_url ); + } else { + $redirect_url = remove_query_arg( 'ppcp-onboarding-error', $redirect_url ); + $this->logger->info( 'Redirect OK: ' . $redirect_url ); } $this->redirector->redirect( $redirect_url ); diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 42ce967ca..fb66e57af 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -685,10 +685,11 @@ listen_for_merchant_id listen_for_vaulting_enabled - + wp_unslash( $_GET['merchantId'] ) wp_unslash( $_GET['merchantIdInPayPal'] ) wp_unslash( $_GET['ppcpToken'] ) + wp_unslash( $_GET['ppcpRetry'] ) wp_unslash( $_POST['ppcp-nonce'] )