Repository navigation
Conversation
…ations
Extend SigningBasketServiceSBSApiTest. Each assertion cites the line of the
NextGenPSD2 1.3.16 OpenAPI file or the Implementation Guidelines section it
rests on, and checks the database as well as the HTTP response.
Covered: upper-case transactionStatus, {href} links, Location and
ASPSP-SCA-Approach on create, empty and duplicate id lists, unsupported
authorisation body variants, transitions after delete and after
authorisation, unknown baskets and authorisations, ownership by TPP,
quarantine of baskets created before ownership was recorded, member
admission, replay of a finalised challenge against another basket, and a
delete racing the final answer.
These scenarios fail on the current implementation by design and are
turned green by the commits that follow. Two scenarios (consent
activation, payment booking) are ignored; they record the target of the
execution phase.
The standard's enumeration for baskets (RCVD, PATC, ACTC, CANC, RJCT) is upper case. The create, get and status responses lower-cased the stored value. Also correct the ResourceDoc examples, which showed ACCP, a code that is not used for baskets. TPPs that matched the lower-case strings must now match the upper-case ones.
The start-authorisation response carried _links.scaStatus as a bare
string; the standard defines it as a hrefType object ({"href": ...}).
Use a basket-specific response type so the payment and consent
authorisation responses, which share the old type, are left alone, and
replace the ResourceDoc example, which listed links the endpoint never
returns.
The PUT on a basket authorisation reused the payment response builder, so
its _links.scaStatus pointed at /payments/sepa-credit-transfers/{id}.
Build the scaStatusResponse for the basket instead: scaStatus plus a
{href} link to /signing-baskets/{basketId}/authorisations/{id}. The body
no longer carries authorisationId, which scaStatusResponse does not
define. Correct the ResourceDoc example to match.
Each list in the body has minItems 1 and the body must carry at least one
entry. A request with {"paymentIds": []} was accepted and produced an
empty basket. Refuse an empty list, and a list that names the same id
twice, with the format error the endpoint already uses for a malformed
body.
POST authorisations never read its body, so a request carrying PSU credentials (updatePsuAuthentication) or an authentication method choice was answered with a fresh challenge and the data discarded. The PUT treated any body without scaAuthenticationData as malformed, giving the confirmation-code and PSU-data variants a format error instead of a statement that they are unsupported. Dispatch on the body variant: an empty or transactionAuthorisation body is accepted; updatePsuAuthentication, selectPsuAuthenticationMethod and authorisationConfirmation are refused with a new error (OBP-35050, reported as SERVICE_INVALID); anything else stays a format error.
…eated The Implementation Guidelines make Location mandatory on a created resource and require ASPSP-SCA-Approach when the approach is fixed, as it is per instance. Neither was sent: Location by no Berlin Group endpoint, and ASPSP-SCA-Approach only on URLs ending in /consents. Send Location on POST /signing-baskets, built from the path the request arrived on so alias paths are honoured, and ASPSP-SCA-Approach on the two calls that create a basket resource. Add an additive response helper that lets a handler derive headers from the created resource.
…changes A signing basket carried only its id and status, so any authenticated caller holding the id could read, cancel or authorise it. Store the consumer that created it, the PSU it is for once known, and the creation time, and make the owner readable through SigningBasketTrait (absent on baskets created before this change). Add the provider operations the following changes build on: transitionSigningBasketStatus is one conditional UPDATE, so callers racing for different transitions out of the same status have exactly one winner; bindSigningBasketPsu binds the PSU once. Creation writes the basket and its members together and removes what it wrote if any step fails. The existing columns are untouched; Schemifier adds the new ones.
The consent authorisation endpoints turn the PSU-ID header into a user id before calling Consent.resolveBerlinGroupPsu, in a private helper of the AIS routes. The signing basket authorisation needs the same step. Move the helper into Consent so both use one implementation; the AIS routes keep their call sites. No behaviour change.
Any authenticated caller who knew a basket id could read it, cancel it or start and answer its authorisation. Every operation that names a basket now goes through one guard, with permissions stated per operation: - reading, the status, and deleting: the creating TPP only; - the authorisation operations: the creating TPP, or the consumer declared in sca_front_end_consumer_ids for the PSU the basket is for. The rule is Consent.checkBerlinGroupConsentAccess, so a basket and a consent are held to the same standard. A basket created before ownership was recorded has no creating TPP and is refused to everyone; the consent prop that re-opens unowned consents does not apply to it. An unknown basket and one the caller may not address get the same answer, 403 reported as RESOURCE_UNKNOWN, so the endpoint does not reveal which ids exist (OBP-35051). An authorisation id the basket does not have, or one issued for another basket, is 404 RESOURCE_UNKNOWN (OBP-35052); the status read no longer answers 200 with a made-up scaStatus, and the wrong ConsentNotFound message is gone from baskets. An answer can no longer be routed to a basket the authorisation was not issued for.
… TPP POST authorisations created the challenge for the session's user. For a client-credentials TPP that is its own pseudo-user, so the one-time password was sent to the TPP and never reached the PSU. Resolve the PSU as the consent authorisation does, with Consent.resolveBerlinGroupPsu: the PSU the basket already names, a genuine PSU in the session, then the PSU-ID header. Bind that PSU to the basket and mint the challenge for them. A header that contradicts the bound PSU is refused like any other attempt to address the basket; with no PSU identifiable the call is refused with PSU_CREDENTIALS_INVALID.
…ng anything The PUT started booking payments and saved the basket as ACTC before it looked at the result of checking the answer, and decided what to do from a re-read of the challenge rather than from that result. Presenting a challenge that had already been finalised, with a wrong answer, against another basket made that basket ACTC and marked its payments completed while the response was an error. The connector's challenge check goes by challenge id alone, so nothing tied the challenge to the basket. Rework the order: 1. the caller may address this basket and this authorisation; 2. the request can succeed at all: the instance enables it, the basket holds no consent, the basket is RCVD, the authorisation is not already finalised or failed, every payment exists and is not already booked; 3. the answer is checked as the PSU the challenge was minted for, so a TPP relaying the PSU's one-time password works; 4. the basket is claimed with one conditional update, RCVD to an internal AUTHORISING state that is reported as RCVD; losing it is a 409, so a delete racing the final answer has one winner; 5. only then do the payments change and the basket becomes ACTC. A wrong, expired or used-up one-time password is now 401 PSU_CREDENTIALS_INVALID, the standard's code for an incorrect OTP, and an authorisation answered twice is 409 STATUS_INVALID. Neither reached a code from the standard's list before. Add signing_basket_authorisation_enabled, default false. Answering an authorisation still starts the booking of the payments without waiting for its outcome, and a basket can report itself authorised while a payment was not booked; until that is replaced, a basket is not authorised on an instance that has not opted in (403 SERVICE_BLOCKED). A basket that holds a consent is refused with 400 SERVICE_INVALID, because nothing activates the consent.
…s it DELETE set the basket to CANC whatever its status, so an authorised basket (ACTC) became cancelled, and POST authorisations minted a challenge for a cancelled basket. Both now require the status the standard's rule implies. DELETE: allowed while the basket is RCVD and no authorisation of it is finalised (L3399); the change is one conditional update from RCVD, so a final answer racing it has exactly one winner. Anything else is 409 STATUS_INVALID. Deleting an already cancelled basket answers 204 and changes nothing. POST authorisations: 409 STATUS_INVALID unless the basket is RCVD. The unconditional status writers are removed from the provider; every status change now goes through transitionSigningBasketStatus.
…ning baskets Http4sBGv13PIS.getOwnPaymentImpl decides whether a caller may address a payment: the TPP that lodged it, acting as a principal that is party to it. A signing basket has to ask the same question about every payment it is asked to hold, so move the rule, unchanged, into BerlinGroupPaymentAccess. The eleven payment routes keep calling their private helper, which now delegates. No behaviour change.
… time Nothing stopped the same payment being put in two baskets, so two authorisations could each try to book it. Record which basket holds a member in a claim row, unique on the member, written together with the basket. A basket naming a member another active basket holds is refused with 409 REFERENCE_STATUS_INVALID and leaves nothing behind. The claim is released when the basket is cancelled or authorised, so the member can join another basket. There is no permanent unique constraint on the member itself. A payment and a consent that happen to share an id do not collide: the claim key carries the member type. The check in front of the insert gives the usual answer, and the unique index decides when two requests get past it together. Members are now read back in the order they were submitted.
…asket A basket was created for any ids at all: invented ones, payments another TPP lodged, payments already booked. Admit each member against what it is. A payment must be one the caller may address, under the rule the payment routes use, and still awaiting SCA, and must be a SEPA credit transfer. A consent must be one the caller may address under the consent rule, created through the Berlin Group API, and not yet authorised or ended. A member that does not exist and one that is not the caller's are answered alike (400 RESOURCE_UNKNOWN), so the endpoint does not reveal which ids exist; one in the wrong state is 409 REFERENCE_STATUS_INVALID; one the ASPSP does not accept is 400 REFERENCE_MIX_INVALID. Where members name a PSU they must all name the same one, and it must be the PSU the request names (a genuine PSU in the session, or PSU-ID). A client-credentials TPP's own pseudo-user is not a PSU and is ignored. The basket records that PSU; otherwise it is bound when an authorisation is started. The PSP role needed follows the members: PISP for payments, AISP for consents, both for a mix. Periodic payments cannot be excluded: the recurrence of a periodic payment is never stored, so one cannot be told from a single payment.
Every signing basket operation demanded the payment initiation role, which does not fit a basket of consents. The role now follows the members: PISP for payments, AISP for consents, both for a mix. On an existing basket it is checked after the caller is known to be entitled to it, so a role check cannot be used to tell which baskets exist, and it is not asked of the ASPSP's own SCA front end, which drives the authorisation under Redirect without a certificate of its own. Add SigningBasketSignedRequestTest. The shared test setup runs with no mandatory headers and no certificate check, so nothing exercised the request signature, the mandatory headers or the PSP role on these routes. It registers the TPP's certificate as a regulated entity and signs every call: unsigned and unregistered requests are refused, a TPP with the payment initiation role creates a basket and receives Location and ASPSP-SCA-Approach, and one with only the account information role is refused with ROLE_INVALID.
Add the new basket errors to each operation's ResourceDoc, and correct a typo in the delete description.
The test-isolation lint reads a def without braces as class-body code and rejects setPropsValues in it. Give the two signing basket helpers braces.
UserReferenceAttributionPolicyTest requires every column that looks like a user id to be named in UserReference. MappedSigningBasket.PsuUserId was not. The PSU is resolved explicitly, by Consent.resolveBerlinGroupPsu, from the bound PSU, a genuine PSU session or the PSU-ID header; the calling agent is never stored and a client-credentials TPP's pseudo-user is never written. Nothing is delegated, so there is nothing to look up, which is the UseAuthenticatedUserId policy, as for the signing basket challenge's expected user.
Executing a basket's authorisation touches several payments and, later, consents, and the standard's basket statuses cannot say that the first payment was booked and the second refused. Keep that per member instead, in a new table with one row per member, unique on (basket, type, member): PENDING, EXECUTING, DONE, FAILED or UNKNOWN, a detail, and the number of attempts. Every move is one conditional update from the states the caller names, so two executors reaching for the same member have exactly one winner, and a claim counts as an attempt. A member left EXECUTING past a lease becomes UNKNOWN, because nothing records whether it took effect. A second stored basket status, EXECUTION_INCOMPLETE, is reported as RCVD like AUTHORISING. Nothing uses these yet.
Answering a basket's authorisation started the booking of its payments without waiting for it and wrote ACTC at once, so the response could say 200 and ACTC while nothing moved. Replace that with an execution service: - each payment is claimed with a conditional update before it is touched; - it is booked through the same connector call the payment routes use, and the call is awaited, so the money has moved when the response is sent; - a payment that already carries a transaction id is never booked again, which makes a repeat or a resumption safe on the mapped connector; - the first member that does not finish stops the run; - the basket becomes ACTC only when every member is DONE, and the members are then released. Otherwise it is stored EXECUTION_INCOMPLETE and reported as RCVD, and each member's own state says what happened; - a failed payment is retried, a limited number of times, only on the mapped connector. On another connector a failure is recorded UNKNOWN, since the connector may have booked before it failed; - a member left UNKNOWN is reconciled by its transaction id, otherwise it is left for an operator. The response keeps scaStatus finalised, the SCA having succeeded, with a message to the PSU when not every payment could be executed. Several payments are still not one transaction: a failure leaves the earlier ones booked.
The standard's statuses cannot say that a basket's first payment was
booked and its second was not. Add GET /signing-baskets/{basketId}/execution,
an extension of the ASPSP, that returns the basket's reported status and,
for each member, its type, id, state, detail and the number of attempts.
Only the TPP that created the basket may read it; anyone else, and an
unknown basket, get the same 403 as every other basket operation.
If the process booking a basket's payments dies after the basket was claimed, the basket stays AUTHORISING and its members stay held, and nothing picked it up. Add a scheduled task that marks members left EXECUTING past a lease as UNKNOWN and executes every basket that has sat AUTHORISING or EXECUTION_INCOMPLETE for the lease again from where it stopped. Members already DONE are skipped, a payment that carries a transaction id is never booked again, and a member left UNKNOWN is reconciled by its transaction id or left for an operator. Several nodes may run it at once: every member and every status change is claimed with a conditional update, and a test with three concurrent executors confirms each payment is booked once. The interval and the lease are configurable, and the task is on by default.
The consent authorisation binds the consent to the PSU, by copying the PSU's authentication context onto it and making the PSU its user, in two steps written inline in the route. A signing basket that activates a consent has to do the same. Move the two steps into Consent.bindBerlinGroupConsentToPsu, which the route now calls, and add BerlinGroupConsentActivation.activate: grant the access an allAccounts consent leaves open, mark the consent valid, bind it. It is idempotent, so a basket resumed after a stop can run it again. The consent authorisation route behaves as before (consent and AIS suites unchanged).
…answered A basket holding a consent could not be authorised: the PUT refused it, because nothing made the consent valid. Execute consent members as well. Before the answer is checked, the consents must exist and still be waiting for authorisation, and the PSU must hold the accounts each names, as when the PSU authorises a consent on its own; none of that changes anything. After the answer, each consent is activated in order with the payments: its access is granted, it becomes valid, and it is bound to the PSU the basket was authorised by. Activation is idempotent, so a consent may be claimed again, up to the attempts allowed, and a consent already valid and bound to the PSU is simply DONE. A PSU who does not hold a consent's accounts is refused with 403 CONSENT_UNKNOWN, the standard's code for a consent that cannot be found with respect to the PSU. The same refusal on the consent authorisation route used to carry the HTTP status in place of a code.
Describe what the stored statuses and the per-member states mean, the properties that govern execution and its resumption, how to find baskets that did not complete, how to reconcile a member left UNKNOWN, and what to do with baskets created before ownership was recorded (quarantined, kept for audit, assigned explicitly if one has to be revived).
|
Owner
Author
|
Superseded by OpenBankProject#2934, which targets origin develop and carries the same branch with the execution work added. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix: signing baskets follow NextGenPSD2 1.3.16 and belong to the TPP that created them
Base:
a5c5f53ac(origin/develop, Merge pull request OpenBankProject#2932). Branch:feature/signing-basket-conformance, 20 commits.What this changes
Response shape and validation (one commit each)
transactionStatusis upper case (RCVD, notrcvd)._links.scaStatusis a{ "href": ... }object; the PUT answer is a basket response linking to/signing-baskets/{basketId}/authorisations/{id}instead of a payment URL.FORMAT_ERROR.updatePsuAuthentication,selectPsuAuthenticationMethod,authorisationConfirmation) are refused with 400SERVICE_INVALIDinstead of being discarded.Locationis sent on the create response, andASPSP-SCA-Approachon the two calls that create a basket resource.Ownership and state
sca_front_end_consumer_ids) acting for the right PSU. The rule isConsent.checkBerlinGroupConsentAccess.RESOURCE_UNKNOWN. An authorisation id the basket does not have answers 404.RCVDand no finalised authorisation (409STATUS_INVALIDotherwise). POST authorisation needsRCVD.RCVD -> AUTHORISINGclaim, and only then the members. A challenge already finalised for one basket can no longer be replayed against another to execute it. A wrong, expired or used-up one-time password is 401PSU_CREDENTIALS_INVALID; an authorisation answered twice is 409.BerlinGroupPaymentAccess; the payment routes delegate to it.Behaviour changes visible to TPPs
transactionStatus: "rcvd""RCVD"(D6)OBP-50200, or success for any holder of the idRESOURCE_UNKNOWN(D1)CANCSTATUS_INVALID(D3)STATUS_INVALID(D4)[], duplicates, invented or already booked idsFORMAT_ERROR/ 400RESOURCE_UNKNOWN/ 409REFERENCE_STATUS_INVALID(D5, D8)SERVICE_INVALID(D7)SERVICE_BLOCKEDunlesssigning_basket_authorisation_enabled=true(default false) (D9)SERVICE_INVALID(D10)PSU_CREDENTIALS_INVALIDNot fixed by this change
Phase 2 does not make payment execution safe. Answering an authorisation still starts the booking of the payments without waiting for it and without reading the outcome, and writes
ACTCregardless. On the baseline a PUT returns 200, stores the payment asCOMPLETEDand the basket asACTC, and neither account moves, even after eight seconds (the target scenario is recorded as an ignored test). Until that is reworked, a basket PUT can report success for money that did not move. This is whysigning_basket_authorisation_enableddefaults to false. Do not release this change on its own as a fix for signing baskets.Remaining for the next phase: the payment execution service with a persisted claim and crash recovery, consent activation and mixed baskets, and three decisions that block it: how a basket represents partial execution, cross-connector idempotency, and what happens to quarantined baskets.
A crash between the
AUTHORISINGclaim and the finalACTCleaves a basket inAUTHORISING(reported asRCVD) and its members held; nothing recovers it yet.Where the task book did not match reality
ACTCand its paymentsCOMPLETEDwhile the response was an error.OBP-40062) was confirmed by reading the code and an existing PIS test, not by an end-to-end run:sca_front_end_consumer_idsis read once at startup, so a test cannot change it.mvn -o test -pl obp-commons,obp-api -DwildcardSuites=...with theOBP_*environment); the task book said they abort.ASPSP-SCA-Approachwas only sent for URLs ending/consents, andLocationis sent by no Berlin Group endpoint. This is not specific to baskets.periodic_payments, the provider compares toperiodic-payments; correcting the comparison makes periodic payment creation return 500).passesPsd2Pisp/passesPsd2Aispdo nothing unlessrequirePsd2Certificates=ONLINE, so the role changes only have an effect there.OBP-40016,OBP-20211,OBP-40014) fell outside the standard's list on every Berlin Group endpoint, becauseBerlinGroupErrorreturns the HTTP status for unmapped codes. Only the basket path was changed.Verification
run_tests_parallel.sh: 4258 tests, 1 failure:DynamicQueryTest"a Dynamic Query is checked when created, must be GET, and needs no user-supplied code". It fails the same way ona5c5f53acand is unrelated. Run against a private copy of~/.m2so the shared repository was not touched.SigningBasketServiceSBSApiTest46 scenarios (2 ignored targets),MappedSigningBasketProviderTest,SigningBasketAccessTest,SigningBasketSignedRequestTest(signed requests with a registered certificate), and the BG PIS (29) and AIS / consent access suites pass.scripts/check_lift_http4s_resource_doc_parity.pyis clean.