diff --git a/apps/api-manager/src/routes/health/+server.ts b/apps/api-manager/src/routes/health/+server.ts index 2fe11f12..f7788a61 100644 --- a/apps/api-manager/src/routes/health/+server.ts +++ b/apps/api-manager/src/routes/health/+server.ts @@ -1,14 +1,27 @@ import type { RequestHandler } from './$types'; -import { healthCheckRegistry } from '@obp/shared/health-check'; -import { get } from 'svelte/store'; +/** + * Liveness probe: this process is up and serving HTTP. It deliberately checks nothing else. + * + * It used to require every monitored dependency to be healthy and answer 503 otherwise, which is + * readiness logic in a liveness endpoint. An orchestrator polling /health would then restart a + * perfectly functional API Manager because a secondary OAuth2 provider was down — it keeps serving + * pages either way, so killing it only widens an unrelated outage. + * + * It also flattened the nuance /status already encodes: summarizeHealth treats the OAuth2 providers + * as one group, so one dead provider alongside a working one is 'partial', not a failure. `every()` + * turned any single unhealthy check — including a provider that was never configured — into a hard + * 503. And it counted "no checks registered" as healthy, the opposite of the rule summarizeHealth + * states for that case ('unknown', never 'healthy'). + * + * Ask /ready for dependency health -- it is the readiness half of this split and answers 503 when + * the dependencies are not there, so a load balancer can route on it. /status renders the same + * verdict for a human but is a page and always answers 200, so it cannot serve as a probe. + * This /health matches the one OBP-API and Hola serve. + */ export const GET: RequestHandler = async () => { - const snapshots = get(healthCheckRegistry.getStore()); - const services = Object.values(snapshots); - const healthy = services.length === 0 || services.every((s) => s.status === 'healthy'); - - return new Response(JSON.stringify({ status: healthy ? 'ok' : 'error' }), { - status: healthy ? 200 : 503, + return new Response(JSON.stringify({ status: 'ok' }), { + status: 200, headers: { 'Content-Type': 'application/json' } }); }; diff --git a/apps/api-manager/src/routes/ready/+server.ts b/apps/api-manager/src/routes/ready/+server.ts new file mode 100644 index 00000000..3802f378 --- /dev/null +++ b/apps/api-manager/src/routes/ready/+server.ts @@ -0,0 +1,62 @@ +import type { RequestHandler } from './$types'; +import { healthCheckRegistry, summarizeHealth } from '@obp/shared/health-check'; + +/** + * Services this app can serve its purpose without. + * + * Empty here: API Manager registers no optional dependency today. The list exists so that when one + * is added, excusing it from readiness is a decision written down in this file. + * + * Named exclusions rather than a list of required services on purpose: a service added later gates + * readiness by default, and someone has to decide, in this file, that it is safe to ignore. The + * other way round, a rename would silently drop a service from the required set and readiness would + * quietly start passing on less than it used to. + */ +const NOT_REQUIRED_TO_SERVE: string[] = []; + +const OAUTH2_PREFIX = 'OAuth2: '; + +/** + * Readiness probe: this instance can do useful work, so route traffic to it. + * + * The other half of the split /health introduced. /health is liveness -- "the process is serving + * HTTP" -- which is what an orchestrator should restart on. Readiness is what a load balancer should + * route on, and it needs a status code rather than a rendered page: /status shows the same picture + * to a human but is a +page and answers 200 whatever it finds. + * + * Deliberately not summarizeHealth's overall verdict. That treats every non-OAuth2 check as core, so + * it reports `unhealthy` when Opey alone is down -- exactly the judgement /health was changed to + * stop making. It stays right for the human view on /status, which shows the detail alongside it. + * + * The rule here: every required dependency healthy, and at least one OAuth2 provider healthy. One + * dead provider beside a working one is degraded, not unready -- refusing traffic then turns a + * degraded login into no login at all. A check that has not reported yet counts as not ready, since + * claiming readiness on no evidence is how a starting instance takes traffic it cannot serve. + */ +export const GET: RequestHandler = async () => { + const summary = summarizeHealth(healthCheckRegistry.getSnapshots()); + const entries = Object.entries(summary.services); + + const required = entries.filter( + ([name]) => !name.startsWith(OAUTH2_PREFIX) && !NOT_REQUIRED_TO_SERVE.includes(name) + ); + const oauth2 = entries.filter(([name]) => name.startsWith(OAUTH2_PREFIX)); + + const blocking = required.filter(([, s]) => s.status !== 'healthy').map(([name]) => name); + const noProvider = oauth2.length > 0 && !oauth2.some(([, s]) => s.status === 'healthy'); + if (noProvider) blocking.push('no OAuth2 provider is available'); + + // No checks at all is not evidence of readiness. + const ready = entries.length > 0 && blocking.length === 0; + + // Service names only. This endpoint is unauthenticated -- a probe has to be reachable before the + // app can prove anything about itself -- so it says whether to route traffic here and which + // dependency is holding that up, and nothing else. summarizeHealth's snapshots carry each + // service's configured URL and the text of its last error; returning those published the + // deployment's internal hostnames, ports and failure modes to anyone who asked. /status shows + // that detail to a signed-in human, which is where it belongs. + return new Response(JSON.stringify({ ready, blocking }), { + status: ready ? 200 : 503, + headers: { 'Content-Type': 'application/json' } + }); +}; diff --git a/apps/portal/src/hooks.server.ts b/apps/portal/src/hooks.server.ts index 6b848e5e..46a6a88b 100644 --- a/apps/portal/src/hooks.server.ts +++ b/apps/portal/src/hooks.server.ts @@ -363,5 +363,25 @@ declare module 'svelte-kit-sessions' { consentRequestId: string; bankId: string; }; + /** + * The Berlin Group consent authorisation this PSU is currently answering. + * + * Held across renders because starting one mints a new challenge and delivers a new OTP, so + * re-deriving it per render would invalidate the code the PSU is looking at. + */ + bgConsentAuthorisation?: { + consentId: string; + authorisationId: string; + }; + /** + * The accounts the PSU ticked on the UK consent screen, bound to the challenge minted for + * that selection. Held here rather than round-tripped through the URL so the consent cannot + * be authorised for accounts that never appeared on the screen they consented from. + */ + ukConsentFlow?: { + consentId: string; + challengeId: string; + accountIds: string[]; + }; } } diff --git a/apps/portal/src/lib/obp/requests.ts b/apps/portal/src/lib/obp/requests.ts index ccea06f8..c116e151 100644 --- a/apps/portal/src/lib/obp/requests.ts +++ b/apps/portal/src/lib/obp/requests.ts @@ -19,6 +19,26 @@ class OBPRequests { logger.info('Initialized.'); } + /** + * The error the API actually reported, or null when the body carries none. + * + * Two shapes reach here. OBP's own endpoints answer `{code, message}`; Berlin Group answers + * `{tppMessages: [{code, text}]}`, which is what NextGenPSD2 specifies. Only the first was + * understood, so every Berlin Group failure fell through to a generic "Error posting OBP data" + * and the reason never reached the PSU -- an account-ownership refusal reads exactly like a + * transport fault. + */ + private static apiError(data: any): { code: string | number; message: string } | null { + if (data?.code && data?.message) { + return { code: data.code, message: data.message }; + } + const tppMessage = data?.tppMessages?.[0]; + if (tppMessage?.text) { + return { code: tppMessage.code ?? 'ERROR', message: tppMessage.text }; + } + return null; + } + private logRateLimitInfo(response: Response, url: string): void { const remaining = response.headers.get('X-Rate-Limit-Remaining'); if (remaining !== null && parseInt(remaining) < 0) { @@ -70,8 +90,9 @@ class OBPRequests { ); } - if (data && data.code && data.message) { - throw new OBPRequestError(data.code, data.message); + const apiError = OBPRequests.apiError(data); + if (apiError) { + throw new OBPRequestError(apiError.code as any, apiError.message); } else { throw new OBPErrorBase(`Error fetching OBP data from ${url}: ${response.statusText}`); } @@ -109,8 +130,9 @@ class OBPRequests { if (!response.ok) { logger.error('Failed to post OBP data:', { statusText: response.statusText, data }); - if (data && data.code && data.message) { - throw new OBPRequestError(data.code, data.message); + const apiError = OBPRequests.apiError(data); + if (apiError) { + throw new OBPRequestError(apiError.code as any, apiError.message); } else { throw new OBPErrorBase(`Error posting OBP data to ${url}: ${response.statusText}`); } @@ -157,8 +179,9 @@ class OBPRequests { if (!response.ok) { logger.error('Failed to delete OBP data:', response.statusText, data); - if (data && data.code && data.message) { - throw new OBPRequestError(data.code, data.message); + const apiError = OBPRequests.apiError(data); + if (apiError) { + throw new OBPRequestError(apiError.code as any, apiError.message); } else { throw new OBPErrorBase(`Error deleting OBP data from ${url}: ${response.statusText}`); } @@ -195,8 +218,9 @@ class OBPRequests { if (!response.ok) { logger.error('Failed to put OBP data:', { statusText: response.statusText, data }); - if (data && data.code && data.message) { - throw new OBPRequestError(data.code, data.message); + const apiError = OBPRequests.apiError(data); + if (apiError) { + throw new OBPRequestError(apiError.code as any, apiError.message); } else { throw new OBPErrorBase(`Error putting OBP data to ${url}: ${response.statusText}`); } @@ -233,8 +257,9 @@ class OBPRequests { if (!response.ok) { logger.error('Failed to patch OBP data:', { statusText: response.statusText, data }); - if (data && data.code && data.message) { - throw new OBPRequestError(data.code, data.message); + const apiError = OBPRequests.apiError(data); + if (apiError) { + throw new OBPRequestError(apiError.code as any, apiError.message); } else { throw new OBPErrorBase(`Error patching OBP data to ${url}: ${response.statusText}`); } diff --git a/apps/portal/src/lib/obp/types.ts b/apps/portal/src/lib/obp/types.ts index 62a9e5eb..641c4bba 100644 --- a/apps/portal/src/lib/obp/types.ts +++ b/apps/portal/src/lib/obp/types.ts @@ -312,6 +312,18 @@ export interface OBPBGPaymentAuthorisation { sca_status: string; } +// Berlin Group Consent Authorisation (SCA) types +export interface OBPBGStartConsentAuthorisation { + scaStatus: string; + authorisationId: string; + pushMessage: string; + _links: { scaStatus: string }; +} +export interface OBPBGConsentAuthorisationResult { + scaStatus: string; + _links?: { scaStatus?: { href?: string } }; +} + // Personal Data Field (User Attribute) export interface OBPPersonalDataField { user_attribute_id: string; diff --git a/apps/portal/src/routes/(protected)/confirm-bg-consent-request-redirect-uri/+page.server.ts b/apps/portal/src/routes/(protected)/confirm-bg-consent-request-redirect-uri/+page.server.ts index 58d939d1..eeea0292 100644 --- a/apps/portal/src/routes/(protected)/confirm-bg-consent-request-redirect-uri/+page.server.ts +++ b/apps/portal/src/routes/(protected)/confirm-bg-consent-request-redirect-uri/+page.server.ts @@ -51,16 +51,17 @@ export async function load(event: RequestEvent) { const jwtPayload = decodeJwtPayload(consent.jwt); const requestHeaders = jwtPayload.request_headers || []; - let tppRedirectUri = ''; - let tppNokRedirectUri = ''; - for (const header of requestHeaders) { - if (header['TPP-Redirect-URI']) { - tppRedirectUri = header['TPP-Redirect-URI']; - } - if (header['TPP-Nok-Redirect-URI']) { - tppNokRedirectUri = header['TPP-Nok-Redirect-URI']; - } - } + // The consent JWT stores these as [{name, values}], the shape OBP's HTTPParam serialises to + // -- not as {"TPP-Redirect-URI": "..."}. Reading them the second way found nothing, ever, so + // the PSU was left on this page after a successful Berlin Group authorisation instead of + // being returned to the TPP that sent them. Under the Redirect approach that return is the + // last step of the ceremony, not a nicety. + const headerValue = (wanted: string): string => { + const match = requestHeaders.find((h: any) => h?.name === wanted); + return match?.values?.[0] ?? ''; + }; + const tppRedirectUri = headerValue('TPP-Redirect-URI'); + const tppNokRedirectUri = headerValue('TPP-Nok-Redirect-URI'); return { consentId, diff --git a/apps/portal/src/routes/(protected)/confirm-bg-consent-request-sca/+page.server.ts b/apps/portal/src/routes/(protected)/confirm-bg-consent-request-sca/+page.server.ts index 5e74f7b2..492acaf3 100644 --- a/apps/portal/src/routes/(protected)/confirm-bg-consent-request-sca/+page.server.ts +++ b/apps/portal/src/routes/(protected)/confirm-bg-consent-request-sca/+page.server.ts @@ -4,56 +4,153 @@ import type { RequestEvent, Actions } from '@sveltejs/kit'; import { redirect, isRedirect } from '@sveltejs/kit'; import { obp_requests } from '$lib/obp/requests'; import { OBPRequestError } from '@obp/shared/obp'; -import { env } from '$env/dynamic/private'; +import type { + OBPBGStartConsentAuthorisation, + OBPBGConsentAuthorisationResult +} from '$lib/obp/types'; + +/** + * Start a Berlin Group consent authorisation and remember which one this PSU is answering. + * + * OBP mints a fresh challenge per POST — a new authorisation id and a newly delivered OTP — so this + * must not run on every render. It is called from load(), and SvelteKit re-runs load() after any + * action that does not redirect: a mistyped OTP would therefore replace the challenge the PSU is + * holding, their retry would answer a challenge whose code they have never seen, and that failure + * would mint another. The PSU can never catch up, and each attempt sends another OTP. + * + * So the id is kept in the session and reused for the whole ceremony. `resend` is the one deliberate + * way to get a new one, for when the code really did expire or never arrived. + */ +async function startAuthorisation(event: RequestEvent, consentId: string, token: string) { + const startResponse: OBPBGStartConsentAuthorisation = await obp_requests.post( + `/berlin-group/v1.3/consents/${consentId}/authorisations`, + { scaAuthenticationData: '' }, + token + ); + await event.locals.session.setData({ + ...event.locals.session.data, + bgConsentAuthorisation: { consentId, authorisationId: startResponse.authorisationId } + }); + // setData alone only mutates the in-memory session: it writes to the store only when + // saveUninitialized is set, which this app does not set. Without save() the id would be + // forgotten the moment this request ended, and the next render would start another + // authorisation -- the very thing this exists to prevent. + await event.locals.session.save(); + return startResponse.authorisationId; +} + +/** Forget the remembered authorisation, so the next consent starts its own. */ +async function forgetAuthorisation(event: { locals: App.Locals; cookies: unknown }) { + const { bgConsentAuthorisation, ...rest } = event.locals.session.data; + await event.locals.session.setData(rest); + await event.locals.session.save(); +} export async function load(event: RequestEvent) { const consentId = event.url.searchParams.get('CONSENT_ID'); - if (!consentId) { return { loadError: 'Missing required parameter: CONSENT_ID.', consentId: '', - }; + authorisationId: '' + }; } - return { consentId }; + const token = event.locals.session.data.oauth?.access_token; + if (!token) { + return { + loadError: 'No access token found in session.', + consentId, + authorisationId: '' + }; + } + + // Already answering one for this consent: reuse it rather than mint a second and invalidate the + // code the PSU is looking at. See startAuthorisation. + const started = event.locals.session.data.bgConsentAuthorisation; + if (started?.consentId === consentId && started.authorisationId) { + return { consentId, authorisationId: started.authorisationId }; + } + + try { + return { consentId, authorisationId: await startAuthorisation(event, consentId, token) }; + } catch (e) { + logger.error('Error starting BG consent authorisation:', e); + let errorMessage = 'Failed to start consent authorisation.'; + if (e instanceof OBPRequestError) { + errorMessage = e.message; + } + return { loadError: errorMessage, consentId, authorisationId: '' }; + } } export const actions = { - default: async ({ request, locals }) => { + /** + * Deliberately mint a new challenge, for a code that expired or never arrived. + * + * This is the only path that is supposed to invalidate the previous one, which is why it is a + * button the PSU presses rather than something that happens to them on every re-render. + */ + resend: async (event) => { + const consentId = (await event.request.formData()).get('consentId') as string; + const token = event.locals.session.data.oauth?.access_token; + if (!consentId || !token) { + return { message: 'Cannot request a new code: the session or the consent id is missing.' }; + } + try { + await startAuthorisation(event as RequestEvent, consentId, token); + return { message: 'A new code has been sent. Use the most recent one.' }; + } catch (e) { + event.locals && logger.error('Error restarting BG consent authorisation:', e); + return { + message: e instanceof OBPRequestError ? e.message : 'Failed to request a new code.' + }; + } + }, + + // Named rather than `default` because `resend` exists: SvelteKit refuses a default action + // alongside named ones. + confirm: async (event) => { + const { request, locals } = event; const formData = await request.formData(); const otp = formData.get('otp') as string; const consentId = formData.get('consentId') as string; + const authorisationId = formData.get('authorisationId') as string; if (!otp) { return { message: 'Please enter the OTP code.' }; } + if (!authorisationId) { + // There is no authorisation to answer because starting one failed, and the reason is + // already on the page above. Telling the PSU to reload sends them round the same loop. + return { + message: + 'This consent has no authorisation to confirm — starting one did not succeed. See the reason above.' + }; + } + const token = locals.session.data.oauth?.access_token; if (!token) { return { message: 'No access token found in session.' }; } - const defaultBankId = env.DEFAULT_BANK_ID; - if (!defaultBankId) { - logger.error('DEFAULT_BANK_ID environment variable is not set'); - return { message: 'Server configuration error: DEFAULT_BANK_ID is not set.' }; - } - try { - const response = await obp_requests.post( - `/obp/v3.1.0/banks/${defaultBankId}/consents/${consentId}/challenge`, - { answer: otp }, + const response: OBPBGConsentAuthorisationResult = await obp_requests.put( + `/berlin-group/v1.3/consents/${consentId}/authorisations/${authorisationId}`, + { scaAuthenticationData: otp }, token ); - if (response.status === 'ACCEPTED' || response.status === 'VALID') { + if (response.scaStatus === 'valid') { + // Done with this one; a later consent must not reuse a spent authorisation id. + await forgetAuthorisation(event); redirect(303, `/confirm-bg-consent-request-redirect-uri?CONSENT_ID=${consentId}`); } return { - message: `Challenge was not accepted. Status: ${response.status}` + message: `Challenge was not accepted. Status: ${response.scaStatus}` }; } catch (e) { if (isRedirect(e)) throw e; diff --git a/apps/portal/src/routes/(protected)/confirm-bg-consent-request-sca/+page.svelte b/apps/portal/src/routes/(protected)/confirm-bg-consent-request-sca/+page.svelte index 41d65eb9..548d4c0b 100644 --- a/apps/portal/src/routes/(protected)/confirm-bg-consent-request-sca/+page.svelte +++ b/apps/portal/src/routes/(protected)/confirm-bg-consent-request-sca/+page.svelte @@ -1,5 +1,12 @@
@@ -23,8 +30,9 @@
{/if} -
+ +
diff --git a/apps/portal/src/routes/(protected)/obp-consent-request/+page.svelte b/apps/portal/src/routes/(protected)/obp-consent-request/+page.svelte index 4c71cda0..18b4d9af 100644 --- a/apps/portal/src/routes/(protected)/obp-consent-request/+page.svelte +++ b/apps/portal/src/routes/(protected)/obp-consent-request/+page.svelte @@ -68,6 +68,86 @@ {/if} + + {#if data.payload.to_account} +
+

+ Payments you are authorising +

+
+ {#if data.payload.from_account?.account_routing?.address} +
+ Paid from: + {data.payload.from_account.account_routing.address} + ({data.payload.from_account.account_routing.scheme}) +
+ {/if} + + {#if data.payload.to_account.counterparty_name} +
+ Paid to: + {data.payload.to_account.counterparty_name} +
+ {/if} + {#if data.payload.to_account.account_routing?.address} +
+ + {data.payload.to_account.counterparty_name + ? 'Their account:' + : 'Paid to:'} + + {data.payload.to_account.account_routing.address} + ({data.payload.to_account.account_routing.scheme}) +
+ {/if} +
+ + {#if data.payload.to_account.limit} + {@const limit = data.payload.to_account.limit} + {@const currency = limit.currency ?? ''} +

+ Limits on those payments +

+ + {/if} +
+ {/if} + {#if !data.payload.everything && data.payload.account_access?.length}

diff --git a/apps/portal/src/routes/(protected)/otp/+page.server.ts b/apps/portal/src/routes/(protected)/otp/+page.server.ts index 4b303bac..80df6529 100644 --- a/apps/portal/src/routes/(protected)/otp/+page.server.ts +++ b/apps/portal/src/routes/(protected)/otp/+page.server.ts @@ -81,7 +81,7 @@ export const actions = { // Step 1: Create authorisation const authResponse = await obp_requests.post( - `/obp/v1.3/berlin-group/${paymentService}/${paymentProduct}/${paymentId}/authorisations`, + `/berlin-group/v1.3/${paymentService}/${paymentProduct}/${paymentId}/authorisations`, {}, token ); @@ -90,7 +90,7 @@ export const actions = { // Step 2: Submit OTP to authorisation await obp_requests.put( - `/obp/v1.3/berlin-group/${paymentService}/${paymentProduct}/${paymentId}/authorisations/${authorisationId}`, + `/berlin-group/v1.3/${paymentService}/${paymentProduct}/${paymentId}/authorisations/${authorisationId}`, { scaAuthenticationData: otp }, token ); diff --git a/apps/portal/src/routes/(protected)/uk-consent-request-sca/+page.server.ts b/apps/portal/src/routes/(protected)/uk-consent-request-sca/+page.server.ts index f1b9b06d..53881415 100644 --- a/apps/portal/src/routes/(protected)/uk-consent-request-sca/+page.server.ts +++ b/apps/portal/src/routes/(protected)/uk-consent-request-sca/+page.server.ts @@ -31,7 +31,13 @@ export async function load(event: RequestEvent) { const token = event.locals.session.data.oauth?.access_token; if (!token) { - return { loadError: 'Unauthorized: No access token found in session.', consentId, bankId, challengeId, oidcReturnUrl: '' }; + return { + loadError: 'Unauthorized: No access token found in session.', + consentId, + bankId, + challengeId, + oidcReturnUrl: '' + }; } const oidcReturnUrl = @@ -58,6 +64,19 @@ export const actions = { return { message: 'Please enter the OTP code.' }; } + // The selection the PSU made on the previous screen, held server-side against the challenge + // that was minted for it. Read from the session rather than the request so what gets + // authorised is what was on the screen they consented from -- see the note where it is + // stored in uk-consent-request/+page.server.ts. + const ukConsentFlow = locals.session.data.ukConsentFlow; + const accountIds = + ukConsentFlow?.consentId === consentId && ukConsentFlow?.challengeId === challengeId + ? ukConsentFlow.accountIds + : []; + if (accountIds.length === 0) { + return { message: 'No accounts were selected for this consent. Please start over.' }; + } + const token = locals.session.data.oauth?.access_token; if (!token) { return { message: 'No access token found in session.' }; @@ -76,7 +95,7 @@ export const actions = { // the PSU only if the challenge answer is correct; a wrong answer leaves it unauthorised. await obp_requests.post( `/obp/v5.1.0/banks/${bankId}/consents/${consentId}/authorise`, - { challenge_id: challengeId, answer: otp }, + { challenge_id: challengeId, answer: otp, account_ids: accountIds }, token ); } catch (e) { @@ -94,7 +113,10 @@ export const actions = { returnUrl.searchParams.set('consent_id', consentId); returnUrl.searchParams.set('consent_status', 'ACCEPTED'); const username = locals.session.data.user?.username; - const provider = locals.session.data.user?.provider; + // oauth, not user: the session records the IdP that authenticated this PSU on the oauth + // entry, and `user` has no `provider` at all. This read was always undefined, so the param + // the comment above says OBP-OIDC needs to resolve the PSU was never actually sent. + const provider = locals.session.data.oauth?.provider; if (username) returnUrl.searchParams.set('username', username); if (provider) returnUrl.searchParams.set('provider', provider); redirect(303, returnUrl.toString()); diff --git a/apps/portal/src/routes/(protected)/uk-consent-request/+page.server.ts b/apps/portal/src/routes/(protected)/uk-consent-request/+page.server.ts index 4038acbe..6ce3388d 100644 --- a/apps/portal/src/routes/(protected)/uk-consent-request/+page.server.ts +++ b/apps/portal/src/routes/(protected)/uk-consent-request/+page.server.ts @@ -31,15 +31,33 @@ export async function load(event: RequestEvent) { const requestedOidcReturnUrl = event.url.searchParams.get('oidc_return_url'); if (!consentId) { - return { loadError: 'Missing required parameter: CONSENT_ID.', consentId: '', bankId: bankId || '', apiStandard, oidcReturnUrl: '' }; + return { + loadError: 'Missing required parameter: CONSENT_ID.', + consentId: '', + bankId: bankId || '', + apiStandard, + oidcReturnUrl: '' + }; } if (!bankId) { - return { loadError: 'Missing required parameter: bank_id.', consentId, bankId: '', apiStandard, oidcReturnUrl: '' }; + return { + loadError: 'Missing required parameter: bank_id.', + consentId, + bankId: '', + apiStandard, + oidcReturnUrl: '' + }; } const token = event.locals.session.data.oauth?.access_token; if (!token) { - return { loadError: 'Unauthorized: No access token found in session.', consentId, bankId, apiStandard, oidcReturnUrl: '' }; + return { + loadError: 'Unauthorized: No access token found in session.', + consentId, + bankId, + apiStandard, + oidcReturnUrl: '' + }; } // oidc_return_url must point back to a configured OIDC provider host — otherwise it becomes @@ -69,7 +87,41 @@ export async function load(event: RequestEvent) { logger.warn('Could not fetch UK consent details:', e); } - return { consentId, bankId, apiStandard, oidcReturnUrl, status, permissions, expirationDateTime }; + // The PSU must pick which of their own accounts this consent's permissions apply to -- + // OBP-API only binds/grants access for accounts the authorising user actually holds + // (POST .../authorise requires account_ids, and rejects any account the PSU doesn't hold). + let userAccounts: { accountId: string; label: string }[] = []; + // "The list could not be loaded" and "there are no accounts" are different facts, and the second + // is a statement about the PSU that the page should not make on the strength of a failed call. + // An empty list with no error reads as "you hold nothing here"; this keeps the reason instead. + let accountsError = ''; + try { + const accountsResponse = await obp_requests.get('/obp/v6.0.0/my/accounts', token); + userAccounts = (accountsResponse.accounts || []) + .filter((account: any) => account.bank_id === bankId) + .map((account: any) => ({ + accountId: account.id, + label: account.label || account.id + })); + } catch (e) { + logger.warn('Could not fetch user accounts:', e); + accountsError = + e instanceof OBPRequestError + ? `Your accounts could not be loaded: ${e.message}` + : 'Your accounts could not be loaded. Please try again.'; + } + + return { + consentId, + bankId, + apiStandard, + oidcReturnUrl, + status, + permissions, + expirationDateTime, + userAccounts, + accountsError + }; } export const actions = { @@ -78,6 +130,7 @@ export const actions = { const consentId = formData.get('consentId') as string; const bankId = formData.get('bankId') as string; const oidcReturnUrlRaw = formData.get('oidcReturnUrl') as string; + const selectedAccountIds = formData.getAll('selectedAccountIds') as string[]; const token = locals.session.data.oauth?.access_token; if (!token) { @@ -94,6 +147,10 @@ export const actions = { return { message: 'No valid return URL was provided to complete the flow.' }; } + if (selectedAccountIds.length === 0) { + return { message: 'Please select at least one account to grant access to.' }; + } + let challengeId = ''; try { // Start SCA: OBP-API issues a one-time challenge (OTP) to the PSU. The consent is only @@ -113,6 +170,22 @@ export const actions = { return { message: errorMessage }; } + // The accounts the PSU ticked are held server-side against the challenge that was just + // minted, not passed through the URL. Carried in the query string they were editable + // between this screen and the answer, so the consent could end up naming accounts that + // never appeared on the screen the PSU consented from -- and the consent record is the + // artefact an audit reads. OBP-API still refuses accounts the PSU does not hold, so the + // exposure was bounded, but "bounded" is not the same as "what they agreed to". + await locals.session.setData({ + ...locals.session.data, + ukConsentFlow: { consentId, challengeId, accountIds: selectedAccountIds } + }); + // setData alone only mutates the in-memory session -- it writes to the store only when + // saveUninitialized is set, which this app does not set. Without save() the selection would + // not survive the redirect below, and the SCA step would refuse a consent the PSU had just + // filled in correctly. + await locals.session.save(); + const params = new URLSearchParams({ CONSENT_ID: consentId, bank_id: bankId, diff --git a/apps/portal/src/routes/(protected)/uk-consent-request/+page.svelte b/apps/portal/src/routes/(protected)/uk-consent-request/+page.svelte index 2f83c74b..542bd117 100644 --- a/apps/portal/src/routes/(protected)/uk-consent-request/+page.svelte +++ b/apps/portal/src/routes/(protected)/uk-consent-request/+page.svelte @@ -1,5 +1,20 @@
@@ -74,17 +89,68 @@ {/if}
+ +
+

+ Select Accounts +

+ {#if data.userAccounts?.length} +

+ Choose which of your accounts the requested permissions apply to: +

+
+ {#each data.userAccounts as account} + + {/each} +
+ {:else if data.accountsError} + +

+ {data.accountsError} +

+ {:else} +

+ You have no accounts at this bank, so this consent cannot be authorised. +

+ {/if} +
+
-
+ -
-
+