Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fast-ways-rest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@forgerock/davinci-client': minor
---

Add QR code collector support to davinci-client
4 changes: 3 additions & 1 deletion .github/actions/setup/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ runs:
registry-url: 'https://registry.npmjs.org'

- name: Update npm
run: npm install -g npm@latest
run: |
corepack enable
corepack install -g npm@latest
shell: bash

- name: Install dependencies
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
with:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

- run: npx nx-cloud fix-ci
- run: pnpm nx fix-ci
if: always()

- uses: codecov/codecov-action@v5
Expand Down
33 changes: 33 additions & 0 deletions e2e/davinci-app/components/qr-code.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Copyright (c) 2025 Ping Identity Corporation. All rights reserved.
*
* This software may be modified and distributed under the terms
* of the MIT license. See the LICENSE file for details.
*/
import type { QrCodeCollector } from '@forgerock/davinci-client/types';

export default function (formEl: HTMLFormElement, collector: QrCodeCollector) {
if (collector.error) {
const errorEl = document.createElement('p');
errorEl.innerText = `QR Code error: ${collector.error}`;
formEl.appendChild(errorEl);
return;
}

const container = document.createElement('div');

const img = document.createElement('img');
img.src = collector.output.src;
img.alt = 'QR Code';
img.setAttribute('data-testid', 'qr-code-image');
container.appendChild(img);

if (collector.output.fallbackText) {
const fallback = document.createElement('p');
fallback.innerText = `Manual code: ${collector.output.fallbackText}`;
fallback.setAttribute('data-testid', 'qr-code-fallback');
container.appendChild(fallback);
}

formEl.appendChild(container);
}
3 changes: 3 additions & 0 deletions e2e/davinci-app/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import multiValueComponent from './components/multi-value.js';
import labelComponent from './components/label.js';
import objectValueComponent from './components/object-value.js';
import fidoComponent from './components/fido.js';
import qrCodeComponent from './components/qr-code.js';

const loggerFn = {
error: () => {
Expand Down Expand Up @@ -223,6 +224,8 @@ const urlParams = new URLSearchParams(window.location.search);
formEl, // You can ignore this; it's just for rendering
collector, // This is the plain object of the collector
);
} else if (collector.type === 'QrCodeCollector') {
qrCodeComponent(formEl, collector);
} else if (collector.type === 'TextCollector') {
textComponent(
formEl, // You can ignore this; it's just for rendering
Expand Down
11 changes: 10 additions & 1 deletion e2e/davinci-app/server-configs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ export const serverConfigs: Record<string, DaVinciConfig> = {
},
/**
* Phone Number Input With Email and Password
*
*/
'20dd0ed0-bb9b-4c8f-9a60-9ebeb4b348e0': {
clientId: '20dd0ed0-bb9b-4c8f-9a60-9ebeb4b348e0',
Expand All @@ -68,4 +67,14 @@ export const serverConfigs: Record<string, DaVinciConfig> = {
'https://auth.pingone.ca/02fb4743-189a-4bc7-9d6c-a919edfe6447/as/.well-known/openid-configuration',
},
},
/** QR Code policy id : aa3c00c3ec25a9721be078f7bf44678d **/
'c12743f9-08e8-4420-a624-71bbb08e9fe1': {
clientId: 'c12743f9-08e8-4420-a624-71bbb08e9fe1',
redirectUri: window.location.origin + '/',
scope: 'openid profile email',
serverConfig: {
wellknown:
'https://auth.pingone.ca/02fb4743-189a-4bc7-9d6c-a919edfe6447/as/.well-known/openid-configuration',
},
},
};
53 changes: 53 additions & 0 deletions e2e/davinci-suites/src/qr-code.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Copyright (c) 2025 Ping Identity Corporation. All rights reserved.
*
* This software may be modified and distributed under the terms
* of the MIT license. See the LICENSE file for details.
*/
import { expect, test } from '@playwright/test';
import { asyncEvents } from './utils/async-events.js';
import { password, username } from './utils/demo-user.js';

const qrCodeUrl =
'/?clientId=c12743f9-08e8-4420-a624-71bbb08e9fe1&acr_values=9da1b93991bcd577947da228ad4c741f';

test('QR code renders after navigating through device registration flow', async ({ page }) => {
const { navigate } = asyncEvents(page);

await navigate(qrCodeUrl);

// Step 1: Login
await page.getByRole('button', { name: 'USER_LOGIN' }).click();
await page.waitForEvent('requestfinished');

await page.getByLabel('Username').fill(username);
await page.getByLabel('Password').fill(password);
await page.getByRole('button', { name: 'Sign On' }).click();
await page.waitForEvent('requestfinished');

// Step 2: Select device registration
await page.getByRole('button', { name: 'DEVICE_REGISTRATION' }).click();
await page.waitForEvent('requestfinished');

// Step 3: Choose "Mobile App" from the device selection screen
await expect(page.getByText('MFA Device Selection - Registration')).toBeVisible();
await page.getByRole('button', { name: 'Mobile App' }).click();
await page.waitForEvent('requestfinished');
Comment on lines +20 to +35
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Playwright, is using page.waitForEvent('requestfinished') after a click considered flaky, and what are the recommended alternatives for deterministic synchronization in E2E tests?

💡 Result:

Yes—page.waitForEvent('requestfinished') after a click() is often flaky/racy in E2E tests because:

  • requestfinished fires for any request that completes (including redirects and even HTTP error responses like 404/503), so you can easily synchronize on the “wrong” request when multiple requests are in flight. [1]
  • If you start waiting after the click, you can miss fast network activity and then hang until timeout (classic race pattern).

Recommended deterministic alternatives

  1. Wait for a specific response (preferred when an API call is the real “done” signal)
    Register the wait before the click and match by URL/predicate:

    const respPromise = page.waitForResponse(r =>
      r.url().includes('/api/checkout') && r.status() === 200
    );
    await page.getByRole('button', { name: 'Buy' }).click();
    await respPromise;
  2. Wait for URL / navigation deterministically (preferred when the user-visible result is navigation)
    Use page.waitForURL() (Playwright explicitly recommends URL waiting patterns for navigation sync). [3]
    Typical race-safe pattern:

    await Promise.all([
      page.waitForURL('**/order/confirmation'),
      page.getByRole('button', { name: 'Buy' }).click(),
    ]);
  3. Wait on user-visible UI state via “web-first” assertions (preferred for true E2E behavior)
    Playwright recommends asserting what the user sees; these assertions auto-wait/retry until the condition is met. [2]

    await page.getByRole('button', { name: 'Buy' }).click();
    await expect(page.getByText('Order confirmed')).toBeVisible();
  4. If you must sync with network, target the request/response—not “any finished request”
    Prefer waitForResponse / waitForRequest with a predicate over requestfinished (which is broad and easy to mismatch). [1]

Source list

[1] Playwright docs: request event order and meaning of requestfinished (incl. redirects and HTTP error responses still ending in requestfinished). (playwright.dev)
[2] Playwright docs: Best Practices—test user-visible behavior; use web-first assertions that auto-wait. (playwright.dev)
[3] Playwright docs: Navigations—“Waiting for navigation” using page.waitForURL(). (playwright.dev)

Citations:


Unscoped requestfinished waits introduce race conditions and test flakiness.

On lines 21, 26, 30, and 35, page.waitForEvent('requestfinished') resolves on any finished request (including unrelated redirects and HTTP error responses). Starting the wait after the click creates a classic race: if the network activity completes before the listener is registered, the test will hang until timeout.

Replace with UI-state waits (web-first assertions that auto-retry) or scoped response waits matched by URL:

Stabilization pattern
-  await page.getByRole('button', { name: 'USER_LOGIN' }).click();
-  await page.waitForEvent('requestfinished');
+  await page.getByRole('button', { name: 'USER_LOGIN' }).click();
+  await expect(page.getByLabel('Username')).toBeVisible();

   await page.getByLabel('Username').fill(username);
   await page.getByLabel('Password').fill(password);
-  await page.getByRole('button', { name: 'Sign On' }).click();
-  await page.waitForEvent('requestfinished');
+  await page.getByRole('button', { name: 'Sign On' }).click();
+  await expect(page.getByRole('button', { name: 'DEVICE_REGISTRATION' })).toBeVisible();

-  await page.getByRole('button', { name: 'DEVICE_REGISTRATION' }).click();
-  await page.waitForEvent('requestfinished');
+  await page.getByRole('button', { name: 'DEVICE_REGISTRATION' }).click();
+  await expect(page.getByText('MFA Device Selection - Registration')).toBeVisible();

   await expect(page.getByText('MFA Device Selection - Registration')).toBeVisible();
-  await page.getByRole('button', { name: 'Mobile App' }).click();
-  await page.waitForEvent('requestfinished');
+  await page.getByRole('button', { name: 'Mobile App' }).click();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@e2e/davinci-suites/src/qr-code.test.ts` around lines 20 - 35, The unscoped
page.waitForEvent('requestfinished') calls after clicks (seen around
page.getByRole(..., { name: 'USER_LOGIN' }), page.getByRole(..., { name: 'Sign
On' }), page.getByRole(..., { name: 'DEVICE_REGISTRATION' }), and
page.getByRole(..., { name: 'Mobile App' })) are racy; replace them with either
(a) web-first UI assertions that auto-retry (e.g. await
expect(page.getByText('MFA Device Selection - Registration')).toBeVisible() or
expecting a navigation/URL change via toHaveURL) or (b) scoped network waits
that start before the action (use Promise.all with page.waitForResponse(resp =>
resp.url().includes('<unique-path>') && resp.status() === 200) and the
.click()), matching the specific request path for the LOGIN, Sign On,
DEVICE_REGISTRATION and Mobile App actions instead of waiting for any finished
request.


// Step 4: QR code should now be visible
const qrImage = page.locator('[data-testid="qr-code-image"]');
await expect(qrImage).toBeVisible({ timeout: 10000 });

// Verify the image has a base64-encoded src
const src = await qrImage.getAttribute('src');
expect(src).toBeTruthy();
expect(src).toContain('data:image/png;base64,');

// Verify fallback text is displayed if present
const fallback = page.locator('[data-testid="qr-code-fallback"]');
const fallbackVisible = await fallback.isVisible();
if (fallbackVisible) {
const fallbackText = await fallback.textContent();
expect(fallbackText).toBeTruthy();
}
Comment on lines +49 to +52
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Strengthen fallback-text assertion to validate actual code content.

toBeTruthy() can still pass for "Manual code: " without a meaningful code. Consider asserting non-empty trimmed content after removing the prefix.

Suggested assertion tweak
   if (fallbackVisible) {
-    const fallbackText = await fallback.textContent();
-    expect(fallbackText).toBeTruthy();
+    const fallbackText = (await fallback.textContent()) ?? '';
+    const manualCode = fallbackText.replace(/^Manual code:\s*/i, '').trim();
+    expect(manualCode).not.toEqual('');
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@e2e/davinci-suites/src/qr-code.test.ts` around lines 49 - 52, The current
assertion uses fallbackVisible and fallback.textContent() with toBeTruthy(),
which allows responses like "Manual code: " to pass; instead read the text via
fallback.textContent(), remove the known prefix (e.g., "Manual code:"), trim the
remainder, and assert that the trimmed string is non-empty (or matches /\S/).
Update the test around the fallbackVisible branch to compute const text = await
fallback.textContent(), strip the prefix and whitespace, then
expect(theStrippedText.length).toBeGreaterThan(0) (or
expect(theStrippedText).toMatch(/\S/)), referencing the existing fallbackVisible
and fallback variables.

});
26 changes: 23 additions & 3 deletions packages/davinci-client/src/lib/collector.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ export type SubmitCollector = ActionCollectorNoUrl<'SubmitCollector'>;
/**
* @interface NoValueCollector - Represents a collector that collects no value; text only for display.
*/
export type NoValueCollectorTypes = 'ReadOnlyCollector' | 'NoValueCollector';
export type NoValueCollectorTypes = 'ReadOnlyCollector' | 'NoValueCollector' | 'QrCodeCollector';

export interface NoValueCollectorBase<T extends NoValueCollectorTypes> {
category: 'NoValueCollector';
Expand All @@ -497,6 +497,21 @@ export interface NoValueCollectorBase<T extends NoValueCollectorTypes> {
};
}

export interface QrCodeCollectorBase {
category: 'NoValueCollector';
error: string | null;
type: 'QrCodeCollector';
id: string;
name: string;
output: {
key: string;
label: string;
type: string;
src: string;
fallbackText: string;
};
}

/**
* Type to help infer the collector based on the collector type
* Used specifically in the returnNoValueCollector wrapper function.
Expand All @@ -507,16 +522,21 @@ export interface NoValueCollectorBase<T extends NoValueCollectorTypes> {
export type InferNoValueCollectorType<T extends NoValueCollectorTypes> =
T extends 'ReadOnlyCollector'
? NoValueCollectorBase<'ReadOnlyCollector'>
: NoValueCollectorBase<'NoValueCollector'>;
: T extends 'QrCodeCollector'
? QrCodeCollectorBase
: NoValueCollectorBase<'NoValueCollector'>;

export type NoValueCollectors =
| NoValueCollectorBase<'NoValueCollector'>
| NoValueCollectorBase<'ReadOnlyCollector'>;
| NoValueCollectorBase<'ReadOnlyCollector'>
| QrCodeCollectorBase;

export type NoValueCollector<T extends NoValueCollectorTypes> = NoValueCollectorBase<T>;

export type ReadOnlyCollector = NoValueCollectorBase<'ReadOnlyCollector'>;

export type QrCodeCollector = QrCodeCollectorBase;

export type UnknownCollector = {
category: 'UnknownCollector';
error: string | null;
Expand Down
76 changes: 76 additions & 0 deletions packages/davinci-client/src/lib/collector.utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
returnObjectValueCollector,
returnSingleValueAutoCollector,
returnObjectValueAutoCollector,
returnQrCodeCollector,
} from './collector.utils.js';
import type {
DaVinciField,
Expand All @@ -31,6 +32,7 @@ import type {
FidoRegistrationField,
PhoneNumberField,
ProtectField,
QrCodeField,
ReadOnlyField,
RedirectField,
StandardField,
Expand Down Expand Up @@ -809,6 +811,80 @@ describe('No Value Collectors', () => {
});
});

describe('returnQrCodeCollector', () => {
it('should return a valid QrCodeCollector with src and fallbackText', () => {
const mockField: QrCodeField = {
type: 'QR_CODE',
key: 'qr-code-field',
content: 'data:image/png;base64,abc123',
fallbackText: '04ZKS2KCIWKXT8FHRX',
};
const result = returnQrCodeCollector(mockField, 2);
expect(result).toEqual({
category: 'NoValueCollector',
error: null,
type: 'QrCodeCollector',
id: 'qr-code-field-2',
name: 'qr-code-field-2',
output: {
key: 'qr-code-field-2',
label: 'data:image/png;base64,abc123',
type: 'QR_CODE',
src: 'data:image/png;base64,abc123',
fallbackText: '04ZKS2KCIWKXT8FHRX',
},
});
});

it('should handle missing fallbackText gracefully', () => {
const mockField: QrCodeField = {
type: 'QR_CODE',
key: 'qr-code-field',
content: 'data:image/png;base64,abc123',
};
const result = returnQrCodeCollector(mockField, 0);
expect(result).toEqual({
category: 'NoValueCollector',
error: null,
type: 'QrCodeCollector',
id: 'qr-code-field-0',
name: 'qr-code-field-0',
output: {
key: 'qr-code-field-0',
label: 'data:image/png;base64,abc123',
type: 'QR_CODE',
src: 'data:image/png;base64,abc123',
fallbackText: '',
},
});
});

it('should set error when content is missing', () => {
const mockField = { type: 'QR_CODE', key: 'qr-code-field' } as unknown as QrCodeField;
const result = returnQrCodeCollector(mockField, 0);
expect(result.error).toContain('Content is not found');
expect(result.output.src).toBe('');
});

it('should set error and fall back to type when key is missing', () => {
const mockField = {
type: 'QR_CODE',
content: 'data:image/png;base64,abc123',
} as unknown as QrCodeField;
const result = returnQrCodeCollector(mockField, 0);
expect(result.error).toContain('Key is not found');
expect(result.id).toBe('QR_CODE-0');
expect(result.name).toBe('QR_CODE-0');
});

it('should accumulate multiple errors when key and content are missing', () => {
const mockField = { type: 'QR_CODE' } as unknown as QrCodeField;
const result = returnQrCodeCollector(mockField, 0);
expect(result.error).toContain('Content is not found');
expect(result.error).toContain('Key is not found');
});
});

describe('returnSingleValueAutoCollector', () => {
it('should create a valid ProtectCollector', () => {
const mockField: ProtectField = {
Expand Down
29 changes: 28 additions & 1 deletion packages/davinci-client/src/lib/collector.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import type {
AutoCollectors,
SingleValueAutoCollectorTypes,
ObjectValueAutoCollectorTypes,
QrCodeCollectorBase,
} from './collector.types.js';
import type {
DeviceAuthenticationField,
Expand All @@ -38,6 +39,7 @@ import type {
MultiSelectField,
PhoneNumberField,
ProtectField,
QrCodeField,
ReadOnlyField,
RedirectField,
SingleSelectField,
Expand Down Expand Up @@ -678,7 +680,7 @@ export function returnObjectValueCollector(
* @returns {NoValueCollector} The constructed NoValueCollector object.
*/
export function returnNoValueCollector<
Field extends ReadOnlyField,
Field extends ReadOnlyField | QrCodeField,
CollectorType extends NoValueCollectorTypes = 'NoValueCollector',
>(field: Field, idx: number, collectorType: CollectorType) {
let error = '';
Expand Down Expand Up @@ -713,6 +715,31 @@ export function returnReadOnlyCollector(field: ReadOnlyField, idx: number) {
return returnNoValueCollector(field, idx, 'ReadOnlyCollector');
}

/**
* @function returnQrCodeCollector - Creates a QrCodeCollector object for displaying QR code images.
* @param {QrCodeField} field - The field object containing key, content, type, and optional fallbackText.
* @param {number} idx - The index to be used in the id of the QrCodeCollector.
* @returns {QrCodeCollectorBase} The constructed QrCodeCollector object.
*/
export function returnQrCodeCollector(field: QrCodeField, idx: number): QrCodeCollectorBase {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we reuse the generic returnNoValueCollector function? I feel like we can change around the properties a bit to make it fit. It feels like fallbackText should be the label and we just need to introduce a new prop for the src.

const base = returnNoValueCollector(field, idx, 'QrCodeCollector');

let error = base.error || '';
if (!('key' in field) || !field.key) {
error = `${error}Key is not found in the field object. `;
}

return {
...base,
error: error || null,
output: {
...base.output,
src: field.content || '',
fallbackText: field.fallbackText || '',
},
Comment on lines +732 to +739
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Normalize output.label for malformed QR payloads.

On Line 736, label is inherited from base.output and can be non-string at runtime when server payloads are malformed. This breaks the QrCodeCollectorBase runtime shape guarantee. Normalize it to '' the same way src/fallbackText are normalized.

Suggested patch
   return {
     ...base,
     error: error || null,
     output: {
       ...base.output,
+      label: typeof base.output.label === 'string' ? base.output.label : '',
       src: field.content || '',
       fallbackText: field.fallbackText || '',
     },
   };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return {
...base,
error: error || null,
output: {
...base.output,
src: field.content || '',
fallbackText: field.fallbackText || '',
},
return {
...base,
error: error || null,
output: {
...base.output,
label: typeof base.output.label === 'string' ? base.output.label : '',
src: field.content || '',
fallbackText: field.fallbackText || '',
},
};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/davinci-client/src/lib/collector.utils.ts` around lines 732 - 739,
The returned object currently spreads base.output into output, but
base.output.label can be a non-string when server payloads are malformed; update
the output normalization in the return of the function (the object that merges
...base and output: {...base.output, src: field.content || '', fallbackText:
field.fallbackText || ''}) to explicitly normalize label to a string (e.g., set
label to base.output.label if typeof base.output.label === 'string' else ''), so
QrCodeCollectorBase's runtime shape guarantee is preserved.

};
}

/**
* @function returnValidator - Creates a validator function based on the provided collector
* @param {ValidatedTextCollector | ObjectValueCollectors | MultiValueCollectors | AutoCollectors} collector - The collector to which the value will be validated
Expand Down
9 changes: 8 additions & 1 deletion packages/davinci-client/src/lib/davinci.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,13 @@ export type ReadOnlyField = {
key?: string;
};

export type QrCodeField = {
type: 'QR_CODE';
key: string;
content: string;
fallbackText?: string;
};

export type RedirectField = {
type: 'SOCIAL_LOGIN_BUTTON';
key: string;
Expand Down Expand Up @@ -221,7 +228,7 @@ export type ComplexValueFields =
| FidoRegistrationField
| FidoAuthenticationField;
export type MultiValueFields = MultiSelectField;
export type ReadOnlyFields = ReadOnlyField;
export type ReadOnlyFields = ReadOnlyField | QrCodeField;
export type RedirectFields = RedirectField;
export type SingleValueFields = StandardField | ValidatedField | SingleSelectField | ProtectField;

Expand Down
Loading