Skip to content

OIDC Client and Journey Client Bug Fixes #511

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ancheetah merged 3 commits into from
Jan 22, 2026
Merged

OIDC Client and Journey Client Bug Fixes #511

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
20a8cdf
fix(journey-client): fix callbackType export
ancheetah Jan 5, 2026
0bc297f
fix(oidc-client): fix token force renew
ancheetah Jan 6, 2026
036d226
chore: comments
ancheetah Jan 17, 2026
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
3 changes: 3 additions & 0 deletions packages/journey-client/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@
*/

export * from './lib/client.store.js';

// Re-export types from internal packages that consumers need
export { callbackType } from '@forgerock/sdk-types';
Copy link
Copy Markdown
Collaborator

@ryanbas21 ryanbas21 Jan 6, 2026

Choose a reason for hiding this comment

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

good catch

1 change: 0 additions & 1 deletion packages/journey-client/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ export type {
Callback,
CallbackType,
StepType,
callbackType,
GenericError,
PolicyRequirement,
FailedPolicyRequirement,
Expand Down
22 changes: 12 additions & 10 deletions packages/oidc-client/src/lib/client.store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,16 +289,18 @@ export async function oidc<ActionType extends ActionTypes = ActionTypes>({
options: storageOptions,
});
}),
Micro.tap(async (tokens) => {
await store.dispatch(
oidcApi.endpoints.revoke.initiate({
accessToken: tokens.accessToken,
clientId: config.clientId,
endpoint: wellknown.revocation_endpoint,
}),
);
await storageClient.remove();
await storageClient.set(tokens);
Micro.tap(async (newTokens) => {
if (tokens && 'accessToken' in tokens) {
await store.dispatch(
oidcApi.endpoints.revoke.initiate({
accessToken: tokens.accessToken,
clientId: config.clientId,
endpoint: wellknown.revocation_endpoint,
}),
);
await storageClient.remove();
}
await storageClient.set(newTokens);
}),
Comment on lines +292 to 304
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jan 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

These changes appear unrelated to the PR objectives.

The PR title indicates this is about fixing callbackType export in journey-client, but this file contains token renewal flow changes in the oidc-client package. Consider whether these changes should be in a separate PR for better traceability and review focus.

🤖 Prompt for AI Agents 
In @packages/oidc-client/src/lib/client.store.ts around lines 292 - 304, The
changes to the token renewal flow (Micro.tap callback updating tokens) are
unrelated to the callbackType export fix; revert or remove the added logic
around Micro.tap, the calls to
store.dispatch(oidcApi.endpoints.revoke.initiate(...)), storageClient.remove(),
and storageClient.set(newTokens) from this PR and instead place them in a
separate focused PR or commit that clearly documents the token renewal behavior;
ensure the current PR only contains the callbackType export fix and no
modifications to the Micro.tap/token revocation flow or storageClient usage.

⚠️ Potential issue | 🔴 Critical

Consider error handling and atomic storage updates.

The token renewal logic has several concerns:

  1. Critical: Revocation failure blocks new token storage — If the old token revocation fails (e.g., token already expired/invalid), the tap will throw and prevent storing newTokens, leaving the user in a broken state. Consider using Micro.tapError or try-catch to ensure new tokens are stored even if revocation fails.

  2. Missing revocation endpoint check — Unlike the token.revoke method (line 331), this code doesn't verify wellknown.revocation_endpoint exists before using it (line 298). This could cause runtime errors if the endpoint is unavailable.

  3. Atomic gap in storage updates — Calling remove() followed by set() creates a brief window where no tokens exist in storage. Concurrent reads during this gap will fail. Consider directly calling storageClient.set(newTokens) without the remove, since set should overwrite existing values.

🔎 Proposed fix with better error handling 
         Micro.tap(async (newTokens) => {
           if (tokens && 'accessToken' in tokens) {
-            await store.dispatch(
-              oidcApi.endpoints.revoke.initiate({
-                accessToken: tokens.accessToken,
-                clientId: config.clientId,
-                endpoint: wellknown.revocation_endpoint,
-              }),
-            );
-            await storageClient.remove();
+            if (wellknown.revocation_endpoint) {
+              try {
+                await store.dispatch(
+                  oidcApi.endpoints.revoke.initiate({
+                    accessToken: tokens.accessToken,
+                    clientId: config.clientId,
+                    endpoint: wellknown.revocation_endpoint,
+                  }),
+                );
+              } catch (error) {
+                log.warn('Failed to revoke old access token during renewal', error);
+              }
+            }
           }
           await storageClient.set(newTokens);
         }),
🤖 Prompt for AI Agents 
In @packages/oidc-client/src/lib/client.store.ts around lines 292 - 304, The
Micro.tap handler currently calls
store.dispatch(oidcApi.endpoints.revoke.initiate(...)) without checking
wellknown.revocation_endpoint and then calls storageClient.remove() before
storageClient.set(newTokens), which can throw and create a storage gap; update
the Micro.tap callback to first check that wellknown.revocation_endpoint is
present before attempting revocation, wrap the revoke call in a try-catch (or
use Micro.tapError) so any revocation failure is logged/ignored and does not
prevent saving newTokens, and avoid calling storageClient.remove(); instead call
storageClient.set(newTokens) directly to atomically overwrite existing tokens
(keeping store.dispatch and oidcApi.endpoints.revoke.initiate as the revocation
call site).

);

Expand Down
Loading