Use Stripe Checkout Element #300
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>
📝 WalkthroughPre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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.
| add_filter('wu_stripe_application_fee_percentage', function() { | ||
| return 3.5; | ||
| }); |
There was a problem hiding this comment.
Fix code style for multi-line closure.
The CI pipeline is failing due to code style issues. The closure needs proper formatting per the project's coding standards.
- add_filter('wu_stripe_application_fee_percentage', function() {
- return 3.5;
- });
+ add_filter(
+ 'wu_stripe_application_fee_percentage',
+ function () {
+ return 3.5;
+ }
+ );📝 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.
| add_filter('wu_stripe_application_fee_percentage', function() { | |
| return 3.5; | |
| }); | |
| add_filter( | |
| 'wu_stripe_application_fee_percentage', | |
| function () { | |
| return 3.5; | |
| } | |
| ); |
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[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
🤖 Prompt for AI Agents
In tests/WP_Ultimo/Gateways/Stripe_OAuth_E2E_Test.php around lines 63-65, the
anonymous function passed to add_filter is not formatted according to project
style; reformat the multi-line closure so the function keyword has a space
before the parentheses (function ()), place the opening brace on the same line,
put the return statement on its own indented line, and place the closing brace
and ); on their own lines to match the project's PHP coding standards.
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.
| add_filter('pre_http_request', function($preempt, $args, $url) { | ||
| if (strpos($url, '/oauth/init') !== false) { | ||
| return [ | ||
| 'response' => ['code' => 200], | ||
| 'body' => wp_json_encode([ | ||
| 'oauthUrl' => 'https://connect.stripe.com/oauth/authorize?client_id=ca_test123&state=encrypted_state&scope=read_write', | ||
| 'state' => 'test_state_123', | ||
| ]), | ||
| ]; | ||
| } | ||
| return $preempt; | ||
| }, 10, 3); |
There was a problem hiding this comment.
Fix closure formatting to pass CI checks.
The multi-line closures violate project coding standards.
- add_filter('pre_http_request', function($preempt, $args, $url) {
+ add_filter(
+ 'pre_http_request',
+ function ($preempt, $args, $url) {
if (strpos($url, '/oauth/init') !== false) {
return [
- 'response' => ['code' => 200],
- 'body' => wp_json_encode([
- 'oauthUrl' => 'https://connect.stripe.com/oauth/authorize?client_id=ca_test123&state=encrypted_state&scope=read_write',
- 'state' => 'test_state_123',
- ]),
+ 'response' => ['code' => 200],
+ 'body' => wp_json_encode(
+ [
+ 'oauthUrl' => 'https://connect.stripe.com/oauth/authorize?client_id=ca_test123&state=encrypted_state&scope=read_write',
+ 'state' => 'test_state_123',
+ ]
+ ),
];
}
return $preempt;
- }, 10, 3);
+ },
+ 10,
+ 3
+ );Also applies to: 130-135
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[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
🪛 PHPMD (2.15.0)
101-101: Avoid unused parameters such as '$args'. (undefined)
(UnusedFormalParameter)
🤖 Prompt for AI Agents
In tests/WP_Ultimo/Gateways/Stripe_OAuth_Test.php around lines 101-112 (and also
apply same change to 130-135), the multi-line anonymous function passed to
add_filter violates project coding standards; replace the multi-line closure
with a single-line closure (or a small named function) that performs the same
logic and returns the preempt value—ensure the entire callback fits on one line
and keep the 10, 3 parameters intact so CI linting passes.
- 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.
| public function get_connect_authorization_url(string $state = ''): string { | ||
| $proxy_url = $this->get_proxy_url(); | ||
| $return_url = admin_url('admin.php?page=wu-settings&tab=payment-gateways'); | ||
|
|
||
| // Call proxy to initialize OAuth | ||
| $response = wp_remote_post( | ||
| $proxy_url . '/oauth/init', | ||
| [ | ||
| 'body' => wp_json_encode( | ||
| [ | ||
| 'returnUrl' => $return_url, | ||
| 'businessData' => $this->get_business_data(), | ||
| 'testMode' => $this->test_mode, | ||
| ] | ||
| ), | ||
| 'headers' => [ | ||
| 'Content-Type' => 'application/json', | ||
| ], | ||
| 'timeout' => 30, | ||
| ] | ||
| ); | ||
|
|
||
| if (is_wp_error($response)) { | ||
| return ''; | ||
| } | ||
|
|
||
| $data = json_decode(wp_remote_retrieve_body($response), true); | ||
|
|
||
| if (empty($data['oauthUrl'])) { | ||
| return ''; | ||
| } | ||
|
|
||
| // Store state for verification | ||
| update_option('wu_stripe_oauth_state', $data['state'], false); | ||
|
|
||
| return $data['oauthUrl']; | ||
| } |
There was a problem hiding this comment.
Harden handling of proxy /oauth/init responses and unused $state
Two points here:
-
Potential fatal on invalid JSON or unexpected body
$data = json_decode(..., true);is used immediately as an array inempty($data['oauthUrl'])and later in$data['state'].- If the proxy (or an upstream layer/WAF) ever returns non‑JSON or an unexpected payload, this can become a fatal error on PHP 8+ instead of a graceful failure.
Consider guarding the decoded body and verifying required keys:
- $data = json_decode(wp_remote_retrieve_body($response), true); - - if (empty($data['oauthUrl'])) { - return ''; - } - - // Store state for verification - update_option('wu_stripe_oauth_state', $data['state'], false); + $body = wp_remote_retrieve_body($response); + $data = json_decode($body, true); + + 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)and treat non‑2xx codes as failure before decoding. -
Unused
$stateparameter$stateis intentionally unused for BC, but PHPMD flags it. To keep tools quiet without changing behavior, you can either add an inline ignore (// @phpstan-ignore-next-line///phpmd:ignore UnusedFormalParameter) or explicitlyunset($state);with a short comment.
🧰 Tools
🪛 PHPMD (2.15.0)
317-317: Avoid unused parameters such as '$state'. (undefined)
(UnusedFormalParameter)
| 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.
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
✏️ Tip: You can customize this high-level summary in your review settings.