Skip to content

fix(mfa-step): create a new step for handling MFA set up page text output callback script#489

Merged
vatsalparikh merged 1 commit into
mainfrom
sdks-4918
May 13, 2026
Merged

fix(mfa-step): create a new step for handling MFA set up page text output callback script#489
vatsalparikh merged 1 commit into
mainfrom
sdks-4918

Conversation

@vatsalparikh
Copy link
Copy Markdown
Contributor

@vatsalparikh vatsalparikh commented May 4, 2026

JIRA Ticket

https://pingidentity.atlassian.net/browse/SDKS-4918

Description

This PR addresses scripted nodes for MFA during 2FA registration process. More details in the ticket

Reasoning

  • MFA enrollment, Get authenticator, and App links screens are UI concerns only, there is no data transformation or extraction happening here. That's why catching these scripts and implementing them in login framework and not in Ping SDK.
  • MFA enrollment, Get authenticator, and App links screens are more than callbacks, they are a combination of multiple callbacks like text output, hidden value, confirmation callback. That's why I implemented them as a separate stage.
  • I was itching to use custom stages and callbacks here, but because the feature is experimental I did not take that route.
  • For rendering the screens, I've tried to reuse Alert, Text and other common components, so the styling is slightly off but follows the styling in the repo for Alert and Text.
  • Open to suggestions on how to better detect all three screens, and whether we should separate out various steps in the mfa enrollment or if you have a better idea on how to handle scripted nodes safely (to avoid xss).

How to test

  • Make sure the FR_REALM_PATH=/ and FR_AM_WELLKNOWN_URL=https://openam-sdks.forgeblocks.com/am/oauth2/realms/root/realms/root/.well-known/openid-configuration are configured so that you are in the root realm and not in the alpha realm
  • Start up the app and login with your admin credentials on https://localhost:8443. These are the credentials you use to login to openam-sdks tenant. Do not use a user you've created in the openam-sdks tenant because those users are not admins.
  • Once you login you should see the Set up screen. Click on 'Set up', then on 'Next', then on 'Download the app'. All three screens were showing text and now show proper nodes.
  • You can refer to the recording, which shows both text scripts before the changes and the nodes after the changes.

Recording

+error.svelte file

I forgot to push the +error.svelte file from Gabriel's comment in journey client PR. Since it's a generic error file I have attached it in this PR.
#468 (comment)

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 4, 2026

🦋 Changeset detected

Latest commit: 9ba8b34

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@forgerock/login-widget Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Base automatically changed from sdks-4796-journey-client to main May 5, 2026 19:04
@vatsalparikh vatsalparikh force-pushed the sdks-4918 branch 2 times, most recently from d3ec6f6 to 0c0887e Compare May 6, 2026 00:25
Copy link
Copy Markdown
Contributor

@SteinGabriel SteinGabriel left a comment

Choose a reason for hiding this comment

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

Initial scan.

Comment thread core/journey/_utilities/map-stage.utilities.ts
Comment thread core/journey/_utilities/map-stage.utilities.ts
Comment thread core/journey/_utilities/map-stage.utilities.ts
@vatsalparikh vatsalparikh force-pushed the sdks-4918 branch 2 times, most recently from bc799f0 to ad2a3e0 Compare May 6, 2026 23:01
@vatsalparikh vatsalparikh requested a review from SteinGabriel May 6, 2026 23:02
Comment thread core/locales/us/en/index.json Outdated
@vatsalparikh vatsalparikh force-pushed the sdks-4918 branch 3 times, most recently from 1bfbb02 to e16d505 Compare May 7, 2026 23:43
Comment thread core/sdk.config.ts
Copy link
Copy Markdown
Contributor

@cerebrl cerebrl left a comment

Choose a reason for hiding this comment

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

This looks good, other than the condition I commented on.

Comment thread core/journey/_utilities/map-stage.utilities.ts Outdated
@vatsalparikh vatsalparikh force-pushed the sdks-4918 branch 4 times, most recently from 3e2e2ad to 2986214 Compare May 12, 2026 13:56
Copy link
Copy Markdown
Contributor

@SteinGabriel SteinGabriel left a comment

Choose a reason for hiding this comment

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

The changes look good and work as expected.
It seems like a changeset is needed, but other than that it looks ready to merge.

@vatsalparikh
Copy link
Copy Markdown
Contributor Author

The changes look good and work as expected. It seems like a changeset is needed, but other than that it looks ready to merge.

Added changeset, thanks!

@vatsalparikh vatsalparikh merged commit eef8b31 into main May 13, 2026
21 checks passed
@vatsalparikh vatsalparikh deleted the sdks-4918 branch May 13, 2026 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants