Skip to content

IS-11570 Add browser back/forward navigation to the HAAPI stepper - #266

Open
aleixsuau wants to merge 9 commits into
devfrom
feature/dev/IS-11570-haapi-stepper-history-navigation
Open

IS-11570 Add browser back/forward navigation to the HAAPI stepper#266
aleixsuau wants to merge 9 commits into
devfrom
feature/dev/IS-11570-haapi-stepper-history-navigation

Conversation

@aleixsuau

@aleixsuau aleixsuau commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

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

Adds useHaapiStepperHistoryNavigation and mounts <HaapiStepperHistoryNavigation /> in the app, wiring the browser Back/Forward buttons to the HAAPI stepper:

  • Records every reached step as a browser-history entry, mirrored in an in-memory model.
  • Back/Forward re-opens the nearest reproducible step (GET link / GET form) in the direction of travel, skipping non-reproducible ones (POST forms, client operations).
  • Truncates abandoned forward branches, mirroring native pushState behaviour.
  • Skips polling steps entirely so they don't flood the history (they re-issue a GET poll action every interval).

Testing

Run a HAAPI flow and use the browser Back/Forward buttons: reproducible steps (GET links / GET forms) are revisited, non-reproducible ones (POST forms, client operations) are skipped.

Unit tests: npx vitest run src/shared/feature/history.

Deferred: R1 (Back on the first step)

R1 ("Back on the initial step must stay in the app, no reload") is not implemented in this PR. The first step reuses the document's load history entry, so pressing Back from it is a cross-document navigation that reloads the app — no popstate fires, so the hook can't intercept it.

Planned follow-up: seed a sentinel history entry behind the first step at mount, so Back lands on it (a same-document popstate) and snaps forward to the first step — trapping Back inside the flow (confirmed acceptable for the auth flow). A comment in useHaapiStepperHistoryNavigation.spec.ts documents this gap so it isn't mistaken for covered.

@aleixsuau
aleixsuau requested a review from luisgoncalves July 23, 2026 13:26
@aleixsuau
aleixsuau marked this pull request as ready for review July 27, 2026 08:49
Copilot AI review requested due to automatic review settings July 27, 2026 08:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds browser Back/Forward integration for the HAAPI stepper by persisting step progress into window.history state and replaying reproducible steps when users navigate the browser history.

Changes:

  • Refactors the HistoryNavigation adapter to provide stateless replace/push/go operations and an unsubscribe-based popstate listener API.
  • Introduces useHaapiStepperHistoryNavigation (plus a headless mounting component) to sync stepper history ↔ browser history and skip non-reproducible entries during navigation.
  • Adds unit tests for reproducible-action classification and history navigation behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/haapi-react-app/src/shared/util/browser-apis.ts Refactors the browser history adapter API to support stepper navigation needs (replace/push/go + unsubscribe).
src/haapi-react-app/src/shared/feature/history/useHaapiStepperHistoryNavigation.ts Implements the core stepper↔browser history synchronization and navigation skipping logic.
src/haapi-react-app/src/shared/feature/history/useHaapiStepperHistoryNavigation.spec.ts Adds unit tests covering recording, skipping, snapping back, and forward-branch truncation.
src/haapi-react-app/src/shared/feature/history/reproducible-action.ts Defines what actions/entries are safe to replay via history navigation.
src/haapi-react-app/src/shared/feature/history/reproducible-action.spec.ts Tests reproducibility classification for links, GET/POST forms, and client operations.
src/haapi-react-app/src/shared/feature/history/HaapiStepperHistoryNavigation.tsx Headless component that mounts the hook inside <HaapiStepper>.
src/haapi-react-app/src/App.tsx Mounts <HaapiStepperHistoryNavigation /> inside the app’s stepper tree.

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

Comment thread src/haapi-react-app/src/shared/feature/history/reproducible-action.ts Outdated
aleixsuau and others added 3 commits July 27, 2026 11:07
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@aleixsuau
aleixsuau force-pushed the feature/dev/IS-11570-haapi-stepper-history-navigation branch from 8388b37 to 695cb32 Compare July 27, 2026 09:25
@vahag-curity

Copy link
Copy Markdown
Contributor

some buttons do not show the text
image

Comment thread src/haapi-react-app/src/shared/util/browser-apis.ts
@vahag-curity

Copy link
Copy Markdown
Contributor

Review summary (IS-11570)

Reviewed against the ticket's acceptance criteria, then verified in a browser against a local Identity Server (Vite dev server on :8443 proxying to idsvr :9443, ui-experience api-driven, real OAuth flows using bankid1 and the html-form / 2FA authenticators). Inline comments carry the details.

Mechanics are clean: tsc -b, eslint, prettier --check and all 15 unit tests pass. The design reads well — single reproducibility predicate, headless mount point, injectable HistoryNavigation.

Confirmed working in the browser:

  • Back/Forward between reproducible steps (selector <-> login form) lands on the right step, no document reload
  • Forward into a non-reproducible frontier snaps back: popstate{2} -> go(-1) -> popstate{1} (AC3 in that direction)
  • Abandoned forward branches are truncated (AC4)

Blocking, in my view: the polling classification and the concurrent-replay desync — both produce user-visible wrong navigation in flows that ship by default — plus R1.

Two findings with no line to hang off:

1. No e2e coverage. test/haapi-react-app-e2e-tests/cypress/e2e already provisions the server and starts a real authorization flow, so cy.go('back') / cy.go('forward') would slot straight in. Worth adding: every defect I found surfaced in the browser and none from the unit suite, and the fake browser diverges from real semantics in at least two ways (see the two spec comments).

2. The ticket's SDK scope isn't in this PR. IS-11570's "In scope", AC5 and Technical Implementation call for historyIndex / canGoBack / canGoForward / back() / forward() in haapi-react-sdk plus a history-navigation.ts; grep finds zero occurrences of any of those anywhere under ui-kit/src, and the history model is reimplemented in the app instead. As a consequence AC3's "that direction is disabled (canGoBack/canGoForward false)" isn't representable. If this is a deliberate re-scope to app-only, the ticket should be updated.

Minor: the ticket says form actions are reproducible when method !== POST; the code uses method === GET. The code is the safer reading (PUT/PATCH/DELETE aren't replay-safe) — the ticket wording is what should change, not the code.

aleixsuau and others added 2 commits July 28, 2026 12:41
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@aleixsuau

Copy link
Copy Markdown
Contributor Author

some buttons do not show the text image

@urre @luisgoncalves I couldn't find this specific case in Confluence. How should we fix it?

aleixsuau and others added 4 commits July 28, 2026 14:47
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…not implemented).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ts the gap); fix template-literal lint error.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@luisgoncalves

luisgoncalves commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

some buttons do not show the text image

@urre @luisgoncalves I couldn't find this specific case in Confluence. How should we fix it?

This is unrelated, right? Probably something to improve on the server response. Maybe create a ticket and add it to the current sprint?

EDIT: this was actually fixed for 11.4 by not showing that specific screen (BankID polling failed) since it was redundant.

@luisgoncalves

Copy link
Copy Markdown
Contributor

I did a first round of local tests and the behavior looks good overall. Haven't checked the code changes yet (I'll get to that later).

I was testing a scenario with "user consent" enabled. E.g. initial request with username ACR, enter username, land on consent screen. In this case, the transition from authentication service to token service clears any session state on authentication, so clicking back issues a request to the username authenticator, which fails. But the history navigation still happened. And now it seems it's not possible to click "forward".

This is a bit of a tricky scenario; the authentication -> authorization transition is done via a POST, so I'm wondering if a new entry should have been added to history?

Btw: since R1 and R2 from the ticket are not addressed and seem to require more discussion, I suggest creating an integration branch for this feature as a whole.

@luisgoncalves luisgoncalves left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I found the current approach in useHaapiStepperHistoryNavigation a bit hard to follow, and it seems more complex than needed.

The way I thought about it, we can keep more on the browser history state, namely, keep an object like:

interface BrowserHistoryEntry {
  action: HaapiStepperNextStepAction;
  payload?: HaapiStepperNextStepPayload;
}

I believe those types from the stepper are serializable (pure data types).

Then, on syncBrowserHistoryOnStepperHistoryChange:

  • history is empty -> return
  • let current browser history entry = browserNavigation.getState()
  • if action ID in latest stepper history entry === action ID in current browser history entry -> return (handles the stepper history change due to browser navigation itself)
  • if stepper history entry is reproducible -> push browser history entry with {action, payload} (or replace if current entry state is null, meaning it's the first)

And on syncStepperHistoryOnBrowserHistoryChange:

  • let current browser history entry = browserNavigation.getState()
  • if current browser history entry -> stepper.nextStep with entry action/playload

Maybe it's missing some corner case, but seems quite simple, without need for local "copy" (BrowserHistory).

The reasoning is that a browser history entry contains the action/payload that led to it. also, an entry is only added if reproducible.

I did a quick PoC and it seems to work well. Did I miss something?

* Headless component that wires the browser's back/forward buttons to the HAAPI stepper history.
* Must be rendered inside `<HaapiStepper>` so it can access the stepper via `useHaapiStepper`.
*/
export function HaapiStepperHistoryNavigation() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You probably prefixed this with HaapiStepper because it requires the stepper. But we've been using this naming convention in the stepper feature in the SDK... so I'm wondering if we should drop the prefix here. I guess it's a detail.

} from '@curity/haapi-react-sdk/haapi-stepper/feature/stepper/haapi-stepper.types';

/** A reproducible step: the action (and payload) that re-opens it when navigating back/forward. */
export interface ReproducibleStep {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not used?


const updateBrowserHistoryAndNavigation =
!isSameStepperHistoryEntry(currentBrowserHistoryEntry, currentStepperHistoryEntry) &&
currentStepperHistoryEntry.step.type !== HAAPI_STEPS.POLLING;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Checking for polling steps here seems like a smell, because we already have other place where we check if a step is reproducible. I think this code also explains why I saw a step resulting from a POST being added to history.

@aleixsuau

aleixsuau commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

I found the current approach in useHaapiStepperHistoryNavigation a bit hard to follow, and it seems more complex than needed.

The way I thought about it, we can keep more on the browser history state, namely, keep an object like:

interface BrowserHistoryEntry {
  action: HaapiStepperNextStepAction;
  payload?: HaapiStepperNextStepPayload;
}

I believe those types from the stepper are serializable (pure data types).

Then, on syncBrowserHistoryOnStepperHistoryChange:

  • history is empty -> return
  • let current browser history entry = browserNavigation.getState()
  • if action ID in latest stepper history entry === action ID in current browser history entry -> return (handles the stepper history change due to browser navigation itself)
  • if stepper history entry is reproducible -> push browser history entry with {action, payload} (or replace if current entry state is null, meaning it's the first)

And on syncStepperHistoryOnBrowserHistoryChange:

  • let current browser history entry = browserNavigation.getState()
  • if current browser history entry -> stepper.nextStep with entry action/playload

Maybe it's missing some corner case, but seems quite simple, without need for local "copy" (BrowserHistory).

The reasoning is that a browser history entry contains the action/payload that led to it. also, an entry is only added if reproducible.

I did a quick PoC and it seems to work well. Did I miss something?

Hi @luisgoncalves 👋

Thanks for taking a look 👍

I explored keeping only reproducible steps in the browser history entry; omitting non-reproducible ones (like a POST result or BankID screen). The catch: while we're on a non-reproducible step, the browser's current position is still on the previous reproducible step, and the screen and the browser are out of sync by one.

Navigation example:

A (reproducible) → B (reproducible) → C (non-reproducible)

The browser only stores A and B. While C is on screen, the browser still thinks we're on B. So when pressing Back once, expecting to go back to B, the browser was already on B, so Back takes you to A, skipping B.
So Back from C lands on A, skipping B — the reproducible step that actually led to C.

The current ("aligned") design gives every step its own entry, so the browser always matches what's on screen. Then Back from C correctly returns to B. This proposal requires access to previous navigation steps' state (which the Navigation API does not allow), hence the need to keep them in memory (browserHistoryRef).

@luisgoncalves

luisgoncalves commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

I found the current approach in useHaapiStepperHistoryNavigation a bit hard to follow, and it seems more complex than needed.
The way I thought about it, we can keep more on the browser history state, namely, keep an object like:

interface BrowserHistoryEntry {
  action: HaapiStepperNextStepAction;
  payload?: HaapiStepperNextStepPayload;
}

I believe those types from the stepper are serializable (pure data types).
Then, on syncBrowserHistoryOnStepperHistoryChange:

  • history is empty -> return
  • let current browser history entry = browserNavigation.getState()
  • if action ID in latest stepper history entry === action ID in current browser history entry -> return (handles the stepper history change due to browser navigation itself)
  • if stepper history entry is reproducible -> push browser history entry with {action, payload} (or replace if current entry state is null, meaning it's the first)

And on syncStepperHistoryOnBrowserHistoryChange:

  • let current browser history entry = browserNavigation.getState()
  • if current browser history entry -> stepper.nextStep with entry action/playload

Maybe it's missing some corner case, but seems quite simple, without need for local "copy" (BrowserHistory).
The reasoning is that a browser history entry contains the action/payload that led to it. also, an entry is only added if reproducible.
I did a quick PoC and it seems to work well. Did I miss something?

Hi @luisgoncalves 👋

Thanks for taking a look 👍

I explored keeping only reproducible steps in the browser history entry; omitting non-reproducible ones (like a POST result or BankID screen). The catch: while we're on a non-reproducible step, the browser's current position is still on the previous reproducible step, and the screen and the browser are out of sync by one.

Navigation example:

A (reproducible) → B (reproducible) → C (non-reproducible)

The browser only stores A and B. While C is on screen, the browser still thinks we're on B. So when pressing Back once, expecting to go back to B, the browser was already on B, so Back takes you to A, skipping B. So Back from C lands on A, skipping B — the reproducible step that actually led to C.

The current ("aligned") design gives every step its own entry, so the browser always matches what's on screen. Then Back from C correctly returns to B. This proposal requires access to previous navigation steps' state (which the Navigation API does not allow), hence the need to keep them in memory (browserHistoryRef).

Thanks for the feedback @aleixsuau . I had missed that case. If I understood correctly, the approach is to find a reproducible step in the direction the user is navigating. To determine the direction, I'd say we only need the "previous index" and the current index. So, the onyl thing that cannot be in the browser pop state is the previous index.

While testing this I thought about a different approach: replace non-reproducible history entries, instead of pushing a new one. This basically omits them from history as soon as another reproducible action comes. In your approach, the non-reproducible entries are skipped, so it's almost the same thing (but in my idea, invalid entries won't show up in the history). Then, the only possible case for a non-reproducible step is the last step in the history.

Another thing I noticed is that POSt-redirect-GET scenarios are always considered non-reproducible, which is not ideal. This seems to be because in this case the stepper history surfaces the POST action (triggered by) with the final step that is the result of a GET (because redirects are followed automatically and not show in the history). This means that a safe point is lost (the triggered by action is the POSt, instead of the GET). One way to test this is with the Opt-in MFA action: selecting a factor, then navigating back takes the user to the authenticator before the action, instead of the factor selection.

I'll see if I can make a proposal.

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.

4 participants