Skip to content

feat(javascript-sdk): ping one signout, flag for iframe skip, remove session call #535

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

Jump to bottom
Merged
merged 4 commits into from
Apr 16, 2025
Merged

feat(javascript-sdk): ping one signout, flag for iframe skip, remove session call #535

merged 4 commits into from
Apr 16, 2025

Conversation

@cerebrl
Copy link
Contributor

@cerebrl cerebrl commented Apr 10, 2025 •
edited
Loading

JIRA Ticket

SDKS-3757 & SDKS-3866

Description

This adds the new PingOne /idpSignoff endpount for ID Token only signoff without redirect.

@changeset-bot
Copy link

changeset-bot bot commented Apr 10, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: bac2177

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

This PR includes changesets to release 1 package
Name Type
@forgerock/javascript-sdk 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
Copy link

nx-cloud bot commented Apr 10, 2025 •
edited
Loading

View your CI Pipeline Execution ↗ for commit bac2177.

Command Status Duration Result
nx affected -t build lint test docs e2e-ci --ve... ✅ Succeeded 4m 54s View ↗
nx-cloud record -- nx format:check --verbose ✅ Succeeded 6s View ↗

☁️ Nx Cloud last updated this comment at 2025-04-16 20:47:23 UTC

@cerebrl cerebrl force-pushed the feat_add-new-signout-endpoint branch from 1b58dbe to c781e77 Compare April 10, 2025 02:53
@cerebrl cerebrl marked this pull request as ready for review April 10, 2025 02:55
@cerebrl cerebrl force-pushed the feat_add-new-signout-endpoint branch from fc690c1 to 86664b0 Compare April 15, 2025 05:16
@cerebrl cerebrl requested a review from ryanbas21 April 15, 2025 05:21
@github-actions github-actions bot removed the root label Apr 15, 2025
@codecov-commenter
Copy link

codecov-commenter commented Apr 15, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 67.30769% with 68 lines in your changes missing coverage. Please review.

Project coverage is 58.47%. Comparing base (f890af3) to head (bac2177).

Files with missing lines Patch % Lines
packages/javascript-sdk/src/token-manager/index.ts 54.94% 41 Missing ⚠️
packages/javascript-sdk/src/fr-user/index.ts 7.40% 25 Missing ⚠️
packages/javascript-sdk/src/oauth2-client/index.ts 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop     #535      +/-   ##
===========================================
+ Coverage    58.33%   58.47%   +0.14%     
===========================================
  Files          105      105              
  Lines        25174    25307     +133     
  Branches      1688     1690       +2     
===========================================
+ Hits         14685    14799     +114     
- Misses       10489    10508      +19

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ryanbas21
ryanbas21 reviewed Apr 15, 2025
View reviewed changes
e2e/autoscript-apps/src/authn-central-login-no-iframe/autoscript.ts
});
} else {
tokens = forgerock.TokenManager.getTokens({
skipBackgroundRequest: true,
Copy link
Contributor

@ryanbas21 ryanbas21 Apr 15, 2025

Choose a reason for hiding this comment

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

i'm concerned with this naming, i just fear another modern | legacy situation where people will read this name and say yeah we want to skip this.

Copy link
Contributor Author

@cerebrl cerebrl Apr 15, 2025

Choose a reason for hiding this comment

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

As mentioned on Slack. I think this is okay, and unintentially using this option won't do any harm. I've added more information in the JSDoc to cover this option's intention.

ryanbas21
ryanbas21 reviewed Apr 15, 2025
View reviewed changes
packages/javascript-sdk/src/token-manager/index.ts
interface GetTokensOptions extends ConfigOptions {
forceRenew?: boolean;
login?: 'embedded' | 'redirect' | undefined;
skipBackgroundRequest?: boolean;
Copy link
Contributor

@ryanbas21 ryanbas21 Apr 15, 2025

Choose a reason for hiding this comment

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

I think we should JS Doc comment this GetTokensOptions interface so that the IDE gets more intellisense on this type. Especially this new addition.

Copy link
Contributor Author

@cerebrl cerebrl Apr 15, 2025

Choose a reason for hiding this comment

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

Done

@cerebrl cerebrl force-pushed the feat_add-new-signout-endpoint branch from 18fc61b to 86a7ede Compare April 15, 2025 20:00
@github-actions github-actions bot added the root label Apr 15, 2025
@ryanbas21
Copy link
Contributor

ryanbas21 commented Apr 15, 2025

Reviewed / Tested.

@cerebrl cerebrl force-pushed the feat_add-new-signout-endpoint branch from 86a7ede to 2530651 Compare April 15, 2025 22:26
@github-actions github-actions bot removed the root label Apr 15, 2025
@cerebrl cerebrl merged commit 4557ace into develop Apr 16, 2025
4 checks passed
@github-actions github-actions bot mentioned this pull request Apr 16, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ryanbas21 ryanbas21 ryanbas21 approved these changes

Assignees

No one assigned

Labels

sdk test

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cerebrl @codecov-commenter @ryanbas21