feat(journey-client): wellknown-endpoint-config-support #525
Conversation
🦋 Changeset detectedLatest commit: 03b785b The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 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 |
📝 WalkthroughWalkthroughAdds OIDC well-known endpoint discovery: new types, utilities, RTK Query API, error helpers, tests, and wiring into journey-client, oidc effects, and shared utilities; journey initialization can now resolve async well-known configuration or use standard normalization. Changes
Sequence DiagramsequenceDiagram
participant App as App
participant JourneyFactory as Journey Factory
participant ConfigResolver as Config Resolver
participant WellknownAPI as Wellknown API
participant Redux as Redux Store
participant JourneyStore as Journey Store
rect rgba(100,150,200,0.5)
Note over App,JourneyStore: Well-known Configuration Path
App->>JourneyFactory: journey(JourneyConfigInput with wellknown URL)
JourneyFactory->>ConfigResolver: hasWellknownConfig(config)?
ConfigResolver-->>JourneyFactory: true
JourneyFactory->>ConfigResolver: resolveAsyncConfig(config)
ConfigResolver->>WellknownAPI: Fetch well-known configuration
WellknownAPI->>Redux: Cache result
WellknownAPI-->>ConfigResolver: WellKnownResponse
ConfigResolver->>ConfigResolver: inferRealmFromIssuer(issuer)
ConfigResolver->>ConfigResolver: Merge configurations
ConfigResolver-->>JourneyFactory: InternalJourneyClientConfig
end
rect rgba(150,100,200,0.5)
Note over App,JourneyStore: Standard Configuration Path
App->>JourneyFactory: journey(JourneyConfigInput)
JourneyFactory->>ConfigResolver: hasWellknownConfig(config)?
ConfigResolver-->>JourneyFactory: false
JourneyFactory->>ConfigResolver: normalizeConfig(config)
ConfigResolver-->>JourneyFactory: InternalJourneyClientConfig
end
JourneyFactory->>JourneyStore: createJourneyStore(resolvedConfig)
JourneyStore->>Redux: Register API, reducer, middleware
Redux-->>JourneyStore: Store created
JourneyStore-->>App: Store ready
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 03b785b
☁️ Nx Cloud last updated this comment at |
@forgerock/davinci-client
@forgerock/device-client
@forgerock/journey-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: |
Codecov Report❌ Patch coverage is ❌ Your project status has failed because the head coverage (19.36%) 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 #525 +/- ##
==========================================
+ Coverage 18.79% 19.36% +0.57%
==========================================
Files 140 148 +8
Lines 27640 28005 +365
Branches 980 1028 +48
==========================================
+ Hits 5195 5424 +229
- Misses 22445 22581 +136
🚀 New features to boost your workflow:
|
|
Deployed 80cb7fe to https://ForgeRock.github.io/ping-javascript-sdk/pr-525/80cb7fea481099a2ae8aafb104b509b9398b29a8 branch gh-pages in ForgeRock/ping-javascript-sdk |
📦 Bundle Size Analysis📦 Bundle Size Analysis🚨 Significant Changes🔺 @forgerock/sdk-utilities - 10.7 KB (+2.2 KB, +25.6%) 📊 Minor Changes📈 @forgerock/oidc-client - 23.8 KB (+0.3 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/journey-client/src/lib/client.store.ts (1)
68-179: Apply request middleware to well-known discovery.
The discovery call is made via a temp store that doesn’t includerequestMiddleware, so any custom headers/auth/fetch logic won’t apply to the well-known request. That can break environments that rely on middleware.🔧 Proposed fix
async function resolveAsyncConfig( config: JourneyConfigInput & { serverConfig: { wellknown: string } }, log: ReturnType<typeof loggerFn>, + requestMiddleware?: RequestMiddleware[], ): Promise<InternalJourneyClientConfig> { @@ - const tempStore = createJourneyStore({ config: tempConfig, logger: log }); + const tempStore = createJourneyStore({ config: tempConfig, logger: log, requestMiddleware }); @@ - resolvedConfig = await resolveAsyncConfig(config, log); + resolvedConfig = await resolveAsyncConfig(config, log, requestMiddleware);
🤖 Fix all issues with AI agents
In `@packages/sdk-effects/oidc/package.json`:
- Line 31: Replace the explicit version for the dependency key
"@reduxjs/toolkit" in package.json (currently "^2.8.0") with the monorepo
catalog spec so it follows the workspace pattern — set the version to
"catalog:^2.8.2" (i.e., change the value for "@reduxjs/toolkit" to
"catalog:^2.8.2") so the package uses the single source of truth defined in
pnpm-workspace.yaml.
🧹 Nitpick comments (4)
packages/sdk-utilities/src/lib/wellknown/wellknown.utils.ts (1)
85-98: Consider adding IPv6 localhost support.The function allows HTTP for
localhostand127.0.0.1, but not for::1(IPv6 localhost) or[::1]. This is a minor edge case but could cause unexpected behavior in IPv6-only development environments.♻️ Optional: Add IPv6 localhost support
// Allow HTTP only for localhost (development) - const isLocalhost = url.hostname === 'localhost' || url.hostname === '127.0.0.1'; + const isLocalhost = + url.hostname === 'localhost' || + url.hostname === '127.0.0.1' || + url.hostname === '[::1]' || + url.hostname === '::1'; const isSecure = url.protocol === 'https:'; const isHttpLocalhost = url.protocol === 'http:' && isLocalhost;packages/oidc-client/src/lib/wellknown.api.ts (2)
33-38: Selector created on every call defeats memoization.
createSelectoris invoked inside the function body, creating a new memoized selector on each call towellknownSelector. This negates the memoization benefits since each call produces a fresh selector instance.Consider creating the selector once per
wellknownUrlor usingcreateWellknownSelectorfrom the re-exported utilities if it handles this pattern.♻️ Option: Cache selectors by URL
+const selectorCache = new Map<string, ReturnType<typeof createSelector>>(); + export function wellknownSelector(wellknownUrl: string, state: RootState) { - const selector = createSelector( - wellknownApi.endpoints.configuration.select(wellknownUrl), - (result) => result?.data, - ); + let selector = selectorCache.get(wellknownUrl); + if (!selector) { + selector = createSelector( + wellknownApi.endpoints.configuration.select(wellknownUrl), + (result) => result?.data, + ); + selectorCache.set(wellknownUrl, selector); + } return selector(state); }Alternatively, if
createWellknownSelector(already exported) provides this functionality, consider using it directly instead of this wrapper.
18-21: Consider combining export and import statements.The separate re-export (line 18) and import (line 21) from the same package can be consolidated.
♻️ Suggested consolidation
-export { wellknownApi, createWellknownSelector } from '@forgerock/sdk-oidc'; - -// Import locally for use in selector below -import { wellknownApi } from '@forgerock/sdk-oidc'; +import { wellknownApi, createWellknownSelector } from '@forgerock/sdk-oidc'; + +export { wellknownApi, createWellknownSelector };packages/journey-client/src/lib/wellknown.utils.test.ts (1)
64-69: Consider adding a comment explaining intentional type casts.The type assertions (
as AsyncJourneyClientConfig,as JourneyClientConfig) are used to bypass TypeScript's checks and test edge cases with invalid inputs. A brief comment would clarify this intent for future maintainers.const config: JourneyConfigInput = { serverConfig: { baseUrl: 'https://am.example.com/am/', wellknown: '', }, - } as AsyncJourneyClientConfig; + } as AsyncJourneyClientConfig; // Intentionally cast to test runtime behavior with empty wellknown
| "@forgerock/sdk-types": "workspace:*", | ||
| "@forgerock/sdk-utilities": "workspace:*" | ||
| "@forgerock/sdk-utilities": "workspace:*", | ||
| "@reduxjs/toolkit": "^2.8.0" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# Search for journey-client package.json to verify the RTK dependency pattern
find . -type f -name "package.json" -path "*/journey-client/package.json" | head -5Repository: ForgeRock/ping-javascript-sdk
Length of output: 111
🏁 Script executed:
# Also search more broadly for journey-client
find . -type d -name "journey-client" | head -5Repository: ForgeRock/ping-javascript-sdk
Length of output: 98
🏁 Script executed:
# Look for pnpm-workspace or catalog configuration files
fd -e "yaml" -e "yml" -e "json" | grep -E "(workspace|catalog|pnpm)" | head -20Repository: ForgeRock/ping-javascript-sdk
Length of output: 107
🏁 Script executed:
# Search for `@reduxjs/toolkit` references in package.json files to understand the pattern used
rg -t json "@reduxjs/toolkit" --max-count 10Repository: ForgeRock/ping-javascript-sdk
Length of output: 523
🏁 Script executed:
# Check pnpm-workspace.yaml to see the catalog setup
cat pnpm-workspace.yamlRepository: ForgeRock/ping-javascript-sdk
Length of output: 685
🌐 Web query:
@reduxjs/toolkit latest stable version npm registry
💡 Result:
Latest stable version of @reduxjs/toolkit on the npm registry: 2.11.2. [1]
Use catalog: for consistency with the monorepo pattern.
All other packages in the monorepo reference @reduxjs/toolkit via the catalog pattern (defined in pnpm-workspace.yaml as ^2.8.2), but this package uses an explicit version ^2.8.0. This breaks the single source of truth for dependency management and keeps the version outdated. Align with the established monorepo pattern:
- "@reduxjs/toolkit": "^2.8.0"
+ "@reduxjs/toolkit": "catalog:"📝 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.
| "@reduxjs/toolkit": "^2.8.0" | |
| "@reduxjs/toolkit": "catalog:" |
🤖 Prompt for AI Agents
In `@packages/sdk-effects/oidc/package.json` at line 31, Replace the explicit
version for the dependency key "@reduxjs/toolkit" in package.json (currently
"^2.8.0") with the monorepo catalog spec so it follows the workspace pattern —
set the version to "catalog:^2.8.2" (i.e., change the value for
"@reduxjs/toolkit" to "catalog:^2.8.2") so the package uses the single source of
truth defined in pnpm-workspace.yaml.
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud is proposing a fix for your failed CI:
These changes fix the e2e test failure by replacing RTK Query with native fetch API in the wellknown configuration resolution. The original implementation created a temporary Redux store to fetch wellknown config, but RTK Query APIs are singletons with global middleware that caused conflicts when multiple journey client instances were created (e.g., after logout). By using fetch directly, we eliminate the store singleton issues while maintaining the wellknown endpoint discovery functionality.
Tip
✅ We verified this fix by re-running @forgerock/journey-suites:e2e-ci--src/protect.test.ts.
Suggested Fix changes
diff --git a/packages/journey-client/src/lib/client.store.ts b/packages/journey-client/src/lib/client.store.ts
index 35c130bd..d7f3ac61 100644
--- a/packages/journey-client/src/lib/client.store.ts
+++ b/packages/journey-client/src/lib/client.store.ts
@@ -17,7 +17,6 @@ import { journeyApi } from './journey.api.js';
import { setConfig } from './journey.slice.js';
import { createStorage } from '@forgerock/storage';
import { createJourneyObject } from './journey.utils.js';
-import { wellknownApi } from './wellknown.api.js';
import {
hasWellknownConfig,
inferRealmFromIssuer,
@@ -80,19 +79,22 @@ async function resolveAsyncConfig(
throw error;
}
- // Create a temporary store to fetch well-known (we need the RTK Query infrastructure)
- const tempConfig: InternalJourneyClientConfig = {
- serverConfig: { baseUrl: baseUrl || '', paths, timeout },
- realmPath: config.realmPath,
- };
- const tempStore = createJourneyStore({ config: tempConfig, logger: log });
-
- // Fetch the well-known configuration
- const { data: wellknownResponse, error: fetchError } = await tempStore.dispatch(
- wellknownApi.endpoints.configuration.initiate(wellknown),
- );
+ // Fetch the well-known configuration directly using fetch API
+ // We avoid using RTK Query here to prevent store singleton issues
+ let wellknownResponse: any;
+ try {
+ const response = await fetch(wellknown, {
+ headers: {
+ Accept: 'application/json',
+ },
+ });
+
+ if (!response.ok) {
+ throw new Error(`HTTP ${response.status}: ${response.statusText}`);
+ }
- if (fetchError || !wellknownResponse) {
+ wellknownResponse = await response.json();
+ } catch (fetchError: any) {
const genericError = createWellknownError(fetchError);
log.error(`${genericError.error}: ${genericError.message}`);
throw new Error(genericError.message);
Or Apply changes locally with:
npx nx-cloud apply-locally o3Fs-SUDD
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
JIRA Ticket
https://pingidentity.atlassian.net/browse/SDKS-4665
Description
Add support for well known endpoint
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.