From edcc5eee3ad9adfa73bba3582afda6c71380169a Mon Sep 17 00:00:00 2001 From: Jason Adams Date: Tue, 22 Aug 2023 10:00:16 -0700 Subject: [PATCH 1/4] Security: remove uses of payment intent secret and existing meta (#6836) --- .../stripe/includes/give-stripe-helpers.php | 41 ++----------- .../class-give-stripe-sepa.php | 22 +++---- .../Stripe/Actions/CreatePaymentIntent.php | 12 +--- .../RemovePaymentIntentSecretMeta.php | 58 +++++++++++++++++++ src/PaymentGateways/ServiceProvider.php | 5 +- 5 files changed, 76 insertions(+), 62 deletions(-) create mode 100644 src/PaymentGateways/Gateways/Stripe/Migrations/RemovePaymentIntentSecretMeta.php diff --git a/includes/gateways/stripe/includes/give-stripe-helpers.php b/includes/gateways/stripe/includes/give-stripe-helpers.php index 3009b57ca7..b9c2022763 100644 --- a/includes/gateways/stripe/includes/give-stripe-helpers.php +++ b/includes/gateways/stripe/includes/give-stripe-helpers.php @@ -630,36 +630,6 @@ function give_stripe_get_application_fee_amount( $amount ) { return round( $amount * give_stripe_get_application_fee_percentage() / 100, 0 ); } -/** - * This function is used to fetch the donation id by meta key. - * - * @param string $id Any String. - * @param string $type intent_id/client_secret - * - * @since 2.5.0 - * - * @return void - */ -function give_stripe_get_donation_id_by( $id, $type ) { - - global $wpdb; - - $donation_id = 0; - - switch ( $type ) { - case 'intent_id': - $donation_id = $wpdb->get_var( $wpdb->prepare( "SELECT donation_id FROM {$wpdb->donationmeta} WHERE meta_key = '_give_stripe_payment_intent_id' AND meta_value = %s LIMIT 1", $id ) ); - break; - - case 'client_secret': - $donation_id = $wpdb->get_var( $wpdb->prepare( "SELECT donation_id FROM {$wpdb->donationmeta} WHERE meta_key = '_give_stripe_payment_intent_client_secret' AND meta_value = %s LIMIT 1", $id ) ); - break; - } - - return $donation_id; - -} - /** * This function is used to set Stripe API Key. * @@ -874,11 +844,12 @@ function give_stripe_is_source_type( $id, $type = 'src' ) { /** * This helper function is used to process Stripe payments. * - * @param array $donation_data Donation form data. - * @param object $stripe_gateway $this data. - * + * @unreleased no longer store the payment intent secret * @since 2.5.0 * + * @param array $donation_data Donation form data. + * @param object $stripe_gateway $this data. + * * @return void */ function give_stripe_process_payment( $donation_data, $stripe_gateway ) { @@ -978,10 +949,6 @@ function give_stripe_process_payment( $donation_data, $stripe_gateway ) { $intent = $stripe_gateway->payment_intent->create( $intent_args ); - // Save Payment Intent Client Secret to donation note and DB. - give_insert_payment_note( $donation_id, 'Stripe Payment Intent Client Secret: ' . $intent->client_secret ); - give_update_meta( $donation_id, '_give_stripe_payment_intent_client_secret', $intent->client_secret ); - // Set Payment Intent ID as transaction ID for the donation. give_set_payment_transaction_id( $donation_id, $intent->id ); give_insert_payment_note( $donation_id, 'Stripe Charge/Payment Intent ID: ' . $intent->id ); diff --git a/includes/gateways/stripe/includes/payment-methods/class-give-stripe-sepa.php b/includes/gateways/stripe/includes/payment-methods/class-give-stripe-sepa.php index ebbf873a69..ff03baf986 100644 --- a/includes/gateways/stripe/includes/payment-methods/class-give-stripe-sepa.php +++ b/includes/gateways/stripe/includes/payment-methods/class-give-stripe-sepa.php @@ -155,14 +155,15 @@ class="give-stripe-sepa-iban-field give-stripe-cc-field" } /** - * This function will be used for donation processing. - * - * @param array $donation_data List of donation data. - * - * @return void - * @since 2.6.1 - * @access public - */ + * This function will be used for donation processing. + * + * @unreleased no longer store the intent secret in the database + * @since 2.6.1 + * + * @param array $donation_data List of donation data. + * + * @return void + */ public function process_payment( $donation_data ) { // Bailout, if the current gateway and the posted gateway mismatched. @@ -287,11 +288,6 @@ public function process_payment( $donation_data ) { $intent = $this->payment_intent->create( $intent_args ); if ( ! empty( $intent->status ) && 'processing' === $intent->status ) { - - // Save Payment Intent Client Secret to donation note and DB. - give_insert_payment_note( $donation_id, 'Stripe Payment Intent Client Secret: ' . $intent->client_secret ); - give_update_meta( $donation_id, '_give_stripe_payment_intent_client_secret', $intent->client_secret ); - // Set Payment Intent ID as transaction ID for the donation. give_set_payment_transaction_id( $donation_id, $intent->id ); give_insert_payment_note( $donation_id, 'Stripe Charge/Payment Intent ID: ' . $intent->id ); diff --git a/src/PaymentGateways/Gateways/Stripe/Actions/CreatePaymentIntent.php b/src/PaymentGateways/Gateways/Stripe/Actions/CreatePaymentIntent.php index 145f8ffbe6..d2615fcdb6 100644 --- a/src/PaymentGateways/Gateways/Stripe/Actions/CreatePaymentIntent.php +++ b/src/PaymentGateways/Gateways/Stripe/Actions/CreatePaymentIntent.php @@ -27,6 +27,7 @@ public function __construct(array $paymentIntentArgs = []) } /** + * @unreleased no longer store the payment intent secret * @since 2.19.0 * * @throws InvalidPropertyName @@ -71,17 +72,6 @@ public function __invoke( 'content' => sprintf(__('Stripe Charge/Payment Intent ID: %s', 'give'), $intent->id()) ]); - DonationNote::create([ - 'donationId' => $donation->id, - 'content' => sprintf(__('Stripe Payment Intent Client Secret: %s', 'give'), $intent->clientSecret()) - ]); - - give_update_meta( - $donation->id, - '_give_stripe_payment_intent_client_secret', - $intent->clientSecret() - ); - if ('requires_action' === $intent->status()) { DonationNote::create([ 'donationId' => $donation->id, diff --git a/src/PaymentGateways/Gateways/Stripe/Migrations/RemovePaymentIntentSecretMeta.php b/src/PaymentGateways/Gateways/Stripe/Migrations/RemovePaymentIntentSecretMeta.php new file mode 100644 index 0000000000..554d154760 --- /dev/null +++ b/src/PaymentGateways/Gateways/Stripe/Migrations/RemovePaymentIntentSecretMeta.php @@ -0,0 +1,58 @@ + '_give_stripe_payment_intent_client_secret'], + ['%s'] + ); + + $commentsTable = DB::prefix('give_comments'); + DB::query( + DB::prepare( + "DELETE FROM {$commentsTable} WHERE comment_type = 'donation' AND comment_content LIKE %s", + 'Stripe Payment Intent Client Secret:%' + ) + ); + } +} diff --git a/src/PaymentGateways/ServiceProvider.php b/src/PaymentGateways/ServiceProvider.php index 60aa969714..a53b22ed1c 100644 --- a/src/PaymentGateways/ServiceProvider.php +++ b/src/PaymentGateways/ServiceProvider.php @@ -14,6 +14,7 @@ use Give\PaymentGateways\Gateways\Stripe\Controllers\UpdateStatementDescriptorAjaxRequestController; use Give\PaymentGateways\Gateways\Stripe\Migrations\AddMissingTransactionIdForUncompletedDonations; use Give\PaymentGateways\Gateways\Stripe\Migrations\AddStatementDescriptorToStripeAccounts; +use Give\PaymentGateways\Gateways\Stripe\Migrations\RemovePaymentIntentSecretMeta; use Give\PaymentGateways\PayPalCommerce\Migrations\RegisterPayPalDonationsRefreshTokenCronJobByMode; use Give\PaymentGateways\PayPalCommerce\Migrations\RemoveLogWithCardInfo; use Give\ServiceProviders\ServiceProvider as ServiceProviderInterface; @@ -68,6 +69,7 @@ public function boot() } /** + * @unreleased add RemovePaymentIntentSecretMeta migration * @since 2.19.6 */ private function registerMigrations() @@ -76,7 +78,8 @@ private function registerMigrations() AddStatementDescriptorToStripeAccounts::class, AddMissingTransactionIdForUncompletedDonations::class, RemoveLogWithCardInfo::class, - RegisterPayPalDonationsRefreshTokenCronJobByMode::class + RemovePaymentIntentSecretMeta::class, + RegisterPayPalDonationsRefreshTokenCronJobByMode::class, ]); } } From 7cd4212a670ce874445b40c1b99f4f8081dd6bb2 Mon Sep 17 00:00:00 2001 From: Jason Adams Date: Wed, 23 Aug 2023 07:41:45 -0700 Subject: [PATCH 2/4] fix: remove extra spacing on classic themes (#6895) Co-authored-by: Joshua Dinh <75056371+JoshuaHungDinh@users.noreply.github.com> --- src/FormBuilder/resources/css/admin-form-builder.css | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/FormBuilder/resources/css/admin-form-builder.css b/src/FormBuilder/resources/css/admin-form-builder.css index 1d96761cca..54f1095331 100644 --- a/src/FormBuilder/resources/css/admin-form-builder.css +++ b/src/FormBuilder/resources/css/admin-form-builder.css @@ -2,4 +2,10 @@ #adminmenumain, #wpadminbar { display: none; -} \ No newline at end of file +} + +/* Prevent classic admin theme layout issues */ +html :where(.wp-block) { + margin: 0; + max-width: unset; +} From c617c1dc97096535a27c1fdf5b03462a406c2463 Mon Sep 17 00:00:00 2001 From: "Kyle B. Johnson" Date: Wed, 23 Aug 2023 11:46:05 -0400 Subject: [PATCH 3/4] Feature: Auto-deactivate the feature plugin (#6896) * Deactivate the Next Gen feature plugin --------- Co-authored-by: Ben Meredith Co-authored-by: Angela Blake --- give.php | 24 ++++++++++++++++++++++ src/FormMigration/functions.php | 6 +++--- tests/Unit/FormMigration/FunctionsTest.php | 12 +++++------ 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/give.php b/give.php index 4180202bd8..c805b7fe8a 100644 --- a/give.php +++ b/give.php @@ -74,6 +74,7 @@ use Give\Log\LogServiceProvider; use Give\MigrationLog\MigrationLogServiceProvider; use Give\MultiFormGoals\ServiceProvider as MultiFormGoalsServiceProvider; +use Give\PaymentGateways\Gateways\TestOffsiteGateway\TestOffsiteGateway; use Give\PaymentGateways\ServiceProvider as PaymentGatewaysServiceProvider; use Give\Promotions\ServiceProvider as PromotionsServiceProvider; use Give\Revenue\RevenueServiceProvider; @@ -257,6 +258,8 @@ public function boot() { $this->setup_constants(); + $this->disableVisualDonationFormBuilderFeaturePlugin(); + // Add compatibility notice for recurring and stripe support with Give 2.5.0. add_action('admin_notices', [$this, 'display_old_recurring_compatibility_notice']); @@ -515,6 +518,27 @@ private function setupExceptionHandler() $handler = new UncaughtExceptionLogger(); $handler->setupExceptionHandler(); } + + protected function disableVisualDonationFormBuilderFeaturePlugin() + { + // Include plugin.php to use is_plugin_active() below. + include_once ABSPATH . 'wp-admin/includes/plugin.php'; + + // Prevent fatal error due to a renamed class. + class_alias(TestOffsiteGateway::class, 'Give\PaymentGateways\Gateways\TestGateway\TestGatewayOffsite', true); + + if ( is_plugin_active('givewp-next-gen/give-visual-form-builder.php' )) { + deactivate_plugins(['givewp-next-gen/give-visual-form-builder.php']); + + add_action('admin_notices', function() { + Give()->notices->register_notice([ + 'id' => 'give-visual-donation-form-builder-feature-plugin-deactivated', + 'description' => __('The Visual Form Builder Beta plugin is no longer needed, since the form builder is included in your current version of GiveWP. To prevent conflicts, the Beta plugin has been deactivated and can be safely deleted.', 'give'), + 'type' => 'info', + ]); + }); + } + } } /** diff --git a/src/FormMigration/functions.php b/src/FormMigration/functions.php index 27141e56bc..b4e04793d0 100644 --- a/src/FormMigration/functions.php +++ b/src/FormMigration/functions.php @@ -16,7 +16,7 @@ * * @return void Note: $formId is an "output argument" - not a return value. */ -function give_redirect_form_id(&$formId, &...$extraReference) { +function _give_redirect_form_id(&$formId, &...$extraReference) { global $wpdb; $formId = absint(DB::get_var( @@ -40,7 +40,7 @@ function give_redirect_form_id(&$formId, &...$extraReference) { * * @return bool */ -function give_is_form_migrated($formId) { +function _give_is_form_migrated($formId) { global $wpdb; return (bool) DB::get_var( @@ -63,7 +63,7 @@ function give_is_form_migrated($formId) { * * @return bool */ -function give_is_form_donations_transferred($formId) { +function _give_is_form_donations_transferred($formId) { global $wpdb; return (bool) DB::get_var( diff --git a/tests/Unit/FormMigration/FunctionsTest.php b/tests/Unit/FormMigration/FunctionsTest.php index ffe043a5d8..c7bd3c2fe6 100644 --- a/tests/Unit/FormMigration/FunctionsTest.php +++ b/tests/Unit/FormMigration/FunctionsTest.php @@ -20,7 +20,7 @@ public function testIsFormRedirected() $formId = $donationFormV2->id; - give_redirect_form_id($formId); + _give_redirect_form_id($formId); $this->assertEquals($donationFormV3->id, $formId); } @@ -34,7 +34,7 @@ public function testIsFormRedirectedWithAdditionalReference() $formId = $donationFormV2->id; $atts['id'] = $donationFormV2->id; - give_redirect_form_id($formId, $atts['id']); + _give_redirect_form_id($formId, $atts['id']); $this->assertEquals($donationFormV3->id, $formId); $this->assertEquals($donationFormV3->id, $atts['id']); @@ -46,14 +46,14 @@ public function testIsFormMigrated() $donationFormV3 = DonationForm::factory()->create(); give_update_meta($donationFormV3->id, 'migratedFormId', $donationFormV2->id); - $this->assertTrue(give_is_form_migrated($donationFormV2->id)); + $this->assertTrue(_give_is_form_migrated($donationFormV2->id)); } public function testIsFormNotMigrated() { $donationFormV2 = $this->createSimpleDonationForm(); - $this->assertFalse(give_is_form_migrated($donationFormV2->id)); + $this->assertFalse(_give_is_form_migrated($donationFormV2->id)); } public function testIsFormDonationsTransferred() @@ -62,13 +62,13 @@ public function testIsFormDonationsTransferred() $donationFormV3 = DonationForm::factory()->create(); give_update_meta($donationFormV3->id, 'transferredFormId', $donationFormV2->id); - $this->assertTrue(give_is_form_donations_transferred($donationFormV2->id)); + $this->assertTrue(_give_is_form_donations_transferred($donationFormV2->id)); } public function testIsFormDonationsNotTransferred() { $donationFormV2 = $this->createSimpleDonationForm(); - $this->assertFalse(give_is_form_donations_transferred($donationFormV2->id)); + $this->assertFalse(_give_is_form_donations_transferred($donationFormV2->id)); } } From f5138cbc6bcbed67d20d0474cadb077e097f5b15 Mon Sep 17 00:00:00 2001 From: Jon Waldstein Date: Wed, 23 Aug 2023 12:06:25 -0400 Subject: [PATCH 4/4] fix: subscriptionPeriod is already being exported --- .../templates/groups/DonationAmount/subscriptionPeriod.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/DonationForms/resources/registrars/templates/groups/DonationAmount/subscriptionPeriod.ts b/src/DonationForms/resources/registrars/templates/groups/DonationAmount/subscriptionPeriod.ts index 858ee9e996..bb095870dd 100644 --- a/src/DonationForms/resources/registrars/templates/groups/DonationAmount/subscriptionPeriod.ts +++ b/src/DonationForms/resources/registrars/templates/groups/DonationAmount/subscriptionPeriod.ts @@ -1,4 +1,4 @@ -import {__} from '@wordpress/i18n'; +import { __ } from "@wordpress/i18n"; /** * @since 3.0.0 @@ -50,7 +50,7 @@ const subscriptionPeriodLabelLookup = { /** * @since 3.0.0 */ -export type subscriptionPeriod = keyof typeof subscriptionPeriodLabelLookup; +type subscriptionPeriod = keyof typeof subscriptionPeriodLabelLookup; /** * @since 3.0.0