Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
88d1a73
fix: correct Berlin Group SCA authorisation URL segment order
hongwei1 Jul 18, 2026
a80a60a
fix(portal): call the Berlin Group native SCA endpoints for BG consen…
hongwei1 Jul 18, 2026
053ff86
feat(portal): let the PSU pick accounts before authorising a UK consent
hongwei1 Jul 22, 2026
9f989a5
fix(portal): preserve query params in the UK consent form action URLs
hongwei1 Jul 22, 2026
b23d022
fix: stop the login page's auto-refresh from racing the OAuth state c…
hongwei1 Jul 22, 2026
6d311a2
fix: make /health a liveness probe again
hongwei1 Jul 31, 2026
be74519
fix: show the PSU who may be paid and how much before they grant a VR…
hongwei1 Aug 7, 2026
61a8156
fix: show the reason a Berlin Group call failed instead of a generic …
hongwei1 Aug 7, 2026
9b16749
fix: return the PSU to the TPP after a Berlin Group authorisation
hongwei1 Aug 7, 2026
621dd2a
fix: stop re-sending the SCA code, and stop guessing why accounts are…
hongwei1 Aug 8, 2026
6509c89
fix: add a readiness probe, and bind the consented accounts server-side
hongwei1 Aug 8, 2026
b058bee
fix: readiness must not hinge on a dependency the app serves fine wit…
hongwei1 Aug 8, 2026
1139309
fix: persist the session changes instead of only mutating them in memory
hongwei1 Aug 8, 2026
d0b142c
fix: keep CONSENT_ID in the query string when the SCA form posts
hongwei1 Aug 8, 2026
f0d0c32
Merge pull request #3 from hongwei1/fix/consent-page-disclosure
hongwei1 Aug 8, 2026
475c6a5
fix: stop the readiness probe publishing the deployment's internals
hongwei1 Aug 8, 2026
a9dd4ea
Merge pull request #4 from hongwei1/fix/ready-probe-minimal
hongwei1 Aug 9, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions apps/api-manager/src/routes/health/+server.ts
Original file line number Diff line number Diff line change
@@ -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' }
});
};
62 changes: 62 additions & 0 deletions apps/api-manager/src/routes/ready/+server.ts
Original file line number Diff line number Diff line change
@@ -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' }
});
};
20 changes: 20 additions & 0 deletions apps/portal/src/hooks.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
};
}
}
45 changes: 35 additions & 10 deletions apps/portal/src/lib/obp/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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}`);
}
Expand Down Expand Up @@ -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}`);
}
Expand Down Expand Up @@ -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}`);
}
Expand Down Expand Up @@ -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}`);
}
Expand Down Expand Up @@ -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}`);
}
Expand Down
12 changes: 12 additions & 0 deletions apps/portal/src/lib/obp/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading
Loading