Skip to content

Conversation

@diegocurbelo
Copy link
Member

@diegocurbelo diegocurbelo commented Nov 17, 2025

Fixes STRIPE-745
Fixes #<github_issue_id>

Changes proposed in this Pull Request:

This PR improves the OAuth optional verbose logs to include additional data:

  • adds the expected state masked value (or false if not set)
  • logs the final state, code, nonce, and connect_response (all masked) as an error in case of failure

Testing instructions

  1. Enable the Oauth connect verbose logging by adding the following filter:
    add_filter( 'wc_stripe_is_verbose_debug_mode_enabled', '__return_true' );
  2. Disconnect your Stripe account
  3. Go to the Onboard page (WooCommerce > Payments > Stripe)
  4. Click Create or connect a test account instead (do not complete the onboarding yet)
  5. Remove the state transient:
    wp transient delete wcs_stripe_connect_state_test
  6. Select your account and complete the process
  7. Check that there are 2 error logged:
    Screenshot 2025-11-17 at 20 10 07
  8. Verify that the attribute stored_state is present, and that it is false:
    Screenshot 2025-11-17 at 20 10 40

  • Covered with tests (or have a good reason not to test in description ☝️)
  • Tested on mobile (or does not apply)

Changelog entry

  • This Pull Request does not require a changelog entry. (Comment required below)
Changelog Entry Comment

Comment

Post merge

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances OAuth Connect verbose logging to improve debugging capabilities when OAuth validation fails. The changes add the stored state value (masked or 'false' if not set) to error logs and implement consistent logging for both successful and failed connection attempts.

  • Stores the transient state value before validation to include it in error logs
  • Adds stored_state, state, code, and nonce (all masked) to verbose debug logs
  • Implements conditional logging that uses error level for failures and debug level for successes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@diegocurbelo diegocurbelo marked this pull request as ready for review November 17, 2025 23:32
@diegocurbelo diegocurbelo requested review from a team, Mayisha and malithsen and removed request for a team November 17, 2025 23:32
Copy link
Contributor

@Mayisha Mayisha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good and it tests as described 👍

✅ Two errors logged
Screenshot 2025-11-18 at 1 41 42 PM

✅ The attribute stored_state is present, and it is false in the additional context of OAuth: Invalid state received from the WCC server log.

Screenshot 2025-11-18 at 1 43 27 PM

Note
stored_state is not present in the additional context of the OAuth: Account connection failed log. Should we add the data here as well?

Copy link
Contributor

@malithsen malithsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@diegocurbelo
Copy link
Member Author

Note stored_state is not present in the additional context of the OAuth: Account connection failed log. Should we add the data here as well?

I don't think it adds any value.... when stored_state and state don't match, we already log an error before returning the error.

Also, to add it to the OAuth: Account connection failed log, we would need to read the transient again to log it... and in general, each function only logs the data it receives/uses.

@diegocurbelo diegocurbelo merged commit baef416 into develop Nov 19, 2025
40 checks passed
@diegocurbelo diegocurbelo deleted the dev/improve-verbose-ouath-logging branch November 19, 2025 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants