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/whole-mangos-find.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@forgerock/journey-client': patch
---

Return `JourneyLoginFailure` by hitting the previously-unreached `LoginFailure` branch when `start()`/`next()` receives a failure payload with a login failure `code`
5 changes: 4 additions & 1 deletion e2e/journey-app/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,11 @@ if (searchParams.get('middleware') === 'true') {
renderComplete();
} else if (step?.type === 'LoginFailure') {
console.error('Journey failed');
renderForm();
renderError();
const errorHtml = errorEl.innerHTML;
step = await journeyClient.start({ journey: journeyName });
renderForm();
errorEl.innerHTML = errorHtml;
Comment on lines +211 to +213
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 | ⚡ Quick win

Guard the restart result before calling renderForm().

journeyClient.start() can return non-Step results. Calling renderForm() unconditionally can throw and break the submit flow on retry failures.

🧰 Tools
🪛 ast-grep (0.43.0)

[warning] 212-212: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: errorEl.innerHTML = errorHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html

(dom-content-modification)


[warning] 212-212: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: errorEl.innerHTML = errorHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html

(unsafe-html-content-assignment)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@e2e/journey-app/main.ts` around lines 211 - 213, The call to
journeyClient.start({ journey: journeyName }) can return non-Step results, so
guard the result before calling renderForm(); check the returned value (the
variable step) is a valid Step (e.g., truthy and has the expected
method/property your flow relies on) and only call renderForm() when that check
passes; if the result is not a valid Step, set errorEl.innerHTML = errorHtml (or
update the UI appropriately) and bail out/return to avoid throwing during retry
handling. Ensure you reference the journeyClient.start call and the step
variable when adding the conditional guard around renderForm().

} else {
console.error('Unknown node status', step);
}
Expand Down
5 changes: 4 additions & 1 deletion packages/journey-client/api-report/journey-client.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { RequestMiddleware } from '@forgerock/sdk-request-middleware';
import { Step } from '@forgerock/sdk-types';
import { StepDetail } from '@forgerock/sdk-types';
import { StepType } from '@forgerock/sdk-types';
import { WellknownResponse } from '@forgerock/sdk-types';

export { ActionTypes }

Expand Down Expand Up @@ -228,7 +229,7 @@ export type JourneyLoginSuccess = AuthResponse & {
getSuccessUrl: () => string | undefined;
};

// @public
// @public (undocumented)
export type JourneyResult = JourneyStep | JourneyLoginSuccess | JourneyLoginFailure | GenericError;

// @public
Expand Down Expand Up @@ -499,6 +500,8 @@ export class ValidatedCreateUsernameCallback extends BaseCallback {
setValidateOnly(value: boolean): void;
}

export { WellknownResponse }

// (No @packageDocumentation comment for this package)

```
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { RequestMiddleware } from '@forgerock/sdk-request-middleware';
import { Step } from '@forgerock/sdk-types';
import { StepDetail } from '@forgerock/sdk-types';
import { StepType } from '@forgerock/sdk-types';
import { WellknownResponse } from '@forgerock/sdk-types';

export { ActionTypes }

Expand Down Expand Up @@ -215,7 +216,7 @@ export type JourneyLoginSuccess = AuthResponse & {
getSuccessUrl: () => string | undefined;
};

// @public
// @public (undocumented)
export type JourneyResult = JourneyStep | JourneyLoginSuccess | JourneyLoginFailure | GenericError;

// @public
Expand Down Expand Up @@ -486,6 +487,8 @@ export class ValidatedCreateUsernameCallback extends BaseCallback {
setValidateOnly(value: boolean): void;
}

export { WellknownResponse }

// (No @packageDocumentation comment for this package)

```
1 change: 1 addition & 0 deletions packages/journey-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
"@forgerock/sdk-utilities": "workspace:*",
"@forgerock/storage": "workspace:*",
"@reduxjs/toolkit": "catalog:",
"effect": "catalog:effect",
"tslib": "catalog:"
},
"devDependencies": {
Expand Down
73 changes: 65 additions & 8 deletions packages/journey-client/src/lib/client.store.test.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
// @vitest-environment node
/*
* Copyright (c) 2025 Ping Identity Corporation. All rights reserved.
* Copyright (c) 2025-2026 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 { callbackType } from '@forgerock/sdk-types';
import { afterEach, describe, expect, test, vi } from 'vitest';

import type { GenericError, Step, WellknownResponse } from '@forgerock/sdk-types';

import { journey } from './client.store.js';
import { createJourneyStep } from './step.utils.js';

import { callbackType, type GenericError, type Step, type WellknownResponse } from '../index.js';

import { JourneyClientConfig } from './config.types.js';

/**
Expand Down Expand Up @@ -76,7 +76,7 @@ function getUrlFromInput(input: RequestInfo | URL): string {
/**
* Helper to setup mock fetch for wellknown + journey responses
*/
function setupMockFetch(journeyResponse: Step | null = null) {
function setupMockFetch(journeyResponse: Step | null = null, authenticateStatus = 200) {
mockFetch.mockImplementation((input: RequestInfo | URL) => {
const url = getUrlFromInput(input);

Expand All @@ -86,8 +86,13 @@ function setupMockFetch(journeyResponse: Step | null = null) {
}

// Journey authenticate endpoint
if (journeyResponse && url.includes('/authenticate')) {
return Promise.resolve(new Response(JSON.stringify(journeyResponse)));
if (url.includes('/authenticate')) {
if (journeyResponse === null) {
return Promise.reject(new Error(`Unexpected fetch: ${url}`));
}
return Promise.resolve(
new Response(JSON.stringify(journeyResponse), { status: authenticateStatus }),
);
}

return Promise.reject(new Error(`Unexpected fetch: ${url}`));
Expand Down Expand Up @@ -154,6 +159,30 @@ describe('journey-client', () => {
}
});

test('start_401WithStepPayload_ReturnsLoginFailure', async () => {
const failurePayload: Step = {
code: 401,
message: 'Access Denied',
reason: 'Unauthorized',
detail: { failureUrl: 'https://example.com/failure' },
};
setupMockFetch(failurePayload, 401);

const client = await journey({ config: mockConfig });
const result = await client.start();

expect(result).toBeDefined();
expect(isGenericError(result)).toBe(false);
expect(result).toHaveProperty('type', 'LoginFailure');

if (!isGenericError(result) && result.type === 'LoginFailure') {
expect(result.payload).toEqual(failurePayload);
expect(result.getCode()).toBe(401);
expect(result.getMessage()).toBe('Access Denied');
expect(result.getReason()).toBe('Unauthorized');
}
});

test('next_WellknownConfig_SendsStepAndReturnsNext', async () => {
const initialStep = createJourneyStep({
authId: 'test-auth-id',
Expand Down Expand Up @@ -194,6 +223,34 @@ describe('journey-client', () => {
}
});

test('next_401WithStepPayload_ReturnsLoginFailure', async () => {
const initialStep = createJourneyStep({
authId: 'test-auth-id',
callbacks: [],
});
const failurePayload: Step = {
code: 401,
message: 'Access Denied',
reason: 'Unauthorized',
detail: { failureUrl: 'https://example.com/failure' },
};
setupMockFetch(failurePayload, 401);

const client = await journey({ config: mockConfig });
const result = await client.next(initialStep, {});

expect(result).toBeDefined();
expect(isGenericError(result)).toBe(false);
expect(result).toHaveProperty('type', 'LoginFailure');

if (!isGenericError(result) && result.type === 'LoginFailure') {
expect(result.payload).toEqual(failurePayload);
expect(result.getCode()).toBe(401);
expect(result.getMessage()).toBe('Access Denied');
expect(result.getReason()).toBe('Unauthorized');
}
});

test('redirect_WellknownConfig_StoresStepAndCallsLocationAssign', async () => {
const mockStepPayload: Step = {
callbacks: [
Expand Down Expand Up @@ -366,7 +423,7 @@ describe('journey-client', () => {

expect(isGenericError(result)).toBe(true);
if (isGenericError(result)) {
expect(result.error).toBe('no_response_data');
expect(result.error).toBe('request_failed');
expect(result.type).toBe('unknown_error');
}
});
Expand Down
41 changes: 14 additions & 27 deletions packages/journey-client/src/lib/client.store.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2025 Ping Identity Corporation. All rights reserved.
* Copyright (c) 2025-2026 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.
Expand All @@ -21,18 +21,15 @@ import { createJourneyStore } from './client.store.utils.js';
import { configSlice } from './config.slice.js';
import { journeyApi } from './journey.api.js';
import { createStorage } from '@forgerock/storage';
import { createJourneyObject } from './journey.utils.js';
import { match } from 'effect/Either';
import { createJourneyObject, parseJourneyResponse } from './journey.utils.js';
import type { JourneyResult } from './journey.utils.js';
import { wellknownApi } from './wellknown.api.js';

import type { JourneyStep } from './step.utils.js';
import type { JourneyClientConfig } from './config.types.js';
import type { RedirectCallback } from './callbacks/redirect-callback.js';
import type { NextOptions, StartParam, ResumeOptions } from './interfaces.js';
import type { JourneyLoginFailure } from './login-failure.utils.js';
import type { JourneyLoginSuccess } from './login-success.utils.js';

/** Result type for journey client methods. */
export type JourneyResult = JourneyStep | JourneyLoginSuccess | JourneyLoginFailure | GenericError;

/** The journey client instance returned by the `journey()` function. */
export interface JourneyClient {
Expand Down Expand Up @@ -158,32 +155,22 @@ export async function journey<ActionType extends ActionTypes = ActionTypes>({
subscribe: store.subscribe,

start: async (options?: StartParam) => {
const { data } = await store.dispatch(journeyApi.endpoints.start.initiate(options));
if (!data) {
const error: GenericError = {
error: 'no_response_data',
message: 'No data received from server when starting journey',
type: 'unknown_error',
};
return error;
}
return createJourneyObject(data);
const response = await store.dispatch(journeyApi.endpoints.start.initiate(options));
return match(parseJourneyResponse(response), {
onLeft: (err): JourneyResult => err,
onRight: (step): JourneyResult => createJourneyObject(step),
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
},

/**
* Submits the current Step payload to the authentication API and retrieves the next JourneyStep in the journey.
*/
next: async (step: JourneyStep, options?: NextOptions) => {
const { data } = await store.dispatch(journeyApi.endpoints.next.initiate({ step, options }));
if (!data) {
const error: GenericError = {
error: 'no_response_data',
message: 'No data received from server when submitting step',
type: 'unknown_error',
};
return error;
}
return createJourneyObject(data);
const response = await store.dispatch(journeyApi.endpoints.next.initiate({ step, options }));
return match(parseJourneyResponse(response), {
onLeft: (err): JourneyResult => err,
onRight: (step): JourneyResult => createJourneyObject(step),
});
},

// TODO: Remove the actual redirect from this method and just return the URL to the caller
Expand Down
Loading
Loading