Skip to content

fix(oidc-client): forward loginHint, acrValues, query via buildAuthorizeOptions - #749

Draft
vatsalparikh wants to merge 1 commit into
mainfrom
fix/oidc-client-build-authorize-options-config
Draft

fix(oidc-client): forward loginHint, acrValues, query via buildAuthorizeOptions#749
vatsalparikh wants to merge 1 commit into
mainfrom
fix/oidc-client-build-authorize-options-config

Conversation

@vatsalparikh

Copy link
Copy Markdown
Contributor

Summary

  • buildAuthorizeOptions only forwarded clientId/redirectUri/scope/responseType from config, ignoring loginHint, nonce, display, prompt, uiLocales, acrValues, and query.
  • authorize.url() already read all seven fields from config correctly (client.store.ts lines 178–184), but authorizeµ — used by authorize.background() — went through buildAuthorizeOptions and dropped them.
  • The result: post-login silent token renewal via authorize.background() never included these values in the authorize URL, even when configured.

Fix

Add the same seven config-field spreads to buildAuthorizeOptions in authorize.request.utils.ts, matching the pattern already in authorize.url(). Caller-supplied options still override config-level values (existing spread order preserved).

Tests

Added four new unit tests to authorize.request.utils.test.ts:

  • Forwards loginHint, acrValues, query from config
  • Forwards nonce, display, prompt, uiLocales from config
  • Caller options override config-level values
  • Undefined config fields are omitted from the output

Test plan

  • nx test oidc-client passes (32 tests in authorize.request.utils, all green)
  • Verify authorize.background() now carries login_hint, acr_values, ui_locales in the authorize URL when configured

…dAuthorizeOptions

authorize.url() read these fields from config but buildAuthorizeOptions (used by
authorizeµ and therefore authorize.background) did not, so post-login silent
renewals dropped them silently.
@changeset-bot

changeset-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 0de9b1e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d0ce0f41-9fe1-4629-8fd6-bc5e1b2158ed

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nx-cloud

nx-cloud Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

View your CI Pipeline Execution ↗ for commit 0de9b1e

Command Status Duration Result
nx run-many -t build --no-agents ✅ Succeeded 1s View ↗
nx affected -t build lint test typecheck e2e-ci ✅ Succeeded 4m 12s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-08-07 23:08:49 UTC

@pkg-pr-new

pkg-pr-new Bot commented Aug 7, 2026

Copy link
Copy Markdown

Open in StackBlitz

@forgerock/davinci-client

pnpm add https://pkg.pr.new/@forgerock/davinci-client@749

@forgerock/device-client

pnpm add https://pkg.pr.new/@forgerock/device-client@749

@forgerock/journey-client

pnpm add https://pkg.pr.new/@forgerock/journey-client@749

@forgerock/oidc-client

pnpm add https://pkg.pr.new/@forgerock/oidc-client@749

@forgerock/protect

pnpm add https://pkg.pr.new/@forgerock/protect@749

@forgerock/sdk-types

pnpm add https://pkg.pr.new/@forgerock/sdk-types@749

@forgerock/sdk-utilities

pnpm add https://pkg.pr.new/@forgerock/sdk-utilities@749

@forgerock/iframe-manager

pnpm add https://pkg.pr.new/@forgerock/iframe-manager@749

@forgerock/sdk-logger

pnpm add https://pkg.pr.new/@forgerock/sdk-logger@749

@forgerock/sdk-oidc

pnpm add https://pkg.pr.new/@forgerock/sdk-oidc@749

@forgerock/sdk-request-middleware

pnpm add https://pkg.pr.new/@forgerock/sdk-request-middleware@749

@forgerock/storage

pnpm add https://pkg.pr.new/@forgerock/storage@749

commit: 0de9b1e

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.06%. Comparing base (eafe277) to head (0de9b1e).
⚠️ Report is 54 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #749       +/-   ##
===========================================
+ Coverage   18.07%   70.06%   +51.99%     
===========================================
  Files         155       22      -133     
  Lines       24398     1871    -22527     
  Branches     1203      316      -887     
===========================================
- Hits         4410     1311     -3099     
+ Misses      19988      560    -19428     
Files with missing lines Coverage Δ
...ges/oidc-client/src/lib/authorize.request.utils.ts 100.00% <100.00%> (+56.66%) ⬆️

... and 139 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Deployed 9219ee0 to https://ForgeRock.github.io/ping-javascript-sdk/pr-749/9219ee041184fcddc12b92c761cfcaa557c4e8c0 branch gh-pages in ForgeRock/ping-javascript-sdk

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

📦 Bundle Size Analysis

📦 Bundle Size Analysis

🆕 New Packages

🆕 @forgerock/davinci-client - 56.7 KB (new)
🆕 @forgerock/sdk-types - 9.1 KB (new)
🆕 @forgerock/sdk-logger - 1.6 KB (new)
🆕 @forgerock/iframe-manager - 3.2 KB (new)
🆕 @forgerock/sdk-oidc - 5.7 KB (new)
🆕 @forgerock/storage - 1.5 KB (new)
🆕 @forgerock/sdk-request-middleware - 4.6 KB (new)
🆕 @forgerock/journey-client - 92.6 KB (new)
🆕 @forgerock/journey-client - 0.0 KB (new)
🆕 @forgerock/sdk-utilities - 18.6 KB (new)
🆕 @forgerock/device-client - 10.0 KB (new)
🆕 @forgerock/device-client - 0.0 KB (new)
🆕 @forgerock/oidc-client - 35.4 KB (new)
🆕 @forgerock/protect - 144.6 KB (new)


14 packages analyzed • Baseline from latest main build

Legend

🆕 New package
🔺 Size increased
🔻 Size decreased
➖ No change

ℹ️ How bundle sizes are calculated
  • Current Size: Total gzipped size of all files in the package's dist directory
  • Baseline: Comparison against the latest build from the main branch
  • Files included: All build outputs except source maps and TypeScript build cache
  • Exclusions: .map, .tsbuildinfo, and .d.ts.map files

🔄 Updated automatically on each push to this PR

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.

2 participants