Skip to content

IS-11327 Unify EBF and BankID error handling #195

Merged
aleixsuau merged 155 commits into
feature/IS-11327/webauthn-error-handlingfrom
feature/IS-11327/ebf-bankid-error-handling
May 29, 2026
Merged

IS-11327 Unify EBF and BankID error handling #195
aleixsuau merged 155 commits into
feature/IS-11327/webauthn-error-handlingfrom
feature/IS-11327/ebf-bankid-error-handling

Conversation

@aleixsuau
Copy link
Copy Markdown
Contributor

@aleixsuau aleixsuau commented May 21, 2026

Jira: https://curity.atlassian.net/browse/IS-11327

Follow-up to the WebAuthn portion of IS-11327 (PR #194). Brings runExternalBrowserFlow and runBankIdAuthentication onto the same ClientOperationResult discriminated-union contract so client-operation failures flow through useHaapiStepper().error.app like WebAuthn's.

Test plan

  1. Apply
    ebf-verification.patch

  2. There is a harness inside runExternalBrowserFlow that short-circuits each failure branch + supplies mock copy (BE hasn't shipped step.metadata.viewData.error.clientOperation.externalBrowserFlow.{launch,resume} yet). Change it to emulate each error.

  3. Start a fresh OAuth flow:

    https://localhost:8443/dev/oauth/authorize?client_id=client-one&response_type=code&redirect_uri=https://localhost:7777/client-one/cb1&scope=openid&state=test
    
  4. Pick openid-wallet1

  5. Click the button**"The authentication process needs to use an external browser"**.

Toggle each branch

Edit EBF_HARNESS_FAILURE_MODE at the top of src/haapi-stepper/feature/actions/client-operation/operations/external-browser-flow/external-browser-flow.ts:

Mode What's exercised
'LAUNCH' window.open returns null → launch-error branch → "External browser flow could not start…"
'RESUME_BAD_ORIGIN' opens about:blank, dispatches a message with attacker origin → resume-error branch (origin guard)
'RESUME_BAD_DATA' opens about:blank, dispatches a message with non-string data → resume-error branch (type guard)
'ABORT' opens about:blank, fires onAbort() → resume-error branch (abort handler)
null production behaviour

What to confirm

  • The step renderer stays mounted after each failure (user-driven retry).
  • The toast text matches the mode (launch vs resume copy).
  • No JS errors in the console — failures route through the resolved ClientOperationResult, never escape to the ErrorBoundary.
  • Network panel shows no popup-launch fetch under LAUNCH mode (runner short-circuits before window.open).

pmhsfelix and others added 30 commits March 2, 2026 17:39
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…and-image

IS-10358 Update README hero image to support dark mode
…vel-watch-mode-for-component-library

IS-10358 Update component-library use watch mode from root level npm start
…-css-docs-to-astro-6

IS-10358 Update CSS Docs to Astro 6
Co-authored-by: markoweb <2862929+markoweb@users.noreply.github.com>
Co-authored-by: markoweb <2862929+markoweb@users.noreply.github.com>
…er-runtime.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…s-site-to-astro6

IS 10358 Update CSS docs site to Astro 6
…S custom property and add openid-wallet color support.
…d-returnUrl-in-background-2

IS-9422: updates templates to support same-device and cross-device flows
…-openid-wallet-authenticator-button

IS-11243 Fix missing color on openid-wallet authenticator
…-returnUrl-in-background

IS-9422: adds template to evaluate browser context needed for BankID …
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aligns External Browser Flow and BankID client-operation runners with the shared ClientOperationResult shape, so EBF failures can be surfaced through the stepper’s app-error path rather than escaping as rejected promises.

Changes:

  • Converts External Browser Flow success/failure handling to resolve ClientOperationResult.
  • Adds EBF error metadata typings and tests for launch/resume failure branches.
  • Updates BankID runner return shape and related mocks/types.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/login-web-app/src/haapi-stepper/util/tests/mocks.ts Adds a typed EBF action mock with an absolute launch URL.
src/login-web-app/src/haapi-stepper/feature/stepper/haapi-stepper.types.ts Exposes a stepper-specific EBF client-operation action type.
src/login-web-app/src/haapi-stepper/feature/actions/client-operation/operations/webauthn/webauthn.spec.ts Refactors WebAuthn tests to use shared mock step helpers.
src/login-web-app/src/haapi-stepper/feature/actions/client-operation/operations/external-browser-flow/typings.ts Adds EBF error type discriminators.
src/login-web-app/src/haapi-stepper/feature/actions/client-operation/operations/external-browser-flow/index.ts Adds barrel exports for the EBF operation module.
src/login-web-app/src/haapi-stepper/feature/actions/client-operation/operations/external-browser-flow/external-browser-flow.ts Converts EBF runner to return success/error result objects and resolve metadata-based app errors.
src/login-web-app/src/haapi-stepper/feature/actions/client-operation/operations/external-browser-flow/external-browser-flow.spec.ts Adds EBF success, launch failure, resume failure, abort, and metadata fallback tests.
src/login-web-app/src/haapi-stepper/feature/actions/client-operation/operations/client-operations.ts Passes current step into EBF and consumes unified runner results directly.
src/login-web-app/src/haapi-stepper/feature/actions/client-operation/operations/bankid/bankid.ts Wraps BankID continuation data in clientOperationData.
src/login-web-app/src/haapi-stepper/data-access/types/haapi-step.types.ts Adds metadata typing for EBF client-operation error messages.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

aleixsuau and others added 8 commits May 28, 2026 12:42
Bring runExternalBrowserFlow onto the same ClientOperationResult contract that
WebAuthn established: always resolves, with { clientOperationData } on success
or { clientOperationError } on a catalogued failure. Three failure paths
(window.open returns null, unexpected origin/non-string data in the resume
message, abort signal) synthesise a HaapiStepperError via getHaapiStepperError
so the stepper surfaces them inline via useHaapiStepper().error.app, with copy
resolved from step.metadata.viewData.error.clientOperation.externalBrowserFlow.
{ launch | resume } — two keys mirroring Velocity's launch-error and
external-flow-end templates.

The EBF runner also fixes a latent listener+popup leak in the previous code:
the unexpected-origin/data branch now goes through cleanup instead of leaving
the message listener registered and the popup open.

runBankIdAuthentication adopts the contract too, but narrowed to the
success-only branch (Promise<{ clientOperationData }>) — the LWA's BankID
launcher has no client-side catchable failures today (anchor.click is fire-
and-forget; the real BankID failures land on the next polling step). When a
client-side failure mode eventually lands, widen to the full union and
introduce currentStep then.

File-layout follow-up: external-browser-flow.ts moves into its own
subdirectory with sibling typings.ts and index.ts barrel, matching the
webauthn/ and bankid/ conventions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Mirror webauthn refactor: remove viewData metadata lookup, return hardcoded copy via EXTERNAL_BROWSER_FLOW_ERROR_MESSAGES (forward-compat path preserved via void currentStep).
- Drop the now-obsolete metadata-key fallback test; remaining specs assert against the exported const.
- Guard openSpy.mock.calls[0][0] indexing with toHaveBeenCalledTimes(1) so a missed call surfaces as a clean assertion failure.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-ui-with-error-notifier

IS-5161 Wrap HaapiStepperStepUI with HaapiStepperErrorNotifier
Comment thread src/login-web-app/src/haapi-stepper/data-access/types/haapi-step.types.ts Outdated
Comment thread src/login-web-app/src/haapi-stepper/data-access/types/haapi-step.types.ts Outdated
luisgoncalves and others added 16 commits May 28, 2026 15:46
…ader-fix-config

IS-11318 LWA: fix propagation of bootstrap configuration in the dev setup
Catch every WebAuthn ceremony failure in the runners and surface it as a
synthesised HaapiUnexpectedProblemStep via the existing error.app pipeline
(rather than escalating to the React error boundary as a programming bug).
Two-bucket discriminator (cancelOrTimeout / failed) matching Velocity
parity. Copy comes from step.metadata.messages.error.clientOperation.webauthn
with empty-string fallback while the BE keys land.

* Runner-level synthesis: runWebAuthn{Registration,Authentication} catch the
  full error catalog (parse DOMException, create/get DOMException, TypeError,
  null credential, unsupported API) and throw a synthesised HaapiStepperError
  built via formatErrorStepData(HaapiUnexpectedProblemStep).
* Dispatcher wrapping: performClientOperation wraps each WebAuthn runner in a
  .then/.catch chain returning ClientOperationResult discriminated union. The
  catch discriminates HaapiStepperError vs raw rejections via a type guard;
  raw errors propagate to the React boundary as programming bugs.
* Type hierarchy: HaapiWebAuthnRegistrationClientOperationAction is now a
  proper discriminated union (passkeys | any-device) so consumers narrow via
  positive + negative type guards without casts. HaapiWebAuthnAnyDeviceArgs
  is also a discriminated union (platform-required | crossPlatform-required)
  expressing the "at least one of two" HAAPI spec invariant.
* Metadata surface: HaapiMetadata.messages.error.clientOperation.webauthn
  carries the per-ceremony copy (cancelOrTimeout shared, registration,
  authentication).
* Auto-start parity: manageWebAuthnAutoStart stays fire-and-forget via
  nextStep(); ceremony failures surface the same way as manual click
  (matching Velocity's .catch(handleError) pattern).
* Tests: 23 runner-level tests in webauthn.spec.ts cover the full catalog
  per ceremony; HaapiStepper.spec.tsx wiring tests consolidated under one
  WebAuthn suite (Auto-Start gating + Registration/Authentication
  success+error routing), dropping the duplicate auto-start error-surfacing
  cases since manual click + auto-start share the dispatcher plumbing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…apiStepperError

* Rename `HaapiMetadata.messages` → `HaapiMetadata.viewData` (and the dependent
  type aliases) so the metadata branch reads as view-customisation data emitted
  by the server rather than HAAPI step messages. Path becomes
  `step.metadata.viewData.error.clientOperation.webauthn.<key>`.
* Extract `getHaapiStepperError` from `webauthn.ts` to `client-operations.ts`
  as a shared helper that takes a resolved message string and synthesises a
  `HaapiStepperError` from a `HaapiUnexpectedProblemStep`. WebAuthn-specific
  message resolution stays in `webauthn.ts` (`getWebAuthnErrorMessage`); other
  client operations (BankID, EBF) can reuse the helper when their per-runtime
  error handling lands.
* Test fixtures + path-access assertions in `webauthn.spec.ts` updated to
  use the new `viewData` key.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ClientOperationResult is no longer WebAuthn-specific — the EBF and BankID
runners will need it next. Extract it into a new sibling typings.ts file
alongside the operation runners; keep the WebAuthn-specific enums where
they are.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Narrows the WebAuthn-runner classifier to honour the typings.ts contract:
only DOMException routes to error.app (NotAllowed/Abort -> CANCEL_OR_TIMEOUT,
any other DOMException -> FAILED). Anything else (TypeError, helper throw,
malformed-payload access errors, etc.) propagates so the React error boundary
catches it instead of being buried under 'Registration/Authentication failed'
copy. Keeps Velocity-parity for users and covers future WebAuthn-spec
DOMException names without an allowlist.

Spec updated: the registration/authentication asserts that TypeError /
'arbitrary non-DOMException' map to the FAILED copy are flipped to
.rejects.toBe(error), and the authentication side gains symmetric coverage.

Addresses PR #194 review feedback from @vahag-curity and @luisgoncalves.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switches mockRunWebAuthnRegistration / mockRunWebAuthnAuthentication from
bare vi.fn() (Mock<any, any>) to vi.fn<typeof runWebAuthnX>(). With the
runner contract Promise<ClientOperationResult> in scope, TypeScript now
rejects mock fixtures that resolve outside the shape — including
mockResolvedValue(undefined) which previously compiled silently and
TypeError'd at runtime.

The Auto-Start beforeEach baseline is updated to resolve to a sentinel
clientOperationError: failedWebAuthnCeremonyError() so an unintended
runner invocation surfaces loudly via error.app instead of progressing
the stepper silently.

Addresses PR #194 review feedback from @vahag-curity: the dispatcher's
destructure has no production hazard — strict mode already prevents
runners from falling off the end of Promise<ClientOperationResult>.
The hazard was test-fixture authoring; this commit closes it at that
layer rather than adding a runtime guard.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…:curityio/ui-kit into feature/IS-11327/webauthn-error-handling
Decouples the WebAuthn runner from the client-operation dispatcher.
Previously the runner imported `getHaapiStepperError` from `client-operations.ts`,
which itself imports the runners — an awkward upward dependency. Co-locating the
helper with `ClientOperationResult` (typings.ts) by way of a sibling `helpers.ts`
follows the same shape used for the shared result type.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bring runExternalBrowserFlow onto the same ClientOperationResult contract that
WebAuthn established: always resolves, with { clientOperationData } on success
or { clientOperationError } on a catalogued failure. Three failure paths
(window.open returns null, unexpected origin/non-string data in the resume
message, abort signal) synthesise a HaapiStepperError via getHaapiStepperError
so the stepper surfaces them inline via useHaapiStepper().error.app, with copy
resolved from step.metadata.viewData.error.clientOperation.externalBrowserFlow.
{ launch | resume } — two keys mirroring Velocity's launch-error and
external-flow-end templates.

The EBF runner also fixes a latent listener+popup leak in the previous code:
the unexpected-origin/data branch now goes through cleanup instead of leaving
the message listener registered and the popup open.

runBankIdAuthentication adopts the contract too, but narrowed to the
success-only branch (Promise<{ clientOperationData }>) — the LWA's BankID
launcher has no client-side catchable failures today (anchor.click is fire-
and-forget; the real BankID failures land on the next polling step). When a
client-side failure mode eventually lands, widen to the full union and
introduce currentStep then.

File-layout follow-up: external-browser-flow.ts moves into its own
subdirectory with sibling typings.ts and index.ts barrel, matching the
webauthn/ and bankid/ conventions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Mirror webauthn refactor: remove viewData metadata lookup, return hardcoded copy via EXTERNAL_BROWSER_FLOW_ERROR_MESSAGES (forward-compat path preserved via void currentStep).
- Drop the now-obsolete metadata-key fallback test; remaining specs assert against the exported const.
- Guard openSpy.mock.calls[0][0] indexing with toHaveBeenCalledTimes(1) so a missed call surfaces as a clean assertion failure.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After moving EBF copy to the hardcoded WEBAUTHN_ERROR_MESSAGES-style
const in the runner, the speculative viewData typing has no consumers.
Removed pending BE actually shipping the keys.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@aleixsuau aleixsuau merged commit 9ae1ab7 into feature/IS-11327/webauthn-error-handling May 29, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.