Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 31 additions & 7 deletions inc/gateways/class-base-stripe-gateway.php
Original file line number Diff line number Diff line change
Expand Up @@ -1544,17 +1544,28 @@
$stripe_customer = $this->get_stripe_client()->customers->retrieve($stripe_customer_id);

/*
* If the customer was deleted, we
* cannot use it again...
* If the customer was deleted, or the response object lacks a
* usable id, we cannot use it again. Fall through to creating
* a fresh Stripe customer below.
*/
if ( $stripe_customer && (! isset($stripe_customer->deleted) || ! $stripe_customer->deleted)) {
if ($stripe_customer && ! empty($stripe_customer->id) && (! isset($stripe_customer->deleted) || ! $stripe_customer->deleted)) {
$customer_exists = true;
}
} catch (\Exception $e) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedCatch

/**
* Silence is golden.
} catch (\Exception $e) {
/*
* The stored Stripe customer id could not be retrieved. Common
* causes are a test/live key mismatch, an account swap, or the
* customer being deleted in the Stripe dashboard. Log the cause
* for debugging and self-heal by creating a fresh customer.
*/
wu_log_add(
'stripe',
sprintf(
'Could not retrieve stored Stripe customer %s — falling back to creating a new one. Reason: %s',
$stripe_customer_id,
$e->getMessage()
)
);
}
}

Expand Down Expand Up @@ -1599,6 +1610,19 @@
}
}

/*
* Final defensive check — guarantees the caller receives either a usable
* \Stripe\Customer (with a non-empty id) or a \WP_Error. Without this,
* a malformed retrieve response could leak an object whose ->id is null
* and fatal in downstream typed parameters.
*/
if (empty($stripe_customer) || empty($stripe_customer->id)) {
return new \WP_Error(
'wu_stripe_no_customer',
__('Could not create or retrieve a Stripe customer record.', 'ultimate-multisite')
);
}

return $stripe_customer;
}

Expand Down Expand Up @@ -2891,7 +2915,7 @@
* Subscription payment received.
*/
} else {
$invoice_currency = strtoupper($invoice->currency ?? 'USD');

Check warning on line 2918 in inc/gateways/class-base-stripe-gateway.php

View workflow job for this annotation

GitHub Actions / Code Quality Checks

Equals sign not aligned with surrounding assignments; expected 19 spaces but found 1 space
$payment_data['total'] = $invoice->total / wu_stripe_get_currency_multiplier($invoice_currency);
$payment_data['subtotal'] = ($invoice->total_excluding_tax / wu_stripe_get_currency_multiplier($invoice_currency)) - $payment_data['discount_total'];
$payment_data['tax_total'] = $invoice->tax / wu_stripe_get_currency_multiplier($invoice_currency);
Expand Down Expand Up @@ -3268,7 +3292,7 @@
$expiration = '';
}

$new_status = 'trialing' === $stripe_status ? Membership_Status::TRIALING : Membership_Status::ACTIVE;

Check warning on line 3295 in inc/gateways/class-base-stripe-gateway.php

View workflow job for this annotation

GitHub Actions / Code Quality Checks

Line indented incorrectly; expected at least 7 tabs, found 6

/*
* Use reactivate() for expired/cancelled memberships so that
Expand Down
30 changes: 29 additions & 1 deletion inc/gateways/class-stripe-checkout-gateway.php
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,30 @@ public function run_preflight() {
$this->setup_api_keys();

/*
* Creates or retrieves the Stripe Customer
* Creates or retrieves the Stripe Customer.
*/
$s_customer = $this->get_or_create_customer($this->customer->get_id());

/*
* Bail early if customer creation/retrieval failed.
*
* get_or_create_customer() is documented to return \Stripe\Customer|\WP_Error,
* but historically the caller dereferenced ->id without checking — which
* fatals on the typed parameter of sync_billing_address_to_stripe() when
* the Stripe API call throws (mismatched test/live keys, deleted customer,
* network failure, etc.). Surface the error to the checkout flow instead.
*/
if (is_wp_error($s_customer)) {
return $s_customer;
}

if (empty($s_customer) || empty($s_customer->id)) {
return new \WP_Error(
'wu_stripe_checkout_no_customer',
__('We could not create or retrieve your Stripe customer record. Please try again, or contact support if the problem persists.', 'ultimate-multisite')
);
}

/*
* Update the Stripe customer with the current billing address.
* This ensures the address is pre-filled in Stripe Checkout.
Expand Down Expand Up @@ -552,6 +572,14 @@ public function get_user_saved_payment_methods() {
* @return void
*/
protected function sync_billing_address_to_stripe(string $stripe_customer_id): void {
/*
* Defensive guard: an empty customer ID would 400 from Stripe and is
* never a valid call. The primary caller (run_preflight) already
* blocks this case, but keep this here for any future caller.
*/
if (empty($stripe_customer_id)) {
return;
}

$billing_address = $this->customer->get_billing_address();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,181 @@ public function test_run_preflight_returns_session_url_and_id(): void {
$context['product']->delete();
}

/**
* Verify that run_preflight() returns a WP_Error (rather than fatalling on
* a null customer id) when get_or_create_customer() fails to retrieve or
* create a Stripe customer.
*
* Regression test for the support report:
* "Stripe Checkout session is returning null customer; please add
* customer_creation => always or guard null customer in
* sync_billing_address_to_stripe()"
*
* TypeError: Stripe_Checkout_Gateway::sync_billing_address_to_stripe():
* Argument #1 ($stripe_customer_id) must be of type string, null given
*
* The root cause is that get_or_create_customer() can return WP_Error (or
* an object whose id is null), but run_preflight() previously dereferenced
* ->id without checking.
*
* @return void
*/
public function test_run_preflight_returns_wp_error_when_customer_creation_fails(): void {
$context = $this->build_checkout_context(false, false);

// Build a Stripe client whose customers service throws on both retrieve
// and create — simulating, for example, a key/account mismatch.
$failing_client = $this->getMockBuilder(StripeClient::class)
->disableOriginalConstructor()
->getMock();

$failing_customers = $this->getMockBuilder(\Stripe\Service\CustomerService::class)
->disableOriginalConstructor()
->getMock();

$failing_customers->method('retrieve')->will(
$this->throwException(new \Stripe\Exception\InvalidRequestException('No such customer', 404))
);
$failing_customers->method('create')->will(
$this->throwException(new \Stripe\Exception\AuthenticationException('Invalid API key provided', 401))
);

$failing_client->method('__get')->willReturnCallback(
function ($property) use ($failing_customers) {
if ('customers' === $property) {
return $failing_customers;
}
return null;
}
);

$this->gateway->set_stripe_client($failing_client);

$result = $this->gateway->run_preflight();

// Must return a WP_Error — never fatal on a null customer id.
$this->assertInstanceOf(
\WP_Error::class,
$result,
'run_preflight() must return WP_Error when Stripe customer cannot be created'
);

// Cleanup
$context['payment']->delete();
$context['membership']->delete();
$context['product']->delete();
}

/**
* Verify that get_or_create_customer() self-heals when the stored Stripe
* customer id cannot be retrieved (e.g. test/live key mismatch, account
* swap, customer deleted in dashboard) by creating a fresh customer.
*
* @return void
*/
public function test_get_or_create_customer_self_heals_on_stale_stored_id(): void {
$customer = self::$customer;

// Pre-seed a stale gateway_customer_id on a membership for this customer.
$stale_membership = wu_create_membership(
[
'customer_id' => $customer->get_id(),
'plan_id' => 0,
'status' => Membership_Status::PENDING,
'recurring' => false,
'gateway' => 'stripe-checkout',
'gateway_customer_id' => 'cus_stale_does_not_exist',
'currency' => 'USD',
]
);

// Build a client whose retrieve throws (stale id) but create succeeds.
$client = $this->getMockBuilder(StripeClient::class)
->disableOriginalConstructor()
->getMock();

$customers_mock = $this->getMockBuilder(\Stripe\Service\CustomerService::class)
->disableOriginalConstructor()
->getMock();

$customers_mock->method('retrieve')->will(
$this->throwException(new \Stripe\Exception\InvalidRequestException('No such customer: cus_stale_does_not_exist', 404))
);

$fresh_customer = \Stripe\Customer::constructFrom(['id' => 'cus_fresh456']);
$customers_mock->method('create')->willReturn($fresh_customer);

$client->method('__get')->willReturnCallback(
function ($property) use ($customers_mock) {
if ('customers' === $property) {
return $customers_mock;
}
return null;
}
);

$this->gateway->set_stripe_client($client);
$this->gateway->set_customer($customer);

$result = $this->gateway->get_or_create_customer($customer->get_id());

$this->assertNotInstanceOf(
\WP_Error::class,
$result,
'get_or_create_customer() must self-heal when the stored Stripe id is stale'
);
$this->assertSame(
'cus_fresh456',
$result->id,
'A fresh Stripe customer id must be returned when the stored one is unretrievable'
);

$stale_membership->delete();
}

/**
* Verify that sync_billing_address_to_stripe() returns early without
* making any API call when given an empty customer id (defensive guard).
*
* @return void
*/
public function test_sync_billing_address_to_stripe_short_circuits_on_empty_id(): void {
$customer = self::$customer;
$this->gateway->set_customer($customer);

// A client whose customers->update would fail the test if called.
$client = $this->getMockBuilder(StripeClient::class)
->disableOriginalConstructor()
->getMock();

$customers_mock = $this->getMockBuilder(\Stripe\Service\CustomerService::class)
->disableOriginalConstructor()
->getMock();

$customers_mock->expects($this->never())->method('update');

$client->method('__get')->willReturnCallback(
function ($property) use ($customers_mock) {
if ('customers' === $property) {
return $customers_mock;
}
return null;
}
);

$this->gateway->set_stripe_client($client);

// Reflect to call the protected method directly.
$reflection = new \ReflectionClass($this->gateway);
$method = $reflection->getMethod('sync_billing_address_to_stripe');
$method->setAccessible(true);

$method->invoke($this->gateway, '');

// If the early return works, customers->update was never called.
$this->addToAssertionCount(1);
}

/**
* Tear down after all tests.
*
Expand Down
Loading