diff --git a/CLAUDE.md b/CLAUDE.md index f5c5d257fb..3ae2cd705d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -243,7 +243,7 @@ Note: `executeFutureCreated` returns 201; pair it with `cc.user.openOrThrowExcep **Use `NEW_ACCOUNT_ID` for PUT-creates-account URLs**: When a `PUT /banks/BANK_ID/accounts/ACCOUNT_ID` *creates* the account (it doesn't exist yet), the middleware's `validateAccount` keys off the literal `ACCOUNT_ID` template var and tries to look it up → 404 before the handler runs. Change the ResourceDoc URL template to `/banks/BANK_ID/accounts/NEW_ACCOUNT_ID` (or any non-standard ALL_CAPS variant) — middleware treats it as a wildcard and skips the lookup, but the path still matches the route pattern. The handler can check "already exists" inline with `Connector.connector.vend.checkBankAccountExists(...)` and return 409/400 as needed. -**Reserved ALL_CAPS literals — don't use them as placeholders**: `ResourceDocMatcher` in `Http4sSupport.scala` keeps an explicit `literalAllCapsSegments` set: `SANDBOX_TAN`, `COUNTERPARTY`, `SEPA`, `FREE_FORM`, `ACCOUNT`, `ACCOUNT_OTP`, `REFUND`, `SIMPLE`, `AGENT_CASH_WITHDRAWAL`, `CARD`, `OPEN_CORRIDOR_PROMISE`, `OPEN_CORRIDOR_SETTLEMENT`, `EMAIL`, `SMS`, `IMPLICIT`, `NOT_EMAIL_NEITHER_SMS`. These are matched as **literals** (real Lift endpoints register them as concrete SCA-method / transaction-request-type segments — e.g. `/banks/BANK_ID/my/consents/EMAIL`). Any other ALL_CAPS segment is a wildcard. If you migrate an endpoint whose URL template uses one of these names as a *placeholder variable* (e.g. v3.0/v4.0 `getUsersByEmail` had `/users/email/EMAIL/terminator` with EMAIL meaning "any email value"), the matcher will only fire when the URL segment is literally `EMAIL` — real callers pass actual addresses and miss the doc entirely → middleware skips auth/role validation → handler 500s on the empty CallContext. Rename the placeholder to something outside the literal set (e.g. `EMAIL` → `USER_EMAIL`), and apply the rename in **both** the http4s `ResourceDoc` and the original Lift `ResourceDoc` (resource-docs aggregation reads both, and `collectResourceDocs` dedup keys off URL + verb). +**Reserved ALL_CAPS literals — don't use them as placeholders**: `ResourceDocMatcher` in `Http4sSupport.scala` treats every value of the `TransactionRequestTypes` and `StrongCustomerAuthentication` enums (plus `NOT_EMAIL_NEITHER_SMS`) as a **literal** URL segment: `literalAllCapsSegments` is built from those enums, so adding a transaction-request type to the enum is all it takes. Any other ALL_CAPS segment is a wildcard. Until 2026-10 the set was kept by hand, and `MOBILE_WALLET`, `UTILITY`, `BULK`, `HOLD`, `CARDANO` and the `ETH_*` types were never added: the v7 MOBILE_WALLET template then claimed every transaction-request type (UTILITY and BULK were validated under the MOBILE_WALLET doc; HOLD/CARDANO/ETH on v7 got an empty 404 instead of falling through to v6). When several templates match, the one with the most literal segments wins, whatever the registration order. `ResourceDocSelfResolveTest` checks that every registered doc's own URL resolves to that doc. If you migrate an endpoint whose URL template uses an enum value as a *placeholder variable* (e.g. v3.0/v4.0 `getUsersByEmail` had `/users/email/EMAIL/terminator` with EMAIL meaning "any email value"), the matcher will only fire when the URL segment is literally `EMAIL` — real callers pass actual addresses and miss the doc entirely → middleware skips auth/role validation → handler 500s on the empty CallContext. Rename the placeholder to something outside the literal set (e.g. `EMAIL` → `USER_EMAIL`), and apply the rename in **both** the http4s `ResourceDoc` and the original Lift `ResourceDoc` (resource-docs aggregation reads both, and `collectResourceDocs` dedup keys off URL + verb). **Bypass roles vs required roles**: Some Lift handlers check entitlements inline as **bypass** conditions inside authorisation helpers — e.g. `checkAuthorisationToCreateTransactionRequest` honours `canCreateAnyTransactionRequest` to let the caller skip the view-permission check, but the role is never a hard requirement. These roles are correctly absent from the Lift ResourceDoc role list — putting them in the doc would make Lift enforce them as required (since Lift DOES enforce doc roles by default), breaking the "view permission OR role" intent. The same holds for http4s middleware. So the trap on migration is the reflex copy: don't move a bypass role from inline-only into `Some(List(...))` just because it appears in the handler. Audit before copying: if the role appears in the Lift handler only inside an authorisation OR-chain ("has view permission OR has role X"), it belongs as `None` in the doc with the inline view/role logic preserved. Bypass roles must stay out of the doc. diff --git a/docs/telemetry_conventions.md b/docs/telemetry_conventions.md index 34637f6f55..7870efb20b 100644 --- a/docs/telemetry_conventions.md +++ b/docs/telemetry_conventions.md @@ -295,6 +295,8 @@ thread and class figures on its own port. Remove it once the Micrometer JVM bind | `obp.api.redis.commands` | timer | `command`, `result` | `Redis.use` | | `cache.gets`, `cache.puts`, `cache.evictions`, `cache.size` | standard | `cache` = `in_memory`, `json_schema`, `message_docs`, `on_behalf_of` | every Guava cache, through `Telemetry.monitorCache` | | `hikaricp.*` | standard | `pool` | the database pool (`CustomDBVendor`) | +| `obp.api.database.connection.held_too_long` | gauge | | connections out of the pool right now for longer than `database_connection_hold_warning_seconds` (`DatabaseConnectionHoldWatch`) | +| `obp.api.database.connection.hold_warnings` | counter | | warnings written by `DatabaseConnectionHoldWatch`, one per connection held too long | | `jvm.*`, `process.*`, `system.*` | standard | | JVM memory, heap after garbage collection (`jvm.memory.usage.after.gc`), garbage collection, threads, classes, CPU, uptime, open files | | `obp.api.instance.info` | gauge, always 1 | `api_instance_id`, `git_commit` | start-up | | the counters in section 11 | | | `TelemetryBindings` | diff --git a/ideas/ASSET_REGISTRY.md b/ideas/ASSET_REGISTRY.md index 72a3d2617f..35b8b0a7ae 100644 --- a/ideas/ASSET_REGISTRY.md +++ b/ideas/ASSET_REGISTRY.md @@ -59,7 +59,9 @@ The work is ordered so that each step changes nothing a caller can see until the | 4 | `Asset` and `AssetStatusHistory` tables, provider and boot seed (§2, §6, §7) | Done, not committed | | 5 | Read-only endpoints `GET /assets`, `GET /assets/ASSET_CODE`, reverse chain lookup (§8), and the Glossary entry (§11) | Done, not committed | | 6 | `isValidCurrencyISOCode` and `currencyDecimalPlaces` read from the registry (§4) | Done, not committed | -| Later | Case-insensitive codes (§4), rejecting excess decimals (§4), write endpoints (§8), amount storage (§5, §7 Part A), precision corrections (§7 Part B), folding `lovelace` and `wei` (§7 Part C, §10) | Each changes behaviour or waits on an open question | +| 7 | Case-insensitive codes (§4): codes upper-cased in requests by `ResourceDocMiddleware`, compared ignoring case | Done, not committed | +| 8 | Rejecting excess decimals (§4): 400 (OBP-10068) from `ResourceDocMiddleware`, crypto assets exempt until §7 | Done, not committed | +| Later | write endpoints (§8), amount storage (§5, §7 Part A), precision corrections (§7 Part B), folding `lovelace` and `wei` (§7 Part C, §10) | Each changes behaviour or waits on an open question | **Step 1.** `CurrencyHandlingTest` is a pure unit test (no server, no database). It asserts correct behaviour only. The three known defects are written as the correct expectation inside `pendingUntilFixed`, so they report as pending now and fail, demanding the wrapper's removal, once fixed: - codes are not accepted in every letter case; @@ -106,6 +108,24 @@ The registry is read once into memory and read again after any write through `As `AssetLookupTest` (`code.asset`, CI shard 8) checks that the seeded registry gives the built-in answer for every code, that a newly registered asset is accepted at once with its own decimal places, and that an empty registry falls back. Suites that rewrite the registry restore the seeded one when they end (`RestoresSeededAssetRegistry`), and the test database resets also clear the in-memory copy; otherwise a later suite in the same JVM, such as `CurrencyHandlingTest`, would read a registry holding only test assets. Run with `FundsAvailableTest`, both `TransactionRequestsTest` suites, the v4.0.0 `AccountTest` and `CardanoTransactionRequestTest`: 104 passed, 3 pending (the known defects), none failed. + +**Step 7.** Currency codes are case-insensitive. `code.asset.CurrencyCodes` holds the rule: +- **Requests.** Once a static ResourceDoc has matched, `ResourceDocMiddleware.withCurrencyCodesUpperCased` upper-cases the currency codes in the request before the endpoint runs. In the JSON body it changes the string fields that the endpoint's example body uses for a currency code (a name ending in `currency` or `currency_code`, ignoring case and underscores); it edits the text, so amounts keep their exact form. In the query string it changes parameters named the same way. Only values that look like a code (2 to 12 letters or digits) are touched. Dynamic Entity and Dynamic Endpoint requests are left as sent. +- **Path segments.** The only currency codes in a URL path are those of `GET /banks/BANK_ID/fx/FROM_CURRENCY_CODE/TO_CURRENCY_CODE`, which already upper-cases them, and `GET /assets/ASSET_CODE`, which already ignores case. +- **Stored codes.** About ten comparisons against a stored currency (transaction request against account currency, funds available, settlement accounts, bulk payments, FX) use `CurrencyCodes.same`, so rows written before this rule in another case still match. `AssetLookup` ignores case, so `jpy` gets 0 decimal places. `ada` is no longer a legacy spelling, because it matches `ADA`. +- **Stored rows.** The runOnce migration `upperCaseStoredCurrencyCodes` (`MigrationOfCurrencyCodesUpperCase`) rewrites the codes in every currency column in upper case, `ada` to `ADA` among them, so database queries that select by currency (for example the FX rate lookup) find old rows too. It leaves codes inside stored JSON alone, and a `mappedcurrency` row whose upper-case twin exists. `lovelace` and `wei` become `LOVELACE` and `WEI`; their amounts are converted later (§7 Part C). `CurrencyCodesUpperCaseMigrationTest` covers it. + +`CurrencyCodesTest` (unit) covers the rule. The two letter-case defects in `CurrencyHandlingTest` are no longer pending. `AssetLookupTest`, `FundsAvailableTest` (lower-case `eur` now gives 200) and `ExchangeRateTest` (an FX rate created with `eur`/`usd` is stored as `EUR`/`USD` and read back with lower-case path segments) were updated. + + +**Step 8.** An amount with more decimal places than its currency allows is refused with 400 (OBP-10068, `InvalidAmountPrecision`) instead of being cut off when stored. `code.asset.AmountPrecision` finds the amounts; `ResourceDocMiddleware.validateAmountPrecision` runs it as the last validation step, after authentication and Roles, so it applies to every static endpoint in every version: +- **Where an amount is found.** In a JSON body, an object with an `amount` field (any letter case) next to its currency: the field named `currency`, or else the object's only currency-named field. In a query string, an `amount` parameter next to a currency parameter. Numbers are parsed exactly (`useBigDecimalForDouble`). Trailing zeros do not count. +- **Not checked.** Unknown codes and non-numbers (left to the endpoint), and crypto assets: `AssetLookup.enforcedDecimalPlaces` is None for a registry `CRYPTO` asset and for `lovelace` and `wei`. Their registered decimal places are still the built-in 2, so enforcing them would refuse 0.001 ETH. They are checked once §7 records their real precision. +- **Not changed.** Amounts OBP computes itself (FX conversion, for example) are still cut off by `convertToSmallestCurrencyUnits`; only amounts a caller sends are refused. +- **Precision used.** The registry's decimal places, which still equal the built-in table (§7 Part B corrects them). So CZK is held at 0 decimal places and 10.50 CZK is refused, as it would be stored as 10 today. + +`code.asset.AmountPrecisionTest` (unit) covers the rule; `code.api.v4_0_0.AmountPrecisionTest` covers account creation and a SANDBOX_TAN transaction request; `FundsAvailableTest` covers a query amount. + --- ## 1. Model diff --git a/obp-api/src/main/resources/props/sample.props.template b/obp-api/src/main/resources/props/sample.props.template index c6f144da83..c960ba902f 100644 --- a/obp-api/src/main/resources/props/sample.props.template +++ b/obp-api/src/main/resources/props/sample.props.template @@ -45,6 +45,12 @@ connector=star #hikari.keepaliveTime=30000 #hikari.maxLifetime=1800000 +# Write a warning to the log (and so to the log cache) when a database connection has been out of +# the pool for longer than this many seconds. The warning names the thread that took it and the +# OBP-API code that thread is running, and a second warning gives the total time once the +# connection goes back. 0 turns this off. +#database_connection_hold_warning_seconds=60 + ## if connector = star, then need to set which connectors will be used. For now, obp support rest, akka. starConnector_supported_types=mapped,internal diff --git a/obp-api/src/main/scala/bootstrap/liftweb/CustomDBVendor.scala b/obp-api/src/main/scala/bootstrap/liftweb/CustomDBVendor.scala index fe5370c93e..0c6333b354 100644 --- a/obp-api/src/main/scala/bootstrap/liftweb/CustomDBVendor.scala +++ b/obp-api/src/main/scala/bootstrap/liftweb/CustomDBVendor.scala @@ -28,8 +28,8 @@ TESOBE (http://www.tesobe.com/) package bootstrap.liftweb import code.api.util.APIUtil +import code.util.DatabaseConnectionHoldWatch import code.util.Helper.MdcLoggable -import com.zaxxer.hikari.pool.ProxyConnection import com.zaxxer.hikari.{HikariConfig, HikariDataSource} import java.sql.Connection @@ -90,7 +90,12 @@ class CustomDBVendor(driverName: String, // Telemetry: the pool's connections, waits and timeouts, as the standard hikaricp_* series. config.setMetricRegistry(code.telemetry.Telemetry.registry) - val ds: HikariDataSource = new HikariDataSource(config) + // Every connection taken from the pool, whichever way it is taken (Lift Mapper, Doobie, the + // request transaction), passes through here, so DatabaseConnectionHoldWatch sees them all. + val ds: HikariDataSource = new HikariDataSource(config) { + override def getConnection(): Connection = DatabaseConnectionHoldWatch.watch(super.getConnection()) + } + DatabaseConnectionHoldWatch.pool = Some(ds) } def createOne: Box[Connection] = { @@ -109,6 +114,8 @@ trait CustomProtoDBVendor extends ConnectionManager with MdcLoggable { createOne } - def releaseConnection(conn: Connection): Unit = {conn.asInstanceOf[ProxyConnection].close()} + // A plain close(): the connection may be DatabaseConnectionHoldWatch's wrapper rather than + // HikariCP's own ProxyConnection, and either way close() returns it to the pool. + def releaseConnection(conn: Connection): Unit = conn.close() } diff --git a/obp-api/src/main/scala/code/api/util/DBUtil.scala b/obp-api/src/main/scala/code/api/util/DBUtil.scala index d5ae725da6..a29bfb8db3 100644 --- a/obp-api/src/main/scala/code/api/util/DBUtil.scala +++ b/obp-api/src/main/scala/code/api/util/DBUtil.scala @@ -39,12 +39,19 @@ object DBUtil { def isSqlServer: Boolean = dbUrl.contains("sqlserver") + /** The endpoint timeout in whole seconds, rounded up, which is the unit JDBC takes. */ + private[util] def queryTimeoutSeconds: Int = + math.max(1L, (Constant.longEndpointTimeoutInMillis + 999) / 1000).toInt + /** * SQL Server-safe alternative to Lift's DB.runQuery. * * Lift's DB.runQuery uses DB.asString which doesn't handle SQL Server's NVARCHAR type * (JDBC type -9), causing MatchError. This function handles all JDBC types properly. * + * The query is cancelled by the database after `long_endpoint_timeout`, so use this for + * read-only queries only. + * * @param query SQL query string * @param params Query parameters (for prepared statement) * @return Tuple of (column names, rows as List[List[String]]) @@ -53,6 +60,12 @@ object DBUtil { DB.use(DefaultConnectionIdentifier) { conn => val stmt = conn.prepareStatement(query) try { + // The database cancels the query once the caller can no longer receive its answer. The + // endpoint timeout (long_endpoint_timeout) answers the caller with a 504, but without this + // the query carried on, holding its pool connection, for as long as it took: one + // aggregate-metrics query held its connection for about ten minutes. Only read-only + // SELECTs come through here, so cancelling never interrupts a write. + stmt.setQueryTimeout(queryTimeoutSeconds) // Set parameters params.zipWithIndex.foreach { case (param, idx) => stmt.setString(idx + 1, param) diff --git a/obp-api/src/main/scala/code/api/util/ErrorMessages.scala b/obp-api/src/main/scala/code/api/util/ErrorMessages.scala index 94c1d07426..89b5010035 100644 --- a/obp-api/src/main/scala/code/api/util/ErrorMessages.scala +++ b/obp-api/src/main/scala/code/api/util/ErrorMessages.scala @@ -153,6 +153,7 @@ object ErrorMessages { val IpPenaltyAlreadyExists = "OBP-10064: This IP address already has a penalty. Remove it first to change it." val IpPenaltyNotFound = "OBP-10065: This IP address has no penalty." val InvalidTrafficWindow = "OBP-10067: Invalid window. Use window=1, 5 or 15 (minutes)." + val InvalidAmountPrecision = "OBP-10068: Invalid amount precision. The amount has more decimal places than its currency allows, and OBP does not round or cut off amounts." val InvalidIpPenalty = "OBP-10066: Invalid IP penalty. per_minute_limit must be 0 or more, duration_minutes between 1 and 10080 (one week), and reason between 1 and 255 characters." // Not an error: the text of the X-Rate-Limit-Warning header a self-service endpoint returns in // shadow mode. SCOPE and LIMIT are replaced at runtime, e.g. "signup" and "5 per hour". diff --git a/obp-api/src/main/scala/code/api/util/Glossary.scala b/obp-api/src/main/scala/code/api/util/Glossary.scala index 58eca2129b..ba4afa12cf 100644 --- a/obp-api/src/main/scala/code/api/util/Glossary.scala +++ b/obp-api/src/main/scala/code/api/util/Glossary.scala @@ -7207,6 +7207,30 @@ object Glossary extends MdcLoggable { """) + glossaryItems += GlossaryItem( + title = "Database Connection Hold Warnings", + description = + s""" + |# Database Connection Hold Warnings + | + |OBP-API keeps a pool of database connections that requests share. A request takes a connection, uses it and gives it back. When work keeps a connection for a long time, the pool can run out, and every request that needs a connection then waits and fails, including requests that would be quick on their own. + | + |An endpoint that takes too long is answered with a timeout, but the database work behind it carries on, and keeps its connection, until the query returns. So the request that was cut off and the work that holds the connection are separate, and the timeout alone does not say which work is holding the pool. + | + |To show this, OBP-API writes a warning to its log when a connection has been out of the pool for too long. ${if (code.util.DatabaseConnectionHoldWatch.enabled) s"On this instance the limit is ${code.util.DatabaseConnectionHoldWatch.holdWarningSeconds} seconds." else "On this instance the warnings are turned off."} The warning gives: + | + |- how long the connection has been held, and how many connections are in use, idle and waited for; + |- the thread that took the connection, and the OBP-API code that thread is running at that moment, which for a query still running is the code waiting for it. + | + |When that connection goes back to the pool, a second warning gives the total time it was held. Both warnings carry the same connection label, so they can be paired. The warnings reach the log cache like any other warning. + | + |[Telemetry](/glossary#Telemetry) reports how many connections are held past the limit right now (`obp_api_database_connection_held_too_long`) and how many warnings have been written (`obp_api_database_connection_hold_warnings_total`), next to the pool's own `hikaricp_*` series. + | + |See also: [Telemetry](/glossary#Telemetry). + | +""") + + glossaryItems += GlossaryItem( title = "Asset", description = @@ -7215,7 +7239,7 @@ object Glossary extends MdcLoggable { | |An **Asset** is a unit that amounts can be held in: a currency such as EUR, a precious metal such as gold (XAU), an accounting unit such as the IMF's Special Drawing Right (XDR), a crypto asset such as ETH, or an asset a bank issues, such as a deposit token, a stablecoin, a bond or a fund share. Each Asset has a code, and that code is the value that appears in the `currency` field of an account, a transaction or a product fee. | - |The **asset registry** lists the Assets an OBP instance knows. It is filled automatically with every currency, metal and accounting unit in ISO 4217 and with the crypto assets XBT, ADA and ETH. Banks will be able to add the assets they issue. + |The **asset registry** lists the Assets an OBP instance knows. It is filled automatically with every currency, metal and accounting unit in ISO 4217 and with the crypto assets XBT, ADA and ETH. | |## What the registry records about an Asset | @@ -7224,7 +7248,7 @@ object Glossary extends MdcLoggable { |- **Decimal places**: how many digits an amount may have after the decimal point, from 0 to 18. EUR has 2, JPY has 0. |- **Issuer**: the bank that issued it, for the types a bank issues. Currencies, metals, accounting units and crypto assets have no issuer. |- **Chain identity**: for a token recorded on a blockchain, the chain and network (for example `CARDANO_MAINNET`) and the token's identity there. A chain's own currency, such as ADA or ETH, has none. - |- **Status**: `ACTIVE` (usable), `SUSPENDED` (for example frozen by a regulator; existing holdings stay visible) or `RETIRED` (for example a bond that has matured). `RETIRED` is final. + |- **Status**: `ACTIVE`, `SUSPENDED` (for example frozen by a regulator) or `RETIRED` (for example a bond that has matured). | |## Assets, Products and Accounts | @@ -7232,11 +7256,19 @@ object Glossary extends MdcLoggable { | |## Administering bank | - |Every Asset is administered at exactly one bank: the issuer for the types a bank issues, and the `SYS` bank for everything else. Changes to an Asset will be made at its administering bank, so the Roles that allow them can always name one bank. + |Every Asset is administered at exactly one bank: the issuer for the types a bank issues, and the `SYS` bank for everything else. + | + |## Codes OBP accepts + | + |The registry decides which currency codes OBP accepts and how many decimal places each one has. OBP also accepts `lovelace` and `wei`, the smallest units of ADA and ETH, which are not Assets of their own. + | + |## Letter case + | + |Currency codes are case-insensitive: `eur`, `Eur` and `EUR` all name the same Asset and get the same decimal places. OBP upper-cases a currency code in a request before the endpoint handles it, so it is validated, compared and stored as `EUR`. This applies to the fields of a JSON request body that hold a currency code (`currency`, `from_currency_code` and the like) and to query parameters named the same way. Wherever OBP compares two currency codes, it ignores letter case. | - |## Current state + |## Decimal places of an amount | - |The registry decides which currency codes OBP accepts and how many decimal places it gives them. For now it gives the same answers as the built-in list OBP used before: codes are matched exactly as written, so `EUR` is accepted and `eur` is not, and an Asset's status is not yet checked. Each code has the same number of decimal places as in that list. The registry does not hold `lovelace` or `wei`, which OBP still accepts: they are the smallest units of ADA and ETH, not Assets of their own. OBP also still accepts `ada` as well as `ADA`. + |OBP does not round or cut off an amount. A request carrying an amount with more decimal places than its currency allows, such as 12.345 EUR or 100.5 JPY, is refused with 400 (OBP-10068). Trailing zeros do not count, so 100.00 JPY is accepted. This applies to every object in a JSON request body that holds an `amount` next to its currency (UK Open Banking's `Amount` and `Currency` included), and to an `amount` query parameter next to a currency parameter. | |## Endpoints | diff --git a/obp-api/src/main/scala/code/api/util/http4s/Http4sSupport.scala b/obp-api/src/main/scala/code/api/util/http4s/Http4sSupport.scala index a94c481123..5bae77aeea 100644 --- a/obp-api/src/main/scala/code/api/util/http4s/Http4sSupport.scala +++ b/obp-api/src/main/scala/code/api/util/http4s/Http4sSupport.scala @@ -34,6 +34,7 @@ import code.api.util.ErrorMessages.{AuthenticatedUserIsRequired, InvalidJsonForm import code.api.util.{AuthHeaderParser, CallContext, RemoteIpUtil, WriteMetricUtil} import code.util.Helper.MdcLoggable import com.openbankproject.commons.model.{Bank, BankAccount, CounterpartyTrait, User, View} +import com.openbankproject.commons.model.enums.{StrongCustomerAuthentication, TransactionRequestTypes} import net.liftweb.common.{Box, Empty, Full} import org.http4s._ import org.http4s.dsl.io._ @@ -837,7 +838,11 @@ object ResourceDocMatcher extends code.util.Helper.MdcLoggable { val segCount = strippedPath.split("/").count(_.nonEmpty) val lookupKey = (verb.toUpperCase, apiVersion, segCount) val candidates = index.getOrElse(lookupKey, Nil) - val result = candidates.find(doc => matchesUrlTemplate(strippedPath, doc.requestUrl)) + // When several templates match, the one with the most literal segments wins, so + // `.../transaction-request-types/SEPA/...` beats `.../transaction-request-types/TRANSACTION_REQUEST_TYPE/...` + // whichever was registered first. On a tie, the first registered wins (maxBy keeps the first maximum). + val matching = candidates.filter(doc => matchesUrlTemplate(strippedPath, doc.requestUrl)) + val result = if (matching.isEmpty) None else Some(matching.maxBy(doc => literalSegmentCount(doc.requestUrl))) if (result.isEmpty) { logger.debug( s"[ResourceDocMatcher] No match for $verb $pathString. " + @@ -885,35 +890,32 @@ object ResourceDocMatcher extends code.util.Helper.MdcLoggable { } /** - * Check if a template segment is a variable (uppercase) - */ - /** - * All-caps URL-segment literals that historically broke the matcher. - * - * `isTemplateVariable` originally returned true for every all-caps + underscore + - * digit segment. That made literals like `SANDBOX_TAN`, `ACCOUNT`, `SEPA` etc. - * indistinguishable from real placeholders like `BANK_ID`, so a ResourceDoc URL - * `/banks/BANK_ID/.../transaction-request-types/SANDBOX_TAN/transaction-requests` - * matched any trans-req-type URL — including v4-only `ACCOUNT` — and the v4 - * request never reached the Lift fallback that knows how to handle it. + * This set holds the all-caps URL segments that are fixed words, not placeholders. * - * We special-case the known literal segments. Anything else stays a wildcard so - * the existing non-standard placeholder convention (NEW_ACCOUNT_ID, GRANT_VIEW_ID, - * FIREHOSE_BANK_ID, EXPLICIT_COUNTERPARTY_ID, SYS_VIEW_ID, …) keeps working - * without an explicit allow-list. + * ResourceDoc templates write placeholders in capitals (`BANK_ID`, `GRANT_VIEW_ID`, + * `TRANSACTION_REQUEST_TYPE`), and OBP writes enum values in capitals too. Where an enum + * value appears in a URL, as in + * `/banks/BANK_ID/accounts/ACCOUNT_ID/VIEW_ID/transaction-request-types/MOBILE_WALLET/transaction-requests`, + * the two look the same. If `MOBILE_WALLET` were read as a placeholder, that template would + * match every transaction-request type, and the middleware would validate UTILITY or BULK + * requests with the MOBILE_WALLET doc (its roles, operationId, enable/disable switch), or + * return 404 for a type that version has no handler for instead of letting the request + * fall through to an older version. * - * Add a value here when a new path uses an all-caps literal (e.g. a new - * transaction-request type or SCA method). + * The set used to be kept by hand and fell behind each time a type was added. It is now + * built from the two enums whose values appear as URL segments, transaction-request types + * and SCA methods, so a new enum value is a literal without anyone remembering to list it. + * Any other all-caps segment stays a placeholder, which keeps the deliberate non-standard + * placeholders (NEW_ACCOUNT_ID, FIREHOSE_BANK_ID, ...) working. */ - private val literalAllCapsSegments: Set[String] = Set( - // transaction-request types - "SANDBOX_TAN", "COUNTERPARTY", "SEPA", "FREE_FORM", - "ACCOUNT", "ACCOUNT_OTP", "REFUND", "SIMPLE", - "AGENT_CASH_WITHDRAWAL", "CARD", - "OPEN_CORRIDOR_PROMISE", "OPEN_CORRIDOR_SETTLEMENT", - // SCA methods (POST /banks/BANK_ID/my/consents/{EMAIL|SMS|IMPLICIT}) - "EMAIL", "SMS", "IMPLICIT", "NOT_EMAIL_NEITHER_SMS" - ) + private[http4s] val literalAllCapsSegments: Set[String] = + TransactionRequestTypes.values.map(_.toString).toSet ++ + StrongCustomerAuthentication.values.map(_.toString) + + // Used as a fixed segment in consent URLs but not an SCA enum value. + "NOT_EMAIL_NEITHER_SMS" + + private def literalSegmentCount(template: String): Int = + template.split("/").count(segment => segment.nonEmpty && !isTemplateVariable(segment)) private def isTemplateVariable(segment: String): Boolean = { segment.nonEmpty && diff --git a/obp-api/src/main/scala/code/api/util/http4s/ResourceDocMiddleware.scala b/obp-api/src/main/scala/code/api/util/http4s/ResourceDocMiddleware.scala index 41b90916c3..8a7897581c 100644 --- a/obp-api/src/main/scala/code/api/util/http4s/ResourceDocMiddleware.scala +++ b/obp-api/src/main/scala/code/api/util/http4s/ResourceDocMiddleware.scala @@ -170,7 +170,7 @@ object ResourceDocMiddleware extends MdcLoggable { OptionT.liftF(Http4sCallContextBuilder.fromRequest(req, apiVersionFromPath)).flatMap { cc => // Cache the body so bridge-cascade hops (v400→v310→v300→…) don't re-read the now-empty stream. // First read won the body in fromRequest; we replay it from cc.httpBody onwards. - val reqWithCachedBody = req.withAttribute(Http4sRequestAttributes.cachedBodyKey, cc.httpBody) + val reqWithOriginalBody = req.withAttribute(Http4sRequestAttributes.cachedBodyKey, cc.httpBody) ResourceDocMatcher.findResourceDoc(req.method.name, req.uri.path, resourceDocIndex) match { case Some(resourceDoc) if !endpointIsEnabled(resourceDoc) => // Disabled by api_disabled_endpoints / api_enabled_endpoints / api_disabled_versions / @@ -181,7 +181,8 @@ object ResourceDocMiddleware extends MdcLoggable { note.operationId = Some(resourceDoc.operationId) note.apiVersion = Some(resourceDoc.implementedInApiVersion.apiShortVersion) } - val ccWithDoc = ResourceDocMatcher.attachToCallContext(cc, resourceDoc) + val (reqWithCachedBody, ccForDoc) = withCurrencyCodesUpperCased(reqWithOriginalBody, cc, resourceDoc) + val ccWithDoc = ResourceDocMatcher.attachToCallContext(ccForDoc, resourceDoc) val pathParams = ResourceDocMatcher.extractPathParams(req.uri.path, resourceDoc) // Validate first (read-only, outside any transaction), then run business logic. // GET/HEAD are safe methods — no writes, no transaction needed; they run on @@ -231,7 +232,7 @@ object ResourceDocMiddleware extends MdcLoggable { // serves the request, and re-validating the credentials on every hop was pure waste. OptionT.liftF( resolveCallerOnce(req, cc).map { resolvedCc => - reqWithCachedBody.withAttribute(Http4sRequestAttributes.callContextKey, resolvedCc) + reqWithOriginalBody.withAttribute(Http4sRequestAttributes.callContextKey, resolvedCc) } ).flatMap(routes.run) } @@ -363,6 +364,7 @@ object ResourceDocMiddleware extends MdcLoggable { context <- processForceError(req, resourceDoc, context) context <- validateAuthType(resourceDoc, context) context <- validateJsonSchema(resourceDoc, context) + context <- validateAmountPrecision(req, context) } yield context result.value.map { @@ -617,6 +619,32 @@ object ResourceDocMiddleware extends MdcLoggable { } } + /** + * This refuses, with 400, a request carrying an amount with more decimal places than its currency + * allows, such as 12.345 EUR. Before this check such an amount was cut off when it was stored, so + * part of it silently disappeared. The amounts it looks at, and the currencies it leaves alone, are + * described in [[code.asset.AmountPrecision]]. + * + * It runs last, once the caller is known to be allowed to make the request, so an unauthenticated + * caller still gets 401 and one without the Role 403. The body it reads is the one in the + * CallContext, whose currency codes are already upper case (withCurrencyCodesUpperCased). + */ + private def validateAmountPrecision(req: Request[IO], ctx: ValidationContext): Validation[ValidationContext] = { + import DSL._ + import code.asset.AmountPrecision + val excess = ctx.callContext.httpBody.flatMap(AmountPrecision.inJsonBody) + .orElse(AmountPrecision.inQuery(req.uri.query.pairs)) + excess match { + case Some(found) => + val message = s"${code.api.util.ErrorMessages.InvalidAmountPrecision} ${found.describe}" + EitherT[IO, Response[IO], ValidationContext]( + ErrorResponseConverter.createErrorResponse(400, message, ctx.callContext) + .map[Either[Response[IO], ValidationContext]](Left(_)) + ) + case None => success(ctx) + } + } + /** * Port of `APIUtil.validateQueryParams` (a `beforeAuthenticateInterceptor` in Lift). * Rejects requests with duplicate query-parameter names with 400 @@ -722,6 +750,43 @@ object ResourceDocMiddleware extends MdcLoggable { } } + /** + * This upper-cases the currency codes in a request before the endpoint sees it, because currency + * codes are case-insensitive (code.asset.CurrencyCodes): `eur` is validated, compared and stored + * as `EUR`. It touches the string values of the body fields that the endpoint's documented example + * body uses for a currency code, and the values of query parameters whose name says they hold one + * (`currency`, `from_currency_code`, ...). Only values that look like a code are changed, and a + * request with nothing to change is returned as it came. The body is changed in the CallContext + * and in the cached body every later hop reads; the query in the request URI and in `cc.url`. + * + * It runs only once a static ResourceDoc has matched, so a Dynamic Entity or Dynamic Endpoint + * request falling through this middleware keeps its body exactly as sent. + */ + private def withCurrencyCodesUpperCased(req: Request[IO], cc: CallContext, resourceDoc: ResourceDoc): (Request[IO], CallContext) = { + import code.asset.CurrencyCodes + def upperCasedIfCode(value: String): String = + if (CurrencyCodes.looksLikeCode(value)) CurrencyCodes.normalise(value) else value + + val currencyFields = CurrencyCodes.currencyFieldsOf(resourceDoc.operationId, resourceDoc.exampleRequestBody) + val body = cc.httpBody.map(CurrencyCodes.normaliseJsonBody(_, currencyFields)) + + val pairs = req.uri.query.pairs + val queryChanged = pairs.exists { case (name, value) => + CurrencyCodes.isCurrencyKey(name) && value.exists(v => upperCasedIfCode(v) != v) + } + val uri = + if (!queryChanged) req.uri + else req.uri.copy(query = Query.fromVector(pairs.map { case (name, value) => + (name, if (CurrencyCodes.isCurrencyKey(name)) value.map(upperCasedIfCode) else value) + })) + + if (body == cc.httpBody && !queryChanged) (req, cc) + else ( + req.withUri(uri).withAttribute(Http4sRequestAttributes.cachedBodyKey, body), + cc.copy(url = if (queryChanged) uri.renderString else cc.url, httpBody = body) + ) + } + /** Ensure the response has JSON content type */ private def ensureJsonContentType(response: Response[IO]): Response[IO] = { response.contentType match { diff --git a/obp-api/src/main/scala/code/api/util/migration/Migration.scala b/obp-api/src/main/scala/code/api/util/migration/Migration.scala index 49618ee331..e0d9ec4749 100644 --- a/obp-api/src/main/scala/code/api/util/migration/Migration.scala +++ b/obp-api/src/main/scala/code/api/util/migration/Migration.scala @@ -193,6 +193,7 @@ object Migration extends MdcLoggable { alterDynamicDataIdLength() renameDynamicEntityRoles() renameDynamicEntityDefinitionRoles() + upperCaseStoredCurrencyCodes() } /** @@ -950,6 +951,19 @@ object Migration extends MdcLoggable { } } + /** + * Rewrite every stored currency code in upper case, because currency codes are case-insensitive + * and are now upper-cased in every request. Rows written before that, such as `ada` or a client's + * `eur`, would otherwise be missed by database queries that select by currency. The columns, and + * what is deliberately left alone, are in [[MigrationOfCurrencyCodesUpperCase]]. + */ + private def upperCaseStoredCurrencyCodes(): Boolean = { + val name = nameOf(upperCaseStoredCurrencyCodes) + runOnce(name) { + MigrationOfCurrencyCodesUpperCase.upperCaseEverywhere(name) + } + } + private def alterDynamicDataIdLength(): Boolean = { val name = nameOf(alterDynamicDataIdLength) runOnce(name) { diff --git a/obp-api/src/main/scala/code/api/util/migration/MigrationOfCurrencyCodesUpperCase.scala b/obp-api/src/main/scala/code/api/util/migration/MigrationOfCurrencyCodesUpperCase.scala new file mode 100644 index 0000000000..595cab0748 --- /dev/null +++ b/obp-api/src/main/scala/code/api/util/migration/MigrationOfCurrencyCodesUpperCase.scala @@ -0,0 +1,213 @@ +/** +Open Bank Project - API +Copyright (C) 2011-2026, TESOBE GmbH. + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . + +Email: contact@tesobe.com +TESOBE GmbH. +Osloer Strasse 16/17 +Berlin 13359, Germany + +This product includes software developed at +TESOBE (http://www.tesobe.com/) + + */ +package code.api.util.migration + +import code.api.util.APIUtil +import code.api.util.migration.Migration.{DbFunction, saveLog} +import code.apiproduct.ApiProduct +import code.asset.CurrencyCodes +import code.bulkpayment.BulkPayment +import code.counterpartylimit.CounterpartyLimit +import code.customer.MappedCustomer +import code.fx.{MappedCurrency, MappedFXRate} +import code.metadata.counterparties.MappedCounterparty +import code.model.dataAccess.MappedBankAccount +import code.opencorridorfees.OpenCorridorFeeAccrual +import code.productfee.ProductFee +import code.standingorders.StandingOrder +import code.transaction.MappedTransaction +import code.transaction_types.MappedTransactionType +import code.transactionrequests.{MappedTransactionRequest, MappedTransactionRequestTypeCharge, TransactionRequestReasons} +import code.util.Helper.MdcLoggable +import net.liftweb.db.DB +import net.liftweb.mapper.BaseMetaMapper +import net.liftweb.util.DefaultConnectionIdentifier + +import scala.collection.mutable.ListBuffer + +/** + * This migration rewrites every stored currency code in upper case, so `eur` and `Eur` become `EUR`. + * + * Currency codes are case-insensitive (code.asset.CurrencyCodes, ideas/ASSET_REGISTRY.md section 4). + * Since that rule, a code in a request is upper-cased before the endpoint sees it, so new rows are + * written upper case. Rows written before it can hold a code in another case: `ada`, `lovelace` and + * `wei` were accepted only in lower case, and a client could store whatever case it sent wherever a + * currency was not validated. Comparisons in the code ignore case, but database queries that select + * by currency (the FX rate lookup, for example) match exactly, so such a row would be missed. After + * this migration every stored code is in the one form the rest of OBP expects. + * + * It touches only the columns that hold a currency code, listed below, and only rows whose value is + * not already trimmed and upper case. Table and column names come from the Mapper definitions. A + * table or column that does not exist on this database is skipped. + * + * `mappedcurrency` keys its rows by the code, so a lower-case row whose upper-case twin is already + * there is left as it is, and reported, rather than failing on the duplicate key. + * + * Currency codes inside stored JSON (the body of a transaction request, for example) are not changed. + * `lovelace` and `wei` become `LOVELACE` and `WEI`; converting their amounts to ADA and ETH is a + * separate later step (ideas/ASSET_REGISTRY.md section 7, Part C). + */ +object MigrationOfCurrencyCodesUpperCase extends MdcLoggable { + + private case class CurrencyColumn(table: BaseMetaMapper, column: String, isPrimaryKey: Boolean = false) + + private def currencyColumns: List[CurrencyColumn] = List( + CurrencyColumn(MappedCurrency, MappedCurrency.mCurrencyCode.dbColumnName, isPrimaryKey = true), + CurrencyColumn(MappedFXRate, MappedFXRate.mFromCurrencyCode.dbColumnName), + CurrencyColumn(MappedFXRate, MappedFXRate.mToCurrencyCode.dbColumnName), + CurrencyColumn(MappedBankAccount, MappedBankAccount.accountCurrency.dbColumnName), + CurrencyColumn(MappedTransaction, MappedTransaction.currency.dbColumnName), + CurrencyColumn(MappedTransactionRequest, MappedTransactionRequest.mCharge_Currency.dbColumnName), + CurrencyColumn(MappedTransactionRequest, MappedTransactionRequest.mBody_Value_Currency.dbColumnName), + CurrencyColumn(MappedTransactionRequestTypeCharge, MappedTransactionRequestTypeCharge.mChargeCurrency.dbColumnName), + CurrencyColumn(TransactionRequestReasons, TransactionRequestReasons.Currency.dbColumnName), + CurrencyColumn(MappedTransactionType, MappedTransactionType.mCustomerFee_Currency.dbColumnName), + CurrencyColumn(StandingOrder, StandingOrder.AmountCurrency.dbColumnName), + CurrencyColumn(BulkPayment, BulkPayment.Currency.dbColumnName), + CurrencyColumn(MappedCustomer, MappedCustomer.mCreditLimitCurrency.dbColumnName), + CurrencyColumn(MappedCounterparty, MappedCounterparty.mCurrency.dbColumnName), + CurrencyColumn(CounterpartyLimit, CounterpartyLimit.Currency.dbColumnName), + CurrencyColumn(ProductFee, ProductFee.Currency.dbColumnName), + CurrencyColumn(ApiProduct, ApiProduct.MonthlySubscriptionCurrency.dbColumnName), + CurrencyColumn(OpenCorridorFeeAccrual, OpenCorridorFeeAccrual.Currency.dbColumnName) + ) + + /** + * This returns true when the table has the column. Names are compared ignoring letter case, as + * DbFunction.tableExists compares table names, because databases store unquoted names differently: + * PostgreSQL in lower case, H2 (used by CI) in upper case. + */ + private def columnExists(actualTableName: String, column: String): Boolean = + DB.use(DefaultConnectionIdentifier) { connection => + val columns = connection.getMetaData.getColumns(null, null, actualTableName, null) + try { + var found = false + while (!found && columns.next()) found = columns.getString("COLUMN_NAME").equalsIgnoreCase(column) + found + } finally columns.close() + } + + private def runUpdate(sql: String): Int = + DB.use(DefaultConnectionIdentifier) { connection => + val statement = connection.createStatement() + try statement.executeUpdate(sql) finally statement.close() + } + + /** + * This returns, for the rows matching the condition, each stored value with how many rows hold it, + * most frequent first. It is read before the update so the log can say what each value became. + */ + private def valueCounts(table: String, column: String, condition: String): List[(String, Int)] = + DB.use(DefaultConnectionIdentifier) { connection => + val statement = connection.createStatement() + try { + val result = statement.executeQuery( + s"SELECT $column, COUNT(*) FROM $table WHERE $condition GROUP BY $column") + try { + val counts = ListBuffer[(String, Int)]() + while (result.next()) counts += ((result.getString(1), result.getInt(2))) + // Sorted here rather than in SQL, so values with the same count come in the same order on every database. + counts.toList.sortBy { case (value, count) => (-count, value) } + } finally result.close() + } finally statement.close() + } + + /** At most this many distinct values are named per column, so one column cannot crowd out the rest. */ + private val MaxValuesNamedPerColumn = 10 + + /** This describes the values that changed, e.g. `eur -> EUR x3, 'Gbp ' -> GBP x1`. */ + private def describeChanges(counts: List[(String, Int)]): String = { + def shown(value: String) = if (value == value.trim) value else s"'$value'" + val named = counts.take(MaxValuesNamedPerColumn).map { case (value, count) => + s"${shown(value)} -> ${CurrencyCodes.normalise(value)} x$count" + } + val more = counts.size - MaxValuesNamedPerColumn + (named ++ (if (more > 0) List(s"and $more other value(s)") else Nil)).mkString(", ") + } + + /** + * This upper-cases the stored currency codes in every column listed above and records what it did. + * + * The migration log gets one entry per column, in this order, so that if the remark is longer than + * the log column holds, the part that is cut off is the least important: the columns that changed + * (with each value and what it became), then rows left alone in `mappedcurrency`, then tables or + * columns not on this database, then the columns where nothing needed changing. The full text is + * also written to the server log, one line per column as it is processed. + */ + def upperCaseEverywhere(name: String): Boolean = { + val startDate = System.currentTimeMillis() + val changedColumns = ListBuffer[String]() + val leftAloneColumns = ListBuffer[String]() + val missingColumns = ListBuffer[String]() + val unchangedColumns = ListBuffer[String]() + + currencyColumns.foreach { currencyColumn => + val table = currencyColumn.table.dbTableName.toLowerCase + val column = currencyColumn.column.toLowerCase + val qualified = s"$table.$column" + val actualTableNames = new scala.collection.mutable.HashMap[String, String]() + if (!DbFunction.tableExists(currencyColumn.table, actualTableNames)) { + missingColumns += s"$qualified (no such table)" + logger.info(s"upperCaseEverywhere says: $qualified skipped, the table does not exist on this database") + } else if (!columnExists(actualTableNames.getOrElse(currencyColumn.table._dbTableNameLC, table), column)) { + missingColumns += s"$qualified (no such column)" + logger.info(s"upperCaseEverywhere says: $qualified skipped, the column does not exist on this database") + } else { + val needsChange = s"$column <> UPPER(TRIM($column))" + val twinExists = s"EXISTS (SELECT 1 FROM $table twin WHERE twin.$column = UPPER(TRIM($table.$column)))" + val toChangeCondition = if (currencyColumn.isPrimaryKey) s"$needsChange AND NOT $twinExists" else needsChange + val toChange = valueCounts(table, column, toChangeCondition) + val changed = if (toChange.isEmpty) 0 else runUpdate(s"UPDATE $table SET $column = UPPER(TRIM($column)) WHERE $toChangeCondition") + val leftAlone = if (currencyColumn.isPrimaryKey) valueCounts(table, column, needsChange) else Nil + + if (changed > 0) { + val line = s"$qualified: $changed row(s) changed (${describeChanges(toChange)})" + changedColumns += line + logger.info(s"upperCaseEverywhere says: $line") + } else { + unchangedColumns += qualified + logger.info(s"upperCaseEverywhere says: $qualified: nothing needed changing") + } + if (leftAlone.nonEmpty) { + val line = s"$qualified: ${leftAlone.map(_._2).sum} row(s) left as they are because the upper-case code is already there (${leftAlone.map(_._1).mkString(", ")})" + leftAloneColumns += line + logger.warn(s"upperCaseEverywhere says: $line") + } + } + } + + val sections = List( + if (changedColumns.isEmpty) "Changed: none" else s"Changed: ${changedColumns.mkString("; ")}", + if (leftAloneColumns.isEmpty) "" else s"Left alone: ${leftAloneColumns.mkString("; ")}", + if (missingColumns.isEmpty) "" else s"Not on this database: ${missingColumns.mkString(", ")}", + if (unchangedColumns.isEmpty) "" else s"Nothing to change: ${unchangedColumns.mkString(", ")}" + ).filter(_.nonEmpty) + val comment = s"Upper-cased stored currency codes in ${currencyColumns.size} columns. ${sections.mkString(". ")}." + saveLog(name, APIUtil.gitCommit, isSuccessful = true, startDate, System.currentTimeMillis(), comment) + true + } +} diff --git a/obp-api/src/main/scala/code/api/v2_1_0/Http4s210.scala b/obp-api/src/main/scala/code/api/v2_1_0/Http4s210.scala index c462bad556..61337055c8 100644 --- a/obp-api/src/main/scala/code/api/v2_1_0/Http4s210.scala +++ b/obp-api/src/main/scala/code/api/v2_1_0/Http4s210.scala @@ -330,7 +330,7 @@ object Http4s210 { _ <- code.util.Helper.booleanToFuture( s"$InvalidTransactionRequestCurrency From Account Currency is ${fromAccount.currency}, but Requested Transaction Currency is: ${transDetailsJson.value.currency}", cc = Some(cc)) { - transDetailsJson.value.currency == fromAccount.currency + code.asset.CurrencyCodes.same(transDetailsJson.value.currency, fromAccount.currency) } parsedJson = com.openbankproject.commons.util.JsonAliases.parse(jsonBody) (createdTransactionRequest, _) <- TransactionRequestTypes.withName(transactionRequestTypeStr) match { diff --git a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala index 353c69bb58..f293021eba 100644 --- a/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala +++ b/obp-api/src/main/scala/code/api/v3_1_0/Http4s310.scala @@ -1403,7 +1403,7 @@ object Http4s310 { } yield { val fundsAvailable = (view.allowed_actions.exists(_ == CAN_QUERY_AVAILABLE_FUNDS), account.balance, account.currency) match { case (false, _, _) => "" - case (true, _, c) if c != ccy => "no" + case (true, _, c) if !code.asset.CurrencyCodes.same(c, ccy) => "no" case (true, b, _) if b.compare(available) >= 0 => "yes" case _ => "no" } diff --git a/obp-api/src/main/scala/code/api/v7_0_0/Http4s700.scala b/obp-api/src/main/scala/code/api/v7_0_0/Http4s700.scala index 9f8d60ff53..6c3262fe3b 100644 --- a/obp-api/src/main/scala/code/api/v7_0_0/Http4s700.scala +++ b/obp-api/src/main/scala/code/api/v7_0_0/Http4s700.scala @@ -207,6 +207,12 @@ object Http4s700 { // // Convention: val → resourceDocs +=, never the other way around. + // The granting Roles of a grant whose bank_id is in the body, naming the bank_id each bank Role + // was checked at: "CanCreateScopeAtOneBank at bank_id SYS or CanCreateScopeAtAnyBank". + private def grantingRolesAt(roles: List[ApiRole], bankId: String): String = + roles.map(role => if (role.requiresBankId && bankId.nonEmpty) s"$role at bank_id $bankId" else role.toString) + .mkString(" or ") + // Route: GET /obp/v7.0.0/root val root: HttpRoutes[IO] = HttpRoutes.of[IO] { case req @ GET -> `prefixPath` / "root" => @@ -538,7 +544,7 @@ object Http4s700 { grantingRoles = canCreateEntitlementAtOneBank :: canCreateEntitlementAtAnyBank :: Nil _ <- if (APIUtil.isSuperAdmin(user.userId)) Future.successful(()) else Helper.booleanToFuture( - UserHasMissingRoles + grantingRoles.mkString(" or "), failCode = 403, cc = Some(cc)) { + UserHasMissingRoles + grantingRolesAt(grantingRoles, body.bank_id), failCode = 403, cc = Some(cc)) { APIUtil.hasAtLeastOneEntitlement(body.bank_id, user.userId, grantingRoles) } // Bank ids are matched exactly, case included: a grant at a bank id naming no bank is a @@ -876,7 +882,7 @@ object Http4s700 { grantingRoles = ApiRole.canCreateScopeAtOneBank :: ApiRole.canCreateScopeAtAnyBank :: Nil _ <- if (APIUtil.isSuperAdmin(user.userId)) Future.successful(()) else Helper.booleanToFuture( - UserHasMissingRoles + grantingRoles.mkString(" or "), failCode = 403, cc = Some(cc)) { + UserHasMissingRoles + grantingRolesAt(grantingRoles, body.bank_id), failCode = 403, cc = Some(cc)) { APIUtil.hasAtLeastOneEntitlement(body.bank_id, user.userId, grantingRoles) } _ <- Helper.booleanToFuture(failMsg = BankNotFound, failCode = 404, cc = Some(cc)) { diff --git a/obp-api/src/main/scala/code/asset/AmountPrecision.scala b/obp-api/src/main/scala/code/asset/AmountPrecision.scala new file mode 100644 index 0000000000..450891ff8a --- /dev/null +++ b/obp-api/src/main/scala/code/asset/AmountPrecision.scala @@ -0,0 +1,131 @@ +/** +Open Bank Project - API +Copyright (C) 2011-2026, TESOBE GmbH. + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . + +Email: contact@tesobe.com +TESOBE GmbH. +Osloer Strasse 16/17 +Berlin 13359, Germany + +This product includes software developed at +TESOBE (http://www.tesobe.com/) + + */ +package code.asset + +import java.util.Locale + +import org.json4s.native.JsonMethods +import org.json4s.{JArray, JDecimal, JDouble, JInt, JLong, JObject, JString, JValue} + +import scala.util.Try + +/** + * This object finds an amount in a request that has more decimal places than its currency allows, + * such as 12.345 EUR or 100.5 JPY (ideas/ASSET_REGISTRY.md, section 4). + * + * OBP stores an amount in the currency's smallest unit, so before this check an amount with too many + * decimal places was silently cut off: 12.345 EUR was stored as 12.34. Refusing it instead means the + * caller learns that the amount cannot be held exactly, and no money disappears. + * + * An amount is found where a request pairs it with its currency: + * + * - In a JSON body, an object with a field named `amount` (in any letter case, so UK Open Banking's + * `Amount` counts) next to the field naming its currency. That is a field named `currency`, or else + * the object's only field whose name holds a currency code (see [[CurrencyCodes.isCurrencyKey]]). + * The amount may be a string or a JSON number; numbers are read exactly, never through a double. + * - In a query string, a parameter named `amount` next to a currency parameter, as in + * `funds-available?currency=EUR&amount=12.34`. + * + * Trailing zeros do not count, so `10.00` is a valid JPY amount. A code OBP does not know, an amount + * that is not a number, and a crypto asset (see [[AssetLookup.enforcedDecimalPlaces]]) are left to + * the endpoint. + */ +object AmountPrecision { + + /** An amount with more decimal places than its currency allows. */ + case class ExcessPrecision(amount: String, currency: String, decimalPlaces: Int, allowedDecimalPlaces: Int) { + def describe: String = + s"The amount $amount $currency has $decimalPlaces decimal place(s), but $currency allows at most $allowedDecimalPlaces." + } + + /** This returns the number of decimal places of an amount, not counting trailing zeros. */ + def decimalPlacesOf(amount: BigDecimal): Int = math.max(0, amount.bigDecimal.stripTrailingZeros.scale) + + private def isAmountKey(name: String): Boolean = name.toLowerCase(Locale.ROOT) == "amount" + + /** This checks one amount against its currency. */ + def check(currency: String, amount: BigDecimal, amountAsSent: String): Option[ExcessPrecision] = + AssetLookup.enforcedDecimalPlaces(currency).flatMap { allowed => + val places = decimalPlacesOf(amount) + if (places > allowed) Some(ExcessPrecision(amountAsSent, CurrencyCodes.normalise(currency), places, allowed)) + else None + } + + private def parseAmount(text: String): Option[BigDecimal] = Try(BigDecimal(text.trim)).toOption + + /** + * This returns the currency that an object's amount is in: the field named `currency`, or else the + * only field whose name holds a currency code. With several such fields and none named `currency` + * the pairing is ambiguous, so there is none. + */ + private def currencyOf(fields: List[(String, String)]): Option[String] = { + val currencyFields = fields.filter { case (name, _) => CurrencyCodes.isCurrencyKey(name) } + currencyFields.find { case (name, _) => name.toLowerCase(Locale.ROOT) == "currency" } + .orElse(if (currencyFields.size == 1) currencyFields.headOption else None) + .map(_._2) + } + + /** This finds the first amount with too many decimal places anywhere in a JSON value. */ + def inJson(json: JValue): Option[ExcessPrecision] = json match { + case JObject(fields) => + val stringFields = fields.collect { case (name, JString(value)) => (name, value) } + val here = for { + currency <- currencyOf(stringFields) + (amount, amountAsSent) <- fields.collectFirst { + case (name, JString(text)) if isAmountKey(name) => parseAmount(text).map(_ -> text) + case (name, JDecimal(number)) if isAmountKey(name) => Some(number -> number.toString) + case (name, JDouble(number)) if isAmountKey(name) => Some(BigDecimal(number.toString) -> number.toString) + case (name, JInt(number)) if isAmountKey(name) => Some(BigDecimal(number) -> number.toString) + case (name, JLong(number)) if isAmountKey(name) => Some(BigDecimal(number) -> number.toString) + }.flatten + found <- check(currency, amount, amountAsSent) + } yield found + here.orElse(fields.iterator.map { case (_, value) => inJson(value) }.collectFirst { case Some(found) => found }) + case JArray(items) => + items.iterator.map(inJson).collectFirst { case Some(found) => found } + case _ => None + } + + /** + * This finds the first amount with too many decimal places in a JSON request body. A body that + * does not mention an amount is not parsed, and one that is not JSON is left to the endpoint. + */ + def inJsonBody(body: String): Option[ExcessPrecision] = + if (body == null || !body.toLowerCase(Locale.ROOT).contains("amount")) None + else Try(JsonMethods.parse(body, useBigDecimalForDouble = true)).toOption.flatMap(inJson) + + /** This finds an amount with too many decimal places in query parameters. */ + def inQuery(parameters: Seq[(String, Option[String])]): Option[ExcessPrecision] = { + val values = parameters.collect { case (name, Some(value)) => (name, value) }.toList + for { + currency <- currencyOf(values) + amountAsSent <- values.collectFirst { case (name, value) if isAmountKey(name) => value } + amount <- parseAmount(amountAsSent) + found <- check(currency, amount, amountAsSent) + } yield found + } +} diff --git a/obp-api/src/main/scala/code/asset/AssetLookup.scala b/obp-api/src/main/scala/code/asset/AssetLookup.scala index b8a113872c..e7d0040169 100644 --- a/obp-api/src/main/scala/code/asset/AssetLookup.scala +++ b/obp-api/src/main/scala/code/asset/AssetLookup.scala @@ -38,15 +38,13 @@ import net.liftweb.util.Helpers.tryo * them from the asset registry ([[Assets]]), so that `APIUtil.isValidCurrencyISOCode` and * `Helper.currencyDecimalPlaces` no longer depend on a list built into the code. * - * The answers are the same as before the registry existed (ideas/ASSET_REGISTRY.md, progress step 6): + * Codes are matched ignoring letter case, so `eur` is EUR and gets EUR's decimal places + * ([[CurrencyCodes]]). Otherwise the answers are the ones OBP gave before the registry existed: * - * - Codes are matched exactly as written, so `EUR` is known and `eur` is not. Accepting any letter - * case is a later, deliberate change (section 4). * - The status of an asset is not checked yet: a suspended or retired code is still known. That - * also comes later, when `isValidCurrencyISOCode` becomes `isUsableAsset` (section 4). - * - Three spellings the built-in list accepts are not registry codes: `ada` (the registry holds - * `ADA`), and `lovelace` and `wei`, the smallest units of ADA and ETH. They stay known, with the - * built-in decimal places, until amounts in those units are converted (section 7, Part C). + * comes later, when `isValidCurrencyISOCode` becomes `isUsableAsset` (section 4). + * - `lovelace` and `wei`, the smallest units of ADA and ETH, are not registry codes but stay known, + * with the built-in decimal places, until amounts in those units are converted (section 7, Part C). * - A code the registry does not hold gets the built-in decimal places, as it did before. * * The registry is read once into memory and read again after any write through [[Assets]]. If it @@ -57,20 +55,23 @@ import net.liftweb.util.Helpers.tryo */ object AssetLookup extends MdcLoggable { - /** The spellings the built-in list accepts that are not codes in the registry. */ - val LegacySpellings: Set[String] = Set("ada", "lovelace", "wei") + /** The codes the built-in list accepts that the registry does not hold, upper case. */ + val LegacySpellings: Set[String] = Set("LOVELACE", "WEI") - /** The decimal places of every registered asset, keyed by its code exactly as stored. */ - @volatile private var decimalPlacesByCode: Option[Map[String, Int]] = None + /** What is kept in memory about a registered asset. */ + private case class Registered(decimalPlaces: Int, assetType: String) + + /** Every registered asset, keyed by its code exactly as stored. */ + @volatile private var assetsByCode: Option[Map[String, Registered]] = None /** Forgets what was read, so the next lookup reads the registry again. Called after every write. */ - def invalidate(): Unit = decimalPlacesByCode = None + def invalidate(): Unit = assetsByCode = None - private def registry: Option[Map[String, Int]] = decimalPlacesByCode.orElse { - tryo(Assets.getAssets().map(asset => asset.assetCode -> asset.decimalPlaces).toMap) match { + private def registeredAssets: Option[Map[String, Registered]] = assetsByCode.orElse { + tryo(Assets.getAssets().map(asset => asset.assetCode -> Registered(asset.decimalPlaces, asset.assetType)).toMap) match { case Full(loaded) if loaded.nonEmpty => - decimalPlacesByCode = Some(loaded) - decimalPlacesByCode + assetsByCode = Some(loaded) + assetsByCode case Full(_) => logger.debug("registry says: the asset registry is empty; using the built-in currency list") None @@ -80,13 +81,48 @@ object AssetLookup extends MdcLoggable { } } - /** Whether OBP knows this currency code, matched exactly as written. */ - def isKnownCode(code: String): Boolean = registry match { - case Some(codes) => codes.contains(code) || LegacySpellings.contains(code) - case None => APIUtil.builtInCurrencyCodes.contains(code) + private def registry: Option[Map[String, Int]] = registeredAssets.map(_.map { case (code, asset) => code -> asset.decimalPlaces }) + + private lazy val builtInCodesUpperCase: Set[String] = APIUtil.builtInCurrencyCodes.map(CurrencyCodes.normalise) + + /** Whether OBP knows this currency code, ignoring letter case. */ + def isKnownCode(code: String): Boolean = code != null && { + val normalised = CurrencyCodes.normalise(code) + registry match { + case Some(codes) => codes.contains(normalised) || LegacySpellings.contains(normalised) + case None => builtInCodesUpperCase.contains(normalised) + } } - /** The number of decimal places of this currency code, matched exactly as written. */ - def decimalPlaces(code: String): Int = - registry.flatMap(_.get(code)).getOrElse(Helper.builtInCurrencyDecimalPlaces(code)) + /** + * The crypto codes the built-in list knows, upper case. Without a registry these stand in for the + * registry's CRYPTO assets. + */ + private val BuiltInCryptoCodes: Set[String] = Set("XBT", "ADA", "ETH") ++ LegacySpellings + + /** + * This returns the number of decimal places an amount in this currency may have, when OBP enforces + * it, ignoring letter case. + * + * It is None for a code OBP does not know, and for crypto assets (`ADA`, `ETH`, `XBT`, and the units + * `lovelace` and `wei`). Their registered decimal places are still the built-in default of 2, not + * their real precision, which comes with the move to exact amount storage (ideas/ASSET_REGISTRY.md + * section 7, Parts A to C); enforcing 2 would refuse an ordinary payment of 0.001 ETH. + */ + def enforcedDecimalPlaces(code: String): Option[Int] = + if (code == null || !isKnownCode(code)) None + else { + val normalised = CurrencyCodes.normalise(code) + val isCrypto = registeredAssets.flatMap(_.get(normalised)) match { + case Some(asset) => asset.assetType == AssetTypes.CRYPTO + case None => BuiltInCryptoCodes.contains(normalised) + } + if (isCrypto) None else Some(decimalPlaces(normalised)) + } + + /** The number of decimal places of this currency code, ignoring letter case. */ + def decimalPlaces(code: String): Int = { + val normalised = CurrencyCodes.normalise(code) + registry.flatMap(_.get(normalised)).getOrElse(Helper.builtInCurrencyDecimalPlaces(normalised)) + } } diff --git a/obp-api/src/main/scala/code/asset/CurrencyCodes.scala b/obp-api/src/main/scala/code/asset/CurrencyCodes.scala new file mode 100644 index 0000000000..957ebff3ad --- /dev/null +++ b/obp-api/src/main/scala/code/asset/CurrencyCodes.scala @@ -0,0 +1,118 @@ +/** +Open Bank Project - API +Copyright (C) 2011-2026, TESOBE GmbH. + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . + +Email: contact@tesobe.com +TESOBE GmbH. +Osloer Strasse 16/17 +Berlin 13359, Germany + +This product includes software developed at +TESOBE (http://www.tesobe.com/) + + */ +package code.asset + +import java.util.Locale + +import code.api.util.CustomJsonFormats +import org.json4s.{Extraction, JArray, JObject, JString, JValue} + +import scala.collection.concurrent.TrieMap +import scala.util.Try + +/** + * This object holds OBP's rule that currency codes are case-insensitive: `eur`, `Eur` and `EUR` + * name the same asset (ideas/ASSET_REGISTRY.md, section 4). + * + * The rule is applied in two ways. Codes coming in are upper-cased before any endpoint sees them: + * ResourceDocMiddleware upper-cases the currency fields of a JSON request body and the currency query + * parameters, so new rows are stored upper case. Codes already stored are compared ignoring case, + * with [[same]], because a row written before this rule may hold a code in another case (`ada`, + * `lovelace` and `wei` were accepted only in lower case). Lookups in the registry ignore case too + * ([[AssetLookup]]). + * + * A field or query parameter holds a currency code when its name, ignoring case and underscores, + * ends in `currency` or `currencycode`: `currency`, `price_currency`, `from_currency_code` and + * UK Open Banking's `Currency` all do, `currency_status` does not. + */ +object CurrencyCodes { + + /** This returns the code in its stored form: trimmed and upper case. */ + def normalise(code: String): String = + if (code == null) code else code.trim.toUpperCase(Locale.ROOT) + + /** This returns true when the two codes name the same asset, whatever their letter case. */ + def same(first: String, second: String): Boolean = + first != null && second != null && normalise(first) == normalise(second) + + /** This returns true when a JSON field or query parameter with this name holds a currency code. */ + def isCurrencyKey(name: String): Boolean = { + val squashed = name.toLowerCase(Locale.ROOT).replace("_", "") + squashed.endsWith("currency") || squashed.endsWith("currencycode") + } + + /** + * A value is only upper-cased when it looks like a code, so free text that happens to sit in such + * a field is left alone. + */ + private val CodeLikeValue = "^[A-Za-z0-9]{2,12}$".r + + def looksLikeCode(value: String): Boolean = CodeLikeValue.findFirstIn(value.trim).isDefined + + /** + * This upper-cases the string values of the given fields wherever they appear in a JSON body. It + * works on the text, so amounts and every other value keep their exact form: parsing and + * re-rendering the body could turn a decimal amount sent as a JSON number into a double. + */ + def normaliseJsonBody(body: String, currencyFields: Set[String]): String = + if (currencyFields.isEmpty || body == null) body + else { + val field = currencyFields.toList.sorted.map(java.util.regex.Pattern.quote).mkString("|") + val pattern = ("\"(" + field + ")\"(\\s*:\\s*)\"([A-Za-z0-9]{2,12})\"").r + pattern.replaceAllIn(body, found => + java.util.regex.Matcher.quoteReplacement("\"" + found.group(1) + "\"" + found.group(2) + "\"" + normalise(found.group(3)) + "\"")) + } + + /** + * This returns the names of the fields in an endpoint's documented example body that hold a + * currency code as a string. Only those fields are upper-cased in that endpoint's requests, so a + * body the endpoint stores as given (a swagger document, a Dynamic Entity definition) is not + * rewritten because it happens to contain a field called `currency`. + */ + def currencyFieldsOf(exampleRequestBody: Any): Set[String] = { + def collect(json: JValue): Set[String] = json match { + case JObject(fields) => fields.flatMap { + case (name, JString(_)) if isCurrencyKey(name) => Set(name) + case (_, other) => collect(other) + }.toSet + case JArray(items) => items.flatMap(collect).toSet + case _ => Set.empty + } + val json: Option[JValue] = exampleRequestBody match { + case value: JValue => Some(value) + case null => None + case other => Try(Extraction.decompose(other)(CustomJsonFormats.formats)).toOption + } + json.map(collect).getOrElse(Set.empty) + } + + private val currencyFieldsByOperationId = TrieMap.empty[String, Set[String]] + + /** This is [[currencyFieldsOf]], worked out once per endpoint. */ + def currencyFieldsOf(operationId: String, exampleRequestBody: => Any): Set[String] = + currencyFieldsByOperationId.getOrElseUpdate(operationId, currencyFieldsOf(exampleRequestBody)) +} diff --git a/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnector.scala b/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnector.scala index 676eb3718f..acb76f3992 100644 --- a/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnector.scala +++ b/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnector.scala @@ -67,7 +67,7 @@ import code.kycdocuments.KycDocuments import code.kycmedias.KycMedias import code.kycstatuses.KycStatuses import code.meetings.Meetings -import code.metadata.counterparties.Counterparties +import code.metadata.counterparties.{Counterparties, MappedCounterparty} import code.model._ import code.model.dataAccess._ import code.productAttributeattribute.MappedProductAttribute @@ -1654,19 +1654,28 @@ object LocalMappedConnector extends Connector with MdcLoggable { otherAccountSecondaryRoutingAddress: String, callContext: Option[CallContext] ): OBPReturnType[Box[CounterpartyTrait]] = Future { - lazy val counterpartyFromRoutings= Counterparties.counterparties.vend.getCounterpartyByRoutings( - otherBankRoutingScheme: String, - otherBankRoutingAddress: String, - otherBranchRoutingScheme: String, - otherBranchRoutingAddress: String, - otherAccountRoutingScheme: String, - otherAccountRoutingAddress: String - ) + // Empty routing values must never be used as a lookup key: matching on ("", "") returns an + // arbitrary counterparty that happens to have the field empty, and a payment to it goes to + // whatever account that counterparty points at. Same guard as getOrCreateCounterparty. + lazy val counterpartyFromRoutings = + if (otherAccountRoutingScheme.trim.nonEmpty && otherAccountRoutingAddress.trim.nonEmpty) + Counterparties.counterparties.vend.getCounterpartyByRoutings( + otherBankRoutingScheme: String, + otherBankRoutingAddress: String, + otherBranchRoutingScheme: String, + otherBranchRoutingAddress: String, + otherAccountRoutingScheme: String, + otherAccountRoutingAddress: String + ) + else Empty - lazy val counterpartyFromSecondaryRouting = Counterparties.counterparties.vend.getCounterpartyBySecondaryRouting( - otherAccountSecondaryRoutingScheme: String, - otherAccountSecondaryRoutingAddress: String - ) + lazy val counterpartyFromSecondaryRouting = + if (otherAccountSecondaryRoutingScheme.trim.nonEmpty && otherAccountSecondaryRoutingAddress.trim.nonEmpty) + Counterparties.counterparties.vend.getCounterpartyBySecondaryRouting( + otherAccountSecondaryRoutingScheme: String, + otherAccountSecondaryRoutingAddress: String + ) + else Empty if(counterpartyFromRoutings.isDefined) { (counterpartyFromRoutings, callContext) @@ -2325,7 +2334,7 @@ object LocalMappedConnector extends Connector with MdcLoggable { settlementAccount.flatMap(settlementAccount => { val fromTransAmtSettlementAccount: BigDecimal = { // In the case we selected the default settlement account INCOMING_ACCOUNT_ID account and that the counterparty currency is different from EUR, we need to calculate the amount in EUR - if (settlementAccount._1.accountId.value == INCOMING_SETTLEMENT_ACCOUNT_ID && settlementAccount._1.currency != fromAccount.currency) { + if (settlementAccount._1.accountId.value == INCOMING_SETTLEMENT_ACCOUNT_ID && !code.asset.CurrencyCodes.same(settlementAccount._1.currency, fromAccount.currency)) { val rate = fx.exchangeRate(currency, settlementAccount._1.currency, Some(bankIdExchangeRate.bankId.value), callContext) Try(-fx.convert(amount, rate)).getOrElse(throw new Exception(s"$InvalidCurrency The requested currency conversion ($currency to ${settlementAccount._1.currency}) is not supported.")) } else fromTransAmt @@ -2350,7 +2359,7 @@ object LocalMappedConnector extends Connector with MdcLoggable { settlementAccount.flatMap(settlementAccount => { val toTransAmtSettlementAccount: BigDecimal = { // In the case we selected the default settlement account OUTGOING_ACCOUNT_ID account and that the counterparty currency is different from EUR, we need to calculate the amount in EUR - if (settlementAccount._1.accountId.value == OUTGOING_SETTLEMENT_ACCOUNT_ID && settlementAccount._1.currency != toAccount.currency) { + if (settlementAccount._1.accountId.value == OUTGOING_SETTLEMENT_ACCOUNT_ID && !code.asset.CurrencyCodes.same(settlementAccount._1.currency, toAccount.currency)) { val rate = fx.exchangeRate(currency, settlementAccount._1.currency, Some(bankIdExchangeRate.bankId.value), callContext) Try(fx.convert(amount, rate)).getOrElse(throw new Exception(s"$InvalidCurrency The requested currency conversion ($currency to ${settlementAccount._1.currency}) is not supported.")) } else toTransAmt @@ -2482,7 +2491,7 @@ object LocalMappedConnector extends Connector with MdcLoggable { settlementAccount.flatMap(settlementAccount => { val fromTransAmtSettlementAccount = { // In the case we selected the default settlement account INCOMING_ACCOUNT_ID account and that the counterparty currency is different from EUR, we need to calculate the amount in EUR - if (settlementAccount._1.accountId.value == INCOMING_SETTLEMENT_ACCOUNT_ID && settlementAccount._1.currency != fromAccount.currency) { + if (settlementAccount._1.accountId.value == INCOMING_SETTLEMENT_ACCOUNT_ID && !code.asset.CurrencyCodes.same(settlementAccount._1.currency, fromAccount.currency)) { val rate = fx.exchangeRate(transactionCurrency, settlementAccount._1.currency, Some(bankIdExchangeRate.bankId.value), callContext) Try(-fx.convert(amount, rate)).getOrElse(throw new Exception(s"$InvalidCurrency The requested currency conversion ($transactionCurrency to ${settlementAccount._1.currency}) is not supported.")) } else fromTransAmt @@ -2507,7 +2516,7 @@ object LocalMappedConnector extends Connector with MdcLoggable { settlementAccount.flatMap(settlementAccount => { val toTransAmtSettlementAccount = { // In the case we selected the default settlement account OUTGOING_ACCOUNT_ID account and that the counterparty currency is different from EUR, we need to calculate the amount in EUR - if (settlementAccount._1.accountId.value == OUTGOING_SETTLEMENT_ACCOUNT_ID && settlementAccount._1.currency != toAccount.currency) { + if (settlementAccount._1.accountId.value == OUTGOING_SETTLEMENT_ACCOUNT_ID && !code.asset.CurrencyCodes.same(settlementAccount._1.currency, toAccount.currency)) { val rate = fx.exchangeRate(transactionCurrency, settlementAccount._1.currency, Some(bankIdExchangeRate.bankId.value), callContext) Try(fx.convert(amount, rate)).getOrElse(throw new Exception(s"$InvalidCurrency The requested currency conversion ($transactionCurrency to ${settlementAccount._1.currency}) is not supported.")) } else toTransAmt @@ -5318,6 +5327,61 @@ object LocalMappedConnector extends Connector with MdcLoggable { Full((APIUtil.getPropsValue("transactionRequests_supported_types", "").split(",").map(x => TransactionRequestType(x)).toList, callContext)) } + /** + * This finds the counterparty a SIMPLE transaction request pays, when its challenge is answered. + * + * The counterparty was resolved from the payer's routing when the request was created, and its id + * is recorded on the request (`LocalMappedConnectorInternal.recordTransactionRequestCounterparty`). + * When the id is there, that counterparty is paid and nothing is resolved again. + * + * Requests created before the id was recorded have only the routing in their body, as the payer + * wrote it. Resolving that used to pay an unrelated account: the schemes were stored normalised + * (`obp` as `OBP`) but looked up as written, so the case-sensitive match missed, and the fallback + * to the secondary routing then matched ("", "") and returned any counterparty with an empty + * secondary routing, owned by anyone. For those requests the routing is now normalised the same + * way as at creation, an empty routing is never used as a lookup key, and only counterparties of + * the paying account are considered. If none matches, the payment fails rather than guess. + */ + private def simpleTransactionRequestCounterparty( + transactionRequest: TransactionRequest, + fromAccount: BankAccount, + bodyToSimple: TransactionRequestSimple, + callContext: Option[CallContext] + ): OBPReturnType[CounterpartyTrait] = { + val recordedCounterpartyId = Option(transactionRequest.counterparty_id).flatMap(id => Option(id.value)).filter(_.trim.nonEmpty) + recordedCounterpartyId match { + case Some(counterpartyId) => + NewStyle.function.getCounterpartyByCounterpartyId(CounterpartyId(counterpartyId), callContext) + case None => Future { + def normalisedScheme(scheme: String): String = net.liftweb.util.StringHelpers.snakify(scheme).toUpperCase + val ofPayingAccount = List[QueryParam[MappedCounterparty]]( + By(MappedCounterparty.mThisBankId, fromAccount.bankId.value), + By(MappedCounterparty.mThisAccountId, fromAccount.accountId.value) + ) + val byPrimaryRouting: Box[CounterpartyTrait] = + if (bodyToSimple.otherAccountRoutingScheme.trim.nonEmpty && bodyToSimple.otherAccountRoutingAddress.trim.nonEmpty) + MappedCounterparty.find(ofPayingAccount ++ List[QueryParam[MappedCounterparty]]( + By(MappedCounterparty.mOtherBankRoutingScheme, normalisedScheme(bodyToSimple.otherBankRoutingScheme)), + By(MappedCounterparty.mOtherBankRoutingAddress, bodyToSimple.otherBankRoutingAddress), + By(MappedCounterparty.mOtherBranchRoutingScheme, normalisedScheme(bodyToSimple.otherBranchRoutingScheme)), + By(MappedCounterparty.mOtherBranchRoutingAddress, bodyToSimple.otherBranchRoutingAddress), + By(MappedCounterparty.mOtherAccountRoutingScheme, normalisedScheme(bodyToSimple.otherAccountRoutingScheme)), + By(MappedCounterparty.mOtherAccountRoutingAddress, bodyToSimple.otherAccountRoutingAddress) + ): _*) + else Empty + val bySecondaryRouting: Box[CounterpartyTrait] = + if (byPrimaryRouting.isEmpty && + bodyToSimple.otherAccountSecondaryRoutingScheme.trim.nonEmpty && bodyToSimple.otherAccountSecondaryRoutingAddress.trim.nonEmpty) + MappedCounterparty.find(ofPayingAccount ++ List[QueryParam[MappedCounterparty]]( + By(MappedCounterparty.mOtherAccountSecondaryRoutingScheme, normalisedScheme(bodyToSimple.otherAccountSecondaryRoutingScheme)), + By(MappedCounterparty.mOtherAccountSecondaryRoutingAddress, bodyToSimple.otherAccountSecondaryRoutingAddress) + ): _*) + else Empty + (unboxFullOrFail(byPrimaryRouting or bySecondaryRouting, callContext, CounterpartyNotFoundByRoutings, 400), callContext) + } + } + } + override def createTransactionAfterChallengeV210(fromAccount: BankAccount, transactionRequest: TransactionRequest, callContext: Option[CallContext]): OBPReturnType[Box[TransactionRequest]] = { // OPEN_CORRIDOR_PROMISE never posts at challenge-answer: a successfully answered challenge // (four-eyes control) admits the promise into the corridor at PENDING, where it accumulates @@ -5432,17 +5496,7 @@ object LocalMappedConnector extends Connector with MdcLoggable { bodyToSimple <- NewStyle.function.tryons(s"$TransactionRequestDetailsExtractException It can not extract to $TransactionRequestBodyCounterpartyJSON", 400, callContext) { body.to_simple.get } - (toCounterparty, callContext) <- NewStyle.function.getCounterpartyByRoutings( - bodyToSimple.otherBankRoutingScheme, - bodyToSimple.otherBankRoutingAddress, - bodyToSimple.otherBranchRoutingScheme, - bodyToSimple.otherBranchRoutingAddress, - bodyToSimple.otherAccountRoutingScheme, - bodyToSimple.otherAccountRoutingAddress, - bodyToSimple.otherAccountSecondaryRoutingScheme, - bodyToSimple.otherAccountSecondaryRoutingAddress, - callContext - ) + (toCounterparty, callContext) <- simpleTransactionRequestCounterparty(transactionRequest, fromAccount, bodyToSimple, callContext) (toAccount, callContext) <- NewStyle.function.getBankAccountFromCounterparty(toCounterparty, true, callContext) counterpartyBody = TransactionRequestBodySimpleJsonV400( to = PostSimpleCounterpartyJson400( diff --git a/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala b/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala index e05e5c28f3..dcc45a9ecb 100644 --- a/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala +++ b/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnectorInternal.scala @@ -81,7 +81,28 @@ import scala.util.Random //Try to keep LocalMappedConnector smaller, so put OBP internal code here. these methods will not be exposed to CBS side. object LocalMappedConnectorInternal extends MdcLoggable { - + + /** + * This records on a newly created transaction request which counterparty it pays. + * + * SIMPLE and OPEN_CORRIDOR_PROMISE requests name their payee by routing, and the counterparty is + * found or created from that routing when the request is created. Without the id, the challenge + * step has to resolve the routing a second time, from the body as the payer wrote it, and that + * second resolution once paid an unrelated account (see `createTransactionAfterChallengeV210`). + * + * Only the mapped transaction-request table is written. With a core banking system connector the + * request is stored by the core banking system, there is no row here, and nothing is recorded: + * the failure is logged and the request carries on. + */ + def recordTransactionRequestCounterparty(transactionRequestId: TransactionRequestId, counterparty: CounterpartyTrait): Future[Unit] = Future { + TransactionRequests.transactionRequestProvider.vend + .saveTransactionRequestCounterpartyIdImpl(transactionRequestId, CounterpartyId(counterparty.counterpartyId)) match { + case Full(true) => () + case outcome => logger.warn(s"recordTransactionRequestCounterparty says: could not record counterparty " + + s"${counterparty.counterpartyId} on transaction request ${transactionRequestId.value}: $outcome") + } + } + def createTransactionRequestBGInternal( initiator: Option[User], paymentServiceType: PaymentServiceTypes, @@ -144,7 +165,7 @@ object LocalMappedConnectorInternal extends MdcLoggable { // Prevent default value for transaction request type (at least). _ <- Helper.booleanToFuture(s"$InvalidTransactionRequestCurrency From Account Currency is ${fromAccount.currency}, but Requested instructedAmount.currency is: ${transactionRequestBody.instructedAmount.currency}", cc = callContext) { - transactionRequestBody.instructedAmount.currency == fromAccount.currency + code.asset.CurrencyCodes.same(transactionRequestBody.instructedAmount.currency, fromAccount.currency) } // Get the threshold for a challenge. i.e. over what value do we require an out of Band security challenge to be sent? @@ -1264,6 +1285,7 @@ object LocalMappedConnectorInternal extends MdcLoggable { getScaMethodAtInstance(transactionRequestType.value).toOption, None, callContext) + _ <- recordTransactionRequestCounterparty(createdTransactionRequest.id, toCounterparty) } yield (createdTransactionRequest, callContext) } diff --git a/obp-api/src/main/scala/code/bankconnectors/opencorridor/OpenCorridorProcessor.scala b/obp-api/src/main/scala/code/bankconnectors/opencorridor/OpenCorridorProcessor.scala index 457da20e37..e51f507943 100644 --- a/obp-api/src/main/scala/code/bankconnectors/opencorridor/OpenCorridorProcessor.scala +++ b/obp-api/src/main/scala/code/bankconnectors/opencorridor/OpenCorridorProcessor.scala @@ -177,6 +177,7 @@ object OpenCorridorProcessor { None, callContext ) + _ <- code.bankconnectors.LocalMappedConnectorInternal.recordTransactionRequestCounterparty(createdTransactionRequest.id, toCounterparty) } yield (createdTransactionRequest, callContext) } diff --git a/obp-api/src/main/scala/code/bulkpayment/BulkPaymentHandler.scala b/obp-api/src/main/scala/code/bulkpayment/BulkPaymentHandler.scala index 5349f7823f..298caf9c80 100644 --- a/obp-api/src/main/scala/code/bulkpayment/BulkPaymentHandler.scala +++ b/obp-api/src/main/scala/code/bulkpayment/BulkPaymentHandler.scala @@ -76,8 +76,8 @@ object BulkPaymentHandler { body.payments.size <= maxItems } _ <- Helper.booleanToFuture(BulkPaymentCurrencyMismatch, 400, callContext) { - body.value.currency == sourceCurrency && - body.payments.forall(_.value.currency == sourceCurrency) + code.asset.CurrencyCodes.same(body.value.currency, sourceCurrency) && + body.payments.forall(payment => code.asset.CurrencyCodes.same(payment.value.currency, sourceCurrency)) } _ <- Helper.booleanToFuture(BulkDuplicateEndToEndId, 400, callContext) { body.payments.map(_.end_to_end_id).distinct.size == body.payments.size diff --git a/obp-api/src/main/scala/code/fx/fx.scala b/obp-api/src/main/scala/code/fx/fx.scala index ee372938c6..e4535684df 100644 --- a/obp-api/src/main/scala/code/fx/fx.scala +++ b/obp-api/src/main/scala/code/fx/fx.scala @@ -108,7 +108,7 @@ object fx extends MdcLoggable { inverseRate: Double ) implicit val formats = CustomJsonFormats.formats - fromCurrency == toCurrency match { + code.asset.CurrencyCodes.same(fromCurrency, toCurrency) match { case true => Some(1) case false => @@ -136,12 +136,12 @@ object fx extends MdcLoggable { } def getFallbackExchangeRate2nd(fromCurrency: String, toCurrency: String): Option[Double] = { - if (fromCurrency == toCurrency) { + if (code.asset.CurrencyCodes.same(fromCurrency, toCurrency)) { Some(1) } else { //logger.debug(s"fromAmount is $fromAmount, toCurrency is ${toCurrency}") val rate: Option[Double] = try { - Some(fallbackExchangeRates.get(fromCurrency).get(toCurrency)) + Some(fallbackExchangeRates.get(code.asset.CurrencyCodes.normalise(fromCurrency)).get(code.asset.CurrencyCodes.normalise(toCurrency))) } catch { case e: NoSuchElementException => None diff --git a/obp-api/src/main/scala/code/telemetry/TelemetryBindings.scala b/obp-api/src/main/scala/code/telemetry/TelemetryBindings.scala index e0c8987884..dcbe15a0eb 100644 --- a/obp-api/src/main/scala/code/telemetry/TelemetryBindings.scala +++ b/obp-api/src/main/scala/code/telemetry/TelemetryBindings.scala @@ -48,8 +48,13 @@ object TelemetryBindings { bindMessageDocs() bindRedisLogger() bindIpPenalties() + bindDatabaseConnectionHoldWatch() } + /** How many database connections are out of the pool right now and held past the warning limit. */ + private def bindDatabaseConnectionHoldWatch(): Unit = + Telemetry.gauge("obp.api.database.connection.held_too_long")(code.util.DatabaseConnectionHoldWatch.heldTooLongCount.toDouble) + /** How many addresses are under an operator's temporary limit. Never which ones: an address is not a tag. */ private def bindIpPenalties(): Unit = Telemetry.gauge("obp.api.ip_penalties.active")(code.api.util.IpPenalties.active().size.toDouble) diff --git a/obp-api/src/main/scala/code/transactionrequests/MappedTransactionRequestProvider.scala b/obp-api/src/main/scala/code/transactionrequests/MappedTransactionRequestProvider.scala index feb8437f94..62e8145a96 100644 --- a/obp-api/src/main/scala/code/transactionrequests/MappedTransactionRequestProvider.scala +++ b/obp-api/src/main/scala/code/transactionrequests/MappedTransactionRequestProvider.scala @@ -256,6 +256,14 @@ object MappedTransactionRequestProvider extends TransactionRequestProvider with } } + override def saveTransactionRequestCounterpartyIdImpl(transactionRequestId: TransactionRequestId, counterpartyId: CounterpartyId): Box[Boolean] = { + val mappedTransactionRequest = MappedTransactionRequest.find(By(MappedTransactionRequest.mTransactionRequestId, transactionRequestId.value)) + mappedTransactionRequest match { + case Full(tr: MappedTransactionRequest) => Full(tr.mCounterpartyId(counterpartyId.value).save) + case _ => Failure(s"Couldn't find transaction request ${transactionRequestId} to set its counterparty id") + } + } + override def saveTransactionRequestDescriptionImpl(transactionRequestId: TransactionRequestId, description: String): Box[Boolean] = { val mappedTransactionRequest = MappedTransactionRequest.find(By(MappedTransactionRequest.mTransactionRequestId, transactionRequestId.value)) mappedTransactionRequest match { diff --git a/obp-api/src/main/scala/code/transactionrequests/TransactionRequests.scala b/obp-api/src/main/scala/code/transactionrequests/TransactionRequests.scala index f046bad45b..acb87ca15e 100644 --- a/obp-api/src/main/scala/code/transactionrequests/TransactionRequests.scala +++ b/obp-api/src/main/scala/code/transactionrequests/TransactionRequests.scala @@ -103,6 +103,13 @@ trait TransactionRequestProvider extends MdcLoggable { def saveTransactionRequestChallengeImpl(transactionRequestId: TransactionRequestId, challenge: TransactionRequestChallenge): Box[Boolean] def saveTransactionRequestStatusImpl(transactionRequestId: TransactionRequestId, status: String): Box[Boolean] def saveTransactionRequestDescriptionImpl(transactionRequestId: TransactionRequestId, description: String): Box[Boolean] + /** + * This records which counterparty a transaction request pays, for request types whose body names + * the payee by routing (SIMPLE, OPEN_CORRIDOR_PROMISE). The counterparty is resolved when the + * request is created; recording its id lets the challenge step pay exactly that counterparty + * instead of resolving the routing a second time. + */ + def saveTransactionRequestCounterpartyIdImpl(transactionRequestId: TransactionRequestId, counterpartyId: CounterpartyId): Box[Boolean] def bulkDeleteTransactionRequestsByTransactionId(transactionId: TransactionId): Boolean def bulkDeleteTransactionRequests(): Boolean } diff --git a/obp-api/src/main/scala/code/util/DatabaseConnectionHoldWatch.scala b/obp-api/src/main/scala/code/util/DatabaseConnectionHoldWatch.scala new file mode 100644 index 0000000000..e28336b74b --- /dev/null +++ b/obp-api/src/main/scala/code/util/DatabaseConnectionHoldWatch.scala @@ -0,0 +1,201 @@ +/** +Open Bank Project - API +Copyright (C) 2011-2026, TESOBE GmbH. + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . + +Email: contact@tesobe.com +TESOBE GmbH. +Osloer Strasse 16/17 +Berlin 13359, Germany + +This product includes software developed at +TESOBE (http://www.tesobe.com/) + + */ + +package code.util + +import code.api.util.APIUtil +import code.util.Helper.MdcLoggable +import com.zaxxer.hikari.HikariDataSource + +import java.lang.reflect.{InvocationHandler, InvocationTargetException, Method, Proxy => JProxy} +import java.sql.Connection +import java.util.concurrent.{ConcurrentHashMap, Executors, TimeUnit} +import java.util.concurrent.atomic.AtomicBoolean +import scala.jdk.CollectionConverters._ +import scala.util.Try + +/** + * This object writes a warning to the log when a database connection has been out of the pool for + * longer than a set time, and says which code is holding it. + * + * The problem it addresses: when the connection pool runs out, every request that needs a + * connection waits and then fails, and nothing in the log says which work is holding the + * connections. A request cut off by the endpoint timeout (`long_endpoint_timeout`) gets its 504 + * answer, but the database work underneath it carries on, and keeps its connection, until the + * query returns, which can take many minutes. + * + * HikariCP has a leak detection of its own, but it writes through its own logger, so its warnings + * never reach the log cache, and the log cache would drop the stack trace even if they did. This + * object writes through MdcLoggable and puts the stack frames into the text of the message. + * + * Taking a connection costs only an entry in a map: no stack trace is captured then. Instead, once + * a connection has been held too long, the sweep reads the stack of the thread that took it, as it + * is at that moment. For a query that is still running, that is exactly the code waiting on it. A + * connection taken by one thread and then used by others (the request transaction in + * RequestScopeConnection works like that) may have a thread that has moved on; the warning then + * says that the thread is no longer running OBP-API code. + * + * When a connection that was warned about goes back to the pool, a second warning gives the total + * time it was held, so a reader can pair the two by the connection label. + * + * The prop `database_connection_hold_warning_seconds` sets the time (default 60 seconds); 0 turns + * the watch off, and connections are then handed out unwrapped. + */ +object DatabaseConnectionHoldWatch extends MdcLoggable { + + /** How long a connection may be held before a warning is written. 0 or less turns the watch off. */ + lazy val holdWarningSeconds: Long = + APIUtil.getPropsAsLongValue("database_connection_hold_warning_seconds", 60L) + + def enabled: Boolean = holdWarningSeconds > 0 + + /** Frames of OBP-API's own code to put in a warning; the rest are library and runtime frames. */ + private val maximumOwnFramesShown = 15 + + /** One connection out of the pool. `warned` is set by the sweep once the warning has been written. */ + private final class Checkout(val takenAtMillis: Long, val thread: Thread) { + @volatile var warned: Boolean = false + val label: String = Integer.toHexString(System.identityHashCode(this)) + def heldSeconds(nowMillis: Long): Long = (nowMillis - takenAtMillis) / 1000 + } + + private val checkouts: java.util.Set[Checkout] = ConcurrentHashMap.newKeySet[Checkout]() + + private val sweeperStarted = new AtomicBoolean(false) + + /** How many connections are out of the pool right now and have been held longer than the limit. */ + def heldTooLongCount: Int = heldTooLongCountAt(System.currentTimeMillis()) + + private[util] def heldTooLongCountAt(nowMillis: Long): Int = { + val limitMillis = holdWarningSeconds * 1000 + checkouts.asScala.count(checkout => nowMillis - checkout.takenAtMillis >= limitMillis) + } + + private lazy val holdWarnings = code.telemetry.Telemetry.counter("obp.api.database.connection.hold_warnings") + + /** + * Returns `connection` wrapped so that closing it, which returns it to the pool, is recorded. + * Every other call goes straight to `connection`. + */ + def watch(connection: Connection): Connection = + if (!enabled) connection + else { + startSweeperOnce() + val checkout = new Checkout(System.currentTimeMillis(), Thread.currentThread()) + checkouts.add(checkout) + JProxy.newProxyInstance( + classOf[Connection].getClassLoader, + Array(classOf[Connection]), + new InvocationHandler { + def invoke(proxy: Any, method: Method, args: Array[AnyRef]): AnyRef = + method.getName match { + case "equals" => java.lang.Boolean.valueOf(proxy.asInstanceOf[AnyRef] eq args(0)) + case "hashCode" => java.lang.Integer.valueOf(System.identityHashCode(proxy)) + case name => + if (name == "close") released(checkout) + try { + if (args == null) method.invoke(connection) else method.invoke(connection, args: _*) + } catch { + case e: InvocationTargetException => throw Option(e.getCause).getOrElse(e) + } + } + } + ).asInstanceOf[Connection] + } + + private def released(checkout: Checkout): Unit = + if (checkouts.remove(checkout) && checkout.warned) { + logger.warn( + s"DatabaseConnectionHoldWatch says: database connection ${checkout.label}, reported earlier as held " + + s"too long, went back to the pool after ${checkout.heldSeconds(System.currentTimeMillis())}s." + ) + } + + private def startSweeperOnce(): Unit = + if (sweeperStarted.compareAndSet(false, true)) { + val scheduler = Executors.newSingleThreadScheduledExecutor { runnable => + val thread = new Thread(runnable, "database-connection-hold-watch") + thread.setDaemon(true) + thread + } + val sweepIntervalSeconds = math.max(1L, math.min(10L, holdWarningSeconds / 2)) + scheduler.scheduleWithFixedDelay(() => { sweep(); () }, sweepIntervalSeconds, sweepIntervalSeconds, TimeUnit.SECONDS) + logger.info(s"DatabaseConnectionHoldWatch says: started; warning after ${holdWarningSeconds}s, checking every ${sweepIntervalSeconds}s") + } + + /** Writes a warning for each connection newly past the limit at `nowMillis`, and returns the warnings. */ + private[util] def sweep(nowMillis: Long = System.currentTimeMillis()): List[String] = + try { + val limitMillis = holdWarningSeconds * 1000 + checkouts.asScala.toList + .filter(checkout => !checkout.warned && nowMillis - checkout.takenAtMillis >= limitMillis) + .map { checkout => + checkout.warned = true + holdWarnings.increment() + val warning = warningFor(checkout, nowMillis) + logger.warn(warning) + warning + } + } catch { + // The sweep must keep running: an exception would cancel the scheduled task for good. + case e: Throwable => + logger.error(s"DatabaseConnectionHoldWatch says: sweep failed: ${e.getMessage}") + Nil + } + + private def warningFor(checkout: Checkout, nowMillis: Long): String = { + val thread = checkout.thread + val frames = thread.getStackTrace.toList + val ownFrames = frames.filter { frame => + val className = frame.getClassName + (className.startsWith("code.") || className.startsWith("bootstrap.")) && + !className.startsWith("code.util.DatabaseConnectionHoldWatch$") + } + val whereNow = + if (ownFrames.isEmpty) + "That thread is no longer running OBP-API code: it handed the connection on to other work, so where the connection is used now is not known." + else { + val shown = ownFrames.take(maximumOwnFramesShown).map(frame => s" at $frame") + val innermost = frames.headOption.map(frame => s" innermost frame: $frame").toList + ("Where that thread is now:" :: innermost ::: shown).mkString("\n") + } + s"DatabaseConnectionHoldWatch says: database connection ${checkout.label} has been out of the pool for " + + s"${checkout.heldSeconds(nowMillis)}s (limit ${holdWarningSeconds}s), taken by thread ${thread.getName} " + + s"(now ${thread.getState}). ${poolSummary}\n$whereNow" + } + + /** The pool whose figures a warning reports; set by CustomDBVendor once the pool exists. */ + @volatile var pool: Option[HikariDataSource] = None + + private def poolSummary: String = + pool.flatMap(dataSource => Option(dataSource.getHikariPoolMXBean)).flatMap { poolBean => + Try( + s"Pool: ${poolBean.getActiveConnections} in use, ${poolBean.getIdleConnections} idle, " + + s"${poolBean.getThreadsAwaitingConnection} waiting for one." + ).toOption + }.getOrElse("") +} diff --git a/obp-api/src/test/scala/code/api/sweep/AuthSweepTest.scala b/obp-api/src/test/scala/code/api/sweep/AuthSweepTest.scala index 73b81365a1..c28b8bac4a 100644 --- a/obp-api/src/test/scala/code/api/sweep/AuthSweepTest.scala +++ b/obp-api/src/test/scala/code/api/sweep/AuthSweepTest.scala @@ -143,8 +143,8 @@ class AuthSweepTest extends ServerSetupWithTestData with DefaultUsers with Sweep * Deviations that are deliberate, with the reason each one is not a defect. * * A signed-off list rather than a hard zero, for the same reason KryoGoldenCompatTest keeps - * knownDrift: a permanently red suite is one people learn to ignore, and the two entries here - * are both behaviour somebody chose and wrote down. Anything NOT listed still fails, and + * knownDrift: a permanently red suite is one people learn to ignore, and an entry here is + * behaviour somebody chose and wrote down. Anything NOT listed still fails, and * adding a line costs a written justification. */ private val expectedAuthDeviation: Map[String, String] = Map( @@ -152,13 +152,7 @@ class AuthSweepTest extends ServerSetupWithTestData with DefaultUsers with Sweep ("Refuses with OBP-20311 'The Request is not signed' -- JWS request signing, a third " + "authentication mechanism alongside user and application. ResourceDoc has no way to " + "declare it: authMode covers user/application only, so neither the doc nor this sweep " + - "can express the requirement. The 401 is correct; only the message differs."), - "OBPv4.0.0-createTransactionRequestFreeForm" -> - ("Answers 400 InsufficientAuthorisationToCreateTransactionRequest rather than 403. The " + - "endpoint deliberately does no upfront view/role check and delegates the decision to " + - "checkAuthorisationToCreateTransactionRequest inside the connector -- its own comment " + - "says so, and an existing test depends on it. Whether an authorisation failure ought to " + - "be 400 at all is a product question, not something to change from inside a sweep.") + "can express the requirement. The 401 is correct; only the message differs.") ) /** Which exemptions were actually needed this run -- see the stale-entry scenario below. */ diff --git a/obp-api/src/test/scala/code/api/util/http4s/ResourceDocMatcherTest.scala b/obp-api/src/test/scala/code/api/util/http4s/ResourceDocMatcherTest.scala index e2ed556298..32bdea6ee9 100644 --- a/obp-api/src/test/scala/code/api/util/http4s/ResourceDocMatcherTest.scala +++ b/obp-api/src/test/scala/code/api/util/http4s/ResourceDocMatcherTest.scala @@ -572,4 +572,67 @@ class ResourceDocMatcherTest extends FeatureSpec with Matchers with GivenWhenThe result should be(None) } } + + feature("ResourceDocMatcher - transaction-request types and other enum values in URLs") { + + // Transaction-request types and SCA methods are written in capitals, like placeholders. + // These scenarios check that the matcher treats them as fixed words, so one type's + // template never claims another type's request. + val transactionRequestsUrl = "/banks/BANK_ID/accounts/ACCOUNT_ID/VIEW_ID/transaction-request-types/%s/transaction-requests" + def transactionRequestsPath(transactionRequestType: String): Uri.Path = + Uri.Path.unsafeFromString(s"$base/banks/gh.29.uk/accounts/8ca8a7e4/owner/transaction-request-types/$transactionRequestType/transaction-requests") + + scenario("A type registered first does not claim another type's request", ResourceDocMatcherTag) { + Given("a MOBILE_WALLET doc registered before a UTILITY doc that requires a role") + val utilityDoc = createResourceDoc("POST", transactionRequestsUrl.format("UTILITY"), "createTransactionRequestUtility") + .copy(roles = Some(List(code.api.util.ApiRole.canCreateUtilityVendResult))) + val resourceDocs = ArrayBuffer( + createResourceDoc("POST", transactionRequestsUrl.format("MOBILE_WALLET"), "createTransactionRequestMobileWallet"), + utilityDoc + ) + + When("a UTILITY request is matched") + val result = ResourceDocMatcher.findResourceDoc("POST", transactionRequestsPath("UTILITY"), resourceDocs) + + Then("the UTILITY doc is chosen, so the middleware enforces the UTILITY doc's role") + result.map(_.partialFunctionName) shouldBe Some("createTransactionRequestUtility") + result.flatMap(_.roles) shouldBe Some(List(code.api.util.ApiRole.canCreateUtilityVendResult)) + } + + scenario("A type with no doc in this version matches nothing", ResourceDocMatcherTag) { + Given("a catalog with only a MOBILE_WALLET doc") + val resourceDocs = ArrayBuffer( + createResourceDoc("POST", transactionRequestsUrl.format("MOBILE_WALLET"), "createTransactionRequestMobileWallet") + ) + + When("a CARDANO request is matched") + val result = ResourceDocMatcher.findResourceDoc("POST", transactionRequestsPath("CARDANO"), resourceDocs) + + Then("no doc matches, so the request can fall through to an older version that serves CARDANO") + result shouldBe None + } + + scenario("A specific type's doc beats a placeholder doc registered before it", ResourceDocMatcherTag) { + Given("a TRANSACTION_REQUEST_TYPE placeholder doc registered before a SEPA doc") + val resourceDocs = ArrayBuffer( + createResourceDoc("POST", transactionRequestsUrl.format("TRANSACTION_REQUEST_TYPE"), "createTransactionRequestAnyType"), + createResourceDoc("POST", transactionRequestsUrl.format("SEPA"), "createTransactionRequestSepa") + ) + + Then("a SEPA request gets the SEPA doc") + ResourceDocMatcher.findResourceDoc("POST", transactionRequestsPath("SEPA"), resourceDocs) + .map(_.partialFunctionName) shouldBe Some("createTransactionRequestSepa") + + And("a type with no doc of its own still gets the placeholder doc") + ResourceDocMatcher.findResourceDoc("POST", transactionRequestsPath("FREE_FORM"), resourceDocs) + .map(_.partialFunctionName) shouldBe Some("createTransactionRequestAnyType") + } + + scenario("Every transaction-request type and SCA method is a fixed word", ResourceDocMatcherTag) { + val enumValues = + com.openbankproject.commons.model.enums.TransactionRequestTypes.values.map(_.toString) ++ + com.openbankproject.commons.model.enums.StrongCustomerAuthentication.values.map(_.toString) + enumValues.filterNot(ResourceDocMatcher.literalAllCapsSegments.contains) shouldBe empty + } + } } diff --git a/obp-api/src/test/scala/code/api/util/http4s/ResourceDocSelfResolveTest.scala b/obp-api/src/test/scala/code/api/util/http4s/ResourceDocSelfResolveTest.scala new file mode 100644 index 0000000000..e35016df93 --- /dev/null +++ b/obp-api/src/test/scala/code/api/util/http4s/ResourceDocSelfResolveTest.scala @@ -0,0 +1,119 @@ +/** +Open Bank Project - API +Copyright (C) 2011-2026, TESOBE GmbH. + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . + +Email: contact@tesobe.com +TESOBE GmbH. +Osloer Strasse 16/17 +Berlin 13359, Germany + +This product includes software developed at +TESOBE (http://www.tesobe.com/) + + */ + +package code.api.util.http4s + +import code.api.util.APIUtil.ResourceDoc +import code.setup.ServerSetup +import org.http4s.Uri +import org.scalatest.Tag + +import scala.collection.mutable.ArrayBuffer + +/** + * This test checks that every registered ResourceDoc is the doc the middleware picks for + * a request to that doc's own URL. + * + * ResourceDocMiddleware does not know which http4s route will serve a request: it re-matches + * the URL against the ResourceDoc templates and takes the doc it finds for roles, the login + * requirement, the bank/account/view lookups, the operationId (telemetry, enable/disable, + * JSON-schema and auth-type validation), Force-Error and currency upper-casing. If one + * template also matches another doc's URL and wins, the other endpoint is validated and + * reported under the wrong doc while its own handler still runs, so its own tests pass. + * That happened with `.../transaction-request-types/MOBILE_WALLET/transaction-requests` in + * v7.0.0, which matched UTILITY, BULK and OPEN_CORRIDOR_PROMISE requests, and with HOLD in + * v6.0.0, which matched CARDANO and Ethereum requests. + * + * For each version's middleware catalog, the test sends the doc's template itself as the + * request path (placeholders such as `BANK_ID` stand in for values, and every template + * matches them) and asserts the matcher returns that same doc. Docs that share a verb and + * URL with another doc in the same catalog are skipped, as no matcher could tell them apart. + */ +class ResourceDocSelfResolveTest extends ServerSetup { + + object ResourceDocSelfResolveTag extends Tag("ResourceDocSelfResolve") + + /** + * Each OBP version's `resourceDocs` buffer is filled when its nested `Implementations` object + * initialises, which normally waits for the first request. Naming the object here forces that. + */ + private def loaded(implementations: Any, docs: ArrayBuffer[ResourceDoc]): ArrayBuffer[ResourceDoc] = docs + + /** Each catalog is the `resourceDocs` buffer a version passes to `ResourceDocMiddleware.apply`. */ + private def catalogs: List[(String, ArrayBuffer[ResourceDoc])] = List( + "v1.2.1" -> loaded(code.api.v1_2_1.Http4s121.Implementations1_2_1, code.api.v1_2_1.Http4s121.resourceDocs), + "v1.3.0" -> loaded(code.api.v1_3_0.Http4s130.Implementations1_3_0, code.api.v1_3_0.Http4s130.resourceDocs), + "v1.4.0" -> loaded(code.api.v1_4_0.Http4s140.Implementations1_4_0, code.api.v1_4_0.Http4s140.resourceDocs), + "v2.0.0" -> loaded(code.api.v2_0_0.Http4s200.Implementations2_0_0, code.api.v2_0_0.Http4s200.resourceDocs), + "v2.1.0" -> loaded(code.api.v2_1_0.Http4s210.Implementations2_1_0, code.api.v2_1_0.Http4s210.resourceDocs), + "v2.2.0" -> loaded(code.api.v2_2_0.Http4s220.Implementations2_2_0, code.api.v2_2_0.Http4s220.resourceDocs), + "v3.0.0" -> loaded(code.api.v3_0_0.Http4s300.Implementations3_0_0, code.api.v3_0_0.Http4s300.resourceDocs), + "v3.1.0" -> loaded(code.api.v3_1_0.Http4s310.Implementations3_1_0, code.api.v3_1_0.Http4s310.resourceDocs), + "v4.0.0" -> loaded(code.api.v4_0_0.Http4s400.Implementations4_0_0, code.api.v4_0_0.Http4s400.resourceDocs), + "v5.0.0" -> loaded(code.api.v5_0_0.Http4s500.Implementations5_0_0, code.api.v5_0_0.Http4s500.resourceDocs), + "v5.1.0" -> loaded(code.api.v5_1_0.Http4s510.Implementations5_1_0, code.api.v5_1_0.Http4s510.resourceDocs), + "v6.0.0" -> loaded(code.api.v6_0_0.Http4s600.Implementations6_0_0, code.api.v6_0_0.Http4s600.resourceDocs), + "v7.0.0" -> loaded(code.api.v7_0_0.Http4s700.Implementations7_0_0, code.api.v7_0_0.Http4s700.resourceDocs), + "Berlin Group v1.3" -> code.api.berlin.group.v1_3.Http4sBGv13.resourceDocs, + "Berlin Group v2" -> code.api.berlin.group.v2.Http4sBGv2.resourceDocs, + "UK Open Banking v2.0.0" -> code.api.UKOpenBanking.v2_0_0.Http4sUKOBv200.resourceDocs, + "UK Open Banking v3.1.0" -> code.api.UKOpenBanking.v3_1_0.Http4sUKOBv310.resourceDocs, + "UK Open Banking v4.0.1" -> code.api.UKOpenBanking.v4_0_1.Http4sUKOBv401.resourceDocs + ) + + private def describe(doc: ResourceDoc): String = + s"${doc.partialFunctionName} (${doc.requestVerb} ${doc.requestUrl})" + + /** Returns one line per doc whose own URL resolves to a different doc, or to none. */ + private def misresolved(docs: ArrayBuffer[ResourceDoc]): List[String] = { + val index = ResourceDocMatcher.buildIndex(docs) + val sameVerbAndUrlCount = docs.groupBy(doc => (doc.requestVerb.toUpperCase, doc.requestUrl)).mapValues(_.size) + docs.toList + .filter(doc => sameVerbAndUrlCount((doc.requestVerb.toUpperCase, doc.requestUrl)) == 1) + .flatMap { doc => + val version = doc.implementedInApiVersion + val path = Uri.Path.unsafeFromString(s"/${version.urlPrefix}/${version}${doc.requestUrl}") + ResourceDocMatcher.findResourceDoc(doc.requestVerb, path, index) match { + case Some(resolved) if resolved eq doc => None + case Some(resolved) => Some(s"${describe(doc)} resolves to ${describe(resolved)}") + case None => Some(s"${describe(doc)} resolves to no doc") + } + } + } + + feature("Every ResourceDoc is the doc the middleware picks for its own URL") { + catalogs.foreach { case (label, docs) => + scenario(s"$label: each doc's own URL resolves to that doc", ResourceDocSelfResolveTag) { + docs should not be empty + val problems = misresolved(docs) + withClue(s"\n${problems.mkString("\n")}\n") { + problems shouldBe empty + } + } + } + } +} diff --git a/obp-api/src/test/scala/code/api/v2_2_0/ExchangeRateTest.scala b/obp-api/src/test/scala/code/api/v2_2_0/ExchangeRateTest.scala index 2ac2e24576..e1482eca92 100644 --- a/obp-api/src/test/scala/code/api/v2_2_0/ExchangeRateTest.scala +++ b/obp-api/src/test/scala/code/api/v2_2_0/ExchangeRateTest.scala @@ -32,6 +32,7 @@ import code.api.util.APIUtil.OAuth._ import code.api.util.ApiRole import code.api.util.ErrorMessages.InvalidISOCurrencyCode import code.consumer.Consumers +import code.entitlement.Entitlement import code.scope.Scope import code.setup.DefaultUsers import com.github.dwickern.macros.NameOf.nameOf @@ -50,6 +51,7 @@ class ExchangeRateTest extends V220ServerSetup with DefaultUsers { */ object VersionOfApi extends Tag(ApiVersion.v2_2_0.toString) object ApiEndpoint1 extends Tag(nameOf(Http4s220.Implementations2_2_0.getCurrentFxRate)) + object ApiEndpoint2 extends Tag(nameOf(Http4s220.Implementations2_2_0.createFx)) override def beforeAll(): Unit = { super.beforeAll() @@ -92,7 +94,28 @@ class ExchangeRateTest extends V220ServerSetup with DefaultUsers { responseGet.code should equal(400) responseGet.body.extract[ErrorMessage].message should startWith (InvalidISOCurrencyCode) } - - } + scenario("Currency codes in any letter case name the same currency", VersionOfApi, ApiEndpoint1, ApiEndpoint2) { + val testBank = testBankId1 + val consumerId = Consumers.consumers.vend.getConsumerByConsumerKey(user1.get._1.key).map(_.id.get.toString).getOrElse("") + Scope.scope.vend.addScope(testBank.value, consumerId, ApiRole.canReadFx.toString()) + Entitlement.entitlement.vend.addEntitlement(testBank.value, resourceUser1.userId, ApiRole.canCreateFxRate.toString()) + + When("We create an FX rate with the currency codes in lower case") + val body = + s"""{"bank_id":"${testBank.value}","from_currency_code":"eur","to_currency_code":"usd", + |"conversion_value":1.5,"inverse_conversion_value":0.6666666666666666,"effective_date":"2026-10-06T00:00:00Z"}""".stripMargin + val responsePut = makePutRequest((v2_2Request / "banks" / testBank.value / "fx").PUT <@ (user1), body) + Then("We should get a 201, and the codes are stored in upper case") + responsePut.code should equal(201) + (responsePut.body \ "from_currency_code").extract[String] should equal("EUR") + (responsePut.body \ "to_currency_code").extract[String] should equal("USD") + + When("We get the rate, naming the currencies in lower case") + val responseGet = makeGetRequest((v2_2Request / "banks" / testBank.value / "fx" / "eur" / "usd").GET <@ (user1)) + Then("We should get the rate we created") + responseGet.code should equal(200) + (responseGet.body \ "conversion_value").extract[Double] should equal(1.5) + } + } } diff --git a/obp-api/src/test/scala/code/api/v3_1_0/FundsAvailableTest.scala b/obp-api/src/test/scala/code/api/v3_1_0/FundsAvailableTest.scala index 1dbe5492fd..68ce17222b 100644 --- a/obp-api/src/test/scala/code/api/v3_1_0/FundsAvailableTest.scala +++ b/obp-api/src/test/scala/code/api/v3_1_0/FundsAvailableTest.scala @@ -130,13 +130,26 @@ class FundsAvailableTest extends V310ServerSetup { Then("We should get a 200") response310.code should equal(200) + When("We make a request v3.1.0 with the currency in lower case") + val response310_lower_case_ccy = makeGetRequest(request310 < "eur", "amount" -> "1")) + Then("We should get a 200, because currency codes are case-insensitive") + response310_lower_case_ccy.code should equal(200) + And("the same answer as for EUR") + (response310_lower_case_ccy.body \ "answer").extract[String] should equal((response310.body \ "answer").extract[String]) + When("We make a request v3.1.0 with all params but currency is invalid") - val response310_invalic_ccy = makeGetRequest(request310 < "eur", "amount" -> "1")) + val response310_invalic_ccy = makeGetRequest(request310 < "QQQ", "amount" -> "1")) Then("We should get a 400") response310_invalic_ccy.code should equal(400) And("error should be " + InvalidISOCurrencyCode) response310_invalic_ccy.body.extract[ErrorMessage].message startsWith(InvalidISOCurrencyCode) + When("We make a request v3.1.0 with an amount that has more decimal places than EUR allows") + val response310_precision = makeGetRequest(request310 < "EUR", "amount" -> "1.234")) + Then("We should get a 400") + response310_precision.code should equal(400) + response310_precision.body.extract[ErrorMessage].message should startWith (InvalidAmountPrecision) + When("We make a request v3.1.0 with all params but amount is invalid") val response310_amount_ccy = makeGetRequest(request310 < "EUR", "amount" -> "bb")) Then("We should get a 400") diff --git a/obp-api/src/test/scala/code/api/v4_0_0/AmountPrecisionTest.scala b/obp-api/src/test/scala/code/api/v4_0_0/AmountPrecisionTest.scala new file mode 100644 index 0000000000..09ef78b191 --- /dev/null +++ b/obp-api/src/test/scala/code/api/v4_0_0/AmountPrecisionTest.scala @@ -0,0 +1,99 @@ +package code.api.v4_0_0 + +import code.api.ResourceDocs1_4_0.SwaggerDefinitionsJSON +import code.api.util.APIUtil.OAuth._ +import code.api.util.{APIUtil, ApiRole} +import code.api.util.ErrorMessages.InvalidAmountPrecision +import code.api.v1_4_0.JSONFactory1_4_0.TransactionRequestAccountJsonV140 +import code.api.v2_0_0.TransactionRequestBodyJsonV200 +import code.api.v3_1_0.CreateAccountResponseJsonV310 +import code.entitlement.Entitlement +import code.setup.APIResponse +import com.openbankproject.commons.model.{AccountRoutingJsonV121, AmountOfMoneyJsonV121, ErrorMessage} +import org.json4s.native.Serialization.write +import org.scalatest.Tag + +/** + * This class tests that a request carrying an amount with more decimal places than its currency + * allows is refused with 400 (OBP-10068), instead of the extra decimal places being cut off when the + * amount is stored. + * + * The check is made once for every endpoint, in ResourceDocMiddleware, so these scenarios use two + * endpoints that carry an amount in their body: creating an account with an opening balance, and + * creating a SANDBOX_TAN transaction request. FundsAvailableTest covers an amount in a query string, + * and code.asset.AmountPrecisionTest covers the check itself. + */ +class AmountPrecisionTest extends V400ServerSetup { + + object AmountPrecisionTag extends Tag("AmountPrecision") + + private val bankId = testBankId1.value + + /** This posts a Create Account request with the given opening balance and returns the response. */ + private def createAccountWithBalance(currency: String, amount: String) = { + val entitlement = Entitlement.entitlement.vend.addEntitlement(bankId, resourceUser1.userId, ApiRole.canCreateAccount.toString) + val body = SwaggerDefinitionsJSON.createAccountRequestJsonV310.copy( + user_id = resourceUser1.userId, + balance = AmountOfMoneyJsonV121(currency, amount), + account_routings = List(AccountRoutingJsonV121(s"scheme-${APIUtil.generateUUID().take(8)}", s"address-${APIUtil.generateUUID().take(8)}"))) + try makePostRequest((v4_0_0_Request / "banks" / bankId / "accounts").POST <@ (user1), write(body)) + finally Entitlement.entitlement.vend.deleteEntitlement(entitlement) + } + + private def messageOf(response: APIResponse): String = response.body.extract[ErrorMessage].message + + feature("Amounts with more decimal places than their currency allows are refused") { + + scenario("An opening balance of 0.001 EUR is refused, because EUR has 2 decimal places", AmountPrecisionTag) { + val response = createAccountWithBalance("EUR", "0.001") + response.code should equal(400) + messageOf(response) should startWith(InvalidAmountPrecision) + messageOf(response) should include("The amount 0.001 EUR has 3 decimal place(s), but EUR allows at most 2.") + } + + scenario("An amount in JPY may not have decimal places at all", AmountPrecisionTag) { + val response = createAccountWithBalance("JPY", "0.5") + response.code should equal(400) + messageOf(response) should include("JPY allows at most 0.") + } + + scenario("The currency code is checked in any letter case", AmountPrecisionTag) { + val response = createAccountWithBalance("eur", "0.001") + response.code should equal(400) + messageOf(response) should include("EUR allows at most 2.") + } + + scenario("Trailing zeros do not count as decimal places", AmountPrecisionTag) { + List(("EUR", "0.000"), ("JPY", "0.00")).foreach { case (currency, amount) => + val response = createAccountWithBalance(currency, amount) + withClue(s"$amount $currency: ${response.body}") { response.code should equal(201) } + } + } + + scenario("Crypto amounts are not checked until their real precision is recorded", AmountPrecisionTag) { + val response = createAccountWithBalance("ETH", "0.001") + response.body.extractOpt[ErrorMessage].map(_.message).getOrElse("") should not include ("OBP-10068") + } + + scenario("A transaction request for 10.005 EUR is refused and nothing is paid", AmountPrecisionTag) { + Given("two EUR accounts") + val fromAccount = createAccountWithBalance("EUR", "0").body.extract[CreateAccountResponseJsonV310] + val toAccount = createAccountWithBalance("EUR", "0").body.extract[CreateAccountResponseJsonV310] + + When("a transaction request for 10.005 EUR is made") + val body = TransactionRequestBodyJsonV200( + TransactionRequestAccountJsonV140(bankId, toAccount.account_id), AmountOfMoneyJsonV121("EUR", "10.005"), "precision test") + val request = (v4_0_0_Request / "banks" / bankId / "accounts" / fromAccount.account_id / "owner" / + "transaction-request-types" / "SANDBOX_TAN" / "transaction-requests").POST <@ (user1) + val response = makePostRequest(request, write(body)) + + Then("it is refused with 400") + response.code should equal(400) + messageOf(response) should include("The amount 10.005 EUR has 3 decimal place(s), but EUR allows at most 2.") + + And("the same request for 10.00 EUR is not refused for its precision") + val accepted = makePostRequest(request, write(body.copy(value = AmountOfMoneyJsonV121("EUR", "10.00")))) + accepted.body.extractOpt[ErrorMessage].map(_.message).getOrElse("") should not include ("OBP-10068") + } + } +} diff --git a/obp-api/src/test/scala/code/api/v4_0_0/SimpleTransactionRequestChallengePayeeTest.scala b/obp-api/src/test/scala/code/api/v4_0_0/SimpleTransactionRequestChallengePayeeTest.scala new file mode 100644 index 0000000000..c4a829f425 --- /dev/null +++ b/obp-api/src/test/scala/code/api/v4_0_0/SimpleTransactionRequestChallengePayeeTest.scala @@ -0,0 +1,184 @@ +/** +Open Bank Project - API +Copyright (C) 2011-2026, TESOBE GmbH. + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . + +Email: contact@tesobe.com +TESOBE GmbH. +Osloer Strasse 16/17 +Berlin 13359, Germany + +This product includes software developed at +TESOBE (http://www.tesobe.com/) + + */ + +package code.api.v4_0_0 + +import code.api.Constant.SYSTEM_OWNER_VIEW_ID +import code.api.util.APIUtil.OAuth._ +import code.api.util.ErrorMessages.attemptedToOpenAnEmptyBox +import code.metadata.counterparties.Counterparties +import code.model.BankAccountX +import code.transactionrequests.MappedTransactionRequest +import com.openbankproject.commons.model.enums.TransactionRequestStatus +import com.openbankproject.commons.model.{AccountId, AmountOfMoneyJsonV121, BankAccount, BankId} +import net.liftweb.mapper.By +import org.json4s.native.Serialization.write +import org.scalatest.Tag + +/** + * This test checks that answering the challenge of a SIMPLE transaction request pays the + * account the payer named, and no other. + * + * A SIMPLE request names its payee by routing. When it is created, the routing schemes are + * normalised (`snakify(scheme).toUpperCase`, so `obp` becomes `OBP`) and a counterparty is found + * or created with them. When the challenge is answered, `createTransactionAfterChallengeV210` + * does not reuse that counterparty: it looks one up again from the routing stored in the request + * body, which still has the scheme as the payer wrote it. The database comparison is + * case-sensitive, so `obp` misses the counterparty stored as `OBP`, and the lookup falls back to + * the secondary routing. When that is empty, as it usually is, the fallback asks for any + * counterparty whose secondary routing is empty, belonging to anyone, and the payment goes to + * whatever account that counterparty points at. + * + * The scenario sets that up: an unrelated account owns a counterparty with an empty secondary + * routing that points at a decoy account, created before the payer's request so that it comes + * first. The payer then sends a SIMPLE request to the intended account with the scheme written + * in lower case, for an amount that needs a challenge, and answers it. + */ +class SimpleTransactionRequestChallengePayeeTest extends V400ServerSetup { + + object VersionOfApi extends Tag("v4.0.0") + object SimpleChallengePayeeTag extends Tag("SimpleTransactionRequestChallengePayee") + + private val currency = "AED" + // Above the challenge threshold, so the request waits for a challenge answer. + private val amount = BigDecimal("30000.00") + + private def balanceOf(bankId: BankId, accountId: AccountId): BigDecimal = + BankAccountX(bankId, accountId).map(_.balance).openOrThrowException(attemptedToOpenAnEmptyBox) + + /** + * Runs the whole story. With `forgetRecordedCounterparty`, the counterparty id recorded at creation + * is cleared before the challenge is answered, which is the state of requests created before the + * id was recorded, so the challenge step has to resolve the routing from the body. + */ + private def payWithChallenge(forgetRecordedCounterparty: Boolean): Unit = { + setPropsValues("transactionRequests_supported_types" -> "SEPA,SANDBOX_TAN,FREE_FORM,COUNTERPARTY,ACCOUNT,ACCOUNT_OTP,SIMPLE,CARD,AGENT_CASH_WITHDRAWAL") + + Given("a payer's account, the account they want to pay, and an unrelated account with a decoy payee") + // Each run gets its own bank and accounts, so the two scenarios do not share balances or counterparties. + val runSuffix = java.util.UUID.randomUUID().toString.take(8) + val bankId = createBank(s"__simple-payee-bank-$runSuffix").bankId + val payerAccount: BankAccount = createAccountRelevantResource(Some(resourceUser1), bankId, AccountId(s"__simple_payer_$runSuffix"), currency) + val intendedAccount: BankAccount = createAccountRelevantResource(None, bankId, AccountId(s"__simple_intended_$runSuffix"), currency) + val unrelatedAccount: BankAccount = createAccountRelevantResource(Some(resourceUser2), bankId, AccountId(s"__simple_unrelated_$runSuffix"), currency) + val decoyAccount: BankAccount = createAccountRelevantResource(None, bankId, AccountId(s"__simple_decoy_$runSuffix"), currency) + + And("the unrelated account owns a counterparty that points at the decoy account and has an empty secondary routing") + Counterparties.counterparties.vend.createCounterparty( + createdByUserId = resourceUser2.userId, + thisBankId = bankId.value, + thisAccountId = unrelatedAccount.accountId.value, + thisViewId = SYSTEM_OWNER_VIEW_ID, + name = "Decoy payee of an unrelated account", + otherAccountRoutingScheme = "OBP", + otherAccountRoutingAddress = decoyAccount.accountId.value, + otherBankRoutingScheme = "OBP", + otherBankRoutingAddress = bankId.value, + otherBranchRoutingScheme = "", + otherBranchRoutingAddress = "", + isBeneficiary = true, + otherAccountSecondaryRoutingScheme = "", + otherAccountSecondaryRoutingAddress = "", + description = "", + currency = currency, + bespoke = Nil + ).openOrThrowException(attemptedToOpenAnEmptyBox) + + val payerBalanceBefore = balanceOf(bankId, payerAccount.accountId) + val intendedBalanceBefore = balanceOf(bankId, intendedAccount.accountId) + val decoyBalanceBefore = balanceOf(bankId, decoyAccount.accountId) + + When("the payer creates a SIMPLE request to the intended account, writing the routing scheme as `obp`") + val simpleBody = TransactionRequestBodySimpleJsonV400( + to = PostSimpleCounterpartyJson400( + name = "Intended payee", + description = "The account the payer wants to pay", + other_bank_routing_scheme = "obp", + other_bank_routing_address = bankId.value, + other_account_routing_scheme = "obp", + other_account_routing_address = intendedAccount.accountId.value, + other_account_secondary_routing_scheme = "", + other_account_secondary_routing_address = "", + other_branch_routing_scheme = "", + other_branch_routing_address = "" + ), + value = AmountOfMoneyJsonV121(currency, amount.toString), + description = "SIMPLE payment that needs a challenge", + charge_policy = "SHARED" + ) + val transactionRequestsRequest = (v4_0_0_Request / "banks" / bankId.value / "accounts" / payerAccount.accountId.value / + SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "SIMPLE" / "transaction-requests").POST <@ user1 + val createResponse = makePostRequest(transactionRequestsRequest, write(simpleBody)) + + Then("the request is created and waits for a challenge answer") + withClue(createResponse.body) { createResponse.code shouldBe 201 } + val transactionRequestId = (createResponse.body \ "id").values.toString + val challengeId = (createResponse.body \ "challenges" \ "id").children.headOption.map(_.values.toString).getOrElse("") + withClue(createResponse.body) { challengeId should not be empty } + + When("the payer answers the challenge") + val answerRequest = (v4_0_0_Request / "banks" / bankId.value / "accounts" / payerAccount.accountId.value / + SYSTEM_OWNER_VIEW_ID / "transaction-request-types" / "SIMPLE" / "transaction-requests" / transactionRequestId / "challenge").POST <@ user1 + if (forgetRecordedCounterparty) { + And("the request carries no recorded counterparty, as requests created before the fix do not") + MappedTransactionRequest.find(By(MappedTransactionRequest.mTransactionRequestId, transactionRequestId)) + .map(_.mCounterpartyId("").saveMe()).openOrThrowException(attemptedToOpenAnEmptyBox) + } + val answerResponse = makePostRequest(answerRequest, write(ChallengeAnswerJson400(id = challengeId, answer = "123"))) + + Then("the payment completes") + withClue(answerResponse.body) { + answerResponse.code shouldBe 202 + (answerResponse.body \ "status").values.toString shouldBe TransactionRequestStatus.COMPLETED.toString + } + + val balanceChanges = + s"payer ${balanceOf(bankId, payerAccount.accountId) - payerBalanceBefore}, " + + s"intended ${balanceOf(bankId, intendedAccount.accountId) - intendedBalanceBefore}, " + + s"decoy ${balanceOf(bankId, decoyAccount.accountId) - decoyBalanceBefore}" + withClue(s"Balance changes: $balanceChanges. ") { + And("the payer is debited and the intended account is credited") + balanceOf(bankId, payerAccount.accountId) shouldBe payerBalanceBefore - amount + balanceOf(bankId, intendedAccount.accountId) shouldBe intendedBalanceBefore + amount + + And("the decoy account of the unrelated counterparty is not") + balanceOf(bankId, decoyAccount.accountId) shouldBe decoyBalanceBefore + } + } + + feature("Answering a SIMPLE transaction request's challenge pays the payee the payer named") { + + scenario("A new request: the counterparty resolved at creation is paid", VersionOfApi, SimpleChallengePayeeTag) { + payWithChallenge(forgetRecordedCounterparty = false) + } + + scenario("A request created before the counterparty was recorded: its routing is resolved among the paying account's counterparties", + VersionOfApi, SimpleChallengePayeeTag) { + payWithChallenge(forgetRecordedCounterparty = true) + } + } +} diff --git a/obp-api/src/test/scala/code/api/v7_0_0/PlatformAppsTest.scala b/obp-api/src/test/scala/code/api/v7_0_0/PlatformAppsTest.scala index d3dbb8e060..513c109cfc 100644 --- a/obp-api/src/test/scala/code/api/v7_0_0/PlatformAppsTest.scala +++ b/obp-api/src/test/scala/code/api/v7_0_0/PlatformAppsTest.scala @@ -28,7 +28,7 @@ package code.api.v7_0_0 import code.api.Constant.DYNAMIC_ENTITY_SYSTEM_LEVEL_BANK_ID import code.api.util.APIUtil.OAuth._ -import code.api.util.ApiRole.{canCreatePlatformApp, canDeletePlatformApp, canGetDynamicEntityDefinitions, canGetPlatformApps} +import code.api.util.ApiRole.{canCreatePlatformApp, canCreateScopeAtOneBank, canDeletePlatformApp, canGetDynamicEntityDefinitions, canGetPlatformApps} import code.api.util.ErrorMessages.{InvalidPlatformAppDeclaration, PlatformAppAlreadyExists, PlatformAppNotFound, UserHasMissingRoles} import code.api.v6_0_0.V600ServerSetup import code.entitlement.Entitlement @@ -51,6 +51,7 @@ class PlatformAppsTest extends V600ServerSetup { object ApiEndpoint2 extends Tag("getPlatformApps") object ApiEndpoint3 extends Tag("deletePlatformApp") object ApiEndpoint4 extends Tag("updateCurrentConsumerPlatformApp") + object ApiEndpoint5 extends Tag("addScope") private def platformApps = v7 / "management" / "platform-apps" private def declaration = v7 / "consumers" / "current" / "platform-app" @@ -134,6 +135,32 @@ class PlatformAppsTest extends V600ServerSetup { } } + scenario("CanCreateScopeAtOneBank grants a missing Scope only at its own bank_id, and a refusal names the bank_id", + ApiEndpoint5, VersionOfApi) { + val consumer = testConsumer2 + val bankId = testBankId1.value + val scopes = v7 / "consumers" / consumer.consumerId.get / "scopes" + def body(bank: String) = compact(render(("bank_id" -> bank) ~ ("role_name" -> canGetDynamicEntityDefinitions.toString))) + val granted = Entitlement.entitlement.vend.addEntitlement(bankId, resourceUser1.userId, canCreateScopeAtOneBank.toString) + var added: net.liftweb.common.Box[Scope] = net.liftweb.common.Empty + try { + When("user1, holding CanCreateScopeAtOneBank at another bank, adds a Scope at SYS") + val refused = makePostRequest(scopes.POST <@ (user1), body(SYS)) + Then("it is refused, and the message names the bank_id the Role was checked at") + refused.code should equal(403) + message(refused) should include(s"$UserHasMissingRoles$canCreateScopeAtOneBank at bank_id $SYS or ") + + When("user1 adds the same Scope at the bank it holds the Role at") + val created = makePostRequest(scopes.POST <@ (user1), body(bankId)) + Then("it is created") + created.code should equal(201) + added = Scope.scope.vend.getScope(bankId, consumer.id.get.toString, canGetDynamicEntityDefinitions.toString) + } finally { + granted.foreach(e => Entitlement.entitlement.vend.deleteEntitlement(Full(e))) + added.foreach(s => Scope.scope.vend.deleteScope(Full(s))) + } + } + scenario("marking an unknown Consumer is a 404", ApiEndpoint1, VersionOfApi) { val granted = grant(canCreatePlatformApp) try { diff --git a/obp-api/src/test/scala/code/api/v7_0_0/TransactionRequestTypeRoutingTest.scala b/obp-api/src/test/scala/code/api/v7_0_0/TransactionRequestTypeRoutingTest.scala new file mode 100644 index 0000000000..91beb2781a --- /dev/null +++ b/obp-api/src/test/scala/code/api/v7_0_0/TransactionRequestTypeRoutingTest.scala @@ -0,0 +1,146 @@ +/** +Open Bank Project - API +Copyright (C) 2011-2026, TESOBE GmbH. + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Affero General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Affero General Public License for more details. + +You should have received a copy of the GNU Affero General Public License +along with this program. If not, see . + +Email: contact@tesobe.com +TESOBE GmbH. +Osloer Strasse 16/17 +Berlin 13359, Germany + +This product includes software developed at +TESOBE (http://www.tesobe.com/) + + */ + +package code.api.v7_0_0 + +import cats.effect.IO +import cats.effect.unsafe.IORuntime +import code.api.v4_0_0.Http4s400 +import code.api.v6_0_0.Http4s600 +import code.setup.ServerSetup +import fs2.Stream +import org.http4s.{Headers, HttpApp, Method, Request, Response, Uri} +import org.scalatest.{GivenWhenThen, Tag} +import org.typelevel.ci.CIString + +/** + * This test checks that a transaction request reaches the ResourceDoc of its own type. + * + * The middleware picks a ResourceDoc by matching the URL against templates, separately from + * the http4s route that runs the handler. Transaction-request types are written in capitals + * like placeholders, so a template such as + * `.../transaction-request-types/MOBILE_WALLET/transaction-requests` used to match every type. + * Two failures followed. In v7.0.0, HOLD, CARDANO and Ethereum requests, which v7 has no + * handler for, were claimed by the MOBILE_WALLET doc and answered with an empty 404 instead + * of falling through to v6.0.0, which serves them. And UTILITY, BULK and OPEN_CORRIDOR_PROMISE + * requests were validated under the MOBILE_WALLET doc, so switching MOBILE_WALLET off switched + * them off too. v6.0.0 had the same problem with its HOLD doc. + * + * Every request here is sent without credentials. A request that reaches the doc of its own + * type is refused with 401 by that doc's login requirement. A request claimed by a disabled + * doc gets 404 instead, and the `X-OBP-Version-Served` header shows which version answered. + * No test data is needed. + */ +class TransactionRequestTypeRoutingTest extends ServerSetup with GivenWhenThen { + + object TransactionRequestTypeRoutingTag extends Tag("TransactionRequestTypeRouting") + + implicit val runtime: IORuntime = IORuntime.global + + private val v700App = Http4s700.wrappedRoutesV700Services.orNotFound + private val v600App = Http4s600.wrappedRoutesV600Services.orNotFound + private val v400App = Http4s400.wrappedRoutesV400Services.orNotFound + + private val versionServedHeader = CIString("X-OBP-Version-Served") + + private def postTransactionRequest(app: HttpApp[IO], version: String, viewId: String, transactionRequestType: String): Response[IO] = { + val path = s"/obp/$version/banks/gh.29.uk/accounts/8ca8a7e4-6d02-40e3-a129-0b2bf89de9f1/$viewId" + + s"/transaction-request-types/$transactionRequestType/transaction-requests" + val request = Request[IO](Method.POST, Uri.unsafeFromString(path), headers = Headers.empty, + body = Stream.emits("{}".getBytes("UTF-8"))) + app.run(request).unsafeRunSync() + } + + private def versionServed(response: Response[IO]): Option[String] = + response.headers.get(versionServedHeader).map(_.head.value) + + feature("v7.0.0 hands transaction-request types it has no handler for to v6.0.0") { + List("HOLD", "CARDANO", "ETH_SEND_TRANSACTION", "ETH_SEND_RAW_TRANSACTION").foreach { transactionRequestType => + scenario(s"POST /obp/v7.0.0/.../transaction-request-types/$transactionRequestType/transaction-requests is served by v6.0.0", TransactionRequestTypeRoutingTag) { + When(s"an unauthenticated $transactionRequestType request is sent to the v7.0.0 prefix") + val response = postTransactionRequest(v700App, "v7.0.0", "owner", transactionRequestType) + + Then("v6.0.0 answers it, refusing it for lack of credentials") + versionServed(response) shouldBe Some("v6.0.0") + response.status.code shouldBe 401 + } + } + + scenario("A transaction-request type native to v7.0.0 is answered by v7.0.0", TransactionRequestTypeRoutingTag) { + val response = postTransactionRequest(v700App, "v7.0.0", "owner", "UTILITY") + versionServed(response) shouldBe None + response.status.code shouldBe 401 + } + } + + feature("Switching one transaction-request type off leaves the other types on") { + + scenario("v7.0.0: disabling MOBILE_WALLET leaves UTILITY, BULK and OPEN_CORRIDOR_PROMISE on", TransactionRequestTypeRoutingTag) { + Given("api_disabled_endpoints names only the MOBILE_WALLET endpoint") + setPropsValues("api_disabled_endpoints" -> "[OBPv7.0.0-createTransactionRequestMobileWallet]") + + Then("MOBILE_WALLET is switched off") + postTransactionRequest(v700App, "v7.0.0", "owner", "MOBILE_WALLET").status.code shouldBe 404 + + And("the other v7.0.0 types still reach their own docs") + List("UTILITY", "BULK", "OPEN_CORRIDOR_PROMISE").foreach { transactionRequestType => + withClue(s"$transactionRequestType: ") { + postTransactionRequest(v700App, "v7.0.0", "owner", transactionRequestType).status.code shouldBe 401 + } + } + } + + scenario("v6.0.0: disabling HOLD leaves CARDANO and the Ethereum types on", TransactionRequestTypeRoutingTag) { + Given("api_disabled_endpoints names only the v6.0.0 HOLD endpoint") + setPropsValues("api_disabled_endpoints" -> "[OBPv6.0.0-createTransactionRequestHold]") + + Then("HOLD is switched off") + postTransactionRequest(v600App, "v6.0.0", "owner", "HOLD").status.code shouldBe 404 + + And("the other v6.0.0 types still reach their own docs") + List("CARDANO", "ETH_SEND_TRANSACTION", "ETH_SEND_RAW_TRANSACTION").foreach { transactionRequestType => + withClue(s"$transactionRequestType: ") { + postTransactionRequest(v600App, "v6.0.0", "owner", transactionRequestType).status.code shouldBe 401 + } + } + } + + scenario("v4.0.0: disabling the ACCOUNT endpoint leaves SEPA and COUNTERPARTY on", TransactionRequestTypeRoutingTag) { + Given("api_disabled_endpoints names only the v4.0.0 ACCOUNT endpoint") + setPropsValues("api_disabled_endpoints" -> "[OBPv4.0.0-createTransactionRequestAccount]") + + Then("SEPA and COUNTERPARTY still reach their own v4.0.0 docs") + List("SEPA", "COUNTERPARTY").foreach { transactionRequestType => + withClue(s"$transactionRequestType: ") { + val response = postTransactionRequest(v400App, "v4.0.0", "owner", transactionRequestType) + response.status.code shouldBe 401 + versionServed(response) shouldBe None + } + } + } + } +} diff --git a/obp-api/src/test/scala/code/asset/AmountPrecisionTest.scala b/obp-api/src/test/scala/code/asset/AmountPrecisionTest.scala new file mode 100644 index 0000000000..b9c4f01532 --- /dev/null +++ b/obp-api/src/test/scala/code/asset/AmountPrecisionTest.scala @@ -0,0 +1,82 @@ +package code.asset + +import org.scalatest.{FeatureSpec, GivenWhenThen, Matchers} + +/** + * This class tests AmountPrecision, which finds an amount in a request that has more decimal places + * than its currency allows: how decimal places are counted, how an amount is paired with its currency + * in a JSON body and in a query string, and which currencies are not checked. + * + * Everything here is a pure function call with no database, so the currencies' decimal places come + * from the built-in list (EUR 2, JPY 0, KWD 3). + */ +class AmountPrecisionTest extends FeatureSpec with Matchers with GivenWhenThen { + + feature("Counting decimal places") { + + scenario("Trailing zeros and exponents are not decimal places") { + AmountPrecision.decimalPlacesOf(BigDecimal("12.34")) shouldBe 2 + AmountPrecision.decimalPlacesOf(BigDecimal("12.340")) shouldBe 2 + AmountPrecision.decimalPlacesOf(BigDecimal("10.00")) shouldBe 0 + AmountPrecision.decimalPlacesOf(BigDecimal("1E+3")) shouldBe 0 + AmountPrecision.decimalPlacesOf(BigDecimal("1e-5")) shouldBe 5 + AmountPrecision.decimalPlacesOf(BigDecimal("-0.125")) shouldBe 3 + } + } + + feature("Amounts in a JSON body") { + + scenario("An amount with too many decimal places is found, however deep it is") { + val found = AmountPrecision.inJsonBody("""{"to":{"bank_id":"b"},"value":{"currency":"EUR","amount":"12.345"}}""") + found.map(_.describe) shouldBe Some("The amount 12.345 EUR has 3 decimal place(s), but EUR allows at most 2.") + } + + scenario("Amounts within their currency's precision pass") { + AmountPrecision.inJsonBody("""{"value":{"currency":"EUR","amount":"12.34"}}""") shouldBe None + AmountPrecision.inJsonBody("""{"value":{"currency":"JPY","amount":"100.00"}}""") shouldBe None + AmountPrecision.inJsonBody("""{"value":{"currency":"KWD","amount":"1.125"}}""") shouldBe None + } + + scenario("An amount sent as a JSON number is read exactly") { + AmountPrecision.inJsonBody("""{"currency":"EUR","amount":12.345}""").map(_.amount) shouldBe Some("12.345") + AmountPrecision.inJsonBody("""{"currency":"EUR","amount":0.1}""") shouldBe None + AmountPrecision.inJsonBody("""{"currency":"JPY","amount":5}""") shouldBe None + } + + scenario("UK Open Banking's Amount and Currency are paired the same way") { + AmountPrecision.inJsonBody("""{"InstructedAmount":{"Amount":"10.001","Currency":"GBP"}}""").map(_.currency) shouldBe Some("GBP") + } + + scenario("Every amount in an array is checked") { + val body = """{"payments":[{"currency":"EUR","amount":"1.00"},{"currency":"EUR","amount":"2.005"}]}""" + AmountPrecision.inJsonBody(body).map(_.amount) shouldBe Some("2.005") + } + + scenario("With several currency fields, the one named currency is the amount's") { + AmountPrecision.inJsonBody("""{"charge_currency":"JPY","currency":"EUR","amount":"1.50"}""") shouldBe None + AmountPrecision.inJsonBody("""{"from_currency":"EUR","to_currency":"JPY","amount":"1.50"}""") shouldBe None + } + + scenario("Amounts the check cannot judge are left to the endpoint") { + AmountPrecision.inJsonBody("""{"currency":"ZZZ","amount":"1.123456"}""") shouldBe None + AmountPrecision.inJsonBody("""{"currency":"EUR","amount":"twelve"}""") shouldBe None + AmountPrecision.inJsonBody("""{"amount":"1.123456"}""") shouldBe None + AmountPrecision.inJsonBody("""not json, amount 1.123""") shouldBe None + } + + scenario("Crypto amounts are not checked until their real precision is recorded") { + AmountPrecision.inJsonBody("""{"currency":"ETH","amount":"0.000000000000000001"}""") shouldBe None + AmountPrecision.inJsonBody("""{"currency":"ADA","amount":"1.123456"}""") shouldBe None + AmountPrecision.inJsonBody("""{"currency":"lovelace","amount":"1.5"}""") shouldBe None + } + } + + feature("Amounts in a query string") { + + scenario("An amount next to a currency parameter is checked") { + AmountPrecision.inQuery(Seq("currency" -> Some("EUR"), "amount" -> Some("1.234"))).map(_.amount) shouldBe Some("1.234") + AmountPrecision.inQuery(Seq("currency" -> Some("EUR"), "amount" -> Some("1.23"))) shouldBe None + AmountPrecision.inQuery(Seq("amount" -> Some("1.234"))) shouldBe None + } + } +} diff --git a/obp-api/src/test/scala/code/asset/AssetLookupTest.scala b/obp-api/src/test/scala/code/asset/AssetLookupTest.scala index 3e9b3a0664..5f8f00ea5f 100644 --- a/obp-api/src/test/scala/code/asset/AssetLookupTest.scala +++ b/obp-api/src/test/scala/code/asset/AssetLookupTest.scala @@ -28,12 +28,19 @@ class AssetLookupTest extends ServerSetup with RestoresSeededAssetRegistry { APIUtil.builtInCurrencyCodes.toList.sorted ++ List("eur", "Eur", "jpy", "kwd", "xbt", "eth", "LOVELACE", "WEI", "", "EUR USD", "978", "USDC", "MRO") + /** Whether the built-in list holds this code, ignoring letter case, as OBP now matches codes. */ + private def builtInListHolds(code: String): Boolean = + APIUtil.builtInCurrencyCodes.exists(builtIn => CurrencyCodes.same(builtIn, code)) + feature("Answers from the seeded registry") { - scenario("Every code is accepted or rejected as the built-in list does, except that ADA is now accepted") { + scenario("Every code is accepted or rejected as the built-in list does, ignoring letter case, and ADA is now accepted") { seedRegistry() - val differences = codesToCompare.filter(code => APIUtil.isValidCurrencyISOCode(code) != APIUtil.builtInCurrencyCodes.contains(code)) + val differences = codesToCompare.filter(code => APIUtil.isValidCurrencyISOCode(code) != builtInListHolds(code)) differences shouldBe Nil + And("a code the built-in list holds in upper case is accepted in lower case too") + APIUtil.isValidCurrencyISOCode("eur") shouldBe true + APIUtil.isValidCurrencyISOCode("Eur") shouldBe true Then("ADA, the registry's code for Cardano's currency, is accepted, and so is the built-in spelling ada") APIUtil.isValidCurrencyISOCode("ADA") shouldBe true APIUtil.isValidCurrencyISOCode("ada") shouldBe true @@ -42,8 +49,8 @@ class AssetLookupTest extends ServerSetup with RestoresSeededAssetRegistry { scenario("Every code has the decimal places of the built-in table") { seedRegistry() val differences = (codesToCompare :+ "ADA").collect { - case code if Helper.currencyDecimalPlaces(code) != Helper.builtInCurrencyDecimalPlaces(code) => - s"$code: registry ${Helper.currencyDecimalPlaces(code)}, built-in ${Helper.builtInCurrencyDecimalPlaces(code)}" + case code if Helper.currencyDecimalPlaces(code) != Helper.builtInCurrencyDecimalPlaces(CurrencyCodes.normalise(code)) => + s"$code: registry ${Helper.currencyDecimalPlaces(code)}, built-in ${Helper.builtInCurrencyDecimalPlaces(CurrencyCodes.normalise(code))}" } differences shouldBe Nil } @@ -69,9 +76,13 @@ class AssetLookupTest extends ServerSetup with RestoresSeededAssetRegistry { Asset.bulkDelete_!!() AssetLookup.invalidate() APIUtil.isValidCurrencyISOCode("EUR") shouldBe true + APIUtil.isValidCurrencyISOCode("eur") shouldBe true APIUtil.isValidCurrencyISOCode("lovelace") shouldBe true - APIUtil.isValidCurrencyISOCode("ADA") shouldBe false + APIUtil.isValidCurrencyISOCode("ada") shouldBe true + APIUtil.isValidCurrencyISOCode("XBT") shouldBe true + APIUtil.isValidCurrencyISOCode("MRO") shouldBe false Helper.currencyDecimalPlaces("JPY") shouldBe 0 + Helper.currencyDecimalPlaces("jpy") shouldBe 0 Helper.currencyDecimalPlaces("KWD") shouldBe 3 } } diff --git a/obp-api/src/test/scala/code/asset/CurrencyCodesTest.scala b/obp-api/src/test/scala/code/asset/CurrencyCodesTest.scala new file mode 100644 index 0000000000..619cac5aa4 --- /dev/null +++ b/obp-api/src/test/scala/code/asset/CurrencyCodesTest.scala @@ -0,0 +1,95 @@ +package code.asset + +import org.json4s.JsonDSL._ +import org.scalatest.{FeatureSpec, GivenWhenThen, Matchers} + +/** + * This class tests CurrencyCodes, which holds OBP's rule that currency codes are case-insensitive: + * how two codes are compared, which field and query parameter names are taken to hold a currency + * code, and how a JSON request body has its currency codes upper-cased without any other value + * changing. + * + * Everything here is a pure function call: no server, no database. + */ +class CurrencyCodesTest extends FeatureSpec with Matchers with GivenWhenThen { + + feature("Comparing and normalising codes") { + + scenario("A code is stored trimmed and in upper case") { + CurrencyCodes.normalise("eur") shouldBe "EUR" + CurrencyCodes.normalise(" Eur ") shouldBe "EUR" + CurrencyCodes.normalise("lovelace") shouldBe "LOVELACE" + } + + scenario("Two codes are the same whatever their letter case") { + CurrencyCodes.same("eur", "EUR") shouldBe true + CurrencyCodes.same("Ada", "ada") shouldBe true + CurrencyCodes.same("EUR", "USD") shouldBe false + } + + scenario("A missing code is not the same as any code") { + CurrencyCodes.same(null, "EUR") shouldBe false + CurrencyCodes.same("EUR", null) shouldBe false + CurrencyCodes.same(null, null) shouldBe false + } + } + + feature("Which names hold a currency code") { + + scenario("Names ending in currency or currency code, in any case and with or without underscores") { + List("currency", "Currency", "price_currency", "from_currency_code", "toCurrencyCode", "CURRENCY") + .filterNot(CurrencyCodes.isCurrencyKey) shouldBe Nil + } + + scenario("Names that only mention currency elsewhere do not") { + List("currency_status", "currencies", "amount", "currency_rate") + .filter(CurrencyCodes.isCurrencyKey) shouldBe Nil + } + } + + feature("Upper-casing the currency codes in a JSON body") { + + val fields = Set("currency", "price_currency") + + scenario("The currency fields are upper-cased wherever they appear, and nothing else changes") { + val body = """{"value":{"currency":"eur","amount":"10.50"},"price_currency":"usd","description":"eur","items":[{"currency":"gbp"}]}""" + CurrencyCodes.normaliseJsonBody(body, fields) shouldBe + """{"value":{"currency":"EUR","amount":"10.50"},"price_currency":"USD","description":"eur","items":[{"currency":"GBP"}]}""" + } + + scenario("Amounts sent as JSON numbers keep their exact form") { + val body = """{"currency":"eur","amount":12345678901234567.123456789012345678}""" + CurrencyCodes.normaliseJsonBody(body, fields) shouldBe + """{"currency":"EUR","amount":12345678901234567.123456789012345678}""" + } + + scenario("A value that does not look like a code is left alone") { + val body = """{"currency":"Euro of the Eurozone"}""" + CurrencyCodes.normaliseJsonBody(body, fields) shouldBe body + } + + scenario("With no currency fields the body is returned as it came") { + val body = """{"currency":"eur"}""" + CurrencyCodes.normaliseJsonBody(body, Set.empty) shouldBe body + } + } + + feature("Finding the currency fields from an endpoint's example body") { + + scenario("String fields whose name holds a currency code, at any depth") { + val example = ("value" -> ("currency" -> "EUR") ~ ("amount" -> "10")) ~ + ("charge_policy" -> "SHARED") ~ + ("payments" -> List(("instructed_currency" -> "EUR") ~ ("amount" -> "1"))) + CurrencyCodes.currencyFieldsOf(example) shouldBe Set("currency", "instructed_currency") + } + + scenario("A field called currency that holds an object, not a code, is not one") { + val example = ("currency" -> ("code" -> "EUR") ~ ("name" -> "Euro")) + CurrencyCodes.currencyFieldsOf(example) shouldBe Set.empty + } + + scenario("An endpoint with no example body has none") { + CurrencyCodes.currencyFieldsOf(null) shouldBe Set.empty + } + } +} diff --git a/obp-api/src/test/scala/code/asset/CurrencyCodesUpperCaseMigrationTest.scala b/obp-api/src/test/scala/code/asset/CurrencyCodesUpperCaseMigrationTest.scala new file mode 100644 index 0000000000..0beefb1479 --- /dev/null +++ b/obp-api/src/test/scala/code/asset/CurrencyCodesUpperCaseMigrationTest.scala @@ -0,0 +1,110 @@ +package code.asset + +import code.api.util.APIUtil +import code.api.util.migration.MigrationOfCurrencyCodesUpperCase +import code.fx.{MappedCurrency, MappedFXRate} +import code.migration.MigrationScriptLog +import code.setup.ServerSetup +import net.liftweb.db.DB +import net.liftweb.mapper.By +import net.liftweb.util.DefaultConnectionIdentifier + +/** + * This class tests the migration that rewrites every stored currency code in upper case + * (MigrationOfCurrencyCodesUpperCase). + * + * Currency codes are case-insensitive, and a code in a request is now upper-cased before it is + * stored, but rows written earlier can hold `eur` or `Eur`. The scenarios check that such a row is + * rewritten, that a row already in upper case is not touched, and that in `mappedcurrency`, which is + * keyed by the code, a lower-case row whose upper-case twin exists is left alone instead of making the + * migration fail on the duplicate key. + */ +class CurrencyCodesUpperCaseMigrationTest extends ServerSetup { + + private val bankId = s"currency-migration-${APIUtil.generateUUID().take(8)}" + + private def addFxRate(fromCurrencyCode: String, toCurrencyCode: String): MappedFXRate = + MappedFXRate.create.mBankId(bankId).mFromCurrencyCode(fromCurrencyCode).mToCurrencyCode(toCurrencyCode) + .mConversionValue(1.5).mInverseConversionValue(1 / 1.5).mEffectiveDate(new java.util.Date()).saveMe() + + private def fxRatesAtTestBank: List[(String, String)] = + MappedFXRate.findAll(By(MappedFXRate.mBankId, bankId)).map(rate => (rate.fromCurrencyCode, rate.toCurrencyCode)).sorted + + private def currencyCodesLike(code: String): List[String] = + MappedCurrency.findAll().map(_.currencyCode).filter(_.equalsIgnoreCase(code)).sorted + + /** Mapper does not let a string primary key be set, so the row is written with SQL. */ + private def addCurrency(code: String): Unit = + DB.use(DefaultConnectionIdentifier) { connection => + val statement = connection.prepareStatement( + s"INSERT INTO ${MappedCurrency.dbTableName} (${MappedCurrency.mCurrencyCode.dbColumnName}, ${MappedCurrency.mCurrencyName.dbColumnName}) VALUES (?, ?)") + try { + statement.setString(1, code) + statement.setString(2, s"Test currency $code") + statement.executeUpdate() + } finally statement.close() + } + + /** The remark the migration recorded under this name. */ + private def recordedRemark(migrationName: String): String = + MigrationScriptLog.find(By(MigrationScriptLog.Name, migrationName)) + .map(_.remark).openOrThrowException(s"the migration $migrationName should have recorded a log entry") + + feature("Upper-casing the stored currency codes") { + + scenario("Codes in lower or mixed case are rewritten in upper case, and upper-case ones are kept") { + Given("FX rates stored with codes in lower, mixed and upper case") + addFxRate("eur", "usd") + addFxRate("Gbp", "JPY") + addFxRate("CHF", "EUR") + + When("the migration runs") + val migrationName = s"testUpperCaseStoredCurrencyCodes_$bankId" + MigrationOfCurrencyCodesUpperCase.upperCaseEverywhere(migrationName) shouldBe true + + Then("every code is in upper case and no rate is lost") + fxRatesAtTestBank shouldBe List(("CHF", "EUR"), ("EUR", "USD"), ("GBP", "JPY")) + + And("the log says, for each column, which values became what") + val remark = recordedRemark(migrationName) + remark should include("mappedfxrate.mfromcurrencycode: 2 row(s) changed (Gbp -> GBP x1, eur -> EUR x1)") + remark should include("mappedfxrate.mtocurrencycode: 1 row(s) changed (usd -> USD x1)") + + When("the migration runs again") + val secondName = s"testUpperCaseStoredCurrencyCodesAgain_$bankId" + MigrationOfCurrencyCodesUpperCase.upperCaseEverywhere(secondName) shouldBe true + + Then("the log names the FX rate columns among those with nothing to change") + val secondRemark = recordedRemark(secondName) + secondRemark should include("Nothing to change:") + secondRemark should include("mappedfxrate.mfromcurrencycode") + secondRemark should not include ("mappedfxrate.mfromcurrencycode: ") + } + + scenario("A currency row whose upper-case code already exists is left as it is") { + Given("a currency stored as qqa and as QQA, and another stored only as qqb") + addCurrency("qqa") + addCurrency("QQA") + addCurrency("qqb") + + When("the migration runs") + val migrationName = s"testUpperCaseStoredCurrencyCodesTwin_$bankId" + MigrationOfCurrencyCodesUpperCase.upperCaseEverywhere(migrationName) shouldBe true + + Then("qqb is rewritten, and qqa stays beside QQA rather than the migration failing") + currencyCodesLike("qqb") shouldBe List("QQB") + currencyCodesLike("qqa") shouldBe List("QQA", "qqa") + + And("the log names both") + val remark = recordedRemark(migrationName) + remark should include("mappedcurrency.mcurrencycode: 1 row(s) changed (qqb -> QQB x1)") + remark should include("mappedcurrency.mcurrencycode: 1 row(s) left as they are because the upper-case code is already there (qqa)") + } + } + + override def afterAll(): Unit = { + MappedFXRate.findAll(By(MappedFXRate.mBankId, bankId)).foreach(_.delete_!) + MappedCurrency.findAll().filter(currency => Set("qqa", "qqb").contains(currency.currencyCode.toLowerCase)).foreach(_.delete_!) + super.afterAll() + } +} diff --git a/obp-api/src/test/scala/code/util/CurrencyHandlingTest.scala b/obp-api/src/test/scala/code/util/CurrencyHandlingTest.scala index 912fab637f..199fa0287d 100644 --- a/obp-api/src/test/scala/code/util/CurrencyHandlingTest.scala +++ b/obp-api/src/test/scala/code/util/CurrencyHandlingTest.scala @@ -93,9 +93,7 @@ class CurrencyHandlingTest extends FeatureSpec with Matchers with GivenWhenThen } scenario("Currency codes are accepted in any letter case") { - pendingUntilFixed { - List("eur", "Eur", "xbt", "eth", "ADA", "LOVELACE", "WEI").filterNot(APIUtil.isValidCurrencyISOCode) shouldBe Nil - } + List("eur", "Eur", "xbt", "eth", "ADA", "LOVELACE", "WEI").filterNot(APIUtil.isValidCurrencyISOCode) shouldBe Nil } } @@ -121,10 +119,8 @@ class CurrencyHandlingTest extends FeatureSpec with Matchers with GivenWhenThen } scenario("The decimal places of a code do not depend on its letter case") { - pendingUntilFixed { - Helper.currencyDecimalPlaces("jpy") shouldBe 0 - Helper.currencyDecimalPlaces("kwd") shouldBe 3 - } + Helper.currencyDecimalPlaces("jpy") shouldBe 0 + Helper.currencyDecimalPlaces("kwd") shouldBe 3 } } diff --git a/obp-api/src/test/scala/code/util/DatabaseConnectionHoldWatchTest.scala b/obp-api/src/test/scala/code/util/DatabaseConnectionHoldWatchTest.scala new file mode 100644 index 0000000000..b866e67e35 --- /dev/null +++ b/obp-api/src/test/scala/code/util/DatabaseConnectionHoldWatchTest.scala @@ -0,0 +1,79 @@ +package code.util + +import java.lang.reflect.{InvocationHandler, Method, Proxy => JProxy} +import java.sql.{Connection, SQLException} +import java.util.concurrent.atomic.AtomicInteger + +import org.scalatest.{FlatSpec, Matchers} + +/** + * DatabaseConnectionHoldWatch wraps every connection taken from the pool. These tests check that + * the wrapper passes calls through unchanged, that a connection held past the limit is warned + * about once with the code holding it, and that returning it to the pool ends the watch. + */ +class DatabaseConnectionHoldWatchTest extends FlatSpec with Matchers { + + /** A stand-in for a pooled connection: counts close() calls and fails rollback() with an SQLException. */ + private class FakeConnection { + val closeCalls = new AtomicInteger(0) + val connection: Connection = JProxy.newProxyInstance( + classOf[Connection].getClassLoader, + Array(classOf[Connection]), + new InvocationHandler { + def invoke(proxy: Any, method: Method, args: Array[AnyRef]): AnyRef = method.getName match { + case "close" => closeCalls.incrementAndGet(); null + case "getAutoCommit" => java.lang.Boolean.FALSE + case "getSchema" => "the_schema" + case "rollback" => throw new SQLException("rollback failed") + case _ => null + } + } + ).asInstanceOf[Connection] + } + + private def limitMillis: Long = DatabaseConnectionHoldWatch.holdWarningSeconds * 1000 + + "the watch" should "be on by default" in { + DatabaseConnectionHoldWatch.enabled shouldBe true + } + + it should "pass calls through to the connection and keep its exceptions as they are" in { + val fake = new FakeConnection + val watched = DatabaseConnectionHoldWatch.watch(fake.connection) + try { + watched.getAutoCommit shouldBe false + watched.getSchema shouldBe "the_schema" + watched.equals(watched) shouldBe true + watched.equals(fake.connection) shouldBe false + an[SQLException] should be thrownBy watched.rollback() + } finally watched.close() + fake.closeCalls.get shouldBe 1 + } + + it should "warn once about a connection held past the limit, naming the code holding it" in { + val fake = new FakeConnection + val watched = DatabaseConnectionHoldWatch.watch(fake.connection) + try { + val past = System.currentTimeMillis() + limitMillis + 1000 + DatabaseConnectionHoldWatch.heldTooLongCountAt(past) should be >= 1 + val warnings = DatabaseConnectionHoldWatch.sweep(past) + val ours = warnings.filter(_.contains(classOf[DatabaseConnectionHoldWatchTest].getName)) + ours should have size 1 + ours.head should include("has been out of the pool for") + ours.head should include(s"taken by thread ${Thread.currentThread().getName}") + DatabaseConnectionHoldWatch.sweep(past + 1000).filter(_.contains(classOf[DatabaseConnectionHoldWatchTest].getName)) shouldBe empty + } finally watched.close() + } + + it should "stop watching a connection once it is back in the pool, even if it is closed twice" in { + val fake = new FakeConnection + val past = System.currentTimeMillis() + limitMillis + 1000 + val before = DatabaseConnectionHoldWatch.heldTooLongCountAt(past) + val watched = DatabaseConnectionHoldWatch.watch(fake.connection) + DatabaseConnectionHoldWatch.heldTooLongCountAt(past) shouldBe before + 1 + watched.close() + watched.close() + DatabaseConnectionHoldWatch.heldTooLongCountAt(past) shouldBe before + fake.closeCalls.get shouldBe 2 + } +}