Fraud Protection: Add report flow with session ID persistence and order event tracking#34
Conversation
src/ApiClient.php
Outdated
| 'context' => $payload, | ||
| ) | ||
| ); | ||
| if ( self::REPORT_ENDPOINT === $path ) { |
There was a problem hiding this comment.
Temporary fix till the data format is discussed at p1773255739721119-slack-C0A3G51RSBW
There was a problem hiding this comment.
I know this is a temporary fix, but I'm wondering if we should move the body construction into verify() and report() instead of branching on the endpoint path here. make_request() is a low-level HTTP method, it probably shouldn't know about the semantic differences between /verify and /report payloads. If we add a third endpoint later, this becomes a growing if/else.
42af872 to
1ff2977
Compare
luizreis
left a comment
There was a problem hiding this comment.
The overall approach is sound, and the session ID persistence handles the different checkout flows well.
I left some comments. The main ones are around the hook-as-API pattern (I think we should expose a public method instead) and the array_merge ordering in ApiClient. The rest are minor.
src/ApiClient.php
Outdated
| 'context' => $payload, | ||
| ) | ||
| ); | ||
| if ( self::REPORT_ENDPOINT === $path ) { |
There was a problem hiding this comment.
I know this is a temporary fix, but I'm wondering if we should move the body construction into verify() and report() instead of branching on the endpoint path here. make_request() is a low-level HTTP method, it probably shouldn't know about the semantic differences between /verify and /report payloads. If we add a third endpoint later, this becomes a growing if/else.
f54b179 to
1f2ff13
Compare
125aff3 to
7ca47c2
Compare
luizreis
left a comment
There was a problem hiding this comment.
Looks good and tests well! I left some comments, but none blocking, so feel free to address at your discretion and ![]()
| $api_client = new \Automattic\WooCommerce\FraudProtection\ApiClient(); | ||
|
|
||
| $order_events_tracker = new \Automattic\WooCommerce\FraudProtection\OrderEventsTracker(); | ||
| $order_events_tracker->init( $api_client ); |
There was a problem hiding this comment.
Just noting we're creating new instances of these per call. It's probably okay to keep it like this, though, as using the bootstrapped instances would be a bit more complex.
I wonder if we should add a feature is enabled check before registering this method (or as part of it), though, to avoid running any code if the feature is disabled.
There was a problem hiding this comment.
Added feature_enabled check with 5521d65
| $this->decision_handler | ||
| ->method( 'apply_decision' ) | ||
| ->willReturn( ApiClient::DECISION_ALLOW ); | ||
| } |
There was a problem hiding this comment.
Nit: Let's add a blank line in between the brace and comment below.
src/ApiClient.php
Outdated
| $payload, | ||
| array( | ||
| 'session_id' => $session_id, | ||
| 'private_key' => '', // Woo will not use private keys for now. |
There was a problem hiding this comment.
Nit: Not really related to this PR, but we could probably drop this line while we're changing the payload assembly.
There was a problem hiding this comment.
Fixed with 371f9f5
Had to remove a test that checked if the private_key would be overwritten by 3rd party calls.
src/ApiClient.php
Outdated
| $blog_id = $this->get_blog_id(); | ||
| if ( ! $blog_id ) { | ||
| FraudProtectionController::log( | ||
| 'error', | ||
| 'Jetpack blog ID not found. Is the site connected to WordPress.com? Failing open with "allow" decision.' | ||
| ); | ||
| return self::DECISION_ALLOW; | ||
| } | ||
| $payload['blog_id'] = $blog_id; |
There was a problem hiding this comment.
This was mostly in make_request to check whether the Jetpack connection was set up. I think report would also benefit from it.
We don't need to include blog_id in the payload anymore (for any of the calls), so what do you think of removing the payload code but keeping the blog ID Jetpack connection check in make_request?
make_request already validates class_exists(Jetpack_Connection_Client::class) and we could add the get_blog_id() check there too, so both endpoints get the validation automatically.
src/OrderEventsTracker.php
Outdated
| /** | ||
| * Register hooks. | ||
| */ | ||
| public function register(): void {} |
There was a problem hiding this comment.
Nit: This class does not register any hooks, could we remove this method?
| ->with( | ||
| 'bb-session-123', | ||
| array( | ||
| 'label' => 'bad', |
There was a problem hiding this comment.
Nit: We're using the raw value here (and in other tests below). Could we use the new constants everywhere instead?
…n ID persistence This update introduces the SessionVerifier to the FraudProtectionController, allowing it to handle session verification and persist the Blackbox session ID to order meta. The SessionVerifier now registers a hook to copy the session ID from the WC session to order meta upon order creation, ensuring that session data is accurately tracked across different checkout flows. Additionally, tests have been added to verify the correct functionality of session ID persistence and its integration with the order creation process.
… hooks This update introduces the OrderEventsTracker to the FraudProtectionController, allowing for enhanced tracking of order events. The OrderEventsTracker is initialized and registered within the controller, ensuring it integrates seamlessly with the existing fraud protection mechanisms. Additionally, the necessary file inclusions have been added to support this new functionality.
This update introduces the `on_fraud_protection_report` method to the `OrderEventsTracker`, allowing 3rd party plugins to report events to the Blackbox API. The `register` method now hooks into `woocommerce_fraud_protection_report`, and the `on_order_refunded` method has been updated to handle exceptions and log errors. Tests have been added to ensure correct functionality and error handling for the new reporting mechanism.
…rt endpoint This update modifies the ApiClient to differentiate the payload structure when the REPORT_ENDPOINT is used. It now directly merges the payload into the body for this specific endpoint, while maintaining the previous structure for other endpoints. Additionally, the corresponding unit tests have been updated to reflect these changes, ensuring the correct keys are asserted in the captured body.
…rt flow on their side as we can't differenciate between a manual regular refund vs a automatic refund caused by a fraudulent purchase.
…ort` action hook for 3rd-party plugins. This change allows payment gateways to report events to the Blackbox API, enhancing the correlation of outcomes with the original fraud-check session.
This update adds a validation check in the on_fraud_protection_report method of the OrderEventsTracker class to ensure that only 'good' or 'bad' statuses are processed. If an invalid status is provided, a warning is logged, and the report is skipped. Additionally, a new unit test has been added to verify this behavior. Minor whitespace adjustments were also made in the SessionVerifier class.
…private_key This update modifies the report method in the ApiClient to ensure that the session_id and private_key cannot be overridden by the payload. A new unit test has been added to verify this behavior, ensuring that the correct values are maintained in the request body. Additionally, minor adjustments were made to the payload merging logic for clarity.
This update introduces a warning log in the OrderEventsTracker class when the session ID is missing from the order meta. This enhancement improves the visibility of potential issues during the Blackbox API reporting process, ensuring that developers are alerted when session IDs are not properly set.
This update introduces a new global function `wc_fraud_protection_report()` for reporting order events to the Blackbox API, replacing the previous action hook. The `OrderEventsTracker` class has been updated to handle the reporting directly, and associated unit tests have been modified to reflect these changes. This refactor simplifies the reporting process for 3rd-party plugins and enhances code clarity.
This update modifies the `wc_fraud_protection_report` function and the `fraud_protection_report` method in the `OrderEventsTracker` class to include a new parameter for the event source, utilizing constants from the `ApiClient`. Additionally, the implementation now validates the source against predefined constants, logging warnings for invalid sources. Unit tests have been updated to reflect these changes, ensuring robust coverage for the new functionality.
Compat layers register outside FraudProtectionController to keep the boundary between first-party components and third-party integrations clean. Making feature_is_enabled() static lets them check the feature flag without being wired into the controller.
…uns for both verify and report endpoints
…cker dependency from FraudProtectionController This update modifies the error handling in the `ApiClient` class to return a `WP_Error` when the blog ID is not found, improving clarity. Additionally, the `OrderEventsTracker` dependency has been removed from the `FraudProtectionController`, streamlining the class and eliminating unnecessary code.
7ca47c2 to
49114b4
Compare
Summary
Adds the report flow so Blackbox can correlate order outcomes (payment failures, chargebacks) back to the original fraud-check session.
SessionVerifier): After a successful verification, the Blackbox session ID is saved to order meta (_wc_fraud_protection_session_id). For blocks checkout / pay-for-order flows the order already exists, so it's written directly. For shortcode checkout (where order_id = 0 at verification time), the session ID is stored in the WC session and copied to order meta via thewoocommerce_checkout_order_createdhook.OrderEventsTracker): Exposes awoocommerce_fraud_protection_reportaction hook so 3rd-party plugins (e.g. payment gateways) can report events (e.g. payment failures) to the Blackbox report endpoint with the stored session ID.ApiClient): The/reportendpoint now sends the payload spread at the top level (alongsidesession_idandprivate_key), while/verifycontinues to wrap it undercontext.SessionVerifierandOrderEventsTrackerare added toFraudProtectionControllerand registered inon_init().All reporting is fire-and-forget: exceptions are caught and logged but never affect the order flow.
Payment gateways will need to trigger the
woocommerce_fraud_protection_reportwhen they have something to report manually because there is no way for our plugin to know if a report is considered good or bad by hooking to the core order note created and refund hooks and there is not way to hook into a refund flow because they are handled by each payment gateway.Test plan
Session ID persistence
_wc_fraud_protection_session_idin its meta (check via WP admin → order → custom fields, orwp post meta get <order_id> _wc_fraud_protection_session_id).woocommerce_checkout_order_created).3rd-party report hook
woocommerce_fraud_protection_reportaction with an order that has a session ID:source: payment_gateway_eventand the correct label/notes.Report endpoint payload format
/reportrequests sendlabel,source,notes, andblog_idat the top level of the JSON body (not nested undercontext)./verifyrequests still wrap the payload undercontext.Fixes WOOSUBS-1438