diff --git a/examples/bank/README.md b/examples/bank/README.md index 8921acaae..3f6ae2d66 100644 --- a/examples/bank/README.md +++ b/examples/bank/README.md @@ -238,36 +238,25 @@ unvouched-for, `RemoteServer` *clears* it, and every bank action then fails with whatever non-empty principal arrives, and anyone who can open a socket can claim to be any customer. -What that does and does not leave standing: per-row ownership *is* enforced by -the models (`db::loadOwned` navigates a row to its owner and compares that with -the session principal), so cross-customer isolation is real and testable — it is -what `another-customer-cannot-touch-your-account.scenario` pins. What is absent -is credential *proof*. A server that needs it swaps in +What that does and does not leave standing: ownership *is* enforced by the +models, in both of the two spellings an action can be addressed by. A row id +goes through `db::loadOwned`, which navigates the row to its owner and compares +that with the session principal; an owner name goes through +`bank::resolveOwner`, which compares the caller-supplied `owner` with the +session principal and refuses on a mismatch. Cross-customer isolation is +therefore real and testable — +`another-customer-cannot-touch-your-account.scenario` pins the first spelling +and `an-owner-named-outright-is-checked-against-the-session.scenario` the +second. What is absent is credential *proof*. A server that needs it swaps in `morph::session::SigningAuthorizer` and issues a token to verify against; bank has none to issue. #### Defects the scenario corpus pins on purpose -Three things the corpus asserts as current behaviour because they are true, not -because they are right. A scenario passing over any of them is a record, not an -endorsement; when one is fixed, the file that pins it is meant to fail. - -- **[morph#471](https://github.com/LASTRADA-Software/morph/issues/471) — a - caller-supplied owner beats the session principal.** - `bank::resolveOwner()` ([`include/bank/core/principal.hpp:24`](include/bank/core/principal.hpp)) - returns `action.owner` whenever it is non-empty and only falls back to the - session principal when it is not; nothing compares the two. Ten actions across - eight models resolve their scope through it — `ListAccounts`, `ListCards`, - `ListPayees`, `ListPayments`, `ListLoans`, `ListBudgets`, - `ListNotifications`, `GenerateStatement`, `OpenAccount` and `MarkAllRead` — - so a signed-in customer who types another customer's username is served that - customer's data, and `MarkAllRead` *writes* to it. `SpendingByKind` consults - no owner at all and answers a caller with no session. Actions addressed by - row id are unaffected: they load the row and check its owner, which is the - pattern the ten should follow. - `an-owner-named-outright-is-not-checked-against-the-session.scenario` is the - inventory: every cross-owner read and the `MarkAllRead` write are `expect - ok` there, beside the id-addressed calls that are correctly refused. +Two things the corpus asserts as current behaviour because they are true, not +because they are right. A scenario passing over either of them is a record, not +an endorsement; when one is fixed, the file that pins it is meant to fail. + - **A DTO's `validate()` shadows the model's own `ValidationError`.** Fifteen bank actions carry a `validate()` predicate on the wire DTO *and* open their `execute()` with `if (!action.validate()) throw ValidationError{"…"}`. Over diff --git a/examples/bank/include/bank/core/principal.hpp b/examples/bank/include/bank/core/principal.hpp index 1b6116c00..57b2c66f1 100644 --- a/examples/bank/include/bank/core/principal.hpp +++ b/examples/bank/include/bank/core/principal.hpp @@ -4,6 +4,8 @@ #include #include +#include "bank/core/errors.hpp" + /// @file /// Helpers for reading the authenticated principal from the morph session /// context. The bridge attaches its default session (set once at login via @@ -20,9 +22,39 @@ namespace bank { return {}; } -/// @brief Returns @p explicitOwner if non-empty, otherwise the session principal. +/// @brief Resolves the owner an owner-named action is scoped to: the session +/// principal, always. +/// +/// Ten actions across eight models carry an `owner` field on the wire +/// (`ListAccounts`, `ListCards`, `ListPayees`, `ListPayments`, `ListLoans`, +/// `ListBudgets`, `ListNotifications`, `GenerateStatement`, `OpenAccount` and +/// `MarkAllRead`). The field is **verified, not trusted**: it may name the +/// caller, in which case it is redundant, or it may be left empty, in which +/// case the session principal stands in. Naming anybody else is refused. +/// +/// The alternative — ignoring `action.owner` outright — was rejected because +/// the field is load-bearing on the wire: it is `CustomerModel`'s bridge +/// routing key (`BRIDGE_MODEL_KEY(CustomerModel, ListAccounts, +/// &ListAccounts::owner)`), so a request naming another customer would be +/// routed to that customer's model instance and then quietly served the +/// caller's own rows. Refusing says what happened instead, and matches +/// `db::loadOwned`, which is how the id-addressed half of the same models has +/// always enforced ownership. +/// +/// An empty session principal matches no name at all, so an anonymous caller +/// naming a real customer is refused here rather than served. An anonymous +/// caller naming *nobody* still gets the empty string back: each call site +/// carries its own `owner.empty()` guard and refuses with a message naming +/// what it was about to do. +/// +/// @throws Unauthorized if @p explicitOwner is non-empty and is not the +/// session principal. [[nodiscard]] inline std::string resolveOwner(const std::string& explicitOwner) { - return explicitOwner.empty() ? sessionPrincipal() : explicitOwner; + std::string principal = sessionPrincipal(); + if (!explicitOwner.empty() && explicitOwner != principal) { + throw Unauthorized{"owner does not match the session principal"}; + } + return principal; } } // namespace bank diff --git a/examples/bank/src/models/budget_model.cpp b/examples/bank/src/models/budget_model.cpp index 562ca04a1..965bd6095 100644 --- a/examples/bank/src/models/budget_model.cpp +++ b/examples/bank/src/models/budget_model.cpp @@ -85,12 +85,21 @@ dto::BudgetList BudgetModel::execute(const dto::ListBudgets& action) { } dto::SpendingReport BudgetModel::execute(const dto::SpendingByKind& action) { + // This action is addressed by account id and carries no `owner` field, so + // `resolveOwner` never sees it and cannot be what scopes it. `db::loadOwned` + // is: it navigates the row to its owner and compares that with the session + // principal, the same guard every other id-addressed action uses. Without + // it the report consulted no owner at all and read any account in the + // database, including for a caller with no session. + const auto account = db::loadOwned(mapper(), action.accountId, sessionPrincipal(), "account"); + const auto accountId = static_cast(account.id.Value()); + // Push the account/direction/time filters into the query so only the rows we // aggregate cross the wire; the by-kind rollup stays in code (no GROUP BY SQL). auto rows = mapper() .Query() - .Where(Lightweight::FieldNameOf<&db::TxnRecord::account>, "=", action.accountId) + .Where(Lightweight::FieldNameOf<&db::TxnRecord::account>, "=", accountId) .Where(Lightweight::FieldNameOf<&db::TxnRecord::direction>, "=", static_cast(TxnDirection::Debit)) .Where(Lightweight::FieldNameOf<&db::TxnRecord::createdAtMs>, ">=", action.sinceMs) .All(); @@ -107,7 +116,7 @@ dto::SpendingReport BudgetModel::execute(const dto::SpendingByKind& action) { } dto::SpendingReport report; - report.accountId = action.accountId; + report.accountId = accountId; report.totalDebitsMinor = totalDebits; report.byKind.reserve(byKind.size()); for (const auto& [kind, spend] : byKind) { diff --git a/scripts/scenario/README.md b/scripts/scenario/README.md index ea1ef6e42..51cbbded2 100644 --- a/scripts/scenario/README.md +++ b/scripts/scenario/README.md @@ -125,20 +125,22 @@ scenarios. `--rung bank` and the `bank` column below are therefore spellings of | `bank/` | `ladder_bank_server` | registration and password-only sign-in, accounts and the ledger behind them, overdraft boundaries, transfers, bills and scheduled/standing instructions, cards, loans and amortisation, budgets, notifications, statements; two customers, the anonymous session, and the stateful account cache | Some of the bank files assert behaviour that is **wrong**, deliberately, and -say so in their own header comments. The largest is -[morph#471](https://github.com/LASTRADA-Software/morph/issues/471): -`bank::resolveOwner()` prefers a caller-supplied owner name over the session -principal, so ten actions serve a signed-in customer another customer's data -and `MarkAllRead` writes to it — -`bank/an-owner-named-outright-is-not-checked-against-the-session.scenario` is -the inventory, its cross-owner steps all `expect ok`. `SpendingByKind` -answering a caller -with no session at all, and `GenerateStatement` reporting a live balance in a -field named `closingBalanceMinor`, are pinned the same way. An `expect ok` over -a defect records what the server does today so the day it changes is not a -silent one; it is not an endorsement, and the fix is meant to turn those -assertions red. That is the opposite arrangement from -`broken-on-purpose.scenario`, which fails today by design. +say so in their own header comments: `GenerateStatement` reporting a live +balance in a field named `closingBalanceMinor` is one, and a DTO's `validate()` +shadowing the model's own error message is another. An `expect ok` over a defect +records what the server does today so the day it changes is not a silent one; it +is not an endorsement, and the fix is meant to turn those assertions red. That +is the opposite arrangement from `broken-on-purpose.scenario`, which fails today +by design. + +`bank/an-owner-named-outright-is-checked-against-the-session.scenario` is what +that arrangement looks like after the fix lands. It was +[morph#471](https://github.com/LASTRADA-Software/morph/issues/471)'s inventory, +written entirely `expect ok` because `bank::resolveOwner()` preferred a +caller-supplied owner name over the session principal and ten actions therefore +served a signed-in customer another customer's data. Flipping those assertions +to `expect err` was that issue's regression test, and the file now pins the +enforcement in the same shape it once pinned the defect. The rung a scenario belongs to is its parent directory name — that is how per-rung action coverage is attributed, so a file loose in `scenarios/` is diff --git a/scripts/scenario/scenarios/bank/a-statement-covers-every-account-an-owner-has.scenario b/scripts/scenario/scenarios/bank/a-statement-covers-every-account-an-owner-has.scenario index 009ce00b3..b4e94da5f 100644 --- a/scripts/scenario/scenarios/bank/a-statement-covers-every-account-an-owner-has.scenario +++ b/scripts/scenario/scenarios/bank/a-statement-covers-every-account-an-owner-has.scenario @@ -89,18 +89,30 @@ expect ok field lines ~ "\"accountId\":$busy,[^}]*\"entryCount\":0" # ── An owner with no accounts at all ──────────────────────────────────────── # A statement with no lines, rather than a refusal: the owner exists and simply -# holds nothing. +# holds nothing. Asserted from that owner's own session, because +# `GenerateStatement` resolves its scope through `bank::resolveOwner`, which +# refuses any `owner` that is not the session principal — see +# an-owner-named-outright-is-checked-against-the-session.scenario. client other model=AuthModel do RegisterUser username=perlis-no-accounts password=none-pass displayName="No Accounts" expect ok field message ~ "^(registered|username already taken)$" +do LoginRequest username=perlis-no-accounts password=none-pass +expect ok capture nobody=$.principal +expect ok field ok == true -use paper -do GenerateStatement owner=perlis-no-accounts fromMs=0 toMs=0 +client empty-paper model=StatementModel principal=$nobody +do GenerateStatement owner=$nobody fromMs=0 toMs=0 expect ok field owner == "perlis-no-accounts" expect ok field lines == [] expect ok field totalCreditsMinor == 0 -# An owner who was never registered has no `users` row to resolve, and that is -# reported as an authorization failure rather than as an empty statement. +# Naming somebody else is refused, and refused *before* the `users` row is +# looked up — so a name that exists and one that never did are answered +# identically. That is the point: resolving first and reporting "unknown user: +# …" told an unauthorized caller whether a username existed. +use paper +do GenerateStatement owner=perlis-no-accounts fromMs=0 toMs=0 +expect err message == "owner does not match the session principal" + do GenerateStatement owner=nobody-ever fromMs=0 toMs=0 -expect err message == "unknown user: nobody-ever" +expect err message == "owner does not match the session principal" diff --git a/scripts/scenario/scenarios/bank/an-owner-named-outright-is-checked-against-the-session.scenario b/scripts/scenario/scenarios/bank/an-owner-named-outright-is-checked-against-the-session.scenario new file mode 100644 index 000000000..d27dca695 --- /dev/null +++ b/scripts/scenario/scenarios/bank/an-owner-named-outright-is-checked-against-the-session.scenario @@ -0,0 +1,243 @@ +# bank — an owner named outright is checked against the session. +# +# morph_scenario.py --server ws://127.0.0.1: \ +# scripts/scenario/scenarios/bank/an-owner-named-outright-is-checked-against-the-session.scenario +# +# Bank has two ways of deciding whose data an action touches, and this file +# pins that both of them now reach the same answer. +# +# * By **row id**, through `db::loadOwned` — which navigates the row to its +# owner and compares that with the session principal. +# another-customer-cannot-touch-your-account.scenario is the file that +# pins that half in its own right; the tail of this one re-reads it beside +# the other half, because the two spellings sit on the same models. +# +# * By **owner name**, through `bank::resolveOwner(action.owner)` — which +# returns the session principal and refuses outright when the caller named +# somebody else: +# +# owner does not match the session principal +# +# Ten actions across eight models resolve their scope that way: +# `ListAccounts`, `ListCards`, `ListPayees`, `ListPayments`, `ListLoans`, +# `ListBudgets`, `ListNotifications`, `GenerateStatement`, `OpenAccount` +# and `MarkAllRead`. Every one of them is exercised below. +# +# This file used to assert the opposite. Until morph#471 was fixed +# (https://github.com/LASTRADA-Software/morph/issues/471) `resolveOwner` +# returned whatever string the caller put in the `owner` field and only fell +# back to the session principal when that field was empty — nothing compared +# the two — so every refusal below was an `expect ok` recording a +# confused-deputy read of another customer's data. Flipping those assertions is +# that issue's regression test; the shape of the file is unchanged so the two +# revisions read against each other. +# +# The field is **verified, not ignored**. Naming yourself still works and is +# asserted below, twice: `owner` is `CustomerModel`'s bridge routing key +# (`BRIDGE_MODEL_KEY(CustomerModel, ListAccounts, &ListAccounts::owner)`), so a +# client that fills it in keeps working, and a client that fills in somebody +# else's name is told so rather than quietly handed its own rows. +# +# Two of the ten are **writes**, and for those an error return is not the +# assertion. `MarkAllRead` is followed by a read from the quarry's own session +# showing the notification still unread, and `OpenAccount` by a search of both +# customers' account lists for the row it tried to create. + +model AuthModel + +client mallory-auth +do RegisterUser username=snooper password=snooper-pass displayName="Snooper" +expect ok field message ~ "^(registered|username already taken)$" +do LoginRequest username=snooper password=snooper-pass +expect ok capture snoop=$.principal +expect ok field ok == true + +client target-auth +do RegisterUser username=quarry password=quarry-pass displayName="Quarry" +expect ok field message ~ "^(registered|username already taken)$" +do LoginRequest username=quarry password=quarry-pass +expect ok capture prey=$.principal +expect ok field ok == true + +# ── The quarry's private banking ──────────────────────────────────────────── +client q-vault model=CustomerModel principal=$prey +do OpenAccount owner=$prey kind=0 currency=0 +expect ok capture qAcct=$.id + +client q-teller model=TransactionModel principal=$prey +do Deposit accountId=$qAcct amountMinor=750000 description="quarry's salary" +expect ok field balanceAfterMinor == 750000 +do Withdraw accountId=$qAcct amountMinor=25000 description="quarry's rent" +expect ok field balanceAfterMinor == 725000 + +client q-book model=PayeeModel principal=$prey +do AddPayee name="Quarry's Therapist" iban=CH9300762011623852957 bankName="UBS" +expect ok capture qPayee=$.id + +client q-bills model=PaymentModel principal=$prey +do PayBill fromAccountId=$qAcct payeeId=$qPayee amountMinor=15000 description="a private matter" +expect ok capture qPayment=$.id + +client q-desk model=LoanModel principal=$prey +do ApplyLoan accountId=$qAcct principalMinor=50000 rateBps=0 termMonths=6 +expect ok capture qLoan=$.id + +client q-wallet model=CardModel principal=$prey +do IssueCard accountId=$qAcct kind=0 dailyLimitMinor=5000 +expect ok capture qCard=$.id + +client q-plan model=BudgetModel principal=$prey +do SetBudget category=quarry-secret monthlyLimitMinor=99000 currency=0 +expect ok capture qBudget=$.id + +client q-alerts model=NotificationModel principal=$prey +do Notify message="Quarry's confidential alert" severity=2 +expect ok capture qNote=$.id + +# ── Nothing the snooper can read by simply naming them ────────────────────── +# The session below is `snooper` throughout. Every one of these is refused by +# the same comparison, in the model, before any `users` row is resolved and +# before any query runs. + +client s-vault model=CustomerModel principal=$snoop +do ListAccounts owner=$prey +expect err message == "owner does not match the session principal" + +client s-book model=PayeeModel principal=$snoop +do ListPayees owner=$prey +expect err message == "owner does not match the session principal" + +client s-bills model=PaymentModel principal=$snoop +do ListPayments owner=$prey +expect err message == "owner does not match the session principal" + +client s-desk model=LoanModel principal=$snoop +do ListLoans owner=$prey +expect err message == "owner does not match the session principal" + +client s-wallet model=CardModel principal=$snoop +do ListCards owner=$prey +expect err message == "owner does not match the session principal" + +client s-plan model=BudgetModel principal=$snoop +do ListBudgets owner=$prey +expect err message == "owner does not match the session principal" + +client s-alerts model=NotificationModel principal=$snoop +do ListNotifications owner=$prey unreadOnly=false +expect err message == "owner does not match the session principal" + +client s-paper model=StatementModel principal=$snoop +do GenerateStatement owner=$prey fromMs=0 toMs=0 +expect err message == "owner does not match the session principal" + +# `SpendingByKind` carries no `owner` field at all — just an account id — so +# `resolveOwner` never sees it and the comparison above cannot be the thing +# that refuses it. It goes through `db::loadOwned` instead, the same guard the +# id-addressed actions use, which is why its message is the other shape. It +# lives on `BudgetModel`. Before morph#471 it consulted no owner whatsoever and +# reported on any account in the database. +use s-plan +do SpendingByKind accountId=$qAcct sinceMs=0 +expect err message == "account belongs to a different owner" + +# ── The write that used to reach across: MarkAllRead ──────────────────────── +# `MarkAllRead` resolves its owner the same way the reads above do, so it is +# refused the same way. The error return is only half the assertion. +use s-alerts +do MarkAllRead owner=$prey +expect err message == "owner does not match the session principal" + +# The other half: the quarry's row is *unchanged*, read from the quarry's own +# session. Still exactly one unread notification, still the same message. +use q-alerts +do ListNotifications owner=$prey unreadOnly=false +expect ok field unreadCount == 1 +expect ok field notifications ~ "Quarry's confidential alert" + +# ── The write that used to create a row: OpenAccount ──────────────────────── +# The sharper of the two, because the resolved owner is not a filter here — it +# becomes the owning user of a newly created `accounts` row. `overdraftMinor` +# is the marker: 987654 is a value no other scenario in this corpus opens an +# account with, so its absence from either customer's list is the assertion +# that no row was created — not merely that the call returned an error. +use s-vault +do OpenAccount owner=$prey kind=0 currency=0 overdraftMinor=987654 +expect err message == "owner does not match the session principal" + +# Not in the quarry's name… +use q-vault +do ListAccounts +expect ok field accounts !~ "\"overdraftMinor\":987654" + +# …and not in the snooper's either. The refusal creates nothing anywhere; it +# does not quietly redirect the account to the caller. The snooper has never +# successfully opened an account in any pass of this file, so the list is empty +# outright rather than merely lacking the marker. +use s-vault +do ListAccounts +expect ok field accounts == [] + +# ── Naming yourself still works ───────────────────────────────────────────── +# The field is verified, not ignored. The same request body that was refused +# above is served when it comes from the session it names. That is what keeps +# the rest of this corpus working — most bank scenarios spell `owner=$who` — and +# what keeps `owner` usable as `CustomerModel`'s bridge routing key. +use q-vault +do ListAccounts owner=$prey +expect ok field accounts ~ "\"id\":$qAcct," + +use q-alerts +do ListNotifications owner=$prey unreadOnly=true +expect ok field notifications ~ "\"id\":$qNote," + +# ── Where the line already held ───────────────────────────────────────────── +# The id-addressed spelling was never affected: it loads the row and compares +# its owner. It is the pattern the ten owner-named actions above were changed +# to match, and the two spellings sit side by side on the same models. +client s-single model=AccountModel principal=$snoop +do GetAccount id=$qAcct +expect err message == "account belongs to a different owner" + +use s-alerts +do MarkRead id=$qNote +expect err message == "notification belongs to a different owner" + +use s-plan +do DeleteBudget id=$qBudget +expect err message == "budget belongs to a different owner" + +use s-desk +do GetLoan id=$qLoan +expect err message == "loan belongs to a different owner" + +use s-wallet +do FreezeCard id=$qCard +expect err message == "card belongs to a different owner" + +# ── A name that was never registered is answered identically ──────────────── +# The comparison happens before the `users` lookup, so an unregistered name and +# a registered one give the same refusal. That closes an oracle: the old +# behaviour resolved the name first and reported "unknown user: …", which told +# an unauthorized caller whether a username existed. +use s-vault +do ListAccounts owner=no-such-customer +expect err message == "owner does not match the session principal" + +# ── Left as found ─────────────────────────────────────────────────────────── +# The quarry clears their own notification, from their own session — both so +# this file is re-runnable against the database it has already run on (the +# unread count above must be 1 on every pass) and because it is the positive +# case for the write the snooper was refused. +use q-alerts +do MarkAllRead owner=$prey +expect ok field ok == true +expect ok field message == "1 marked read" + +use q-alerts +do ListNotifications unreadOnly=false +expect ok field unreadCount == 0 + +use q-plan +do DeleteBudget id=$qBudget +expect ok field ok == true diff --git a/scripts/scenario/scenarios/bank/an-owner-named-outright-is-not-checked-against-the-session.scenario b/scripts/scenario/scenarios/bank/an-owner-named-outright-is-not-checked-against-the-session.scenario deleted file mode 100644 index a3a5cd0c5..000000000 --- a/scripts/scenario/scenarios/bank/an-owner-named-outright-is-not-checked-against-the-session.scenario +++ /dev/null @@ -1,173 +0,0 @@ -# bank — the gap between the actions that check who is asking and the ones that -# take an owner's name at face value. -# -# morph_scenario.py --server ws://127.0.0.1: \ -# scripts/scenario/scenarios/bank/an-owner-named-outright-is-not-checked-against-the-session.scenario -# -# This file asserts behaviour that is **wrong**, deliberately, because it is the -# behaviour bank has and a corpus that only recorded the parts that work would -# be worth less than nothing on the day somebody changed them. It is filed as -# morph#471 (https://github.com/LASTRADA-Software/morph/issues/471); the -# `expect ok`s below are that issue's acceptance criteria in reverse, so a -# reader must not take this file passing for an endorsement. -# -# Bank has two ways of deciding whose data an action touches: -# -# * By **row id**, through `db::loadOwned` — which navigates the row to its -# owner and compares that with the session principal. That is genuine -# enforcement, and another-customer-cannot-touch-your-account.scenario is -# the file that pins it. -# -# * By **owner name**, through `bank::resolveOwner(action.owner)` — which -# returns whatever string the caller put in the `owner` field and only falls -# back to the session principal when that field is empty. Nothing compares -# the two. The resolved name is used solely to look a `users` row up. -# -# So every list-shaped action carrying an `owner` field will read another -# customer's data for anyone who types their username, and `MarkAllRead` will -# *write* to it. `SpendingByKind` is worse again: it takes a bare `accountId`, -# consults no owner at all, and reports on any account in the database. -# -# When morph#471 is fixed, this file will fail, and it should: the fix is to -# compare the resolved owner with `sessionPrincipal()` and refuse when they -# differ. The `expect ok`s below are the inventory of everything that change -# has to cover. - -model AuthModel - -client mallory-auth -do RegisterUser username=snooper password=snooper-pass displayName="Snooper" -expect ok field message ~ "^(registered|username already taken)$" -do LoginRequest username=snooper password=snooper-pass -expect ok capture snoop=$.principal -expect ok field ok == true - -client target-auth -do RegisterUser username=quarry password=quarry-pass displayName="Quarry" -expect ok field message ~ "^(registered|username already taken)$" -do LoginRequest username=quarry password=quarry-pass -expect ok capture prey=$.principal -expect ok field ok == true - -# ── The quarry's private banking ──────────────────────────────────────────── -client q-vault model=CustomerModel principal=$prey -do OpenAccount owner=$prey kind=0 currency=0 -expect ok capture qAcct=$.id - -client q-teller model=TransactionModel principal=$prey -do Deposit accountId=$qAcct amountMinor=750000 description="quarry's salary" -expect ok field balanceAfterMinor == 750000 -do Withdraw accountId=$qAcct amountMinor=25000 description="quarry's rent" -expect ok field balanceAfterMinor == 725000 - -client q-book model=PayeeModel principal=$prey -do AddPayee name="Quarry's Therapist" iban=CH9300762011623852957 bankName="UBS" -expect ok capture qPayee=$.id - -client q-bills model=PaymentModel principal=$prey -do PayBill fromAccountId=$qAcct payeeId=$qPayee amountMinor=15000 description="a private matter" -expect ok capture qPayment=$.id - -client q-desk model=LoanModel principal=$prey -do ApplyLoan accountId=$qAcct principalMinor=50000 rateBps=0 termMonths=6 -expect ok capture qLoan=$.id - -client q-plan model=BudgetModel principal=$prey -do SetBudget category=quarry-secret monthlyLimitMinor=99000 currency=0 -expect ok capture qBudget=$.id - -client q-alerts model=NotificationModel principal=$prey -do Notify message="Quarry's confidential alert" severity=2 -expect ok capture qNote=$.id - -# ── Everything the snooper can read by simply naming them ─────────────────── -# The session below is `snooper` throughout. Every one of these succeeds. - -client s-vault model=CustomerModel principal=$snoop -do ListAccounts owner=$prey -expect ok field accounts ~ "\"id\":$qAcct," -# 750000 in, 25000 rent out, 15000 bill out, 50000 loan disbursed in. -expect ok field accounts ~ "\"balanceMinor\":760000" - -client s-book model=PayeeModel principal=$snoop -do ListPayees owner=$prey -expect ok field payees ~ "\"id\":$qPayee," -expect ok field payees ~ "Quarry's Therapist" - -client s-bills model=PaymentModel principal=$snoop -do ListPayments owner=$prey -expect ok field payments ~ "\"id\":$qPayment," -expect ok field payments ~ "a private matter" - -client s-desk model=LoanModel principal=$snoop -do ListLoans owner=$prey -expect ok field loans ~ "\"id\":$qLoan," - -client s-plan model=BudgetModel principal=$snoop -do ListBudgets owner=$prey -expect ok field budgets ~ "\"id\":$qBudget," -expect ok field budgets ~ "quarry-secret" - -client s-alerts model=NotificationModel principal=$snoop -do ListNotifications owner=$prey unreadOnly=false -expect ok field notifications ~ "Quarry's confidential alert" -expect ok field unreadCount == 1 - -client s-paper model=StatementModel principal=$snoop -# Pinned to this pass's account rather than to the statement's totals: a -# statement fans out over every account its owner has ever held, and this file -# opens one more on each pass. 25000 rent plus a 15000 bill. -do GenerateStatement owner=$prey fromMs=0 toMs=0 -expect ok field owner == "quarry" -expect ok field lines ~ "\"accountId\":$qAcct,[^}]*\"debitsMinor\":40000," - -# `SpendingByKind` needs no owner at all — just an account id, which -# `ListAccounts` above has already handed over. It lives on `BudgetModel`. -use s-plan -do SpendingByKind accountId=$qAcct sinceMs=0 -expect ok field accountId == $qAcct -expect ok field totalDebitsMinor == 40000 - -# ── And the one that lets a stranger *write* ──────────────────────────────── -# `MarkAllRead` resolves its owner the same way, so this marks the quarry's -# notification read from the snooper's session. -use s-alerts -do MarkAllRead owner=$prey -expect ok field ok == true -expect ok field message == "1 marked read" - -use s-alerts -do ListNotifications owner=$prey unreadOnly=false -expect ok field unreadCount == 0 - -# ── Where the line actually holds ─────────────────────────────────────────── -# The moment an action names a *row* instead of an owner, the guard bites — and -# the two spellings sit side by side on the same model. -client s-single model=AccountModel principal=$snoop -do GetAccount id=$qAcct -expect err message == "account belongs to a different owner" - -use s-alerts -do MarkRead id=$qNote -expect err message == "notification belongs to a different owner" - -use s-plan -do DeleteBudget id=$qBudget -expect err message == "budget belongs to a different owner" - -use s-desk -do GetLoan id=$qLoan -expect err message == "loan belongs to a different owner" - -# ── An owner that was never registered is still refused ───────────────────── -# The resolved name has to reach a `users` row, so this is a lookup failure -# rather than an authorization one — which is itself a way of asking the server -# whether a username exists. -use s-vault -do ListAccounts owner=no-such-customer -expect err message == "unknown user: no-such-customer" - -# Left as found. -use q-plan -do DeleteBudget id=$qBudget -expect ok field ok == true diff --git a/scripts/scenario/scenarios/bank/banking-without-a-session-is-refused.scenario b/scripts/scenario/scenarios/bank/banking-without-a-session-is-refused.scenario index f6d43db5f..4a4ebd976 100644 --- a/scripts/scenario/scenarios/bank/banking-without-a-session-is-refused.scenario +++ b/scripts/scenario/scenarios/bank/banking-without-a-session-is-refused.scenario @@ -17,15 +17,23 @@ # see "no session" as a special case — it sees an empty principal, and every # owner-resolving action refuses on that. # -# The refusals come in two shapes, and which one an action gives away is a +# The refusals come in three shapes, and which one an action gives away is a # property of how it resolves its owner, not of how important it is: # -# * "no session principal" — the action checked for an empty owner itself. -# * "... belongs to a different owner" — the action did not check, and the -# empty string simply failed to match the row's real owner further down. +# * "no session principal" — the action carries an `owner` field, the caller +# left it empty, `bank::resolveOwner` returned the empty principal, and the +# action's own guard refused it. +# * "owner does not match the session principal" — the action carries an +# `owner` field and the caller *filled it in*. `resolveOwner` compares it +# with the session principal and refuses, and an empty principal matches +# nothing. This is the shape morph#471 added; before it, naming an owner +# was how an anonymous caller got served that owner's data. +# * "... belongs to a different owner" — the action is addressed by row id, +# so there is no `owner` field to check, and the empty principal simply +# failed to match the row's real owner further down. # # `SchedulePayment` and `CreateStandingOrder` are the clearest case of the -# second shape: unlike their sibling `PayBill` they have no empty-owner guard at +# third shape: unlike their sibling `PayBill` they have no empty-owner guard at # all, so an anonymous caller is turned away by the ownership comparison instead. model AuthModel @@ -141,26 +149,52 @@ expect err message == "account belongs to a different owner" do CloseAccount id=$acct expect err message == "account belongs to a different owner" -# ── One action an anonymous client can still reach ────────────────────────── -# `SpendingByKind` consults no owner and no session at all, so it answers -# anybody. Asserted here for the same reason as in -# an-owner-named-outright-is-not-checked-against-the-session.scenario: it is the -# behaviour bank has, and a corpus that quietly omitted it would let the day it -# changes pass unnoticed. It is recorded under morph#471 -# (https://github.com/LASTRADA-Software/morph/issues/471) as an aggravating -# detail beside that issue's ten owner-resolving actions, so this `expect ok` -# is a record rather than an endorsement. use anon-plan do SpendingByKind accountId=$acct sinceMs=0 -expect ok field accountId == $acct -expect ok field totalDebitsMinor == 0 +expect err message == "account belongs to a different owner" + +# ── Refused by the owner comparison instead ──────────────────────────────── +# The shape morph#471 added. Everything above left `owner` empty, which is the +# only case the old `resolveOwner` consulted the session for; filling it in was +# how an anonymous caller reached a real customer's data, because the field was +# taken verbatim and compared with nothing. It is now compared with the session +# principal, and an empty principal matches no name at all — so the anonymous +# client is refused before any `users` row is looked up. +use anon-vault +do ListAccounts owner=$who +expect err message == "owner does not match the session principal" + +do OpenAccount owner=$who kind=0 currency=0 overdraftMinor=876543 +expect err message == "owner does not match the session principal" + +use anon-book +do ListPayees owner=$who +expect err message == "owner does not match the session principal" + +use anon-alerts +do ListNotifications owner=$who unreadOnly=false +expect err message == "owner does not match the session principal" + +do MarkAllRead owner=$who +expect err message == "owner does not match the session principal" + +use anon-paper +do GenerateStatement owner=$who fromMs=0 toMs=0 +expect err message == "owner does not match the session principal" # ── Nothing anonymous left a mark ─────────────────────────────────────────── +# The balance is untouched, and the account the anonymous `OpenAccount` above +# named the member as the owner of was never created: 876543 is a marker +# overdraft no other scenario in this corpus opens an account with. client check model=AccountModel principal=$who do GetAccount id=$acct expect ok field balanceMinor == 100000 expect ok field status == 0 +use member +do ListAccounts +expect ok field accounts !~ "\"overdraftMinor\":876543" + # ── Installing the principal is the whole difference ──────────────────────── # Same connection as the anonymous vault above, one `session` step later. use anon-vault