oidc-client-ts
oidc-client-ts copied to clipboard
refresh userinfo data on demand
Fixes #846 #852
Checklist
- [X] This PR makes changes to the public API
- [X] I have included links for closing relevant issue numbers
This PR adds a new parameter "refreshUserInfo" to "loadUser", so that it's possible to refresh (and store) updated userinfo claims for the logged user. loadUser will refresh the token, if expired, otherwise it will simply reload userinfo.
In order for this to be enabled, loadUserInfo should be enabled in the settings.
NOTE: if the token is refreshed during the procedure, old claims that "disappeared" will be removed. Otherwise, they will be kept.
It also:
- Adds a new configuration property "legacyMergeClaimsBehavior" (defaulted to
truefor backwards compatibility) that, if disabled, overwrites claims instead of transforming types. - Exposes a new
getUserInfomethod in OidcClient to perform the raw userinfo data retrieval.
LMKWYT
Codecov Report
Base: 77.69% // Head: 78.11% // Increases project coverage by +0.42% :tada:
Coverage data is based on head (
885099d) compared to base (ebe7545). Patch coverage: 95.83% of modified lines in pull request are covered.
Additional details and impacted files
@@ Coverage Diff @@
## main #877 +/- ##
==========================================
+ Coverage 77.69% 78.11% +0.42%
==========================================
Files 44 45 +1
Lines 1690 1723 +33
Branches 331 338 +7
==========================================
+ Hits 1313 1346 +33
- Misses 340 341 +1
+ Partials 37 36 -1
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 78.11% <95.83%> (+0.42%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/UserManager.ts | 58.59% <76.92%> (+0.72%) |
:arrow_up: |
| src/ClaimsService.ts | 100.00% <100.00%> (ø) |
|
| src/OidcClient.ts | 93.49% <100.00%> (+0.27%) |
:arrow_up: |
| src/OidcClientSettings.ts | 92.30% <100.00%> (+0.24%) |
:arrow_up: |
| src/ResponseValidator.ts | 88.88% <100.00%> (-1.74%) |
:arrow_down: |
| src/UserInfoService.ts | 75.00% <100.00%> (+5.43%) |
:arrow_up: |
| src/User.ts | 88.00% <0.00%> (+8.00%) |
:arrow_up: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
Thanks for providing this. I like the claims parts, yes it makes it easier when we split it in its own service. This code is almost ready to merge. With the refreshUserInfo i am not so happy yet because of the edge case handling (expired, signinSilent).
Idea:
- keep
refreshUserInfoin getUser, but without edge case handling, e..g the caller must take care the User is still valid... if not doingsigninSilent, ...
Simple without edge cases:
if (refreshUserInfo) {
logger.debug("refreshing user info");
user.profile = await this._client.getUserInfo(user);
await this.storeUser(user);
logger.debug("user updated in storage");
}
Would be nice if you could split this up into two merge requests.
keep refreshUserInfo in getUser, but without edge case handling, e..g the caller must take care the User is still valid... if not doing signinSilent, ...
What you added as an example was exactly my first implementation. After that, my reasoning was: UserManager is described as higher level API, so it should help users to obtain what they want without the hassle of multiple checks.
I can definitely simplify this. What do you think should happen if the token expired? We should throw an exception?
Would be nice if you could split this up into two merge requests.
Yeah no problem, you mean one for "splitting the claims service and changing the merge behavior" and the other for "adding the userinfo refresh behavior" ?
@marcoreni Curious if your still working on this? I've got a use case for this functionality and would love to be able to use it.
@marcoreni Curious if your still working on this? I've got a use case for this functionality and would love to be able to use it.
Same here
Hi all,
sadly, we did not have time to wait on the discussion on https://github.com/authts/oidc-client-ts/pull/881, so we forked the library and implemented a basic version of the "on demand update" (that served our purposes). The implementation is available on these two commits but I think it's not ready for merge for the following reasons: 1- it's based on https://github.com/authts/oidc-client-ts/pull/881 which has been superseded by https://github.com/authts/oidc-client-ts/pull/904 2- it does not have tests 3- it does not integrate the changes requested above
I don't think I'll have time to tackle this again soon.