Auth rate limiting, 409, Organisation

ideas/CAPTURE_RELEASE_TRANSACTION_REQUEST_TYPES.md

@@ -0,0 +1,180 @@
+# Draft v2: `CAPTURE` and `RELEASE` Transaction Request Types
+
+**Status**: design draft, not yet implemented. Section 6 is the open question that needs to be resolved before code lands.
+
+## Background
+
+OBP already has a `HOLD` transaction request type (`POST /banks/{BANK_ID}/accounts/{ACCOUNT_ID}/owner/transaction-request-types/HOLD/transaction-requests`, see `obp-api/src/main/scala/code/api/v6_0_0/APIMethods600.scala:180`). HOLD moves funds from a parent account into an auto-created `HOLDING`-type sub-account, linked back via the `RELEASER_ACCOUNT_ID` account attribute.
+
+What's missing:
+
+- A `CAPTURE` step that commits held funds to a counterparty (turns the reservation into a real transfer to the recipient).
+- A `RELEASE` step that returns held funds to the parent account (cancels the reservation).
+
+Both are needed to support trading-style settlement sagas (offer/order → match → capture vs cancel → release), where funds must be reserved before a counterparty is known.
+
+This document drafts those two new types.
+
+---
+
+## Changes since v1
+
+- Path uses generic `{ACCOUNT_ID}` (not a HOLDING-specific path segment) — same as every other transaction-request type.
+- Destination uses ACCOUNT shape (`{bank_id, account_id}`), no counterparty resolution.
+- `RELEASE` destination comes from the referenced HOLD's `from_account_id`, not from the holding account's `RELEASER_ACCOUNT_ID` attribute.
+- The `hold_transaction_request_id` field is **flagged as still under discussion** — see §6.
+
+---
+
+## 1. Endpoints
+
+```
+POST /obp/v6.0.0/banks/{BANK_ID}/accounts/{ACCOUNT_ID}/owner/transaction-request-types/CAPTURE/transaction-requests
+POST /obp/v6.0.0/banks/{BANK_ID}/accounts/{ACCOUNT_ID}/owner/transaction-request-types/RELEASE/transaction-requests
+```
+
+`{ACCOUNT_ID}` is the source of the transfer — i.e., the HOLDING sub-account that received the funds when the HOLD ran. The path doesn't enforce "type = HOLDING" explicitly; the integrity check falls out of the body validation (§6).
+
+## 2. `CAPTURE` body — `TransactionRequestBodyCaptureJsonV600`
+
+```json
+{
+  "hold_transaction_request_id": "abc-123-...",
+  "to": {
+    "bank_id": "gh.29.uk",
+    "account_id": "seller-fiat-account"
+  },
+  "value": { "currency": "EUR", "amount": "250.00" },
+  "description": "Settlement of trade trade-789"
+}
+```
+
+| Field | Required | Purpose |
+|---|---|---|
+| `hold_transaction_request_id` | TBD (§6) | Linkage to originating HOLD |
+| `to.bank_id` / `to.account_id` | yes | In-bank ACCOUNT-style destination |
+| `value.currency` / `value.amount` | yes | Amount and currency to capture |
+| `description` | no | Free-form note |
+
+## 3. `RELEASE` body — `TransactionRequestBodyReleaseJsonV600`
+
+```json
+{
+  "hold_transaction_request_id": "abc-123-...",
+  "value": { "currency": "EUR", "amount": "250.00" },
+  "description": "Offer offer-456 cancelled by user"
+}
+```
+
+No `to` field. Two options for resolving the destination, depending on §6:
+
+- **If we keep `hold_transaction_request_id`**: destination = the referenced HOLD's `from_account_id`.
+- **If we drop it**: destination = the source account's `RELEASER_ACCOUNT_ID` attribute (only viable resolution path without the linkage).
+
+If `value.amount` is omitted, the server releases the full remaining balance (definition of "remaining" depends on §6).
+
+## 4. Response
+
+`transactionRequestWithChargeJSON400` — same as every other transaction-request type. If we keep §6, attach two attributes to the resulting transaction request:
+
+- `hold_transaction_request_id = abc-123-...`
+- `hold_purpose = capture` *or* `hold_purpose = release`
+
+## 5. Validation
+
+Common to both types:
+
+1. `{ACCOUNT_ID}` exists and the caller has the required view permission (same as every other transaction-request).
+2. Standard body validation (positive amount, valid currency, etc.).
+3. `value.amount` must not exceed the source account's available balance. (Already enforced by the underlying transfer machinery.)
+
+§6-dependent (only if we keep `hold_transaction_request_id`):
+
+4. The HOLD referenced exists, was a HOLD, has status `COMPLETED`.
+5. The HOLD's destination = `{ACCOUNT_ID}` in the URL. (Otherwise `HoldDoesNotMatchAccount`.)
+6. `value.currency` matches the HOLD's currency.
+7. Sum of completed `CAPTURE` + `RELEASE` against this HOLD plus `value.amount` ≤ HOLD amount. (`CaptureExceedsHoldRemaining` / `ReleaseExceedsHoldRemaining`.)
+
+## 6. Open question: do we need `hold_transaction_request_id`?
+
+This is the question that's still under discussion. Two designs:
+
+### Design A — keep the linkage
+
+CAPTURE/RELEASE bodies carry `hold_transaction_request_id`. The resulting transaction stores it as an attribute. Per-HOLD "remaining balance" is computed on demand:
+
+```
+remaining(hold_id) = hold.amount
+                   − Σ amount of CAPTURE txns linked to hold_id, status=COMPLETED
+                   − Σ amount of RELEASE txns linked to hold_id, status=COMPLETED
+```
+
+**What this gives us**
+
+- Server-enforced invariant: a HOLD cannot be over-captured or over-released; partial fills compose cleanly.
+- The optional `GET .../transaction-requests/{HOLD_ID}/balance` helper makes sense.
+- Audit/regulatory readers see "this transfer was the capture of HOLD X" without having to reason from context.
+- Distinguishes CAPTURE/RELEASE from "ordinary transfer that happens to leave a HOLDING account" at the data-model level.
+
+**What it costs**
+
+- Each operation does one extra lookup (HOLD row + sum-of-related-attributes).
+- Concurrent CAPTUREs against the same HOLD need transactional balance-check (already true for any debit transfer, but now the check is per-HOLD too).
+- Schema-level additions: just two new transaction-request attributes (`hold_transaction_request_id`, `hold_purpose`) — no new table.
+
+### Design B — drop the linkage
+
+CAPTURE/RELEASE are typed by intent only. The HOLDING account is a fungible bucket; capture/release just transfer in or out of it. No per-HOLD tracking. RELEASE's destination has to come from the HOLDING account's `RELEASER_ACCOUNT_ID` attribute (so we'd need that attribute to be load-bearing again).
+
+**What this gives us**
+
+- Simpler API. Bodies match existing ACCOUNT-type transaction requests one-for-one.
+- One source of truth for held funds: the HOLDING account's balance.
+- Trading orchestrator (or any consumer) tracks per-HOLD bookkeeping itself if it cares.
+
+**What it costs**
+
+- The system can't tell you "how much of HOLD X is still held" — only "how much is in the HOLDING account in total." Multiple HOLDs against the same parent become indistinguishable post-hoc.
+- The CAPTURE/RELEASE types reduce to "labelled transfers from a HOLDING account": almost no semantic gain over the existing ACCOUNT type beyond the label itself.
+- `RELEASER_ACCOUNT_ID` attribute on the holding account becomes mandatory and load-bearing for RELEASE to work.
+
+### Recommendation
+
+**Design A** — keep `hold_transaction_request_id`. Without it, the new types add little over plain ACCOUNT transfers, and the per-HOLD invariant ("captured + released ≤ original HOLD amount") is exactly the kind of guarantee that belongs in the API, not in every consumer. Cost is two attributes and a sum-aggregation query — well-bounded.
+
+**But** — if the view is that the trading orchestrator (or any consumer) is the rightful owner of per-HOLD bookkeeping and OBP-API should stay primitive, Design B is internally consistent and a smaller commitment.
+
+## 7. Other open questions
+
+1. **API version**: land in v6.0.0 (alongside HOLD) or v7.0.0 (gets idempotency middleware + new patterns automatically)? Mild preference for v7.0.0.
+2. **Entitlements**: `canCaptureHoldAtOneBank` / `canReleaseHoldAtOneBank` (plus AnyBank variants), or piggy-back on existing transaction-request entitlements? Probably new ones for clarity.
+3. **HOLD expiry**: should an unconsumed HOLD auto-release after a TTL, or stay parked until the orchestrator releases it? (Ties into §6: auto-release only really makes sense in Design A where "remaining" is a first-class concept.)
+4. **`GET .../balance` helper**: pure read endpoint exposing the per-HOLD remaining computation. Only meaningful in Design A.
+5. **Naming**: `hold_transaction_request_id` vs `original_transaction_request_id` — keep specific or go generic for future use?
+
+---
+
+## 8. How the trading orchestrator uses these
+
+Per-trade settlement saga (one trade, two HOLDs — buyer's fiat HOLD, seller's token HOLD):
+
+```
+1. CAPTURE buyer's fiat HOLD,  to=seller's fiat account,  amount=trade.value
+2. CAPTURE seller's token HOLD, to=buyer's token account, amount=trade.qty
+   On any failure: release the unconsumed remainder, mark trade FAILED.
+```
+
+Cancel an unfilled offer:
+
+```
+1. RELEASE the HOLD (no amount → full remaining balance).
+```
+
+Partial fill:
+
+```
+1. CAPTURE the matched portion.
+2. (Optional) RELEASE the unmatched portion if the offer is being closed.
+```
+
+Each step is an ordinary transaction-request-create call; idempotent via the `Idempotency-Key` header in the v7 idempotency middleware; auditable via the attached `hold_transaction_request_id` + `hold_purpose` attributes (Design A) or the source-account balance and type-on-the-record (Design B).

obp-api/src/main/protobuf/signal.proto

@@ -0,0 +1,82 @@
+syntax = "proto3";
+package code.obp.grpc.signal.g1;
+
+import "google/protobuf/timestamp.proto";
+
+// Mirrors SignalMessageJsonV600. The payload field carries the JSON-encoded
+// message body verbatim — proto has no native JValue, and round-tripping
+// through google.protobuf.Struct can reorder/lose nesting.
+message SignalMessage {
+  string message_id = 1;
+  string channel_name = 2;
+  string sender_consumer_id = 3;
+  string sender_user_id = 4;
+  string to_user_id = 5;            // empty string = broadcast
+  google.protobuf.Timestamp timestamp = 6;
+  string message_type = 7;
+  string payload_json = 8;          // JSON-encoded payload
+}
+
+// Mirrors SignalChannelInfoJsonV600
+message SignalChannelInfo {
+  string channel_name = 1;
+  int64 message_count = 2;
+  int64 ttl_seconds = 3;
+}
+
+// --- Publish: 1:1 with POST /signal/channels/{name}/messages ---
+
+message PublishRequest {
+  string channel_name = 1;
+  string to_user_id = 2;            // empty = broadcast
+  string message_type = 3;
+  string payload_json = 4;          // JSON-encoded payload
+}
+
+message PublishResponse {
+  string message_id = 1;
+  string channel_name = 2;
+  google.protobuf.Timestamp timestamp = 3;
+  int64 channel_message_count = 4;
+}
+
+// --- Fetch: 1:1 with GET /signal/channels/{name}/messages ---
+// Privacy filter applied server-side: caller sees broadcasts plus messages
+// to/from themselves. Same logic as REST.
+
+message FetchRequest {
+  string channel_name = 1;
+  int32 offset = 2;
+  int32 limit = 3;
+}
+
+message FetchResponse {
+  string channel_name = 1;
+  repeated SignalMessage messages = 2;
+  int64 total_count = 3;
+  bool has_more = 4;
+}
+
+// --- ListChannels: 1:1 with GET /signal/channels ---
+// Returns broadcast-visible channels only, matching REST behaviour.
+
+message ListChannelsRequest {}
+
+message ListChannelsResponse {
+  repeated SignalChannelInfo channels = 1;
+}
+
+// --- Subscribe: live-only stream of new messages ---
+// No catch-up, no replay. Late joiners ask other agents.
+// Privacy filter applied server-side, same as REST Fetch.
+
+message SubscribeRequest {
+  string channel_name = 1;
+}
+
+service SignalChannelsService {
+  rpc Publish(PublishRequest) returns (PublishResponse);
+  rpc Fetch(FetchRequest) returns (FetchResponse);
+  rpc ListChannels(ListChannelsRequest) returns (ListChannelsResponse);
+  rpc Subscribe(SubscribeRequest) returns (stream SignalMessage);
+}

obp-api/src/main/scala/bootstrap/liftweb/Boot.scala

@@ -86,6 +86,7 @@ import code.etag.MappedETag
 import code.featuredapicollection.FeaturedApiCollection
 import code.fx.{MappedCurrency, MappedFXRate}
 import code.group.Group
+import code.organisation.Organisation
 import code.kycchecks.MappedKycCheck
 import code.kycdocuments.MappedKycDocument
 import code.kycmedias.MappedKycMedia
@@ -1210,6 +1211,7 @@ object ToSchemify {
     CounterpartyAttributeMapper,
     BankAccountBalance,
     Group,
+    Organisation,
     AccountAccessRequest,
     code.chat.ChatRoom,
     code.chat.Participant,

obp-api/src/main/scala/code/api/GatewayLogin.scala

@@ -238,6 +238,12 @@ object GatewayLogin extends RestHelper with MdcLoggable {
   def getOrCreateResourceUser(jwtPayload: String, callContext: Option[CallContext]) : Box[(User, Option[String], Option[CallContext])] = {
     val username = getFieldFromPayloadJson(jwtPayload, "login_user_name")
     logger.debug("login_user_name: " + username)
+    // Pre-credential rate limit. Disabled by default; controlled via auth.rate_limit.* props.
+    // In shadow mode trips are logged and Right is returned; only enforce mode produces Left.
+    AuthRateLimiter.check(APIUtil.getRemoteIpAddress(), gateway, username) match {
+      case Left(_)  => return Failure(ErrorMessages.TooManyRequests)
+      case Right(_) => // continue
+    }
     val cbsAndCallContextBox = refreshBankAccounts(jwtPayload, callContext)
     for {
       tuple <- cbsAndCallContextBox match {
@@ -303,6 +309,11 @@ object GatewayLogin extends RestHelper with MdcLoggable {
     val jti = getFieldFromPayloadJson(jwtPayload, "jti")
     val consentId = if (jti.isEmpty) None else Some(jti)
     logger.debug("login_user_name: " + username)
+    // Pre-credential rate limit. Disabled by default; controlled via auth.rate_limit.* props.
+    AuthRateLimiter.check(APIUtil.getRemoteIpAddress(), gateway, username) match {
+      case Left(_)  => return Future.successful(Failure(ErrorMessages.TooManyRequests))
+      case Right(_) => // continue
+    }
     val cbsAndCallContextF = refreshBankAccountsFuture(jwtPayload, callContext)
     for {
       cbs <- cbsAndCallContextF

obp-api/src/main/scala/code/api/OBPRestHelper.scala

@@ -423,7 +423,10 @@ trait OBPRestHelper extends RestHelper with MdcLoggable {
         }
       }
     }
-    else if (APIUtil.getPropsAsBoolValue("allow_gateway_login", false) && hasGatewayHeader(authorization)) {
+    else if (hasGatewayHeader(authorization)) {
+      if (!APIUtil.getPropsAsBoolValue("allow_gateway_login", false)) {
+        Full(errorJsonResponse(ErrorMessages.GatewayLoginIsDisabled, 401))
+      } else {
       logger.info("allow_gateway_login-getRemoteIpAddress: " + remoteIpAddress )
       APIUtil.getPropsValue("gateway.host") match {
         case Full(h) if h.split(",").toList.exists(_.equalsIgnoreCase(remoteIpAddress) == true) => // Only addresses from white list can use this feature
@@ -462,8 +465,12 @@ trait OBPRestHelper extends RestHelper with MdcLoggable {
         case _ =>
           Failure(ErrorMessages.GatewayLoginUnknownError)
       }
-    } 
-    else if (APIUtil.getPropsAsBoolValue("allow_dauth", false) && hasDAuthHeader(cc.requestHeaders)) {
+      }
+    }
+    else if (hasDAuthHeader(cc.requestHeaders)) {
+      if (!APIUtil.getPropsAsBoolValue("allow_dauth", false)) {
+        Full(errorJsonResponse(ErrorMessages.DAuthIsDisabled, 401))
+      } else {
       logger.info("allow_dauth-getRemoteIpAddress: " + remoteIpAddress )
       APIUtil.getPropsValue("dauth.host") match {
         case Full(h) if h.split(",").toList.exists(_.equalsIgnoreCase(remoteIpAddress) == true) => // Only addresses from white list can use this feature
@@ -499,7 +506,8 @@ trait OBPRestHelper extends RestHelper with MdcLoggable {
         case _ =>
           Failure(ErrorMessages.DAuthUnknownError)
       }
-    } 
+      }
+    }
     else {
       fn(cc)
     }

obp-api/src/main/scala/code/api/cache/RedisMessaging.scala

@@ -41,6 +41,8 @@ object RedisMessaging extends MdcLoggable {
       jedis.ltrim(key, -channelMaxMessages.toLong, -1)
       // Refresh TTL on every publish
       jedis.expire(key, channelTtlSeconds)
+      // Pub/sub notification for live gRPC subscribers — fire-and-forget, no persistence
+      jedis.publish(s"obp_signal:$channelName", messageJson)
       length
     } catch {
       case e: Throwable =>

obp-api/src/main/scala/code/api/dauth.scala

@@ -137,6 +137,11 @@ object DAuth extends RestHelper with MdcLoggable {
     val userName = getFieldFromPayloadJson(jwtPayload, "smart_contract_address")
     val provider = "dauth."+getFieldFromPayloadJson(jwtPayload, "network_name")
     logger.debug("login_user_name: " + userName)
+    // Pre-credential rate limit. Disabled by default; controlled via auth.rate_limit.* props.
+    AuthRateLimiter.check(APIUtil.getRemoteIpAddress(), provider, userName) match {
+      case Left(_)  => return Failure(ErrorMessages.TooManyRequests)
+      case Right(_) => // continue
+    }
     for {
       tuple <-
         UserX.getOrCreateDauthResourceUser(provider, userName) match {
@@ -157,7 +162,12 @@ object DAuth extends RestHelper with MdcLoggable {
     val username = getFieldFromPayloadJson(jwtPayload, "smart_contract_address")
     val provider = "dauth."+ getFieldFromPayloadJson(jwtPayload, "network_name")
     logger.debug("login_user_name: " + username)
-
+    // Pre-credential rate limit. Disabled by default; controlled via auth.rate_limit.* props.
+    AuthRateLimiter.check(APIUtil.getRemoteIpAddress(), provider, username) match {
+      case Left(_)  => return Future.successful(Failure(ErrorMessages.TooManyRequests))
+      case Right(_) => // continue
+    }
+
     for {
       tuple <- Future { UserX.getOrCreateDauthResourceUser(provider, username)} map {
           case (Full(u)) =>

obp-api/src/main/scala/code/api/directlogin.scala

@@ -168,6 +168,9 @@ object DirectLogin extends RestHelper with MdcLoggable {
       } else if (userId == AuthUser.userEmailNotValidatedStateCode) {
         message = ErrorMessages.UserEmailNotValidated
         httpCode = 401
+      } else if (userId == AuthUser.rateLimitExceededStateCode) {
+        message = ErrorMessages.TooManyRequests
+        httpCode = 429
       } else {
         val jwtPayloadAsJson =
           """{

obp-api/src/main/scala/code/api/openidconnect.scala

@@ -100,6 +100,10 @@ object OpenIdConnect extends OBPRestHelper with MdcLoggable {
   }
 
   private def callbackUrlCommonCode(identityProvider: Int): JsonResponse = {
+    if (!APIUtil.getPropsAsBoolValue("allow_openid_connect", true)) {
+      return errorJsonResponse(ErrorMessages.OpenIDConnectIsDisabled, 401)
+    }
+
     val (code, state, sessionState) = extractParams(S)
     logger.debug("(code, state, sessionState) = " + (code, state, sessionState))
     logger.debug("S.receivedCookies = " + S.receivedCookies)