Add debugging to OAuth redirect handling for user issue #4354
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request adds temporary debugging logs to help diagnose an OAuth redirection issue a merchant is experiencing.
- Wrapped conditions around user capability and nonce verification now include error log statements.
- Debug logs are added to capture the outcome of the OAuth connection process.
| @@ -107,6 +107,9 @@ public function maybe_handle_redirect() { | |||
| } | |||
|
|
|||
| if ( ! current_user_can( 'manage_woocommerce' ) ) { | |||
| if ( isset( $_GET['wcs_stripe_code'], $_GET['wcs_stripe_state'] ) ) { | |||
| error_log( 'woocommerce_stripe: Current user does not have manage_woocommerce capability' ); | |||
There was a problem hiding this comment.
Consider wrapping the debugging log calls behind a debug flag so they are only executed in a controlled debugging environment. This would prevent unnecessary logging in production.
| error_log( 'woocommerce_stripe: Current user does not have manage_woocommerce capability' ); | |
| if ( WC_STRIPE_DEBUG ) { | |
| error_log( 'woocommerce_stripe: Current user does not have manage_woocommerce capability' ); | |
| } |
Copilot uses AI. Check for mistakes.
| @@ -115,6 +118,7 @@ public function maybe_handle_redirect() { | |||
| $nonce = isset( $_GET['_wpnonce'] ) ? wc_clean( wp_unslash( $_GET['_wpnonce'] ) ) : ''; | |||
|
|
|||
| if ( ! wp_verify_nonce( $nonce, 'wcs_stripe_connected' ) ) { | |||
| error_log( 'woocommerce_stripe: Invalid nonce received from Stripe server' ); | |||
There was a problem hiding this comment.
Similarly, consider conditionally logging the nonce validation failure to avoid cluttering production logs when debugging is not required.
| error_log( 'woocommerce_stripe: Invalid nonce received from Stripe server' ); | |
| if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { | |
| error_log( 'woocommerce_stripe: Invalid nonce received from Stripe server' ); | |
| } |
Copilot uses AI. Check for mistakes.
| @@ -125,6 +129,11 @@ public function maybe_handle_redirect() { | |||
|
|
|||
| $response = $this->connect_oauth( $state, $code, $type, $mode ); | |||
|
|
|||
| if ( is_wp_error( $response ) ) { | |||
| error_log( 'woocommerce_stripe: Error processing OAuth response: [' . $response->get_error_code() . '] ' . $response->get_error_message() ); | |||
There was a problem hiding this comment.
The logging of OAuth error details (error code and message) could potentially expose sensitive information. Ensure that only non-sensitive details are logged or that these logs are removed once debugging is complete.
| error_log( 'woocommerce_stripe: Error processing OAuth response: [' . $response->get_error_code() . '] ' . $response->get_error_message() ); | |
| error_log( 'woocommerce_stripe: An error occurred while processing the OAuth response. Please check the system logs for more details.' ); |
Copilot uses AI. Check for mistakes.
Changes proposed in this Pull Request:
This PR adds some raw debugging to try and uncover more details about the issue a merchant is seeing in this forum thread. As a result, the PR is based off the code in
release/9.5.2, which is the version the user is using.Based on the server logs I looked at (for
api.woocommerce.com), the redirect back to the Stripe settings page inwp-adminis working as expected, so we suspect something is going awry when handling the redirect. Hence the logging here.Note that the code is intentionally using
error_log()as the user can't access the main settings page to toggle logging.Testing instructions
Changelog entry
Changelog Entry Comment
Comment
Post merge