fix: preserve original 3DS authentication results - #14294
Longsueynin wants to merge 1 commit into
Conversation
|
Review changes with Changed Files
|
abhisheksharma2411
left a comment
abhisheksharma2411
left a comment
There was a problem hiding this comment.
The regression this fixes is the kind that only shows up in production — an authentication result that exists at confirm and is gone by the time anyone reads the payment back. The test is the right shape for it:
// Sync/capture can report an auth code without repeating the 3DS result.
let connector = json!({method: {"auth_code": "123456"}});
let updated = update_additional_payment_data_with_connector_response_pm_data(Some(stored), Some(connector))asserting the stored authentication_data survives a later connector response that doesn't mention it, across Card, Apple Pay and Google Pay. That's exactly the failure mode — the second response wins and silently drops a field it never carried — and covering all three methods matters because they store under different pointers.
if auth_code.is_none() && authentication_data.is_none() returning early is a good detail too: an authentication failure legitimately has no auth code, so the alternative would have been emitting an empty container on every such response.
I checked what actually lands in the field, since it's untyped, and on this connector it's narrow:
let authentication_data = payment_data.three_d_secure_result
.map(|description| serde_json::json!({ "three_d_secure_result": description }));— a human-readable description ("Authentication offered but not used", "Authentication unavailable", "Authentication rejected"). No CAVV, no ECI, no DS transaction id. So nothing sensitive is being newly exposed here, which was my first question given this ends up on the payment response.
That's worth writing down somewhere, because the type won't enforce it. authentication_data: Option<serde_json::Value> on a response-exposed struct is an open container, and the next connector to implement this will copy Worldpay's shape as the precedent. A 3DS result is exactly the place where a cardholder authentication value could plausibly get added "because the connector returned it" — and CAVV/ECI on a payment-retrieve response is a different conversation from a status description. A doc comment on the field saying what belongs in it (connector-reported authentication outcome, not authentication credentials) costs one line and outlives the reviewer.
One wire-format confirmation worth making explicit in the PR body: WalletAdditionalDataForCard carries no skip_serializing_if on any field, so the new key will appear as "authentication_data": null on every response for that struct, not only when populated. That's consistent with its existing siblings (auth_code, issuer_name, issuer_country all behave the same), so it's house style rather than a new problem — but it is a visible response change for integrators diffing payloads, and "no database migration" in the description may read as "no client-visible change" when the response body does gain a key.
trans_status on ExternalAuthenticationDetailsResponse being an enum rather than a string is the right call — that one has a fixed EMV domain and deserves the type.
|
[should-fix] The new |
3 participants
Expose stored EMV
trans_statusand preserve Worldpay XML 3DS results for cards, Apple Pay and Google Pay across confirm/retrieve updates. Adds optional API fields; no database migration.Closes #14293.