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).
The description said the username is replaced with DELETED-<random-string>. Rendered as markdown that is an unclosed HTML element, so the description does not parse as XML, which API Explorer and ResourceDocsTest both require (they parse the first few descriptions of the list). The list is unordered, so whether the test hits this one depends on which ResourceDocs exist: adding a ResourceDoc anywhere can move it into the first three and fail ResourceDocsTest. Put the text in a code span. Every one of the 805 v6.0.0 descriptions now parses.
The parity audit compares each ResourceDoc description with the Lift baseline. The deleteUser description now puts DELETED-<random-string> in a code span so that it parses as XML; record that as a reviewed difference, using the helper, rather than editing the baseline.
There was already a reviewed entry for the deleteUser description, with the digest of the text before the code span. It now matched nothing, which the audit rejects as stale. Keep one entry, with the current digest and both reasons.
Activating a consent set it valid and then bound it to the PSU. A stop between the two left a consent that was valid and bound to nobody, which the resumption read as no longer waiting for authorisation and refused on every attempt, so it could never be repaired. Bind before the status changes: the status is now the last step, so a stop leaves the consent received, with its access granted and perhaps bound, and running the activation again completes it. A consent left valid and unbound by the earlier order is completed by binding it. A consent valid for another PSU is still refused.
…t again A payment records the principal that lodged it and, when it was lodged for somebody, the one it was lodged for, and the rule that lets a TPP address a payment accepts either. The basket read the PSU only from the second, so a payment a PSU lodged themselves, with nothing recorded as lodged for, joined a basket that named nobody. When an authorisation was started a different PSU could then be bound to it, and that PSU authorised a payment that was not theirs. Read the PSU from whichever of the two is a person (the TPP's own pseudo-user is not), judged against the consumer that created the basket rather than the caller, since the SCA front end is not the TPP. Check it again where the PSU is fixed: when the authorisation is started, and when the answer is checked, because members can change between the two. A PSU that every member naming one is not for is refused like any other attempt to address the basket.
Admission to a basket takes a payment stored RCVD or INITIATED, but the executor treated only RCVD as waiting for authorisation, so a member that was let in failed when its turn came. Both now read the same set.
The payment routes record COMPLETED after booking; the basket executor recorded only its own member state. On a connector that does not set the status itself the payment stayed RCVD while its member was DONE and the basket ACTC, where the outdated-payment task could still reject it. Say COMPLETED after booking, and also where a payment is found already booked or is reconciled, since the status may be what an interrupted run missed. A failure to record it is logged and does not undo the booking.
The members' execution rows were written after the basket was claimed and their result was ignored, and a basket with no rows counted as having every member done, so a run that stopped between the claim and the rows, or one whose rows could not be written, finished ACTC with nothing booked. Record the members from the basket itself whenever it is executed (the call is idempotent, so the resumption repairs such a basket), and treat a basket with no members as not complete. Also decide whether the booking connector is the mapped one from the connector actually in use: with a single configured connector the method routing table is not consulted, so a REST or RabbitMQ backend that timed out after booking was treated as mapped, recorded FAILED and booked again by the retry, instead of being left UNKNOWN.
…nswer fails for good The answer was accepted whenever the connector returned a challenge, but a connector can return the challenge itself with a failed status for a wrong one-time password. Only a challenge recorded as finalised authorises the basket now. When the answer fails for good (the connector reports failed, or the allowed attempts are used up) the basket is rejected with its payments and frees its members, as the payment authorisation does for a failed answer, so a basket cannot be given a fresh authorisation and a fresh allowance of guesses over and over.
Take every timestamp the provider writes from the JVM, as the Mapper's own columns are, instead of the database clock: a database in another zone shifted the lease by hours, so a basket still being executed could be taken over or never be resumed. Move a basket that was looked at but did not finish behind the ones not yet tried. The resumption takes the oldest few unfinished baskets, and a stuck basket never moved, so enough of them kept every later basket from being resumed. Answer two requests that race for the same member with the same refusal whichever way the loser lost, including when the unique index rather than the check rejected it.
…ket that cannot progress A request's database work is one transaction that commits when the response is sent. The claim of a basket, the member states and the release of its members were written inside it, so a node that died after a remote connector had booked a payment and before the response lost all of it: the basket was RCVD again, nothing said a booking was under way, and the same answer could be sent once more. Write the ledger on a connection taken from the pool directly and commit it at once. Nothing else the request writes touches those rows, so the two connections do not wait for each other. With the ledger surviving, a member left UNKNOWN on the mapped connector has no transaction id exactly when its booking did not commit, so the resumption now records it as FAILED and claims it again; other connectors still leave it for an operator. Add the end an incomplete basket never had: when every member that is not done has failed as often as it is allowed to, the basket is EXECUTION_FAILED (reported RCVD), is no longer picked up, and frees what it held. Run the reads of an execution inside a Future, so that an exception from one basket is that basket's failure and the resumption still reaches the others and moves the failing one to the back of the queue. Correct the comment on how a refused creation is rolled back.
…rs per basket A payment held by an active basket could also be authorised on its own, and each authorisation would book it once. Refuse starting and answering the payment's own authorisation while a basket holds it (409 STATUS_INVALID). Require every payment of a basket to be waiting for SCA when its authorisation is answered, as the executor does, instead of only not COMPLETED: a payment rejected, cancelled or failed in the meantime stopped the run after the other members had been booked. Count wrong answers for the basket over all its authorisations. Each new authorisation brings a new one-time password and a new allowance on its own challenge, so a TPP could keep guessing by starting authorisations; the answer that uses the basket's allowance up now rejects the basket and its payments.
…pped one A connector that does not accept a one-time password can return the challenge itself with a failed or unfinished status, and a booking can fail on a connector that may have booked before it failed. Neither could be produced by the mapped connector the suite runs on, so both rules were only read, not run. Replace the connector with one that answers named methods itself and passes the rest to the connector in use. A challenge reported as failed is a refusal and rejects the basket with its payments; one that is not finalised authorises nothing and leaves the basket as it was; a booking failure is recorded UNKNOWN and never retried, where the same failure on the mapped connector is FAILED and claimed again.
|
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.
No description provided.