Skip to content

logout callback support#1119

Merged
pamapa merged 3 commits into
mainfrom
feat-logout-callback
Feb 9, 2024
Merged

logout callback support#1119
pamapa merged 3 commits into
mainfrom
feat-logout-callback

Conversation

@pamapa

@pamapa pamapa commented Feb 5, 2024

Copy link
Copy Markdown
Member

Closes/fixes #issue

Checklist

  • This PR makes changes to the public API
  • I have included links for closing relevant issue numbers

@pamapa pamapa added the enhancement New feature or request label Feb 5, 2024
@pamapa

pamapa commented Feb 5, 2024

Copy link
Copy Markdown
Member Author

Required, see authts/oidc-client-ts#1379

@pamapa pamapa force-pushed the feat-logout-callback branch from d77d46e to c712a2c Compare February 5, 2024 15:41
@codecov

codecov Bot commented Feb 5, 2024

Copy link
Copy Markdown

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (4225497) 83.44% compared to head (210f1c0) 83.44%.
Report is 12 commits behind head on main.

Files Patch % Lines
src/AuthProvider.tsx 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1119      +/-   ##
==========================================
- Coverage   83.44%   83.44%   -0.01%     
==========================================
  Files          10       10              
  Lines         145      151       +6     
  Branches       25       27       +2     
==========================================
+ Hits          121      126       +5     
- Misses         17       18       +1     
  Partials        7        7              
Flag Coverage Δ
unittests 83.44% <85.71%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@pamapa pamapa force-pushed the feat-logout-callback branch 2 times, most recently from a5fa857 to 6fc9081 Compare February 6, 2024 09:35
@PSanetra

PSanetra commented Feb 6, 2024

Copy link
Copy Markdown
Contributor

@pamapa As far as I see, the AuthProvider already supports the post_logout_redirect_uri and popup_post_logout_redirect_uri. Does it make sense to automatically check what is supposed to be checked by isSignoutCallback manually? I think it would be nice to avoid the need for such a manual check.

@pamapa pamapa force-pushed the feat-logout-callback branch 2 times, most recently from 3854d66 to 590433c Compare February 6, 2024 12:24
@pamapa

pamapa commented Feb 6, 2024

Copy link
Copy Markdown
Member Author

@PSanetra I adapted according to your idea the MR. Its still possible to change the match function, as i do not know all kind of possibilities about routing and no window, etc... But there is a default implementation, thus most users do not need to care....

Comment thread src/AuthProvider.tsx Outdated
@pamapa pamapa force-pushed the feat-logout-callback branch 5 times, most recently from 50dd0a5 to 39823e3 Compare February 7, 2024 11:29
@pamapa

pamapa commented Feb 8, 2024

Copy link
Copy Markdown
Member Author

I will have to remove the default behavior, as it might break existing applications, which already implement that callback by themself, we can not do that for a minor release...

@pamapa pamapa force-pushed the feat-logout-callback branch from efeb1b2 to 210f1c0 Compare February 9, 2024 13:48
@pamapa pamapa merged commit 7a542d3 into main Feb 9, 2024
@pamapa pamapa deleted the feat-logout-callback branch February 9, 2024 13:54
@pamapa pamapa added this to the 3.1.0 milestone Feb 9, 2024
@Aeolun

Aeolun commented Apr 16, 2024

Copy link
Copy Markdown

@PSanetra @pamapa Can we have a release for this issue? I can't call anything on UserManager directly, and my signouts are timing out.

@pamapa

pamapa commented Apr 16, 2024

Copy link
Copy Markdown
Member Author

@Aeolun I made the release, thanks for the hint, i almost forgot about it....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants