fix(journey-client): add support for KBA allowUserDefinedQuestions flag #500
Conversation
🦋 Changeset detectedLatest commit: e99b374 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
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 |
WalkthroughThe changes add support for user-defined KBA (Knowledge-Based Authentication) questions by introducing an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as KBA UI Component
participant CB as KbaCreateCallback
User->>UI: Access KBA question form
UI->>CB: Check isAllowedUserDefinedQuestions()
CB-->>UI: Returns allowUserDefinedQuestions flag
alt User-defined option enabled
User->>UI: Select "user-defined" choice
UI->>UI: Show custom question input
User->>UI: Enter custom question text
User->>UI: Enter answer
UI->>CB: Update callback with custom values
else Predefined question
User->>UI: Select predefined question
User->>UI: Enter answer
UI->>CB: Update callback with predefined choice
end
CB-->>User: Callback submitted
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit e99b374
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/journey-client/src/lib/callbacks/kba-create-callback.test.ts (1)
27-31: Unit coverage for allowUserDefinedQuestions is appropriateExtending the fixture payload with
allowUserDefinedQuestions: trueand assertingisAllowedUserDefinedQuestions()returnstruegives direct coverage of the new accessor and wiring. Shared payload usage across tests is acceptable here since none depend on default values.If you want to harden tests further, you could add a small separate test for the default
falsebehavior when the flag is omitted.Also applies to: 58-61
e2e/journey-app/components/kba-create.ts (1)
37-43: Conditional user-defined option wiring looks goodAppending the
"user-defined"option only whenisAllowedUserDefinedQuestions()is true is a clean way to gate the custom-question path off the callback payload, and the value/text are consistent with how the tests drive the select.You might consider hoisting the sentinel
'user-defined'string into a small local constant to avoid drift between the option setup and the comparison in the change handler.e2e/journey-suites/src/registration.test.ts (1)
44-49: Custom KBA question flow and console assertions line up with the UIDriving security question 7 via the
"user-defined"option, then asserting the corresponding console messages for both questions and answers, matches the behavior implemented in the KBA component and gives good end-to-end coverage of the new flag-driven path.The inline comment above the Answer 8 fill still mentions
"Pizza"while the test uses"AAA Engineering"; consider updating the comment to avoid confusion during future maintenance.Also applies to: 67-70
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/config.json(1 hunks).changeset/eleven-baboons-battle.md(1 hunks)e2e/journey-app/components/kba-create.ts(2 hunks)e2e/journey-suites/src/registration.test.ts(2 hunks)packages/journey-client/src/lib/callbacks/kba-create-callback.test.ts(2 hunks)packages/journey-client/src/lib/callbacks/kba-create-callback.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-22T15:14:00.044Z
Learnt from: ryanbas21
Repo: ForgeRock/ping-javascript-sdk PR: 430
File: packages/journey-client/src/lib/callbacks/name-callback.ts:9-15
Timestamp: 2025-10-22T15:14:00.044Z
Learning: In packages/journey-client, callback classes are internal implementation details not part of the public API. The callbacks barrel (src/lib/callbacks/index.ts) intentionally only exports the base JourneyCallback class. Internal code imports concrete callback classes directly from their individual files (e.g., factory.ts, journey-client.ts).
Applied to files:
packages/journey-client/src/lib/callbacks/kba-create-callback.tspackages/journey-client/src/lib/callbacks/kba-create-callback.test.ts
🧬 Code graph analysis (1)
packages/journey-client/src/lib/callbacks/kba-create-callback.test.ts (1)
packages/journey-client/src/lib/callbacks/kba-create-callback.ts (1)
KbaCreateCallback(14-68)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Mend Code Security Check
🔇 Additional comments (3)
.changeset/eleven-baboons-battle.md (1)
1-5: Changeset entry is clear and correctly scopedPatch bump for
@forgerock/journey-clientwith a concise description of the new KBA flag support looks good and matches the code changes..changeset/config.json (1)
15-30: Ignoring journey-app and journey-suites in changesets makes senseAdding
@forgerock/journey-appand@forgerock/journey-suitestoignoreis consistent with treating them as demo/e2e tooling and prevents unnecessary releases.packages/journey-client/src/lib/callbacks/kba-create-callback.ts (1)
36-41: New accessor correctly surfaces the allowUserDefinedQuestions flag
isAllowedUserDefinedQuestionscleanly wrapsgetOutputByNamewith afalsedefault, which is a safe fallback when the flag is absent and aligns with the existing callback API style.
| const selectedQuestion = (event.target as HTMLInputElement).value; | ||
| if (selectedQuestion === 'user-defined') { | ||
| // If user-defined option is selected, prompt for custom question | ||
| const customQuestionLabel = document.createElement('label'); | ||
| customQuestionLabel.htmlFor = `${collectorKey}-question-user-defined`; | ||
| customQuestionLabel.innerText = 'Type your question ' + idx + ':'; | ||
|
|
||
| const customQuestionInput = document.createElement('input'); | ||
| customQuestionInput.type = 'text'; | ||
| customQuestionInput.id = `${collectorKey}-question-user-defined`; | ||
| customQuestionInput.placeholder = 'Type your question'; | ||
|
|
||
| container.lastElementChild?.before(customQuestionLabel); | ||
| container.lastElementChild?.before(customQuestionInput); | ||
| customQuestionInput.addEventListener('input', (e) => { | ||
| callback.setQuestion((e.target as HTMLInputElement).value); | ||
| console.log('Custom question ' + idx + ':', callback.getInputValue(0)); | ||
| }); | ||
| } else { | ||
| callback.setQuestion((event.target as HTMLInputElement).value); | ||
| console.log('Selected question ' + idx + ':', callback.getInputValue(0)); | ||
| } |
There was a problem hiding this comment.
Fix DOM insertion order and avoid duplicate custom question inputs
The dynamic custom-question handling works functionally, but there are a couple of rough edges:
- Using
container.lastElementChild?.before(...)inserts the custom label/input between the answer label and answer input, so the “Answer N” label is visually separated from its input. - Each time the user re-selects the “user-defined” option, a new label/input pair and event listener are created; nothing prevents duplicates.
You can keep behavior the same while improving UX and robustness by:
- Anchoring the custom fields directly after the question
<select>. - Creating the custom input only once per component.
- Typing the event target as
HTMLSelectElementand using achangelistener which is more idiomatic for<select>.
For example:
- questionInput.addEventListener('input', (event) => {
- const selectedQuestion = (event.target as HTMLInputElement).value;
+ questionInput.addEventListener('change', (event) => {
+ const selectEl = event.target as HTMLSelectElement;
+ const selectedQuestion = selectEl.value;
if (selectedQuestion === 'user-defined') {
- // If user-defined option is selected, prompt for custom question
- const customQuestionLabel = document.createElement('label');
- customQuestionLabel.htmlFor = `${collectorKey}-question-user-defined`;
- customQuestionLabel.innerText = 'Type your question ' + idx + ':';
-
- const customQuestionInput = document.createElement('input');
- customQuestionInput.type = 'text';
- customQuestionInput.id = `${collectorKey}-question-user-defined`;
- customQuestionInput.placeholder = 'Type your question';
-
- container.lastElementChild?.before(customQuestionLabel);
- container.lastElementChild?.before(customQuestionInput);
- customQuestionInput.addEventListener('input', (e) => {
- callback.setQuestion((e.target as HTMLInputElement).value);
- console.log('Custom question ' + idx + ':', callback.getInputValue(0));
- });
+ // If user-defined option is selected, prompt for custom question
+ let customQuestionInput = container.querySelector<HTMLInputElement>(
+ `#${collectorKey}-question-user-defined`,
+ );
+
+ if (!customQuestionInput) {
+ const customQuestionLabel = document.createElement('label');
+ customQuestionLabel.htmlFor = `${collectorKey}-question-user-defined`;
+ customQuestionLabel.innerText = 'Type your question ' + idx + ':';
+
+ customQuestionInput = document.createElement('input');
+ customQuestionInput.type = 'text';
+ customQuestionInput.id = `${collectorKey}-question-user-defined`;
+ customQuestionInput.placeholder = 'Type your question';
+
+ // Insert the custom question fields immediately after the select
+ questionInput.after(customQuestionLabel, customQuestionInput);
+
+ customQuestionInput.addEventListener('input', (e) => {
+ callback.setQuestion((e.target as HTMLInputElement).value);
+ console.log('Custom question ' + idx + ':', callback.getInputValue(0));
+ });
+ }
} else {
- callback.setQuestion((event.target as HTMLInputElement).value);
+ callback.setQuestion(selectEl.value);
console.log('Selected question ' + idx + ':', callback.getInputValue(0));
}
});This keeps the console output and tests intact while tightening the DOM behavior.
Also applies to: 88-90
🤖 Prompt for AI Agents
In e2e/journey-app/components/kba-create.ts around lines 64-85 (and similarly
lines 88-90), the select handling inserts duplicate custom inputs and places
them before the answer label/input; change the listener to use a "change" event
on an HTMLSelectElement, type the event target as HTMLSelectElement, and anchor
the custom label/input directly after the question <select> (e.g.,
insertAdjacentElement or nextSibling insertion) instead of using
container.lastElementChild.before(...). Ensure you only create the custom
label/input once by checking for an existing element with the user-defined id
(or querySelector) and reuse it (and its event listener) on subsequent
selections; when switching away from "user-defined" hide or remove the existing
custom input and update callback.setQuestion(...) from the select's value or the
persistent custom input's value accordingly.
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (18.81%) is below the target coverage (40.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #500 +/- ##
==========================================
+ Coverage 18.79% 18.81% +0.02%
==========================================
Files 140 140
Lines 27640 27647 +7
Branches 980 981 +1
==========================================
+ Hits 5195 5202 +7
Misses 22445 22445
🚀 New features to boost your workflow:
|
@forgerock/davinci-client
@forgerock/oidc-client
@forgerock/protect
@forgerock/sdk-types
@forgerock/sdk-utilities
@forgerock/iframe-manager
@forgerock/sdk-logger
@forgerock/sdk-oidc
@forgerock/sdk-request-middleware
@forgerock/storage
commit: |
|
Deployed 4595a14 to https://ForgeRock.github.io/ping-javascript-sdk/pr-500/4595a140668121ec13b9c766fb8764870a4ac723 branch gh-pages in ForgeRock/ping-javascript-sdk |
📦 Bundle Size Analysis📦 Bundle Size Analysis🚨 Significant Changes🔻 @forgerock/journey-client - 0.0 KB (-82.4 KB, -100.0%) 📊 Minor Changes📈 @forgerock/journey-client - 82.4 KB (+0.1 KB) ➖ No Changes➖ @forgerock/device-client - 9.2 KB 13 packages analyzed • Baseline from latest Legend🆕 New package ℹ️ How bundle sizes are calculated
🔄 Updated automatically on each push to this PR |
JIRA Ticket
https://pingidentity.atlassian.net/browse/SDKS-4284
Description
Adds helper function to get allowUserDefinedQuestions flag. Adds tests.
Added changeset
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.