UK Open Banking consent authorisation with SCA, plus OAuth/session security hardening#4
Open
hongwei1 wants to merge 16 commits into
Open
UK Open Banking consent authorisation with SCA, plus OAuth/session security hardening#4hongwei1 wants to merge 16 commits into
hongwei1 wants to merge 16 commits into
Conversation
# Conflicts: # package-lock.json
…i-manager - add path-traversal, absolute-URL, and timeout guards to the api-manager generic proxy route, matching the portal proxy's existing protections - validate oidc_return_url against configured OIDC provider origins before storing it, closing an open-redirect / consent-data leak path - reject protocol-relative redirect_to values in api-manager login/callback routes instead of only checking for a leading slash - escape markdown link hrefs and restrict them to http(s)/relative targets on the featured page to prevent attribute-breakout and javascript: XSS - stop treating a missing id_token on refresh as a hard failure; OIDC refresh responses are not required to reissue one - move email-validation off the page load into an explicit form action so a GET (link scanner, prefetch) can no longer consume the single-use token - set Referrer-Policy and skip analytics injection on token-in-URL routes to stop the reset token leaking via Referer/analytics - redact refresh_token/code/client_secret/password fields before logging OAuth request bodies, and drop the access-token preview log line - treat an undecodable or missing-exp access token as expired instead of throwing, so a bad token triggers a refresh instead of a 500
…relative markdown links Lock in the behavior of the OAuth/redirect/redaction security fixes with unit tests, and close a gap the tests exposed on the featured page. - reject protocol-relative (//host) and backslash (/\host) hrefs in the featured-page markdown sanitizer, mirroring the shared isSafeHref guard - extract the featured-page markdown helpers into apps/portal/src/lib/markdown/featuredMarkdown.ts so the sanitizer and renderer are importable and testable; the page imports them back - add tests for isSafeRelativeRedirect and redactUrlEncodedBody - add XSS-payload tests for markdownToHtml (javascript:/data:/protocol-relative hrefs collapse to '#', attribute breakout is neutralized, text is escaped) - add OAuth2ProviderFactory trusted-origin tests via an injected fake strategy - update client.test.ts to the fail-closed checkAccessTokenExpiration behavior (bad/missing-exp/undecodable token -> expired, never throws) and to the current createLogger two-arg console format
Rewrite the test files that were left red against a removed architecture (the obp_oauth singleton and the refreshAccessTokenInSession free function were replaced by the OAuth2ProviderFactory/Manager and SessionOAuthHelper). - session.test.ts: test SessionOAuthHelper (getSessionOAuth / updateTokensInSession / refreshAccessToken) via a fake provider injected into the factory singleton, with the current messages and fail-closed paths - oauth-flow.test.ts: integration across the provider factory + session helper - login.server.test.ts: cover the current /login/obp GET (redirect to the first available provider, or 500 when none is configured) - login.svelte.test.ts: cover the current multi-provider login page (provider buttons, empty state, error and unavailable-provider blocks) without @testing-library/user-event - register/page.server.test.ts: align to the current action (v6.0.0 endpoint, two-arg post, username length guard, OBP-error and redirect handling) - replace the brittle shared demo-route render spec (which fought SvelteKit's $env/dynamic/public virtual module in vitest) with a real unit test of the avatar identicon generator - test/setup.ts: import afterEach/vi so the client vitest project (no globals) stops crashing at load - vite.config.ts: allow serving the hoisted node_modules so the svelte client test project runs from a git worktree - api-manager: passWithNoTests so its empty suite does not fail CI
Add a GitHub Actions workflow that installs dependencies, syncs SvelteKit, and runs the vitest unit suites across all workspaces on every pull request, so the portal and shared test suites act as a merge gate.
api-manager received the same security hardening as portal but had no test infrastructure. Add a server-side vitest setup and cover the security-relevant behavior: - client.ts checkAccessTokenExpiration fail-closed (bad/missing-exp/undecodable token treated as expired, never throws) - the generic proxy fallback handler: 401 without a user or access token, 400 for empty / traversal / absolute-path / absolute-URL / null-byte paths (and no upstream fetch on rejection), and a valid path forwarded to OBP with the bearer token and query string Drop the passWithNoTests stopgap now that the suite has real tests.
OBP-API returns payload as a JValue that is parsed server-side, so it arrives as a plain object, not a JSON string. Calling JSON.parse() on it threw and was silently swallowed, leaving the Consent Details box and Access Level/Valid For/Valid From fields blank on the consent approval page.
fix: stop double-parsing already-parsed consent request payload
Handle the UK Open Banking consent flow that OBP-OIDC now redirects into. The login entry captures consent_id + api_standard into the consent-flow cookie; after authentication the callback routes UK consents to a new uk-consent-request approval page. On approval it calls OBP-API to authorise the consent as the current PSU, then redirects back to OBP-OIDC's consent-callback with username + provider so a consent-bound authorization code is minted. Deny returns consent_status=REJECTED. The existing OBP consent-request flow is unchanged.
On approval, /uk-consent-request now starts an SCA challenge via OBP-API and routes to a new /uk-consent-request-sca page to collect the OTP. The consent is only authorised — and control returned to OBP-OIDC — once the correct OTP is submitted; a wrong OTP is rejected and the consent stays unauthorised.
…kage refactor Upstream moved the OAuth client and provider factory into @obp/shared/server/oauth and reduced the app-level files to thin re-exports. Our branch had hardened the old inline versions. Reconciliation: - apps/*/lib/oauth/client.ts: adopt upstream's re-export; the shared client already carries the redactUrlEncodedBody token-log redaction, so no security regression. - apps/portal/lib/oauth/providerFactory.ts: adopt upstream's instantiate-from-shared wiring. - packages/shared/.../providerFactory.ts: port getTrustedOidcOrigins() and isTrustedOidcReturnUrl() onto the shared OAuth2ProviderFactory, so the open-redirect guard the UK/OBP consent pages and login route depend on remains available on the exported factory instance. Verified: no residual conflict markers; svelte-check shows no new type errors in the resolved files and the guard resolves for all callers.
test: cover the security-fix batch with unit tests, repair broken suites, add CI
…ent details
The OIDC callback only matched api_standard === 'UKOpenBanking' (v3.1),
so a v4.0.1 consent (api_standard === 'UKOpenBankingV401') silently fell
through past the UK branch instead of reaching /uk-consent-request. Match
both standards and thread api_standard through as a query param so the
approval page knows which version's GET-consent endpoint to call.
Both UK consent pages previously showed a generic approval with no
permission detail, or (on the OBP consent page) dumped the raw internal
JSON payload with empty account_access/entitlements arrays. Render the
actual requested permissions, status and expiry instead:
- uk-consent-request: fetch GET /open-banking/{v3.1|v4.0.1/aisp}/account-access-consents/{id}
(no ownership check pre-authorisation, matching the UK spec's PSU-facing
consent-status lookup) and show Status/Expires/Permissions.
- obp-consent-request: render account_access and entitlements as readable
lists instead of a raw JSON dump.
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.
Summary
Adds a complete UK Open Banking (AISP) account-access consent authorisation flow with real SCA to the Portal + shared OAuth stack, plus a batch of OAuth/session/proxy security hardening and consent-page fixes. Reconciled with the recent move of the OAuth client/factory into
@obp/shared/server/oauth.What's included
UK Open Banking consent authorisation + SCA
(protected)/uk-consent-request(approve/deny) and(protected)/uk-consent-request-sca(OTP entry).AWAITINGAUTHORISATION -> AUTHORISED. This fills the previously-missing "authorisation binding" step for UK consents lodged byclient_credentials.AWAITINGAUTHORISATIONconsents.Security hardening
redactUrlEncodedBody).oidc_return_url: an OIDC return URL is only honoured when its origin matches a configured provider's authorization endpoint (OAuth2ProviderFactory.isTrustedOidcReturnUrl). Applied on the login route and every consent page.Fixes
Merge reconciliation
Upstream moved the OAuth client and provider factory into
@obp/shared/server/oauthand reduced the app-level files to thin re-exports. This branch had hardened the old inline versions, so:oauth/client.tsfiles adopt the upstream re-export (the shared client already carries the log redaction).apps/portal/lib/oauth/providerFactory.tsadopts the upstream instantiate-from-shared wiring.isTrustedOidcReturnUrl/getTrustedOidcOriginsguard is ported onto the sharedOAuth2ProviderFactory, so it stays available on the exported factory instance for all callers.Verification
svelte-checkshows no new type errors in the reconciled OAuth files; the redirect guard resolves for all callers.