feat(oidc-client): implement tokens.get #369

cerebrl · 2025-08-11T21:55:08Z

JIRA Ticket

Description

This PR introduces local token get method with token auto-renew. This also introduces oauthThreshold that will renew a valid token if it's within X time until expiration. The default is 30 seconds. This also extracts the bulk of exchange out of the top-level client file into it's own file for better sharing.

nx-cloud · 2025-08-11T21:55:48Z

View your CI Pipeline Execution ↗ for commit 7cb0519

Command	Status	Duration	Result
`nx run-many -t build`	✅ Succeeded	<1s	View ↗
`nx affected -t build typecheck lint test e2e-ci`	✅ Succeeded	2m 11s	View ↗
`nx-cloud record -- nx format:check`	✅ Succeeded	1s	View ↗

☁️ Nx Cloud last updated this comment at 2025-08-14 15:41:17 UTC

github-actions · 2025-08-11T21:57:29Z

Deployed b3e84fb to https://ForgeRock.github.io/ping-javascript-sdk/pr-369/b3e84fb754a3b2acfaf5365c187464c1d6663b05 branch gh-pages in ForgeRock/ping-javascript-sdk

github-actions · 2025-08-11T21:57:33Z

📦 Bundle Size Analysis

🚨 Significant Changes

🔺 @forgerock/oidc-client - 21.1 KB (+3.1 KB, +17.4%)

📊 Minor Changes

📈 @forgerock/sdk-types - 5.9 KB (+0.0 KB)
📈 @forgerock/sdk-request-middleware - 4.4 KB (+0.2 KB)

➖ No Changes

➖ @forgerock/protect - 152.3 KB
➖ @forgerock/davinci-client - 34.1 KB
➖ @forgerock/sdk-utilities - 4.0 KB
➖ @forgerock/device-client - 9.2 KB
➖ @forgerock/iframe-manager - 2.4 KB
➖ @forgerock/storage - 1.4 KB
➖ @forgerock/sdk-logger - 1.6 KB
➖ @forgerock/sdk-oidc - 3.5 KB

11 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}

changeset-bot · 2025-08-11T23:54:07Z

🦋 Changeset detected

Latest commit: 7cb0519

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages

Name	Type
@forgerock/oidc-client	Minor
@forgerock/davinci-client	Minor
@forgerock/sdk-types	Minor
@forgerock/sdk-utilities	Minor
@forgerock/iframe-manager	Minor
@forgerock/sdk-logger	Minor
@forgerock/sdk-oidc	Minor
@forgerock/sdk-request-middleware	Minor
@forgerock/storage	Minor

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

ryanbas21 · 2025-08-12T15:56:08Z

packages/oidc-client/src/lib/client.store.ts

+                  store,
+                  options: storageOptions,
+                }).pipe(
+                  Micro.flatMap((tokens) =>


may be slightly more readable if we don't nest this pipe inside of the outter pipe, and just keep them on one level.

ryanbas21 · 2025-08-12T15:59:11Z

packages/oidc-client/src/lib/client.store.ts

+                  endpoint: wellknown.token_endpoint,
+                  store,
+                  options: storageOptions,
+                }).pipe(


similar comment here

I think I addressed this as it was the same as the other comment.

ryanbas21 · 2025-08-12T16:00:08Z

packages/oidc-client/src/lib/exchange.request.ts

+  store,
+  options,
+}: BuildTokenExchangeµParams) {
+  return Micro.sync(() => createValuesµ(code, config, state, endpoint, options)).pipe(


unneccessary Micro.sync I think?

I actually made a mistake in naming createValuesµ as it didn't return a Micro, but I fixed that and removed the Micro here.

ryanbas21 · 2025-08-12T16:00:33Z

packages/oidc-client/src/lib/exchange.request.ts

+    Micro.flatMap((options) => validateValuesµ(options)),
+    Micro.flatMap((requestOptions) =>
+      Micro.promise(
+        async () => await store.dispatch(oidcApi.endpoints.exchange.initiate(requestOptions)),


unneeccessary await and async

ryanbas21

Non blocking comments that are mostly semantic. Looks good

codecov-commenter · 2025-08-13T04:01:28Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.58%. Comparing base (e393843) to head (7cb0519).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #369   +/-   ##
=======================================
  Coverage   57.58%   57.58%           
=======================================
  Files          32       32           
  Lines        2044     2044           
  Branches      320      320           
=======================================
  Hits         1177     1177           
  Misses        867      867

Files with missing lines	Coverage Δ
packages/davinci-client/src/lib/davinci.api.ts	`0.41% <ø> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

packages/oidc-client/src/lib/authorize.request.types.ts

packages/oidc-client/src/lib/client.store.ts

ancheetah · 2025-08-13T15:54:29Z

packages/oidc-client/src/lib/exchange.request.ts

+    ),
+    Micro.flatMap((requestOptions) =>
+      Micro.promise(
+        async () => await store.dispatch(oidcApi.endpoints.exchange.initiate(requestOptions)),


This oidcApi endpoint returns a TokenExchangeResponse type but the type returned by buildTokenExchangeµ is OauthTokens which has properties that don't exist in TokenExchangeResponse. Did you means to remove TokenExchangeResponse and replace it with OauthTokens?

Good catch. Let me work on this.

packages/oidc-client/src/lib/client.store.ts

ryanbas21 · 2025-08-13T18:47:05Z

packages/oidc-client/src/lib/client.store.ts

+          log,
+          authorizeOptions,
+        ).pipe(
+          Micro.flatMap((response): Micro.Micro<OauthTokens, TokenExchangeErrorResponse, never> => {


Do we need the types here, I assume there was a reason we added them explicitly here instead of the inferred type

This is a good question, and I'll try my best to articulate it in as comment, but it may require a chat.

I like using inferred types as I've been developing, but as I was "tightening up" the code, and ensuring that what was being returned was exactly what I intended, inferred types was hindering me.

By explicitly declaring the types, I was declaring the contract, and if I strayed from it by changing some internals, I was immediately notified that I broke the contract. Does that make sense?

I'm totally fine with that. I just was double checking that it was with a purpose and not because we just didn't have something being inferred.

I actually think declaring types has benefits, especially at the edges of code that consumers touch.

ryanbas21 · 2025-08-13T18:49:24Z

packages/oidc-client/src/lib/exchange.request.ts

+      Micro.sync(() => log.error('Error handling token exchange response', error)),
+    ),
+    Micro.flatMap((data) =>
+      Micro.promise(async () => {


Micro.promise seems unneeded here, therefore making flatMap unneccessary. I think this can just be simplified to a map

Yup, good catch!

ryanbas21 · 2025-08-13T18:58:26Z

packages/oidc-client/src/lib/client.store.ts

+              store,
+              options: storageOptions,
+            }).pipe(
+              Micro.flatMap((tokens) =>


nitpick, but I think tap maybe more semantic. I think it should unwrap tokens still

This is a good callout. I've made the change in my local branch.

ancheetah · 2025-08-13T19:41:20Z

packages/oidc-client/src/lib/client.store.ts

        const state = store.getState();
        const wellknown = wellknownSelector(wellknownUrl, state);

        if (!wellknown?.end_session_endpoint) {


Unrelated to tokens.get but I didn't notice this before. Do we need to add a check for || !wellknown.ping_end_idp_session_endpoint? It seems the logout method calls one or the other depending on the server.

Hmm, this is actually a good question. This check is more like a quick check that we have a wellknown response, more or less. It's not really intended to be a complete error proof check. I'll have to think on it.

ancheetah · 2025-08-13T22:19:00Z

One more unrelated to tokens.get, sorry. In the logout method should we be passing some key to storageClient.remove()? If not, then I'm not sure what exactly is being removed.

ping-javascript-sdk/packages/oidc-client/src/lib/client.store.ts

Lines 446 to 449 in 764854f

    
           // Delete local token and return combined results 
        
           Micro.flatMap(([sessionResponse, revokeResponse]) => 
        
             Micro.promise(async () => { 
        
               const deleteResponse = await storageClient.remove();

cerebrl · 2025-08-14T02:14:09Z

@ancheetah

One more unrelated to tokens.get, sorry. In the logout method should we be passing some key to storageClient.remove()? If not, then I'm not sure what exactly is being removed.

ping-javascript-sdk/packages/oidc-client/src/lib/client.store.ts

Lines 446 to 449 in 764854f

// Delete local token and return combined results

Micro.flatMap(([sessionResponse, revokeResponse]) =>

Micro.promise(async () => {

const deleteResponse = await storageClient.remove();

No, the key is derived from the name saved within the closure of the storageClient from the initialization here: https://github.com/ForgeRock/ping-javascript-sdk/blob/main/packages/oidc-client/src/lib/client.store.ts#L59

cerebrl force-pushed the oidc-localget branch from 8442efd to a594bb5 Compare August 11, 2025 23:54

cerebrl force-pushed the oidc-localget branch from a594bb5 to e393db9 Compare August 12, 2025 00:14

ryanbas21 reviewed Aug 12, 2025

View reviewed changes

ryanbas21 approved these changes Aug 12, 2025

View reviewed changes

cerebrl force-pushed the oidc-localget branch from e393db9 to 764854f Compare August 13, 2025 03:58

ancheetah reviewed Aug 13, 2025

View reviewed changes

ryanbas21 reviewed Aug 13, 2025

View reviewed changes

ancheetah reviewed Aug 13, 2025

View reviewed changes

cerebrl force-pushed the oidc-localget branch from 764854f to 2e7e210 Compare August 13, 2025 23:00

cerebrl force-pushed the oidc-localget branch from 2e7e210 to 4c65b18 Compare August 14, 2025 02:22

feat(oidc-client): implement tokens.get

7cb0519

cerebrl force-pushed the oidc-localget branch from 4c65b18 to 7cb0519 Compare August 14, 2025 15:37

ancheetah approved these changes Aug 14, 2025

View reviewed changes

cerebrl merged commit 777e771 into main Aug 14, 2025
3 checks passed

cerebrl deleted the oidc-localget branch August 14, 2025 19:45

ryanbas21 mentioned this pull request Aug 12, 2025

Release PR #345

Open

feat(oidc-client): implement tokens.get #369

feat(oidc-client): implement tokens.get #369

Uh oh!

Conversation

cerebrl commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

JIRA Ticket

Description

Uh oh!

nx-cloud bot commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📦 Bundle Size Analysis

📦 Bundle Size Analysis

🚨 Significant Changes

Legend

Uh oh!

changeset-bot bot commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanbas21 left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ancheetah Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ancheetah commented Aug 13, 2025

Uh oh!

cerebrl commented Aug 14, 2025

Uh oh!

Uh oh!

Reviewers

cerebrl commented Aug 11, 2025 •

edited

Loading

nx-cloud bot commented Aug 11, 2025 •

edited

Loading

github-actions bot commented Aug 11, 2025 •

edited

Loading

github-actions bot commented Aug 11, 2025 •

edited

Loading

changeset-bot bot commented Aug 11, 2025 •

edited

Loading

codecov-commenter commented Aug 13, 2025 •

edited

Loading

ancheetah Aug 13, 2025 •

edited

Loading