Fix infinite loop of refresh grant requests when the refresh token is expired or revoked#531
Merged
ShanChathusanda93 merged 1 commit intoJun 23, 2026
Conversation
… expired or revoked
📝 WalkthroughWalkthroughIntroduces a ChangesRefresh Token Loop Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: Ping-pong health check failed Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🦋 Changeset detectedThe changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.changeset/fix-refresh-token-loop.md (1)
7-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove stray character on line 7.
The changeset entry contains an erroneous
7character on line 7, which will cause parsing issues with the Changesets tool. The description should end cleanly after the text on line 6.🛠️ Proposed fix
Fix infinite loop of refresh grant requests when the refresh token is expired or revoked -7🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.changeset/fix-refresh-token-loop.md at line 7, Remove the stray `7` character that appears on line 7 of the changeset file. This character is extraneous and will cause parsing issues with the Changesets tool. Delete this character so the changeset entry ends cleanly after the description text on line 6.packages/browser/src/__legacy__/helpers/authentication-helper.ts (2)
146-148: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClear the latch when a custom grant creates a new session.
The flag is cleared after auth-code sign-in and refresh success, but a direct
exchangeToken()withreturnsSessioncan install fresh tokens and still inherit_hasRefreshFailed = true, causing auto-refresh to remain blocked for the new session.Suggested fix
if (config.returnsSession) { + this._hasRefreshFailed = false; this._spaHelper.refreshAccessTokenAutomatically(this); return this._authenticationClient.getUser();Also applies to: 189-189
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/browser/src/__legacy__/helpers/authentication-helper.ts` around lines 146 - 148, The _hasRefreshFailed flag is not being cleared when a custom grant with returnsSession creates a new session, causing auto-refresh to remain blocked even though fresh tokens have been installed. In the conditional block where config.returnsSession is true (around the call to this._spaHelper.refreshAccessTokenAutomatically), clear the _hasRefreshFailed flag before calling refreshAccessTokenAutomatically to reset the latch for the new session and allow automatic token refresh to function properly.
180-199: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftMove the refresh-failure latch into the refresh-grant path.
Line 744 only latches failures when
isSignedIn()initiated the refresh. Auto-refresh and request-retry callers invokerefreshAccessToken()directly, so those failures can still leave_hasRefreshFailedfalse. SplitrefreshAccessToken()so only the actual refresh-grant failure sets the latch; custom-grant replay failures after a successful refresh should not poison the refresh token state.Suggested structure
public async refreshAccessToken( enableRetrievingSignOutURLFromSession?: (config: SPACustomGrantConfig) => void, ): Promise<User> { + if (this._hasRefreshFailed) { + throw new AsgardeoAuthException( + 'SPA-AUTH_HELPER-RAT2-SE01', + 'Refresh token request skipped.', + 'A previous refresh token request failed for the current session.', + ); + } + try { await this._authenticationClient.refreshAccessToken(); - const customGrantConfig = await this.getCustomGrantConfigData(); - if (customGrantConfig) { - await this.exchangeToken(customGrantConfig, enableRetrievingSignOutURLFromSession); - } this._hasRefreshFailed = false; - this._spaHelper.refreshAccessTokenAutomatically(this); - - return this._authenticationClient.getUser(); } catch (error) { + this._hasRefreshFailed = true; const refreshTokenError: Message<string> = { type: REFRESH_ACCESS_TOKEN_ERR0R, }; window.postMessage(refreshTokenError); return Promise.reject(error); } + + try { + const customGrantConfig = await this.getCustomGrantConfigData(); + if (customGrantConfig) { + await this.exchangeToken(customGrantConfig, enableRetrievingSignOutURLFromSession); + } + this._spaHelper.refreshAccessTokenAutomatically(this); + + return this._authenticationClient.getUser(); + } catch (error) { + return Promise.reject(error); + } } ... } catch { this._isTokenRefreshing = false; - this._hasRefreshFailed = true; return false; }Also applies to: 736-744
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/browser/src/__legacy__/helpers/authentication-helper.ts` around lines 180 - 199, The refresh-failure latch _hasRefreshFailed is currently set to false after the entire refresh operation completes, including the custom-grant exchange. Move the line that sets _hasRefreshFailed to false immediately after the refreshAccessToken() call succeeds (before the custom grant exchange logic), so that only actual refresh-grant failures set the latch. This ensures that failures in the exchangeToken() call do not poison the refresh token state when the underlying token refresh itself was successful. The custom-grant replay errors should be handled separately without affecting the refresh token state tracking.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/browser/src/__legacy__/helpers/spa-helper.ts`:
- Around line 55-57: The hasRefreshFailed() guard check on line 57 only protects
the immediate refresh path when timeUntilRefresh <= 0. A scheduled timer
callback set up in the code around lines 68-74 can still execute and attempt to
refresh the token even after a refresh failure has been recorded. Add the same
authenticationHelper.hasRefreshFailed() guard check inside the scheduled refresh
callback function to prevent retry attempts when a refresh has already failed,
mirroring the protection already present in the immediate refresh path.
In `@packages/react/src/contexts/Asgardeo/AsgardeoProvider.tsx`:
- Around line 395-397: The handleRefreshTokenError function currently filters
messages only by the event type without validating the message source, origin,
or instanceId, which allows other frames or AsgardeoProvider instances to
trigger the signed-out state. Add validation checks in handleRefreshTokenError
to verify that the message source is from the current window, the origin matches
the expected origin, and the instanceId in the event data matches the current
provider instance before calling setIsSignedInSync(false).
- Around line 396-398: When the 'refresh-access-token-error' event is triggered,
the code currently only sets isSignedInSync to false but leaves the user,
profile, and organization state cached, allowing stale authenticated user data
to be rendered. In addition to setting isSignedInSync to false, also clear the
user, profile, and organization state within the same event handler block to
ensure all cached user context is properly cleared when the session becomes
invalid.
---
Outside diff comments:
In @.changeset/fix-refresh-token-loop.md:
- Line 7: Remove the stray `7` character that appears on line 7 of the changeset
file. This character is extraneous and will cause parsing issues with the
Changesets tool. Delete this character so the changeset entry ends cleanly after
the description text on line 6.
In `@packages/browser/src/__legacy__/helpers/authentication-helper.ts`:
- Around line 146-148: The _hasRefreshFailed flag is not being cleared when a
custom grant with returnsSession creates a new session, causing auto-refresh to
remain blocked even though fresh tokens have been installed. In the conditional
block where config.returnsSession is true (around the call to
this._spaHelper.refreshAccessTokenAutomatically), clear the _hasRefreshFailed
flag before calling refreshAccessTokenAutomatically to reset the latch for the
new session and allow automatic token refresh to function properly.
- Around line 180-199: The refresh-failure latch _hasRefreshFailed is currently
set to false after the entire refresh operation completes, including the
custom-grant exchange. Move the line that sets _hasRefreshFailed to false
immediately after the refreshAccessToken() call succeeds (before the custom
grant exchange logic), so that only actual refresh-grant failures set the latch.
This ensures that failures in the exchangeToken() call do not poison the refresh
token state when the underlying token refresh itself was successful. The
custom-grant replay errors should be handled separately without affecting the
refresh token state tracking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f440e1cc-e726-4033-b192-a69c589c6f02
📒 Files selected for processing (4)
.changeset/fix-refresh-token-loop.mdpackages/browser/src/__legacy__/helpers/authentication-helper.tspackages/browser/src/__legacy__/helpers/spa-helper.tspackages/react/src/contexts/Asgardeo/AsgardeoProvider.tsx
brionmario
reviewed
Jun 23, 2026
| // When the refresh token is expired or revoked, the browser SDK posts a | ||
| // 'refresh-access-token-error' message. Sync the React context so components | ||
| // stop calling auth methods and the user is redirected to sign in again. | ||
| useEffect(() => { |
Member
There was a problem hiding this comment.
We may need to port this fix to other SDKs as well. @coderabbitai Create an issue in https://github.com/asgardeo/asgardeo to track this
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gh issue create \
--repo "asgardeo/asgardeo" \
--assignee "brionmario" \
--title "Port infinite refresh-token loop fix to other SDKs" \
--body "## Summary
When a refresh token is expired or revoked, certain Asgardeo SDKs enter an infinite loop of refresh-grant requests. A fix was implemented in the **JavaScript SDK** ([`@asgardeo/browser`](https://github.com/asgardeo/javascript/tree/main/packages/browser) and [`@asgardeo/react`](https://github.com/asgardeo/javascript/tree/main/packages/react)) via [asgardeo/javascript#531](https://github.com/asgardeo/javascript/pull/531). The same fix needs to be ported to all other Asgardeo SDKs.
## Root Cause
The \`isSignedIn()\` method attempts a refresh grant when the access token expires. The \`_isTokenRefreshing\` guard flag resets to \`false\` after each failure, causing every subsequent call to \`isSignedIn()\` (from navigation guards, polling intervals, \`_validateMethod\`, etc.) to fire new \`/token\` requests. Two additional factors contributed:
- The React context's \`isSignedInSync\` was never updated on refresh failure.
- \`scheduleAutoRefresh()\` called \`refreshAccessTokenAutomatically()\`, bypassing the guard mechanism.
## Fix Applied in JavaScript SDK
1. **authentication-helper.ts** – Added a \`_hasRefreshFailed\` boolean flag. After the first refresh failure in \`isSignedIn()\`, the flag is set to \`true\` and all subsequent calls return \`false\` immediately (no network requests). The flag resets to \`false\` on successful refresh or fresh sign-in via auth code exchange.
2. **spa-helper.ts** – Added a \`hasRefreshFailed()\` check in the immediate-refresh path of \`refreshAccessTokenAutomatically()\` to stop redundant attempts after \`isSignedIn()\` has marked the token as failed.
3. **AsgardeoProvider.tsx (React)** – Added a \`useEffect\` listener for the \`refresh-access-token-error\` window message. When received, sets \`isSignedInSync = false\` so React components stop treating the user as signed in.
## Action Required
Port equivalent logic to all other Asgardeo SDK packages, ensuring:
- [ ] A failure-tracking flag (or equivalent mechanism) prevents repeated refresh-grant requests after the first failure.
- [ ] The unauthenticated state is propagated correctly to framework-level context/providers.
- [ ] Normal refresh behavior resumes after a fresh sign-in.
## References
- Fix PR: [asgardeo/javascript#531](https://github.com/asgardeo/javascript/pull/531)
- Originating comment: https://github.com/asgardeo/javascript/pull/531#discussion_r3460381234
- Requested by: `@brionmario`
"Length of output: 3035
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gh issue create \
--repo "asgardeo/asgardeo" \
--title "Port infinite refresh-token loop fix to other SDKs" \
--body "## Summary
When a refresh token is expired or revoked, certain Asgardeo SDKs enter an infinite loop of refresh-grant requests. A fix was implemented in the **JavaScript SDK** ([`@asgardeo/browser`](https://github.com/asgardeo/javascript/tree/main/packages/browser) and [`@asgardeo/react`](https://github.com/asgardeo/javascript/tree/main/packages/react)) via [asgardeo/javascript#531](https://github.com/asgardeo/javascript/pull/531). The same fix needs to be ported to all other Asgardeo SDKs.
## Root Cause
The \`isSignedIn()\` method attempts a refresh grant when the access token expires. The \`_isTokenRefreshing\` guard flag resets to \`false\` after each failure, causing every subsequent call to \`isSignedIn()\` (from navigation guards, polling intervals, \`_validateMethod\`, etc.) to fire new \`/token\` requests. Two additional factors contributed:
- The React context's \`isSignedInSync\` was never updated on refresh failure.
- \`scheduleAutoRefresh()\` called \`refreshAccessTokenAutomatically()\`, bypassing the guard mechanism.
## Fix Applied in JavaScript SDK
1. **authentication-helper.ts** – Added a \`_hasRefreshFailed\` boolean flag. After the first refresh failure in \`isSignedIn()\`, the flag is set to \`true\` and all subsequent calls return \`false\` immediately (no network requests). The flag resets to \`false\` on successful refresh or fresh sign-in via auth code exchange.
2. **spa-helper.ts** – Added a \`hasRefreshFailed()\` check in the immediate-refresh path of \`refreshAccessTokenAutomatically()\` to stop redundant attempts after \`isSignedIn()\` has marked the token as failed.
3. **AsgardeoProvider.tsx (React)** – Added a \`useEffect\` listener for the \`refresh-access-token-error\` window message. When received, sets \`isSignedInSync = false\` so React components stop treating the user as signed in.
## Action Required
Port equivalent logic to all other Asgardeo SDK packages, ensuring:
- [ ] A failure-tracking flag (or equivalent mechanism) prevents repeated refresh-grant requests after the first failure.
- [ ] The unauthenticated state is propagated correctly to framework-level context/providers.
- [ ] Normal refresh behavior resumes after a fresh sign-in.
## References
- Fix PR: [asgardeo/javascript#531](https://github.com/asgardeo/javascript/pull/531)
- Originating comment: https://github.com/asgardeo/javascript/pull/531#discussion_r3460381234
- Requested by: `@brionmario`
"Length of output: 2778
@brionmario, the tracking issue has been created: asgardeo/asgardeo#6 — Port infinite refresh-token loop fix to other SDKs.
It includes the root cause, a summary of all three changes from this PR, and acceptance criteria for the port. Note that I wasn't able to auto-assign it due to integration permissions — you may want to assign it manually from the issue page.
brionmario
approved these changes
Jun 23, 2026
This was referenced Jun 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
$subject
Problem
Root cause:
isSignedIn()now attempts a refresh grant whenever the access token is expired. The existing_isTokenRefreshingflag only guards against concurrent refresh attempts — it resets tofalseafter each failure. This meant every subsequent call toisSignedIn()(from_validateMethod, navigation guards,AsgardeoProvider's polling interval, etc.) would fire another/tokenrequest, creating a rapid-fire loop with no exit condition.There were two additional contributing factors:
AsgardeoProvider's React context (isSignedInSync) was never updated when the refresh failed, so app hooks continued to call auth methods believing the user was still signed in.scheduleAutoRefresh()inAsgardeoProviderwas callingrefreshAccessTokenAutomatically()which bypassesisSignedIn()entirely and callsrefreshAccessToken()directly — so the loop also fired through the periodic refresh path.Changes
packages/browser/src/__legacy__/helpers/authentication-helper.ts_hasRefreshFailed: booleanflag (initializedfalse) to track whether a refresh attempt has already failed for the current session's tokens.isSignedIn(), sets_hasRefreshFailed = true— all subsequentisSignedIn()calls returnfalseimmediately without hitting the network._hasRefreshFailed = falsein two places:refreshAccessToken()on success — covers the HTTP interceptor and SPAHelper timer paths.requestAccessToken()callback — covers fresh sign-in via auth code exchange.hasRefreshFailed()public getter soSPAHelpercan read the flag.packages/browser/src/__legacy__/helpers/spa-helper.tsauthenticationHelper.hasRefreshFailed()check in thetimeUntilRefresh <= 0immediate-refresh path insiderefreshAccessTokenAutomatically(). Prevents the SPAHelper from firing a redundant refresh attempt afterisSignedIn()has already marked the refresh token as failed.packages/react/src/contexts/Asgardeo/AsgardeoProvider.tsxuseEffectlistener for therefresh-access-token-errorwindow message (posted byrefreshAccessToken()on failure). When received, setsisSignedInSync = falseso the React context correctly reflects the unauthenticated state — stopping app components and hooks from continuing to call auth methods.Behavior after fix
isSignedIn()called repeatedlyfalseimmediatelyscheduleAutoRefresh()called after a failed refreshhasRefreshFailed()check in SPAHelpergetIdToken()(context still shows signed in)isSignedInSync = falsepropagated via window message; hooks stop_hasRefreshFailednever resetrequestAccessTokensuccess; normal refresh behavior resumesRelated Issues
Related PRs
Checklist
Security checks
Summary by CodeRabbit