From 53d57dc5c26fdb5f771aa9c616aa8e2942842390 Mon Sep 17 00:00:00 2001 From: Transaction Science Date: Sat, 17 Oct 2026 04:27:27 -0400 Subject: [PATCH] op-3ds2: an unconverted foreign amount is not under an EUR SCA cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `low_value_eligible` compared `intent.amount.minor_units` against the RTS Article 16 caps of EUR 30 / EUR 100 whatever currency the intent was denominated in, under this comment: // RTS 16 thresholds. We treat all non-EUR currencies conservatively // by checking the minor-unit value against EUR limits assuming a // 1:1 EUR equivalency; production deployments convert via op-fx. The substitution is not conservative. It is conservative only for currencies worth *less* than EUR per minor unit. A GBP 30.00 payment (minor_units 3000, about EUR 35) and a CHF 30.00 payment (about EUR 32) each satisfy `3000 <= 3000`, so `evaluate()` pushed `LowValueTransaction` and the orchestrator authorised with no SCA challenge above the Article 16 ceiling. Five such payments run to about EUR 175 under a cap documented as EUR 100. The FX conversion was not computed, and the uncomputed result was treated as a value inside the cap — while `op_core::Money` carries the currency, so the information needed to refuse was present and discarded. The second clause compounded it by adding a possibly-non-EUR figure to `cumulative_low_value_24h_eur_minor`, a field documented "in EUR minor units", so the 24 h cap was summed across mixed units. The same hole sat in the TRA branch, which the sweep that found this did not report: `FraudRateBracket::tra_cap_minor_units` is documented "the TRA cap (EUR minor units)" and was compared to the raw minor-unit count too, at EUR 100 / 250 / 500. GBP 450.00 is about EUR 527 and passed the UpTo1Bp cap of EUR 500. Neither was visible because every fixture in the file is built by an `eur()` helper, so no test ever presented a non-EUR amount. The fix makes the unknown unusable rather than permissive: - `ExemptionContext::eur_equivalent_minor: Option` carries the amount converted to EUR minor units, supplied by the caller that holds the rate (the comment already pointed at `op-fx`, which is not even a dependency of this crate). - `amount_in_eur_minor()` returns the EUR figure for an EUR amount, the supplied conversion for anything else, and `None` when there is none. Both EUR-denominated caps now take that `Option` and decline when it is absent: an unconverted figure is not a figure below a cap. - The cumulative clause adds two EUR figures rather than two different units, and saturates. The total is caller-supplied, and a plain `+` overflowing i64 wraps negative in release and so slips under the cap, while panicking in debug. `ExemptionContext` derives `Default` and every construction site in the repo uses `..Default::default()`, so the new field is additive and defaults to `None` — the direction that withholds the exemption. Tests cover both caps on both sides: an unconverted GBP 30.00 is not low-value, the same intent with a supplied EUR 35.00 is not and with EUR 25.00 is, an EUR amount still needs no conversion, an unconverted GBP 450.00 is not under the TRA cap while EUR 460.00 is, and a 24 h total of i64::MAX does not slip under EUR 100. Co-Authored-By: Claude Opus 5 --- openpay/crates/op-3ds2/src/exemption.rs | 198 ++++++++++++++++++++++-- 1 file changed, 186 insertions(+), 12 deletions(-) diff --git a/openpay/crates/op-3ds2/src/exemption.rs b/openpay/crates/op-3ds2/src/exemption.rs index 7ab1ed04d..b96ca6640 100644 --- a/openpay/crates/op-3ds2/src/exemption.rs +++ b/openpay/crates/op-3ds2/src/exemption.rs @@ -40,7 +40,7 @@ //! reflects this. use chrono::{DateTime, Utc}; -use op_core::Money; +use op_core::{Currency, Money}; use serde::{Deserialize, Serialize}; use crate::auth_response::TransactionStatus; @@ -202,6 +202,17 @@ pub struct ExemptionContext { /// Cumulative low-value charges in the trailing 24 h, in EUR /// minor units, for this cardholder. pub cumulative_low_value_24h_eur_minor: i64, + /// The intent's amount converted to **EUR minor units**, when the + /// caller holds an FX rate (see `op-fx`). + /// + /// The RTS caps this module enforces — Article 16 low-value and + /// Article 18 TRA — are denominated in EUR, so a non-EUR amount is + /// not comparable to them until it is converted. `None` means no + /// conversion was supplied, which for a non-EUR amount makes those + /// two exemptions ineligible rather than eligible: an unconverted + /// figure is not a figure below the cap. EUR amounts need nothing + /// here. + pub eur_equivalent_minor: Option, /// Count of consecutive low-value-exempt payments without SCA. pub consecutive_low_value_count: u32, /// Requestor's PSP-certified fraud-rate bracket. @@ -295,8 +306,12 @@ pub fn evaluate(intent: &PaymentIntent, ctx: &ExemptionContext) -> Vec &'static str { } } +/// The intent's amount in EUR minor units, or `None` when that is not +/// knowable here. +/// +/// An EUR amount is already in EUR minor units. Any other currency +/// needs [`ExemptionContext::eur_equivalent_minor`]; without it the +/// amount cannot be compared to an EUR-denominated RTS cap. +/// +/// This used to substitute the raw minor-unit figure 1:1 and call that +/// conservative. It is conservative only for currencies worth *less* +/// than EUR per minor unit: GBP 30.00 and CHF 30.00 both satisfied +/// `3000 <= 3000` while exceeding the EUR 30 Article 16 ceiling, so the +/// uncomputed conversion was being treated as a result inside the cap. +/// `Money` carries the currency, so the information needed to refuse +/// was present and discarded. +fn amount_in_eur_minor(intent: &PaymentIntent, ctx: &ExemptionContext) -> Option { + if intent.amount.currency == Currency::EUR { + Some(intent.amount.minor_units) + } else { + ctx.eur_equivalent_minor + } +} + fn low_value_eligible(intent: &PaymentIntent, ctx: &ExemptionContext) -> bool { - // RTS 16 thresholds. We treat all non-EUR currencies conservatively - // by checking the minor-unit value against EUR limits assuming a - // 1:1 EUR equivalency; production deployments convert via op-fx. - let single_cap = 30_00; - let cumulative_cap = 100_00; - let consecutive_cap = 5; - intent.amount.minor_units <= single_cap - && ctx.cumulative_low_value_24h_eur_minor + intent.amount.minor_units <= cumulative_cap - && ctx.consecutive_low_value_count < consecutive_cap + // RTS Article 16: EUR 30 per transaction, EUR 100 cumulative across + // a rolling 24 h, at most 5 consecutive. All three are EUR figures, + // so an amount that could not be expressed in EUR is not eligible. + let Some(eur_minor) = amount_in_eur_minor(intent, ctx) else { + return false; + }; + const SINGLE_CAP: i64 = 30_00; + const CUMULATIVE_CAP: i64 = 100_00; + const CONSECUTIVE_CAP: u32 = 5; + // saturating: the cumulative total is caller-supplied, and a plain + // `+` overflowing i64 wraps negative in release and so slips under + // the cap, while panicking in debug. Saturation exceeds the cap in + // both profiles. + eur_minor <= SINGLE_CAP + && ctx + .cumulative_low_value_24h_eur_minor + .saturating_add(eur_minor) + <= CUMULATIVE_CAP + && ctx.consecutive_low_value_count < CONSECUTIVE_CAP } const EEA_COUNTRIES: &[&str] = &[ @@ -366,7 +413,6 @@ fn is_eea(country: &str) -> bool { #[cfg(test)] mod tests { use super::*; - use op_core::Currency; fn eur(minor: i64) -> Money { Money::from_minor(minor, Currency::EUR) @@ -545,4 +591,132 @@ mod tests { assert_eq!(FraudRateBracket::UpTo13Bp.tra_cap_minor_units(), Some(100_00)); assert_eq!(FraudRateBracket::AboveTraThreshold.tra_cap_minor_units(), None); } + + // ---- EUR-denominated caps vs non-EUR amounts -------------------- + // + // The RTS Article 16 and Article 18 caps are EUR figures. The + // evaluator used to compare a raw minor-unit count against them + // whatever the currency, calling the 1:1 substitution conservative. + // It is not: GBP and CHF are worth more than EUR per minor unit. + + fn gbp(minor: i64) -> Money { + Money::from_minor(minor, Currency::GBP) + } + + fn is_low_value(ex: &[EligibleExemption]) -> bool { + ex.iter() + .any(|e| matches!(e, EligibleExemption::LowValueTransaction { .. })) + } + + fn is_tra(ex: &[EligibleExemption]) -> bool { + ex.iter() + .any(|e| matches!(e, EligibleExemption::TransactionRiskAnalysis { .. })) + } + + /// GBP 30.00 is about EUR 35, over the Article 16 ceiling, but its + /// minor-unit count is exactly the EUR cap. Without a conversion the + /// evaluator cannot know which, so the exemption is not available. + #[test] + fn an_unconverted_foreign_amount_is_not_low_value() { + let intent = PaymentIntent { + amount: gbp(30_00), + ..base_intent(30_00) + }; + let ex = evaluate(&intent, &base_ctx()); + assert!( + !is_low_value(&ex), + "GBP 30.00 exceeds the EUR 30 cap and no rate was supplied: {ex:?}" + ); + } + + /// With a rate supplied, the converted figure decides it — on both + /// sides of the cap. This is the pair that shows the fix is about + /// knowing the amount, not about refusing foreign currency. + #[test] + fn a_supplied_conversion_decides_the_low_value_cap() { + let intent = PaymentIntent { + amount: gbp(30_00), + ..base_intent(30_00) + }; + + let over = ExemptionContext { + eur_equivalent_minor: Some(35_00), + ..base_ctx() + }; + assert!( + !is_low_value(&evaluate(&intent, &over)), + "EUR 35.00 is over the EUR 30 cap" + ); + + let under = ExemptionContext { + eur_equivalent_minor: Some(25_00), + ..base_ctx() + }; + assert!( + is_low_value(&evaluate(&intent, &under)), + "EUR 25.00 is under the EUR 30 cap" + ); + } + + /// An EUR amount is already in EUR minor units, so it still clears + /// with no conversion supplied. Guards the common path. + #[test] + fn an_eur_amount_needs_no_conversion() { + let ctx = base_ctx(); + assert!(ctx.eur_equivalent_minor.is_none()); + assert!(is_low_value(&evaluate(&base_intent(25_00), &ctx))); + } + + /// The same hole in the TRA branch: `tra_cap_minor_units` is an EUR + /// figure, and GBP 450.00 is about EUR 527 — over the UpTo1Bp cap of + /// EUR 500 while its minor-unit count sits under it. + #[test] + fn an_unconverted_foreign_amount_is_not_under_the_tra_cap() { + let intent = PaymentIntent { + amount: gbp(450_00), + ..base_intent(450_00) + }; + let ctx = ExemptionContext { + fraud_rate_bracket: FraudRateBracket::UpTo1Bp, + tra_score: Some(0.05), + ..Default::default() + }; + assert!( + !is_tra(&evaluate(&intent, &ctx)), + "no rate was supplied, so the EUR 500 cap cannot be applied" + ); + + let converted_over = ExemptionContext { + eur_equivalent_minor: Some(527_00), + ..ctx.clone() + }; + assert!( + !is_tra(&evaluate(&intent, &converted_over)), + "EUR 527 is over the EUR 500 cap" + ); + + let converted_under = ExemptionContext { + eur_equivalent_minor: Some(460_00), + ..ctx + }; + assert!( + is_tra(&evaluate(&intent, &converted_under)), + "EUR 460 is under the EUR 500 cap" + ); + } + + /// The cumulative total is caller-supplied. A plain `+` overflowing + /// i64 wraps negative in release and slips under the cap; saturation + /// exceeds it in both profiles. + #[test] + fn a_cumulative_total_that_would_overflow_does_not_slip_under_the_cap() { + let ctx = ExemptionContext { + cumulative_low_value_24h_eur_minor: i64::MAX, + ..base_ctx() + }; + assert!( + !is_low_value(&evaluate(&base_intent(1_00), &ctx)), + "a 24 h total of i64::MAX is not under EUR 100" + ); + } } -- 2.54.0 (Apple Git-157)