Skip to content

Commit

Permalink
Add min amount validation to server side one time payments (#6665)
Browse files Browse the repository at this point in the history
We already have client side validation for this, but we'd like to enforce
server side too as this makes us more robust.

I've tested this in CODE. In order to do this I had to temporarily remove the
client side validation. The error message we show in this case is quite
generic, but I think in general we don't expect users to hit this as the client
side validation should kick in before the request even makes it to the backend.
  • Loading branch information
tjmw authored Jan 10, 2025
1 parent 932f9e0 commit ff416fa
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 15 deletions.
21 changes: 14 additions & 7 deletions support-payment-api/src/main/scala/model/Currency.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import enumeratum.{CirceEnum, Enum, EnumEntry}

import scala.collection.immutable.IndexedSeq

case class Range(min: BigDecimal, max: BigDecimal) {
def contains(amount: BigDecimal): Boolean = (amount >= min) && (amount <= max)
}

// Models currencies supported by the API
sealed trait Currency extends EnumEntry

Expand All @@ -23,13 +27,16 @@ object Currency extends Enum[Currency] with CirceEnum[Currency] {

case object NZD extends Currency

def exceedsMaxAmount(amount: BigDecimal, currency: Currency): Boolean = {
val maxAmount = currency match {
case AUD => 16000
case USD => 10000
case _ => 2000
}
def isAmountOutOfBounds(amount: BigDecimal, currency: Currency): Boolean = {
// Users can opt in to add 4% to cover the transaction cost
amount > maxAmount * 1.04
val transactionCostAsPercentage = 1.04

val currencyRange = currency match {
case AUD => Range(min = 1, max = 16000 * transactionCostAsPercentage)
case USD => Range(min = 1, max = 10000 * transactionCostAsPercentage)
case _ => Range(min = 1, max = 2000 * transactionCostAsPercentage)
}

!currencyRange.contains(amount)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ trait Paypal {
class PaypalService(config: PaypalConfig)(implicit pool: PaypalThreadPool) extends Paypal with StrictLogging {

def createPayment(createPaypalPaymentData: CreatePaypalPaymentData): PaypalResult[Payment] = {
if (model.Currency.exceedsMaxAmount(createPaypalPaymentData.amount, createPaypalPaymentData.currency)) {
Left(PaypalApiError.fromString("Amount exceeds the maximum allowed ")).toEitherT[Future]
if (model.Currency.isAmountOutOfBounds(createPaypalPaymentData.amount, createPaypalPaymentData.currency)) {
Left(PaypalApiError.fromString("Amount is outside the allowed range ")).toEitherT[Future]
} else {
Either
.catchNonFatal {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ class SingleAccountStripeService(config: StripeAccountConfig)(implicit pool: Str
).asJava

def createCharge(data: LegacyStripeChargeRequest): EitherT[Future, StripeApiError, Charge] = {
if (model.Currency.exceedsMaxAmount(data.paymentData.amount, data.paymentData.currency)) {
Left(StripeApiError.fromString("Amount exceeds the maximum allowed ", Some(config.publicKey))).toEitherT[Future]
if (model.Currency.isAmountOutOfBounds(data.paymentData.amount, data.paymentData.currency)) {
Left(StripeApiError.fromString("Amount is outside the allowed range ", Some(config.publicKey))).toEitherT[Future]
} else {
Future(Charge.create(getChargeParams(data), requestOptions)).attemptT
.bimap(
Expand Down Expand Up @@ -79,8 +79,8 @@ class SingleAccountStripeService(config: StripeAccountConfig)(implicit pool: Str
def createPaymentIntent(
data: StripePaymentIntentRequest.CreatePaymentIntent,
): EitherT[Future, StripeApiError, PaymentIntent] = {
if (model.Currency.exceedsMaxAmount(data.paymentData.amount, data.paymentData.currency)) {
Left(StripeApiError.fromString("Amount exceeds the maximum allowed ", Some(config.publicKey))).toEitherT[Future]
if (model.Currency.isAmountOutOfBounds(data.paymentData.amount, data.paymentData.currency)) {
Left(StripeApiError.fromString("Amount is outside the allowed range ", Some(config.publicKey))).toEitherT[Future]
} else
{
Future {
Expand Down
22 changes: 22 additions & 0 deletions support-payment-api/src/test/scala/model/CurrencyTest.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package model

import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.must.Matchers

class CurrencyTest extends AnyFlatSpec with Matchers {
"exceedsMaxAmount" should "return true if the amount is greater than the allowed range" in {
Currency.isAmountOutOfBounds(2100, Currency.GBP) mustBe true
}

it should "return true if the amount is below the allowed range" in {
Currency.isAmountOutOfBounds(0.5, Currency.GBP) mustBe true
}

it should "return false if the amount is within the allowed range" in {
Currency.isAmountOutOfBounds(100, Currency.GBP) mustBe false
}

it should "allow the amount to be 4% larger than the nominal max to allow for covering transaction cost" in {
Currency.isAmountOutOfBounds(2080, Currency.GBP) mustBe false
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -114,14 +114,14 @@ class PaypalServiceSpec extends AnyFlatSpec with Matchers with MockitoSugar with
it should "return an error if the payment amount exceeds Australia max" in new PaypalServiceTestFixture {
val createPaypalPaymentData = CreatePaypalPaymentData(Currency.AUD, 16640.50, "url", "url")
whenReady(paypalService.createPayment(createPaypalPaymentData).value) { result =>
result mustBe (Left(PaypalApiError.fromString("Amount exceeds the maximum allowed ")))
result mustBe (Left(PaypalApiError.fromString("Amount is outside the allowed range ")))
}
}

it should "return an error if the payment amount exceeds non-Australia max" in new PaypalServiceTestFixture {
val createPaypalPaymentData = CreatePaypalPaymentData(Currency.GBP, 2080.50, "url", "url")
whenReady(paypalService.createPayment(createPaypalPaymentData).value) { result =>
result mustBe (Left(PaypalApiError.fromString("Amount exceeds the maximum allowed ")))
result mustBe (Left(PaypalApiError.fromString("Amount is outside the allowed range ")))
}
}

Expand Down

0 comments on commit ff416fa

Please sign in to comment.