Skip to content
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

feat: add clearTokensAfterRedirect option to signOut - OKTA-443730 #1010

Closed
wants to merge 19 commits into from
Closed

feat: add clearTokensAfterRedirect option to signOut - OKTA-443730 #1010

wants to merge 19 commits into from

Conversation

shuowu
Copy link
Contributor

@shuowu shuowu commented Nov 18, 2021 •
edited by shuowu-okta

In this PR:

  • adds clearTokensAfterRedirect option to the signOut method
  • adds e2e to simulate cross tabs signOut with secure route (auto trigger signInWithRedirect when authState is false)
  • adds unit tests

@shuowu shuowu marked this pull request as draft November 18, 2021 19:46
jaredperreault-okta
jaredperreault-okta reviewed Nov 18, 2021
View reviewed changes
lib/OktaAuth.ts Outdated
@@ -445,6 +461,28 @@ class OktaAuth implements SDKInterface, SigninAPI, SignoutAPI {
return logoutUri;
}

async signOutSSO(options: SignoutOptions) {
var postLogoutRedirectUri = options.postLogoutRedirectUri
Copy link
Contributor

Choose a reason for hiding this comment

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

should we verify there is an active session (a valid login) before attempting to signOut?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's handled by the /logout endpoint. Also, I would prefer to leave this check to the devs side, since it most probably will need to trigger a network request.

If no Okta session exists, this endpoint has no effect and the browser is redirected immediately to the Okta sign-in page or the post_logout_redirect_uri (if specified).

https://developer.okta.com/docs/reference/api/oidc/#logout

@codecov-commenter
Copy link

codecov-commenter commented Nov 18, 2021 •
edited

Codecov Report

Merging #1010 (4c298df) into master (6e0a543) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1010      +/-   ##
==========================================
+ Coverage   92.48%   92.51%   +0.03%     
==========================================
  Files         134      134              
  Lines        3697     3712      +15     
  Branches      772      775       +3     
==========================================
+ Hits         3419     3434      +15     
  Misses        278      278
Impacted Files Coverage Δ
lib/types/Token.ts 58.33% <ø> (ø)
lib/OktaAuth.ts 88.02% <100.00%> (+0.12%) ⬆️
lib/TokenManager.ts 94.94% <100.00%> (+0.24%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e0a543...4c298df. Read the comment docs.

@shuowu-okta shuowu-okta force-pushed the sw-add-granular-signOutSSO-method-OKTA-443730 branch from cbca6a8 to b694c15 Compare November 30, 2021 20:03
@shuowu-okta shuowu-okta changed the title feat: add granular signOutSSO method - OKTA-443730 feat: add clearTokensAfterRedirect option to signOut - OKTA-443730 Nov 30, 2021
@shuowu-okta shuowu-okta marked this pull request as ready for review December 1, 2021 16:03
@shuowu-okta shuowu-okta force-pushed the sw-add-granular-signOutSSO-method-OKTA-443730 branch from 85d8b86 to ed6e3ab Compare December 16, 2021 17:27
eng-prod-CI-bot-okta pushed a commit that referenced this pull request Dec 20, 2021
@shuowu @eng-prod-CI-bot-okta
feat: support clearPendingRemove option in signOut method
6a5dc3d 
OKTA-443730
<<<Jenkins Check-In of Tested SHA: b780721 for eng_productivity_ci_bot_okta@okta.com>>>
Artifact: okta-auth-js
Files changed count: 18
PR Link: "#1010"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaredperreault-okta jaredperreault-okta jaredperreault-okta left review comments

@aarongranick-okta aarongranick-okta aarongranick-okta approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@shuowu @codecov-commenter @aarongranick-okta @jaredperreault-okta @eng-prod-CI-bot-okta