diff --git a/CLAUDE.md b/CLAUDE.md index 87b3216457..74e7dcf070 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -13,15 +13,15 @@ > **Migration status**: the Lift → http4s migration is complete — see the "CI (shard map + run tips)" section below for the historical-status note. The former in-place strategy/progress-tracker doc (`LIFT_HTTP4S_MIGRATION.md`) was retired once the migration finished; this file documents the resulting architecture and the gotchas encountered building it. -The goal is a full http4s migration — replace Lift Web across all version files and remove it entirely. **API versions are tech-agnostic**: a version bump means a changed/new API signature, never a framework change. Framework migration happens in-place inside the existing version file. v7.0.0 currently serves 46 endpoints; most arrived there for historical reasons and stay as-is. +The goal is a full http4s migration — replace Lift Web across all version files and remove it entirely. **API versions are tech-agnostic**: a version bump means a changed/new API signature, never a framework change. Framework migration happens in-place inside the existing version file. v7.0.0 currently registers 117 ResourceDocs; most arrived there for historical reasons and stay as-is. **Request priority chain** (`Http4sApp.baseServices`): `corsHandler` (OPTIONS short-circuit) → `AppsPage` → `StatusPage` → `Http4sResourceDocs` → v510 → v600 → v500 → v700 → Berlin Group v2 → UK v2.0 → UK v3.1 → Berlin Group v1.3 (+Alias) → v400 → v310 → v300 → v220 → v210 → v200 → v140 → v130 → v121 → `dynamicEntityRoutes` → `dynamicEndpointRoutes` → DirectLogin → OpenIdConnect → AliveCheck → `notFoundCatchAll` (JSON 404). There is no Lift fallback — `Http4sLiftWebBridge` has been removed. Any unhandled `/obp/*` path returns a JSON 404 from `notFoundCatchAll`; it does not fall through to Lift. **Key files**: `Http4s700.scala` (v7.0.0 endpoints), `Http4s200.scala` (v2.0.0 endpoints — 37 own + path-rewriting bridge to Http4s140), `Http4s140.scala` (v1.4.0 endpoints — 11 own + path-rewriting bridge to Http4s130), `Http4s130.scala` (v1.3.0 endpoints — 3 own + path-rewriting bridge to Http4s121), `Http4s121.scala` (v1.2.1 endpoints — all 323 API1_2_1Test scenarios), `Http4sSupport.scala` (EndpointHelpers + recordMetric), `ResourceDocMiddleware.scala` (auth, entity resolution, transaction wrapper), `IdempotencyMiddleware.scala` (Redis-backed idempotency, opt-in via `Idempotency-Key` header, nested inside ResourceDocMiddleware), `RequestScopeConnection.scala` (DB transaction propagation to Futures). -**v7.0.0 native endpoints** (49 ResourceDocs): root, corePrivateAccountsAllBanks, createMyBank, getMyBanks, deleteEntitlement, addEntitlement, getAccountAccessTrace, getConsentsConfig, getPasswordPolicy, getErrorMessages, getUserByUserId, createTradingOffer, getTradingOffer, getTradingOffers, cancelTradingOffer, createMarketOrder, getMarketOrder, cancelMarketOrder, createMarketMatch, getMarketTrade, requestSettlement, notifyDeposit, requestWithdrawal, createPaymentAuth, capturePaymentAuth, releasePaymentAuth, getPaymentAuth, createTestEmail, createValidationEmail, createOrganisation, getOrganisations, getOrganisation, updateOrganisation, deleteOrganisation, createRoutingScheme, getRoutingSchemes, getRoutingScheme, updateRoutingScheme, deleteRoutingScheme, getBankSupportedRoutingSchemes, putBankSupportedRoutingScheme, createPayeeLookup, createTransactionRequestMobileWallet, createTransactionRequestUtility, createTransactionRequestOpenCorridor, createTransactionRequestBulk, factoryResetSystemView. These carry genuinely v7-specific signatures/behaviour. The 20 duplicate "POC" endpoints originally added as migration scaffolding (getBanks, getBank, getCurrentUser, getCoreAccountById, getPrivateAccountByIdFull, getExplicitCounterpartyById, getFeatures, getScannedApiVersions, getConnectors, getProviders, getUsers, getCustomersAtOneBank, getCustomerByCustomerId, getAccountsAtBank, getCacheConfig, getCacheInfo, getDatabasePoolInfo, getStoredProcedureConnectorHealth, getMigrations, getCacheNamespaces) were **removed** — they cascade to their v6 twin via `v700ToV600Bridge` (getExplicitCounterpartyById → v4, no v6/v5 twin), `X-OBP-Version-Served: v6.0.0`. Kept deliberately in v7: `deleteEntitlement` (204), `addEntitlement` (409), `getUserByUserId` (404) — intentional RESTful response-code improvements over the older v6 200/400 convention. +**v7.0.0 native endpoints** (117 ResourceDocs). The authoritative list is the `resourceDocs += ResourceDoc(...)` calls in `Http4s700.scala` — read them there rather than from a copy in this file, which is what went stale before: this paragraph used to enumerate 49 names when the file held more than twice that. These carry genuinely v7-specific signatures/behaviour. The 20 duplicate "POC" endpoints originally added as migration scaffolding (getBanks, getBank, getCurrentUser, getCoreAccountById, getPrivateAccountByIdFull, getExplicitCounterpartyById, getFeatures, getScannedApiVersions, getConnectors, getProviders, getUsers, getCustomersAtOneBank, getCustomerByCustomerId, getAccountsAtBank, getCacheConfig, getCacheInfo, getDatabasePoolInfo, getStoredProcedureConnectorHealth, getMigrations, getCacheNamespaces) were **removed** — they cascade to their v6 twin via `v700ToV600Bridge` (getExplicitCounterpartyById → v4, no v6/v5 twin), `X-OBP-Version-Served: v6.0.0`. One of those 20 has since come back: `getCurrentUser` is native in v7 again, because its response gained genuinely v7-specific content (the caller's own mobile phone fields, and `on_behalf_of`). Kept deliberately in v7: `deleteEntitlement` (204), `addEntitlement` (409), `getUserByUserId` (404) — intentional RESTful response-code improvements over the older v6 200/400 convention. -**Tests**: `Http4s700RoutesTest` (91 scenarios, port 8087). `makeHttpRequest` returns `(Int, JValue, Map[String, String])`. `makeHttpRequestWithBody(method, path, body, headers)` for POST/PUT. +**Tests**: `Http4s700RoutesTest` (179 scenarios). The suite drives `Http4s700.wrappedRoutesV700Services.orNotFound` in-process; it starts no server and listens on no port. `makeHttpRequest(path, headers)` returns `(Int, JValue, Map[String, String])` and is GET-only; `makeHttpRequestWithBody(method, path, body, headers)` for POST/PUT; `makeHttpRequestWithMethod(method, path, headers)` for a verb with no body, such as DELETE. ## Migrating a Lift Endpoint to http4s Rules apply regardless of which version file the endpoint lives in. Use v7.0.0 only when the API signature is new or changed; otherwise migrate in-place in the original version file. diff --git a/ON_BEHALF_OF_USER_ID_PLAN.md b/ON_BEHALF_OF_USER_ID_PLAN.md index 8e30a57c7d..4c495cfd5d 100644 --- a/ON_BEHALF_OF_USER_ID_PLAN.md +++ b/ON_BEHALF_OF_USER_ID_PLAN.md @@ -1,11 +1,22 @@ # On-behalf-of user id — making ownership-by-the-human automatic Written 2026-09-02 evening, for pickup 2026-09-03. This is the only document: no separate -checklist. Track progress here by marking items done in place. **Status: Phases 0 and 1 committed 2026-09-02 (`2d86f4e9e`, `715989c11`). Litmus tests 1–5 run -2026-09-03 against the local instance (results inline under "Manual tests after Phase 1"); litmus 2 -exposed a v7 `addEntitlement` middleware bug, fixed in the working tree. Phase 2 item 1 -(AccountHolders) done 2026-09-03 (working tree). Next: Phase 2 item 2 (UserCustomerLink) and the -Phase 4 frozen policy test.** `AbacRuleTests` fails locally for an unrelated props reason (see Phase 1 note 2). Background and the reasoning are on the Portal page `/developers/opey-permissions` +checklist. Track progress here by marking items done in place. **Status on 2026-09-19: Phases 0 +and 1 are committed (2026-09-02, `2d86f4e9e` and `715989c11`). Phase 3's nineteen explicit-target +guards and the sweep that proves each one is reachable are green +(`ExplicitTargetConsentUserSweepTest`, 2026-09-13), and so are Phase 4's tests +(`AgentDelegationTest`, extended alongside each Phase 2 row; `UserReferenceAttributionPolicyTest` +2026-09-11; `OnBehalfOfOwnershipSweepTest` 2026-09-13). Phase 2 is the +live front and the only one being worked through a table at a time: five providers resolve the +on-behalf-of user today (AccountHolders, UserCustomerLink, DynamicEntity/DynamicData, Bank, +Counterparty), which is eight of the sixty-two references that need wiring; the other fifty-four +are listed by hand in `OnBehalfOfOwnershipSweepTest.notYetWired`, each with the reason it is still +open. The webhook row was audited on 2026-09-16 and deliberately deferred rather than wired; the +two v7 notification-webhook delete endpoints that audit turned out to need are committed +(`f2dddcd16`, 2026-09-17), and everything in Phases 0 to 4 is now committed — nothing in this plan +is sitting in the working tree. Open after that: the rest of Phase 2's mechanical batch, the +`onBehalfOfMode` endpoint tag of Decision 10, which is not built, Decision 12's +`NotImplementedForConsentUser` policy, which is an idea and not built, and Phase 5.** `AbacRuleTests` fails locally for an unrelated props reason (see Phase 1 note 2). Background and the reasoning are on the Portal page `/developers/opey-permissions` (OBP-Frontend, uncommitted) and in `OBP-Frontend/CONSENT_ESCALATION_GAP.md`. Working rules: the user commits, the assistant never does. The provider is the mechanism; @@ -51,7 +62,11 @@ Every request under a consent carries three identities, one job each: Goal: the persistence layer defaults durable user references to the on-behalf-of user, so endpoints are correct without remembering. Authorisation stays on the consent user (ConsentUtil's isolation comment, ~line 1195, explains why act-as-human is not an option; -`experimental_become_user_that_created_consent` stays deprecated). +`experimental_become_user_that_created_consent`, the props toggle that logged the human on in +place of the consent user, was **removed 2026-09-21** — it was the one switch that could turn the +whole delegation model off, and while it existed every property this plan establishes was +conditional on an operator's props file. `Boot.warnAboutRemovedProps` names it for one release so +an instance that still sets it is told rather than silently changed). ## What already exists (reuse, don't duplicate) @@ -114,10 +129,10 @@ with a `vend` is a table-backed provider and the name would read as a new table. * Invariant: the result row is an original user (isOriginalUser); a consent user whose consent names * another consent user is a data bug (WARN + Failure — the only case that cannot fall back). * Takes only the id on purpose: nothing request-asserted (body/header/query) can steer it. */ -def onBehalfOfUserIdOf(userId: String): Box[String] +def resolveOnBehalfOfUserId(userId: String): Box[String] /** True when `userId` acts for itself and may own durable state. */ -def actsForSelf(userId: String): Boolean = onBehalfOfUserIdOf(userId).exists(_ == userId) +def actsForSelf(userId: String): Boolean = resolveOnBehalfOfUserId(userId).exists(_ == userId) // ---- what a provider gets back: everything it should store, plus the log line ----------- case class Attribution( @@ -129,7 +144,7 @@ case class Attribution( def isDelegated: Boolean = userId != onBehalfOfUserId /** the single value for the column `ref` names, per its policy */ def userIdToStore: String = ref.policy match { - case KeepUserId => userId + case UseAuthenticatedUserId => userId case UseOnBehalfOfUserId => onBehalfOfUserId case Reject => userId // unreachable: attributionOf fails first } @@ -137,7 +152,7 @@ case class Attribution( // ---- the entry point providers actually call --------------------------------------------- /** Attribution for writing column `ref` as `userId`. Applies `ref.policy`: - * KeepUserId / UseOnBehalfOfUserId → Full(attribution), WARN naming `ref` when isDelegated + * UseAuthenticatedUserId / UseOnBehalfOfUserId → Full(attribution), WARN naming `ref` when isDelegated * Reject → Full(attribution) if !isDelegated, else Failure(InvalidUserId … names a consent user) */ def attributionOf(userId: String, ref: UserReference): Box[Attribution] @@ -147,12 +162,12 @@ def attributedUserId(userId: String, ref: UserReference): Box[String] = attribut `UserReference` is the policy file as code (main tree, see "The policy file" below). The `ref` argument is chosen by provider code, never from the request, so the "no caller-asserted input" property of -`onBehalfOfUserIdOf` still holds. `consentId` is derived inside the resolver, never passed in, for +`resolveOnBehalfOfUserId` still holds. `consentId` is derived inside the resolver, never passed in, for the same reason. Implementation notes: -1. `onBehalfOfUserIdOf` = the chain `addEntitlement` and `CallContext.accountableUserId` both +1. `resolveOnBehalfOfUserId` = the chain `addEntitlement` and `CallContext.accountableUserId` both inline today (`ResourceUser.find(By(userId_)) → CreatedByConsentId → getConsentByConsentId → consent.userId`). Both then delegate to it; CallContext keeps its `consentCreator.or(consenter)` precedence in front. @@ -184,34 +199,35 @@ Call sites after Phase 1: def onBehalfOfUser: Box[User] = consentCreator.or(consenter).or(user) // was humanUser def onBehalfOfUserId: String = // was accountableUserId consentCreator.or(consenter).map(_.userId).filter(_.nonEmpty) - .openOr(Users.users.vend.onBehalfOfUserIdOf(user.map(_.userId).openOr(""))) + .openOr(Users.users.vend.resolveOnBehalfOfUserId(user.map(_.userId).openOr(""))) // MappedEntitlements.addEntitlement: the magic-string exemption becomes a reference choice -val ref = if (createdByProcess == Constant.consent_user) UserReference.ConsentEntitlementUser - else UserReference.EntitlementUser +val ref = if (createdByProcess == Constant.consent_user) UserReference.Entitlement_UserId_ConsentScope + else UserReference.Entitlement_UserId for { targetUserId <- Users.users.vend.attributedUserId(userId, ref); ... } // MappedTransactionRequestProvider: a record-both table, one call, two columns -for { a <- Users.users.vend.attributionOf(userId, UserReference.TransactionRequest) } yield +for { a <- Users.users.vend.attributionOf(userId, UserReference.TransactionRequest_UserId) } yield tr.mUserId(a.userId).mOnBehalfOfUserId(a.onBehalfOfUserId) ``` ### The policy file — an attribution policy for every user-reference column (from a grep of Mapped classes) **Written 2026-09-02: `obp-api/src/main/scala/code/users/UserReference.scala` is now the source of -truth — 72 references, 9 not-a-user-id exclusions.** The tables below were the draft; the file was +truth — 75 references (13 `UseAuthenticatedUserId`, 59 `UseOnBehalfOfUserId`, 3 `Reject`), 6 +not-a-user-id exclusions.** The tables below were the draft; the file was generated from an inventory of every model in `ToSchemify.models` and covers more than the tables. Columns the draft missed, and the policy given (change in the file if wrong): | policy | added | |---|---| -| `KeepUserId` | `AuthUser.user` (login row), `OpenIDConnectToken.AuthUserPrimaryKey`, `MappedUserRefreshes.mUserId`, `MetricArchive.userId`, `DynamicDataAccess.GrantedBy` (audit) | +| `UseAuthenticatedUserId` | `AuthUser.user` (login row), `OpenIDConnectToken.AuthUserPrimaryKey`, `MappedUserRefreshes.mUserId`, `MetricArchive.userId`, `DynamicDataAccess.GrantedBy` (audit) | | `UseOnBehalfOfUserId` | `MappedUserScope.mUserId`, `DirectDebit.UserId`, `DynamicData.UserId`, `DynamicDataAccess.UserId`, `MappedCounterpartyWhereTag.user`, `MappedTag.user`, `MappedWhereTag.user`, `MappedTransactionImage.user`, `MappedCustomerMessage.user`, `MappedKycDocument.user`, `MappedKycStatus.user`, `MappedSocialMedia.user`, `MappedKycCheck.user`, `SignatoryPanel.UserIds`, `ChatMessage.MentionedUserIds` | | `Reject` | `Token.userForeignKey` (OAuth token issued to a consent user) | | not a user id | `MappedBankAccount.holder`, `MappedTransaction.counterpartyAccountHolder`, `AccountAccessRequest.CheckerComment`, `MappedKycCheck.mStaffName`, `MappedMeeting.mStaffToken`, `MappedEntitlement.mCreatedByProcess`, `ResourceUser.userId_` / `CreatedByConsentId` / `CreatedByUserInvitationId` | `AccountAccessRequest` is three references (requestor, target, checker). Record-both tables are one -reference with two fields (`TransactionRequest`). Classes are named as fully-qualified strings, not +reference with two fields (`TransactionRequest_UserId`). Classes are named as fully-qualified strings, not `classOf`, so the file imports nothing and cannot trigger Mapper initialisation. Rule: **the agent owns nothing durable.** Only the consent's own authorisation rows stay on the consent user. @@ -221,11 +237,11 @@ column takes when the user is a consent user (or an agent user with an on-behalf | policy | meaning | |---|---| -| `KeepUserId` | the authenticated user's own id; no resolver | +| `UseAuthenticatedUserId` | the authenticated user's own id; no resolver | | `UseOnBehalfOfUserId` | the on-behalf-of user's id, via the resolver in the provider | | `Reject` | the request is refused with 400 | -"Record both" below is a table-level description: one `KeepUserId` column and one +"Record both" below is a table-level description: one `UseAuthenticatedUserId` column and one `UseOnBehalfOfUserId` column on the same row. Such tables make one `attributionOf` call with a table-level reference and write both fields of the `Attribution`. @@ -235,31 +251,61 @@ The policy file is **main-tree Scala**, because `Users.attributionOf` reads it a ```scala sealed trait AttributionPolicy object AttributionPolicy { - case object KeepUserId extends AttributionPolicy - case object UseOnBehalfOfUserId extends AttributionPolicy - case object Reject extends AttributionPolicy + case object UseAuthenticatedUserId extends AttributionPolicy + case object UseOnBehalfOfUserId extends AttributionPolicy + case object Reject extends AttributionPolicy } -/** One value per user-reference column (or per record-both table). Naming: . */ -sealed abstract class UserReference(val policy: AttributionPolicy, val mapper: Class[_], val fields: List[String]) +/** One value per user-reference column (or per record-both table). */ +sealed abstract class UserReference(val policy: AttributionPolicy, val mapperClass: String, + val fields: List[String], val note: String) object UserReference { - case object AccountAccessUser extends UserReference(KeepUserId, classOf[AccountAccess], List("user")) - case object ConsentEntitlementUser extends UserReference(KeepUserId, classOf[MappedEntitlement], List("mUserId")) // createdByProcess == consent_user - case object EntitlementUser extends UserReference(UseOnBehalfOfUserId, classOf[MappedEntitlement], List("mUserId")) - case object AccountHolderUser extends UserReference(UseOnBehalfOfUserId, classOf[MapperAccountHolders], List("user")) - case object TransactionRequest extends UserReference(UseOnBehalfOfUserId, classOf[MappedTransactionRequest], List("mUserId", "mOnBehalfOfUserId")) // record both - case object ConsentCreator extends UserReference(Reject, classOf[MappedConsent], List("mUserId")) - case object OAuthConsumerCreator extends UserReference(Reject, classOf[Consumer], List("createdByUserId")) - // … one per row of the tables below + case object AccountAccess_UserFk extends UserReference(UseAuthenticatedUserId, "code.views.system.AccountAccess", List("user"), "…") + case object Entitlement_UserId_ConsentScope extends UserReference(UseAuthenticatedUserId, "code.entitlement.MappedEntitlement", List("mUserId"), "only when createdByProcess == consent_user …") + case object Entitlement_UserId extends UserReference(UseOnBehalfOfUserId , "code.entitlement.MappedEntitlement", List("mUserId"), "the role holder …") + case object AccountHolders_User extends UserReference(UseOnBehalfOfUserId , "code.accountholders.MapperAccountHolders", List("user"), "…") + case object Bank_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.model.dataAccess.MappedBank", List("CreatedByUserId"), "…") + case object TransactionRequest_UserId extends UserReference(UseOnBehalfOfUserId, "code.transactionrequests.MappedTransactionRequest", List("mUserId", "mOnBehalfOfUserId"), "record both") + case object Consent_UserId extends UserReference(Reject, "code.consent.MappedConsent", List("mUserId"), "…") + case object Consumer_CreatedByUserId extends UserReference(Reject, "code.model.Consumer", List("createdByUserId"), "…") + // … one per user-reference column in ToSchemify.models — 75 today val all: List[UserReference] = List(...) // the frozen test walks this } ``` +**Naming (settled 2026-09-15).** A reference is named after the **column it governs**, not after a +role: `
_`, with the `Mapped`/`Mapper` class prefix and Lift's `m` field prefix +dropped. So `MappedBank.CreatedByUserId` → `Bank_CreatedByUserId`, `MapperAccountHolders.user` → +`AccountHolders_User`. + +A reference that governs **two** columns — the record-both tables, and the tables where one policy +covers a `CreatedByUserId`/`UpdatedByUserId` pair — is named after the first only, and `fields` +carries both: `MappedTransactionRequest.{mUserId, mOnBehalfOfUserId}` → `TransactionRequest_UserId`. +Enumerating the second column in the name was tried and dropped, because it made `Table_A_B` +ambiguous: `TransactionRequest_UserId_OnBehalfOfUserId` had a second *column* in third position +while `Entitlement_UserId_ConsentScope` has a *disambiguator* there, and nothing in the name said +which. `fields` is the contract and the name is only the handle, so `Table_Column` now holds +everywhere and a third part means exactly one thing. The earlier role labels (`BankCreator`, +`AccountHolderUser`, …) read as English but did not say which column they wrote, which is the one +thing a reader of this table needs. One invented disambiguator: `MappedEntitlement.mUserId` carries +two policies, so the consent-engine one is `Entitlement_UserId_ConsentScope`. + +The underscore is doing real work and is not decoration. `Counterparty_CreatedByUserId` +is legible where the run-together form was not, and it marks the table/column boundary that a reader +otherwise has to guess at (`AccountHolders_User` vs `AccountHolder_sUser`). A dot would read better +still, but a Scala `case object` identifier cannot contain one — it would mean nesting each table in +its own wrapper object, which regroups the file **by table** when the thing that governs behaviour is +the **policy**, and three tables (`MappedEntitlement`, `ChatMessage`, `DynamicDataAccess`) carry two +policies each and would have to straddle the sections. An underscore buys the same boundary flat. + +`mapperClass` is a fully-qualified **string**, not `classOf`, so the file imports nothing and cannot +trigger Mapper initialisation; `note` carries the reason, and every one of the 75 has one. + Carrying `mapper` + `fields` on each value is what lets the Phase-4 frozen test tie every reflected Mapper column to exactly one reference (one column may have two references only when they differ by process, as `MappedEntitlement.mUserId` does). -### KeepUserId — authorisation materialisation, NO resolver +### UseAuthenticatedUserId — authorisation materialisation, NO resolver | # | class | field | note | |---|---|---|---| | 1 | `views/system/AccountAccess` | user id | views copied from the JWT each request; ALL_CONSUMERS rows; has lifecycle GC | @@ -270,7 +316,7 @@ they differ by process, as `MappedEntitlement.mUserId` does). | 6 | `chat/MappedChatMessage` | `SenderUserId` | sender = the user is truthful; `MentionedUserIds` are humans by construction | | 7 | `api/pemusage/MappedPemUsage` | `LastUserId` | audit | -### Record both — `KeepUserId` column + `UseOnBehalfOfUserId` column on one row +### Record both — `UseAuthenticatedUserId` column + `UseOnBehalfOfUserId` column on one row | # | class | user field | on-behalf-of field | action | |---|---|---|---|---| | 8 | `metrics/MappedMetrics` | `userId` | via `consent_reference_id` | none | @@ -283,7 +329,7 @@ they differ by process, as `MappedEntitlement.mUserId` does). |---|---|---|---| | 12 | `accountholders/MapperAccountHolders` | `user` FK | `getOrCreateAccountHolder(user, …)` (:39) — resolve `user` first | | 13 | `usercustomerlinks/MappedUserCustomerLink` | `mUserId` | `createUserCustomerLink(userId, …)` (:14) | -| 14 | `accountapplication/MappedAccountApplication` | `mUserId` | create (v3.1 endpoint already guards; make provider default) | +| 14 | `accountapplication/MappedAccountApplication` | `mUserId` | **Corrected 2026-09-15: do NOT make the provider default.** The note above predates the Phase 3 guards. `user_id` is always explicit at `Http4s310.scala:3228` — `userId = postedData.user_id`, never defaulted to the caller — so there is no implicit-self path for a redirect to fire on. If one did fire it would silently substitute an id the caller explicitly named, turning a correct 400 into a quiet rewrite and inverting the doctrine (explicit → refuse, implicit → redirect). Same shape as `AccountAccessRequest_TargetUserId`: guard at the endpoint, which is already done, and reclassify rather than wire. | | 15 | `accountaccessrequest/AccountAccessRequest` | `RequestorUserId`, `TargetUserId`, `CheckerUserId` | create + approve (v6 endpoints already guard target) | | 16 | `entitlementrequest/MappedEntitlementRquests` | `mUserId` | create (v3.0 endpoint resolves already) | | 17 | `apicollection/ApiCollection` | `UserId` | create | @@ -310,11 +356,11 @@ they differ by process, as `MappedEntitlement.mUserId` does). ### Phase 1 deliverables (all ✅ 2026-09-02) 1. `obp-api/src/main/scala/code/users/UserReference.scala`: `AttributionPolicy`, `Attribution`, and `UserReference` with **one case object per row of the tables above (all 32)** and `all` listing them. Not a database table, and not these markdown tables: the markdown is the working draft, the Scala file is what runs (via `Users.attributionOf`) and what `UserReferenceAttributionPolicyTest` (Phase 4) checks. -2. `Users` trait: `onBehalfOfUserIdOf`, `actsForSelf`, `attributionOf`, `attributedUserId`. +2. `Users` trait: `resolveOnBehalfOfUserId`, `actsForSelf`, `attributionOf`, `attributedUserId`. 3. `LiftUsers`: the implementation, with the cache rule and the `isOriginalUser` check. 4. `CallContext.onBehalfOfUserId` delegates to the resolver (precedence kept). -5. `MappedEntitlements.addEntitlement` via `attributedUserId` with `ConsentEntitlementUser` / `EntitlementUser`. -6. `MappedTransactionRequestProvider` via one `attributionOf(userId, TransactionRequest)` call, both columns. +5. `MappedEntitlements.addEntitlement` via `attributedUserId` with `Entitlement_UserId_ConsentScope` / `Entitlement_UserId`. +6. `MappedTransactionRequestProvider` via one `attributionOf(userId, TransactionRequest_UserId)` call, both columns. 7. `AgentDelegationTest` scenarios (Phase 4, item 1) green; grep for any other inline copy of the chain and point it at the resolver. ## Manual tests after Phase 1 (litmus, against a running instance) — **run 2026-09-03** @@ -343,7 +389,7 @@ H's normal token. 2. **Entitlement redirect.** As C: `POST /obp/v7.0.0/users//entitlements` with a role C may grant. Expect 201 and the entitlement's `user_id` = H, not C. Then `GET /obp/v6.0.0/users/current` as H shows the role. Log has one WARN from - `attribution EntitlementUser` naming C, H, and the consent id. + `attribution Entitlement_UserId` naming C, H, and the consent id. 3. **Consent-engine exemption.** Create a new consent as H and use it once. The consent user's own rows in `entitlement` (createdByProcess `consent_user`) are on the consent user, not on H. 4. **Payment attribution.** As C: create a transaction request (`SANDBOX_TAN` is enough) on one of @@ -351,7 +397,7 @@ H's normal token. `monbehalfofuserid` = H. Then `GET .../transaction-requests` as H lists it. 5. **Reject.** As C: `POST /obp/v5.1.0/my/consents/IMPLICIT` (create a consent while being a consent user). Until Phase 3 this still succeeds — it is the litmus that Phase 3 is needed. - After Phase 3: 400 `OBP-30107` naming `ConsentCreator`. + After Phase 3: 400 `OBP-30107` naming `Consent_UserId`. 6. **BG late binding (if a BG sandbox is set up).** Create a BG consent via the TPP flow, call `/users/current` with it before authorisation: `on_behalf_of` null. Authorise as H, call again within a minute: `on_behalf_of.user_id` = H (proves the unbound answer was not cached). @@ -364,7 +410,7 @@ Pattern, one line at the top of each create/link method, naming the column being ```scala for { - ownerId <- Users.users.vend.attributedUserId(userId, UserReference.AccountHolderUser) // WARNs when delegated + ownerId <- Users.users.vend.attributedUserId(userId, UserReference.AccountHolders_User) // WARNs when delegated ... ``` @@ -374,29 +420,31 @@ reference — are caught by the Phase-4 sweep; the second is also visible in rev 1. Providers that take a `User` (AccountHolders): resolve to id, re-fetch the on-behalf-of `User` once (cached). 2. Keep endpoint-level `cc.onBehalfOfUserId` uses; they become redundant clarity, not the mechanism. -3. `KeepUserId` writers that share a provider method with a `UseOnBehalfOfUserId` path (views materialiser, consent entitlements) pass a different `UserReference` (e.g. `ConsentEntitlementUser` vs `EntitlementUser`); no more string-typed exemptions. +3. `UseAuthenticatedUserId` writers that share a provider method with a `UseOnBehalfOfUserId` path (views materialiser, consent entitlements) pass a different `UserReference` (e.g. `Entitlement_UserId_ConsentScope` vs `Entitlement_UserId`); no more string-typed exemptions. -Order of attack (highest strand-risk first): AccountHolders → UserCustomerLink → AccountApplication → UserAuthContext → ApiCollection/UserAttribute → the rest mechanically. +Order of attack (highest strand-risk first): AccountHolders → UserCustomerLink → AccountApplication → UserAuthContext → ApiCollection/UserAttribute → the rest mechanically. That order has been departed from twice and both times for a reason worth repeating: DynamicEntity/DynamicData (row 3) came early because the Portal and API Manager conversation entities needed it, and Bank (row 4) came early because the stranding it describes had already happened on the live instance. So the named order now applies to what is left, not to what has been done. Progress: | # | provider | status | |---|---|---| -| 1 | `MapperAccountHolders.getOrCreateAccountHolder` (`AccountHolderUser`) | ✅ 2026-09-03. Resolves `user.userId` via `attributedUserId`, re-fetches the on-behalf-of `User` once when delegated, writes the row for it. All five callers (v5/v7 createAccount via `BankAccountCreation`, holding accounts, `AfterApiAuth`, `AuthUser.refreshUser`, sandbox import) go through it. `AgentDelegationTest` has three scenarios (consent user → human holds; original user unchanged; unbound consent fails closed). Endpoint-level `cc.onBehalfOfUserId` in v5/v7 createAccount stays as clarity. | -| 2 | `MappedUserCustomerLink.createUserCustomerLink` | ✅ 2026-09-10 (working tree). Provider resolves via `linkOwnerUserId` on the three methods keyed by a single user id: `createUserCustomerLink`, `getOCreateUserCustomerLink`, and the two-argument `getUserCustomerLink`. Those three had to move together: the two-argument lookup is every caller's "already linked?" pre-check immediately before a create, and the table carries `UniqueIndex(mUserId, mCustomerId)` — a redirected create paired with an unredirected pre-check passes the check on the consent user and then breaks the index on the human (500, not the intended 400 `CustomerAlreadyExistsForUser`, since `createUserCustomerLink` has no `tryo`). `getUserCustomerLinksByUserId` is deliberately **not** resolved: it also serves the admin lookup at `GET /banks/BANK_ID/user_customer_links/users/USER_ID`, where the id is an explicit target and rewriting it would silently answer a different question; endpoints meaning "my links" pass the resolved id themselves. Phase 3 guards added to all five explicit-target callers (v1.4.0 `addCustomer`, v2.0.0 / v2.1.0 `createCustomer` — guarded only when `user_id` is supplied, since an omitted one means the caller and the provider redirects it — and v2.0.0 / v4.0.0 `createUserCustomerLinks`), each with `InvalidUserId` added to the ResourceDoc error list and a digest-bound `parity_allowlist.json` entry. `AgentDelegationTest` has five scenarios (consent user → human; original user unchanged; unbound consent fails closed; the pre-check asks about the row the create would write; listing by user id is not redirected). 33 scenarios green. | -| 3 | `DynamicData.UserId` (`DynamicDataUser`), `DynamicEntity.UserId` (`DynamicEntityUser`) | ✅ 2026-09-04 (working tree). Provider `MappedDynamicDataProvider` resolves the caller on **every** entry point (save, update, get, getAll, delete, existsData): personal rows are keyed by the same column on reads and writes, so the redirect must be symmetric or a consent user could not read back what it wrote. Definition creator resolved in `MappedDynamicEntityProvider.createOrUpdate`. **Decided 2026-09-04 (access control): a consent user gets no `personal_requires_role=false` waiver** — on `/my` endpoints it must hold the entity's role, so a Consent has to name the entity explicitly before its holder reaches the human's personal rows (`Http4sDynamicEntity.personalRoleWaived`); the projection read path resolves the owner the same way (`personalRowOwner`). Doc strings of the six My endpoints say so; `UserHasMissingRoles` is now always in their error lists. Tests: `AgentDelegationTest` (provider + definition) and `DynamicEntityConsentUserTest` (HTTP: human no role → 201; consent without role → 403 naming the role; consent with roles → 201, row readable by both, stored on the human). Consumer: the Portal / API Manager Opey conversation entities (`obp_portal_opey_conversation`, `obp_manager_opey_conversation`); the apps write those as the human; **built 2026-09-04 in OBP-Frontend** (definitions, startup bootstrap, `ConversationRecorder`, rows under My Data). **Out of scope here: row-level (ACL) entities** — `DynamicDataAccess.UserId` bootstrap grant and the `allows` checks both stay on the consent user (consistent with each other: rows strand, nothing leaks); `DynamicDataAccessUser` is a later Phase 2 row. | -| 4 | `MappedBank.CreatedByUserId` (`BankCreator`) | ✅ 2026-09-14. `LocalMappedConnector.bankCreatorUserId` resolves the caller before `createOrUpdateBank` stamps the row. Two sources in the order `CallContext.onBehalfOfUserId` uses: the request layer (`consentCreator`/`consenter`, which a BG/UK consent carries on the request and the stored chain cannot know) wins, otherwise `attributedUserId(_, BankCreator)` walks the stored chain, applies the policy and logs the delegation. **Closes the defect seen in the wild 2026-09-03**: a bank created through Opey under a temporary consent had `createdbyuserid` = the consent user, so it dropped out of every "banks created by me" read once that consent was revoked. Only one of the four `MappedBank.create` sites sets the column — the Boot, sandbox-import and internal-connector paths have no user and leave it empty. The read side (`Http4s700` self-service quota) already counted via `humanAndAgentUserIds`, so it keeps matching either way; write and read now agree on the human. `AgentDelegationTest` has five scenarios (original user unchanged; consent user → human; unbound consent fails closed; request-layer consenter wins; no authenticated user leaves it empty), and the `BankCreator` entry is gone from `OnBehalfOfOwnershipSweepTest.notYetWired` — the ratchet fails if it comes back. 49 scenarios green. | +| 1 | `MapperAccountHolders.getOrCreateAccountHolder` (`AccountHolders_User`) | ✅ 2026-09-03. Resolves `user.userId` via `attributedUserId`, re-fetches the on-behalf-of `User` once when delegated, writes the row for it. All five callers (v5/v7 createAccount via `BankAccountCreation`, holding accounts, `AfterApiAuth`, `AuthUser.refreshUser`, sandbox import) go through it. `AgentDelegationTest` has three scenarios (consent user → human holds; original user unchanged; unbound consent fails closed). Endpoint-level `cc.onBehalfOfUserId` in v5/v7 createAccount stays as clarity. | +| 2 | `MappedUserCustomerLink.createUserCustomerLink` | ✅ 2026-09-10, committed. Provider resolves via `linkOwnerUserId` on the three methods keyed by a single user id: `createUserCustomerLink`, `getOCreateUserCustomerLink`, and the two-argument `getUserCustomerLink`. Those three had to move together: the two-argument lookup is every caller's "already linked?" pre-check immediately before a create, and the table carries `UniqueIndex(mUserId, mCustomerId)` — a redirected create paired with an unredirected pre-check passes the check on the consent user and then breaks the index on the human (500, not the intended 400 `CustomerAlreadyExistsForUser`, since `createUserCustomerLink` has no `tryo`). `getUserCustomerLinksByUserId` is deliberately **not** resolved: it also serves the admin lookup at `GET /banks/BANK_ID/user_customer_links/users/USER_ID`, where the id is an explicit target and rewriting it would silently answer a different question; endpoints meaning "my links" pass the resolved id themselves. Phase 3 guards added to all five explicit-target callers (v1.4.0 `addCustomer`, v2.0.0 / v2.1.0 `createCustomer` — guarded only when `user_id` is supplied, since an omitted one means the caller and the provider redirects it — and v2.0.0 / v4.0.0 `createUserCustomerLinks`), each with `InvalidUserId` added to the ResourceDoc error list and a digest-bound `parity_allowlist.json` entry. `AgentDelegationTest` has five scenarios (consent user → human; original user unchanged; unbound consent fails closed; the pre-check asks about the row the create would write; listing by user id is not redirected). 33 scenarios green. | +| 3 | `DynamicData.UserId` (`DynamicData_UserId`), `DynamicEntity.UserId` (`DynamicEntity_UserId`) | ✅ 2026-09-04, committed. Provider `MappedDynamicDataProvider` resolves the caller on **every** entry point (save, update, get, getAll, delete, existsData): personal rows are keyed by the same column on reads and writes, so the redirect must be symmetric or a consent user could not read back what it wrote. Definition creator resolved in `MappedDynamicEntityProvider.createOrUpdate`. **Decided 2026-09-04 (access control): a consent user gets no `personal_requires_role=false` waiver** — on `/my` endpoints it must hold the entity's role, so a Consent has to name the entity explicitly before its holder reaches the human's personal rows (`Http4sDynamicEntity.personalRoleWaived`); the projection read path resolves the owner the same way (`personalRowOwner`). Doc strings of the six My endpoints say so; `UserHasMissingRoles` is now always in their error lists. Tests: `AgentDelegationTest` (provider + definition) and `DynamicEntityConsentUserTest` (HTTP: human no role → 201; consent without role → 403 naming the role; consent with roles → 201, row readable by both, stored on the human). Consumer: the Portal / API Manager Opey conversation entities (`obp_portal_opey_conversation`, `obp_manager_opey_conversation`); the apps write those as the human; **built 2026-09-04 in OBP-Frontend** (definitions, startup bootstrap, `ConversationRecorder`, rows under My Data). **Out of scope here: row-level (ACL) entities** — `DynamicDataAccess.UserId` bootstrap grant and the `allows` checks both stay on the consent user (consistent with each other: rows strand, nothing leaks); `DynamicDataAccess_UserId` is a later Phase 2 row. | +| 4 | `MappedBank.CreatedByUserId` (`Bank_CreatedByUserId`) | ✅ 2026-09-14. `LocalMappedConnector.bankCreatorUserId` resolves the caller before `createOrUpdateBank` stamps the row. Two sources in the order `CallContext.onBehalfOfUserId` uses: the request layer (`consentCreator`/`consenter`, which a BG/UK consent carries on the request and the stored chain cannot know) wins, otherwise `attributedUserId(_, Bank_CreatedByUserId)` walks the stored chain, applies the policy and logs the delegation. **Closes the defect seen in the wild 2026-09-03**: a bank created through Opey under a temporary consent had `createdbyuserid` = the consent user, so it dropped out of every "banks created by me" read once that consent was revoked. Only one of the four `MappedBank.create` sites sets the column — the Boot, sandbox-import and internal-connector paths have no user and leave it empty. The read side (`Http4s700` self-service quota) already counted via `humanAndAgentUserIds`, so it keeps matching either way; write and read now agree on the human. `AgentDelegationTest` has five scenarios (original user unchanged; consent user → human; unbound consent fails closed; request-layer consenter wins; no authenticated user leaves it empty), and the `Bank_CreatedByUserId` entry is gone from `OnBehalfOfOwnershipSweepTest.notYetWired` — the ratchet fails if it comes back. 49 scenarios green. | +| 5 | `MappedCounterparty.mCreatedByUserId` (`Counterparty_CreatedByUserId`) | ✅ 2026-09-15, and the first row decided as **record both** rather than redirect. `MapperCounterparties.counterpartyCreators` makes one `attributionOf` call and writes the actor to `mCreatedByUserId` and the human to a new `mCreatedByOnBehalfOfUserId`. Two reasons it is not a redirect: (a) a counterparty is the control on **where money may be sent**, so "which agent created this" has to be answerable from the row rather than by correlating a timestamp against a metrics table with its own retention — the same argument that made `MappedTransactionRequest` record both; (b) `mCreatedByUserId` is published as `created_by_user_id` on the **v2.2.0 and v4.0.0** counterparty responses, so redirecting it would make a STABLE field report a human for something an agent did (`CounterpartyTest` still green, confirming the API is unchanged). Safe at provider level because none of the four call sites takes a user id from the request — v2.2.0 `createCounterparty`, v4.0.0 `createExplicitCounterparty` and `createCounterpartyForAnyAccount`, and the v5.0.0 VRP consent flow all pass the caller's own id — so there is no explicit target a redirect could silently substitute. The new column is **internal**: not on `CounterpartyTrait` (obp-commons, implemented by the remote-connector DTOs) and not in any JSON, because adding a field to those STABLE responses would change the frozen contract; a v7 read can expose it later. No migration — Schemifier adds new columns, as it did for `MappedTransactionRequest.mOnBehalfOfUserId`. **Limitation**: the provider receives only a `String`, so unlike `bankCreatorUserId` it cannot honour the request layer's `consentCreator`/`consenter`; a BG/UK consent with no stored human yet falls back to the actor, which is the documented fail-closed behaviour. `AgentDelegationTest` has four scenarios (original user in both columns; consent user → actor + human; unbound consent fails closed; a broken chain still names the actor rather than blanking the audit column). 53 scenarios green. | +| 6 | the three webhook creator columns: `AccountWebhook_CreatedByUserId`, `SystemAccountNotificationWebhook_CreatedByUserId`, `BankAccountNotificationWebhook_CreatedByUserId` | **Deferred 2026-09-16, deliberately, and the reason is written down in `todo/webhook_attribution.md`.** The audit corrected two things this plan believed. First, the creator column is *not* an ownership key: the only read that treats it as one, `getAccountWebhooksByUserIdFuture`, has no caller anywhere in the repo, while the live list endpoint `getAccountWebhooks` (`Http4s310.scala:625`) is gated on `canGetWebhooks`, returns every webhook at the bank and treats `user_id` as an optional filter the caller supplies. Editing does not consult the column either. So nobody is locked out of an agent-created webhook, and the `UserReference` comment that claimed otherwise has been corrected. Second, `created_by_user_id` is published on the v3.1.0 and both v4.0.0 responses, so a redirect would make a STABLE field report a person for something an agent did. The agreed direction is therefore **record both**, as `MappedCounterparty` does, and the argument is sharper here than for a counterparty because nothing garbage-collects a webhook when its Consent is revoked (Phase 5 item 3 declines revocation GC), so an agent-created webhook keeps POSTing account events forever with `created_by_user_id` naming an identity that no longer exists. It is not built, because the webhook code had not been read in a long time and the audit found two dead paths in it. The three entries in `OnBehalfOfOwnershipSweepTest.notYetWired` therefore carry their own `webhookDeferred` reason instead of the generic `mechanicalBatch` one, so that nobody picks them up as a quick win. **The second dead path is now closed**: `deleteSystemAccountNotificationWebhookFuture` and `deleteBankAccountNotificationWebhookFuture` existed but no endpoint called either, so a notification webhook could not be removed over the API by anyone. v7.0.0 gained `deleteSystemAccountNotificationWebhook` (`DELETE /web-hooks/account/notifications/on-create-transaction/WEBHOOK_ID`, role `canDeleteSystemAccountNotificationWebhook`) and `deleteBankAccountNotificationWebhook` (the same path under `/banks/BANK_ID`, role `canDeleteAccountNotificationWebhookAtOneBank`), both answering 204, with `NotificationWebhookNotFound` (OBP-30151) and `DeleteWebhookError` (OBP-30152). A webhook belonging to another bank reads as 404 rather than 403, because the role is held per bank and 403 would let a caller with the role at one bank discover which webhook ids exist at every other bank. Eight scenarios in `Http4s700RoutesTest`; 179 scenarios green on 2026-09-17. The endpoints, the two roles and the two error codes are committed in `f2dddcd16`. Adding attribution to a row that could not be deleted would have been the wrong order. **What "deferred" means on the wire, since the word does not say it:** nothing is refused and nothing is redirected. A consent user holding `canCreateWebhook` that POSTs to `/banks/BANK_ID/account-web-hooks` gets the ordinary **201**, and the row is stamped with the *agent's* id, because all three create endpoints pass `user.userId` straight to the provider (`Http4s310.scala:2473`, `Http4s400.scala:5638`, `Http4s400.scala:5661`) and no provider calls `attributionOf`. The human is recorded nowhere on the row. That is exactly the pre-plan behaviour, so "deferred" is the status quo continuing, not a hold: the webhook keeps firing after the Consent is revoked and `created_by_user_id` then names an identity that no longer exists. Decision 12 is the proposal to make that state say so out loud instead of looking like success. The first dead path, `getAccountWebhooksByUserIdFuture` with no caller, is untouched. | **After Phase 2 — the third set of things a Consent carries.** Personal resources are owned, not granted, so delegating them through entity Roles over-grants. Design settled 2026-09-04 in `ideas/CONSENT_MY_RESOURCES.md` (`my_resources` wrapper with `personal_dynamic_entities`, -`api_collections`, ... as typed lists). **Built 2026-09-04 (working tree) for `personal_dynamic_entities`**; the +`api_collections`, ... as typed lists). **Built 2026-09-04 and committed, for `personal_dynamic_entities`**; the interim entity-Role gate is replaced by the `my_resources` check (`Http4sDynamicEntity.consentCoversPersonalResource`). Client side (OBP-MCP, Opey, OBP-Frontend) built 2026-09-04 too, see the note. ## Phase 3 — explicit-target guards (endpoint 400s) -Doctrine (settled 2026-09-01): implicit self → redirect in provider; explicit `USER_ID` naming a consent user → 400 `InvalidUserId … names a consent user`. Already done: addEntitlement (v2.0/v7), addUserToGroup (v6), createAccount (v2.0/v3.1/v4.0/v5.0/v7), grantUserAccessToViewById (v5.1), account access requests (v6), account applications (v3.1). To sweep: API collections, user attributes, auth contexts, KYC/meeting staff ids, webhooks with explicit ids (createUserCustomerLink was done 2026-09-10, row 2 of Phase 2). `Reject` columns refuse in the provider (`attributionOf` returns Failure); endpoints map that to 400 and may keep an early explicit check for a nicer message, but the floor holds without them. +Doctrine (settled 2026-09-01): implicit self → redirect in provider; explicit `USER_ID` naming a consent user → 400 `InvalidUserId … names a consent user`. Already done: addEntitlement (v2.0/v7), addUserToGroup (v6), createAccount (v2.0/v3.1/v4.0/v5.0/v7), grantUserAccessToViewById (v5.1), account access requests (v6), account applications (v3.1). To sweep: API collections, user attributes, auth contexts, KYC/meeting staff ids (createUserCustomerLink was done 2026-09-10, row 2 of Phase 2). Webhooks were on this list and came off it on 2026-09-16: no webhook endpoint takes a user id as a target, since neither create body carries one, and the only `user_id` any of them accepts is the optional filter on the v3.1.0 `getAccountWebhooks` read, which names nothing durable. There is nothing there for an explicit-target guard to refuse, so the webhook question is entirely a Phase 2 one. `Reject` columns refuse in the provider (`attributionOf` returns Failure); endpoints map that to 400 and may keep an early explicit check for a nicer message, but the floor holds without them. **Tests — ✅ 2026-09-13, `code/api/sweep/ExplicitTargetConsentUserSweepTest.scala`, 7 scenarios green** (shard 8, the catch-all; `code.api.sweep` is not in the shard table). 20 probes over the 19 guards that exist today, driven in-process through `Http4sApp.httpApp` like the other sweeps in that package. Shape: @@ -412,33 +460,33 @@ The 19 guards, by version: v1.4.0 `addCustomer`; v2.0.0 `createAccount`, `create ## Phase 4 — tests -1. **`AgentDelegationTest`** — extend: `onBehalfOfUserIdOf` for original user / consent user / dangling consent (fails closed) / cache hit after consent later bound (BG case) / consent whose user is itself a consent user → Failure; `attributionOf` for each of the three policies. -2. **`UserReferenceAttributionPolicyTest`** — ✅ **2026-09-11**, `obp-api/src/test/scala/code/users/`, 6 scenarios green (shard 8, the catch-all). Iterates `ToSchemify.models`, reflects Mapper fields matching `(?i)userid|createdby|grantedby|holder`, and asserts: every such column is named by a `UserReference` or listed in `notUserIdColumns`; every `UserReference` names a Mapper that is in the schema; every named field exists; a column named by several references has references that differ by *policy* (the deliberate case is `MappedEntitlement.mUserId` — `EntitlementUser` vs `ConsentEntitlementUser`); no column is both given a policy and excluded; and no `notUserIdColumns` entry is inert. +1. **`AgentDelegationTest`** — extend: `resolveOnBehalfOfUserId` for original user / consent user / dangling consent (fails closed) / cache hit after consent later bound (BG case) / consent whose user is itself a consent user → Failure; `attributionOf` for each of the three policies. +2. **`UserReferenceAttributionPolicyTest`** — ✅ **2026-09-11**, `obp-api/src/test/scala/code/users/`, 6 scenarios green (shard 8, the catch-all). Iterates `ToSchemify.models`, reflects Mapper fields matching `(?i)userid|createdby|grantedby|holder`, and asserts: every such column is named by a `UserReference` or listed in `notUserIdColumns`; every `UserReference` names a Mapper that is in the schema; every named field exists; a column named by several references has references that differ by *policy* (the deliberate case is `MappedEntitlement.mUserId` — `Entitlement_UserId` vs `Entitlement_UserId_ConsentScope`); no column is both given a policy and excluded; and no `notUserIdColumns` entry is inert. **Found on first run — the map was not complete:** - - `ApiProductSubscription.CreatedByUserId` and `DynamicGlossaryItem.CreatedByUserId` had no policy at all. Both tables landed after the policy file was written, which is exactly the drift this test exists to catch. Added as `ApiProductSubscriptionCreator` / `DynamicGlossaryItemCreator`, both `UseOnBehalfOfUserId` (consistent with the other `*Creator` references). + - `ApiProductSubscription.CreatedByUserId` and `DynamicGlossaryItem.CreatedByUserId` had no policy at all. Both tables landed after the policy file was written, which is exactly the drift this test exists to catch. Added as `ApiProductSubscription_CreatedByUserId` / `DynamicGlossaryItem_CreatedByUserId`, both `UseOnBehalfOfUserId` (consistent with the other `CreatedByUserId` references). - `PemUsageLastUser` named `code.api.pemusage.PemUsage`, which is **not in `ToSchemify.models`** — so it has no table. It is an unwired stub: `MappedPemUsageProvider`'s body is empty and nothing outside its own package references it. The policy entry was removed; if PemUsage is ever wired up this test will demand it back. **The dead stub itself was left in place** — deleting a feature skeleton is a separate call. - Four `notUserIdColumns` entries (`AccountAccessRequest.CheckerComment`, `DynamicChangeRequest.CheckerComment`, `MappedKycCheck.mStaffName`, `MappedMeeting.mStaffToken`) excluded columns the pattern never catches, i.e. gave no cover while looking like they did. Removed, reasons kept as a comment. **Pattern width was measured, not guessed.** A wider pattern (adding `user|staff|checker|requestor|owner|sender|granted`) surfaces 19 columns, of which 17 are noise (`Username`, `superUser`, `UseRowLevelAccess`, `UserAgreementId`, `userAuthenticationURL`, …) and **zero** are genuine unclaimed user-id columns. The narrow pattern does miss bare `user` / `user_fk` style names, but every such column in the schema is already declared, so there is no live gap. Kept narrow; do not re-litigate without re-measuring. 3. **`OnBehalfOfOwnershipSweepTest`** — ✅ **2026-09-13**, `code/api/sweep/`, 4 scenarios green (shard 8, the catch-all). Its subject is the *redirect* — the implicit-self half of the doctrine, where the caller IS a consent user — so it is the complement of `ExplicitTargetConsentUserSweepTest` (Phase 3), not a superset of it. - **Departure from the plan as written, deliberate.** "Call every `UseOnBehalfOfUserId` create endpoint with the consent JWT; assert no row in any such table references the consent user's id" is red on the day it is written and stays red for months: **5 of the 53 `UseOnBehalfOfUserId` references are wired today** (`TransactionRequest`, `EntitlementUser`, `AccountHolderUser`, `UserCustomerLinkUser`, `DynamicEntityUser`/`DynamicDataUser` — Phase 2 rows 1–3), and all 3 `Reject` references are unwired. A permanently red suite is one people learn to ignore — the same reasoning `AuthSweepTest.expectedAuthDeviation` records for its own two entries. So the shape is a **shrink-only ratchet**: + **Departure from the plan as written, deliberate.** "Call every `UseOnBehalfOfUserId` create endpoint with the consent JWT; assert no row in any such table references the consent user's id" is red on the day it is written and stays red for months: **8 of the 59 `UseOnBehalfOfUserId` references are wired today** (`TransactionRequest_UserId`, `Entitlement_UserId`, `AccountHolders_User`, `UserCustomerLink_UserId`, `DynamicEntity_UserId`/`DynamicData_UserId`, `Bank_CreatedByUserId`, `Counterparty_CreatedByUserId` — Phase 2 rows 1–5), and all 3 `Reject` references are unwired. A permanently red suite is one people learn to ignore — the same reasoning `AuthSweepTest.expectedAuthDeviation` records for its own two entries. So the shape is a **shrink-only ratchet**: - - **Inventory (source scan).** Every `UseOnBehalfOfUserId` / `Reject` reference is either named somewhere in `main` outside the policy file, or listed in `notYetWired` with a reason. Neither → fail (a new table nobody decided about). Listed *and* now used → fail, so wiring one forces the list to shrink. The list is **written out by hand, 56 entries**, not derived from "what main does not reference": a derived list agrees with reality by construction and both assertions would be checking it against a copy of itself — the failure mode `SweepCoverageDriftCheckTest` exists to prevent elsewhere in that package. + - **Inventory (source scan).** Every `UseOnBehalfOfUserId` / `Reject` reference is either named somewhere in `main` outside the policy file, or listed in `notYetWired` with a reason. Neither → fail (a new table nobody decided about). Listed *and* now used → fail, so wiring one forces the list to shrink. The list is **written out by hand, 54 entries**, not derived from "what main does not reference": a derived list agrees with reality by construction and both assertions would be checking it against a copy of itself — the failure mode `SweepCoverageDriftCheckTest` exists to prevent elsewhere in that package. - **Ownership (runtime, the real property).** A Consent is minted for `resourceUser1` over the wire (`POST /my/consents/IMPLICIT` → challenge → `Consent-JWT`), the consent user asks `/users/current` for its own id, then creates an account with `user_id` omitted. The account holder must be the human; and no row in **any** `UseOnBehalfOfUserId` column may reference the consent user, except in the tables the inventory says are unwired. - - **The scan works (runtime, negative control).** The same consent user creates an API collection — `ApiCollectionUser` is unwired — and the scan must *find* that row. Without it, the ownership scenario passes just as happily when the scan reads nothing at all. The scenario also asserts `ApiCollectionUser` is still unwired, so it fails loudly rather than silently rotting once it gets wired. + - **The scan works (runtime, negative control).** The same consent user creates an API collection — `ApiCollection_UserId` is unwired — and the scan must *find* that row. Without it, the ownership scenario passes just as happily when the scan reads nothing at all. The scenario also asserts `ApiCollection_UserId` is still unwired, so it fails loudly rather than silently rotting once it gets wired. **The scan reads SQL, not the Mapper**, because the columns are three different shapes: the `user_id` string, a `MappedLongForeignKey` to `ResourceUser`'s primary key (`MapperAccountHolders.user`), and a list of ids (`SignatoryPanel.UserIds`). `DBUtil.runQuery` stringifies every column, so one comparison — equal to the primary key, or containing the user_id — covers all three. - **Found on first run: the scan needs the shared-column discriminator.** `MappedEntitlement.mUserId` is named by `EntitlementUser` (`UseOnBehalfOfUserId`) *and* `ConsentEntitlementUser` (`KeepUserId`), split by `createdByProcess == consent_user`. Scanning the column as a whole reports every Consent's own materialised scope as a leak — rows that are *meant* to sit on the consent user and are revoked with it. Fixed by a `sharedColumnDiscriminator` map naming the field the provider branches on, plus a fourth scenario asserting every policy-disagreeing column has one. That is the other half of `UserReferenceAttributionPolicyTest`'s "two references on one column must differ by policy": where they differ, the scan has to be told how. + **Found on first run: the scan needs the shared-column discriminator.** `MappedEntitlement.mUserId` is named by `Entitlement_UserId` (`UseOnBehalfOfUserId`) *and* `Entitlement_UserId_ConsentScope` (`UseAuthenticatedUserId`), split by `createdByProcess == consent_user`. Scanning the column as a whole reports every Consent's own materialised scope as a leak — rows that are *meant* to sit on the consent user and are revoked with it. Fixed by a `sharedColumnDiscriminator` map naming the field the provider branches on, plus a fourth scenario asserting every policy-disagreeing column has one. That is the other half of `UserReferenceAttributionPolicyTest`'s "two references on one column must differ by policy": where they differ, the scan has to be told how. 4. Existing `ConsentObpTest` / `ConsentTest` keep passing (35033 now only AnyBank). ## Phase 5 — follow-through 1. Portal page `/developers/opey-permissions`: shrink "Attribution Is Not Yet Universal" to one line once the sweep test is green; use the vocabulary above there too. 2. Memory: write `on-behalf-of-user-id-plan` (none exists yet) pointing at this file, then mark built. -3. Optional later: consent revocation GC for consent-user rows (`KeepUserId`) — still declined for now. +3. Optional later: consent revocation GC for consent-user rows (`UseAuthenticatedUserId`) — still declined for now. ## Decisions (settled 2026-09-02) @@ -465,7 +513,7 @@ The 19 guards, by version: v1.4.0 `addCustomer`; v2.0.0 `createAccount`, `create 10. **Endpoint-level tag: `onBehalfOfMode`, verb-shaped values.** The projection of `AttributionPolicy` onto endpoints is not 1:1 — redirect-vs-guard is a distinction that exists only at endpoint level - (`AccountAccessRequestTarget` is policy `UseOnBehalfOfUserId` but an explicit target the endpoint + (`AccountAccessRequest_TargetUserId` is policy `UseOnBehalfOfUserId` but an explicit target the endpoint refuses), and most endpoints touch no user column at all. So the endpoint enum needs one value the column enum lacks, plus a default. Field name mirrors its `ResourceDoc` sibling `authMode: EndpointAuthMode`; values are verb-shaped rather than reusing the policy words: @@ -501,6 +549,42 @@ The 19 guards, by version: v1.4.0 `addCustomer`; v2.0.0 `createAccount`, `create reads the granting User's Customers; a grant for another Bank does not open this one; a write-only grant does not grant reading; a malformed entry is refused at consent creation). Shard 6 (`code.api.v7_0_0`). +12. **`NotImplementedForConsentUser` — a fourth policy, so an unwired reference answers instead of + stranding a row.** *Simon's idea, 2026-09-19; recorded here, not built, not yet decided.* Today the + doctrine has two answers for a consent user: **redirect** (the provider writes the human, Phase 2) and + **reject** (400 `InvalidUserId`, Phase 3, for a request that explicitly names a consent user). The 54 + references in `OnBehalfOfOwnershipSweepTest.notYetWired` have neither, so they get a third answer nobody + chose: **success**, with the agent's id in the column. That is the worst of the three, because it is + indistinguishable from the wired case at the call site — the caller gets a 201 and a row that will + strand. The proposal is to make "not wired yet" a *declared* state that answers on the wire: + a fourth `AttributionPolicy` value, `NotImplementedForConsentUser`, whose `attributionOf` returns a + `Failure` carrying a new error — `NotImplementedForConsentUser = "OBP-10062: ..."` is the next free code + — mapping to **501**, which is the honest status: not "you may not", but "this server cannot yet do this + correctly for an agent identity." An original user calling the same endpoint is unaffected, since the + policy only fires when the caller is a consent user. + + Why it is worth doing. (a) It is **discoverable before the call**: the error joins the endpoint's + `ResourceDoc` error list, so an agent reading the resource docs — which is how OBP-MCP and Opey plan + their calls — can see which endpoints are agent-safe without trying them. That is the "return the + development process and progress" part: the API itself reports how far this plan has got, per endpoint, + instead of a markdown file nobody outside this repo reads. (b) It makes the Phase 4 ratchet **runtime** + rather than test-only: `notYetWired` and the policy file stop being two lists that can disagree. + (c) It converts a silent future defect into a loud present one — the webhook case (row 6) is precisely + a row that succeeds now and misbehaves months later. + + Why it is not simply switched on for all 54. It is a **behaviour change for working agent flows**: an + agent that creates an API collection or a user attribute today gets a 201, and would get a 501. So the + value is chosen **per reference, deliberately**, exactly like `record-both` was for + `Counterparty_CreatedByUserId` — the question for each row becomes "wire it, or declare it not + implemented?", and the row is not allowed to stay silent. The natural first candidate is the three + webhook references, where the deferral is already written down and the consequence of succeeding is + durable (nothing GCs a webhook on revocation). Open sub-questions, none of them blocking the idea: + whether 501 or 403 reads better to a client library that retries on 5xx; whether the endpoint or the + provider emits it (provider, for the same reason the redirect lives there — but then the error has to + survive the `Box` → HTTP mapping with its status intact); and whether the declaration belongs on the + `UserReference` alone or also on Decision 10's `onBehalfOfMode` endpoint tag, which is the thing a + `ResourceDoc` can actually carry. + ## Risks 1. **Silent redirects hide bugs** → WARN on every delegated attribution + the sweep test; redirects are the net, endpoints stay explicit. diff --git a/ideas/WRITE_AUDIT_TABLE.md b/ideas/WRITE_AUDIT_TABLE.md new file mode 100644 index 0000000000..98d8a06cf5 --- /dev/null +++ b/ideas/WRITE_AUDIT_TABLE.md @@ -0,0 +1,182 @@ +# A write-audit table: who changed what, including deletes + +Written 2026-09-16. Not built. Companion to `ON_BEHALF_OF_USER_ID_PLAN.md`, which puts the *owner* +on the row; this is about putting the *history* somewhere. + +## The gap + +On-row attribution answers "whose is this?" — `MapperAccountHolders.user` says the human holds the +account, so every existing endpoint finds it. What it cannot answer: + +- **Who actually did this?** Only for the tables that record both (`MappedTransactionRequest`, + and now `MappedCounterparty`). Everywhere else the actor is recoverable only by correlating a + timestamp against `MappedMetric`, which is a fuzzy join against a table with its own retention. +- **What did this consent do?** No user-id column can answer the reverse lookup. This is the + question you ask when a consent turns out to have been compromised. +- **What was deleted?** Nothing. On-row columns die with the row. For a counterparty — the control + on where money may be sent — deleting one is as security-relevant as creating it. + +## It does not replace the on-row columns + +Two different questions, and neither substitutes: + +| | answers | how you query it | +|---|---|---| +| on-row `user_id` / `on_behalf_of_user_id` | whose is this, **now** | `WHERE` clause on the table itself | +| write-audit table | what happened, **when**, by whom | scan/replay by table + pk, or by consent | + +Ownership reads must not require replaying a log, and audit tables get archived — there is already a +`MetricsArchiveScheduler` doing exactly that to metrics. Keep both. + +## Shape + +One append-only table: + +``` +obp_write_audit + id bigserial + table_name text -- from TG_TABLE_NAME + row_pk text -- the changed row's primary key + operation text -- INSERT | UPDATE | DELETE + authenticated_user_id text -- who made the call + on_behalf_of_user_id text -- who they were acting for + consent_reference_id text -- under which grant (null when no consent) + changed_at timestamptz + row_data jsonb -- optional; see "volume" below +``` + +`consent_reference_id` is the column that buys the reverse lookup, and it is the same key +`MappedMetric` already carries, so the two join cleanly. + +## Approach A — Postgres trigger (Simon's preference; yes, it can be generic) + +It genuinely can. Three Postgres features make one function serve every table: + +- `TG_TABLE_NAME` / `TG_OP` — the trigger knows which table and which operation fired it +- `to_jsonb(NEW)` / `to_jsonb(OLD)` — captures the whole row **without knowing its columns** +- `current_setting('obp.x', true)` — reads a transaction-local variable; the `true` means "return + NULL if unset" instead of raising + +```sql +CREATE OR REPLACE FUNCTION obp_write_audit_fn() RETURNS trigger AS $$ +BEGIN + INSERT INTO obp_write_audit( + table_name, row_pk, operation, + authenticated_user_id, on_behalf_of_user_id, consent_reference_id, + changed_at, row_data) + VALUES ( + TG_TABLE_NAME, + COALESCE(to_jsonb(NEW)->>'id', to_jsonb(OLD)->>'id'), + TG_OP, + current_setting('obp.authenticated_user_id', true), + current_setting('obp.on_behalf_of_user_id', true), + current_setting('obp.consent_reference_id', true), + now(), + CASE TG_OP WHEN 'DELETE' THEN to_jsonb(OLD) ELSE to_jsonb(NEW) END); + RETURN NULL; +END; $$ LANGUAGE plpgsql; +``` + +Attaching it is a loop, not 200 hand-written statements: + +```sql +DO $$ DECLARE t text; BEGIN + FOR t IN SELECT table_name FROM information_schema.tables + WHERE table_schema = current_schema() AND table_type = 'BASE TABLE' + AND table_name <> 'obp_write_audit' + LOOP + EXECUTE format('DROP TRIGGER IF EXISTS obp_audit_t ON %I', t); + EXECUTE format('CREATE TRIGGER obp_audit_t AFTER INSERT OR UPDATE OR DELETE ON %I + FOR EACH ROW EXECUTE FUNCTION obp_write_audit_fn()', t); + END LOOP; +END $$; +``` + +The application sets the variables once per request, at the point `withRequestTransaction` already +opens the transaction: + +```sql +SELECT set_config('obp.authenticated_user_id', ?, true); -- true = TRANSACTION-local +``` + +`is_local := true` matters: HikariCP hands the same physical connection to the next request, and a +session-scoped variable would leak the previous caller's identity into it. Transaction-local is +discarded at commit or rollback. + +### What this buys + +Total coverage. It catches writes that bypass the providers entirely — connector code, migrations, +schedulers, and anyone typing SQL into a console. No application path can forget it, which is the +failure mode of every approach that relies on a developer remembering. + +### What it costs + +1. **Postgres only.** `DBUtil.isSqlServer` exists, so this is not a Postgres-only product. Either + audit is a Postgres-only feature, or a SQL Server equivalent gets written and maintained, or the + app-level fallback covers other databases. This is the main argument against, and it needs an + answer before starting. +2. **Re-attaching after Schemifier.** New tables arrive without the trigger, and if Schemifier ever + recreates a table the trigger goes with it. The attach loop must run at boot, after Schemifier, + and be idempotent — the `DROP TRIGGER IF EXISTS` above is what makes re-running safe. +3. **Recursion.** The audit table must be excluded, or every audit insert audits itself. +4. **Writes with no user** — Boot, the sandbox import, schedulers — log NULLs. That is correct and + arguably useful: it distinguishes "system did this" from "someone did this". + +## Approach B — Mapper lifecycle hooks + +`beforeSave` / `afterDelete`, already used in three places (`MappedTransaction`, `UserAgreement`, +`ViewDefinition`). Database-agnostic, and the caller is reachable through the request scope that +`withRequestTransaction` establishes. + +Misses anything that does not go through Lift Mapper: Doobie queries (`DoobieConsentQueries`, +`DoobieMetricsQueries`), raw `DBUtil.runQuery`, migrations. That is a real and growing set. + +## Approach C — explicit calls in providers + +Rejected. Same failure mode the `UserReference` ratchet exists to police: it depends on people +remembering, and the gap is invisible until someone looks. + +## Synchronous, not async + +The instinct is to make this async to avoid contention. I would not, at least not first. + +**In-transaction**, the audit row commits or rolls back with the change it describes. The log +cannot claim something that did not happen, or miss something that did. That atomicity is the +property an auditor actually asks about, and a trigger gives it for free. + +**Async** (the `MetricBatchWriter` pattern) is right for metrics, because metrics are observability +and losing a few rows is acceptable. An audit trail is not that class of thing: it can lose entries +on crash, and it can record events for transactions that later rolled back. + +The contention worry is also mostly overstated for append-only inserts — Postgres handles high-rate +appends well. The real costs are storage growth and vacuum, and the standard answers are monthly +partitioning plus a retention policy, not asynchrony. Measure before trading atomicity away. + +### Volume + +`to_jsonb(NEW)` stores the entire row, which is the expensive part. Options, in increasing cost: +pk + operation only; pk + operation + changed columns; full row. Starting with the first and +turning on `row_data` for a named set of security-relevant tables is probably the right default. + +## Scope + +"Every write" is ~200 mappers. If the trigger approach is taken the loop covers them all at once, +which is an argument for it — but the *retention* question then applies to all of them too. + +If starting narrower, the tables that authorise money movement or access are the ones where deletes +matter most: counterparties, transaction requests, account access, consents, entitlements, account +holders. + +## Sequencing constraint + +`MigrationOfConsentReferenceIdUuid` is in flight and is delete-and-replace with no backfill — the +counter becomes a UUID and existing values are not converted. Anything storing a consent reference +before that lands ends up holding dangling references. Wait for it. + +## Open questions + +- Postgres-only audit, or a SQL Server equivalent, or app-level fallback for other databases? +- Retention: how long, and does the archive follow the `MetricsArchiveScheduler` pattern? +- Is `row_data` on by default, or per-table? +- Does the audit table need to be tamper-evident (hash chain), or is append-only plus database + permissions enough for the compliance story? diff --git a/obp-api/src/main/scala/bootstrap/liftweb/Boot.scala b/obp-api/src/main/scala/bootstrap/liftweb/Boot.scala index 0f9fcf81ec..94e6049506 100644 --- a/obp-api/src/main/scala/bootstrap/liftweb/Boot.scala +++ b/obp-api/src/main/scala/bootstrap/liftweb/Boot.scala @@ -397,6 +397,8 @@ class Boot extends MdcLoggable { warnAboutEmailDeliveryConfiguration() + warnAboutRemovedProps() + OAuth2Login.logConfigWarnings() createBootstrapOidcOperatorUser() @@ -775,6 +777,38 @@ class Boot extends MdcLoggable { } } + /** + * Warn at startup about props that this instance still sets but that OBP no longer reads. + * + * A prop that has been removed from the code is simply ignored, so an operator who set it + * deliberately gets a silent change of behaviour on upgrade and no way to find out. Naming it + * here turns that into one line in the boot log. Entries are (prop name, what to know now). + */ + private def warnAboutRemovedProps(): Unit = { + val removedProps = List( + "experimental_become_user_that_created_consent" -> + ("A Consent-JWT now always authenticates as the consent user (the agent identity the " + + "Consent minted), never as the human who created the Consent. Setting this to true used " + + "to log that human on instead. The human is still recorded on what the call creates - " + + "see on_behalf_of_user_id - but the caller's own permissions are the Consent's.") + ) + val stillSet = removedProps.filter { case (name, _) => + APIUtil.getPropsValue(name).exists(_.trim.nonEmpty) + } + if (stillSet.nonEmpty) { + logger.warn("========================================================================") + logger.warn("WARNING: this instance sets props that OBP no longer reads:") + stillSet.foreach { case (name, explanation) => + logger.warn("") + logger.warn(s" $name") + logger.warn(s" $explanation") + } + logger.warn("") + logger.warn("Remove them from your props file; they have no effect.") + logger.warn("========================================================================") + } + } + /** * Warn at startup about email-delivery configuration that would silently break * signup-validation and password-reset flows. Both flows embed a link built from diff --git a/obp-api/src/main/scala/code/accountholders/AccountHolders.scala b/obp-api/src/main/scala/code/accountholders/AccountHolders.scala index 5ca6779e70..956c665506 100644 --- a/obp-api/src/main/scala/code/accountholders/AccountHolders.scala +++ b/obp-api/src/main/scala/code/accountholders/AccountHolders.scala @@ -55,7 +55,7 @@ trait AccountHolders { */ def getAccountsHeldByUser(user: User, source: Option[String] = None): Set[BankIdAccountId] /** Links the account to its holder. The holder is the on-behalf-of user of `user` - * (UserReference.AccountHolderUser): a consent user never holds an account, the user its + * (UserReference.AccountHolders_User): a consent user never holds an account, the user its * consent names does. Same user for an original user. */ def getOrCreateAccountHolder(user: User, bankAccountUID :BankIdAccountId, source: Option[String] = None): Box[MapperAccountHolders] //There is no AccountHolder trait, database structure different with view def deleteAccountHolder(user: User, bankAccountUID :BankIdAccountId): Box[Boolean] diff --git a/obp-api/src/main/scala/code/accountholders/MapperAccountHolders.scala b/obp-api/src/main/scala/code/accountholders/MapperAccountHolders.scala index fe2a4afb17..805331842d 100644 --- a/obp-api/src/main/scala/code/accountholders/MapperAccountHolders.scala +++ b/obp-api/src/main/scala/code/accountholders/MapperAccountHolders.scala @@ -64,7 +64,7 @@ object MapperAccountHolders extends MapperAccountHolders with AccountHolders wit //Note, this method, will not check the existing of bankAccount, any value of BankIdAccountId //Can create the MapperAccountHolders. // - // On-behalf-of guard (attribution policy UserReference.AccountHolderUser): an account is + // On-behalf-of guard (attribution policy UserReference.AccountHolders_User): an account is // held by the on-behalf-of user. When `user` is a consent user the holder row is written // for the user the consent names, so the account does not strand when the consent dies. // For an original user this is a no-op. The resolver logs every redirect. @@ -77,7 +77,7 @@ object MapperAccountHolders extends MapperAccountHolders with AccountHolders wit /** The user the holder row is written for: `user` itself, or its on-behalf-of user. */ private def accountHolderUserFor(user: User): Box[User] = - Users.users.vend.attributedUserId(user.userId, code.users.UserReference.AccountHolderUser).flatMap { holderUserId => + Users.users.vend.attributedUserId(user.userId, code.users.UserReference.AccountHolders_User).flatMap { holderUserId => if (holderUserId == user.userId) Full(user) else Users.users.vend.getUserByUserId(holderUserId) ?~ s"getOrCreateAccountHolder: on-behalf-of user $holderUserId of ${user.userId} not found" } diff --git a/obp-api/src/main/scala/code/api/dynamic/entity/Http4sDynamicEntity.scala b/obp-api/src/main/scala/code/api/dynamic/entity/Http4sDynamicEntity.scala index a159903648..460847187b 100644 --- a/obp-api/src/main/scala/code/api/dynamic/entity/Http4sDynamicEntity.scala +++ b/obp-api/src/main/scala/code/api/dynamic/entity/Http4sDynamicEntity.scala @@ -250,7 +250,7 @@ object Http4sDynamicEntity extends MdcLoggable { /** * Personal ("my") endpoints and consent users. A consent user's rows resolve to the User its - * consent names (UserReference.DynamicDataUser in the provider), so without a scope check any + * consent names (UserReference.DynamicData_UserId in the provider), so without a scope check any * consent, however narrow, would reach that User's personal rows. The scope is the consent's * `my_resources` claim: the consent user passes only if the claim lists this entity with the * needed action. The entity role is then required exactly as for anyone else (only when the @@ -267,7 +267,7 @@ object Http4sDynamicEntity extends MdcLoggable { /** The user personal rows are read for: the caller, or the user its consent names. Same rule as the provider. */ private def personalRowOwner(userIdOpt: Option[String], isPersonalEntity: Boolean): Option[String] = - if (isPersonalEntity) userIdOpt.map(id => code.users.Users.users.vend.attributedUserId(id, code.users.UserReference.DynamicDataUser).openOr(id)) + if (isPersonalEntity) userIdOpt.map(id => code.users.Users.users.vend.attributedUserId(id, code.users.UserReference.DynamicData_UserId).openOr(id)) else userIdOpt /** The entity's role, checked per the entity's auth mode (entitlements, scopes, either or both). */ diff --git a/obp-api/src/main/scala/code/api/util/ApiRole.scala b/obp-api/src/main/scala/code/api/util/ApiRole.scala index 90aae056fb..407672630d 100644 --- a/obp-api/src/main/scala/code/api/util/ApiRole.scala +++ b/obp-api/src/main/scala/code/api/util/ApiRole.scala @@ -681,6 +681,12 @@ object ApiRole extends MdcLoggable{ case class CanCreateAccountNotificationWebhookAtOneBank(requiresBankId: Boolean = true) extends ApiRole lazy val canCreateAccountNotificationWebhookAtOneBank = CanCreateAccountNotificationWebhookAtOneBank() + case class CanDeleteSystemAccountNotificationWebhook(requiresBankId: Boolean = false) extends ApiRole + lazy val canDeleteSystemAccountNotificationWebhook = CanDeleteSystemAccountNotificationWebhook() + + case class CanDeleteAccountNotificationWebhookAtOneBank(requiresBankId: Boolean = true) extends ApiRole + lazy val canDeleteAccountNotificationWebhookAtOneBank = CanDeleteAccountNotificationWebhookAtOneBank() + case class CanUpdateWebhook(requiresBankId: Boolean = true) extends ApiRole lazy val canUpdateWebhook = CanUpdateWebhook() diff --git a/obp-api/src/main/scala/code/api/util/ApiSession.scala b/obp-api/src/main/scala/code/api/util/ApiSession.scala index 8b580ec7d1..b7a559ded3 100644 --- a/obp-api/src/main/scala/code/api/util/ApiSession.scala +++ b/obp-api/src/main/scala/code/api/util/ApiSession.scala @@ -276,9 +276,9 @@ case class CallContext( val delegatedUserId = consentCreator.or(consenter).map(_.userId).filter(_.nonEmpty) delegatedUserId.openOr { val authenticatedUserId = user.map(_.userId).openOr("") - // The resolver (Users.onBehalfOfUserIdOf) owns the consent chain; a Failure there (invariant + // The resolver (Users.resolveOnBehalfOfUserId) owns the consent chain; a Failure there (invariant // broken) is already logged and, at this String-typed level, can only fall back to the caller. - code.users.Users.users.vend.onBehalfOfUserIdOf(authenticatedUserId).openOr(authenticatedUserId) + code.users.Users.users.vend.resolveOnBehalfOfUserId(authenticatedUserId).openOr(authenticatedUserId) } } def userPrimaryKey: UserPrimaryKey = user.map(_.userPrimaryKey).openOrThrowException(AuthenticatedUserIsRequired) diff --git a/obp-api/src/main/scala/code/api/util/ConsentUtil.scala b/obp-api/src/main/scala/code/api/util/ConsentUtil.scala index 54b9fa4c44..5c0e5919a3 100644 --- a/obp-api/src/main/scala/code/api/util/ConsentUtil.scala +++ b/obp-api/src/main/scala/code/api/util/ConsentUtil.scala @@ -656,39 +656,40 @@ object Consent extends MdcLoggable { case Full(mc) => ccWithOnBehalf.copy(consentReferenceId = Some(mc.consentReferenceId)) case _ => ccWithOnBehalf } - if (cc.consentCreator.nonEmpty && - APIUtil.getPropsAsBoolValue(nameOfProperty = "experimental_become_user_that_created_consent", defaultValue = false)) { - logger.warn("WARNING: experimental_become_user_that_created_consent is DEPRECATED and will be removed soon. Please unset this property.") - logger.info("experimental_become_user_that_created_consent = true") - logger.info(s"${cc.consentCreator.map(_.userId).getOrElse("")} is logged on instead of Consent user") - Future(cc.consentCreator, Some(cc)) // Just propagate the consent creator back - } else { - logger.info("experimental_become_user_that_created_consent = false") - logger.info(s"Getting Consent user (consent.sub: ${consent.sub}, consent.iss: ${consent.iss})") - // 1. Get or Create a User - getOrCreateUser(consent.sub, consent.iss, Some(consent.jti), None, None) map { - case (Full(user), newUser) => - // 2. Assign entitlements to the User - addEntitlements(user, consent) match { - case Full(user) => - // 3. Copy Auth Context to the User - copyAuthContextOfConsentToUser(consent.jti, user.userId, newUser) match { - case Full(_) => - // 4. Assign views to the User - (grantAccessToViews(user, consent), Some(cc)) - case failure@Failure(_, _, _) => // Handled errors - (failure, Some(cc)) - case _ => - (Failure(ErrorMessages.UnknownError), Some(cc)) - } - case failure@Failure(msg, exp, chain) => // Handled errors - (Failure(msg), Some(cc)) - case _ => - (Failure(CannotAddEntitlement + consentAsJwt), Some(cc)) - } - case _ => - (Failure(CannotGetOrCreateUser + consentAsJwt), Some(cc)) - } + // A Consent-JWT always authenticates as the consent user: the agent identity the Consent + // minted, never the human who created the Consent. There used to be a props toggle here, + // experimental_become_user_that_created_consent, which logged the creator on instead. It has + // been removed and should not come back. Becoming the human dissolves the isolation the + // Consent model rests on: the caller would get the human's entitlements and views instead of + // the narrower set the Consent grants, every metric row would name the human, and the + // on-behalf-of work (ON_BEHALF_OF_USER_ID_PLAN.md) would have nothing to attribute, because + // the caller and the on-behalf-of user would be one identity. The human stays reachable + // where it is genuinely needed: cc.consentCreator holds them, CallContext.onBehalfOfUserId + // resolves them, and the providers write durable rows for them. + logger.info(s"Getting Consent user (consent.sub: ${consent.sub}, consent.iss: ${consent.iss})") + // 1. Get or Create a User + getOrCreateUser(consent.sub, consent.iss, Some(consent.jti), None, None) map { + case (Full(user), newUser) => + // 2. Assign entitlements to the User + addEntitlements(user, consent) match { + case Full(user) => + // 3. Copy Auth Context to the User + copyAuthContextOfConsentToUser(consent.jti, user.userId, newUser) match { + case Full(_) => + // 4. Assign views to the User + (grantAccessToViews(user, consent), Some(cc)) + case failure@Failure(_, _, _) => // Handled errors + (failure, Some(cc)) + case _ => + (Failure(ErrorMessages.UnknownError), Some(cc)) + } + case failure@Failure(msg, exp, chain) => // Handled errors + (Failure(msg), Some(cc)) + case _ => + (Failure(CannotAddEntitlement + consentAsJwt), Some(cc)) + } + case _ => + (Failure(CannotGetOrCreateUser + consentAsJwt), Some(cc)) } } 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 e12bad0742..f4531dfe74 100644 --- a/obp-api/src/main/scala/code/api/util/ErrorMessages.scala +++ b/obp-api/src/main/scala/code/api/util/ErrorMessages.scala @@ -109,6 +109,7 @@ object ErrorMessages { val RowLevelAccessNotEnabled = "OBP-09021: The row-access endpoints are only available for dynamic entities created with use_row_level_access = true." val DynamicEntityJoinRequiresProjection = "OBP-09022: obp_exists / obp_not_exists join queries require the SQL projection backend (dynamic_entity.indexing.backend=auto on a supported database). This deployment serves Dynamic Entity reads in-memory, where joins are not supported." val DynamicEntityUpdateNotSchemaCompatible = "OBP-09023: Operation is not allowed, because this DynamicEntity already has data. The definition of a populated entity can only be changed in schema-compatible ways: the entity name, the set of property names and each property's type must stay the same, and no property may be added to 'required'. Changing indexed, index, example, description, minLength, maxLength and the read/write role settings is allowed. Delete all the data before making a structural change." + val DynamicEntityRecordIdTooLong = "OBP-09024: The id of this DynamicEntity record is too long. A record id is stored in a column of 255 characters. Please supply a shorter id, or leave the id field out of the request body and one will be generated." // General messages (OBP-10XXX) @@ -467,6 +468,8 @@ object ErrorMessages { val CreateCustomerLinkError = "OBP-30148: Could not create the Customer Link." val UpdateCustomerLinkError = "OBP-30149: Could not update the Customer Link." val InvestigationReportNotAvailable = "OBP-30150: Investigation Report is only available in mapped mode (connector=mapped)." + val NotificationWebhookNotFound = "OBP-30151: Account Notification Webhook not found. Please specify a valid value for WEBHOOK_ID." + val DeleteWebhookError = "OBP-30152: Could not delete the Webhook." val CreateWebhookError = "OBP-30047: Cannot create Webhook" val GetWebhooksError = "OBP-30048: Cannot get Webhooks" @@ -999,6 +1002,10 @@ object ErrorMessages { // the published side. Both call sites name the constant rather than the literal, so nothing but // the number moves. val PaymentNotInitiatedByCaller = "OBP-40062: The addressed payment was not initiated by you. " + val ChallengeNotAddressedToCaller = "OBP-40063: This Strong Customer Authentication challenge is addressed to another user and cannot be answered by you. " + + "A payment started on somebody else's behalf is authorised by that person, not by the caller that started it." + val PaymentChallengeHasNoOnBehalfOfUser = "OBP-40064: This payment needs Strong Customer Authentication, but the user it is being made for could not be determined, " + + "so there is nobody who can be asked to authorise it. A consent that names the user it acts for is required before a payment of this size can be started." // Exceptions (OBP-50XXX) val UnknownError = "OBP-50000: Unknown Error." val FutureTimeoutException = "OBP-50001: Future Timeout Exception." diff --git a/obp-api/src/main/scala/code/api/util/NewStyle.scala b/obp-api/src/main/scala/code/api/util/NewStyle.scala index 6808f8d2ee..390a57bb10 100644 --- a/obp-api/src/main/scala/code/api/util/NewStyle.scala +++ b/obp-api/src/main/scala/code/api/util/NewStyle.scala @@ -3760,6 +3760,26 @@ object NewStyle extends MdcLoggable{ } } + // The id of a Dynamic Entity record is kept in a database column of a fixed width + // (code.DynamicData.DynamicData.DynamicDataId). A caller is allowed to supply that id instead of + // letting one be generated, which is how an entity is given a natural key such as a country code, + // so the value can be any length at all. Check it here and answer with a message that says what is + // wrong: without this check an over-long id only fails once it reaches the database driver, and the + // caller is told nothing more useful than "value too long for type character varying". + val maximumRecordIdLength = code.DynamicData.DynamicData.DynamicDataId.maxLen + val overLongRecordId: Option[String] = requestBodyDynamicInstance.flatMap { body => + body \ DynamicEntityHelper.createEntityId(entityName) match { + case JString(recordId) if recordId.length > maximumRecordIdLength => Some(recordId) + case _ => None + } + } + if (overLongRecordId.isDefined) { + return Helper.booleanToFuture( + s"$DynamicEntityRecordIdTooLong The maximum length is $maximumRecordIdLength characters, the given id is ${overLongRecordId.get.length} characters long.", + cc = callContext)(false) + .map(it => (it.map(_.asInstanceOf[JValue]), callContext)) + } + requestBodyDynamicInstance match { // @(variable-binding pattern), we can use the empty variable // If there is not instance in requestBody, we just call the `dynamicEntityProcess` directly. 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 65c5640ebd..abaa974c2a 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 @@ -189,6 +189,7 @@ object Migration extends MdcLoggable { dropFastFirehoseAccountsViews(startedBeforeSchemifier) alterDynamicResourceDocBodyFieldsLength() alterDynamicResourceDocTextFieldsLength() + alterDynamicDataIdLength() } /** @@ -764,6 +765,13 @@ object Migration extends MdcLoggable { } } + private def alterDynamicDataIdLength(): Boolean = { + val name = nameOf(alterDynamicDataIdLength) + runOnce(name) { + MigrationOfDynamicDataIdFieldLength.alterColumnDynamicDataIdLength(name) + } + } + private def alterMetricColumnConsumerIdLength(): Boolean = { val name = nameOf(alterMetricColumnConsumerIdLength) runOnce(name) { diff --git a/obp-api/src/main/scala/code/api/util/migration/MigrationOfDynamicDataIdFieldLength.scala b/obp-api/src/main/scala/code/api/util/migration/MigrationOfDynamicDataIdFieldLength.scala new file mode 100644 index 0000000000..a5a4ffe017 --- /dev/null +++ b/obp-api/src/main/scala/code/api/util/migration/MigrationOfDynamicDataIdFieldLength.scala @@ -0,0 +1,96 @@ +/** +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.DynamicData.DynamicData +import code.api.util.APIUtil +import code.api.util.migration.Migration.{DbFunction, saveLog} +import net.liftweb.common.Full +import net.liftweb.mapper.Schemifier + +/** + * This migration widens the column that holds the identifier of a single Dynamic Entity record. + * + * The column started life as a `MappedUUID`, which is 36 characters wide because that is the length + * of a UUID. A caller may however supply the identifier itself in the request body rather than let + * one be generated, which is how a Dynamic Entity is given a natural key such as a country code or + * the name of a scheme. Such a value is not a UUID and is often longer than 36 characters, and it + * used to reach the database unchanged and fail there with + * "value too long for type character varying(36)", surfacing to the caller as an opaque server error. + * + * The new width is 255 characters, which is what the two other columns holding this same identifier + * already use: the DynamicDataId column of the row level access list in DynamicDataAccess, and the + * `data_id` column of the SQL projection tables. Widening a column never invalidates existing rows, + * so there is nothing to back fill and nothing to undo. Lift's Schemifier creates columns but never + * widens one that already exists, which is why an existing database needs this migration at all. + */ +object MigrationOfDynamicDataIdFieldLength { + + def alterColumnDynamicDataIdLength(name: String): Boolean = { + DbFunction.tableExists(DynamicData) match { + case true => + val startDate = System.currentTimeMillis() + val commitId: String = APIUtil.gitCommit + var isSuccessful = false + + val executedSql = + DbFunction.maybeWrite(true, Schemifier.infoF _) { + APIUtil.getPropsValue("db.driver") match { + case Full(dbDriver) if dbDriver.contains("com.microsoft.sqlserver.jdbc.SQLServerDriver") => + () => + """ + |ALTER TABLE dynamicdata ALTER COLUMN dynamicdataid varchar(255); + |""".stripMargin + case _ => + () => + """ + |ALTER TABLE dynamicdata ALTER COLUMN dynamicdataid TYPE character varying(255); + |""".stripMargin + } + } + + val endDate = System.currentTimeMillis() + val comment: String = + s"""Executed SQL: + |$executedSql + |""".stripMargin + isSuccessful = true + saveLog(name, commitId, isSuccessful, startDate, endDate, comment) + isSuccessful + + case false => + val startDate = System.currentTimeMillis() + val commitId: String = APIUtil.gitCommit + val isSuccessful = false + val endDate = System.currentTimeMillis() + val comment: String = + s"""${DynamicData._dbTableNameLC} table does not exist""".stripMargin + saveLog(name, commitId, isSuccessful, startDate, endDate, comment) + isSuccessful + } + } +} 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 219a262965..c462bad556 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 @@ -235,7 +235,7 @@ object Http4s210 { val commonTxReqErrors = List(AuthenticatedUserIsRequired, InvalidBankIdFormat, InvalidAccountIdFormat, InvalidJsonFormat, BankNotFound, AccountNotFound, InsufficientAuthorisationToCreateTransactionRequest, InvalidTransactionRequestType, InvalidNumber, NotPositiveAmount, InvalidTransactionRequestCurrency, - TransactionDisabled, UnknownError) + PaymentChallengeHasNoOnBehalfOfUser, TransactionDisabled, UnknownError) resourceDocs += ResourceDoc( implementedInApiVersion, nameOf(createTransactionRequest) + "SandboxTan", "POST", @@ -460,7 +460,7 @@ object Http4s210 { AuthenticatedUserIsRequired, InvalidBankIdFormat, InvalidAccountIdFormat, InvalidJsonFormat, BankNotFound, UserNoPermissionAccessView, TransactionRequestStatusNotInitiated, TransactionRequestTypeHasChanged, InvalidTransactionRequestChallengeId, - AllowedAttemptsUsedUp, TransactionDisabled, UnknownError) + AllowedAttemptsUsedUp, ChallengeNotAddressedToCaller, TransactionDisabled, UnknownError) private val answerChallengeTags = List(apiTagTransactionRequest, apiTagPSD2PIS, apiTagPsd2) diff --git a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala index 8aa9ba3fde..3dbb097707 100644 --- a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala +++ b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala @@ -3194,6 +3194,7 @@ object Http4s400 { InvalidNumber, NotPositiveAmount, InvalidTransactionRequestCurrency, + PaymentChallengeHasNoOnBehalfOfUser, TransactionDisabled, UnknownError ), @@ -3235,6 +3236,7 @@ object Http4s400 { InvalidNumber, NotPositiveAmount, InvalidTransactionRequestCurrency, + PaymentChallengeHasNoOnBehalfOfUser, TransactionDisabled, UnknownError ), @@ -3269,6 +3271,7 @@ object Http4s400 { InvalidNumber, NotPositiveAmount, InvalidTransactionRequestCurrency, + PaymentChallengeHasNoOnBehalfOfUser, TransactionDisabled, UnknownError ), @@ -3310,6 +3313,7 @@ object Http4s400 { InvalidNumber, NotPositiveAmount, InvalidTransactionRequestCurrency, + PaymentChallengeHasNoOnBehalfOfUser, TransactionDisabled, UnknownError ), @@ -3351,6 +3355,7 @@ object Http4s400 { InvalidNumber, NotPositiveAmount, InvalidTransactionRequestCurrency, + PaymentChallengeHasNoOnBehalfOfUser, TransactionDisabled, UnknownError ), @@ -3379,6 +3384,7 @@ object Http4s400 { InvalidNumber, NotPositiveAmount, InvalidTransactionRequestCurrency, + PaymentChallengeHasNoOnBehalfOfUser, TransactionDisabled, UnknownError ), @@ -3413,6 +3419,7 @@ object Http4s400 { InvalidNumber, NotPositiveAmount, InvalidTransactionRequestCurrency, + PaymentChallengeHasNoOnBehalfOfUser, TransactionDisabled, UnknownError ), @@ -3454,6 +3461,7 @@ object Http4s400 { InvalidNumber, NotPositiveAmount, InvalidTransactionRequestCurrency, + PaymentChallengeHasNoOnBehalfOfUser, TransactionDisabled, UnknownError ), @@ -3517,6 +3525,7 @@ object Http4s400 { InvalidNumber, NotPositiveAmount, InvalidTransactionRequestCurrency, + PaymentChallengeHasNoOnBehalfOfUser, TransactionDisabled, UnknownError ), @@ -3623,6 +3632,15 @@ object Http4s400 { val isOwnChallenge = challenges.find(_.challengeId == challengeAnswerJson.id) .exists(_.expectedUserId == user.userId) for { + // Where the challenge is somebody else's, the branch below drops the user check, so that + // an account with several required answers can be worked through. That latitude is for + // people who share an account. It is not extended to an agent: a payment started on a + // person's behalf is authorised by that person, otherwise the agent would only have to + // get the code read out to it, which is the relay Strong Customer Authentication exists + // to prevent. See ON_BEHALF_OF_USER_ID_PLAN.md, Decision 9. + _ <- code.util.Helper.booleanToFuture(ChallengeNotAddressedToCaller, failCode = 403, cc = Some(cc)) { + isOwnChallenge || user.isOriginalUser + } (isValidated, _) <- if (isOwnChallenge) NewStyle.function.validateChallengeAnswer( challengeAnswerJson.id, challengeAnswerJson.answer, SuppliedAnswerType.PLAIN_TEXT_VALUE, Some(cc)) @@ -3749,6 +3767,7 @@ object Http4s400 { TransactionRequestStatusNotInitiated, TransactionRequestTypeHasChanged, AllowedAttemptsUsedUp, + ChallengeNotAddressedToCaller, TransactionDisabled, UnknownError ), 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 bedb50f089..bb8452b27a 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 @@ -35,7 +35,7 @@ import code.api.Constant._ import code.api.ResourceDocs1_4_0.SwaggerDefinitionsJSON._ import code.api.util.APIUtil.{EmptyBody, _} import code.api.util.{APIUtil, ApiRole, CallContext, CustomJsonFormats, Glossary, NewStyle} -import code.api.util.ApiRole.{canAttachOpenCorridorPromise, canConfigureAmqpBankBroker, canGetMessageOutbox, canRetryMessageOutbox, canSettleOpenCorridor, canCreateAccount, canCreateEntitlementAtAnyBank, canCreateEntitlementAtOneBank, canCreateMetricsArchiveRun, canCreateGlossaryItem, canCreateOrganisation, canCreateRoutingScheme, canCreateTestEmail, canCreateUtilityVendResult, canDeleteEntitlementAtAnyBank, canDeleteGlossaryItem, canDeleteOrganisation, canDeleteRoutingScheme, canDeleteSchedulerJobLock, canGetAccountAccessTrace, canGetAnyOrganisation, canGetAnyUser, canGetCacheConfig, canGetCacheInfo, canGetCacheNamespaces, canGetConfig, canGetConnectorHealth, canGetCustomersAtOneBank, canGetDatabasePoolInfo, canGetMetricsDiagnostics, canGetMigrations, canGetSchedulerJobLocks, canReadMetrics, canUpdateBankSupportedRoutingScheme, canUpdateGlossaryItem, canUpdateOrganisation, canUpdateRoutingScheme, canUpdateSystemView} +import code.api.util.ApiRole.{canAttachOpenCorridorPromise, canConfigureAmqpBankBroker, canGetMessageOutbox, canRetryMessageOutbox, canSettleOpenCorridor, canCreateAccount, canCreateEntitlementAtAnyBank, canCreateEntitlementAtOneBank, canCreateMetricsArchiveRun, canCreateGlossaryItem, canCreateOrganisation, canCreateRoutingScheme, canCreateTestEmail, canCreateUtilityVendResult, canDeleteAccountNotificationWebhookAtOneBank, canDeleteEntitlementAtAnyBank, canDeleteGlossaryItem, canDeleteOrganisation, canDeleteRoutingScheme, canDeleteSchedulerJobLock, canDeleteSystemAccountNotificationWebhook, canGetAccountAccessTrace, canGetAnyOrganisation, canGetAnyUser, canGetCacheConfig, canGetCacheInfo, canGetCacheNamespaces, canGetConfig, canGetConnectorHealth, canGetCustomersAtOneBank, canGetDatabasePoolInfo, canGetMetricsDiagnostics, canGetMigrations, canGetSchedulerJobLocks, canReadMetrics, canUpdateBankSupportedRoutingScheme, canUpdateGlossaryItem, canUpdateOrganisation, canUpdateRoutingScheme, canUpdateSystemView} import code.api.util.CommonsEmailWrapper import code.model.dataAccess.{AuthUser, BankAccountCreation, MappedBank, ResourceUser} import code.consent.Consents @@ -3400,6 +3400,87 @@ object Http4s700 { http4sPartialFunction = Some(deleteRoutingScheme) ) + // ─── Account Notification Webhooks: delete ─────────────────────────────── + // + // The two create endpoints arrived in v4.0.0 with no way to remove what they made, so a + // notification webhook was permanent for everyone, by every route: the provider's delete + // methods existed but nothing called them. These are that missing half. + + val deleteSystemAccountNotificationWebhook: HttpRoutes[IO] = HttpRoutes.of[IO] { + case req @ DELETE -> `prefixPath` / "web-hooks" / "account" / "notifications" / "on-create-transaction" / webhookId => + EndpointHelpers.withUserDelete(req) { (_, cc) => + val provider = code.webhook.SystemAccountNotificationWebhookTrait.systemAccountNotificationWebhook.vend + for { + _ <- provider.getSystemAccountNotificationWebhookByIdFuture(webhookId) + .map(unboxFullOrFail(_, Some(cc), NotificationWebhookNotFound, 404)) + _ <- provider.deleteSystemAccountNotificationWebhookFuture(webhookId) + .map(unboxFullOrFail(_, Some(cc), DeleteWebhookError, 400)) + } yield () + } + } + + resourceDocs += ResourceDoc( + implementedInApiVersion, + nameOf(deleteSystemAccountNotificationWebhook), + "DELETE", + "/web-hooks/account/notifications/on-create-transaction/WEBHOOK_ID", + "Delete system level Account Notification Webhook", + """Delete a system level account notification webhook, so that it stops firing for transactions created anywhere on this instance. + | + |The webhook id is the `webhook_id` returned when the webhook was created. + | + |Deletion is permanent: the row is removed rather than deactivated, so a webhook deleted in error has to be created again. + | + |Authentication is Required.""".stripMargin, + EmptyBody, + EmptyBody, + List($AuthenticatedUserIsRequired, UserHasMissingRoles, NotificationWebhookNotFound, + DeleteWebhookError, UnknownError), + apiTagWebhook :: apiTagBank :: Nil, + Some(List(canDeleteSystemAccountNotificationWebhook)), + http4sPartialFunction = Some(deleteSystemAccountNotificationWebhook) + ) + + val deleteBankAccountNotificationWebhook: HttpRoutes[IO] = HttpRoutes.of[IO] { + case req @ DELETE -> `prefixPath` / "banks" / _ / "web-hooks" / "account" / "notifications" / "on-create-transaction" / webhookId => + EndpointHelpers.withUserAndBankDelete(req) { (_, bank, cc) => + val provider = code.webhook.BankAccountNotificationWebhookTrait.bankAccountNotificationWebhook.vend + for { + webhook <- provider.getBankAccountNotificationWebhookByIdFuture(webhookId) + .map(unboxFullOrFail(_, Some(cc), NotificationWebhookNotFound, 404)) + // A webhook belonging to another bank is reported as not found rather than forbidden. + // The role is held per bank, so answering 403 here would let a caller with the role at + // one bank discover which webhook ids exist at every other bank. + _ <- Future(if (webhook.bankId == bank.bankId.value) Full(true) else net.liftweb.common.Empty) + .map(unboxFullOrFail(_, Some(cc), NotificationWebhookNotFound, 404)) + _ <- provider.deleteBankAccountNotificationWebhookFuture(webhookId) + .map(unboxFullOrFail(_, Some(cc), DeleteWebhookError, 400)) + } yield () + } + } + + resourceDocs += ResourceDoc( + implementedInApiVersion, + nameOf(deleteBankAccountNotificationWebhook), + "DELETE", + "/banks/BANK_ID/web-hooks/account/notifications/on-create-transaction/WEBHOOK_ID", + "Delete bank level Account Notification Webhook", + """Delete a bank level account notification webhook, so that it stops firing for transactions created on the specified bank. + | + |The webhook id is the `webhook_id` returned when the webhook was created. A webhook belonging to a different bank is reported as not found. + | + |Deletion is permanent: the row is removed rather than deactivated, so a webhook deleted in error has to be created again. + | + |Authentication is Required.""".stripMargin, + EmptyBody, + EmptyBody, + List($AuthenticatedUserIsRequired, UserHasMissingRoles, $BankNotFound, + NotificationWebhookNotFound, DeleteWebhookError, UnknownError), + apiTagWebhook :: apiTagBank :: Nil, + Some(List(canDeleteAccountNotificationWebhookAtOneBank)), + http4sPartialFunction = Some(deleteBankAccountNotificationWebhook) + ) + val getBankSupportedRoutingSchemes: HttpRoutes[IO] = HttpRoutes.of[IO] { case req @ GET -> `prefixPath` / "banks" / _ / "supported-routing-schemes" => EndpointHelpers.withUserAndBank(req) { (_, bank, cc) => @@ -7057,7 +7138,7 @@ object Http4s700 { /** The User whose links are read: the caller, or the User its Consent acts for. Same rule as the provider. */ private def linkedCustomerOwnerId(userId: String): String = code.users.Users.users.vend - .attributedUserId(userId, code.users.UserReference.UserCustomerLinkUser).openOr(userId) + .attributedUserId(userId, code.users.UserReference.UserCustomerLink_UserId).openOr(userId) val getMyCustomersAtBank: HttpRoutes[IO] = HttpRoutes.of[IO] { case req @ GET -> `prefixPath` / "banks" / _ / "my" / "customers" => diff --git a/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnector.scala b/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnector.scala index b2354a5463..676eb3718f 100644 --- a/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnector.scala +++ b/obp-api/src/main/scala/code/bankconnectors/LocalMappedConnector.scala @@ -3432,7 +3432,7 @@ object LocalMappedConnector extends Connector with MdcLoggable { * - the request layer (`consentCreator` / `consenter`) knows things the stored chain cannot, * because a Berlin Group / UK consent carries its consenter on the request; * - otherwise the attribution, which walks the stored consent chain, applies the - * `BankCreator` policy and is the one place a delegated write is logged. + * `Bank_CreatedByUserId` policy and is the one place a delegated write is logged. * * ON_BEHALF_OF_USER_ID_PLAN.md, Phase 2. */ @@ -3441,7 +3441,7 @@ object LocalMappedConnector extends Connector with MdcLoggable { case None => "" case Some(callerUserId) => val fromStoredChain = Users.users.vend - .attributedUserId(callerUserId, code.users.UserReference.BankCreator) + .attributedUserId(callerUserId, code.users.UserReference.Bank_CreatedByUserId) .openOr(callerUserId) callContext .flatMap(cc => cc.consentCreator.or(cc.consenter).toOption) @@ -4822,6 +4822,75 @@ object LocalMappedConnector extends Connector with MdcLoggable { */ + /** + * This works out which person has to answer the Strong Customer Authentication challenge for a + * payment, which is not always the caller that started that payment. + * + * A payment can be started by an agent: a consent user, minted by a Consent that a person granted + * to an application such as Opey or an MCP client. The money being moved is still the person's, so + * the challenge has to reach that person. They are the one an OTP can actually be delivered to -- + * an agent identity has no email address and no phone number of its own -- and they are the one + * whose authorisation Strong Customer Authentication exists to obtain in the first place. Leaving + * the challenge on the agent gives one of two bad outcomes: on an instance that sends a real OTP + * nothing is delivered and the payment sits at INITIATED for ever behind a challenge nobody can + * answer, and on an instance configured with the DUMMY SCA method the agent simply authorises its + * own payment. + * + * The person is read back from the transaction request that has just been written rather than + * resolved a second time here. That row has already applied + * UserReference.TransactionRequest_UserId to the caller, and it also honours the consentCreator / + * consenter that a Berlin Group or UK consent carries on the request itself, which the stored + * consent chain cannot know. Reading it back keeps the challenge and the payment naming one and + * the same person, where a second independent resolution could drift from it. The policy + * governing the column this ends up in is + * UserReference.ExpectedChallengeAnswer_ExpectedUserId_TransactionRequest. + * + * @return the person to address the challenge to, or a Failure when no such person can be + * determined. That happens when the caller is an agent whose Consent does not name the + * person it acts for, and it is deliberately fail-closed: the payment is refused rather + * than parked behind a challenge that can never be answered. + */ + // Visible to the rest of `code` rather than fully private so that AgentDelegationTest can put the + // decision itself under test; driving it through createTransactionRequestv210 would need a whole + // bank, account and view fixture to reach one branch. + private[code] def paymentChallengeRecipient(transactionRequest: TransactionRequest, initiator: User): Box[User] = { + // Applying the policy here is what logs the delegation against the column about to be written, + // and it is what decides the answer if the transaction request row carries none. + val attributedUserId: Option[String] = Users.users.vend + .attributionOf(initiator.userId, code.users.UserReference.ExpectedChallengeAnswer_ExpectedUserId_TransactionRequest) + .toOption.map(_.onBehalfOfUserId).filter(_.nonEmpty) + val recipientUserId = transactionRequest.on_behalf_of_user_id.filter(_.nonEmpty) + .orElse(attributedUserId) + .getOrElse(initiator.userId) + if (recipientUserId == initiator.userId) { + // The caller is acting for themselves. This is the ordinary case for a person logging in + // directly, and it is also what a broken consent chain collapses to, so an agent that has + // landed here must still be refused. + if (initiator.isOriginalUser) Full(initiator) + else { + logger.warn(s"paymentChallengeRecipient says: the caller (${initiator.userId}) of transaction request " + + s"(${transactionRequest.id.value}) is a consent user and its Consent does not name the user it acts for, " + + s"so there is nobody to address the payment challenge to. Refusing the payment.") + Failure(PaymentChallengeHasNoOnBehalfOfUser) + } + } else { + // Delegated. The plan's invariant is that an on-behalf-of user is always an original user, so + // a stored chain saying otherwise is a data bug, and it is refused rather than acted on. + Users.users.vend.getUserByUserId(recipientUserId) match { + case Full(recipient) if recipient.isOriginalUser => Full(recipient) + case Full(_) => + logger.warn(s"paymentChallengeRecipient says: the on-behalf-of user ($recipientUserId) of transaction " + + s"request (${transactionRequest.id.value}) is itself a consent user, which breaks the one-hop " + + s"invariant. Refusing the payment.") + Failure(PaymentChallengeHasNoOnBehalfOfUser) + case _ => + logger.warn(s"paymentChallengeRecipient says: the on-behalf-of user ($recipientUserId) of transaction " + + s"request (${transactionRequest.id.value}) does not exist. Refusing the payment.") + Failure(PaymentChallengeHasNoOnBehalfOfUser) + } + } + } + override def createTransactionRequestv210(initiator: User, viewId: ViewId, fromAccount: BankAccount, @@ -4921,11 +4990,16 @@ object LocalMappedConnector extends Connector with MdcLoggable { } case TransactionRequestStatus.INITIATED => for { + // The challenge belongs to the person the payment is for, who is not the caller when an + // agent started it. See paymentChallengeRecipient. + challengeRecipient <- Future { paymentChallengeRecipient(transactionRequest, initiator) } map { + unboxFullOrFail(_, callContext, PaymentChallengeHasNoOnBehalfOfUser, 400) + } //if challenge necessary, create a new one (challengeId, callContext) <- createChallenge( fromAccount.bankId, fromAccount.accountId, - initiator.userId, + challengeRecipient.userId, transactionRequestType: TransactionRequestType, transactionRequest.id.value, scaMethod, @@ -5093,7 +5167,8 @@ object LocalMappedConnector extends Connector with MdcLoggable { } else { // return the lists of users, who need to be answered the challenges def getUsersForChallenges(bankId: BankId, - accountId: AccountId) = { + accountId: AccountId, + challengeRecipient: User) = { Connector.connector.vend.getAccountAttributesByAccount(bankId, accountId, None) map { _._1.map { x => { @@ -5101,21 +5176,31 @@ object LocalMappedConnector extends Connector with MdcLoggable { for ( permission <- Views.views.vend.permissions(BankIdAccountId(bankId, accountId)) ) yield { - permission.views.exists(view =>view.view.allowed_actions.exists( _ == CAN_ANSWER_TRANSACTION_REQUEST_CHALLENGE)) + // An agent that holds a view on this account is skipped even when that view + // carries the permission: a payment challenge is answered by a person, and + // an agent identity has no email address or phone number an OTP could reach. + // See paymentChallengeRecipient for the whole argument. + (permission.views.exists(view =>view.view.allowed_actions.exists( _ == CAN_ANSWER_TRANSACTION_REQUEST_CHALLENGE)) && + permission.user.isOriginalUser) match { case true => Some(permission.user) case _ => None } } - } else List(Some(initiator)) + } else List(Some(challengeRecipient)) }.flatten.distinct } } } for { + // The challenge belongs to the person the payment is for, who is not the caller when + // an agent started it. See paymentChallengeRecipient. + challengeRecipient <- Future { paymentChallengeRecipient(transactionRequest, initiator) } map { + unboxFullOrFail(_, callContext, PaymentChallengeHasNoOnBehalfOfUser, 400) + } //if challenge necessary, create a new one - users <- getUsersForChallenges(fromAccount.bankId, fromAccount.accountId) + users <- getUsersForChallenges(fromAccount.bankId, fromAccount.accountId, challengeRecipient) //now we support multiple challenges. We can support multiple people to answer the challenges. //So here we return the challengeIds. (challenges, callContext) <- Connector.connector.vend.createChallengesC2( diff --git a/obp-api/src/main/scala/code/dynamicEntity/MapppedDynamicDataProvider.scala b/obp-api/src/main/scala/code/dynamicEntity/MapppedDynamicDataProvider.scala index 16fe94c064..6bc839b26b 100644 --- a/obp-api/src/main/scala/code/dynamicEntity/MapppedDynamicDataProvider.scala +++ b/obp-api/src/main/scala/code/dynamicEntity/MapppedDynamicDataProvider.scala @@ -30,7 +30,7 @@ package code.DynamicData import org.json4s._ import code.api.util.CustomJsonFormats import code.api.util.ErrorMessages.DynamicDataNotFound -import code.util.MappedUUID +import code.api.util.APIUtil.generateUUID import net.liftweb.common.{Box, Failure, Full} import com.openbankproject.commons.util.json import org.json4s.JObject @@ -51,14 +51,14 @@ object MappedDynamicDataProvider extends DynamicDataProvider with CustomJsonForm /** * The user a row belongs to: the caller, or the user its consent names (attribution policy - * UserReference.DynamicDataUser). Applied on every entry point that takes a userId, for + * UserReference.DynamicData_UserId). Applied on every entry point that takes a userId, for * reads as well as writes, so a consent user reads, updates and deletes the same rows it * writes. The resolver logs each redirect; a Failure (invariant broken) keeps the caller. * The endpoint decides who may reach this provider (a consent user needs the entity's role, * see Http4sDynamicEntity.personalRoleWaived). ON_BEHALF_OF_USER_ID_PLAN.md, Phase 2. */ private def ownerOf(userId: Option[String]): Option[String] = - userId.map(id => code.users.Users.users.vend.attributedUserId(id, code.users.UserReference.DynamicDataUser).openOr(id)) + userId.map(id => code.users.Users.users.vend.attributedUserId(id, code.users.UserReference.DynamicData_UserId).openOr(id)) override def save(bankId: Option[String], entityName: String, requestBody: JObject, userId: Option[String], isPersonalEntity: Boolean): Box[DynamicDataT] = { val idName = getIdName(entityName) @@ -290,7 +290,20 @@ class DynamicData extends DynamicDataT with LongKeyedMapper[DynamicData] with Id override def getSingleton = DynamicData - object DynamicDataId extends MappedUUID(this) + /** + * The identifier of a single Dynamic Entity record. This column used to be a `MappedUUID`, which is + * 36 characters wide because that is the length of a UUID. A caller may however supply the id itself + * in the request body instead of letting one be generated, which is how a Dynamic Entity is given a + * natural key such as a country code or a scheme name, so the value is not always a UUID and can be + * considerably longer than 36 characters. Such a value used to reach Postgres unchanged and fail + * there with "value too long for type character varying(36)". The column is therefore an ordinary + * string of 255 characters, the same width as the two other columns that hold this same id: the + * row level access list in DynamicDataAccess, and the `data_id` column of the SQL projection tables. + * The default value stays a generated UUID, so a request that omits the id behaves exactly as before. + */ + object DynamicDataId extends MappedString(this, 255) { + override def defaultValue = generateUUID() + } object DynamicEntityName extends MappedString(this, 255) object DataJson extends MappedText(this) diff --git a/obp-api/src/main/scala/code/dynamicEntity/MapppedDynamicEntityProvider.scala b/obp-api/src/main/scala/code/dynamicEntity/MapppedDynamicEntityProvider.scala index b2110964ce..8efae61032 100644 --- a/obp-api/src/main/scala/code/dynamicEntity/MapppedDynamicEntityProvider.scala +++ b/obp-api/src/main/scala/code/dynamicEntity/MapppedDynamicEntityProvider.scala @@ -109,9 +109,9 @@ object MappedDynamicEntityProvider extends DynamicEntityProvider with CustomJson val saved = entityToPersist .EntityName(dynamicEntity.entityName) .MetadataJson(dynamicEntity.metadataJson) - // Definition creator resolves to the on-behalf-of user (UserReference.DynamicEntityUser): + // Definition creator resolves to the on-behalf-of user (UserReference.DynamicEntity_UserId): // a consent user owns nothing durable. ON_BEHALF_OF_USER_ID_PLAN.md, Phase 2. - .UserId(code.users.Users.users.vend.attributedUserId(dynamicEntity.userId, code.users.UserReference.DynamicEntityUser).openOr(dynamicEntity.userId)) + .UserId(code.users.Users.users.vend.attributedUserId(dynamicEntity.userId, code.users.UserReference.DynamicEntity_UserId).openOr(dynamicEntity.userId)) .BankId(dynamicEntity.bankId.getOrElse(null)) .HasPersonalEntity(dynamicEntity.hasPersonalEntity) .HasPublicAccess(dynamicEntity.hasPublicAccess) diff --git a/obp-api/src/main/scala/code/entitlement/MappedEntitlements.scala b/obp-api/src/main/scala/code/entitlement/MappedEntitlements.scala index cddde96e5a..646a4ef6b1 100644 --- a/obp-api/src/main/scala/code/entitlement/MappedEntitlements.scala +++ b/obp-api/src/main/scala/code/entitlement/MappedEntitlements.scala @@ -207,8 +207,8 @@ object MappedEntitlementsProvider extends EntitlementProvider with MdcLoggable { // (ConsentEntitlementUser); every other grant is written to the on-behalf-of user // (EntitlementUser). The resolver logs the redirect. ON_BEHALF_OF_USER_ID_PLAN.md. val ref = - if (createdByProcess == code.api.Constant.consent_user) code.users.UserReference.ConsentEntitlementUser - else code.users.UserReference.EntitlementUser + if (createdByProcess == code.api.Constant.consent_user) code.users.UserReference.Entitlement_UserId_ConsentScope + else code.users.UserReference.Entitlement_UserId val targetUserId = code.users.Users.users.vend.attributedUserId(userId, ref) match { case Full(id) => id case f: Failure => return f diff --git a/obp-api/src/main/scala/code/metadata/counterparties/MapperCounterparties.scala b/obp-api/src/main/scala/code/metadata/counterparties/MapperCounterparties.scala index 0b5878fbc7..3dff270e9c 100644 --- a/obp-api/src/main/scala/code/metadata/counterparties/MapperCounterparties.scala +++ b/obp-api/src/main/scala/code/metadata/counterparties/MapperCounterparties.scala @@ -219,6 +219,30 @@ object MapperCounterparties extends Counterparties with MdcLoggable { By(MappedCounterparty.mThisViewId, viewId.value))) } + /** + * (who made the call, who they were acting for) for a counterparty about to be written. + * + * Record-both rather than redirect. A counterparty is the control on where money may be sent, + * so "which agent created this" must be answerable from the row itself — not reconstructed by + * matching a timestamp against a metrics table that has its own retention. And mCreatedByUserId + * is published as `created_by_user_id` on the v2.2.0 and v4.0.0 counterparty responses, so + * redirecting it would make a STABLE field report a human for something an agent did. + * + * Only the caller's id is needed to get both: attributionOf resolves the human from it. What a + * provider cannot do without a CallContext is honour the request layer's consentCreator / + * consenter, which a Berlin Group or UK consent carries on the request rather than in the + * stored chain — for those, an unbound consent falls back to the caller, which is the + * documented fail-closed behaviour. + * + * ON_BEHALF_OF_USER_ID_PLAN.md, Phase 2. + */ + private def counterpartyCreators(createdByUserId: String): (String, String) = + if (createdByUserId == null || createdByUserId.isEmpty) (createdByUserId, createdByUserId) + else Users.users.vend.attributionOf(createdByUserId, code.users.UserReference.Counterparty_CreatedByUserId) match { + case Full(attribution) => (attribution.userId, attribution.onBehalfOfUserId) + case _ => (createdByUserId, createdByUserId) + } + override def createCounterparty( createdByUserId: String, thisBankId: String, @@ -239,10 +263,12 @@ object MapperCounterparties extends Counterparties with MdcLoggable { bespoke: List[CounterpartyBespoke] ): Box[CounterpartyTrait] = { tryo{ + val (actorUserId, onBehalfOfUserId) = counterpartyCreators(createdByUserId) val mappedCounterparty = MappedCounterparty.create .mCounterPartyId(APIUtil.createExplicitCounterpartyId) //We create the Counterparty_Id here, it means, it will be created in each connector. .mName(name) - .mCreatedByUserId(createdByUserId) + .mCreatedByUserId(actorUserId) + .mCreatedByOnBehalfOfUserId(onBehalfOfUserId) .mThisBankId(thisBankId) .mThisAccountId(thisAccountId) .mThisViewId(thisViewId) @@ -506,6 +532,19 @@ class MappedCounterparty extends CounterpartyTrait with LongKeyedMapper[MappedCo def getSingleton = MappedCounterparty object mCreatedByUserId extends MappedString(this, 36) + /** + * The human the creator was acting for, when a Consent was involved; otherwise the same as + * mCreatedByUserId. Record-both, as MappedTransactionRequest does — a counterparty controls + * where money may be sent, so both "who did this" and "whose is this" have to be answerable + * from the row rather than by correlating a timestamp against the metrics table. + * + * Not on CounterpartyTrait and not in any JSON: the trait lives in obp-commons and is + * implemented by the remote-connector DTOs, and adding a field to the v2.2.0/v4.0.0 + * counterparty responses would change a STABLE contract. Internal audit column for now. + * + * ON_BEHALF_OF_USER_ID_PLAN.md, Phase 2. + */ + object mCreatedByOnBehalfOfUserId extends MappedString(this, 36) object mName extends MappedString(this, 36) object mThisBankId extends MappedString(this, 36) object mThisAccountId extends AccountIdString(this) diff --git a/obp-api/src/main/scala/code/transactionChallenge/MappedChallengeProvider.scala b/obp-api/src/main/scala/code/transactionChallenge/MappedChallengeProvider.scala index 67407b7cae..c01b6b20b5 100644 --- a/obp-api/src/main/scala/code/transactionChallenge/MappedChallengeProvider.scala +++ b/obp-api/src/main/scala/code/transactionChallenge/MappedChallengeProvider.scala @@ -110,6 +110,13 @@ object MappedChallengeProvider extends ChallengeProvider { ): Box[ChallengeTrait] = { for{ challenge <- getChallenge(challengeId) ?~! s"${ErrorMessages.InvalidTransactionRequestChallengeId}" + // Whether the caller is the person this challenge was addressed to is settled before the + // attempt counter moves. Somebody who was never sent the code cannot be "getting it wrong", + // and counting their call would let them use up the allowance of the person who was sent it + // -- an agent that politely tried to answer its human's payment challenge three times would + // lock that payment out. The answer itself is not looked at on this path. + _ <- if (userId.forall(_ == challenge.expectedUserId)) Full(true) + else Failure(ErrorMessages.ChallengeNotAddressedToCaller) newAttemptCounterValue <- tryo(code.bankconnectors.DoobieChallengeQueries.incrementAndGetChallengeCounter(challengeId)) ?~! "Failed to update challenge attempt counter" createDateTime = challenge.createdAt.get challengeTTL : Long = Helpers.seconds(APIUtil.transactionRequestChallengeTtl) @@ -121,6 +128,8 @@ object MappedChallengeProvider extends ChallengeProvider { val currentHashedAnswer = BCrypt.hashpw(challengeAnswer, challenge.salt).substring(0, 44) val expectedHashedAnswer = challenge.expectedAnswer val answerMatches = currentHashedAnswer == expectedHashedAnswer + // The caller was already matched against expectedUserId above, before the attempt counter + // moved; this repeats it so the condition stays readable on its own. val userMatches = userId.forall(_ == challenge.expectedUserId) if (answerMatches && userMatches) { markChallengeSuccessful(challengeId) diff --git a/obp-api/src/main/scala/code/transactionrequests/MappedTransactionRequestProvider.scala b/obp-api/src/main/scala/code/transactionrequests/MappedTransactionRequestProvider.scala index 0d1e2f6663..feb8437f94 100644 --- a/obp-api/src/main/scala/code/transactionrequests/MappedTransactionRequestProvider.scala +++ b/obp-api/src/main/scala/code/transactionrequests/MappedTransactionRequestProvider.scala @@ -143,7 +143,7 @@ object MappedTransactionRequestProvider extends TransactionRequestProvider with // Note: We don't save transaction_ids, status and challenge here. // Record both: mUserId = the authenticated user, mOnBehalfOfUserId = who the payment is for. - // The attribution call (UserReference.TransactionRequest) answers the second; the request + // The attribution call (UserReference.TransactionRequest_UserId) answers the second; the request // layer's consentCreator / consenter take precedence over its DB chain, as in // CallContext.onBehalfOfUserId. The call is made even when they already name the // on-behalf-of user, because it is the one place a delegated write is logged. @@ -154,7 +154,7 @@ object MappedTransactionRequestProvider extends TransactionRequestProvider with val authenticatedUserId: Option[String] = callContext.flatMap(_.user.toOption).map(_.userId).filter(_.nonEmpty) val transactionRequestAttribution: Option[code.users.Attribution] = authenticatedUserId.flatMap(userId => - code.users.Users.users.vend.attributionOf(userId, code.users.UserReference.TransactionRequest).toOption) + code.users.Users.users.vend.attributionOf(userId, code.users.UserReference.TransactionRequest_UserId).toOption) val onBehalfOfUserIdOption: Option[String] = callContext.flatMap(cc => cc.consentCreator.or(cc.consenter).toOption).map(_.userId).filter(_.nonEmpty) .orElse(transactionRequestAttribution.map(_.onBehalfOfUserId)) diff --git a/obp-api/src/main/scala/code/usercustomerlinks/MappedUserCustomerLink.scala b/obp-api/src/main/scala/code/usercustomerlinks/MappedUserCustomerLink.scala index c267e559e9..b53169cadd 100644 --- a/obp-api/src/main/scala/code/usercustomerlinks/MappedUserCustomerLink.scala +++ b/obp-api/src/main/scala/code/usercustomerlinks/MappedUserCustomerLink.scala @@ -41,7 +41,7 @@ import com.openbankproject.commons.ExecutionContext.Implicits.global object MappedUserCustomerLinkProvider extends UserCustomerLinkProvider { /** - * On-behalf-of guard (attribution policy UserReference.UserCustomerLinkUser): a User-Customer + * On-behalf-of guard (attribution policy UserReference.UserCustomerLink_UserId): a User-Customer * link is owned by the on-behalf-of user. When the caller is a consent user the row is written * for the user the consent names, so the link does not strand when the consent dies. For an * original user this is a no-op. The resolver logs every redirect. @@ -60,7 +60,7 @@ object MappedUserCustomerLinkProvider extends UserCustomerLinkProvider { * ON_BEHALF_OF_USER_ID_PLAN.md, Phase 2 row 2. */ private def linkOwnerUserId(userId: String): String = - Users.users.vend.attributedUserId(userId, UserReference.UserCustomerLinkUser).openOr(userId) + Users.users.vend.attributedUserId(userId, UserReference.UserCustomerLink_UserId).openOr(userId) def createUserCustomerLink(userId: String, customerId: String, dateInserted: Date, isActive: Boolean): Box[UserCustomerLink] = { val ownerUserId = linkOwnerUserId(userId) diff --git a/obp-api/src/main/scala/code/users/LiftUsers.scala b/obp-api/src/main/scala/code/users/LiftUsers.scala index 8ebad921c8..7185e79e88 100644 --- a/obp-api/src/main/scala/code/users/LiftUsers.scala +++ b/obp-api/src/main/scala/code/users/LiftUsers.scala @@ -50,12 +50,15 @@ object LiftUsers extends Users with MdcLoggable{ // ---- on-behalf-of resolution (ON_BEHALF_OF_USER_ID_PLAN.md, Phase 1) ---------------------- - /** What the chain resolved to, and whether the answer is stable enough to cache. */ + /** This holds the outcome of one resolution: the person the caller acts for, the Consent that + * said so, and whether the answer is settled enough to be worth caching. */ private case class Resolved(onBehalfOfUserId: Box[String], consentId: Option[String], cacheable: Boolean) - /** Non-empty, bound answers only: the consent -> human binding never changes once set. The - * "consent has no human yet" answer (BG/UK before authorisation) must not be pinned, or a - * consent bound a minute later stays on the consent user for the TTL. */ + /** This cache holds settled answers only. Once a Consent names its person that binding never + * changes, so it is safe to keep. The answer "this consent has no person yet", which is what a + * Berlin Group or UK consent gives before it is authorised, must never be cached: the consent + * may be authorised a moment later, and a cached answer would keep every write on the agent + * until the entry expired. */ private lazy val onBehalfOfCacheTtlSeconds: Long = APIUtil.getPropsAsLongValue("on_behalf_of_user_id.cache_ttl_seconds", 600L) private lazy val onBehalfOfCache: com.google.common.cache.Cache[String, Resolved] = @@ -66,8 +69,31 @@ object LiftUsers extends Users with MdcLoggable{ private def nonBlank(s: String): Boolean = s != null && s.nonEmpty + /** + * This walks from the authenticated caller to the person it acts for. It is exactly one hop: + * + * ResourceUser(userId).isConsentUser -> CreatedByConsentId -> Consent -> consent.userId + * + * and the user that lands on must itself be an original user. There is no loop and no second + * hop by design: a consent user cannot create a Consent (UserReference.Consent_UserId is + * policy Reject), so a well-formed chain is always one step deep. That is what makes the + * result checkable — see ON_BEHALF_OF_USER_ID_PLAN.md, Decision 4. + * + * Every branch below FAILS CLOSED, i.e. keeps the caller, except one. Keeping the caller means + * the row is stored against the agent identity and strands when the consent dies — bad, but + * local and visible in the WARN. Guessing a human instead would silently attribute writes to + * someone who never authorised them, which is worse. The single exception is a consent naming + * another consent user: that breaks the one-hop invariant outright, so it returns a Failure + * rather than a fallback, because there is no answer that is even arguably right. + * + * Takes only the id on purpose (see the trait): nothing request-asserted can steer it. + */ private def resolveOnBehalfOf(userId: String): Resolved = { + // An empty caller id is not an error here — anonymous and system paths reach writers too. + // Hand it straight back so callers get "" rather than a Failure they would have to unpick. if (!nonBlank(userId)) return Resolved(Full(userId), None, cacheable = false) + // Every UseOnBehalfOfUserId write costs this lookup, so consent callers would otherwise pay + // two extra reads per row written. Only stable answers were put here — see the cache above. val cached = if (onBehalfOfCacheTtlSeconds > 0) Option(onBehalfOfCache.getIfPresent(userId)) else None if (cached.isDefined) return cached.get val resolved: Resolved = ResourceUser.find(By(ResourceUser.userId_, userId)) match { @@ -79,48 +105,128 @@ object LiftUsers extends Users with MdcLoggable{ case Full(target) if target.isOriginalUser => Resolved(Full(consent.userId), Some(consentId), cacheable = true) case Full(_) => - logger.warn(s"onBehalfOfUserIdOf: consent user $userId's consent $consentId names ${consent.userId}, which is itself a consent user — invariant broken, refusing") + logger.warn(s"resolveOnBehalfOfUserId: consent user $userId's consent $consentId names ${consent.userId}, which is itself a consent user — invariant broken, refusing") Resolved(Failure(s"${ErrorMessages.InvalidUserId} consent $consentId names a consent user as its on-behalf-of user"), Some(consentId), cacheable = false) case _ => - logger.warn(s"onBehalfOfUserIdOf: consent user $userId's consent $consentId names unknown user ${consent.userId}; keeping $userId (fails closed)") + logger.warn(s"resolveOnBehalfOfUserId: consent user $userId's consent $consentId names unknown user ${consent.userId}; keeping $userId (fails closed)") Resolved(Full(userId), Some(consentId), cacheable = false) } case Full(_) => - logger.warn(s"onBehalfOfUserIdOf: consent user $userId's consent $consentId has no human yet (not authorised); keeping $userId (fails closed, not cached)") + logger.warn(s"resolveOnBehalfOfUserId: consent user $userId's consent $consentId has no human yet (not authorised); keeping $userId (fails closed, not cached)") Resolved(Full(userId), Some(consentId), cacheable = false) case _ => - logger.warn(s"onBehalfOfUserIdOf: consent user $userId names consent $consentId, which does not exist; keeping $userId (fails closed)") + logger.warn(s"resolveOnBehalfOfUserId: consent user $userId names consent $consentId, which does not exist; keeping $userId (fails closed)") Resolved(Full(userId), Some(consentId), cacheable = false) } + // An ordinary user: acts for itself. Cacheable because a user that is not consent-minted + // can never become one — CreatedByConsentId is written at creation and never updated. case Full(_) => Resolved(Full(userId), None, cacheable = true) case _ => - logger.warn(s"onBehalfOfUserIdOf: no ResourceUser $userId; keeping it (fails closed)") + logger.warn(s"resolveOnBehalfOfUserId: no ResourceUser $userId; keeping it (fails closed)") Resolved(Full(userId), None, cacheable = false) } + // Only the two settled answers are stored: "ordinary user, acts for itself" and "consent user, + // bound to this human". Everything else is a transient or broken state that can legitimately + // change within the TTL — most importantly a BG/UK consent authorised a moment from now, which + // must not stay pinned to the agent for the next ten minutes. if (resolved.cacheable && onBehalfOfCacheTtlSeconds > 0) onBehalfOfCache.put(userId, resolved) resolved } - override def onBehalfOfUserIdOf(userId: String): Box[String] = resolveOnBehalfOf(userId).onBehalfOfUserId - + override def resolveOnBehalfOfUserId(userId: String): Box[String] = resolveOnBehalfOf(userId).onBehalfOfUserId + + /** + * The one entry point a provider calls before writing a user id into a column. + * + * `ref` says WHICH COLUMN is about to be written. Each UserReference value names a Mapper class + * and one or more of its fields, and carries the policy chosen for them — UserReference.Bank_CreatedByUserId + * names MappedBank.CreatedByUserId, and applies to that column only. + * + * It has to be a parameter rather than something derived from the caller, because the right id + * depends on the column and not just on who is calling. MappedEntitlement.mUserId is the case + * that proves it: the same consent user writing that one column gets a different answer + * depending on which process is writing. Entitlement_UserId (UseOnBehalfOfUserId) is a role being + * granted to somebody, so it lands on the human; Entitlement_UserId_ConsentScope (UseAuthenticatedUserId) is the + * consent engine copying the Consent's own scope onto the agent, so it must stay on the agent. + * Same column, opposite policies. + * + * Which policy applies to which column is recorded in UserReference.scala and nowhere else; + * this method only applies it. + * + * ---- Callers, as worked examples (2026-09-15) ---- + * + * A. Single-column writers. Want one id, so they use the `attributedUserId` convenience and + * fall back to the caller, so that a Failure can never blank the column. + * + * caller reference passed + * --------------------------------------------- -------------------- + * MappedUserCustomerLink.linkOwnerUserId UserCustomerLink_UserId + * MapperAccountHolders.getOrCreateAccountHolder AccountHolders_User + * LocalMappedConnector.bankCreatorUserId Bank_CreatedByUserId + * + * B. Record-both tables. Call attributionOf directly, because they need both ids out of the + * one Attribution rather than just the single value to store. + * + * caller reference columns written + * -------------------------------- ------------------ ----------------------------- + * MappedTransactionRequestProvider TransactionRequest mUserId + mOnBehalfOfUserId + * + * C. One column, two policies. The reference is chosen per process, then passed in. + * + * caller reference chosen when + * --------------------------------- ---------------------- ---------------------------- + * MappedEntitlements.addEntitlement Entitlement_UserId_ConsentScope createdByProcess == + * Constant.consent_user + * MappedEntitlements.addEntitlement Entitlement_UserId otherwise + * + * D. Reads. These resolve too, and must, or an agent cannot see back what it just wrote: + * personal rows are keyed by the same column on both sides, so the redirect has to be + * symmetric. The last two are endpoint-level rather than provider-level — where a handler + * decides WHOSE rows to read, it has to ask the same question the provider asks on write. + * + * caller reference covers + * ------------------------------------ -------------------- ---------------------- + * MapppedDynamicDataProvider DynamicData_UserId save/update/get/delete + * MapppedDynamicEntityProvider DynamicEntity_UserId definition creator + * Http4sDynamicEntity.personalRowOwner DynamicData_UserId projection read path + * Http4s700.linkedCustomerOwnerId UserCustomerLink_UserId v7 "my customers" + * + * This is also the audit point. A delegated write logs here and nowhere else, which is why + * providers should call it even when they already know the on-behalf-of user from the request + * layer (see LocalMappedConnector.bankCreatorUserId) — and why naming the reference in main is + * what OnBehalfOfOwnershipSweepTest's ratchet counts as "wired". + */ override def attributionOf(userId: String, ref: UserReference): Box[Attribution] = ref.policy match { - case AttributionPolicy.KeepUserId => + // Audit and authorisation-materialisation columns: the caller's own id IS the truthful value, + // so the resolver is not consulted at all. Deliberate — it also keeps these writes free of the + // two extra reads resolution costs on a cache miss. + case AttributionPolicy.UseAuthenticatedUserId => Full(Attribution(userId, userId, None, ref)) + // Ownership columns: store the human. Note the WARN fires only when the answer actually + // differs from the caller, so ordinary traffic stays quiet and every line in the log is a + // real delegated write, naming the reference and the column it landed in. case AttributionPolicy.UseOnBehalfOfUserId => - val r = resolveOnBehalfOf(userId) - r.onBehalfOfUserId.map { h => - val a = Attribution(userId, h, r.consentId, ref) - if (a.isDelegated) - logger.warn(s"attribution ${ref.name}: user $userId is a consent user (consent ${r.consentId.getOrElse("?")}); writing on-behalf-of user $h to ${ref.mapperClass}.${ref.fields.mkString("/")}") - a + val resolved = resolveOnBehalfOf(userId) + resolved.onBehalfOfUserId.map { onBehalfOfUserId => + val attribution = Attribution(userId, onBehalfOfUserId, resolved.consentId, ref) + if (attribution.isDelegated) + logger.warn(s"attribution ${ref.name}: user $userId is a consent user (consent ${resolved.consentId.getOrElse("?")}); writing on-behalf-of user $onBehalfOfUserId to ${ref.mapperClass}.${ref.fields.mkString("/")}") + attribution } + // Things an agent must not do at all, whoever it acts for: minting a Consent (nested + // delegation) or an OAuth consumer/token (credentials that outlive the consent). There is no + // redirect that would make these safe — a Consent created "for" the human would be one the + // human never granted — so this returns a Failure and the endpoint turns it into a 400. + // NOTE: as of 2026-09-15 nothing calls attributionOf with a Reject reference, so the refusal + // is pinned by AgentDelegationTest but not yet reachable over HTTP. Wiring the consent-create + // path is tracked in ON_BEHALF_OF_USER_ID_PLAN.md, Phase 2/3. case AttributionPolicy.Reject => - val r = resolveOnBehalfOf(userId) - r.onBehalfOfUserId.flatMap { h => - if (h == userId) Full(Attribution(userId, h, r.consentId, ref)) + val resolved = resolveOnBehalfOf(userId) + resolved.onBehalfOfUserId.flatMap { onBehalfOfUserId => + if (onBehalfOfUserId == userId) Full(Attribution(userId, onBehalfOfUserId, resolved.consentId, ref)) else { - logger.warn(s"attribution ${ref.name}: user $userId is a consent user (on behalf of $h); a consent user must not write ${ref.mapperClass}.${ref.fields.mkString("/")} — rejected") - Failure(s"${ErrorMessages.InvalidUserId} ${ref.name}: user $userId is a consent user; this action must be performed by the user it acts for ($h)") + logger.warn(s"attribution ${ref.name}: user $userId is a consent user (on behalf of $onBehalfOfUserId); a consent user must not write ${ref.mapperClass}.${ref.fields.mkString("/")} — rejected") + Failure(s"${ErrorMessages.InvalidUserId} ${ref.name}: user $userId is a consent user; this action must be performed by the user it acts for ($onBehalfOfUserId)") } } } @@ -407,8 +513,8 @@ object LiftUsers extends Users with MdcLoggable{ s"getUsersV600F says: batched $totalEntitlements entitlement(s) and $totalAgreements agreement(s) across ${userIds.size} user(s)" ) - rows.map { r => - (r, entitlementsByUserId.getOrElse(r.userId, Nil), agreementsByUserId.getOrElse(r.userId, Nil)) + rows.map { user => + (user, entitlementsByUserId.getOrElse(user.userId, Nil), agreementsByUserId.getOrElse(user.userId, Nil)) } } } diff --git a/obp-api/src/main/scala/code/users/UserReference.scala b/obp-api/src/main/scala/code/users/UserReference.scala index 356554bfb6..9be9a92dbd 100644 --- a/obp-api/src/main/scala/code/users/UserReference.scala +++ b/obp-api/src/main/scala/code/users/UserReference.scala @@ -28,25 +28,52 @@ TESOBE (http://www.tesobe.com/) package code.users /** - * Attribution policy: what a user-reference column stores when the caller is a consent user. - * Design and vocabulary: OBP-API/ON_BEHALF_OF_USER_ID_PLAN.md ("The policy file"). + * This trait answers a single question: when an API call is made by an agent acting for a person, + * whose user id should be written to the database? * - * - KeepUserId the authenticated user's own id; no resolver - * - UseOnBehalfOfUserId the on-behalf-of user's id, via Users.onBehalfOfUserIdOf - * - Reject a consent user must not do this at all: Failure -> 400 + * A person can grant a Consent to a piece of software. That software then calls the API under an + * identity of its own, with its own user id, created for the Consent and discarded when the Consent + * is revoked. We call such an identity a consent user. If the software creates a bank account and + * OBP records the consent user as the account holder, the account now belongs to an identity that + * is about to disappear, and the person who asked for it can no longer see it. + * + * Every column that stores a user id therefore needs a decision, and this trait is the set of + * decisions available. There are three of them: + * + * - UseAuthenticatedUserId means store the id of whoever actually made the call, agent included, + * and resolve nothing. It suits audit trails, and the rows carrying a Consent's own permissions, + * which are meant to die with it. + * - UseOnBehalfOfUserId means store the id of the person the agent is acting for, which + * Users.resolveOnBehalfOfUserId looks up. It suits anything the person owns. + * - Reject means a consent user must not do this at all, and the call fails with a 400. + * + * The wider design, and the vocabulary it uses, is written up in ON_BEHALF_OF_USER_ID_PLAN.md. */ sealed trait AttributionPolicy object AttributionPolicy { - case object KeepUserId extends AttributionPolicy - case object UseOnBehalfOfUserId extends AttributionPolicy - case object Reject extends AttributionPolicy + case object UseAuthenticatedUserId extends AttributionPolicy + case object UseOnBehalfOfUserId extends AttributionPolicy + case object Reject extends AttributionPolicy } /** - * What a provider gets back from Users.attributionOf: everything it should store, plus the - * facts the resolver logged. userId is the authenticated caller; onBehalfOfUserId is who owns - * what the call creates (== userId for an original user acting alone; for a KeepUserId - * reference the resolver is not consulted and it is simply userId). + * This class holds the two user ids a database write can choose between, and says which of them to + * use. Users.attributionOf builds one and returns it. + * + * The code that asks for it is whatever is about to store a row: in OBP that means a provider, + * which is the class owning reads and writes for one table, such as MapperCounterparties or + * MappedEntitlements. It passes in the id of the caller and the UserReference for the column it is + * filling, and gets one of these back. + * + * The two ids are userId, the caller who authenticated, which is the agent whenever an agent made + * the call; and onBehalfOfUserId, the person that call was made for. They are equal when nobody is + * delegating, and equal again when the reference's policy is UseAuthenticatedUserId, because that + * policy never looks the person up. consentId names the Consent that established the delegation, + * where there was one. + * + * A provider filling a single column need not choose between them itself: userIdToStore applies the + * policy and returns the one id to write. Only the few tables that keep both columns, a transaction + * request and a counterparty, read userId and onBehalfOfUserId directly. */ case class Attribution( userId: String, @@ -54,8 +81,12 @@ case class Attribution( consentId: Option[String], ref: UserReference ) { + /** This is true when an agent made the call for somebody else, and false when the caller was + * acting only for themselves. */ def isDelegated: Boolean = userId != onBehalfOfUserId - /** The single value for the column(s) `ref` names, per its policy. */ + + /** This is the one id to write into the column or columns that `ref` names, chosen by applying + * that reference's policy. */ def userIdToStore: String = ref.policy match { case AttributionPolicy.UseOnBehalfOfUserId => onBehalfOfUserId case _ => userId @@ -63,12 +94,37 @@ case class Attribution( } /** - * One value per user-reference column (or per record-both table). This file IS the policy - * table: Users.attributionOf reads it at runtime, and UserReferenceAttributionPolicyTest - * (frozen-style) asserts every Mapper column whose name looks like a user reference is named - * by exactly one value here (or listed in notUserIdColumns). A new table fails until sorted. + * This class is one entry in OBP's table of decisions. Each value below names a database column + * that holds a user id and says which AttributionPolicy governs it, so the file as a whole is the + * table; Users.attributionOf reads it at runtime to work out which id to write. + * + * A test keeps the table complete. UserReferenceAttributionPolicyTest reflects over every Mapper in + * ToSchemify.models, picks out each field whose name looks like it holds a user id (it matches + * userid, createdby, grantedby and holder), and requires every one of them to be either named by a + * value in this file, or listed in notUserIdColumns with the reason it is not a user id after all. + * + * So adding a Mapper with a column such as UserId or CreatedByUserId will fail the build, and it + * will keep failing until you do one of those two things. That is intended rather than an + * obstacle. A column nobody has decided about silently stores the agent's id whenever a Consent is + * involved, and the first sign of trouble is a person reporting that something they created through + * an agent has vanished. Failing on the day the table is added puts the question in front of the + * developer who knows what the column is for. It has caught two columns so far, + * ApiProductSubscription.CreatedByUserId and DynamicGlossaryItem.CreatedByUserId, both added after + * this file was first written. * - * mapperClass is the fully-qualified Mapper class; fields are its field object names. + * Adding an entry does not commit you to redirecting anything. Choosing UseAuthenticatedUserId, or + * excluding the column, satisfies the test just as well as choosing UseOnBehalfOfUserId does, since + * it only insists that somebody answered the question. Whether a provider then really applies the + * policy it declared is a separate guard, OnBehalfOfOwnershipSweepTest. + * + * The constructor takes the policy; mapperClass, the fully-qualified Mapper class written as a + * string so that this file imports nothing and cannot trigger Mapper initialisation; fields, the + * names of the field objects on that Mapper; and note, the reason this policy was chosen, which + * every value carries. + * + * A value is named after the column it governs, as Table_Column, dropping the Mapped or Mapper + * prefix from the class and Lift's m prefix from the field. Where one reference governs two + * columns it is named after the first, and fields lists both. */ sealed abstract class UserReference( val policy: AttributionPolicy, @@ -76,178 +132,193 @@ sealed abstract class UserReference( val fields: List[String], val note: String = "" ) { + /** This is the value's own name, as written below, e.g. "Bank_CreatedByUserId". The tests and + * the log lines identify a reference by it. */ def name: String = getClass.getSimpleName.stripSuffix("$") } object UserReference { import AttributionPolicy._ - // ---- KeepUserId: authorisation materialisation and audit of the actor - case object AccountAccessUser extends UserReference(KeepUserId , "code.views.system.AccountAccess", List("user_fk"), "views copied from the consent JWT each request; has lifecycle GC") - case object ConsentEntitlementUser extends UserReference(KeepUserId , "code.entitlement.MappedEntitlement", List("mUserId"), "only when createdByProcess == consent_user: the consent engine copying the consent's own scope") - case object EntitlementGrantedBy extends UserReference(KeepUserId , "code.entitlement.MappedEntitlement", List("mGrantedByUserId"), "audit: who granted") - case object UserLocksUser extends UserReference(KeepUserId , "code.userlocks.UserLocks", List("UserId"), "lock the authenticated user") - case object ExpectedChallengeAnswerUser extends UserReference(KeepUserId , "code.transactionChallenge.MappedExpectedChallengeAnswer", List("ExpectedUserId"), "the challenge is answered by the initiating user") - case object ChatMessageSender extends UserReference(KeepUserId , "code.chat.ChatMessage", List("SenderUserId"), "sender = the authenticated user is truthful") - case object MetricUser extends UserReference(KeepUserId , "code.metrics.MappedMetric", List("userId"), "record both: on-behalf-of via consent_reference_id at read time") - case object MetricArchiveUser extends UserReference(KeepUserId , "code.metrics.MetricArchive", List("userId"), "as MetricUser") - case object ConnectorTraceUser extends UserReference(KeepUserId , "code.metrics.ConnectorTrace", List("userId"), "as MetricUser") - case object DynamicDataAccessGrantedBy extends UserReference(KeepUserId , "code.DynamicData.DynamicDataAccess", List("GrantedBy"), "audit: who granted") - case object AuthUserResourceUser extends UserReference(KeepUserId , "code.model.dataAccess.AuthUser", List("user"), "login row -> its own ResourceUser; not attribution") - case object OpenIDConnectTokenUser extends UserReference(KeepUserId , "code.token.OpenIDConnectToken", List("AuthUserPrimaryKey"), "token belongs to the login; not attribution") - case object UserRefreshesUser extends UserReference(KeepUserId , "code.UserRefreshes.MappedUserRefreshes", List("mUserId"), "operational: refresh of the authenticated user's own account list") + // The values are grouped by policy, because the policy is what governs behaviour. Within a + // group the order is not significant. + + // ---- UseAuthenticatedUserId: the agent's own id is the truthful thing to store here, either + // ---- because the row is an audit record of who acted, or because it carries a Consent's own + // ---- permissions and is meant to be revoked along with that Consent. + case object AccountAccess_UserFk extends UserReference(UseAuthenticatedUserId, "code.views.system.AccountAccess", List("user_fk"), "views copied from the consent JWT each request; has lifecycle GC") + case object Entitlement_UserId_ConsentScope extends UserReference(UseAuthenticatedUserId, "code.entitlement.MappedEntitlement", List("mUserId"), "only when createdByProcess == consent_user: the consent engine copying the consent's own scope") + case object Entitlement_GrantedByUserId extends UserReference(UseAuthenticatedUserId, "code.entitlement.MappedEntitlement", List("mGrantedByUserId"), "audit: who granted") + case object UserLocks_UserId extends UserReference(UseAuthenticatedUserId, "code.userlocks.UserLocks", List("UserId"), "lock the authenticated user") + case object ExpectedChallengeAnswer_ExpectedUserId extends UserReference(UseAuthenticatedUserId, "code.transactionChallenge.MappedExpectedChallengeAnswer", List("ExpectedUserId"), "consent and signing-basket authorisation: the caller IS the person authorising, so the challenge is theirs") + case object ChatMessage_SenderUserId extends UserReference(UseAuthenticatedUserId, "code.chat.ChatMessage", List("SenderUserId"), "sender = the authenticated user is truthful") + case object Metric_UserId extends UserReference(UseAuthenticatedUserId, "code.metrics.MappedMetric", List("userId"), "record both: on-behalf-of via consent_reference_id at read time") + case object MetricArchive_UserId extends UserReference(UseAuthenticatedUserId, "code.metrics.MetricArchive", List("userId"), "as Metric_UserId") + case object ConnectorTrace_UserId extends UserReference(UseAuthenticatedUserId, "code.metrics.ConnectorTrace", List("userId"), "as Metric_UserId") + case object DynamicDataAccess_GrantedBy extends UserReference(UseAuthenticatedUserId, "code.DynamicData.DynamicDataAccess", List("GrantedBy"), "audit: who granted") + case object AuthUser_User extends UserReference(UseAuthenticatedUserId, "code.model.dataAccess.AuthUser", List("user"), "login row -> its own ResourceUser; not attribution") + case object OpenIDConnectToken_AuthUserPrimaryKey extends UserReference(UseAuthenticatedUserId, "code.token.OpenIDConnectToken", List("AuthUserPrimaryKey"), "token belongs to the login; not attribution") + case object UserRefreshes_UserId extends UserReference(UseAuthenticatedUserId, "code.UserRefreshes.MappedUserRefreshes", List("mUserId"), "operational: refresh of the authenticated user's own account list") - // ---- UseOnBehalfOfUserId: ownership / attribution (record-both tables list both columns) - case object TransactionRequest extends UserReference(UseOnBehalfOfUserId, "code.transactionrequests.MappedTransactionRequest", List("mUserId", "mOnBehalfOfUserId"), "record both: mUserId = userId, mOnBehalfOfUserId = onBehalfOfUserId") - case object EntitlementUser extends UserReference(UseOnBehalfOfUserId, "code.entitlement.MappedEntitlement", List("mUserId"), "the role holder; the consent-engine case is ConsentEntitlementUser") - case object AccountHolderUser extends UserReference(UseOnBehalfOfUserId, "code.accountholders.MapperAccountHolders", List("user")) - case object UserCustomerLinkUser extends UserReference(UseOnBehalfOfUserId, "code.usercustomerlinks.MappedUserCustomerLink", List("mUserId")) - case object AccountApplicationUser extends UserReference(UseOnBehalfOfUserId, "code.accountapplication.MappedAccountApplication", List("mUserId")) - case object AccountAccessRequestRequestor extends UserReference(UseOnBehalfOfUserId, "code.accountaccessrequest.AccountAccessRequest", List("RequestorUserId")) - case object AccountAccessRequestTarget extends UserReference(UseOnBehalfOfUserId, "code.accountaccessrequest.AccountAccessRequest", List("TargetUserId"), "explicit target: a consent user named here is rejected at the endpoint") - case object AccountAccessRequestChecker extends UserReference(UseOnBehalfOfUserId, "code.accountaccessrequest.AccountAccessRequest", List("CheckerUserId")) - case object DynamicChangeRequestRequestor extends UserReference(UseOnBehalfOfUserId, "code.dynamicchangerequest.DynamicChangeRequest", List("RequestorUserId"), "maker of a dynamic-code change") - case object DynamicChangeRequestChecker extends UserReference(UseOnBehalfOfUserId, "code.dynamicchangerequest.DynamicChangeRequest", List("CheckerUserId"), "checker; must differ from the requestor") - case object EntitlementRequestUser extends UserReference(UseOnBehalfOfUserId, "code.entitlementrequest.MappedEntitlementRequest", List("mUserId")) - case object UserScopeUser extends UserReference(UseOnBehalfOfUserId, "code.scope.MappedUserScope", List("mUserId")) - case object ApiCollectionUser extends UserReference(UseOnBehalfOfUserId, "code.apicollection.ApiCollection", List("UserId")) - case object UserAttributeUser extends UserReference(UseOnBehalfOfUserId, "code.users.UserAttribute", List("UserId")) - case object UserAgreementUser extends UserReference(UseOnBehalfOfUserId, "code.users.UserAgreement", List("UserId")) - case object UserInitActionUser extends UserReference(UseOnBehalfOfUserId, "code.users.UserInitAction", List("UserId")) - case object UserAuthContextUser extends UserReference(UseOnBehalfOfUserId, "code.context.MappedUserAuthContext", List("mUserId"), "consent copies the on-behalf-of user's contexts into ConsentAuthContext separately") - case object UserAuthContextUpdateUser extends UserReference(UseOnBehalfOfUserId, "code.context.MappedUserAuthContextUpdate", List("mUserId")) - case object DynamicEntityUser extends UserReference(UseOnBehalfOfUserId, "code.dynamicEntity.DynamicEntity", List("UserId")) - case object DynamicDataUser extends UserReference(UseOnBehalfOfUserId, "code.DynamicData.DynamicData", List("UserId")) - case object DynamicDataAccessUser extends UserReference(UseOnBehalfOfUserId, "code.DynamicData.DynamicDataAccess", List("UserId")) - case object DynamicEndpointUser extends UserReference(UseOnBehalfOfUserId, "code.DynamicEndpoint.DynamicEndpoint", List("UserId")) - case object DynamicResourceDocCreator extends UserReference(UseOnBehalfOfUserId, "code.dynamicResourceDoc.DynamicResourceDoc", List("CreatedByUserId", "UpdatedByUserId")) - case object DynamicMessageDocCreator extends UserReference(UseOnBehalfOfUserId, "code.dynamicMessageDoc.DynamicMessageDoc", List("CreatedByUserId", "UpdatedByUserId")) - case object ConnectorMethodCreator extends UserReference(UseOnBehalfOfUserId, "code.connectormethod.ConnectorMethod", List("CreatedByUserId", "UpdatedByUserId")) - case object AbacRuleCreator extends UserReference(UseOnBehalfOfUserId, "code.abacrule.AbacRule", List("CreatedByUserId", "UpdatedByUserId")) - case object CounterpartyCreator extends UserReference(UseOnBehalfOfUserId, "code.metadata.counterparties.MappedCounterparty", List("mCreatedByUserId")) - case object CounterpartyWhereTagUser extends UserReference(UseOnBehalfOfUserId, "code.metadata.counterparties.MappedCounterpartyWhereTag", List("user")) - case object ApiProductSubscriptionCreator extends UserReference(UseOnBehalfOfUserId, "code.apiproductsubscription.ApiProductSubscription", List("CreatedByUserId"), "a subscription outlives the Consent that took it out") - case object DynamicGlossaryItemCreator extends UserReference(UseOnBehalfOfUserId, "code.glossaryitem.DynamicGlossaryItem", List("CreatedByUserId")) - case object BankCreator extends UserReference(UseOnBehalfOfUserId, "code.model.dataAccess.MappedBank", List("CreatedByUserId"), "creator grant already resolved at the endpoint") - case object OrganisationCreator extends UserReference(UseOnBehalfOfUserId, "code.organisation.Organisation", List("CreatedByUserId")) - case object PayeeLookupCreator extends UserReference(UseOnBehalfOfUserId, "code.payeelookup.PayeeLookup", List("CreatedByUserId")) - case object RoutingSchemeCreator extends UserReference(UseOnBehalfOfUserId, "code.routingscheme.RoutingScheme", List("CreatedByUserId")) - case object UtilityPaymentCallbackCreator extends UserReference(UseOnBehalfOfUserId, "code.utilitypayment.UtilityPaymentCallback", List("CreatedByUserId")) - case object StandingOrderUser extends UserReference(UseOnBehalfOfUserId, "code.standingorders.StandingOrder", List("UserId")) - case object DirectDebitUser extends UserReference(UseOnBehalfOfUserId, "code.directdebit.DirectDebit", List("UserId")) - case object MandateCreator extends UserReference(UseOnBehalfOfUserId, "code.mandate.Mandate", List("CreatedByUserId", "UpdatedByUserId")) - case object SignatoryPanelUsers extends UserReference(UseOnBehalfOfUserId, "code.mandate.SignatoryPanel", List("UserIds"), "list of user ids") - case object AccountWebhookCreator extends UserReference(UseOnBehalfOfUserId, "code.webhook.MappedAccountWebhook", List("mCreatedByUserId")) - case object SystemAccountNotificationWebhookCreator extends UserReference(UseOnBehalfOfUserId, "code.webhook.SystemAccountNotificationWebhook", List("CreatedByUserId")) - case object BankAccountNotificationWebhookCreator extends UserReference(UseOnBehalfOfUserId, "code.webhook.BankAccountNotificationWebhook", List("CreatedByUserId")) - case object ChatRoomCreator extends UserReference(UseOnBehalfOfUserId, "code.chat.ChatRoom", List("CreatedByUserId"), "Portal chat: a human's room") - case object ChatParticipantUser extends UserReference(UseOnBehalfOfUserId, "code.chat.Participant", List("UserId")) - case object ChatReactionUser extends UserReference(UseOnBehalfOfUserId, "code.chat.Reaction", List("UserId")) - case object ChatEmailDigestStateUser extends UserReference(UseOnBehalfOfUserId, "code.chat.ChatEmailDigestState", List("UserId")) - case object ChatMessageMentionedUsers extends UserReference(UseOnBehalfOfUserId, "code.chat.ChatMessage", List("MentionedUserIds"), "explicit targets, humans by construction") - case object CrmEventUser extends UserReference(UseOnBehalfOfUserId, "code.crm.MappedCrmEvent", List("mUserId")) - case object KycCheckUser extends UserReference(UseOnBehalfOfUserId, "code.kycchecks.MappedKycCheck", List("user"), "the customer's user") - case object KycCheckStaff extends UserReference(UseOnBehalfOfUserId, "code.kycchecks.MappedKycCheck", List("mStaffUserId"), "staff = human operator") - case object KycDocumentUser extends UserReference(UseOnBehalfOfUserId, "code.kycdocuments.MappedKycDocument", List("user")) - case object KycStatusUser extends UserReference(UseOnBehalfOfUserId, "code.kycstatuses.MappedKycStatus", List("user")) - case object SocialMediaUser extends UserReference(UseOnBehalfOfUserId, "code.socialmedia.MappedSocialMedia", List("user")) - case object CustomerMessageUser extends UserReference(UseOnBehalfOfUserId, "code.customer.MappedCustomerMessage", List("user")) - case object MeetingCustomerUser extends UserReference(UseOnBehalfOfUserId, "code.meetings.MappedMeeting", List("mCustomerUserId")) - case object MeetingStaffUser extends UserReference(UseOnBehalfOfUserId, "code.meetings.MappedMeeting", List("mStaffUserId"), "staff = human operator") - case object TagUser extends UserReference(UseOnBehalfOfUserId, "code.metadata.tags.MappedTag", List("user")) - case object WhereTagUser extends UserReference(UseOnBehalfOfUserId, "code.metadata.wheretags.MappedWhereTag", List("user")) - case object TransactionImageUser extends UserReference(UseOnBehalfOfUserId, "code.metadata.transactionimages.MappedTransactionImage", List("user")) + // ---- UseOnBehalfOfUserId: the row belongs to the person, so it must outlive the Consent that + // ---- created it. A handful of these tables keep both ids, and those name two fields. + case object TransactionRequest_UserId extends UserReference(UseOnBehalfOfUserId , "code.transactionrequests.MappedTransactionRequest", List("mUserId", "mOnBehalfOfUserId"), "record both: mUserId = userId, mOnBehalfOfUserId = onBehalfOfUserId") + case object ExpectedChallengeAnswer_ExpectedUserId_TransactionRequest extends UserReference(UseOnBehalfOfUserId, "code.transactionChallenge.MappedExpectedChallengeAnswer", List("ExpectedUserId"), "payment SCA: the challenge belongs to the human whose money moves, never to the agent that started the payment") + case object Entitlement_UserId extends UserReference(UseOnBehalfOfUserId , "code.entitlement.MappedEntitlement", List("mUserId"), "the role holder; the consent-engine case is Entitlement_UserId_ConsentScope") + case object AccountHolders_User extends UserReference(UseOnBehalfOfUserId , "code.accountholders.MapperAccountHolders", List("user"), "the human holds the account; one held by a per-consent identity strands when the consent dies") + case object UserCustomerLink_UserId extends UserReference(UseOnBehalfOfUserId , "code.usercustomerlinks.MappedUserCustomerLink", List("mUserId"), "a Customer is linked to a human; a link on an agent identity dies with its Consent") + case object AccountApplication_UserId extends UserReference(UseOnBehalfOfUserId , "code.accountapplication.MappedAccountApplication", List("mUserId"), "explicit target: user_id comes from the request and is guarded at the endpoint, so the provider redirect is unreachable -- see ON_BEHALF_OF_USER_ID_PLAN.md row 14") + case object AccountAccessRequest_RequestorUserId extends UserReference(UseOnBehalfOfUserId , "code.accountaccessrequest.AccountAccessRequest", List("RequestorUserId"), "who asked for access; the request outlives the session it was made in") + case object AccountAccessRequest_TargetUserId extends UserReference(UseOnBehalfOfUserId , "code.accountaccessrequest.AccountAccessRequest", List("TargetUserId"), "explicit target: a consent user named here is rejected at the endpoint") + case object AccountAccessRequest_CheckerUserId extends UserReference(UseOnBehalfOfUserId , "code.accountaccessrequest.AccountAccessRequest", List("CheckerUserId"), "who approved; maker/checker evidence has to name a human") + case object DynamicChangeRequest_RequestorUserId extends UserReference(UseOnBehalfOfUserId , "code.dynamicchangerequest.DynamicChangeRequest", List("RequestorUserId"), "maker of a dynamic-code change") + case object DynamicChangeRequest_CheckerUserId extends UserReference(UseOnBehalfOfUserId , "code.dynamicchangerequest.DynamicChangeRequest", List("CheckerUserId"), "checker; must differ from the requestor") + case object EntitlementRequest_UserId extends UserReference(UseOnBehalfOfUserId , "code.entitlementrequest.MappedEntitlementRequest", List("mUserId"), "who asked for the role; the grant that follows lands on a human") + case object UserScope_UserId extends UserReference(UseOnBehalfOfUserId , "code.scope.MappedUserScope", List("mUserId"), "the scope holder") + case object ApiCollection_UserId extends UserReference(UseOnBehalfOfUserId , "code.apicollection.ApiCollection", List("UserId"), "the user's own saved collection, created through POST /my/api-collections") + case object UserAttribute_UserId extends UserReference(UseOnBehalfOfUserId , "code.users.UserAttribute", List("UserId"), "the user's own attribute, created through the /my/ endpoints") + case object UserAgreement_UserId extends UserReference(UseOnBehalfOfUserId , "code.users.UserAgreement", List("UserId"), "the user's own acceptance of terms") + case object UserInitAction_UserId extends UserReference(UseOnBehalfOfUserId , "code.users.UserInitAction", List("UserId"), "the user's own onboarding action") + case object UserAuthContext_UserId extends UserReference(UseOnBehalfOfUserId , "code.context.MappedUserAuthContext", List("mUserId"), "consent copies the on-behalf-of user's contexts into ConsentAuthContext separately") + case object UserAuthContextUpdate_UserId extends UserReference(UseOnBehalfOfUserId , "code.context.MappedUserAuthContextUpdate", List("mUserId"), "as UserAuthContext_UserId") + case object DynamicEntity_UserId extends UserReference(UseOnBehalfOfUserId , "code.dynamicEntity.DynamicEntity", List("UserId"), "the definition's creator; a definition outlives the Consent that created it") + case object DynamicData_UserId extends UserReference(UseOnBehalfOfUserId , "code.DynamicData.DynamicData", List("UserId"), "personal rows, and the one reference where the redirect MUST be symmetric: MapppedDynamicDataProvider resolves on save/update/get/delete alike, because a row keyed by this column on both sides is otherwise written by an agent and then invisible to it") + case object DynamicDataAccess_UserId extends UserReference(UseOnBehalfOfUserId , "code.DynamicData.DynamicDataAccess", List("UserId"), "row-level ACL. Deliberately still on the consent user today, because the bootstrap grant and the allows check have to agree with each other -- rows strand, nothing leaks. A later Phase 2 row; see the plan") + case object DynamicEndpoint_UserId extends UserReference(UseOnBehalfOfUserId , "code.DynamicEndpoint.DynamicEndpoint", List("UserId"), "the dynamic endpoint's creator; outlives the Consent") + case object DynamicResourceDoc_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.dynamicResourceDoc.DynamicResourceDoc", List("CreatedByUserId", "UpdatedByUserId"), "and UpdatedByUserId; a dynamic artefact outlives the Consent that created it") + case object DynamicMessageDoc_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.dynamicMessageDoc.DynamicMessageDoc", List("CreatedByUserId", "UpdatedByUserId"), "and UpdatedByUserId; a dynamic artefact outlives the Consent that created it") + case object ConnectorMethod_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.connectormethod.ConnectorMethod", List("CreatedByUserId", "UpdatedByUserId"), "and UpdatedByUserId; a dynamic artefact outlives the Consent that created it") + case object AbacRule_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.abacrule.AbacRule", List("CreatedByUserId", "UpdatedByUserId"), "and UpdatedByUserId; an access rule outlives the Consent that created it") + case object Counterparty_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.metadata.counterparties.MappedCounterparty", List("mCreatedByUserId", "mCreatedByOnBehalfOfUserId"), "record both: a counterparty controls where money may be sent, so the actor stays on mCreatedByUserId (and is published as created_by_user_id) while the human goes to mCreatedByOnBehalfOfUserId") + case object CounterpartyWhereTag_User extends UserReference(UseOnBehalfOfUserId , "code.metadata.counterparties.MappedCounterpartyWhereTag", List("user"), "who tagged the counterparty's location") + case object ApiProductSubscription_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.apiproductsubscription.ApiProductSubscription", List("CreatedByUserId"), "a subscription outlives the Consent that took it out") + case object DynamicGlossaryItem_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.glossaryitem.DynamicGlossaryItem", List("CreatedByUserId"), "outlives the Consent that created it") + case object Bank_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.model.dataAccess.MappedBank", List("CreatedByUserId"), "creator grant already resolved at the endpoint") + case object Organisation_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.organisation.Organisation", List("CreatedByUserId"), "outlives the Consent that created it") + case object PayeeLookup_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.payeelookup.PayeeLookup", List("CreatedByUserId"), "outlives the Consent that created it") + case object RoutingScheme_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.routingscheme.RoutingScheme", List("CreatedByUserId"), "outlives the Consent that created it") + case object UtilityPaymentCallback_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.utilitypayment.UtilityPaymentCallback", List("CreatedByUserId"), "outlives the Consent that created it") + case object StandingOrder_UserId extends UserReference(UseOnBehalfOfUserId , "code.standingorders.StandingOrder", List("UserId"), "the payer; a standing order keeps executing long after any Consent expires") + case object DirectDebit_UserId extends UserReference(UseOnBehalfOfUserId , "code.directdebit.DirectDebit", List("UserId"), "the payer; a mandate keeps executing long after any Consent expires") + case object Mandate_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.mandate.Mandate", List("CreatedByUserId", "UpdatedByUserId"), "and UpdatedByUserId; a mandate authorises payment and outlives the Consent that created it") + case object SignatoryPanel_UserIds extends UserReference(UseOnBehalfOfUserId , "code.mandate.SignatoryPanel", List("UserIds"), "list of user ids") + case object AccountWebhook_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.webhook.MappedAccountWebhook", List("mCreatedByUserId"), "published as created_by_user_id on v3.1.0 and v4.0.0; nothing reads it as an ownership key, so the human is not locked out -- but a webhook outlives the Consent that made it and keeps sending account events, so the human behind it should be recorded. Record-both, like Counterparty. See todo/webhook_attribution.md") + case object SystemAccountNotificationWebhook_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.webhook.SystemAccountNotificationWebhook", List("CreatedByUserId"), "as AccountWebhook_CreatedByUserId") + case object BankAccountNotificationWebhook_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.webhook.BankAccountNotificationWebhook", List("CreatedByUserId"), "as AccountWebhook_CreatedByUserId") + case object ChatRoom_CreatedByUserId extends UserReference(UseOnBehalfOfUserId , "code.chat.ChatRoom", List("CreatedByUserId"), "Portal chat: a human's room") + case object Participant_UserId extends UserReference(UseOnBehalfOfUserId , "code.chat.Participant", List("UserId"), "the person in the room") + case object Reaction_UserId extends UserReference(UseOnBehalfOfUserId , "code.chat.Reaction", List("UserId"), "the person who reacted") + case object ChatEmailDigestState_UserId extends UserReference(UseOnBehalfOfUserId , "code.chat.ChatEmailDigestState", List("UserId"), "the person the digest is for") + case object ChatMessage_MentionedUserIds extends UserReference(UseOnBehalfOfUserId , "code.chat.ChatMessage", List("MentionedUserIds"), "explicit targets, humans by construction") + case object CrmEvent_UserId extends UserReference(UseOnBehalfOfUserId , "code.crm.MappedCrmEvent", List("mUserId"), "the user the event concerns") + case object KycCheck_User extends UserReference(UseOnBehalfOfUserId , "code.kycchecks.MappedKycCheck", List("user"), "the customer's user") + case object KycCheck_StaffUserId extends UserReference(UseOnBehalfOfUserId , "code.kycchecks.MappedKycCheck", List("mStaffUserId"), "staff = human operator") + case object KycDocument_User extends UserReference(UseOnBehalfOfUserId , "code.kycdocuments.MappedKycDocument", List("user"), "the customer's user") + case object KycStatus_User extends UserReference(UseOnBehalfOfUserId , "code.kycstatuses.MappedKycStatus", List("user"), "the customer's user") + case object SocialMedia_User extends UserReference(UseOnBehalfOfUserId , "code.socialmedia.MappedSocialMedia", List("user"), "the customer's user") + case object CustomerMessage_User extends UserReference(UseOnBehalfOfUserId , "code.customer.MappedCustomerMessage", List("user"), "the customer's user") + case object Meeting_CustomerUserId extends UserReference(UseOnBehalfOfUserId , "code.meetings.MappedMeeting", List("mCustomerUserId"), "the customer side of the meeting") + case object Meeting_StaffUserId extends UserReference(UseOnBehalfOfUserId , "code.meetings.MappedMeeting", List("mStaffUserId"), "staff = human operator") + case object Tag_User extends UserReference(UseOnBehalfOfUserId , "code.metadata.tags.MappedTag", List("user"), "the author of the annotation; it outlives the Consent") + case object WhereTag_User extends UserReference(UseOnBehalfOfUserId , "code.metadata.wheretags.MappedWhereTag", List("user"), "the author of the annotation; it outlives the Consent") + case object TransactionImage_User extends UserReference(UseOnBehalfOfUserId , "code.metadata.transactionimages.MappedTransactionImage", List("user"), "the author of the annotation; it outlives the Consent") - // ---- Reject: a consent user must not do this at all - case object ConsentCreator extends UserReference(Reject , "code.consent.MappedConsent", List("mUserId"), "a consent user creating a consent = nested delegation") - case object OAuthConsumerCreator extends UserReference(Reject , "code.model.Consumer", List("createdByUserId"), "credentials outlive the consent") - case object OAuthTokenUser extends UserReference(Reject , "code.model.Token", List("userForeignKey"), "credentials outlive the consent") + // ---- Reject: an agent must not do this at all, because what it would create outlives the + // ---- Consent and would let the delegation extend itself. + case object Consent_UserId extends UserReference(Reject , "code.consent.MappedConsent", List("mUserId"), "a consent user creating a consent = nested delegation") + case object Consumer_CreatedByUserId extends UserReference(Reject , "code.model.Consumer", List("createdByUserId"), "credentials outlive the consent") + case object Token_UserForeignKey extends UserReference(Reject , "code.model.Token", List("userForeignKey"), "credentials outlive the consent") - /** Every reference; the frozen test walks this. */ + /** This lists every reference declared above. The tests walk this list rather than reflecting + * over the file, so a value declared above but left out here is invisible to them; the two have + * to be kept in step by hand. */ lazy val all: List[UserReference] = List( - AccountAccessUser, - ConsentEntitlementUser, - EntitlementGrantedBy, - UserLocksUser, - ExpectedChallengeAnswerUser, - ChatMessageSender, - MetricUser, - MetricArchiveUser, - ConnectorTraceUser, - DynamicDataAccessGrantedBy, - AuthUserResourceUser, - OpenIDConnectTokenUser, - UserRefreshesUser, - TransactionRequest, - EntitlementUser, - AccountHolderUser, - UserCustomerLinkUser, - AccountApplicationUser, - AccountAccessRequestRequestor, - AccountAccessRequestTarget, - AccountAccessRequestChecker, - DynamicChangeRequestRequestor, - DynamicChangeRequestChecker, - EntitlementRequestUser, - UserScopeUser, - ApiCollectionUser, - UserAttributeUser, - UserAgreementUser, - UserInitActionUser, - UserAuthContextUser, - UserAuthContextUpdateUser, - DynamicEntityUser, - DynamicDataUser, - DynamicDataAccessUser, - DynamicEndpointUser, - DynamicResourceDocCreator, - DynamicMessageDocCreator, - ConnectorMethodCreator, - AbacRuleCreator, - CounterpartyCreator, - CounterpartyWhereTagUser, - ApiProductSubscriptionCreator, - DynamicGlossaryItemCreator, - BankCreator, - OrganisationCreator, - PayeeLookupCreator, - RoutingSchemeCreator, - UtilityPaymentCallbackCreator, - StandingOrderUser, - DirectDebitUser, - MandateCreator, - SignatoryPanelUsers, - AccountWebhookCreator, - SystemAccountNotificationWebhookCreator, - BankAccountNotificationWebhookCreator, - ChatRoomCreator, - ChatParticipantUser, - ChatReactionUser, - ChatEmailDigestStateUser, - ChatMessageMentionedUsers, - CrmEventUser, - KycCheckUser, - KycCheckStaff, - KycDocumentUser, - KycStatusUser, - SocialMediaUser, - CustomerMessageUser, - MeetingCustomerUser, - MeetingStaffUser, - TagUser, - WhereTagUser, - TransactionImageUser, - ConsentCreator, - OAuthConsumerCreator, - OAuthTokenUser + AccountAccess_UserFk, + Entitlement_UserId_ConsentScope, + Entitlement_GrantedByUserId, + UserLocks_UserId, + ExpectedChallengeAnswer_ExpectedUserId, + ExpectedChallengeAnswer_ExpectedUserId_TransactionRequest, + ChatMessage_SenderUserId, + Metric_UserId, + MetricArchive_UserId, + ConnectorTrace_UserId, + DynamicDataAccess_GrantedBy, + AuthUser_User, + OpenIDConnectToken_AuthUserPrimaryKey, + UserRefreshes_UserId, + TransactionRequest_UserId, + Entitlement_UserId, + AccountHolders_User, + UserCustomerLink_UserId, + AccountApplication_UserId, + AccountAccessRequest_RequestorUserId, + AccountAccessRequest_TargetUserId, + AccountAccessRequest_CheckerUserId, + DynamicChangeRequest_RequestorUserId, + DynamicChangeRequest_CheckerUserId, + EntitlementRequest_UserId, + UserScope_UserId, + ApiCollection_UserId, + UserAttribute_UserId, + UserAgreement_UserId, + UserInitAction_UserId, + UserAuthContext_UserId, + UserAuthContextUpdate_UserId, + DynamicEntity_UserId, + DynamicData_UserId, + DynamicDataAccess_UserId, + DynamicEndpoint_UserId, + DynamicResourceDoc_CreatedByUserId, + DynamicMessageDoc_CreatedByUserId, + ConnectorMethod_CreatedByUserId, + AbacRule_CreatedByUserId, + Counterparty_CreatedByUserId, + CounterpartyWhereTag_User, + ApiProductSubscription_CreatedByUserId, + DynamicGlossaryItem_CreatedByUserId, + Bank_CreatedByUserId, + Organisation_CreatedByUserId, + PayeeLookup_CreatedByUserId, + RoutingScheme_CreatedByUserId, + UtilityPaymentCallback_CreatedByUserId, + StandingOrder_UserId, + DirectDebit_UserId, + Mandate_CreatedByUserId, + SignatoryPanel_UserIds, + AccountWebhook_CreatedByUserId, + SystemAccountNotificationWebhook_CreatedByUserId, + BankAccountNotificationWebhook_CreatedByUserId, + ChatRoom_CreatedByUserId, + Participant_UserId, + Reaction_UserId, + ChatEmailDigestState_UserId, + ChatMessage_MentionedUserIds, + CrmEvent_UserId, + KycCheck_User, + KycCheck_StaffUserId, + KycDocument_User, + KycStatus_User, + SocialMedia_User, + CustomerMessage_User, + Meeting_CustomerUserId, + Meeting_StaffUserId, + Tag_User, + WhereTag_User, + TransactionImage_User, + Consent_UserId, + Consumer_CreatedByUserId, + Token_UserForeignKey ) - /** Mapper fields the frozen test's name pattern matches but which are not user ids. + /** This lists the columns whose names look like user ids to UserReferenceAttributionPolicyTest + * but which do not in fact hold one, so no policy applies. Each entry is the Mapper class, the + * field, and why it is excluded. * - * Only columns the pattern actually catches belong here; UserReferenceAttributionPolicyTest fails - * on an entry that matches nothing, because an inert exclusion looks like cover it is not giving. - * (Removed for that reason: AccountAccessRequest.CheckerComment, DynamicChangeRequest.CheckerComment, - * MappedKycCheck.mStaffName, MappedMeeting.mStaffToken -- all free text, and none of them matched.) */ + * Only columns the test's pattern really catches belong here. It fails on an entry that matches + * nothing, because an exclusion covering no column reads as cover it is not giving. Four were + * removed for that reason: AccountAccessRequest.CheckerComment, DynamicChangeRequest.CheckerComment, + * MappedKycCheck.mStaffName and MappedMeeting.mStaffToken, all free text and none of them matched. */ val notUserIdColumns: List[(String, String, String)] = List( ("code.model.dataAccess.MappedBankAccount", "holder", "free-text holder name"), ("code.transaction.MappedTransaction", "counterpartyAccountHolder", "free-text name"), @@ -257,5 +328,7 @@ object UserReference { ("code.model.dataAccess.ResourceUser", "CreatedByUserInvitationId", "invitation id") ) + /** This returns every reference that carries the given policy. The sweep tests use it to ask, + * for instance, which columns are supposed to belong to the person rather than the agent. */ def byPolicy(p: AttributionPolicy): List[UserReference] = all.filter(_.policy == p) } diff --git a/obp-api/src/main/scala/code/users/Users.scala b/obp-api/src/main/scala/code/users/Users.scala index 2e7c870840..00a969ebd7 100644 --- a/obp-api/src/main/scala/code/users/Users.scala +++ b/obp-api/src/main/scala/code/users/Users.scala @@ -90,12 +90,13 @@ trait Users { def getUsers(queryParams: List[OBPQueryParam]): Future[List[(ResourceUser, Box[List[Entitlement]], Option[List[UserAgreement]])]] /** - * Get users via a Doobie-based SQL JOIN across resourceuser, authuser and - * mappedbadloginattempt. Returns pre-joined rows plus the user's entitlements - * and most-recent-per-type agreements, fetched in batch. + * This searches for users with a single SQL join, written in Doobie, across the resourceuser, + * authuser and mappedbadloginattempt tables. It returns the joined rows, and with each row the + * user's entitlements and their most recent agreement of each type, all fetched in batches + * rather than one query per user. * - * Supported OBPQueryParam filters: OBPProvider, OBPUsername, OBPIsDeleted, - * OBPLockedStatus, OBPRoleName, OBPBankId, OBPLimit, OBPOffset. + * It understands these OBPQueryParam filters: OBPProvider, OBPUsername, OBPIsDeleted, + * OBPLockedStatus, OBPRoleName, OBPBankId, OBPLimit and OBPOffset. */ def getUsersV600F(queryParams: List[OBPQueryParam]) : Future[List[(DoobieUserQueries.UserSearchRow, List[code.entitlement.Entitlement], List[UserAgreement])]] @@ -114,26 +115,39 @@ trait Users { // ---- on-behalf-of resolution (ON_BEHALF_OF_USER_ID_PLAN.md, Phase 1) ---------------------- - /** The on-behalf-of user id for `userId`. - * consent user -> the consent's userId (read at call time: BG/UK consents bind their human - * only at authorisation, so it is never copied at creation) - * original user -> userId unchanged - * Fails closed: unknown user / dangling consent id / consent with no human yet -> userId (+ WARN). - * Invariant: the result is an original user (isOriginalUser); a consent whose user is itself a - * consent user is a data bug -> WARN + Failure, the one case that cannot fall back. - * Takes only the id on purpose: nothing request-asserted (body/header/query) can steer it. */ - def onBehalfOfUserIdOf(userId: String): Box[String] - - /** True when `userId` acts for itself and may own durable state. */ - def actsForSelf(userId: String): Boolean = onBehalfOfUserIdOf(userId).exists(_ == userId) - - /** Attribution for writing the column(s) `ref` names as `userId`. Applies `ref.policy`: - * KeepUserId -> Full(userId as both), resolver not consulted - * UseOnBehalfOfUserId -> Full(resolved), WARN naming `ref` when delegated - * Reject -> Full if `userId` acts for itself, else Failure(InvalidUserId ...) */ + /** This resolves the caller's user id to the user id of the person they are acting for. + * + * When the caller is a consent user, the answer is the user named by its Consent, looked up at + * the moment of the call rather than copied when the consent user was made: a Berlin Group or UK + * consent does not know its person until the person authorises it. When the caller is an ordinary + * user, the answer is the id it was given. + * + * It fails closed, meaning that where it cannot find an answer it keeps the caller's own id and + * logs a warning. That covers an unknown user, a consent id pointing at nothing, and a consent + * with no person attached yet. There is one case it will not fall back on: a Consent whose user + * is itself a consent user breaks the rule that resolution is a single hop, so rather than guess + * it warns and returns a Failure. + * + * It takes the id alone, and deliberately so. Nothing the caller asserts in the request — a body, + * a header, a query parameter — can influence whom the write is attributed to. */ + def resolveOnBehalfOfUserId(userId: String): Box[String] + + /** This is true when the given user acts only for itself, and may therefore own rows that + * outlive any Consent. It is false for an agent acting for somebody else. */ + def actsForSelf(userId: String): Boolean = resolveOnBehalfOfUserId(userId).exists(_ == userId) + + /** This works out whose user id to write into the column that `ref` names, for a call made by + * `userId`, and returns both candidate ids in an Attribution. + * + * What it does depends on the reference's policy. Under UseAuthenticatedUserId it returns the + * caller as both ids without resolving anything. Under UseOnBehalfOfUserId it resolves the + * person the caller acts for, and logs a warning naming the reference whenever the two differ, + * so every delegated write leaves a trace. Under Reject it succeeds only if the caller acts for + * itself, and otherwise returns a Failure that the endpoint turns into a 400. */ def attributionOf(userId: String, ref: UserReference): Box[Attribution] - /** Convenience for single-column writers: the one value to store. */ + /** This is a shortcut for code filling a single column: it asks attributionOf the same question + * and hands back just the one id to store, rather than the whole Attribution. */ def attributedUserId(userId: String, ref: UserReference): Box[String] = attributionOf(userId, ref).map(_.userIdToStore) def saveResourceUser(resourceUser: ResourceUser) : Box[ResourceUser] diff --git a/obp-api/src/test/scala/code/api/sweep/ExplicitTargetConsentUserSweepTest.scala b/obp-api/src/test/scala/code/api/sweep/ExplicitTargetConsentUserSweepTest.scala index 30af8c1702..77c7057712 100644 --- a/obp-api/src/test/scala/code/api/sweep/ExplicitTargetConsentUserSweepTest.scala +++ b/obp-api/src/test/scala/code/api/sweep/ExplicitTargetConsentUserSweepTest.scala @@ -55,7 +55,9 @@ import org.scalatest.Tag import scala.io.Source /** - * Phase 3 of ON_BEHALF_OF_USER_ID_PLAN.md: an EXPLICIT user id naming a consent user is refused. + * This suite checks that a request naming a consent user explicitly, by putting its id in the body + * or the path, is refused rather than quietly rewritten. It is Phase 3 of + * ON_BEHALF_OF_USER_ID_PLAN.md. * * The plan splits every user-reference column two ways. Where the caller means "me", the provider * quietly redirects the write to the human the caller acts for (Phase 2) -- an agent that creates diff --git a/obp-api/src/test/scala/code/api/sweep/OnBehalfOfOwnershipSweepTest.scala b/obp-api/src/test/scala/code/api/sweep/OnBehalfOfOwnershipSweepTest.scala index 155cf97be4..e6177f39b6 100644 --- a/obp-api/src/test/scala/code/api/sweep/OnBehalfOfOwnershipSweepTest.scala +++ b/obp-api/src/test/scala/code/api/sweep/OnBehalfOfOwnershipSweepTest.scala @@ -51,9 +51,11 @@ import org.scalatest.Tag import scala.io.Source /** - * Phase 4 item 3 of ON_BEHALF_OF_USER_ID_PLAN.md: what a consent user creates is owned by the human. + * This suite checks that what an agent creates ends up owned by the person it acts for, rather + * than by the throwaway identity the Consent gave it. It is item 3 of Phase 4 in + * ON_BEHALF_OF_USER_ID_PLAN.md. * - * The complement of ExplicitTargetConsentUserSweepTest. That suite covers the half of the doctrine + * It is the complement of ExplicitTargetConsentUserSweepTest. That suite covers the half of the doctrine * where the caller names a target by id and is refused; this one covers the half where the caller * means "me" and the provider silently redirects the write to the human the Consent was granted by. * The caller here IS a consent user, driven by a real Consent JWT minted over the wire. @@ -62,9 +64,12 @@ import scala.io.Source * * The plan says: "call every UseOnBehalfOfUserId create endpoint with the consent JWT; assert no * row in any such table references the consent user's id". Taken literally that suite is red on the - * day it is written and stays red for months: of the 53 UseOnBehalfOfUserId references, - * FIVE are wired today (TransactionRequest, EntitlementUser, AccountHolderUser, - * UserCustomerLinkUser, DynamicEntityUser/DynamicDataUser -- Phase 2 rows 1 to 3). A permanently + * day it is written and stays red for months: of the 62 references this scenario covers (the 59 + * UseOnBehalfOfUserId ones and the 3 Reject ones), EIGHT are wired today (TransactionRequest_UserId, + * Entitlement_UserId, AccountHolders_User, UserCustomerLink_UserId, + * DynamicEntity_UserId/DynamicData_UserId, Bank_CreatedByUserId, Counterparty_CreatedByUserId -- + * Phase 2 rows 1 to 5), and the other 54 are the notYetWired list below. + * A permanently * red suite is one people learn to ignore, which is the same reasoning AuthSweepTest's * expectedAuthDeviation records for its own two entries. * @@ -81,7 +86,7 @@ import scala.io.Source * reference the consent user -- except in the tables the inventory says are not wired yet. * * 3. THE SCAN ITSELF WORKS (runtime, negative control). The same consent user creates an API - * collection, whose reference (ApiCollectionUser) is NOT wired. The scan must find that row. + * collection, whose reference (ApiCollection_UserId) is NOT wired. The scan must find that row. * Without this, scenario 2 passes just as happily if the scan is silently looking at nothing -- * which is the failure mode a table-driven assertion has and a hand-written one does not. */ @@ -91,12 +96,17 @@ class OnBehalfOfOwnershipSweepTest extends ServerSetupWithTestData with DefaultU implicit val runtime: IORuntime = IORuntime.global + private val webhookDeferred = + "deferred 2026-09-16, not mechanical: agreed as record-both (like Counterparty), but the webhook " + + "code has two dead paths already and is not to be touched in a hurry. todo/webhook_attribution.md." + private val mechanicalBatch = "Phase 2 mechanical batch: the provider does not call attributionOf yet, so a consent user's " + "row is stored against the consent user and dies with the Consent." /** - * References whose provider does not consult the resolver yet (Phase 2 is incremental). + * This lists the references whose provider does not consult the resolver yet, because Phase 2 is + * being done a table at a time rather than all at once. * * Written out by hand rather than derived from "what main does not reference". A derived list * would agree with reality by construction: the undecided check could never fail, because @@ -108,66 +118,65 @@ class OnBehalfOfOwnershipSweepTest extends ServerSetupWithTestData with DefaultU * This list may only shrink. */ private val notYetWired: Map[String, String] = Map( - "AccountApplicationUser" -> mechanicalBatch, - "AccountAccessRequestRequestor" -> mechanicalBatch, - "AccountAccessRequestChecker" -> mechanicalBatch, - "DynamicChangeRequestRequestor" -> mechanicalBatch, - "DynamicChangeRequestChecker" -> mechanicalBatch, - "EntitlementRequestUser" -> mechanicalBatch, - "UserScopeUser" -> mechanicalBatch, - "ApiCollectionUser" -> mechanicalBatch, - "UserAttributeUser" -> mechanicalBatch, - "UserAgreementUser" -> mechanicalBatch, - "UserInitActionUser" -> mechanicalBatch, - "UserAuthContextUser" -> mechanicalBatch, - "UserAuthContextUpdateUser" -> mechanicalBatch, - "DynamicDataAccessUser" -> mechanicalBatch, - "DynamicEndpointUser" -> mechanicalBatch, - "DynamicResourceDocCreator" -> mechanicalBatch, - "DynamicMessageDocCreator" -> mechanicalBatch, - "ConnectorMethodCreator" -> mechanicalBatch, - "AbacRuleCreator" -> mechanicalBatch, - "CounterpartyCreator" -> mechanicalBatch, - "CounterpartyWhereTagUser" -> mechanicalBatch, - "ApiProductSubscriptionCreator" -> mechanicalBatch, - "DynamicGlossaryItemCreator" -> mechanicalBatch, - "OrganisationCreator" -> mechanicalBatch, - "PayeeLookupCreator" -> mechanicalBatch, - "RoutingSchemeCreator" -> mechanicalBatch, - "UtilityPaymentCallbackCreator" -> mechanicalBatch, - "StandingOrderUser" -> mechanicalBatch, - "DirectDebitUser" -> mechanicalBatch, - "MandateCreator" -> mechanicalBatch, - "SignatoryPanelUsers" -> mechanicalBatch, - "AccountWebhookCreator" -> mechanicalBatch, - "SystemAccountNotificationWebhookCreator" -> mechanicalBatch, - "BankAccountNotificationWebhookCreator" -> mechanicalBatch, - "ChatRoomCreator" -> mechanicalBatch, - "ChatParticipantUser" -> mechanicalBatch, - "ChatReactionUser" -> mechanicalBatch, - "ChatEmailDigestStateUser" -> mechanicalBatch, - "ChatMessageMentionedUsers" -> mechanicalBatch, - "CrmEventUser" -> mechanicalBatch, - "KycCheckUser" -> mechanicalBatch, - "KycCheckStaff" -> mechanicalBatch, - "KycDocumentUser" -> mechanicalBatch, - "KycStatusUser" -> mechanicalBatch, - "SocialMediaUser" -> mechanicalBatch, - "CustomerMessageUser" -> mechanicalBatch, - "MeetingCustomerUser" -> mechanicalBatch, - "MeetingStaffUser" -> mechanicalBatch, - "TagUser" -> mechanicalBatch, - "WhereTagUser" -> mechanicalBatch, - "TransactionImageUser" -> mechanicalBatch, - "AccountAccessRequestTarget" -> + "AccountApplication_UserId" -> mechanicalBatch, + "AccountAccessRequest_RequestorUserId" -> mechanicalBatch, + "AccountAccessRequest_CheckerUserId" -> mechanicalBatch, + "DynamicChangeRequest_RequestorUserId" -> mechanicalBatch, + "DynamicChangeRequest_CheckerUserId" -> mechanicalBatch, + "EntitlementRequest_UserId" -> mechanicalBatch, + "UserScope_UserId" -> mechanicalBatch, + "ApiCollection_UserId" -> mechanicalBatch, + "UserAttribute_UserId" -> mechanicalBatch, + "UserAgreement_UserId" -> mechanicalBatch, + "UserInitAction_UserId" -> mechanicalBatch, + "UserAuthContext_UserId" -> mechanicalBatch, + "UserAuthContextUpdate_UserId" -> mechanicalBatch, + "DynamicDataAccess_UserId" -> mechanicalBatch, + "DynamicEndpoint_UserId" -> mechanicalBatch, + "DynamicResourceDoc_CreatedByUserId" -> mechanicalBatch, + "DynamicMessageDoc_CreatedByUserId" -> mechanicalBatch, + "ConnectorMethod_CreatedByUserId" -> mechanicalBatch, + "AbacRule_CreatedByUserId" -> mechanicalBatch, + "CounterpartyWhereTag_User" -> mechanicalBatch, + "ApiProductSubscription_CreatedByUserId" -> mechanicalBatch, + "DynamicGlossaryItem_CreatedByUserId" -> mechanicalBatch, + "Organisation_CreatedByUserId" -> mechanicalBatch, + "PayeeLookup_CreatedByUserId" -> mechanicalBatch, + "RoutingScheme_CreatedByUserId" -> mechanicalBatch, + "UtilityPaymentCallback_CreatedByUserId" -> mechanicalBatch, + "StandingOrder_UserId" -> mechanicalBatch, + "DirectDebit_UserId" -> mechanicalBatch, + "Mandate_CreatedByUserId" -> mechanicalBatch, + "SignatoryPanel_UserIds" -> mechanicalBatch, + "AccountWebhook_CreatedByUserId" -> webhookDeferred, + "SystemAccountNotificationWebhook_CreatedByUserId" -> webhookDeferred, + "BankAccountNotificationWebhook_CreatedByUserId" -> webhookDeferred, + "ChatRoom_CreatedByUserId" -> mechanicalBatch, + "Participant_UserId" -> mechanicalBatch, + "Reaction_UserId" -> mechanicalBatch, + "ChatEmailDigestState_UserId" -> mechanicalBatch, + "ChatMessage_MentionedUserIds" -> mechanicalBatch, + "CrmEvent_UserId" -> mechanicalBatch, + "KycCheck_User" -> mechanicalBatch, + "KycCheck_StaffUserId" -> mechanicalBatch, + "KycDocument_User" -> mechanicalBatch, + "KycStatus_User" -> mechanicalBatch, + "SocialMedia_User" -> mechanicalBatch, + "CustomerMessage_User" -> mechanicalBatch, + "Meeting_CustomerUserId" -> mechanicalBatch, + "Meeting_StaffUserId" -> mechanicalBatch, + "Tag_User" -> mechanicalBatch, + "WhereTag_User" -> mechanicalBatch, + "TransactionImage_User" -> mechanicalBatch, + "AccountAccessRequest_TargetUserId" -> ("explicit target, so the endpoint refuses a consent user rather than redirecting -- covered " + "by ExplicitTargetConsentUserSweepTest. The provider redirect is unreachable from the API."), - "ConsentCreator" -> + "Consent_UserId" -> ("Reject, not yet enforced: a consent user can still create a Consent (nested delegation). " + "attributionOf already returns Failure for it -- AgentDelegationTest pins that -- but no " + "caller consults it. ON_BEHALF_OF_USER_ID_PLAN.md Phase 3, still open."), - "OAuthConsumerCreator" -> "Reject, not yet enforced: no caller consults attributionOf.", - "OAuthTokenUser" -> "Reject, not yet enforced: no caller consults attributionOf." + "Consumer_CreatedByUserId" -> "Reject, not yet enforced: no caller consults attributionOf.", + "Token_UserForeignKey" -> "Reject, not yet enforced: no caller consults attributionOf." ) // ── which references main actually uses ──────────────────────────────────── @@ -184,36 +193,64 @@ class OnBehalfOfOwnershipSweepTest extends ServerSetupWithTestData with DefaultU } /** - * Reference names mentioned anywhere in main outside the policy file itself. + * This is the set of reference names mentioned anywhere in the main sources, outside the policy + * file itself. * * "Mentioned" is weaker than "applied correctly" -- a provider could name the reference and still * store the wrong id. That stronger question is what scenario 2 answers at runtime; this one only * has to separate "somebody has been here" from "nobody has", which is exactly what the ratchet * needs to know. + * + * Comments are stripped first, and that is load-bearing rather than tidiness. Documenting a + * reference is not wiring it: on 2026-09-15 a doc comment on LiftUsers.attributionOf explaining + * WHY a consent user cannot create a Consent named `UserReference.Consent_UserId` in prose, and + * the ratchet promptly reported the (entirely correct) Consent_UserId exemption as stale. A guard + * that fires when someone writes a comment teaches people to write fewer comments, and worse, + * lets a reference be marked wired without a single call site. + * + * The pattern has to admit `_`: reference names carry one at the table/column boundary + * (Bank_CreatedByUserId). Without it the match stops at the table half, which is not a name in + * `all`, so the intersect drops it and every wired reference reads as unwired. */ private lazy val usedInMain: Set[String] = { val names = UserReference.all.map(_.name).toSet - val pattern = """UserReference\.([A-Za-z]+)""".r + val pattern = """UserReference\.([A-Za-z_]+)""".r scalaFilesUnder(mainSourceRoot) .filterNot(_.getPath.endsWith("code/users/UserReference.scala")) .flatMap { f => val source = Source.fromFile(f, "UTF-8") - try pattern.findAllMatchIn(source.mkString).map(_.group(1)).toList finally source.close() + try pattern.findAllMatchIn(withoutComments(source.mkString)).map(_.group(1)).toList + finally source.close() } .toSet .intersect(names) } + /** + * This returns the given Scala source with its block, scaladoc and line comments blanked out, so + * that a name appearing only in a comment is not mistaken for a use of it. + * + * Deliberately a pair of regexes rather than a lexer: the only thing read out of the result is + * `UserReference.Xxx`, so the one way this can be wrong -- a `//` inside a string literal + * truncating the rest of that line -- would have to be followed by a UserReference mention in + * the same literal to matter, which no call site looks like. + */ + private def withoutComments(source: String): String = + source + .replaceAll("(?s)/\\*.*?\\*/", " ") + .replaceAll("//[^\n]*", " ") + // ── the leak scan ────────────────────────────────────────────────────────── private lazy val metaByClassName: Map[String, MetaMapper[_]] = ToSchemify.models.map(meta => meta.getClass.getName.stripSuffix("$") -> meta).toMap /** - * How to tell two references apart when they name the SAME column with different policies. + * This says how to tell two references apart when they name the same column under different + * policies, which the scan needs in order to read that column at all. * - * MappedEntitlement.mUserId is the one case in the policy file: EntitlementUser - * (UseOnBehalfOfUserId, the role holder) and ConsentEntitlementUser (KeepUserId, the consent + * MappedEntitlement.mUserId is the one case in the policy file: Entitlement_UserId + * (UseOnBehalfOfUserId, the role holder) and Entitlement_UserId_ConsentScope (UseAuthenticatedUserId, the consent * engine copying the Consent's own scope onto the consent user). MappedEntitlements.addEntitlement * picks between them on createdByProcess, so a scan of that column that does not apply the same * discriminator reports every consent's own materialised scope as a leak -- which it is not; those @@ -222,10 +259,19 @@ class OnBehalfOfOwnershipSweepTest extends ServerSetupWithTestData with DefaultU * Reference name -> (discriminator field, the value that belongs to the OTHER reference). */ private val sharedColumnDiscriminator: Map[String, (String, String)] = Map( - "EntitlementUser" -> ("mCreatedByProcess", code.api.Constant.consent_user) + "Entitlement_UserId" -> ("mCreatedByProcess", code.api.Constant.consent_user), + // MappedExpectedChallengeAnswer.ExpectedUserId is the second case. One table holds two kinds of + // challenge. A payment challenge names a transaction request, and belongs to the human whose + // money is moving (ExpectedChallengeAnswer_ExpectedUserId_TransactionRequest). A consent or + // signing-basket authorisation challenge names no transaction request, and belongs to whoever + // is doing the authorising, which is the caller (ExpectedChallengeAnswer_ExpectedUserId) -- so + // a consent user legitimately appears in that column, and the scan must not read it as a leak. + "ExpectedChallengeAnswer_ExpectedUserId_TransactionRequest" -> ("TransactionRequestId", "") ) - /** Columns named by references that disagree on policy, so the scan must be told how to split them. */ + /** This lists the columns that more than one reference names under differing policies. Every one + * of them must appear in sharedColumnDiscriminator above, or the scan cannot tell which rows + * belong to which reference. */ private lazy val sharedColumns: List[(String, List[UserReference])] = UserReference.all .flatMap(ref => ref.fields.map(field => s"${ref.mapperClass}.$field" -> ref)) @@ -359,7 +405,7 @@ class OnBehalfOfOwnershipSweepTest extends ServerSetupWithTestData with DefaultU withClue( s"""|${unsplit.size} column(s) are named by references with different policies, but the scan - |has no discriminator for them -- so it would read the KeepUserId rows as leaks from the + |has no discriminator for them -- so it would read the UseAuthenticatedUserId rows as leaks from the |UseOnBehalfOfUserId reference, or miss real leaks by widening the exemption. Add an entry |to sharedColumnDiscriminator naming the field the provider branches on: |${unsplit.mkString("\n")} @@ -413,9 +459,9 @@ class OnBehalfOfOwnershipSweepTest extends ServerSetupWithTestData with DefaultU val agentId = currentUserIdOf(headers) val agentKey = primaryKeyOf(agentId) - withClue("ApiCollectionUser must still be unwired for this control to mean anything; " + + withClue("ApiCollection_UserId must still be unwired for this control to mean anything; " + "once it is wired, replace it here with another unwired reference: ") { - usedInMain should not contain "ApiCollectionUser" + usedInMain should not contain "ApiCollection_UserId" } val (status, body) = callApi("POST", "/obp/v4.0.0/my/api-collections", headers, @@ -427,7 +473,7 @@ class OnBehalfOfOwnershipSweepTest extends ServerSetupWithTestData with DefaultU withClue("the api collection was stored against the consent user, but the scan did not see " + "it -- so the scan in the scenario above is not looking where it claims to: ") { - referencesOwnedBy(agentId, agentKey) should contain("ApiCollectionUser") + referencesOwnedBy(agentId, agentKey) should contain("ApiCollection_UserId") } } } diff --git a/obp-api/src/test/scala/code/api/util/AgentDelegationTest.scala b/obp-api/src/test/scala/code/api/util/AgentDelegationTest.scala index fc01379b0d..39666e1300 100644 --- a/obp-api/src/test/scala/code/api/util/AgentDelegationTest.scala +++ b/obp-api/src/test/scala/code/api/util/AgentDelegationTest.scala @@ -35,12 +35,15 @@ import code.api.util.{Consent, ConsentLinkedCustomers, ConsentMyResources} import code.api.v1_4_0.JSONFactory1_4_0.TransactionRequestAccountJsonV140 import code.api.v2_1_0.TransactionRequestBodySandBoxTanJSON import code.bankconnectors.LocalMappedConnector +import code.metadata.counterparties.{MappedCounterparty, MapperCounterparties} import code.consent.MappedConsent import code.model.dataAccess.ResourceUser import code.setup.ServerSetup import code.model.dataAccess.MappedBank +import code.transactionChallenge.{MappedChallengeProvider, MappedExpectedChallengeAnswer} import code.transactionrequests.{MappedTransactionRequest, MappedTransactionRequestProvider} import code.users.{AttributionPolicy, UserReference, Users} +import com.openbankproject.commons.model.enums.ChallengeType import com.openbankproject.commons.model.{AccountId, AmountOfMoney, AmountOfMoneyJsonV121, BankAccount, BankAccountCommons, BankId, BankIdAccountId, TransactionRequestCharge, TransactionRequestId, TransactionRequestType} import net.liftweb.common.{Box, Failure, Full} import net.liftweb.mapper.By @@ -199,11 +202,11 @@ class AgentDelegationTest extends ServerSetup { } } - feature("Users.onBehalfOfUserIdOf — the resolver") { + feature("Users.resolveOnBehalfOfUserId — the resolver") { scenario("an original user resolves to itself", AgentDelegationTag) { val human = createUser() - Users.users.vend.onBehalfOfUserIdOf(human.userId) shouldBe Full(human.userId) + Users.users.vend.resolveOnBehalfOfUserId(human.userId) shouldBe Full(human.userId) Users.users.vend.actsForSelf(human.userId) shouldBe true } @@ -211,27 +214,27 @@ class AgentDelegationTest extends ServerSetup { val human = createUser() val consent = MappedConsent.create.mUserId(human.userId).saveMe() val agent = createUser(createdByConsentId = Some(consent.consentId)) - Users.users.vend.onBehalfOfUserIdOf(agent.userId) shouldBe Full(human.userId) + Users.users.vend.resolveOnBehalfOfUserId(agent.userId) shouldBe Full(human.userId) Users.users.vend.actsForSelf(agent.userId) shouldBe false } scenario("a dangling consent id keeps the caller (fails closed)", AgentDelegationTag) { val agent = createUser(createdByConsentId = Some(generateUUID())) - Users.users.vend.onBehalfOfUserIdOf(agent.userId) shouldBe Full(agent.userId) + Users.users.vend.resolveOnBehalfOfUserId(agent.userId) shouldBe Full(agent.userId) } scenario("an unknown user id keeps itself (fails closed)", AgentDelegationTag) { val id = generateUUID() - Users.users.vend.onBehalfOfUserIdOf(id) shouldBe Full(id) + Users.users.vend.resolveOnBehalfOfUserId(id) shouldBe Full(id) } scenario("BG-style: consent with no human yet keeps the caller, and is NOT pinned in the cache", AgentDelegationTag) { val consent = MappedConsent.create.saveMe() // mUserId empty until authorisation val agent = createUser(createdByConsentId = Some(consent.consentId)) - Users.users.vend.onBehalfOfUserIdOf(agent.userId) shouldBe Full(agent.userId) + Users.users.vend.resolveOnBehalfOfUserId(agent.userId) shouldBe Full(agent.userId) val human = createUser() consent.mUserId(human.userId).saveMe() // authorisation binds the human - Users.users.vend.onBehalfOfUserIdOf(agent.userId) shouldBe Full(human.userId) + Users.users.vend.resolveOnBehalfOfUserId(agent.userId) shouldBe Full(human.userId) } scenario("invariant: a consent whose user is itself a consent user is refused, not resolved", AgentDelegationTag) { @@ -240,7 +243,7 @@ class AgentDelegationTest extends ServerSetup { val agent1 = createUser(createdByConsentId = Some(consent1.consentId)) val consent2 = MappedConsent.create.mUserId(agent1.userId).saveMe() // names a consent user: data bug val agent2 = createUser(createdByConsentId = Some(consent2.consentId)) - Users.users.vend.onBehalfOfUserIdOf(agent2.userId) shouldBe a[Failure] + Users.users.vend.resolveOnBehalfOfUserId(agent2.userId) shouldBe a[Failure] // and CallContext falls back to the caller rather than throwing CallContext(user = Full(agent2)).onBehalfOfUserId shouldBe agent2.userId } @@ -248,11 +251,11 @@ class AgentDelegationTest extends ServerSetup { feature("Users.attributionOf — the policy-aware entry point") { - scenario("KeepUserId stores the caller and does not consult the resolver", AgentDelegationTag) { + scenario("UseAuthenticatedUserId stores the caller and does not consult the resolver", AgentDelegationTag) { val human = createUser() val consent = MappedConsent.create.mUserId(human.userId).saveMe() val agent = createUser(createdByConsentId = Some(consent.consentId)) - val a = Users.users.vend.attributionOf(agent.userId, UserReference.ConsentEntitlementUser).openOrThrowException("expected Full") + val a = Users.users.vend.attributionOf(agent.userId, UserReference.Entitlement_UserId_ConsentScope).openOrThrowException("expected Full") a.userIdToStore shouldBe agent.userId a.onBehalfOfUserId shouldBe agent.userId a.isDelegated shouldBe false @@ -263,18 +266,18 @@ class AgentDelegationTest extends ServerSetup { val human = createUser() val consent = MappedConsent.create.mUserId(human.userId).saveMe() val agent = createUser(createdByConsentId = Some(consent.consentId)) - val a = Users.users.vend.attributionOf(agent.userId, UserReference.EntitlementUser).openOrThrowException("expected Full") + val a = Users.users.vend.attributionOf(agent.userId, UserReference.Entitlement_UserId).openOrThrowException("expected Full") a.userId shouldBe agent.userId a.onBehalfOfUserId shouldBe human.userId a.userIdToStore shouldBe human.userId a.isDelegated shouldBe true a.consentId shouldBe Some(consent.consentId) - Users.users.vend.attributedUserId(agent.userId, UserReference.EntitlementUser) shouldBe Full(human.userId) + Users.users.vend.attributedUserId(agent.userId, UserReference.Entitlement_UserId) shouldBe Full(human.userId) } scenario("UseOnBehalfOfUserId for an original user is a no-op with no consent", AgentDelegationTag) { val human = createUser() - val a = Users.users.vend.attributionOf(human.userId, UserReference.AccountHolderUser).openOrThrowException("expected Full") + val a = Users.users.vend.attributionOf(human.userId, UserReference.AccountHolders_User).openOrThrowException("expected Full") a.userIdToStore shouldBe human.userId a.isDelegated shouldBe false a.consentId shouldBe None @@ -282,10 +285,10 @@ class AgentDelegationTest extends ServerSetup { scenario("Reject is Full for an original user and Failure for a consent user", AgentDelegationTag) { val human = createUser() - Users.users.vend.attributionOf(human.userId, UserReference.ConsentCreator).map(_.userIdToStore) shouldBe Full(human.userId) + Users.users.vend.attributionOf(human.userId, UserReference.Consent_UserId).map(_.userIdToStore) shouldBe Full(human.userId) val consent = MappedConsent.create.mUserId(human.userId).saveMe() val agent = createUser(createdByConsentId = Some(consent.consentId)) - val rejected = Users.users.vend.attributionOf(agent.userId, UserReference.ConsentCreator) + val rejected = Users.users.vend.attributionOf(agent.userId, UserReference.Consent_UserId) rejected shouldBe a[Failure] rejected.asInstanceOf[Failure].msg should include(ErrorMessages.InvalidUserId) } @@ -297,7 +300,7 @@ class AgentDelegationTest extends ServerSetup { r.fields should not be empty Class.forName(r.mapperClass) // resolves, or the reference names a class that does not exist } - UserReference.byPolicy(AttributionPolicy.Reject).map(_.name) should contain allOf ("ConsentCreator", "OAuthConsumerCreator") + UserReference.byPolicy(AttributionPolicy.Reject).map(_.name) should contain allOf ("Consent_UserId", "Consumer_CreatedByUserId") } } @@ -322,7 +325,7 @@ class AgentDelegationTest extends ServerSetup { } } - feature("getOrCreateAccountHolder goes through the attribution policy (AccountHolderUser)") { + feature("getOrCreateAccountHolder goes through the attribution policy (AccountHolders_User)") { scenario("an account created by a consent user is held by its on-behalf-of user", AgentDelegationTag) { val human = createUser() @@ -353,7 +356,7 @@ class AgentDelegationTest extends ServerSetup { } } - feature("DynamicData rows go through the attribution policy (DynamicDataUser), reads and writes alike") { + feature("DynamicData rows go through the attribution policy (DynamicData_UserId), reads and writes alike") { val entityName = "agent_delegation_note" def noteJson(id: String): JObject = (s"${entityName}_id" -> id) ~ ("name" -> "written by an agent") @@ -384,7 +387,7 @@ class AgentDelegationTest extends ServerSetup { dynamicData.delete(None, entityName, id, Some(human.userId), isPersonalEntity = true) shouldBe Full(true) } - scenario("a dynamic entity definition created by a consent user is owned by its on-behalf-of user (DynamicEntityUser)", AgentDelegationTag) { + scenario("a dynamic entity definition created by a consent user is owned by its on-behalf-of user (DynamicEntity_UserId)", AgentDelegationTag) { val human = createUser() val consent = MappedConsent.create.mUserId(human.userId).saveMe() val agent = createUser(createdByConsentId = Some(consent.consentId)) @@ -401,7 +404,7 @@ class AgentDelegationTest extends ServerSetup { } } - feature("User-Customer links go through the attribution policy (UserCustomerLinkUser)") { + feature("User-Customer links go through the attribution policy (UserCustomerLink_UserId)") { def links = code.usercustomerlinks.UserCustomerLink.userCustomerLink.vend @@ -526,7 +529,7 @@ class AgentDelegationTest extends ServerSetup { } } - feature("Transaction requests record both ids (UserReference.TransactionRequest)") { + feature("Transaction requests record both ids (UserReference.TransactionRequest_UserId)") { scenario("a request made by an original user names that user in both columns", AgentDelegationTag) { val human = createUser() @@ -553,7 +556,7 @@ class AgentDelegationTest extends ServerSetup { val agent1 = createUser(createdByConsentId = Some(consent1.consentId)) val consent2 = MappedConsent.create.mUserId(agent1.userId).saveMe() // names a consent user: data bug val agent2 = createUser(createdByConsentId = Some(consent2.consentId)) - Users.users.vend.onBehalfOfUserIdOf(agent2.userId) shouldBe a[Failure] // the precondition this pins + Users.users.vend.resolveOnBehalfOfUserId(agent2.userId) shouldBe a[Failure] // the precondition this pins val row = storedTransactionRequestFor(agent2) storedField(row.mUserId.get) shouldBe agent2.userId storedField(row.mOnBehalfOfUserId.get) shouldBe "" @@ -568,7 +571,110 @@ class AgentDelegationTest extends ServerSetup { } } - feature("Banks record the human who created them (UserReference.BankCreator)") { + feature("A payment challenge can only be answered by the person it was addressed to") { + + /** Saves one OBP_TRANSACTION_REQUEST_CHALLENGE for `expectedUserId` with a known answer. */ + def savedChallengeFor(expectedUserId: String, answer: String): String = { + val challengeId = generateUUID() + val salt = org.mindrot.jbcrypt.BCrypt.gensalt() + MappedChallengeProvider.saveChallenge( + challengeId = challengeId, + transactionRequestId = generateUUID(), + salt = salt, + expectedAnswer = org.mindrot.jbcrypt.BCrypt.hashpw(answer, salt).substring(0, 44), + expectedUserId = expectedUserId, + scaMethod = None, + scaStatus = None, + consentId = None, + basketId = None, + authenticationMethodId = None, + challengeType = ChallengeType.OBP_TRANSACTION_REQUEST_CHALLENGE.toString + ).openOrThrowException("expected the challenge to be saved") + challengeId + } + + def attemptCounterOf(challengeId: String): Int = + MappedExpectedChallengeAnswer.find(By(MappedExpectedChallengeAnswer.ChallengeId, challengeId)) + .openOrThrowException("expected the challenge row to exist").AttemptCounter.get + + scenario("the person it was addressed to can answer it", AgentDelegationTag) { + val human = createUser() + val challengeId = savedChallengeFor(human.userId, "123456") + MappedChallengeProvider.validateChallenge(challengeId, "123456", Some(human.userId)) shouldBe a[Full[_]] + } + + scenario("somebody else is refused, and is told the challenge is not theirs", AgentDelegationTag) { + val human = createUser() + val agent = createUser() + val challengeId = savedChallengeFor(human.userId, "123456") + val result = MappedChallengeProvider.validateChallenge(challengeId, "123456", Some(agent.userId)) + result shouldBe a[Failure] + result.asInstanceOf[Failure].msg should include(ErrorMessages.ChallengeNotAddressedToCaller) + } + + // The defect this closes: the attempt counter used to be incremented before the caller was + // matched against the challenge, so an agent politely trying to answer its human's payment + // challenge burned that human's three attempts and locked the payment out. + scenario("a call from somebody else does not use up the allowance of the person it belongs to", AgentDelegationTag) { + val human = createUser() + val agent = createUser() + val challengeId = savedChallengeFor(human.userId, "123456") + MappedChallengeProvider.validateChallenge(challengeId, "123456", Some(agent.userId)) shouldBe a[Failure] + MappedChallengeProvider.validateChallenge(challengeId, "999999", Some(agent.userId)) shouldBe a[Failure] + attemptCounterOf(challengeId) shouldBe 0 + MappedChallengeProvider.validateChallenge(challengeId, "123456", Some(human.userId)) shouldBe a[Full[_]] + } + } + + feature("Payment challenges are addressed to the human (UserReference.ExpectedChallengeAnswer_ExpectedUserId_TransactionRequest)") { + + /** Writes a transaction request as `user` and asks who must answer its SCA challenge. */ + def challengeRecipientFor(user: ResourceUser): Box[com.openbankproject.commons.model.User] = { + val row = storedTransactionRequestFor(user) + val transactionRequest = row.toTransactionRequest + .getOrElse(fail("expected the stored transaction request to convert back")) + LocalMappedConnector.paymentChallengeRecipient(transactionRequest, user) + } + + scenario("a payment started by a person is authorised by that same person", AgentDelegationTag) { + val human = createUser() + challengeRecipientFor(human).map(_.userId) shouldBe Full(human.userId) + } + + // The point of the whole feature: an agent has no email address and no phone number of its own, + // so a challenge left on it is either never delivered (the payment sits at INITIATED for ever) + // or, on an instance using the DUMMY SCA method, answered by the agent itself. + scenario("a payment started by an agent is authorised by the human it acts for", AgentDelegationTag) { + val human = createUser() + val consent = MappedConsent.create.mUserId(human.userId).saveMe() + val agent = createUser(createdByConsentId = Some(consent.consentId)) + val recipient = challengeRecipientFor(agent) + recipient.map(_.userId) shouldBe Full(human.userId) + recipient.map(_.userId) should not be Full(agent.userId) + } + + scenario("an agent whose consent names nobody cannot authorise the payment, and is refused", AgentDelegationTag) { + val consent = MappedConsent.create.mUserId("").saveMe() + val agent = createUser(createdByConsentId = Some(consent.consentId)) + challengeRecipientFor(agent) shouldBe a[Failure] + } + + scenario("a broken consent chain is refused rather than left on the agent", AgentDelegationTag) { + val human = createUser() + val consent1 = MappedConsent.create.mUserId(human.userId).saveMe() + val agent1 = createUser(createdByConsentId = Some(consent1.consentId)) + val consent2 = MappedConsent.create.mUserId(agent1.userId).saveMe() // names a consent user: data bug + val agent2 = createUser(createdByConsentId = Some(consent2.consentId)) + challengeRecipientFor(agent2) shouldBe a[Failure] + } + + scenario("the reference carries the on-behalf-of policy, and the consent one still does not", AgentDelegationTag) { + UserReference.ExpectedChallengeAnswer_ExpectedUserId_TransactionRequest.policy shouldBe AttributionPolicy.UseOnBehalfOfUserId + UserReference.ExpectedChallengeAnswer_ExpectedUserId.policy shouldBe AttributionPolicy.UseAuthenticatedUserId + } + } + + feature("Banks record the human who created them (UserReference.Bank_CreatedByUserId)") { /** Create a bank through the connector as `callerUserId`, and return the stored row. */ def createBankAs(callContext: Option[CallContext]): MappedBank = { @@ -618,4 +724,63 @@ class AgentDelegationTest extends ServerSetup { storedField(createBankAs(None).CreatedByUserId.get) shouldBe "" } } + + feature("Counterparties record both ids (UserReference.Counterparty_CreatedByUserId)") { + + /** Create a counterparty through the provider as `callerUserId`, and return the stored row. */ + def createCounterpartyAs(callerUserId: String): MappedCounterparty = { + val name = s"agent-delegation-cp-${generateUUID().take(8)}" + MapperCounterparties.createCounterparty( + createdByUserId = callerUserId, thisBankId = "agent-delegation-bank", + thisAccountId = generateUUID(), thisViewId = "owner", name = name, + otherAccountRoutingScheme = "IBAN", otherAccountRoutingAddress = "DE89370400440532013000", + otherBankRoutingScheme = "BIC", otherBankRoutingAddress = "COBADEFF", + otherBranchRoutingScheme = "", otherBranchRoutingAddress = "", isBeneficiary = true, + otherAccountSecondaryRoutingScheme = "", otherAccountSecondaryRoutingAddress = "", + description = "agent delegation test", currency = "EUR", bespoke = Nil + ).openOrThrowException("expected the counterparty to be created") + MappedCounterparty.find(By(MappedCounterparty.mName, name)) + .openOrThrowException("expected the counterparty row to have been written") + } + + // Record-both, not redirect: mCreatedByUserId is published as created_by_user_id on the + // v2.2.0/v4.0.0 responses, so it must keep saying who actually made the call. + scenario("a counterparty created by an original user names that user in both columns", AgentDelegationTag) { + val human = createUser() + val row = createCounterpartyAs(human.userId) + storedField(row.mCreatedByUserId.get) shouldBe human.userId + storedField(row.mCreatedByOnBehalfOfUserId.get) shouldBe human.userId + } + + scenario("a counterparty created by a consent user names the agent and its on-behalf-of user", AgentDelegationTag) { + val human = createUser() + val consent = MappedConsent.create.mUserId(human.userId).saveMe() + val agent = createUser(createdByConsentId = Some(consent.consentId)) + val row = createCounterpartyAs(agent.userId) + storedField(row.mCreatedByUserId.get) shouldBe agent.userId + storedField(row.mCreatedByOnBehalfOfUserId.get) shouldBe human.userId + } + + scenario("a consent user whose consent has no human yet acts for itself (fails closed)", AgentDelegationTag) { + val consent = MappedConsent.create.mUserId("").saveMe() + val agent = createUser(createdByConsentId = Some(consent.consentId)) + val row = createCounterpartyAs(agent.userId) + storedField(row.mCreatedByUserId.get) shouldBe agent.userId + storedField(row.mCreatedByOnBehalfOfUserId.get) shouldBe agent.userId + } + + // A broken chain must not blank the audit column: who sent money where is the question this + // table exists to answer, so the actor is kept even when the human cannot be resolved. + scenario("a broken consent chain still names the actor in both columns", AgentDelegationTag) { + val human = createUser() + val consent1 = MappedConsent.create.mUserId(human.userId).saveMe() + val agent1 = createUser(createdByConsentId = Some(consent1.consentId)) + val consent2 = MappedConsent.create.mUserId(agent1.userId).saveMe() + val agent2 = createUser(createdByConsentId = Some(consent2.consentId)) + Users.users.vend.resolveOnBehalfOfUserId(agent2.userId) shouldBe a[Failure] + val row = createCounterpartyAs(agent2.userId) + storedField(row.mCreatedByUserId.get) shouldBe agent2.userId + storedField(row.mCreatedByOnBehalfOfUserId.get) shouldBe agent2.userId + } + } } diff --git a/obp-api/src/test/scala/code/api/v6_0_0/DynamicEntityRecordIdLengthTest.scala b/obp-api/src/test/scala/code/api/v6_0_0/DynamicEntityRecordIdLengthTest.scala new file mode 100644 index 0000000000..b5eac848ab --- /dev/null +++ b/obp-api/src/test/scala/code/api/v6_0_0/DynamicEntityRecordIdLengthTest.scala @@ -0,0 +1,140 @@ +/** +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.v6_0_0 + +import code.api.util.APIUtil.OAuth._ +import code.api.util.ApiRole._ +import code.api.util.ErrorMessages._ +import code.entitlement.Entitlement +import com.openbankproject.commons.model.ErrorMessage +import com.openbankproject.commons.util.ApiVersion +import com.openbankproject.commons.util.JsonAliases._ +import org.json4s.JsonAST.JArray +import org.json4s.JsonDSL._ +import org.json4s._ +import org.json4s.native.Serialization.write +import org.scalatest.Tag + +/** + * These tests cover the length of the id of a single Dynamic Entity record. + * + * A caller may supply that id in the request body rather than let one be generated, which is how an + * entity is given a natural key such as a country code or the name of a scheme. The column holding the + * id used to be 36 characters wide, the length of a UUID, so any natural key longer than that failed + * deep in the database driver and the caller was told only "value too long for type character + * varying(36)". The column is now 255 characters wide, the same width as the two other columns that + * hold this same id, and an id longer than the column is refused up front with a message that says so. + * + * The first scenario locks in the width, the second locks in the error. + */ +class DynamicEntityRecordIdLengthTest extends V600ServerSetup { + + object VersionOfApi extends Tag(ApiVersion.v6_0_0.toString) + + private val entityName = "record_id_length_probe" + + /** A single string field, plus the personal endpoints so no entity role is needed to write a record. */ + private def entityDefinition: JValue = + ("entity_name" -> entityName) ~ + ("has_personal_entity" -> true) ~ + ("schema" -> + ("description" -> "Entity used to test the length of a record id.") ~ + ("required" -> List("name")) ~ + ("properties" -> + ("name" -> ("type" -> "string") ~ ("example" -> "Alice")))) + + private def createEntityDefinition(): String = { + Entitlement.entitlement.vend.addEntitlement("", resourceUser1.userId, CanCreateSystemLevelDynamicEntity.toString) + val request = (v6_0_0_Request / "management" / "system-dynamic-entities").POST <@ (user1) + val response = makePostRequest(request, write(entityDefinition)) + response.code should equal(201) + (response.body \ "dynamic_entity_id").extract[String] + } + + private def deleteEntityDefinition(dynamicEntityId: String): Unit = { + Entitlement.entitlement.vend.addEntitlement("", resourceUser1.userId, CanDeleteSystemLevelDynamicEntity.toString) + makeDeleteRequest((v6_0_0_Request / "management" / "system-dynamic-entities" / dynamicEntityId).DELETE <@ (user1)) + } + + /** The name of the id field of this entity, as the API generates it: `_id`. */ + private val recordIdField = s"${entityName}_id" + + feature("The id of a Dynamic Entity record may be supplied by the caller") { + + scenario("An id longer than a UUID is stored and can be read back", VersionOfApi) { + val dynamicEntityId = createEntityDefinition() + try { + // The exact natural key that used to fail: 53 characters, well past the old 36 character column. + val naturalKey = "OTHER_PERMANENTLY_CHEMICALLY_BOUND_CARBON_IN_PRODUCTS" + naturalKey.length should be > 36 + + When("a record is created with that id in the request body") + val createRequest = (dynamicEntity_Request / "my" / entityName).POST <@ (user1) + val createResponse = makePostRequest(createRequest, write((recordIdField -> naturalKey) ~ ("name" -> "Alice"))) + + Then("the record is created and keeps the id it was given") + createResponse.code should equal(201) + (createResponse.body \ entityName \ recordIdField).extract[String] should equal(naturalKey) + + And("it can be read back by that same id") + val getResponse = makeGetRequest((dynamicEntity_Request / "my" / entityName / naturalKey).GET <@ (user1)) + getResponse.code should equal(200) + (getResponse.body \ entityName \ recordIdField).extract[String] should equal(naturalKey) + (getResponse.body \ entityName \ "name").extract[String] should equal("Alice") + } finally { + deleteEntityDefinition(dynamicEntityId) + } + } + + scenario("An id longer than the column is refused with a clear error and nothing is stored", VersionOfApi) { + val dynamicEntityId = createEntityDefinition() + try { + val tooLongId = "x" * 256 + + When("a record is created with an id longer than the column") + val createRequest = (dynamicEntity_Request / "my" / entityName).POST <@ (user1) + val createResponse = makePostRequest(createRequest, write((recordIdField -> tooLongId) ~ ("name" -> "Alice"))) + + Then("the request is rejected as a bad request, not as a server error") + createResponse.code should equal(400) + + And("the message names the limit and the length that was given") + val message = createResponse.body.extract[ErrorMessage].message + message should include(DynamicEntityRecordIdTooLong) + message should include("255") + message should include("256") + + And("no record was stored") + val listResponse = makeGetRequest((dynamicEntity_Request / "my" / entityName).GET <@ (user1)) + listResponse.code should equal(200) + (listResponse.body \ s"${entityName}_list").asInstanceOf[JArray].arr.size should equal(0) + } finally { + deleteEntityDefinition(dynamicEntityId) + } + } + } +} diff --git a/obp-api/src/test/scala/code/api/v7_0_0/Http4s700RoutesTest.scala b/obp-api/src/test/scala/code/api/v7_0_0/Http4s700RoutesTest.scala index 842c2b152d..0874f2d517 100644 --- a/obp-api/src/test/scala/code/api/v7_0_0/Http4s700RoutesTest.scala +++ b/obp-api/src/test/scala/code/api/v7_0_0/Http4s700RoutesTest.scala @@ -34,8 +34,8 @@ import code.api.util.http4s.Http4sStandardHeaders import code.api.Constant.SYSTEM_OWNER_VIEW_ID import code.api.ResponseHeader import code.api.util.APIUtil -import code.api.util.ApiRole.{canAttachOpenCorridorPromise, canConfigureAmqpBankBroker, canGetMessageOutbox, canRetryMessageOutbox, canSettleOpenCorridor, canCreateAccount, canCreateEntitlementAtAnyBank, canCreateOrganisation, canCreateRoutingScheme, canCreateUtilityVendResult, canDeleteEntitlementAtAnyBank, canDeleteOrganisation, canDeleteRoutingScheme, canDeleteSchedulerJobLock, canUpdateSystemView, canGetAccountAccessTrace, canGetAnyOrganisation, canGetAnyUser, canGetCacheConfig, canGetCacheInfo, canGetCacheNamespaces, canGetCardsForBank, canGetConnectorHealth, canCreateMetricsArchiveRun, canGetCustomersAtOneBank, canGetDatabasePoolInfo, canGetMetricsDiagnostics, canGetMigrations, canGetSchedulerJobLocks, canReadResourceDoc, canUpdateBankSupportedRoutingScheme, canUpdateOrganisation, canUpdateRoutingScheme} -import code.api.util.ErrorMessages.{AccountIdAlreadyExists, AuthenticatedUserIsRequired, BankNotFound, DuplicateUsername, EntitlementAlreadyExists, InvalidAccountRoutings, InvalidJsonFormat, InvalidJsonValue, InvalidOrganisationIdFormat, InvalidPhoneNumber, InvalidRoutingSchemeName, UserFilterParametersNotSupported, InvalidTransactionRequestId, MessageOutboxRowNotFound, MessageOutboxRowNotSticky, MobileWalletDestinationNotFound, MobileWalletInvalidMsisdn, AmqpBankBrokerNotConfigured, OpenCorridorDisabled, OpenCorridorPromiseEvidenceConflict, OpenCorridorPromiseNotPending, OpenCorridorPromiseTypeMismatch, OpenCorridorSameBankNotAllowed, OpenCorridorSettlementAddressMissing, OpenCorridorSettlementNotFound, OrganisationAlreadyExists, OrganisationNotFound, PayeeLookupAddressMismatch, PayeeLookupIdentifierTypeNotRegistered, PayeeNotFound, RoutingSchemeAlreadyExists, RoutingSchemeExampleAddressMismatch, RoutingSchemeNotFound, SelfServiceBankCreationDisabled, SelfServiceBankLimitReached, SystemViewNotFound, UserHasMissingRoles, UserNotFoundByUserId, UtilityIdentifierTypeWrongCategory, UtilityInvalidIdentifier, UtilityTransactionRequestNotFound} +import code.api.util.ApiRole.{canAttachOpenCorridorPromise, canConfigureAmqpBankBroker, canGetMessageOutbox, canRetryMessageOutbox, canSettleOpenCorridor, canCreateAccount, canCreateEntitlementAtAnyBank, canCreateOrganisation, canCreateRoutingScheme, canCreateUtilityVendResult, canDeleteAccountNotificationWebhookAtOneBank, canDeleteEntitlementAtAnyBank, canDeleteOrganisation, canDeleteRoutingScheme, canDeleteSchedulerJobLock, canDeleteSystemAccountNotificationWebhook, canUpdateSystemView, canGetAccountAccessTrace, canGetAnyOrganisation, canGetAnyUser, canGetCacheConfig, canGetCacheInfo, canGetCacheNamespaces, canGetCardsForBank, canGetConnectorHealth, canCreateMetricsArchiveRun, canGetCustomersAtOneBank, canGetDatabasePoolInfo, canGetMetricsDiagnostics, canGetMigrations, canGetSchedulerJobLocks, canReadResourceDoc, canUpdateBankSupportedRoutingScheme, canUpdateOrganisation, canUpdateRoutingScheme} +import code.api.util.ErrorMessages.{AccountIdAlreadyExists, AuthenticatedUserIsRequired, BankNotFound, DuplicateUsername, EntitlementAlreadyExists, InvalidAccountRoutings, InvalidJsonFormat, InvalidJsonValue, InvalidOrganisationIdFormat, InvalidPhoneNumber, InvalidRoutingSchemeName, UserFilterParametersNotSupported, InvalidTransactionRequestId, MessageOutboxRowNotFound, MessageOutboxRowNotSticky, MobileWalletDestinationNotFound, MobileWalletInvalidMsisdn, NotificationWebhookNotFound, AmqpBankBrokerNotConfigured, OpenCorridorDisabled, OpenCorridorPromiseEvidenceConflict, OpenCorridorPromiseNotPending, OpenCorridorPromiseTypeMismatch, OpenCorridorSameBankNotAllowed, OpenCorridorSettlementAddressMissing, OpenCorridorSettlementNotFound, OrganisationAlreadyExists, OrganisationNotFound, PayeeLookupAddressMismatch, PayeeLookupIdentifierTypeNotRegistered, PayeeNotFound, RoutingSchemeAlreadyExists, RoutingSchemeExampleAddressMismatch, RoutingSchemeNotFound, SelfServiceBankCreationDisabled, SelfServiceBankLimitReached, SystemViewNotFound, UserHasMissingRoles, UserNotFoundByUserId, UtilityIdentifierTypeWrongCategory, UtilityInvalidIdentifier, UtilityTransactionRequestNotFound} import code.utilitypayment.{UtilityCallbackStatus, UtilityPaymentCallbacks} import code.scheduler.JobScheduler import net.liftweb.mapper.By @@ -2079,6 +2079,139 @@ class Http4s700RoutesTest extends ServerSetupWithTestData { } } + // ── Account Notification Webhook deletes ────────────────────────────────── + // + // v4.0.0 shipped the two creates with no delete, so these rows could not be removed by anyone. + // The provider is driven directly to make the fixtures; creating them over HTTP would test the + // v4 endpoints rather than these. + + private def createSystemNotificationWebhook(): String = { + val created = scala.concurrent.Await.result( + code.webhook.SystemAccountNotificationWebhookTrait.systemAccountNotificationWebhook.vend + .createSystemAccountNotificationWebhookFuture( + userId = resourceUser1.userId, + triggerName = code.api.util.ApiTrigger.onCreateTransaction.toString, + url = "https://example.com/hook", + httpMethod = "POST", + httpProtocol = "HTTP/1.1"), + scala.concurrent.duration.Duration(20, "seconds")) + created.openOrThrowException("could not create the system notification webhook fixture").webhookId + } + + private def createBankNotificationWebhook(bankId: String): String = { + val created = scala.concurrent.Await.result( + code.webhook.BankAccountNotificationWebhookTrait.bankAccountNotificationWebhook.vend + .createBankAccountNotificationWebhookFuture( + bankId = bankId, + userId = resourceUser1.userId, + triggerName = code.api.util.ApiTrigger.onCreateTransaction.toString, + url = "https://example.com/hook", + httpMethod = "POST", + httpProtocol = "HTTP/1.1"), + scala.concurrent.duration.Duration(20, "seconds")) + created.openOrThrowException("could not create the bank notification webhook fixture").webhookId + } + + feature("Http4s700 deleteSystemAccountNotificationWebhook endpoint") { + + scenario("Reject unauthenticated DELETE", Http4s700RoutesTag) { + val (statusCode, _, _) = makeHttpRequestWithMethod( + "DELETE", "/obp/v7.0.0/web-hooks/account/notifications/on-create-transaction/does-not-matter") + statusCode shouldBe 401 + } + + scenario("Return 403 when missing canDeleteSystemAccountNotificationWebhook", Http4s700RoutesTag) { + val headers = Map("DirectLogin" -> s"token=${token2.value}") + val (statusCode, _, _) = makeHttpRequestWithMethod( + "DELETE", "/obp/v7.0.0/web-hooks/account/notifications/on-create-transaction/does-not-matter", headers) + statusCode shouldBe 403 + } + + scenario("Return 404 for an unknown webhook id", Http4s700RoutesTag) { + addEntitlement("", resourceUser1.userId, canDeleteSystemAccountNotificationWebhook.toString) + val headers = Map("DirectLogin" -> s"token=${token1.value}") + val (statusCode, json, _) = makeHttpRequestWithMethod( + "DELETE", "/obp/v7.0.0/web-hooks/account/notifications/on-create-transaction/no-such-webhook", headers) + statusCode shouldBe 404 + messageOf(json) should startWith(NotificationWebhookNotFound) + } + + scenario("Return 204 and remove the row when role granted", Http4s700RoutesTag) { + addEntitlement("", resourceUser1.userId, canDeleteSystemAccountNotificationWebhook.toString) + val webhookId = createSystemNotificationWebhook() + + val headers = Map("DirectLogin" -> s"token=${token1.value}") + val (statusCode, _, _) = makeHttpRequestWithMethod( + "DELETE", s"/obp/v7.0.0/web-hooks/account/notifications/on-create-transaction/$webhookId", headers) + statusCode shouldBe 204 + + And("the row should be gone") + val fetched = scala.concurrent.Await.result( + code.webhook.SystemAccountNotificationWebhookTrait.systemAccountNotificationWebhook.vend + .getSystemAccountNotificationWebhookByIdFuture(webhookId), scala.concurrent.duration.Duration(20, "seconds")) + fetched.isDefined shouldBe false + + And("deleting it a second time should answer 404") + val (secondStatusCode, _, _) = makeHttpRequestWithMethod( + "DELETE", s"/obp/v7.0.0/web-hooks/account/notifications/on-create-transaction/$webhookId", headers) + secondStatusCode shouldBe 404 + } + } + + feature("Http4s700 deleteBankAccountNotificationWebhook endpoint") { + + scenario("Reject unauthenticated DELETE", Http4s700RoutesTag) { + val bankId = testBankId1.value + val (statusCode, _, _) = makeHttpRequestWithMethod( + "DELETE", s"/obp/v7.0.0/banks/$bankId/web-hooks/account/notifications/on-create-transaction/does-not-matter") + statusCode shouldBe 401 + } + + scenario("Return 403 when missing canDeleteAccountNotificationWebhookAtOneBank", Http4s700RoutesTag) { + val bankId = testBankId1.value + val headers = Map("DirectLogin" -> s"token=${token2.value}") + val (statusCode, _, _) = makeHttpRequestWithMethod( + "DELETE", s"/obp/v7.0.0/banks/$bankId/web-hooks/account/notifications/on-create-transaction/does-not-matter", headers) + statusCode shouldBe 403 + } + + scenario("Return 204 and remove the row when role granted", Http4s700RoutesTag) { + val bankId = testBankId1.value + addEntitlement(bankId, resourceUser1.userId, canDeleteAccountNotificationWebhookAtOneBank.toString) + val webhookId = createBankNotificationWebhook(bankId) + + val headers = Map("DirectLogin" -> s"token=${token1.value}") + val (statusCode, _, _) = makeHttpRequestWithMethod( + "DELETE", s"/obp/v7.0.0/banks/$bankId/web-hooks/account/notifications/on-create-transaction/$webhookId", headers) + statusCode shouldBe 204 + + And("the row should be gone") + val fetched = scala.concurrent.Await.result( + code.webhook.BankAccountNotificationWebhookTrait.bankAccountNotificationWebhook.vend + .getBankAccountNotificationWebhookByIdFuture(webhookId), scala.concurrent.duration.Duration(20, "seconds")) + fetched.isDefined shouldBe false + } + + scenario("A webhook belonging to another bank reads as not found, not forbidden", Http4s700RoutesTag) { + val ownerBankId = testBankId1.value + val otherBankId = testBankId2.value + addEntitlement(otherBankId, resourceUser1.userId, canDeleteAccountNotificationWebhookAtOneBank.toString) + val webhookId = createBankNotificationWebhook(ownerBankId) + + val headers = Map("DirectLogin" -> s"token=${token1.value}") + val (statusCode, json, _) = makeHttpRequestWithMethod( + "DELETE", s"/obp/v7.0.0/banks/$otherBankId/web-hooks/account/notifications/on-create-transaction/$webhookId", headers) + statusCode shouldBe 404 + messageOf(json) should startWith(NotificationWebhookNotFound) + + And("the webhook must still exist at its own bank") + val fetched = scala.concurrent.Await.result( + code.webhook.BankAccountNotificationWebhookTrait.bankAccountNotificationWebhook.vend + .getBankAccountNotificationWebhookByIdFuture(webhookId), scala.concurrent.duration.Duration(20, "seconds")) + fetched.isDefined shouldBe true + } + } + feature("Http4s700 getBankSupportedRoutingSchemes endpoint") { scenario("Reject unauthenticated GET", Http4s700RoutesTag) { diff --git a/obp-api/src/test/scala/code/users/UserReferenceAttributionPolicyTest.scala b/obp-api/src/test/scala/code/users/UserReferenceAttributionPolicyTest.scala index 63684bff54..6a3740c32c 100644 --- a/obp-api/src/test/scala/code/users/UserReferenceAttributionPolicyTest.scala +++ b/obp-api/src/test/scala/code/users/UserReferenceAttributionPolicyTest.scala @@ -33,11 +33,12 @@ import net.liftweb.mapper.MetaMapper import org.scalatest.Tag /** - * Frozen-style guard over the attribution policy table (`UserReference`), in the spirit of - * `code.util.FrozenClassTest`: the policy file must name every user-reference column in the schema, - * and must not name columns that do not exist. + * This test guards the attribution policy table in `UserReference` against the database schema + * drifting away from it. It works in the same way as `code.util.FrozenClassTest`: the policy file + * must name every column in the schema that holds a user id, and must not name a column that does + * not exist. * - * Why this exists. Attribution is enforced in the providers, one `UserReference` at a time + * Why it exists. Attribution is enforced in the providers, one `UserReference` at a time * (ON_BEHALF_OF_USER_ID_PLAN.md Phase 2). Doing that by hand is only safe if the map of what needs * doing is provably complete — otherwise a table added next week grows a `UserId` column that * silently strands rows on consent users, and nothing anywhere says so. This test is that proof. @@ -52,13 +53,15 @@ class UserReferenceAttributionPolicyTest extends ServerSetup { object UserReferenceTag extends Tag("UserReferenceAttributionPolicy") - /** Column names that look like a user reference. Matches the plan's pattern. */ + /** This is how the test decides a column holds a user id: by its name. It is the same pattern + * the plan uses. */ private val userReferenceNamePattern = "(?i).*(userid|createdby|grantedby|holder).*".r private def mapperClassName(meta: MetaMapper[_]): String = meta.getClass.getName.stripSuffix("$") - /** (mapperClass, fieldName) for every schema field whose name looks like a user reference. */ + /** This lists every column in the live schema whose name says it holds a user id, as a pair of + * the Mapper class and the field. It is what the policy table has to account for. */ private lazy val schemaUserReferenceColumns: List[(String, String)] = for { meta <- ToSchemify.models @@ -66,7 +69,9 @@ class UserReferenceAttributionPolicyTest extends ServerSetup { if userReferenceNamePattern.pattern.matcher(field.name).matches() } yield (mapperClassName(meta), field.name) - /** (mapperClass, fieldName) -> the references naming it. */ + /** This is the other side of the comparison: for each column the policy table names, the + * references that name it. A column normally has one, but may have two where they differ by + * policy. */ private lazy val declaredColumns: Map[(String, String), List[UserReference]] = UserReference.all .flatMap(ref => ref.fields.map(field => (ref.mapperClass, field) -> ref)) @@ -129,8 +134,8 @@ class UserReferenceAttributionPolicyTest extends ServerSetup { } } - // MappedEntitlement.mUserId is the deliberate case: EntitlementUser (UseOnBehalfOfUserId) and - // ConsentEntitlementUser (KeepUserId) name the same column, chosen per createdByProcess. + // MappedEntitlement.mUserId is the deliberate case: Entitlement_UserId (UseOnBehalfOfUserId) and + // Entitlement_UserId_ConsentScope (UseAuthenticatedUserId) name the same column, chosen per createdByProcess. scenario("a column named by more than one UserReference has references that differ by policy", UserReferenceTag) { val ambiguous = declaredColumns.toList .filter { case (_, refs) => refs.size > 1 && refs.map(_.policy).distinct.size == 1 } diff --git a/scripts/resource_doc_baseline/parity_allowlist.json b/scripts/resource_doc_baseline/parity_allowlist.json index 0fe94a29d9..9e54338d83 100644 --- a/scripts/resource_doc_baseline/parity_allowlist.json +++ b/scripts/resource_doc_baseline/parity_allowlist.json @@ -1602,6 +1602,86 @@ "reason": "Documents the consumer pin as implemented: adds the caller as the final fallback and states that a Consent naming no consumer is refused with ConsentConsumerIsRequired rather than stored with a null consumer_id. Text-only.", "lift_digest": "7397560b818a5d9f4eb31a701499aea10c0ecd8d89d15496c0904d90984b84a6", "http4s_digest": "881b4fe9e74e92efbe1177f1548d7ad4aa85be90f0ec44789ecf779267335f6b" + }, + { + "version": "v4_0_0", + "endpoint": "answerTransactionRequestChallenge", + "field": "errorResponseBodies", + "reason": "Adds ChallengeNotAddressedToCaller (OBP-40063): a payment challenge is answered by the person the payment is for, so an agent acting under a Consent is refused rather than allowed to relay a code it was never sent. See ON_BEHALF_OF_USER_ID_PLAN.md Decision 9.", + "lift_digest": "13648f17562c1654881963b4978024f82e1edcdb92729397a2b015b291939ddb", + "http4s_digest": "c584fa1579db6b30ed7c21cf3dc7da57f50dcad7abba332ea1a58dd4c37808c4" + }, + { + "version": "v4_0_0", + "endpoint": "createTransactionRequestAccount", + "field": "errorResponseBodies", + "reason": "Adds PaymentChallengeHasNoOnBehalfOfUser (OBP-40064): a payment needing Strong Customer Authentication is refused when no person can be determined to address the challenge to, instead of being parked behind a challenge nobody can answer. See ON_BEHALF_OF_USER_ID_PLAN.md Decision 9.", + "lift_digest": "4eac30b6b708ad9f30e688ef1327a01ff7f4280f982f77063851c9483e3179c2", + "http4s_digest": "36b31730de78f93aa41f09da7fe44eff78fe76dac497497e7562b84454a2e3b0" + }, + { + "version": "v4_0_0", + "endpoint": "createTransactionRequestAccountOtp", + "field": "errorResponseBodies", + "reason": "Adds PaymentChallengeHasNoOnBehalfOfUser (OBP-40064): a payment needing Strong Customer Authentication is refused when no person can be determined to address the challenge to, instead of being parked behind a challenge nobody can answer. See ON_BEHALF_OF_USER_ID_PLAN.md Decision 9.", + "lift_digest": "4eac30b6b708ad9f30e688ef1327a01ff7f4280f982f77063851c9483e3179c2", + "http4s_digest": "36b31730de78f93aa41f09da7fe44eff78fe76dac497497e7562b84454a2e3b0" + }, + { + "version": "v4_0_0", + "endpoint": "createTransactionRequestAgentCashWithDrawal", + "field": "errorResponseBodies", + "reason": "Adds PaymentChallengeHasNoOnBehalfOfUser (OBP-40064): a payment needing Strong Customer Authentication is refused when no person can be determined to address the challenge to, instead of being parked behind a challenge nobody can answer. See ON_BEHALF_OF_USER_ID_PLAN.md Decision 9.", + "lift_digest": "4eac30b6b708ad9f30e688ef1327a01ff7f4280f982f77063851c9483e3179c2", + "http4s_digest": "36b31730de78f93aa41f09da7fe44eff78fe76dac497497e7562b84454a2e3b0" + }, + { + "version": "v4_0_0", + "endpoint": "createTransactionRequestCard", + "field": "errorResponseBodies", + "reason": "Adds PaymentChallengeHasNoOnBehalfOfUser (OBP-40064): a payment needing Strong Customer Authentication is refused when no person can be determined to address the challenge to, instead of being parked behind a challenge nobody can answer. See ON_BEHALF_OF_USER_ID_PLAN.md Decision 9.", + "lift_digest": "4eac30b6b708ad9f30e688ef1327a01ff7f4280f982f77063851c9483e3179c2", + "http4s_digest": "36b31730de78f93aa41f09da7fe44eff78fe76dac497497e7562b84454a2e3b0" + }, + { + "version": "v4_0_0", + "endpoint": "createTransactionRequestCounterparty", + "field": "errorResponseBodies", + "reason": "Adds PaymentChallengeHasNoOnBehalfOfUser (OBP-40064): a payment needing Strong Customer Authentication is refused when no person can be determined to address the challenge to, instead of being parked behind a challenge nobody can answer. See ON_BEHALF_OF_USER_ID_PLAN.md Decision 9.", + "lift_digest": "4eac30b6b708ad9f30e688ef1327a01ff7f4280f982f77063851c9483e3179c2", + "http4s_digest": "36b31730de78f93aa41f09da7fe44eff78fe76dac497497e7562b84454a2e3b0" + }, + { + "version": "v4_0_0", + "endpoint": "createTransactionRequestFreeForm", + "field": "errorResponseBodies", + "reason": "Adds PaymentChallengeHasNoOnBehalfOfUser (OBP-40064): a payment needing Strong Customer Authentication is refused when no person can be determined to address the challenge to, instead of being parked behind a challenge nobody can answer. See ON_BEHALF_OF_USER_ID_PLAN.md Decision 9.", + "lift_digest": "4eac30b6b708ad9f30e688ef1327a01ff7f4280f982f77063851c9483e3179c2", + "http4s_digest": "36b31730de78f93aa41f09da7fe44eff78fe76dac497497e7562b84454a2e3b0" + }, + { + "version": "v4_0_0", + "endpoint": "createTransactionRequestRefund", + "field": "errorResponseBodies", + "reason": "Adds PaymentChallengeHasNoOnBehalfOfUser (OBP-40064): a payment needing Strong Customer Authentication is refused when no person can be determined to address the challenge to, instead of being parked behind a challenge nobody can answer. See ON_BEHALF_OF_USER_ID_PLAN.md Decision 9.", + "lift_digest": "4eac30b6b708ad9f30e688ef1327a01ff7f4280f982f77063851c9483e3179c2", + "http4s_digest": "36b31730de78f93aa41f09da7fe44eff78fe76dac497497e7562b84454a2e3b0" + }, + { + "version": "v4_0_0", + "endpoint": "createTransactionRequestSepa", + "field": "errorResponseBodies", + "reason": "Adds PaymentChallengeHasNoOnBehalfOfUser (OBP-40064): a payment needing Strong Customer Authentication is refused when no person can be determined to address the challenge to, instead of being parked behind a challenge nobody can answer. See ON_BEHALF_OF_USER_ID_PLAN.md Decision 9.", + "lift_digest": "4eac30b6b708ad9f30e688ef1327a01ff7f4280f982f77063851c9483e3179c2", + "http4s_digest": "36b31730de78f93aa41f09da7fe44eff78fe76dac497497e7562b84454a2e3b0" + }, + { + "version": "v4_0_0", + "endpoint": "createTransactionRequestSimple", + "field": "errorResponseBodies", + "reason": "Adds PaymentChallengeHasNoOnBehalfOfUser (OBP-40064): a payment needing Strong Customer Authentication is refused when no person can be determined to address the challenge to, instead of being parked behind a challenge nobody can answer. See ON_BEHALF_OF_USER_ID_PLAN.md Decision 9.", + "lift_digest": "4eac30b6b708ad9f30e688ef1327a01ff7f4280f982f77063851c9483e3179c2", + "http4s_digest": "36b31730de78f93aa41f09da7fe44eff78fe76dac497497e7562b84454a2e3b0" } ] }