-
-
Notifications
You must be signed in to change notification settings - Fork 16
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.
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
feat: Added callbacks onTokenExpiring and onTokenExpired #91
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -784,15 +784,6 @@ abstract class OidcUserManagerBase { | |
if (user == null) { | ||
tokenEventsManager.unload(); | ||
} else { | ||
if (!discoveryDocument.grantTypesSupportedOrDefault | ||
.contains(OidcConstants_GrantType.refreshToken)) { | ||
//Server doesn't support refresh_token grant. | ||
return; | ||
} | ||
if (user.token.refreshToken == null) { | ||
// Can't refresh the access token anyway. | ||
return; | ||
} | ||
if (user.token.expiresIn == null) { | ||
// Can't know how much time is left. | ||
return; | ||
|
@@ -803,6 +794,8 @@ abstract class OidcUserManagerBase { | |
|
||
@protected | ||
Future<void> handleTokenExpiring(OidcToken event) async { | ||
settings.onTokenExpiring?.call(event); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we still refresh using also would it make sense to expose a stream of expiring/expired events instead of callbacks ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Maybe there are scenarios where the user has to do some preparatory work before refresh can happen? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Whatever you prefer. I could also add events to the existing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ahmednfwela Any news from your side? Are there any open points that prevent you from merging this PR? This is somehow important to me as I'm currently referencing my fork in our production app 🙈 |
||
|
||
final refreshToken = event.refreshToken; | ||
if (refreshToken == null) { | ||
return; | ||
|
@@ -843,6 +836,8 @@ abstract class OidcUserManagerBase { | |
|
||
@protected | ||
void handleTokenExpired(OidcToken event) { | ||
settings.onTokenExpired?.call(event); | ||
|
||
if (!settings.supportOfflineAuth) { | ||
forgetUser(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is removed cause
handleTokenExpiring
should be called even if there is no refreshToken...