Skip to content

refactor(oidc-and-protect): refactor login framework to use the new oidc and protect clients from Ping Orch SDKs - #552

Merged
vatsalparikh merged 10 commits into
mainfrom
SDKS-5216
Jul 27, 2026
Merged

refactor(oidc-and-protect): refactor login framework to use the new oidc and protect clients from Ping Orch SDKs#552
vatsalparikh merged 10 commits into
mainfrom
SDKS-5216

Conversation

@vatsalparikh

@vatsalparikh vatsalparikh commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

JIRA Ticket

https://pingidentity.atlassian.net/browse/SDKS-5216

Technical decisions

  • Using normal oidc client config instead of unified config because it’s easier, avoids importing sdk utils package
  • Defaults for OIDC
    • scope defaults to 'openid' in the Zod schema
    • redirectUri, clientId, serverConfig.wellknown are all required, no defaults provided
  • Cleaned up login app's register function. We no longer need oauth or user in login app, so that logic is completely removed from login app

Tests performed

  • Sample app login widget

    • - login flow, todo loading
    • - Verified local storage and cookies to check both journey client and oidc client flows
    • - Verified logout needs both, journey client terminate and oidc client user logout. Journey client terminate removes cookies, oidc user logout removes local storage data
    • - Protect tests pass
  • Login Framework

    • - Login app works as expected (however it only uses journey client so no impact whatsoever)
    • - e2e widget modal login and logout works as expected
    • - e2e widget inline login works, logout fails (it failed in main branch too, this is an existing issue, unrelated to journey client terminate. The AM session cookie isn't sent when terminate() makes a cross-origin request from https://localhost:8443 to openam-sdks.forgeblocks.com. The CORS config is fine and third-party cookies are allowed in the browser, but the AM session cookie itself has SameSite restrictions that prevent it being included in the cross-origin POST.)
    • - Added back (unskipped) protect tests, both initialization and evaluation tests pass

How to test

Pull https://github.com/ForgeRock/sdk-sample-apps/tree/migrate-login-widget-2.0 branch locally and test journey, oidc, and protect flows.
Here is the PR that accommodates all the updates necessary in the sample app ForgeRock/sdk-sample-apps#123

Recordings

The recordings below show how both journey client terminate and oidc user logout are necessary in user.logout function in login widget. They both perform two different operations. The recordings also show how tokens are correctly stored in cookie and local storage respectively, verifying that login, tokens, logout all work as expected.

  1. Removed OIDC user logout: https://github.com/user-attachments/assets/881192d3-4bba-41e1-9e79-da20c768908a
  2. Removed Journey Terminate: https://github.com/user-attachments/assets/6b60b73b-ec49-4e17-92aa-89e5515e2026
  3. Happy Path tokens and logout work as expected: https://github.com/user-attachments/assets/e4da1712-bc30-46be-bc04-2719dd812cfc

@changeset-bot

changeset-bot Bot commented Jul 17, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 2d6c90a

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

This PR includes changesets to release 1 package
Name Type
@forgerock/login-widget Major

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

@vatsalparikh
vatsalparikh requested review from SteinGabriel, ancheetah, cerebrl and ryanbas21 and removed request for ryanbas21 July 17, 2026 16:29
@vatsalparikh
vatsalparikh force-pushed the SDKS-5216 branch 3 times, most recently from 133fa33 to 8da22b6 Compare July 17, 2026 21:02

@cerebrl cerebrl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd like to stick with the pattern of using Svelte Stores for managing state and it's dependencies. Svelte refers to these as "custom stores" or "derived stores". You can see examples of them in this repo. Here's one that's fairly simple: https://github.com/ForgeRock/forgerock-web-login-framework/blob/main/packages/login-widget/src/lib/_utilities/component.utilities.ts.

Comment thread core/oauth/oauth.store.ts Outdated
Comment thread core/oauth/oauth.store.ts Outdated
Comment thread core/oauth/oauth.store.ts Outdated
Comment thread core/protect/protect.store.ts
Comment thread core/user/user.store.ts Outdated
Comment thread packages/login-widget/src/lib/widget.api.ts Outdated
Comment thread core/protect/protect.store.ts Outdated
Comment thread packages/login-widget/src/lib/widget.api.ts Outdated
Comment thread core/journey/callbacks/ping-protect-initialize/ping-protect-initialize.svelte Outdated
Comment thread core/journey/callbacks/ping-protect-evaluation/ping-protect-evaluation.svelte Outdated
Comment thread core/journey/callbacks/ping-protect-evaluation/ping-protect-evaluation.svelte Outdated
Comment thread core/journey/callbacks/ping-protect-initialize/ping-protect-initialize.svelte Outdated
@vatsalparikh
vatsalparikh force-pushed the SDKS-5216 branch 3 times, most recently from ae9d8b4 to 6f5e1a3 Compare July 21, 2026 16:40
@vatsalparikh
vatsalparikh marked this pull request as draft July 21, 2026 16:58
@vatsalparikh
vatsalparikh force-pushed the SDKS-5216 branch 10 times, most recently from 3ad1b32 to 06d5227 Compare July 22, 2026 01:43
@vatsalparikh

Copy link
Copy Markdown
Contributor Author

I'd like to stick with the pattern of using Svelte Stores for managing state and it's dependencies. Svelte refers to these as "custom stores" or "derived stores". You can see examples of them in this repo. Here's one that's fairly simple: main/packages/login-widget/src/lib/_utilities/component.utilities.ts.

Really appreciate your feedback here. Took me several iterations to fully understand and implement a declarative, derived, observer based svelte store. Thank you so much for taking the time to review it deeply and providing all the suggestions. It was a challenging and very rewarding experience!

I have created an oidc store for oidc client initialization. widget.api file composes it and passes it to both oauth and user stores. They consume the client. The code is ready for another review now. I believe some minor tweaks can be made but the code is structurally sound now in the sense that I've avoided promises, race conditions, and other issues with the help of declarative, observable pattern based code!

The PR is ready for review again, @cerebrl !

@vatsalparikh
vatsalparikh marked this pull request as ready for review July 22, 2026 14:58
@vatsalparikh
vatsalparikh force-pushed the SDKS-5216 branch 3 times, most recently from 60a7fe6 to 5d1904c Compare July 22, 2026 23:11
Comment thread packages/login-widget/src/lib/types.ts
Comment thread packages/login-widget/src/lib/widget.api.ts Outdated
Comment thread packages/login-widget/src/lib/widget.api.ts Outdated
Comment thread packages/login-widget/src/lib/widget.api.ts Outdated
Comment thread packages/login-widget/src/lib/widget.api.ts Outdated
Comment thread packages/login-widget/src/lib/widget.api.ts Outdated
Comment thread packages/login-widget/src/lib/widget.api.ts Outdated
Comment thread packages/login-widget/src/lib/widget.api.ts Outdated
@vatsalparikh

vatsalparikh commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Some comments on Protect. Is there existing e2e for Protect as well?

Yes, there are some e2e protect tests here: https://github.com/ForgeRock/forgerock-web-login-framework/blob/SDKS-5216/core/protect/protect.store.test.ts

@SteinGabriel SteinGabriel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Everything on my end has been addressed and the changes look good. There are only a few files with stale copyright dates, but other than that everything looks great.

Stale copyright dates:

  • core/journey/callbacks/ping-protect-initialize/ping-protect-initialize.svelte
  • e2e/tests/widget/modal/widget-modal.login.test.js
  • packages/login-widget/scripts/copyTypes.mjs

@vatsalparikh

Copy link
Copy Markdown
Contributor Author

Everything on my end has been addressed and the changes look good. There are only a few files with stale copyright dates, but other than that everything looks great.

Stale copyright dates:

  • core/journey/callbacks/ping-protect-initialize/ping-protect-initialize.svelte
  • e2e/tests/widget/modal/widget-modal.login.test.js
  • packages/login-widget/scripts/copyTypes.mjs

done, thanks!

@vatsalparikh
vatsalparikh merged commit 6758e54 into main Jul 27, 2026
21 checks passed
@vatsalparikh
vatsalparikh deleted the SDKS-5216 branch July 27, 2026 16:29
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.

5 participants