Conversation
Replace direct OAuth implementation with secure proxy server architecture to keep platform credentials safe and never expose them in distributed code. ## Security Improvements: ✅ **No credentials in distributed code** - Platform Client ID and Secret Keys never leave the proxy server at ultimatemultisite.com ✅ **Encrypted communication** - OAuth codes encrypted during transmission ✅ **Centralized control** - Rotate credentials without updating plugin ✅ **Usage tracking** - Proxy database tracks all connected sites ## Changes to Base Stripe Gateway (inc/gateways/class-base-stripe-gateway.php): ### New Methods: - `get_proxy_url()` - Returns proxy server URL (filterable) - `get_business_data()` - Provides site info for Stripe form prefill ### Updated Methods: - `get_connect_authorization_url()` - Now calls proxy /oauth/init endpoint instead of directly constructing Stripe OAuth URL - `handle_oauth_callbacks()` - Handles encrypted codes from proxy (looks for wcs_stripe_code and wcs_stripe_state parameters) - `exchange_code_for_keys()` - Calls proxy /oauth/keys endpoint to exchange encrypted code for API keys (replaces process_oauth_callback) - `handle_disconnect()` - Notifies proxy server when disconnecting ### Removed Methods: - `get_platform_client_id()` - No longer needed (credentials on proxy) - `get_platform_secret_key()` - No longer needed (credentials on proxy) ### Security Fixes: - Sanitize all $_GET input variables (wcs_stripe_code, _wpnonce) ## Changes to Stripe Gateway (inc/gateways/class-stripe-gateway.php): - Remove client ID check (proxy handles this now) - Auto-fixed code style issues ## Test Updates: - Updated `test_oauth_authorization_url_generation()` to mock proxy response - Updated `test_oauth_authorization_url_requires_client_id()` to test proxy failure - All tests passing: 19 tests, 53 assertions ✅ ## Architecture Flow: 1. User clicks "Connect with Stripe" 2. Plugin calls https://ultimatemultisite.com/wp-json/stripe-connect/v1/oauth/init 3. Proxy returns OAuth URL with encrypted state 4. User authorizes on Stripe 5. Stripe redirects to proxy /oauth/redirect endpoint 6. Proxy encrypts code and redirects back to site 7. Plugin calls proxy /oauth/keys to exchange encrypted code 8. Proxy decrypts code, exchanges with Stripe, returns tokens 9. Plugin saves tokens locally ## Proxy Server Repository: https://github.com/superdav42/stripe-connect-proxy ## Deployment Notes: The proxy server plugin must be deployed to ultimatemultisite.com and configured with platform credentials before OAuth connections will work. See INTEGRATION.md in the proxy repository for deployment instructions. ## Backwards Compatibility: Fully backwards compatible with existing direct API key mode. Sites that have already connected via OAuth will continue to work. New OAuth connections will automatically use the proxy server. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
📝 WalkthroughWalkthroughReplaces Stripe Card Element with Payment Element, adds Stripe Connect (OAuth) scaffolding and settings/UI, implements payment status polling and scheduled verification, syncs billing address for Stripe Checkout, updates frontend Stripe JS lifecycle and styling, and adds PHPUnit and E2E Cypress tests for OAuth and Stripe flows. Changes
Sequence Diagram(s)sequenceDiagram
%% Payment Element flow (high level)
participant User
participant Browser
participant StripeJS as Stripe.js
participant Server
rect rgba(230,245,255,0.5)
User->>Browser: Submit checkout form
Browser->>StripeJS: Ensure Payment Element initialized / mount
Browser->>StripeJS: elements.submit() (local validation)
alt client_secret present & valid
Browser->>StripeJS: confirmPayment / confirmSetup (billing details)
StripeJS->>StripeJS: network call to Stripe
StripeJS-->>Browser: success (intent id)
Browser->>Server: Resubmit form with intent id
Server-->>Browser: final checkout response
else validation/error
StripeJS-->>Browser: error -> show in `payment-errors`
Note right of Browser: Prevent form submission
end
end
sequenceDiagram
%% OAuth connect flow (high level)
participant Admin
participant Browser
participant Server
participant Proxy
participant StripeAPI
rect rgba(240,255,230,0.5)
Admin->>Browser: Click "Connect with Stripe"
Browser->>Server: Request connect URL (get_connect_authorization_url)
Server->>Proxy: Request OAuth init / encrypted URL
Proxy-->>Server: Returns auth URL / state
Server-->>Browser: Redirect to Stripe OAuth URL
Browser->>StripeAPI: User completes OAuth on Stripe
StripeAPI-->>Browser: Redirect back with code -> Server callback
Browser->>Server: handle_oauth_callbacks (receives code)
Server->>Proxy: Exchange encrypted code for tokens
Proxy-->>Server: Returns access_token, refresh_token, account_id
Server->>Server: Persist tokens, set authentication_mode = 'oauth'
Server-->>Admin: Show connected state with `oauth_account_id`
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
assets/js/gateways/stripe.js (1)
66-88: Payment Element initialization is correct, but style update is ineffective.The Payment Element is correctly created with the
clientSecretand mounted. However, the call towu_stripe_update_payment_element_stylesat line 83 will not work as noted above. Consider integrating the dynamic styles into the initialappearanceobject at lines 73-75.inc/gateways/class-base-stripe-gateway.php (1)
467-482: Consider using admin notices instead of wp_die for OAuth errors.Using
wp_die()provides a poor user experience when OAuth fails. Consider redirecting back to settings with an error notice instead.if (is_wp_error($response)) { - wp_die(esc_html__('Failed to connect to proxy server', 'ultimate-multisite')); + $redirect_url = add_query_arg( + [ + 'page' => 'wu-settings', + 'tab' => 'payment-gateways', + 'stripe_oauth_error' => 'proxy_connection_failed', + ], + admin_url('admin.php') + ); + wp_safe_redirect($redirect_url); + exit; }tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php (1)
61-85: Consider cleaning up filters after tests.Filters added in tests persist and may affect subsequent tests. Consider adding a
tearDownmethod to remove filters or storing the filter callback to remove it after assertions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
assets/js/gateways/stripe.js(4 hunks)inc/gateways/class-base-stripe-gateway.php(6 hunks)inc/gateways/class-stripe-gateway.php(8 hunks)tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php(1 hunks)tests/WP_Ultimo/Gateways/Stripe_OAuth_Test.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
assets/js/gateways/stripe.js (1)
assets/js/checkout.js (1)
promises(864-864)
tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php (2)
inc/functions/settings.php (1)
wu_save_setting(51-54)inc/gateways/class-base-stripe-gateway.php (3)
init(194-211)is_using_oauth(292-294)get_authentication_mode(282-284)
tests/WP_Ultimo/Gateways/Stripe_OAuth_Test.php (2)
inc/functions/settings.php (2)
wu_save_setting(51-54)wu_get_setting(37-40)inc/gateways/class-base-stripe-gateway.php (4)
init(194-211)get_authentication_mode(282-284)is_using_oauth(292-294)get_connect_authorization_url(356-392)
inc/gateways/class-stripe-gateway.php (2)
inc/functions/settings.php (1)
wu_register_settings_field(88-91)inc/gateways/class-base-stripe-gateway.php (3)
is_using_oauth(292-294)get_disconnect_url(400-410)get_connect_authorization_url(356-392)
🪛 Biome (2.1.2)
assets/js/gateways/stripe.js
[error] 27-45: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
[error] 302-302: Invalid typeof comparison.
Compare with one of the following string literals:
(lint/correctness/useValidTypeof)
🪛 GitHub Check: Code Quality Checks
tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php
[warning] 168-168:
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space
[failure] 146-146:
Only one argument is allowed per line in a multi-line function call
[failure] 146-146:
Opening parenthesis of a multi-line function call must be the last content on the line
[failure] 145-145:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 143-143:
Expected 1 space after FUNCTION keyword; 0 found
[failure] 143-143:
Only one argument is allowed per line in a multi-line function call
[failure] 143-143:
Opening parenthesis of a multi-line function call must be the last content on the line
[warning] 128-128:
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space
[warning] 105-105:
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space
[warning] 81-81:
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space
[failure] 65-65:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 63-63:
Expected 1 space after FUNCTION keyword; 0 found
[failure] 63-63:
Only one argument is allowed per line in a multi-line function call
[failure] 63-63:
Opening parenthesis of a multi-line function call must be the last content on the line
[warning] 53-53:
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space
tests/WP_Ultimo/Gateways/Stripe_OAuth_Test.php
[warning] 157-157:
Array double arrow not aligned correctly; expected 4 space(s) between "'state'" and double arrow, but found 1.
[warning] 155-155:
Array double arrow not aligned correctly; expected 5 space(s) between "'body'" and double arrow, but found 1.
[warning] 140-140:
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space
[warning] 118-118:
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space
🪛 Gitleaks (8.30.0)
tests/WP_Ultimo/Gateways/Stripe_OAuth_Test.php
[high] 66-66: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 85-85: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 128-128: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 198-198: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 243-243: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 303-303: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 PHPMD (2.15.0)
inc/gateways/class-base-stripe-gateway.php
356-356: Avoid unused parameters such as '$state'. (undefined)
(UnusedFormalParameter)
tests/WP_Ultimo/Gateways/Stripe_OAuth_Test.php
151-151: Avoid unused parameters such as '$args'. (undefined)
(UnusedFormalParameter)
180-180: Avoid unused parameters such as '$args'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: PHP 8.3
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: Build Plugin for Testing
- GitHub Check: cypress (8.1, chrome)
🔇 Additional comments (17)
assets/js/gateways/stripe.js (1)
374-413: LGTM - Payment Element confirmation flow.The
handlePaymentElementfunction correctly usesconfirmPaymentwithredirect: 'if_required'and properly handles both success (resubmit) and error (display) cases.inc/gateways/class-stripe-gateway.php (4)
48-49: LGTM - OAuth callback hook registration.The
admin_inithook properly delegates to the parent class'shandle_oauth_callbacksmethod for OAuth flow processing.
893-910: State regeneration on every render.A new OAuth state is generated each time this method is called (e.g., on page refresh). While this is secure, it means that if a user opens the settings page in multiple tabs, only the last tab's OAuth flow will work. This is generally acceptable behavior for OAuth CSRF protection.
745-753: LGTM - Dual element containers for Payment Element migration.The addition of
#payment-elementalongside the existing#card-elementenables gradual migration to the modern Payment Element while maintaining backwards compatibility.
134-150: LGTM - Direct API keys toggle.The new
stripe_show_direct_keystoggle allows advanced users to enter manual API keys while keeping OAuth as the recommended default.inc/gateways/class-base-stripe-gateway.php (7)
84-131: LGTM - OAuth state properties.The new protected properties properly encapsulate OAuth-related state including tokens, account ID, and application fee. Default values are appropriately set.
148-163: LGTM - Stripe Connect account header configuration.The
stripe_accountheader is correctly set when using OAuth mode, enabling API calls on behalf of the connected account.
223-274: LGTM - OAuth precedence in key setup.The
setup_api_keysmethod correctly prioritizes OAuth tokens over direct API keys, with proper fallback behavior for backwards compatibility.
418-438: LGTM - OAuth callback security.The handler properly verifies the CSRF state before processing OAuth callbacks and uses nonce verification for disconnect requests. All
$_GETinputs are properly sanitized.
349-392: Unused$stateparameter is intentional.The static analysis correctly identifies that
$stateis unused. The docblock at line 353 correctly documents this as kept for compatibility since the proxy server now generates and manages the state. No change needed.
1493-1498: LGTM - Application fee integration.The application fee is correctly applied only when using OAuth mode with a non-zero fee percentage, using Stripe's
application_fee_percentparameter.
522-565: LGTM - Disconnect handling clears all tokens.The disconnect handler correctly clears OAuth tokens for both test and live modes, ensuring complete cleanup regardless of the current sandbox mode setting. The non-blocking proxy notification is appropriate.
tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php (1)
138-189: LGTM - Comprehensive E2E OAuth flow test.The test thoroughly validates the complete OAuth lifecycle including initial setup, token storage, application fee loading, and fallback to direct keys when OAuth is disconnected.
tests/WP_Ultimo/Gateways/Stripe_OAuth_Test.php (4)
151-162: Unused$argsparameter is expected for WordPress filter signature.The
pre_http_requestfilter requires the signature($preempt, $args, $url). The unused$argsparameter is intentional and cannot be removed. Consider adding a PHPDoc annotation to suppress the warning.- add_filter('pre_http_request', function($preempt, $args, $url) { + // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- Required by filter signature + add_filter('pre_http_request', function($preempt, $args, $url) {
64-75: Gitleaks false positives for test API keys.The flagged API keys (
pk_test_direct_123,sk_test_direct_123, etc.) are intentionally fake test fixtures, not real credentials. These follow Stripe's test key naming convention and pose no security risk.
17-42: LGTM - Well-structured test setup.The test class properly initializes a clean state before each test by clearing all Stripe settings. The use of
wu_save_settingwith empty values ensures test isolation.
334-347: LGTM - Disconnect URL security test.This test properly verifies that the disconnect URL includes the required nonce for CSRF protection and all necessary query parameters.
superdav42
changed the title
Performance Improvements: - Implement lazy loading for OAuth initialization - Move proxy HTTP request from page load to button click - Add get_oauth_init_url() method for deferred OAuth flow - Update handle_oauth_callbacks() to process init requests Application Fee Removal: - Remove application_fee_percentage property and methods - Remove fee application from subscription creation - Clean up OAuth tests to remove fee-related assertions - Remove 4 application fee tests from test suite Technical Details: - OAuth URL generation now happens only when user clicks "Connect with Stripe" - State generation moved to init handler for better security - All 15 OAuth tests passing (44 assertions) This eliminates unnecessary HTTP requests on every settings page load and simplifies the codebase by removing unused application fee logic. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php (1)
90-92: Fix closure formatting to pass CI checks.The multi-line closure violates project coding standards. Apply proper formatting.
- add_filter('wu_stripe_platform_client_id', function() { - return 'ca_platform_test_123'; - }); + add_filter( + 'wu_stripe_platform_client_id', + function () { + return 'ca_platform_test_123'; + } + );
🧹 Nitpick comments (2)
inc/gateways/class-base-stripe-gateway.php (2)
318-361: Remove unused$stateparameter.The
$stateparameter is never used in the method body since state is now managed by the proxy server. Consider removing it or documenting why it's kept for interface compatibility.- public function get_connect_authorization_url(string $state = ''): string { + public function get_connect_authorization_url(): string {Based on static analysis hint about unused parameter.
422-433: Consider adding error logging for failed state verification.When state verification fails, the code silently ignores the callback. Adding a log entry would help diagnose OAuth issues in production.
// Verify CSRF state $expected_state = get_option('wu_stripe_oauth_state'); if ($expected_state && $expected_state === $state) { $this->exchange_code_for_keys($encrypted_code); + } else { + wu_log_add('stripe', 'OAuth callback state verification failed', \Psr\Log\LogLevel::WARNING); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
inc/gateways/class-base-stripe-gateway.php(5 hunks)inc/gateways/class-stripe-gateway.php(8 hunks)tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php(1 hunks)tests/WP_Ultimo/Gateways/Stripe_OAuth_Test.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
inc/gateways/class-stripe-gateway.php (2)
inc/functions/settings.php (1)
wu_register_settings_field(88-91)inc/gateways/class-base-stripe-gateway.php (3)
is_using_oauth(279-281)get_disconnect_url(390-400)get_oauth_init_url(372-382)
inc/gateways/class-base-stripe-gateway.php (2)
inc/functions/settings.php (2)
wu_get_setting(37-40)wu_save_setting(51-54)inc/functions/string-helpers.php (1)
wu_replace_dashes(50-53)
tests/WP_Ultimo/Gateways/Stripe_OAuth_Test.php (4)
tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php (1)
setUp(27-30)inc/functions/settings.php (2)
wu_save_setting(51-54)wu_get_setting(37-40)inc/gateways/class-stripe-gateway.php (1)
Stripe_Gateway(28-909)inc/gateways/class-base-stripe-gateway.php (4)
init(186-203)get_authentication_mode(269-271)is_using_oauth(279-281)get_connect_authorization_url(325-361)
tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php (2)
inc/functions/settings.php (1)
wu_save_setting(51-54)inc/gateways/class-base-stripe-gateway.php (3)
init(186-203)is_using_oauth(279-281)get_authentication_mode(269-271)
🪛 GitHub Check: Code Quality Checks
tests/WP_Ultimo/Gateways/Stripe_OAuth_Test.php
[warning] 183-183:
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space
[warning] 160-160:
Equals sign not aligned with surrounding assignments; expected 6 spaces but found 1 space
[warning] 138-138:
Equals sign not aligned with surrounding assignments; expected 5 spaces but found 1 space
[warning] 115-115:
Equals sign not aligned with surrounding assignments; expected 5 spaces but found 1 space
[warning] 107-107:
Array double arrow not aligned correctly; expected 4 space(s) between "'state'" and double arrow, but found 1.
[failure] 105-105:
Opening parenthesis of a multi-line function call must be the last content on the line
[warning] 105-105:
Array double arrow not aligned correctly; expected 5 space(s) between "'body'" and double arrow, but found 1.
[failure] 101-101:
Expected 1 space after FUNCTION keyword; 0 found
[failure] 101-101:
Only one argument is allowed per line in a multi-line function call
[failure] 101-101:
Opening parenthesis of a multi-line function call must be the last content on the line
[failure] 1-1:
Class file names should be based on the class name with "class-" prepended. Expected class-stripe-oauth-test.php, but found Stripe_OAuth_Test.php.
[failure] 1-1:
Filenames should be all lowercase with hyphens as word separators. Expected stripe-oauth-test.php, but found Stripe_OAuth_Test.php.
tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php
[warning] 112-112:
Equals sign not aligned with surrounding assignments; expected 7 spaces but found 1 space
[failure] 92-92:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 90-90:
Expected 1 space after FUNCTION keyword; 0 found
[failure] 90-90:
Only one argument is allowed per line in a multi-line function call
[failure] 90-90:
Opening parenthesis of a multi-line function call must be the last content on the line
[warning] 75-75:
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space
[warning] 53-53:
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space
🪛 Gitleaks (8.30.0)
tests/WP_Ultimo/Gateways/Stripe_OAuth_Test.php
[high] 65-65: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 84-84: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 148-148: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 193-193: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 253-253: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 PHPMD (2.15.0)
inc/gateways/class-base-stripe-gateway.php
325-325: Avoid unused parameters such as '$state'. (undefined)
(UnusedFormalParameter)
tests/WP_Ultimo/Gateways/Stripe_OAuth_Test.php
101-101: Avoid unused parameters such as '$args'. (undefined)
(UnusedFormalParameter)
130-130: Avoid unused parameters such as '$args'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: PHP 8.2
- GitHub Check: PHP 7.4
- GitHub Check: Build Plugin for Testing
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
🔇 Additional comments (20)
inc/gateways/class-stripe-gateway.php (5)
48-49: LGTM - OAuth callback hook properly wired.The OAuth callback handler is correctly hooked to
admin_init, which ensures OAuth redirects and disconnects are processed in the admin context before any output is sent.
104-150: OAuth settings fields are well-structured.The new settings fields for Stripe authentication are properly registered with appropriate visibility controls via
requirearrays. The toggle for direct API keys provides good backwards compatibility.
183-186: Direct key fields now correctly gated behind toggle.The
stripe_show_direct_keysrequirement ensures API key fields only appear when the user explicitly opts for manual key entry, keeping the OAuth flow as the default recommended path.Also applies to: 203-207, 224-228, 245-249
745-753: Payment Element container added for modern Stripe integration.The new
payment-elementdiv provides the mount point for Stripe's modern Payment Element, while keeping the legacycard-elementcontainer for fallback support.
860-908: OAuth connection HTML method is well-implemented.The
get_oauth_connection_html()method properly escapes all output usingesc_html(),esc_url(), and includes i18n-ready strings. The connected/disconnected states are clearly distinguished with appropriate visual styling.inc/gateways/class-base-stripe-gateway.php (6)
84-122: OAuth properties well-defined with sensible defaults.The new OAuth-related properties (
is_connect_enabled,oauth_access_token,oauth_account_id,platform_client_id,authentication_mode) are properly typed and initialized with secure defaults.
140-155: Stripe client correctly configured for Connected Accounts.The
stripe_accountheader is properly injected when using OAuth mode, which ensures API calls are directed to the connected merchant's account rather than the platform account.
215-261: Dual authentication logic is correct and well-documented.The
setup_api_keys()method properly prioritizes OAuth tokens over direct keys, setting appropriate flags for each mode. The fallback behavior ensures backwards compatibility.
408-441: OAuth callback handling has proper security controls.The handler correctly verifies nonces for init and disconnect actions, and validates the CSRF state for OAuth callbacks. The page context check (
'wu-settings' === $_GET['page']) adds an additional layer of protection.
450-517: Token exchange implementation is secure and handles errors appropriately.The method properly validates the response status code and required fields before saving tokens. Using
wp_die()for critical errors is acceptable for an admin-initiated flow.
525-568: Disconnect handler properly clears all OAuth tokens.The method correctly clears tokens for both test and live modes, and uses non-blocking request to notify the proxy. This ensures a clean disconnect regardless of proxy availability.
tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php (3)
27-30: LGTM - Test setup correctly resets state.The
setUp()method properly calls the helper to clear all Stripe settings before each test, ensuring test isolation.
35-56: Good test coverage for OAuth token persistence.The test verifies that saved OAuth tokens are correctly loaded by the gateway and that OAuth mode is properly detected.
85-128: Comprehensive OAuth flow simulation test.This test effectively validates the complete OAuth lifecycle including initial setup, token persistence, mode detection, and fallback to direct keys after disconnect.
tests/WP_Ultimo/Gateways/Stripe_OAuth_Test.php (6)
29-41: Good test setup with proper state isolation.The
setUp()method correctly clears relevant Stripe settings and initializes a fresh gateway instance for each test.
46-58: Solid coverage for authentication mode detection.The tests properly verify OAuth mode detection, direct mode detection, and that OAuth takes precedence when both are configured.
Also applies to: 63-74, 79-94
65-66: Gitleaks false positive - these are intentionally fake test keys.The detected "API keys" like
pk_test_direct_123andsk_test_direct_123are clearly placeholder values for unit testing, not actual secrets. No action needed.Also applies to: 84-85, 148-149, 193-194, 253-254
170-187: Good verification of account ID handling.These tests properly verify that
oauth_account_idis loaded in OAuth mode and remains empty in direct mode, using reflection appropriately for testing private state.Also applies to: 192-208
213-245: Thorough disconnect verification.The test comprehensively verifies that all OAuth tokens (test and live) are properly cleared during disconnect.
284-297: Good security verification for disconnect URL.This test confirms the disconnect URL contains proper nonce and routing parameters.
- Remove platform_client_id property from Base_Stripe_Gateway - Remove platform_client_id filter usage from OAuth E2E test - Update test step numbering for clarity - All 15 OAuth tests passing (44 assertions) Platform credentials are kept secure on the proxy server and never exposed in the distributed plugin code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php (3)
59-80: Test name vs assertion mismatchThe method name suggests asserting that the Stripe client is configured with the
Stripe-Accountheader, but the test only verifies thatoauth_account_idis loaded via reflection. Either:
- Rename the test to reflect what it actually verifies (e.g.
test_oauth_account_id_loaded_from_settings), or- Extend it to inject a mock
StripeClientviaset_stripe_client()and assert that calls are made with the expected connected account context.
128-142: Consider clearing sandbox mode and transient OAuth state in test reset
clear_all_stripe_settings()wipes most Stripe-related options but leavesstripe_sandbox_modeandwu_stripe_oauth_stateuntouched. For more deterministic tests (and to future‑proof against new tests that don’t explicitly set sandbox mode/state), consider resetting those here as well.
52-56: Style warnings about equals alignmentStatic analysis is flagging equals‑sign alignment on some assignments (e.g., Lines 53, 75, 107). If you’re aiming for a clean PHPCS run, align these per the project’s coding standard or disable the rule locally if alignment isn’t desired.
Also applies to: 74-80, 108-112
inc/gateways/class-base-stripe-gateway.php (3)
84-115: OAuth state fields and defaults are consistent with dual‑mode design
$is_connect_enabled,$oauth_access_token,$oauth_account_id, and$authentication_modeprovide a clear separation between direct and OAuth modes, and defaultingauthentication_modeto'direct'preserves existing behavior. You might optionally set$is_connect_enabled = falseexplicitly in the non‑OAuth branches ofsetup_api_keys()to make the state resets more obvious when reconnect flows evolve, but functionally this is fine.
275-309: Proxy URL and business metadataUsing a filterable proxy URL (
wu_stripe_connect_proxy_url) is a good escape hatch for staging/self‑hosting.get_business_data()is fine as a first pass; as a future enhancement you may want to pullcountryfrom site or network settings instead of hard‑coding'US'.
517-560: Disconnect flow and cleanup are sensible
- Notifying the proxy via a non‑blocking POST to
/deauthorizekeeps admin UX responsive.- Clearing both test and live OAuth tokens while leaving direct keys intact matches the intended “fallback to direct mode after disconnect” behavior.
- Redirecting back to settings with a
stripe_disconnected=1flag is a clean way to show a confirmation notice.You might optionally also clear any lingering
wu_stripe_oauth_stateoption here, thoughexchange_code_for_keys()already deletes it on successful connect.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
inc/gateways/class-base-stripe-gateway.php(5 hunks)tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
inc/gateways/class-base-stripe-gateway.php (3)
inc/functions/settings.php (2)
wu_get_setting(37-40)wu_save_setting(51-54)inc/functions/string-helpers.php (1)
wu_replace_dashes(50-53)inc/gateways/class-base-gateway.php (1)
get_id(196-199)
tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php (2)
inc/functions/settings.php (1)
wu_save_setting(51-54)inc/gateways/class-base-stripe-gateway.php (2)
is_using_oauth(271-273)get_authentication_mode(261-263)
🪛 GitHub Check: Code Quality Checks
tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php
[warning] 107-107:
Equals sign not aligned with surrounding assignments; expected 7 spaces but found 1 space
[warning] 75-75:
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space
[warning] 53-53:
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space
🪛 PHPMD (2.15.0)
inc/gateways/class-base-stripe-gateway.php
317-317: Avoid unused parameters such as '$state'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: PHP 8.4
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: Build Plugin for Testing
🔇 Additional comments (7)
tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php (2)
35-56: Good coverage of OAuth token loading and mode detectionThis test correctly simulates the callback state and asserts both
is_using_oauth()andoauth_account_idwiring; it matches the newsetup_api_keys()behavior.
85-123: Nice end‑to‑end simulation of OAuth → direct fallbackThe flow simulation (connect via OAuth, then clear tokens and fall back to direct keys) lines up with
setup_api_keys()andis_using_oauth(). This is a solid safety net for regressions in auth mode switching.inc/gateways/class-base-stripe-gateway.php (5)
132-147: Stripe client configuration for Connect accountsUsing a single
StripeClientinstance with'api_key' => $this->secret_keyand'stripe_account' => $this->oauth_account_idwhenis_using_oauth()is true is the right pattern for directing calls to a connected account. Just be aware that once the client is instantiated, subsequent changes to$secret_keyor$oauth_account_idin the same request will not be reflected unless you reset$this->stripe_client.If there’s any doubt, please confirm in the Stripe PHP client docs for your pinned version that
'stripe_account'is still the correct config key for defaulting theStripe-Accountheader.
207-253: Dual‑mode key setup (OAuth vs direct) looks correct
setup_api_keys()now:
- Prefers
{id}_test/live_access_tokenand related OAuth fields when present.- Falls back to
{id}_test/live_pk_keyand{id}_test/live_sk_keyotherwise.
It also keeps Stripe’s global API key in sync with$this->secret_key, so downstream calls use the right credentials. This matches how the new tests exercise mode switching.
255-273: Auth mode helpers are simple and work well with tests/UI
get_authentication_mode()andis_using_oauth()(requiring both'oauth'mode and$is_connect_enabled) provide a clean way for both tests and UI to differentiate modes. This aligns with the new E2E tests and should make future feature flags straightforward.
364-392: Local init/disconnect URLs are straightforward and nonce‑protected
get_oauth_init_url()andget_disconnect_url()generate admin URLs with dedicated nonces, keeping the actual proxy traffic deferred until the user clicks the actions. This is a good pattern for avoiding background proxy calls on every settings page load.
400-433: Callback handling is mostly solid; ensure it only runs in the intended admin context
- All
$_GETinputs that are used beyond simple existence checks are sanitized viasanitize_text_field( wp_unslash( … ) ), and OAuth callback CSRF is guarded via the storedwu_stripe_oauth_state.- OAuth init and disconnect flows are nonce‑protected and restricted to
page=wu-settings.Just confirm that
handle_oauth_callbacks()is only hooked on admin requests (e.g.,admin_initor similar) and not on front‑end traffic, to avoid unnecessary proxy calls orwp_die()surprises in non‑admin contexts.
| protected function exchange_code_for_keys(string $encrypted_code): void { | ||
| $proxy_url = $this->get_proxy_url(); | ||
|
|
||
| // Call proxy to exchange code for keys | ||
| $response = wp_remote_post( | ||
| $proxy_url . '/oauth/keys', | ||
| [ | ||
| 'body' => wp_json_encode( | ||
| [ | ||
| 'code' => $encrypted_code, | ||
| 'testMode' => $this->test_mode, | ||
| ] | ||
| ), | ||
| 'headers' => [ | ||
| 'Content-Type' => 'application/json', | ||
| ], | ||
| 'timeout' => 30, | ||
| ] | ||
| ); | ||
|
|
||
| if (is_wp_error($response)) { | ||
| wp_die(esc_html__('Failed to connect to proxy server', 'ultimate-multisite')); | ||
| } | ||
|
|
||
| $status_code = wp_remote_retrieve_response_code($response); | ||
| $body = wp_remote_retrieve_body($response); | ||
|
|
||
| if (200 !== $status_code) { | ||
| wp_die(esc_html__('Failed to obtain access token', 'ultimate-multisite')); | ||
| } | ||
|
|
||
| $data = json_decode($body, true); | ||
|
|
||
| if (empty($data['accessToken']) || empty($data['accountId'])) { | ||
| wp_die(esc_html__('Invalid response from proxy', 'ultimate-multisite')); | ||
| } | ||
|
|
||
| // Delete state after successful exchange | ||
| delete_option('wu_stripe_oauth_state'); | ||
|
|
||
| $id = wu_replace_dashes($this->get_id()); | ||
|
|
||
| // Save tokens | ||
| if ($this->test_mode) { | ||
| wu_save_setting("{$id}_test_access_token", $data['secretKey']); | ||
| wu_save_setting("{$id}_test_refresh_token", $data['refreshToken'] ?? ''); | ||
| wu_save_setting("{$id}_test_account_id", $data['accountId']); | ||
| wu_save_setting("{$id}_test_publishable_key", $data['publishableKey']); | ||
| } else { | ||
| wu_save_setting("{$id}_live_access_token", $data['secretKey']); | ||
| wu_save_setting("{$id}_live_refresh_token", $data['refreshToken'] ?? ''); | ||
| wu_save_setting("{$id}_live_account_id", $data['accountId']); | ||
| wu_save_setting("{$id}_live_publishable_key", $data['publishableKey']); | ||
| } | ||
|
|
||
| // Redirect back to settings | ||
| $redirect_url = add_query_arg( | ||
| [ | ||
| 'page' => 'wu-settings', | ||
| 'tab' => 'payment-gateways', | ||
| 'stripe_connected' => '1', | ||
| ], | ||
| admin_url('admin.php') | ||
| ); | ||
|
|
||
| wp_safe_redirect($redirect_url); | ||
| exit; | ||
| } |
There was a problem hiding this comment.
Robustness of /oauth/keys exchange and response handling
A few details worth tightening here:
-
JSON decode and response‑shape validation
- As with
/oauth/init,$data = json_decode($body, true);is used immediately as an array. An HTML error page or invalid JSON will cause a fatal before you hit thewp_die('Invalid response from proxy')branch.
Consider:
- $data = json_decode($body, true); - - if (empty($data['accessToken']) || empty($data['accountId'])) { - wp_die(esc_html__('Invalid response from proxy', 'ultimate-multisite')); - } + $data = json_decode($body, true); + + if (! is_array($data) || empty($data['accountId']) || empty($data['secretKey']) || empty($data['publishableKey'])) { + wp_die(esc_html__('Invalid response from proxy', 'ultimate-multisite')); + }
- As with
-
accessTokenvssecretKeymismatch- The validity check currently requires
$data['accessToken'], but you only persist$data['secretKey']into{id}_*_access_tokenoptions and never useaccessTokenafterward. - If the proxy were ever to omit
accessTokenwhile still providingsecretKey/accountId, this would incorrectly trigger the “Invalid response from proxy” path.
Aligning the validation with the fields you actually use (notably
secretKey,publishableKey, andaccountId) will make this more robust and self‑consistent. - The validity check currently requires
-
HTTP status handling is good, but consider surfacing more detail
- You already check for non‑200 responses and call
wp_die('Failed to obtain access token'). That’s fine for now; if support needs increase, you might later log$status_codeand a trimmed$body(or a proxy‑provided error message) viawu_log_add()for easier debugging.
- You already check for non‑200 responses and call
🤖 Prompt for AI Agents
In inc/gateways/class-base-stripe-gateway.php lines 442-509, the code assumes
json_decode($body, true) returns a valid array and validates on accessToken
while later using secretKey; update the handling to first decode and verify the
result is an array (check json_last_error() or is_array) and call wp_die with a
clear message if decoding fails, then change the required fields check to
validate the actual fields you persist/use (secretKey, publishableKey, and
accountId) and accept refreshToken as optional; keep existing status/error
wp_die paths but consider logging status/body for diagnostics if desired.
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
assets/js/gateways/stripe.js (1)
38-51: ** Async promise executor swallows errors.**This pattern was flagged in a previous review and remains unresolved. The
asyncexecutor with an emptycatchblock and unconditionalresolve()will hide validation failures.🔎 Recommended fix
- promises.push( - new Promise(async (resolve, reject) => { - try { - // Validate the Payment Element before submission - const { error } = await elements.submit(); - - if (error) { - reject(error); - } else { - resolve(); - } - } catch (err) { - reject(err); - } - }) - ); + promises.push( + elements.submit().then(({ error }) => { + if (error) { + return Promise.reject(error); + } + }) + );inc/gateways/class-base-stripe-gateway.php (2)
317-353: ** Harden JSON decode and handle malformed proxy responses.**This concern was flagged in a previous review and remains unresolved. If the proxy returns non-JSON or an unexpected structure,
$data['oauthUrl']and$data['state']access can fatal on PHP 8+.🔎 Recommended hardening
$data = json_decode(wp_remote_retrieve_body($response), true); - if (empty($data['oauthUrl'])) { + if (! is_array($data) || empty($data['oauthUrl']) || empty($data['state'])) { return ''; } // Store state for verification update_option('wu_stripe_oauth_state', $data['state'], false);Optionally also check
wp_remote_retrieve_response_code($response)before decoding.
442-509: ** Strengthen/oauth/keysresponse validation.**As noted in a previous review, the code validates
$data['accessToken']but only uses$data['secretKey']. If the proxy omitsaccessTokenwhile providing the other fields, this incorrectly triggers the error path. Additionally, the JSON decode lacks array validation.🔎 Recommended validation improvements
$data = json_decode($body, true); - if (empty($data['accessToken']) || empty($data['accountId'])) { + if (! is_array($data) || empty($data['accountId']) || empty($data['secretKey']) || empty($data['publishableKey'])) { wp_die(esc_html__('Invalid response from proxy', 'ultimate-multisite')); }This aligns validation with the fields actually persisted (secretKey, publishableKey, accountId) and handles malformed responses gracefully.
🧹 Nitpick comments (1)
inc/managers/class-gateway-manager.php (1)
739-774: Scheduling logic is appropriate but has unused parameters.The method correctly schedules verification only for pending Stripe payments and uses the gateway's scheduling method. However, PHPMD flags
$customer,$cart, and$typeas unused. Since this is a hook handler matching a specific signature, these are acceptable (the hook provides them).If you want to silence the static analysis warnings without changing behavior, you can add a comment:
// phpcs:disable Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsedAlternatively, explicitly unset them with a brief comment explaining they're required by the hook signature.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
assets/js/gateways/stripe.jsinc/admin-pages/class-wizard-admin-page.phpinc/checkout/class-checkout-pages.phpinc/gateways/class-base-stripe-gateway.phpinc/gateways/class-stripe-gateway.phpinc/managers/class-gateway-manager.php
🧰 Additional context used
🧬 Code graph analysis (5)
inc/managers/class-gateway-manager.php (4)
inc/functions/helper.php (2)
wu_request(132-137)wu_log_add(208-211)inc/models/class-payment.php (3)
get_status(500-503)get_gateway(524-527)get_membership(278-281)inc/database/payments/class-payment-status.php (1)
Payment_Status(22-104)inc/gateways/class-base-stripe-gateway.php (2)
verify_and_complete_payment(3376-3473)schedule_payment_verification(3487-3503)
inc/gateways/class-base-stripe-gateway.php (4)
inc/functions/settings.php (2)
wu_get_setting(37-40)wu_save_setting(51-54)inc/functions/string-helpers.php (1)
wu_replace_dashes(50-53)inc/admin-pages/class-base-admin-page.php (1)
get_id(236-239)inc/gateways/class-base-gateway.php (1)
get_id(196-199)
assets/js/gateways/stripe.js (1)
assets/js/checkout.js (1)
promises(864-864)
inc/gateways/class-stripe-gateway.php (2)
inc/functions/settings.php (1)
wu_register_settings_field(88-91)inc/gateways/class-base-stripe-gateway.php (3)
is_using_oauth(271-273)get_disconnect_url(382-392)get_oauth_init_url(364-374)
inc/checkout/class-checkout-pages.php (5)
inc/functions/helper.php (1)
wu_request(132-137)inc/managers/class-gateway-manager.php (1)
get_gateway(473-476)inc/models/class-membership.php (2)
get_gateway(1642-1645)get_status(1546-1549)inc/models/class-payment.php (2)
get_gateway(524-527)get_status(500-503)inc/database/payments/class-payment-status.php (1)
Payment_Status(22-104)
🪛 Biome (2.1.2)
assets/js/gateways/stripe.js
[error] 38-51: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
🪛 PHPMD (2.15.0)
inc/managers/class-gateway-manager.php
751-751: Avoid unused parameters such as '$customer'. (undefined)
(UnusedFormalParameter)
751-751: Avoid unused parameters such as '$cart'. (undefined)
(UnusedFormalParameter)
751-751: Avoid unused parameters such as '$type'. (undefined)
(UnusedFormalParameter)
inc/gateways/class-base-stripe-gateway.php
317-317: Avoid unused parameters such as '$state'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: PHP 8.0
🔇 Additional comments (17)
inc/admin-pages/class-wizard-admin-page.php (1)
208-222: LGTM — clean extraction of CSS classes into a helper method.The refactor improves maintainability by centralizing the wrapper classes. No functional change.
inc/gateways/class-stripe-gateway.php (4)
48-49: LGTM — OAuth callback handling properly wired.The
admin_inithook correctly delegates OAuth flow handling to the base class method. Placement and approach look good.
104-150: OAuth UI fields look solid.The new authentication section, connection status block, and direct keys toggle provide a clear UI flow. The visibility conditions (
requirearrays) properly gate fields, and all strings are internationalized.
745-751: Payment Element container migration looks correct.The shift from
card-elementandcard-errorstopayment-elementandpayment-errorsaligns with the Stripe Payment Element API. This matches the JavaScript changes that mount the Payment Element.
852-899: OAuth connection status HTML rendering is well-structured.The method provides clear visual feedback for connected vs. disconnected states, displays the account ID, and uses proper escaping. The use of Tailwind utility classes and dashicons keeps the UI consistent.
assets/js/gateways/stripe.js (2)
130-256: Payment Element initialization and mode switching logic looks solid.The dynamic mode selection (payment vs. setup) based on order total, reinitialization on mode/amount changes, and cleanup logic are all well-structured. The use of
currentElementsModeandcurrentElementsAmountprevents unnecessary reinitializations.
284-331: Style synchronization properly useselements.update().The use of
elements.update({ appearance: { ... } })(line 315) is correct for updating Payment Element appearance at runtime, as confirmed by Stripe documentation. This resolves concerns from the previous review.inc/gateways/class-base-stripe-gateway.php (4)
127-147: Stripe-Account header correctly injected for OAuth mode.The conditional
stripe_accountheader configuration ensures API calls are properly scoped to the connected account. Implementation aligns with Stripe Connect best practices.
207-253: OAuth token preference and authentication mode detection look solid.The dual-mode authentication (OAuth preferred, direct keys fallback) is correctly implemented. Token and key retrieval logic properly handles both test and live modes, and the authentication mode is set appropriately.
3365-3473: Fallback payment verification logic is well-designed.The method correctly handles setup vs. payment intents, validates payment status before attempting verification, checks Stripe intent status, and completes the payment locally when succeeded. Error handling and logging are appropriate.
3486-3503: Scheduled verification properly uses Action Scheduler.The method schedules a fallback verification job with deduplication (checks if already scheduled) and a configurable delay. Integration with the verification handler is clean.
inc/managers/class-gateway-manager.php (3)
116-130: New verification endpoints and hooks properly wired.The AJAX endpoint (
wu_check_payment_status), scheduled verification handler (wu_verify_stripe_payment), and checkout hook (wu_checkout_done) are correctly registered and aligned with the fallback verification architecture.
587-667: AJAX payment status check is well-structured.The handler validates the payment hash, retrieves the payment, handles completion, determines the gateway (with membership fallback), restricts verification to Stripe gateways, and delegates to the gateway's verification method. Error paths and success responses are appropriate.
669-737: Scheduled verification handler is robust.The method supports both legacy (single arg) and new (array) formats for backward compatibility, validates payment status, determines the gateway with fallback logic, restricts to Stripe family gateways, and logs outcomes appropriately.
inc/checkout/class-checkout-pages.php (3)
38-41: Payment status polling hook properly registered.The
wp_enqueue_scriptshook correctly wires the polling script enqueue logic for the thank you page.
212-213: Error message aliases provide compatibility.The
invalid_keyandexpired_keyaliases map to the existinginvalidkeyandexpiredkeymessages, ensuring backward compatibility for password reset flows.
676-777: Payment status polling implementation is well-designed.The method correctly:
- Identifies the thank you page context (
paymenthash +status=done)- Restricts polling to pending Stripe payments
- Provides gateway fallback via membership
- Localizes configuration (interval, max attempts, messages)
- Adds appropriate inline CSS for status states
The polling provides a user-friendly fallback when webhooks are delayed.
…te into stripe-claude
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
When the billing address field is configured to show only ZIP and Country (zip_and_country mode), these fields are now automatically hidden when any Stripe gateway is active. Both Stripe Payment Element and Stripe Checkout collect Country and ZIP in their own UI, making separate fields redundant. Changes: - Add :style binding to billing address field wrappers that hides them when gateway starts with 'stripe' - Use Vue :style binding instead of v-show for better compatibility with server-rendered in-DOM templates - Add sync_billing_address_to_stripe() method to pre-fill billing address in Stripe Checkout modal 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
inc/gateways/class-stripe-checkout-gateway.php (1)
549-556: Consider using appropriate log level for errors.The
wu_log_addfunction defaults toINFOlevel, but this is logging an exception/error condition. UsingERRORorWARNINGlevel would improve log filtering and alerting.🔎 Proposed fix
+use Psr\Log\LogLevel; + wu_log_add( 'stripe-checkout', sprintf( 'Failed to sync billing address to Stripe customer %s: %s', $stripe_customer_id, $exception->getMessage() - ) + ), + LogLevel::ERROR );inc/checkout/signup-fields/class-signup-field-billing-address.php (1)
288-288: Consider case-insensitive gateway comparison.The current check uses
gateway.startsWith('stripe'), which is case-sensitive. If gateway IDs might use mixed casing (e.g., 'Stripe', 'STRIPE'), this won't match.🔎 Proposed case-insensitive check
- $field['wrapper_html_attr'][':style'] = "{ display: gateway && gateway.startsWith('stripe') ? 'none' : '' }"; + $field['wrapper_html_attr'][':style'] = "{ display: gateway && gateway.toLowerCase().startsWith('stripe') ? 'none' : '' }";
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
inc/checkout/signup-fields/class-signup-field-billing-address.phpinc/gateways/class-stripe-checkout-gateway.php
🧰 Additional context used
🧬 Code graph analysis (1)
inc/gateways/class-stripe-checkout-gateway.php (3)
inc/gateways/class-base-stripe-gateway.php (2)
convert_to_stripe_address(1328-1338)get_stripe_client(132-147)inc/models/class-customer.php (2)
get_display_name(211-220)get_email_address(284-293)inc/functions/helper.php (1)
wu_log_add(208-211)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Plugin for Testing
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (8.1, chrome)
🔇 Additional comments (2)
inc/gateways/class-stripe-checkout-gateway.php (1)
224-228: LGTM!Good placement - syncing billing address after customer retrieval and before session creation ensures the Stripe Checkout modal has pre-filled data.
inc/checkout/signup-fields/class-signup-field-billing-address.php (1)
272-289: Remove the verification request—the implementation is correct.The
gatewayvariable is properly initialized in the Vue checkout instance (assets/js/checkout.js), and all Stripe gateway identifiers ('stripe'and'stripe-checkout') consistently use lowercase. The case-sensitivestartsWith('stripe')check appropriately matches both Stripe gateway types without false positives.
| if ($this->customer->get_display_name()) { | ||
| $update_data['name'] = $this->customer->get_display_name(); | ||
| } | ||
|
|
||
| if ($this->customer->get_email_address()) { | ||
| $update_data['email'] = $this->customer->get_email_address(); | ||
| } |
There was a problem hiding this comment.
Guard against placeholder values from deleted users.
Per the relevant snippets, get_display_name() returns "User Deleted" and get_email_address() returns "none" when the user is empty/deleted. These truthy strings will pass the current checks and be sent to Stripe, potentially overwriting valid customer data.
🔎 Proposed fix
/*
* Also update name and email if available.
*/
- if ($this->customer->get_display_name()) {
- $update_data['name'] = $this->customer->get_display_name();
- }
+ $user = $this->customer->get_user();
+ if ($user && $user->display_name) {
+ $update_data['name'] = $user->display_name;
+ }
- if ($this->customer->get_email_address()) {
- $update_data['email'] = $this->customer->get_email_address();
- }
+ if ($user && $user->user_email) {
+ $update_data['email'] = $user->user_email;
+ }Committable suggestion skipped: line range outside the PR's diff.
# Conflicts: # inc/checkout/signup-fields/class-signup-field-billing-address.php
- Add 030 spec for Stripe checkout flow (initial payment verification) - Add 040 spec for Stripe subscription renewal using Test Clocks - Creates Stripe Test Clock with subscription server-side - Advances clock past billing cycle to trigger renewal invoice - Processes renewal (pays invoice, creates payment, renews membership) - Verifies correct state: 2 payments, times_billed=2, active membership - Add PHP fixtures for setup, clock advancement, renewal processing, verification, and cleanup - Add Stripe gateway setup fixture and checkout verification fixture - Add Stripe card fill helper command (fillStripeCard) - Update e2e.yml workflow to run Stripe tests when secrets are available - Forward Stripe env vars in cypress.config.test.js for CI - Increase CI timeout to 45 minutes for test clock advancement - Add run-tests.sh to .gitignore (contains local test keys) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ition The `secrets` context is not available in step `if` expressions. Check the env var inside the shell script instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
…ddon The .wp-env.json references ../addons/ultimate-multisite-domain-seller which is not part of this repository. The override removes it for CI so wp-env start doesn't fail on the missing plugin. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
The override file approach doesn't replace arrays in wp-env. Instead, use a Node script to modify .wp-env.json directly, filtering out addon plugins and clearing mappings that reference files not present in the CI environment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Fix all issues with AI agents
In @.github/workflows/e2e.yml:
- Around line 170-214: The skip guard currently only checks STRIPE_TEST_SK_KEY;
update it to verify both STRIPE_TEST_SK_KEY and STRIPE_TEST_PK_KEY are present
(i.e., test for either being empty using a logical OR) before running the
Cypress specs, and adjust the echo message to mention both missing keys so tests
are skipped when either the secret or publishable key is not configured.
In `@inc/checkout/class-checkout-pages.php`:
- Around line 757-785: The inline CSS is being added with wp_add_inline_style
using the 'wu-checkout' handle which isn't enqueued on thank-you pages, so move
the CSS to a handle that is present: either attach it to the existing
'wu-payment-status-poll' script using
wp_add_inline_script('wu-payment-status-poll', ...) with the CSS string (and
'before' position), or register and enqueue a minimal style handle (e.g.,
wp_register_style('wu-payment-status', false);
wp_enqueue_style('wu-payment-status'); ) and then call
wp_add_inline_style('wu-payment-status', ...) so the .wu-payment-status styles
are output on the confirmation page; update the code in class-checkout-pages.php
where wp_add_inline_style('wu-checkout', ...) is currently used.
- Around line 738-739: In class-checkout-pages.php update the 'ajax_url' value
(the array key 'ajax_url' in the payment data returned by the class) to use the
main site admin-ajax endpoint instead of admin_url('admin-ajax.php'); replace
the current admin_url('admin-ajax.php') usage with
get_admin_url(wu_get_main_site_id(), 'admin-ajax.php') so AJAX requests point to
the main site's admin-ajax.php consistent with other code paths.
In `@inc/managers/class-gateway-manager.php`:
- Around line 719-736: The code calls
$gateway->verify_and_complete_payment($payment_id) and immediately indexes
$result['success'] and $result['message']; add a defensive check after the call
to ensure $result is an array (and contains the expected keys) before using
those indexes — e.g., verify is_array($result) && array_key_exists('success',
$result) && array_key_exists('message', $result); if the check fails, call
wu_log_add (same 'stripe' logger) with a warning about an unexpected response
from verify_and_complete_payment and return early (or set a safe default result)
so subsequent logging/processing does not throw notices.
- Around line 596-667: The ajax_check_payment_status handler lacks a nonce check
and trusts the return of $gateway->verify_and_complete_payment; add a nonce
verification (use wp_verify_nonce on a nonce passed in the AJAX request,
rejecting with wp_send_json_error if invalid) before doing any processing in
ajax_check_payment_status, and make the call to
$gateway->verify_and_complete_payment($payment->get_id()) defensive by wrapping
it in try/catch, validating that $result is an array and not a WP_Error (or
null) before accessing $result['success'], and respond with wp_send_json_error
on exceptions/invalid return values; ensure you reference
ajax_check_payment_status, $payment, $gateway, verify_and_complete_payment,
$result, wp_verify_nonce and wp_send_json_error when implementing these checks.
In `@tests/e2e/cypress/fixtures/advance-stripe-test-clock.php`:
- Around line 21-23: The multi-line call to
$stripe->testHelpers->testClocks->advance must follow linter rules: move the
opening parenthesis to the end of the method call line, put each argument on its
own line (so $clock_id on one line and the associative array ['frozen_time' =>
$target_timestamp] on its own line), and place the closing parenthesis on its
own line; update the call to advance(...) accordingly while keeping the same
arguments and order.
- Around line 40-51: Reformat the multi-line wp_json_encode calls so the opening
parenthesis is the last character on its line and the closing parenthesis is on
its own line: replace the first echo wp_json_encode([ ... ]); (the one building
success/status/frozen_time/clock_id using $status, $clock, $clock_id) and the
second echo wp_json_encode([ ... ]); in the catch block (using $e->getMessage()
and $e->getCode()) with versions that have "echo wp_json_encode(" on one line,
the array entries on subsequent lines, then a standalone ")" line followed by
the semicolon.
- Around line 35-44: Replace non‑Yoda comparisons for $status: change the if
check if ($status === 'ready') to if ('ready' === $status) and change the array
entry 'success' => $status === 'ready' to 'success' => 'ready' === $status; keep
the rest of the wp_json_encode array (including 'frozen_time' =>
$clock->frozen_time ?? null and 'clock_id' => $clock_id) intact and ensure the
multi-line wp_json_encode call remains consistently formatted.
In `@tests/e2e/cypress/fixtures/process-stripe-renewal.php`:
- Around line 42-48: The arrays passed into wp_json_encode in the error branch
(the array containing 'error', 'invoice_count', 'billing_reasons' created inside
the anonymous function) and the success branch (the similar array around lines
128-138) have misaligned => operators; fix both by aligning the array keys and
their => operators vertically so the arrows line up (adjust spacing/padding
around keys like 'error', 'invoice_count', 'billing_reasons' and their
counterparts in the success array) while preserving the existing keys, values,
and the array_map callback.
- Around line 100-105: The currency multiplier currently uses
wu_stripe_get_currency_multiplier() with no args causing USD (100) to be
assumed; update the logic that computes $currency_multiplier to pass the
invoice's currency (from $renewal_invoice->currency) into
wu_stripe_get_currency_multiplier(...) and use that returned divisor when
computing $total so zero-decimal currencies (e.g., JPY) are handled correctly,
preserving the existing fallback behavior if the function does not exist or
returns null.
- Around line 78-88: The expiration calculation uses new \DateTime() which may
use a different timezone than WordPress; change the creation of $renewal_date to
use WP's timezone (e.g., new \DateTime('now', wp_timezone()) or
DateTimeImmutable with wp_timezone()), then continue using
$renewal_date->setTimestamp($end_timestamp) and ->setTime(23,59,59) so the
comparison with $stripe_estimated_charge_timestamp and final
$renewal_date->format('Y-m-d H:i:s') happen in the same timezone as the webhook
handler.
In `@tests/e2e/cypress/integration/030-stripe-checkout-flow.spec.js`:
- Around line 29-36: The cy.exec call in the test interpolates pkKey and skKey
directly into a shell command (in 030-stripe-checkout-flow.spec.js) which
creates a shell-injection risk; fix by changing the exec invocation to pass the
keys via the env option (e.g. cy.exec(command, { env: { STRIPE_PK: pkKey,
STRIPE_SK: skKey }, timeout: 60000 })) and update the PHP fixture (currently
reading $args) to read from those environment variables, or if you cannot change
the fixture, validate/sanitize pkKey and skKey before interpolation using a
strict regex that enforces the expected pk_test_ / sk_test_ formats and
reject/throw on mismatch.
- Around line 86-92: The test directly sets inputs using jQuery .val(), which
won't trigger Vue reactivity for fields like "#field-billing_zip_code" or
'[name="billing_address[billing_zip_code]"]'; update the test to interact with
the inputs in a way that fires events (e.g., use Cypress .type(...) on
cy.get("#field-billing_zip_code") /
cy.get('[name="billing_address[billing_zip_code]"]') or, if keeping direct value
assignment, dispatch an 'input'/'change' event after setting the value so the
component's v-model updates accordingly; ensure you apply this change in the
block that currently inspects $body.find(...) and sets .val("94105").
In `@tests/e2e/cypress/integration/040-stripe-renewal-flow.spec.js`:
- Around line 28-34: The cy.exec calls interpolate secrets (pkKey, skKey and
later values like setupData.test_clock_id) directly into shell strings; instead
pass secrets via the cy.exec options.env object (e.g., call cy.exec(`npx wp-env
run tests-cli wp eval-file ${CONTAINER_FIXTURES}/setup-stripe-gateway.php`, {
timeout: 60000, env: { PK_KEY: pkKey, SK_KEY: skKey } })) and update the PHP
helper (setup-stripe-gateway.php) to read these via
getenv('PK_KEY')/getenv('SK_KEY'); do the same for subsequent cy.exec
invocations that use setupData.test_clock_id (pass TEST_CLOCK_ID in env) rather
than interpolating into the command string so no secrets/IDs are embedded in the
shell command.
🧹 Nitpick comments (6)
inc/checkout/class-checkout-pages.php (1)
718-755: Script and CSS enqueued even for non-pending Stripe payments.When
$is_pendingisfalse(payment already completed/failed), the script, localized data, and CSS are still loaded on the page. The early return should include the pending check to avoid unnecessary asset loading.♻️ Suggested fix
- if (! $is_stripe_payment) { + if (! $is_stripe_payment || ! $is_pending) { return; }With this change,
$is_pendingand theshould_polllocalized variable become unnecessary — you can remove them.inc/managers/class-gateway-manager.php (1)
751-774: Unused parameters$customer,$cart,$typeare expected for hook signature — consider a brief docblock note.PHPMD flags these as unused, but they're required to match the
wu_checkout_doneaction signature (registered with priority 10 and 5 args on line 130). This is fine — a small inline comment (e.g.,// Hook signature requires all 5 params) would silence future reviewers.tests/e2e/cypress/integration/030-stripe-checkout-flow.spec.js (1)
49-58: Hardcodedcy.wait()calls reduce test reliability.Lines 49 and 58 use
cy.wait(3000). These are flaky-test magnets — too short on slow CI, wasted time on fast machines. Where possible, prefer waiting on a specific DOM condition (e.g., a loading spinner disappearing, or a specific element appearing). That said, this is a common pragmatic tradeoff in E2E Stripe tests.tests/e2e/cypress/integration/040-stripe-renewal-flow.spec.js (1)
37-63: Test interdependency: each test depends onsetupDatafrom the previous test.The sequential tests share mutable state via
setupData. If any earlier test fails, all subsequent tests fail with misleading assertion errors (e.g.,expect(undefined).to.exist). This is a known Cypress limitation for multi-step flows and acceptable here, but consider adding a clearer skip mechanism:// At the top of each dependent test: if (!setupData.test_clock_id) { cy.log('Skipping: prerequisite test did not complete'); return; // or this.skip() in a function() callback }Also applies to: 65-86, 88-115
tests/e2e/cypress/fixtures/setup-stripe-renewal-test.php (2)
146-153: Minor alignment issues flagged by linter.Static analysis flags inconsistent spacing around
=on lines 149-152. Cosmetic only, but easy to fix.Proposed alignment fix
if (is_string($initial_invoice)) { - $invoice_obj = $stripe->invoices->retrieve($initial_invoice); - $gateway_pay_id = $invoice_obj->charge ?? $invoice_obj->payment_intent ?? $initial_invoice; + $invoice_obj = $stripe->invoices->retrieve($initial_invoice); + $gateway_pay_id = $invoice_obj->charge ?? $invoice_obj->payment_intent ?? $initial_invoice; } else { - $gateway_pay_id = $initial_invoice->charge ?? $initial_invoice->payment_intent ?? ''; + $gateway_pay_id = $initial_invoice->charge ?? $initial_invoice->payment_intent ?? ''; }
185-190: Linter flags alignment in the catch block — minor cosmetic issue.Proposed fix
echo wp_json_encode([ - 'error' => $e->getMessage(), - 'code' => $e->getCode(), + 'error' => $e->getMessage(), + 'code' => $e->getCode(), ]);
| - name: Run Stripe Tests (After Setup) | ||
| id: stripe-tests | ||
| env: | ||
| STRIPE_TEST_PK_KEY: ${{ secrets.STRIPE_TEST_PK_KEY }} | ||
| STRIPE_TEST_SK_KEY: ${{ secrets.STRIPE_TEST_SK_KEY }} | ||
| run: | | ||
| if [ -z "$STRIPE_TEST_SK_KEY" ]; then | ||
| echo "⏭️ Skipping Stripe tests: STRIPE_TEST_SK_KEY secret not configured" | ||
| exit 0 | ||
| fi | ||
|
|
||
| set +e | ||
| echo "=== Starting Stripe Tests ===" | ||
|
|
||
| STRIPE_TESTS=( | ||
| "tests/e2e/cypress/integration/030-stripe-checkout-flow.spec.js" | ||
| "tests/e2e/cypress/integration/040-stripe-renewal-flow.spec.js" | ||
| ) | ||
|
|
||
| TOTAL_FAILURES=0 | ||
|
|
||
| for TEST_SPEC in "${STRIPE_TESTS[@]}"; do | ||
| echo "Running: $TEST_SPEC" | ||
|
|
||
| npx cypress run \ | ||
| --config-file cypress.config.test.js \ | ||
| --spec "$TEST_SPEC" \ | ||
| --browser ${{ matrix.browser }} | ||
|
|
||
| CYPRESS_EXIT_CODE=$? | ||
|
|
||
| if [ $CYPRESS_EXIT_CODE -eq 0 ]; then | ||
| echo "✅ $TEST_SPEC passed" | ||
| else | ||
| echo "❌ $TEST_SPEC failed with exit code $CYPRESS_EXIT_CODE" | ||
| TOTAL_FAILURES=$((TOTAL_FAILURES + 1)) | ||
| fi | ||
| done | ||
|
|
||
| if [ $TOTAL_FAILURES -gt 0 ]; then | ||
| echo "❌ $TOTAL_FAILURES Stripe test(s) failed" | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "✅ All Stripe tests passed!" |
There was a problem hiding this comment.
Consider also checking STRIPE_TEST_PK_KEY in the skip guard.
Line 176 only checks STRIPE_TEST_SK_KEY, but the Cypress tests also need STRIPE_TEST_PK_KEY (forwarded via config). If only the secret key is configured but the publishable key is missing, tests would run but likely fail during gateway setup.
Suggested fix
- if [ -z "$STRIPE_TEST_SK_KEY" ]; then
- echo "⏭️ Skipping Stripe tests: STRIPE_TEST_SK_KEY secret not configured"
+ if [ -z "$STRIPE_TEST_SK_KEY" ] || [ -z "$STRIPE_TEST_PK_KEY" ]; then
+ echo "⏭️ Skipping Stripe tests: Stripe test key secrets not configured"
exit 0
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Run Stripe Tests (After Setup) | |
| id: stripe-tests | |
| env: | |
| STRIPE_TEST_PK_KEY: ${{ secrets.STRIPE_TEST_PK_KEY }} | |
| STRIPE_TEST_SK_KEY: ${{ secrets.STRIPE_TEST_SK_KEY }} | |
| run: | | |
| if [ -z "$STRIPE_TEST_SK_KEY" ]; then | |
| echo "⏭️ Skipping Stripe tests: STRIPE_TEST_SK_KEY secret not configured" | |
| exit 0 | |
| fi | |
| set +e | |
| echo "=== Starting Stripe Tests ===" | |
| STRIPE_TESTS=( | |
| "tests/e2e/cypress/integration/030-stripe-checkout-flow.spec.js" | |
| "tests/e2e/cypress/integration/040-stripe-renewal-flow.spec.js" | |
| ) | |
| TOTAL_FAILURES=0 | |
| for TEST_SPEC in "${STRIPE_TESTS[@]}"; do | |
| echo "Running: $TEST_SPEC" | |
| npx cypress run \ | |
| --config-file cypress.config.test.js \ | |
| --spec "$TEST_SPEC" \ | |
| --browser ${{ matrix.browser }} | |
| CYPRESS_EXIT_CODE=$? | |
| if [ $CYPRESS_EXIT_CODE -eq 0 ]; then | |
| echo "✅ $TEST_SPEC passed" | |
| else | |
| echo "❌ $TEST_SPEC failed with exit code $CYPRESS_EXIT_CODE" | |
| TOTAL_FAILURES=$((TOTAL_FAILURES + 1)) | |
| fi | |
| done | |
| if [ $TOTAL_FAILURES -gt 0 ]; then | |
| echo "❌ $TOTAL_FAILURES Stripe test(s) failed" | |
| exit 1 | |
| fi | |
| echo "✅ All Stripe tests passed!" | |
| - name: Run Stripe Tests (After Setup) | |
| id: stripe-tests | |
| env: | |
| STRIPE_TEST_PK_KEY: ${{ secrets.STRIPE_TEST_PK_KEY }} | |
| STRIPE_TEST_SK_KEY: ${{ secrets.STRIPE_TEST_SK_KEY }} | |
| run: | | |
| if [ -z "$STRIPE_TEST_SK_KEY" ] || [ -z "$STRIPE_TEST_PK_KEY" ]; then | |
| echo "⏭️ Skipping Stripe tests: Stripe test key secrets not configured" | |
| exit 0 | |
| fi | |
| set +e | |
| echo "=== Starting Stripe Tests ===" | |
| STRIPE_TESTS=( | |
| "tests/e2e/cypress/integration/030-stripe-checkout-flow.spec.js" | |
| "tests/e2e/cypress/integration/040-stripe-renewal-flow.spec.js" | |
| ) | |
| TOTAL_FAILURES=0 | |
| for TEST_SPEC in "${STRIPE_TESTS[@]}"; do | |
| echo "Running: $TEST_SPEC" | |
| npx cypress run \ | |
| --config-file cypress.config.test.js \ | |
| --spec "$TEST_SPEC" \ | |
| --browser ${{ matrix.browser }} | |
| CYPRESS_EXIT_CODE=$? | |
| if [ $CYPRESS_EXIT_CODE -eq 0 ]; then | |
| echo "✅ $TEST_SPEC passed" | |
| else | |
| echo "❌ $TEST_SPEC failed with exit code $CYPRESS_EXIT_CODE" | |
| TOTAL_FAILURES=$((TOTAL_FAILURES + 1)) | |
| fi | |
| done | |
| if [ $TOTAL_FAILURES -gt 0 ]; then | |
| echo "❌ $TOTAL_FAILURES Stripe test(s) failed" | |
| exit 1 | |
| fi | |
| echo "✅ All Stripe tests passed!" |
🤖 Prompt for AI Agents
In @.github/workflows/e2e.yml around lines 170 - 214, The skip guard currently
only checks STRIPE_TEST_SK_KEY; update it to verify both STRIPE_TEST_SK_KEY and
STRIPE_TEST_PK_KEY are present (i.e., test for either being empty using a
logical OR) before running the Cypress specs, and adjust the echo message to
mention both missing keys so tests are skipped when either the secret or
publishable key is not configured.
| 'payment_hash' => $payment_hash, | ||
| 'ajax_url' => admin_url('admin-ajax.php'), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "payment.status|payment_status|wu_payment.*poll|wu_check_payment" --type=php -C3Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 33946
🏁 Script executed:
# Also search for add_action calls with ajax
rg -n "add_action.*wp_ajax" --type=php -C2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 10159
🏁 Script executed:
# Let's look at the specific file to understand context
head -750 inc/checkout/class-checkout-pages.php | tail -50Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1826
🏁 Script executed:
# Find Gateway Manager class and its init/constructor
rg -n "class.*Gateway.*Manager|function init|function __construct" inc/managers/class-gateway-manager.php -A5 | head -50Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 334
🏁 Script executed:
# Check if there's any is_main_site or multisite-specific handling in the hook registration
rg -n "is_main_site|is_multisite|switch_to_blog|restore_current_blog" inc/managers/class-gateway-manager.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 136
🏁 Script executed:
# Look for where Gateway Manager is instantiated
rg -n "new.*Gateway.*Manager|Gateway_Manager" inc/ -l | head -10Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 227
🏁 Script executed:
# Check the actual handler implementation to see if it accesses data from the current site
sed -n '596,650p' inc/managers/class-gateway-manager.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1671
🏁 Script executed:
# Find the wu_get_payment_by_hash function
rg -n "function wu_get_payment_by_hash|def wu_get_payment_by_hash" --type=php -A10Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 637
🏁 Script executed:
# Check if payments are stored globally (main site) or per-site
rg -n "payments.*table|register_table.*payment" inc/ --type=php | head -20Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 388
🏁 Script executed:
# Look for any switch_to_blog or site context handling in payment queries
rg -n "switch_to_blog|restore_current_blog|blog_id|get_current_blog_id" inc/managers/class-gateway-manager.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Check where the AJAX hook is registered and if there's any site filtering
sed -n '100,150p' inc/managers/class-gateway-manager.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1594
🏁 Script executed:
# Check where Gateway Manager singleton is instantiated
rg -n "Gateway_Manager::get_instance|gateway_manager\|new Gateway_Manager" inc/ --type=phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 873
🏁 Script executed:
# Check the Singleton trait to understand how it works
rg -n "trait Singleton|get_instance|instance" inc/traits/class-singleton.php -A5Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 151
🏁 Script executed:
# Check if wp_ajax hooks are global or site-specific in WordPress multisite
# by looking at how the payment status polling script is enqueued
sed -n '694,760p' inc/checkout/class-checkout-pages.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 2272
🏁 Script executed:
# Look for any documentation or comments about multisite handling
rg -n "multisite|is_multisite|subsite|sub-site" inc/managers/class-gateway-manager.php -iRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 2116
🏁 Script executed:
# Check how WP Ultimo loads and initializes managers
rg -n "plugins_loaded|on_load" inc/class-wp-ultimo.php -B2 -A5 | head -40Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 337
🏁 Script executed:
# Verify if Gateway Manager's on_load method is called and if it's site-aware
sed -n '66,180p' inc/managers/class-gateway-manager.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 3148
🏁 Script executed:
# Check the payment-status-poll.js to see how it handles the AJAX call
find . -name "payment-status-poll.js" -type f 2>/dev/nullRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Look for assets directory
fd -t f "payment-status-poll" --type fRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Check the Gateway Manager's verify_and_complete_payment to see if there's site context handling
rg -n "verify_and_complete_payment" inc/gateways/ --type=php -A15 | head -60Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1304
🏁 Script executed:
# Check if payments table is truly global by looking at the table registration
sed -n '1,60p' inc/database/payments/class-payments-table.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1025
🏁 Script executed:
# Look for any blog_id or site-specific columns in the payments table
rg -n "blog_id|site_id|get_site_id" inc/database/payments/ --type=phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Check the base manager to understand how the Gateway Manager extends it
rg -n "class Base_Manager|is_main_site|current_site" inc/managers/class-base-manager.php -B2 -A10 | head -50Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 274
🏁 Script executed:
# Check if wp_ajax hooks are truly site-specific or global in WordPress multisite
# by searching for any site-specific AJAX handling patterns in the codebase
rg -n "wp_ajax.*switch_to_blog|wp_ajax.*restore_current_blog" inc/ --type=phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Verify the Payment::get_by_hash method to confirm it accesses the global table
rg -n "get_by_hash|class Payment" inc/models/class-payment.php -A20 | head -60Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 566
🏁 Script executed:
# Check if there are any tests that verify multisite AJAX functionality
find . -name "*test*" -o -name "*spec*" -type f | xargs grep -l "multisite\|ajax\|admin_url" 2>/dev/null | head -5Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 300
🏁 Script executed:
# Look at how admin_url is documented and whether there's any multisite-specific handling
rg -n "admin_url.*admin-ajax" inc/ --type=php -B3 -A3Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 4440
Use the main site's admin-ajax.php for consistency with multisite handling patterns.
The AJAX handler is registered globally and the payments table is global, so requests technically work from subsite admin-ajax.php URLs. However, the codebase already uses get_admin_url(wu_get_main_site_id(), 'admin-ajax.php') for similar AJAX operations in email and invoice template customization. Align this to use the main site's admin-ajax.php URL for consistency and to avoid potential future issues with site context.
🤖 Prompt for AI Agents
In `@inc/checkout/class-checkout-pages.php` around lines 738 - 739, In
class-checkout-pages.php update the 'ajax_url' value (the array key 'ajax_url'
in the payment data returned by the class) to use the main site admin-ajax
endpoint instead of admin_url('admin-ajax.php'); replace the current
admin_url('admin-ajax.php') usage with get_admin_url(wu_get_main_site_id(),
'admin-ajax.php') so AJAX requests point to the main site's admin-ajax.php
consistent with other code paths.
| // Add inline CSS for the status messages | ||
| wp_add_inline_style( | ||
| 'wu-checkout', | ||
| ' | ||
| .wu-payment-status { | ||
| padding: 12px 16px; | ||
| border-radius: 6px; | ||
| margin-bottom: 16px; | ||
| font-weight: 500; | ||
| } | ||
| .wu-payment-status-pending, | ||
| .wu-payment-status-checking { | ||
| background-color: #fef3cd; | ||
| color: #856404; | ||
| border: 1px solid #ffc107; | ||
| } | ||
| .wu-payment-status-completed { | ||
| background-color: #d4edda; | ||
| color: #155724; | ||
| border: 1px solid #28a745; | ||
| } | ||
| .wu-payment-status-timeout, | ||
| .wu-payment-status-error { | ||
| background-color: #f8d7da; | ||
| color: #721c24; | ||
| border: 1px solid #f5c6cb; | ||
| } | ||
| ' | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for wu-checkout registration and enqueuing in the codebase
rg -n "wu-checkout" --type=php | head -40Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 4819
🏁 Script executed:
#!/bin/bash
# Get context around lines 757-785 in the checkout-pages file
wc -l inc/checkout/class-checkout-pages.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 122
🏁 Script executed:
#!/bin/bash
# Read the section around lines 757-785 to understand context
sed -n '740,790p' inc/checkout/class-checkout-pages.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1592
🏁 Script executed:
#!/bin/bash
# Get broader context to find the function/method name
sed -n '720,760p' inc/checkout/class-checkout-pages.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1509
🏁 Script executed:
#!/bin/bash
# Check the class structure and method definitions
rg -n "function|public|protected|private" inc/checkout/class-checkout-pages.php | grep -B5 -A5 "75[0-9]"Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Look at where class-checkout-pages is instantiated and used
rg -n "class-checkout-pages\|checkout-pages" --type=php | head -20Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Look for function definitions around line 700-750
sed -n '700,730p' inc/checkout/class-checkout-pages.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 920
🏁 Script executed:
#!/bin/bash
# Get all function/method definitions in the file with line numbers
rg -n "^\s*(public|private|protected)?\s*function" inc/checkout/class-checkout-pages.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1445
🏁 Script executed:
#!/bin/bash
# Find where maybe_enqueue_payment_status_poll is called/hooked
rg -n "maybe_enqueue_payment_status_poll" --type=phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 306
🏁 Script executed:
#!/bin/bash
# Check the init method and hooks in class-checkout-pages
sed -n '30,95p' inc/checkout/class-checkout-pages.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 2280
🏁 Script executed:
#!/bin/bash
# Check when/where wu-checkout is enqueued in class-checkout.php
sed -n '2620,2650p' inc/checkout/class-checkout.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1042
🏁 Script executed:
#!/bin/bash
# Find where register_scripts from class-checkout is called
rg -n "register_scripts" inc/checkout/class-checkout.php | head -5Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 199
🏁 Script executed:
#!/bin/bash
# Search for hooks/actions that call register_scripts on checkout
rg -n "register_scripts\|class-checkout" inc/checkout/class-checkout.php | grep -i "add_action\|add_filter" | head -10Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Check the Checkout class initialization to see when register_scripts is called
sed -n '1,150p' inc/checkout/class-checkout.php | grep -A5 -B5 "register_scripts"Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Find where wu_checkout_scripts action is fired
rg -n "wu_checkout_scripts" --type=phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 830
🏁 Script executed:
#!/bin/bash
# Check the initialization of Checkout class
sed -n '150,250p' inc/checkout/class-checkout.php | head -60Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1516
🏁 Script executed:
#!/bin/bash
# Check what's enqueued on thank-you pages - search for enqueue in render_confirmation_page and related methods
sed -n '674,700p' inc/checkout/class-checkout-pages.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 988
🏁 Script executed:
#!/bin/bash
# Check if wu_checkout_scripts is fired on thank-you page
rg -n "render_confirmation_page\|do_action.*wu_checkout_scripts" inc/checkout/class-checkout-pages.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Get full context of render_confirmation_page method
sed -n '674,695p' inc/checkout/class-checkout-pages.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 760
🏁 Script executed:
#!/bin/bash
# Check the confirmation template to see if it enqueues wu-checkout
find . -path "./views/*" -name "*confirmation*" -type fRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 114
🏁 Script executed:
#!/bin/bash
# Look for confirmation template
find . -name "confirmation.php" -type fRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 114
🏁 Script executed:
#!/bin/bash
# Check where wu_checkout_scripts is fired on the thank-you/confirmation page
rg -n "do_action.*wu_checkout_scripts" views/ --type=phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 153
🏁 Script executed:
#!/bin/bash
# Read the confirmation template to see what it does
cat views/checkout/confirmation.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1703
🏁 Script executed:
#!/bin/bash
# Check if confirmation page calls any actions related to scripts
rg -n "do_action\|wp_enqueue" views/checkout/confirmation.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Check if 'wu-checkout' is ever enqueued on pages outside of checkout form context
rg -n "wp_enqueue.*wu-checkout" --type=phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 701
🏁 Script executed:
#!/bin/bash
# Verify: does the thank-you page trigger wp_enqueue_scripts while wu_checkout_scripts fires?
# Check the page flow - look for how pages are rendered
sed -n '1,50p' inc/checkout/class-checkout-pages.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1337
🏁 Script executed:
#!/bin/bash
# Double-check by looking at the full maybe_enqueue_payment_status_poll function
sed -n '694,786p' inc/checkout/class-checkout-pages.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 2828
Inline styles depend on wu-checkout handle being enqueued, which is not present on thank-you pages.
wp_add_inline_style('wu-checkout', ...) will silently fail on the thank-you page because the wu-checkout stylesheet is only enqueued during checkout form rendering (via the wu_checkout_scripts action in class-checkout.php), not during confirmation page display. The status message styles won't render.
Attach the inline CSS to the wu-payment-status-poll script you're already enqueuing instead:
wp_add_inline_script('wu-payment-status-poll', 'your_css_here', 'before');Or register and enqueue a minimal styles dependency:
wp_register_style('wu-payment-status', false);
wp_enqueue_style('wu-payment-status');
wp_add_inline_style('wu-payment-status', '...');🤖 Prompt for AI Agents
In `@inc/checkout/class-checkout-pages.php` around lines 757 - 785, The inline CSS
is being added with wp_add_inline_style using the 'wu-checkout' handle which
isn't enqueued on thank-you pages, so move the CSS to a handle that is present:
either attach it to the existing 'wu-payment-status-poll' script using
wp_add_inline_script('wu-payment-status-poll', ...) with the CSS string (and
'before' position), or register and enqueue a minimal style handle (e.g.,
wp_register_style('wu-payment-status', false);
wp_enqueue_style('wu-payment-status'); ) and then call
wp_add_inline_style('wu-payment-status', ...) so the .wu-payment-status styles
are output on the confirmation page; update the code in class-checkout-pages.php
where wp_add_inline_style('wu-checkout', ...) is currently used.
| public function ajax_check_payment_status(): void { | ||
|
|
||
| $payment_hash = wu_request('payment_hash'); | ||
|
|
||
| if (empty($payment_hash)) { | ||
| wp_send_json_error(['message' => __('Payment hash is required.', 'ultimate-multisite')]); | ||
| } | ||
|
|
||
| $payment = wu_get_payment_by_hash($payment_hash); | ||
|
|
||
| if (! $payment) { | ||
| wp_send_json_error(['message' => __('Payment not found.', 'ultimate-multisite')]); | ||
| } | ||
|
|
||
| // If already completed, return success | ||
| if ($payment->get_status() === \WP_Ultimo\Database\Payments\Payment_Status::COMPLETED) { | ||
| wp_send_json_success( | ||
| [ | ||
| 'status' => 'completed', | ||
| 'message' => __('Payment completed.', 'ultimate-multisite'), | ||
| ] | ||
| ); | ||
| } | ||
|
|
||
| // Only try to verify Stripe payments | ||
| $gateway_id = $payment->get_gateway(); | ||
|
|
||
| if (empty($gateway_id)) { | ||
| // Check membership gateway as fallback | ||
| $membership = $payment->get_membership(); | ||
| $gateway_id = $membership ? $membership->get_gateway() : ''; | ||
| } | ||
|
|
||
| if (! in_array($gateway_id, ['stripe', 'stripe-checkout'], true)) { | ||
| wp_send_json_success( | ||
| [ | ||
| 'status' => $payment->get_status(), | ||
| 'message' => __('Non-Stripe payment, cannot verify.', 'ultimate-multisite'), | ||
| ] | ||
| ); | ||
| } | ||
|
|
||
| // Get the gateway instance and verify | ||
| $gateway = wu_get_gateway($gateway_id); | ||
|
|
||
| if (! $gateway || ! method_exists($gateway, 'verify_and_complete_payment')) { | ||
| wp_send_json_success( | ||
| [ | ||
| 'status' => $payment->get_status(), | ||
| 'message' => __('Gateway does not support verification.', 'ultimate-multisite'), | ||
| ] | ||
| ); | ||
| } | ||
|
|
||
| $result = $gateway->verify_and_complete_payment($payment->get_id()); | ||
|
|
||
| if ($result['success']) { | ||
| wp_send_json_success( | ||
| [ | ||
| 'status' => $result['status'] ?? 'completed', | ||
| 'message' => $result['message'], | ||
| ] | ||
| ); | ||
| } else { | ||
| wp_send_json_success( | ||
| [ | ||
| 'status' => $result['status'] ?? 'pending', | ||
| 'message' => $result['message'], | ||
| ] | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing nonce verification and no type check on verify_and_complete_payment result.
Two concerns with this AJAX handler:
-
No nonce check: This endpoint is registered for
nopriv(unauthenticated) users and relies solely on the payment hash for authorization. While the hash provides some obscurity, anyone who obtains a hash can repeatedly triggerverify_and_complete_payment, which may interact with the Stripe API. Consider adding a nonce or rate-limiting mechanism. -
Unchecked result type: Line 652 accesses
$result['success']without verifying that$resultis an array. Ifverify_and_complete_paymentreturnsnull, aWP_Error, or throws, this will fatal.
Proposed defensive check for the result
$result = $gateway->verify_and_complete_payment($payment->get_id());
- if ($result['success']) {
+ if (! is_array($result)) {
+ wp_send_json_error(['message' => __('Unexpected verification result.', 'ultimate-multisite')]);
+ }
+
+ if (! empty($result['success'])) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function ajax_check_payment_status(): void { | |
| $payment_hash = wu_request('payment_hash'); | |
| if (empty($payment_hash)) { | |
| wp_send_json_error(['message' => __('Payment hash is required.', 'ultimate-multisite')]); | |
| } | |
| $payment = wu_get_payment_by_hash($payment_hash); | |
| if (! $payment) { | |
| wp_send_json_error(['message' => __('Payment not found.', 'ultimate-multisite')]); | |
| } | |
| // If already completed, return success | |
| if ($payment->get_status() === \WP_Ultimo\Database\Payments\Payment_Status::COMPLETED) { | |
| wp_send_json_success( | |
| [ | |
| 'status' => 'completed', | |
| 'message' => __('Payment completed.', 'ultimate-multisite'), | |
| ] | |
| ); | |
| } | |
| // Only try to verify Stripe payments | |
| $gateway_id = $payment->get_gateway(); | |
| if (empty($gateway_id)) { | |
| // Check membership gateway as fallback | |
| $membership = $payment->get_membership(); | |
| $gateway_id = $membership ? $membership->get_gateway() : ''; | |
| } | |
| if (! in_array($gateway_id, ['stripe', 'stripe-checkout'], true)) { | |
| wp_send_json_success( | |
| [ | |
| 'status' => $payment->get_status(), | |
| 'message' => __('Non-Stripe payment, cannot verify.', 'ultimate-multisite'), | |
| ] | |
| ); | |
| } | |
| // Get the gateway instance and verify | |
| $gateway = wu_get_gateway($gateway_id); | |
| if (! $gateway || ! method_exists($gateway, 'verify_and_complete_payment')) { | |
| wp_send_json_success( | |
| [ | |
| 'status' => $payment->get_status(), | |
| 'message' => __('Gateway does not support verification.', 'ultimate-multisite'), | |
| ] | |
| ); | |
| } | |
| $result = $gateway->verify_and_complete_payment($payment->get_id()); | |
| if ($result['success']) { | |
| wp_send_json_success( | |
| [ | |
| 'status' => $result['status'] ?? 'completed', | |
| 'message' => $result['message'], | |
| ] | |
| ); | |
| } else { | |
| wp_send_json_success( | |
| [ | |
| 'status' => $result['status'] ?? 'pending', | |
| 'message' => $result['message'], | |
| ] | |
| ); | |
| } | |
| } | |
| public function ajax_check_payment_status(): void { | |
| $payment_hash = wu_request('payment_hash'); | |
| if (empty($payment_hash)) { | |
| wp_send_json_error(['message' => __('Payment hash is required.', 'ultimate-multisite')]); | |
| } | |
| $payment = wu_get_payment_by_hash($payment_hash); | |
| if (! $payment) { | |
| wp_send_json_error(['message' => __('Payment not found.', 'ultimate-multisite')]); | |
| } | |
| // If already completed, return success | |
| if ($payment->get_status() === \WP_Ultimo\Database\Payments\Payment_Status::COMPLETED) { | |
| wp_send_json_success( | |
| [ | |
| 'status' => 'completed', | |
| 'message' => __('Payment completed.', 'ultimate-multisite'), | |
| ] | |
| ); | |
| } | |
| // Only try to verify Stripe payments | |
| $gateway_id = $payment->get_gateway(); | |
| if (empty($gateway_id)) { | |
| // Check membership gateway as fallback | |
| $membership = $payment->get_membership(); | |
| $gateway_id = $membership ? $membership->get_gateway() : ''; | |
| } | |
| if (! in_array($gateway_id, ['stripe', 'stripe-checkout'], true)) { | |
| wp_send_json_success( | |
| [ | |
| 'status' => $payment->get_status(), | |
| 'message' => __('Non-Stripe payment, cannot verify.', 'ultimate-multisite'), | |
| ] | |
| ); | |
| } | |
| // Get the gateway instance and verify | |
| $gateway = wu_get_gateway($gateway_id); | |
| if (! $gateway || ! method_exists($gateway, 'verify_and_complete_payment')) { | |
| wp_send_json_success( | |
| [ | |
| 'status' => $payment->get_status(), | |
| 'message' => __('Gateway does not support verification.', 'ultimate-multisite'), | |
| ] | |
| ); | |
| } | |
| $result = $gateway->verify_and_complete_payment($payment->get_id()); | |
| if (! is_array($result)) { | |
| wp_send_json_error(['message' => __('Unexpected verification result.', 'ultimate-multisite')]); | |
| } | |
| if (! empty($result['success'])) { | |
| wp_send_json_success( | |
| [ | |
| 'status' => $result['status'] ?? 'completed', | |
| 'message' => $result['message'], | |
| ] | |
| ); | |
| } else { | |
| wp_send_json_success( | |
| [ | |
| 'status' => $result['status'] ?? 'pending', | |
| 'message' => $result['message'], | |
| ] | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@inc/managers/class-gateway-manager.php` around lines 596 - 667, The
ajax_check_payment_status handler lacks a nonce check and trusts the return of
$gateway->verify_and_complete_payment; add a nonce verification (use
wp_verify_nonce on a nonce passed in the AJAX request, rejecting with
wp_send_json_error if invalid) before doing any processing in
ajax_check_payment_status, and make the call to
$gateway->verify_and_complete_payment($payment->get_id()) defensive by wrapping
it in try/catch, validating that $result is an array and not a WP_Error (or
null) before accessing $result['success'], and respond with wp_send_json_error
on exceptions/invalid return values; ensure you reference
ajax_check_payment_status, $payment, $gateway, verify_and_complete_payment,
$result, wp_verify_nonce and wp_send_json_error when implementing these checks.
| $gateway = wu_get_gateway($gateway_id); | ||
|
|
||
| if (! $gateway || ! method_exists($gateway, 'verify_and_complete_payment')) { | ||
| wu_log_add('stripe', sprintf('Scheduled payment verification: Gateway %s not found or does not support verification', $gateway_id), LogLevel::WARNING); | ||
| return; | ||
| } | ||
|
|
||
| $result = $gateway->verify_and_complete_payment($payment_id); | ||
|
|
||
| wu_log_add( | ||
| 'stripe', | ||
| sprintf( | ||
| 'Scheduled payment verification for payment %d: %s - %s', | ||
| $payment_id, | ||
| $result['success'] ? 'SUCCESS' : 'PENDING', | ||
| $result['message'] | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Same unchecked result type issue as the AJAX handler.
Line 726 calls verify_and_complete_payment and line 733 accesses $result['success'] without checking whether $result is an array. Apply the same defensive check here.
Proposed fix
$result = $gateway->verify_and_complete_payment($payment_id);
+ if (! is_array($result)) {
+ wu_log_add('stripe', sprintf('Scheduled payment verification for payment %d: unexpected result type', $payment_id), LogLevel::WARNING);
+ return;
+ }
+
wu_log_add(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $gateway = wu_get_gateway($gateway_id); | |
| if (! $gateway || ! method_exists($gateway, 'verify_and_complete_payment')) { | |
| wu_log_add('stripe', sprintf('Scheduled payment verification: Gateway %s not found or does not support verification', $gateway_id), LogLevel::WARNING); | |
| return; | |
| } | |
| $result = $gateway->verify_and_complete_payment($payment_id); | |
| wu_log_add( | |
| 'stripe', | |
| sprintf( | |
| 'Scheduled payment verification for payment %d: %s - %s', | |
| $payment_id, | |
| $result['success'] ? 'SUCCESS' : 'PENDING', | |
| $result['message'] | |
| ) | |
| ); | |
| $gateway = wu_get_gateway($gateway_id); | |
| if (! $gateway || ! method_exists($gateway, 'verify_and_complete_payment')) { | |
| wu_log_add('stripe', sprintf('Scheduled payment verification: Gateway %s not found or does not support verification', $gateway_id), LogLevel::WARNING); | |
| return; | |
| } | |
| $result = $gateway->verify_and_complete_payment($payment_id); | |
| if (! is_array($result)) { | |
| wu_log_add('stripe', sprintf('Scheduled payment verification for payment %d: unexpected result type', $payment_id), LogLevel::WARNING); | |
| return; | |
| } | |
| wu_log_add( | |
| 'stripe', | |
| sprintf( | |
| 'Scheduled payment verification for payment %d: %s - %s', | |
| $payment_id, | |
| $result['success'] ? 'SUCCESS' : 'PENDING', | |
| $result['message'] | |
| ) | |
| ); |
🤖 Prompt for AI Agents
In `@inc/managers/class-gateway-manager.php` around lines 719 - 736, The code
calls $gateway->verify_and_complete_payment($payment_id) and immediately indexes
$result['success'] and $result['message']; add a defensive check after the call
to ensure $result is an array (and contains the expected keys) before using
those indexes — e.g., verify is_array($result) && array_key_exists('success',
$result) && array_key_exists('message', $result); if the check fails, call
wu_log_add (same 'stripe' logger) with a warning about an unexpected response
from verify_and_complete_payment and return early (or set a safe default result)
so subsequent logging/processing does not throw notices.
| $renewal_date = new \DateTime(); | ||
| $renewal_date->setTimestamp($end_timestamp); | ||
| $renewal_date->setTime(23, 59, 59); | ||
|
|
||
| $stripe_estimated_charge_timestamp = $end_timestamp + (2 * HOUR_IN_SECONDS); | ||
|
|
||
| if ($stripe_estimated_charge_timestamp > $renewal_date->getTimestamp()) { | ||
| $renewal_date->setTimestamp($stripe_estimated_charge_timestamp); | ||
| } | ||
|
|
||
| $expiration = $renewal_date->format('Y-m-d H:i:s'); |
There was a problem hiding this comment.
Expiration may differ from webhook handler due to missing timezone.
new \DateTime() uses PHP's default timezone, while the webhook handler likely uses WordPress's configured timezone (e.g., via wp_timezone()). If they differ, setTime(23, 59, 59) and the subsequent comparison will produce a different expiration than the real handler, potentially causing flaky assertions in verify-stripe-renewal-results.php.
Suggested fix
- $renewal_date = new \DateTime();
+ $renewal_date = new \DateTime('now', wp_timezone());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $renewal_date = new \DateTime(); | |
| $renewal_date->setTimestamp($end_timestamp); | |
| $renewal_date->setTime(23, 59, 59); | |
| $stripe_estimated_charge_timestamp = $end_timestamp + (2 * HOUR_IN_SECONDS); | |
| if ($stripe_estimated_charge_timestamp > $renewal_date->getTimestamp()) { | |
| $renewal_date->setTimestamp($stripe_estimated_charge_timestamp); | |
| } | |
| $expiration = $renewal_date->format('Y-m-d H:i:s'); | |
| $renewal_date = new \DateTime('now', wp_timezone()); | |
| $renewal_date->setTimestamp($end_timestamp); | |
| $renewal_date->setTime(23, 59, 59); | |
| $stripe_estimated_charge_timestamp = $end_timestamp + (2 * HOUR_IN_SECONDS); | |
| if ($stripe_estimated_charge_timestamp > $renewal_date->getTimestamp()) { | |
| $renewal_date->setTimestamp($stripe_estimated_charge_timestamp); | |
| } | |
| $expiration = $renewal_date->format('Y-m-d H:i:s'); |
🤖 Prompt for AI Agents
In `@tests/e2e/cypress/fixtures/process-stripe-renewal.php` around lines 78 - 88,
The expiration calculation uses new \DateTime() which may use a different
timezone than WordPress; change the creation of $renewal_date to use WP's
timezone (e.g., new \DateTime('now', wp_timezone()) or DateTimeImmutable with
wp_timezone()), then continue using $renewal_date->setTimestamp($end_timestamp)
and ->setTime(23,59,59) so the comparison with
$stripe_estimated_charge_timestamp and final $renewal_date->format('Y-m-d
H:i:s') happen in the same timezone as the webhook handler.
| // 7. Get invoice total (convert from Stripe cents) | ||
| $currency_multiplier = function_exists('wu_stripe_get_currency_multiplier') | ||
| ? wu_stripe_get_currency_multiplier() | ||
| : 100; | ||
|
|
||
| $total = $renewal_invoice->amount_paid / $currency_multiplier; |
There was a problem hiding this comment.
Currency multiplier ignores the invoice's actual currency.
wu_stripe_get_currency_multiplier() is called without arguments, defaulting to USD (divisor 100). If the invoice currency is a zero-decimal currency (e.g., JPY), the total will be divided by 100 incorrectly. Pass the actual currency from the invoice:
Suggested fix
$currency_multiplier = function_exists('wu_stripe_get_currency_multiplier')
- ? wu_stripe_get_currency_multiplier()
+ ? wu_stripe_get_currency_multiplier(strtoupper($renewal_invoice->currency))
: 100;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 7. Get invoice total (convert from Stripe cents) | |
| $currency_multiplier = function_exists('wu_stripe_get_currency_multiplier') | |
| ? wu_stripe_get_currency_multiplier() | |
| : 100; | |
| $total = $renewal_invoice->amount_paid / $currency_multiplier; | |
| // 7. Get invoice total (convert from Stripe cents) | |
| $currency_multiplier = function_exists('wu_stripe_get_currency_multiplier') | |
| ? wu_stripe_get_currency_multiplier(strtoupper($renewal_invoice->currency)) | |
| : 100; | |
| $total = $renewal_invoice->amount_paid / $currency_multiplier; |
🤖 Prompt for AI Agents
In `@tests/e2e/cypress/fixtures/process-stripe-renewal.php` around lines 100 -
105, The currency multiplier currently uses wu_stripe_get_currency_multiplier()
with no args causing USD (100) to be assumed; update the logic that computes
$currency_multiplier to pass the invoice's currency (from
$renewal_invoice->currency) into wu_stripe_get_currency_multiplier(...) and use
that returned divisor when computing $total so zero-decimal currencies (e.g.,
JPY) are handled correctly, preserving the existing fallback behavior if the
function does not exist or returns null.
| cy.exec( | ||
| `npx wp-env run tests-cli wp eval-file /var/www/html/wp-content/plugins/ultimate-multisite/tests/e2e/cypress/fixtures/setup-stripe-gateway.php '${pkKey}' '${skKey}'`, | ||
| { timeout: 60000 } | ||
| ).then((result) => { | ||
| const data = JSON.parse(result.stdout.trim()); | ||
| cy.log(`Stripe setup: ${JSON.stringify(data)}`); | ||
| expect(data.success).to.equal(true); | ||
| }); |
There was a problem hiding this comment.
Shell injection risk: env var values interpolated directly into shell command.
pkKey and skKey are string-interpolated into the cy.exec() shell command without escaping. While Stripe test keys contain only safe characters in practice, this is a bad pattern that could break or be exploited if the values ever contain shell metacharacters like ', $, or ;.
Consider passing them as environment variables to the exec call instead:
Proposed fix
- cy.exec(
- `npx wp-env run tests-cli wp eval-file /var/www/html/wp-content/plugins/ultimate-multisite/tests/e2e/cypress/fixtures/setup-stripe-gateway.php '${pkKey}' '${skKey}'`,
- { timeout: 60000 }
+ cy.exec(
+ `npx wp-env run tests-cli wp eval-file /var/www/html/wp-content/plugins/ultimate-multisite/tests/e2e/cypress/fixtures/setup-stripe-gateway.php`,
+ { timeout: 60000, env: { STRIPE_PK: pkKey, STRIPE_SK: skKey } }Note: This would require the PHP fixture to read from env vars instead of $args. If that's not feasible, at minimum validate that the keys match the expected pk_test_/sk_test_ format before interpolation.
🤖 Prompt for AI Agents
In `@tests/e2e/cypress/integration/030-stripe-checkout-flow.spec.js` around lines
29 - 36, The cy.exec call in the test interpolates pkKey and skKey directly into
a shell command (in 030-stripe-checkout-flow.spec.js) which creates a
shell-injection risk; fix by changing the exec invocation to pass the keys via
the env option (e.g. cy.exec(command, { env: { STRIPE_PK: pkKey, STRIPE_SK:
skKey }, timeout: 60000 })) and update the PHP fixture (currently reading $args)
to read from those environment variables, or if you cannot change the fixture,
validate/sanitize pkKey and skKey before interpolation using a strict regex that
enforces the expected pk_test_ / sk_test_ formats and reject/throw on mismatch.
| cy.get("body").then(($body) => { | ||
| if ($body.find("#field-billing_zip_code").length > 0) { | ||
| $body.find("#field-billing_zip_code").val("94105"); | ||
| } else if ($body.find('[name="billing_address[billing_zip_code]"]').length > 0) { | ||
| $body.find('[name="billing_address[billing_zip_code]"]').val("94105"); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Setting .val() directly won't trigger Vue/JS change events.
jQuery .val("94105") sets the DOM value but doesn't fire input or change events. If the checkout form relies on Vue reactivity (which it appears to, given the v-model patterns), the value may not propagate to the form state. Consider using Cypress .type() or dispatching an input event after setting the value.
Proposed alternative
- cy.get("body").then(($body) => {
- if ($body.find("#field-billing_zip_code").length > 0) {
- $body.find("#field-billing_zip_code").val("94105");
- } else if ($body.find('[name="billing_address[billing_zip_code]"]').length > 0) {
- $body.find('[name="billing_address[billing_zip_code]"]').val("94105");
- }
- });
+ cy.get("body").then(($body) => {
+ const zipSelector = $body.find("#field-billing_zip_code").length > 0
+ ? "#field-billing_zip_code"
+ : '[name="billing_address[billing_zip_code]"]';
+ if ($body.find(zipSelector).length > 0) {
+ cy.get(zipSelector).clear().type("94105");
+ }
+ });🤖 Prompt for AI Agents
In `@tests/e2e/cypress/integration/030-stripe-checkout-flow.spec.js` around lines
86 - 92, The test directly sets inputs using jQuery .val(), which won't trigger
Vue reactivity for fields like "#field-billing_zip_code" or
'[name="billing_address[billing_zip_code]"]'; update the test to interact with
the inputs in a way that fires events (e.g., use Cypress .type(...) on
cy.get("#field-billing_zip_code") /
cy.get('[name="billing_address[billing_zip_code]"]') or, if keeping direct value
assignment, dispatch an 'input'/'change' event after setting the value so the
component's v-model updates accordingly; ensure you apply this change in the
block that currently inspects $body.find(...) and sets .val("94105").
| cy.exec( | ||
| `npx wp-env run tests-cli wp eval-file ${CONTAINER_FIXTURES}/setup-stripe-gateway.php '${pkKey}' '${skKey}'`, | ||
| { timeout: 60000 } | ||
| ).then((result) => { | ||
| const data = JSON.parse(result.stdout.trim()); | ||
| expect(data.success).to.equal(true); | ||
| }); |
There was a problem hiding this comment.
Same shell injection pattern as the checkout test — interpolating secrets into shell strings.
Same concern as 030-stripe-checkout-flow.spec.js. The skKey is also reused in subsequent cy.exec calls (lines 42, 76, 96, 150) with data from Stripe API responses (setupData.test_clock_id, etc.). While these values are controlled, prefer a safer interpolation pattern.
🤖 Prompt for AI Agents
In `@tests/e2e/cypress/integration/040-stripe-renewal-flow.spec.js` around lines
28 - 34, The cy.exec calls interpolate secrets (pkKey, skKey and later values
like setupData.test_clock_id) directly into shell strings; instead pass secrets
via the cy.exec options.env object (e.g., call cy.exec(`npx wp-env run tests-cli
wp eval-file ${CONTAINER_FIXTURES}/setup-stripe-gateway.php`, { timeout: 60000,
env: { PK_KEY: pkKey, SK_KEY: skKey } })) and update the PHP helper
(setup-stripe-gateway.php) to read these via getenv('PK_KEY')/getenv('SK_KEY');
do the same for subsequent cy.exec invocations that use setupData.test_clock_id
(pass TEST_CLOCK_ID in env) rather than interpolating into the command string so
no secrets/IDs are embedded in the shell command.
1 participant
Overview
This PR integrates a secure proxy server architecture for Stripe Connect OAuth flow, eliminating the need to expose platform credentials in the distributed Ultimate Multisite plugin code.
Security Improvements
✅ No credentials in distributed code - Platform Client ID and Secret Keys never leave the proxy server at ultimatemultisite.com
✅ Encrypted communication - OAuth codes encrypted (AES-256-CBC) during transmission
✅ Centralized control - Rotate credentials without updating plugin
✅ Usage tracking - Proxy database tracks all connected sites
Changes
Base Stripe Gateway (
inc/gateways/class-base-stripe-gateway.php)New Methods:
get_proxy_url()- Returns proxy server URL (filterable viawu_stripe_connect_proxy_url)get_business_data()- Provides site info for Stripe Connect form prefillUpdated Methods:
get_connect_authorization_url()- Now calls proxy/oauth/initendpointhandle_oauth_callbacks()- Handles encrypted codes from proxy (wcs_stripe_code,wcs_stripe_stateparameters)exchange_code_for_keys()- Calls proxy/oauth/keysendpoint (replacesprocess_oauth_callback())handle_disconnect()- Notifies proxy server when disconnectingRemoved Methods:
get_platform_client_id()- Credentials now on proxyget_platform_secret_key()- Credentials now on proxySecurity Fixes:
$_GETinput variablesStripe Gateway (
inc/gateways/class-stripe-gateway.php)Tests
test_oauth_authorization_url_generation()to mock proxy responsetest_oauth_authorization_url_requires_client_id()to test proxy failureArchitecture Flow
Proxy Server
The proxy server is now available at: https://github.com/superdav42/stripe-connect-proxy
Features:
Deployment Requirements
https://ultimatemultisite.com/wp-json/stripe-connect/v1/oauth/redirectSee proxy repository INTEGRATION.md for detailed deployment instructions.
Backwards Compatibility
✅ Fully backwards compatible with existing direct API key mode
✅ Sites with existing OAuth connections continue to work
✅ New OAuth connections automatically use proxy server
Testing
# Run OAuth tests vendor/bin/phpunit --filter Stripe_OAuth --testdoxAll tests passing with no regressions.
🤖 Generated with Claude Code
Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com
Summary by CodeRabbit
New Features
Tests
Bug Fixes
Chores