feat(oidc-client): implement authorize API #344

cerebrl · 2025-07-09T00:44:02Z

Suggestions for using stronger design patterns for authorize. Treat this as non-working pseudocode.

changeset-bot · 2025-07-09T00:44:05Z

🦋 Changeset detected

Latest commit: 375d0ca

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

This PR includes changesets to release 9 packages

Name	Type
@forgerock/iframe-manager	Minor
@forgerock/sdk-oidc	Minor
@forgerock/oidc-client	Minor
@forgerock/davinci-client	Minor
@forgerock/sdk-types	Minor
@forgerock/sdk-utilities	Minor
@forgerock/sdk-logger	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

nx-cloud · 2025-07-09T00:44:53Z

View your CI Pipeline Execution ↗ for commit 375d0ca

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

☁️ Nx Cloud last updated this comment at 2025-07-24 20:52:48 UTC

ryanbas21 · 2025-07-16T13:02:28Z

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

+) {
+  const buildAuthorizeRequestµ = buildAuthorizeOptionsµ(wellknown, config, options).pipe(
+    Micro.flatMap(([url, config, options]) => createAuthorizeUrlµ(url, config, options)),
+    (effect) => {


Any specific reason we are using this arrow function style here?

ryanbas21

some initial looks over will look again

ryanbas21 · 2025-07-16T13:04:47Z

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

+  const buildAuthorizeRequestµ = buildAuthorizeOptionsµ(wellknown, config, options).pipe(
+    Micro.flatMap(([url, config, options]) => createAuthorizeUrlµ(url, config, options)),
+    (effect) => {
+      return Micro.matchEffect(effect, {


i'd be curious if just using tapError and tap would be better fits here

https://effect-ts.github.io/effect/effect/Micro.ts.html#taperror

https://effect-ts.github.io/effect/effect/Micro.ts.html#tap

tapError would only run on error channels, and tap on success channels

ryanbas21 · 2025-07-16T13:12:45Z

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

+            return Micro.gen(function* () {
+              if ('authorizeResponse' in response) {
+                log.debug('Received authorize response', response.authorizeResponse);
+                return yield* Micro.succeed(response.authorizeResponse as AuthorizeSuccessResponse);


I think the succeed here is redundant in a gen it should implicitly return a Micro here

ryanbas21 · 2025-07-16T13:13:57Z

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

+              }
+              log.error('Error in authorize response', response);
+              const errorResponse = response as { error: string; error_description: string };
+              return yield* createAuthorizeErrorµ(errorResponse, wellknown, config, options);


Would we want to Fail out of this effect here? This keeps the error in the success channel, is that what we want?

Micro/Effect should always keep Errors are values, so i think it would make sense to Fail out of it.

ryanbas21 · 2025-07-16T13:14:48Z

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

+         * redirect based server supporting iframes. An example would be PingAM.
+         */
+        return authorizeIframeµ(url, config).pipe(
+          Micro.flatMap((response) => {


I think given the below changes this could just be a map now?

ryanbas21 · 2025-07-16T13:22:36Z

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

+
+export function authorizeFetchµ(url: string) {
+  return Micro.tryPromise({
+    try: async () => {


Nothing inherently wrong with this, just going to say how I usually write this.

Because tryPromise takes a Promise, I find it easier to just pass in fetch with no awaiting. then break down the steps following it in their own sequences. This allows for more granular error handling in my opinion.

Micro.tryPromise({ try: (signal) => fetch('....', { signal }), catch: (e) => new MyFetchError() }).pipe( Micro.flatMap(res => Micro.tryPromise({ try: () => res.json(), catch: (err) => new ResJsonErr() })));

(not sure we can use Data if we use Micro, but it's just a helper around making errors, not required in the below example)
https://effect.website/play#5b10175aea48

ryanbas21 · 2025-07-16T13:23:03Z

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

+      }
+
+      return {
+        error: 'Authorization Notwork Failure',


Notwork spelling?

ryanbas21 · 2025-07-16T13:23:37Z

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

+
+export function authorizeIframeµ(url: string, config: OidcConfig) {
+  return Micro.tryPromise({
+    try: async () => {


np; unneccessary body?

ryanbas21 · 2025-07-16T13:28:09Z

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

+          return err;
+        }
+
+        const result = await authorizeµ(wellknown, config, log, options);


I'm curious if it may be more clear to runPromiseExit here where we explicitly want to run the effect.

this keeps the actual authorize effect self-contained and composable later on?

ryanbas21 · 2025-07-16T14:53:29Z

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

+export async function authorizeµ(
+  wellknown: WellKnownResponse,
+  config: OidcConfig,
+  log: CustomLogger,


this would be a good use case for a service (like we discussed) so we don't need to pass around the logger throughout the api

ryanbas21 · 2025-07-16T18:49:31Z

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

+         */
+        return authorizeFetchµ(url).pipe(
+          Micro.flatMap((response) => {
+            return Micro.gen(function* () {


I actually think this entire gen is unneccessary since you're just returning effects you can do that out of flatMap and no need for gen

ryanbas21 · 2025-07-16T18:55:44Z

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

+import type { OidcConfig } from './config.types.js';
+import { AuthorizeSuccessResponse } from './authorize.request.types.js';
+
+export async function authorizeµ(


export async function authorizeµ( wellknown: WellKnownResponse, config: OidcConfig, log: CustomLogger, options?: GetAuthorizationUrlOptions, ) { return buildAuthorizeOptionsµ(wellknown, config, options).pipe( Micro.flatMap(([url, config, options]) => createAuthorizeUrlµ(url, config, options)), Micro.tap((url) => log.debug('Created authorization URL', { url })), /** * This probably needs to change because `log` doesn't reutrn an effect / micro and we want to return one from here * really `log` needs to be wrapped in a service i guess */ Micro.tapError((url) => Micro.fail(log.error('Created authorization URL', { url }))), Micro.flatMap(([url, config, options]) => { if (options.responseMode === 'pi.flow') { /** * If we support the pi.flow field, this means we are using a PingOne server. * PingOne servers do not support redirection through iframes because they * set iframe's to DENY. */ return authorizeFetchµ(url).pipe( Micro.flatMap((response) => { if ('authorizeResponse' in response) { log.debug('Received authorize response', response.authorizeResponse); return Micro.succeed(response.authorizeResponse as AuthorizeSuccessResponse); } log.error('Error in authorize response', response); const errorResponse = response as { error: string; error_description: string }; return Micro.fail(createAuthorizeErrorµ(errorResponse, wellknown, config, options)); }), ); } else { /** * If the response mode is not pi.flow, then we are likely using a traditional * redirect based server supporting iframes. An example would be PingAM. */ return authorizeIframeµ(url, config).pipe( Micro.flatMap((response) => { if ('code' in response && 'state' in response) { log.debug('Received authorization code', response); return Micro.succeed(response as unknown as AuthorizeSuccessResponse); } log.error('Error in authorize response', response); const errorResponse = response as { error: string; error_description: string }; return Micro.fail(createAuthorizeErrorµ(errorResponse, wellknown, config, options)); }), ); } }), /** * This could be moved out of this authorize function */ Micro.runPromiseExit, ); }

did a rewrite, feel free to pick and choose but really the key parts are undid a few unneccessary layers of Micro and then explicitly Fail when we should return an error.

one smaller re-write

export function authorizeµ( wellknown: WellKnownResponse, config: OidcConfig, log: CustomLogger, options?: GetAuthorizationUrlOptions, ) { return buildAuthorizeOptionsµ(wellknown, config, options).pipe( Micro.flatMap(([url, config, options]) => createAuthorizeUrlµ(url, config, options)), Micro.tap((url) => log.debug('Created authorization URL', { url })), Micro.tapError((url) => Micro.sync(() => log.error('Created authorization URL', { url }))), Micro.flatMap(([url, config, options]) => { if (options.responseMode === 'pi.flow') { return authorizeFetchµ(url).pipe( Micro.map((response) => { if ('authorizeResponse' in response) { return response.authorizeResponse as AuthorizeSuccessResponse; } const errorResponse = response as { error: string; error_description: string }; return Micro.fail(createAuthorizeErrorµ(errorResponse, wellknown, config, options)); }), Micro.tap((response) => log.debug('Received authorize response', response)), Micro.tapError((response) => Micro.sync(() => log.error('Error in authorize response', response)), ), ); } else { return authorizeIframeµ(url, config).pipe( Micro.map((response) => { if ('code' in response && 'state' in response) { return response as unknown as AuthorizeSuccessResponse; } const errorResponse = response as { error: string; error_description: string }; return Micro.fail(createAuthorizeErrorµ(errorResponse, wellknown, config, options)); }), Micro.tap((response) => log.debug('Received authorize response', response)), Micro.tapError((response) => Micro.sync(() => log.error('Error in authorize response', response)), ), ); } }), ); }

ryanbas21 · 2025-07-16T22:09:18Z

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

+    }),
+  );
+
+  return Micro.runPromiseExit(buildAuthorizeRequestµ);


Thinking about this more, wouldn't this be against our pattern of running effects at the edge? i'd think we'd want this to be as close to the edge of the program as possible.

I'd advocate this happen at the top level if possible

Yeah, I agree with all of your comments. This one especially. I was actually going to move this to the top file, but didn't get around to it.

ancheetah · 2025-07-18T13:41:08Z

packages/oidc-client/README.md

+const authResponse = oidcClient.authorize.background(); // Returns code and state if successful, error and Auth URL if not
+const authUrl = oidcClient.authorize.url(); // Returns Auth URL or error
+
+// Tokens API


Do the tokens api and user api exist on the OIDC client yet? Didn't see them on the client store

Yeah, this is just the authorize feature. Token exchange is found here: #348. The other token related methods are not implemeneted yet.

ancheetah · 2025-07-18T14:29:29Z

packages/sdk-effects/iframe-manager/src/lib/iframe-manager.effects.ts

              if (hasErrorParams(searchParams, errorParams)) {
                cleanup();
-                reject(parsedParams); // Reject with all parsed params for context
+                resolve(parsedParams); // Reject with all parsed params for context


Why not reject and catch it with a Micro.fail in authorizeIframeµ?

I think this is reasonable now that the code is more mature. I'll reevaluate this today.

ancheetah · 2025-07-18T14:30:06Z

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

@@ -0,0 +1,81 @@
+import { CustomLogger } from '@forgerock/sdk-logger';


Missing copyright in some files

introduce the oidc-client with the authorize api. authoirize handles the calling of authorize routes based on the well-known config. when a p1 env is detected, it will use a post request otherwise, a background request is done via a hidden iframe

codecov-commenter · 2025-07-24T20:52:51Z

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 60.44%. Comparing base (a369259) to head (375d0ca).
Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/davinci-client/src/lib/davinci.api.ts	0.00%	1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (40.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #344      +/-   ##
==========================================
- Coverage   60.46%   60.44%   -0.03%     
==========================================
  Files          33       33              
  Lines        2044     2045       +1     
  Branches      291      292       +1     
==========================================
  Hits         1236     1236              
- Misses        808      809       +1

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

🚀 New features to boost your workflow:

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

github-actions · 2025-07-24T20:53:07Z

Deployed 08b4667 to https://ForgeRock.github.io/ping-javascript-sdk/pr-344/08b4667dfd2f5c4e2f052e2661b8125bc3a91e94 branch gh-pages in ForgeRock/ping-javascript-sdk

github-actions · 2025-07-24T20:53:11Z

📦 Bundle Size Analysis

🚨 Significant Changes

🔺 @forgerock/oidc-client - 7.3 KB (+6.1 KB, +513.4%)

📊 Minor Changes

📈 @forgerock/davinci-client - 34.3 KB (+0.0 KB)
📈 @forgerock/sdk-oidc - 3.5 KB (+0.1 KB)
📉 @forgerock/iframe-manager - 2.4 KB (-0.0 KB)

➖ No Changes

➖ @pingidentity/protect - 108.4 KB
➖ @forgerock/device-client - 9.2 KB
➖ @forgerock/sdk-types - 5.6 KB
➖ @forgerock/sdk-utilities - 4.0 KB
➖ @forgerock/sdk-request-middleware - 4.2 KB
➖ @forgerock/storage - 1.3 KB
➖ @forgerock/sdk-logger - 1.6 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}

cerebrl force-pushed the justin_oidc-mods branch 3 times, most recently from abc572c to 0b1e1dd Compare July 14, 2025 18:37

cerebrl changed the base branch from oidc-client-authorize to main July 16, 2025 06:42

cerebrl changed the title ~~feat(oidc-client): implement stronger patterns~~ feat(oidc-client): implement authorize API Jul 16, 2025

ryanbas21 reviewed Jul 16, 2025

View reviewed changes

ancheetah reviewed Jul 18, 2025

View reviewed changes

cerebrl force-pushed the justin_oidc-mods branch 3 times, most recently from 249d35d to e615b24 Compare July 23, 2025 22:59

ryanbas21 and others added 4 commits July 24, 2025 15:49

feat: implement-authorize-oidc-client

dc4d4bd

introduce the oidc-client with the authorize api. authoirize handles the calling of authorize routes based on the well-known config. when a p1 env is detected, it will use a post request otherwise, a background request is done via a hidden iframe

feat(oidc-client): implement stronger patterns

297bf2a

feat(oidc-client): introduce Effect in authorize

42b48fb

feat(oidc-client): improve Effect usage

375d0ca

cerebrl force-pushed the justin_oidc-mods branch from e615b24 to 375d0ca Compare July 24, 2025 20:50

ancheetah approved these changes Jul 24, 2025

View reviewed changes

cerebrl merged commit 26c6983 into main Jul 24, 2025
4 checks passed

cerebrl deleted the justin_oidc-mods branch July 24, 2025 21:00

ryanbas21 mentioned this pull request Jul 24, 2025

Release PR #345

Open

		@@ -0,0 +1,81 @@
		import { CustomLogger } from '@forgerock/sdk-logger';

feat(oidc-client): implement authorize API #344

feat(oidc-client): implement authorize API #344

Uh oh!

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

🦋 Changeset detected

Uh oh!

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

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanbas21 left a comment

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!

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 Jul 16, 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!

ryanbas21 Jul 16, 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!

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!

codecov-commenter commented Jul 24, 2025

Codecov Report

Uh oh!

github-actions bot commented Jul 24, 2025

Uh oh!

github-actions bot commented Jul 24, 2025

📦 Bundle Size Analysis

📦 Bundle Size Analysis

🚨 Significant Changes

Legend

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

cerebrl commented Jul 9, 2025 •

edited

Loading

changeset-bot bot commented Jul 9, 2025 •

edited

Loading

nx-cloud bot commented Jul 9, 2025 •

edited

Loading

ryanbas21 Jul 16, 2025 •

edited

Loading

ryanbas21 Jul 16, 2025 •

edited

Loading