fix(common): prevent infinite auth refresh retry loop on permanent token failure (#5893)
Co-authored-by: nivedin <nivedinp@gmail.com>
This commit is contained in:
parent
08921786e7
commit
4a61e3464d
8 changed files with 427 additions and 25 deletions
|
|
@ -0,0 +1,205 @@
|
|||
import { describe, test, expect, vi } from "vitest"
|
||||
import { createAuthRetryGuard } from "../retryAuthGuard"
|
||||
|
||||
const refreshSuccess = () => Promise.resolve(true)
|
||||
const refreshFailure = () => Promise.resolve(false)
|
||||
const refreshThrow = () => Promise.reject(new Error("network error"))
|
||||
|
||||
describe("createAuthRetryGuard", () => {
|
||||
describe("success resets failure count", () => {
|
||||
test("returns true on successful refresh", async () => {
|
||||
const guard = createAuthRetryGuard(vi.fn())
|
||||
|
||||
expect(await guard.execute(refreshSuccess)).toBe(true)
|
||||
})
|
||||
|
||||
test("resets failure count after a success", async () => {
|
||||
const onExhausted = vi.fn()
|
||||
const guard = createAuthRetryGuard(onExhausted)
|
||||
|
||||
// Accumulate 2 failures
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
|
||||
// Success resets the counter
|
||||
await guard.execute(refreshSuccess)
|
||||
|
||||
// 3 more failures needed to exhaust
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
expect(onExhausted).not.toHaveBeenCalled()
|
||||
|
||||
await guard.execute(refreshFailure)
|
||||
expect(onExhausted).toHaveBeenCalledOnce()
|
||||
})
|
||||
})
|
||||
|
||||
describe("exhaustion after MAX_RETRIES (3)", () => {
|
||||
test("calls onExhausted after 3 consecutive failures", async () => {
|
||||
const onExhausted = vi.fn()
|
||||
const guard = createAuthRetryGuard(onExhausted)
|
||||
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
expect(onExhausted).not.toHaveBeenCalled()
|
||||
|
||||
await guard.execute(refreshFailure)
|
||||
expect(onExhausted).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
test("returns false for every failed attempt", async () => {
|
||||
const guard = createAuthRetryGuard(vi.fn())
|
||||
|
||||
expect(await guard.execute(refreshFailure)).toBe(false)
|
||||
expect(await guard.execute(refreshFailure)).toBe(false)
|
||||
expect(await guard.execute(refreshFailure)).toBe(false)
|
||||
})
|
||||
|
||||
test("short-circuits to false after exhaustion without calling refreshFn", async () => {
|
||||
const guard = createAuthRetryGuard(vi.fn())
|
||||
|
||||
// Exhaust the guard
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
|
||||
const refreshFn = vi.fn(refreshSuccess)
|
||||
expect(await guard.execute(refreshFn)).toBe(false)
|
||||
expect(refreshFn).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe("onExhausted runs at most once", () => {
|
||||
test("does not call onExhausted again on subsequent execute calls", async () => {
|
||||
const onExhausted = vi.fn()
|
||||
const guard = createAuthRetryGuard(onExhausted)
|
||||
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
|
||||
expect(onExhausted).toHaveBeenCalledOnce()
|
||||
})
|
||||
})
|
||||
|
||||
describe("thrown errors count as failures", () => {
|
||||
test("treats a thrown refreshFn as a failed attempt", async () => {
|
||||
const onExhausted = vi.fn()
|
||||
const guard = createAuthRetryGuard(onExhausted)
|
||||
|
||||
await guard.execute(refreshThrow)
|
||||
await guard.execute(refreshThrow)
|
||||
await guard.execute(refreshThrow)
|
||||
|
||||
expect(onExhausted).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
test("does not propagate the error to the caller", async () => {
|
||||
const guard = createAuthRetryGuard(vi.fn())
|
||||
|
||||
await expect(guard.execute(refreshThrow)).resolves.toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("onExhausted failure handling", () => {
|
||||
test("stays exhausted if onExhausted throws", async () => {
|
||||
const onExhausted = vi.fn(() => {
|
||||
throw new Error("sign-out failed")
|
||||
})
|
||||
const guard = createAuthRetryGuard(onExhausted)
|
||||
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
|
||||
// Guard is still exhausted — short-circuits
|
||||
const refreshFn = vi.fn(refreshSuccess)
|
||||
expect(await guard.execute(refreshFn)).toBe(false)
|
||||
expect(refreshFn).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test("does not propagate onExhausted error to caller", async () => {
|
||||
const guard = createAuthRetryGuard(() =>
|
||||
Promise.reject(new Error("sign-out failed"))
|
||||
)
|
||||
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
|
||||
await expect(guard.execute(refreshFailure)).resolves.toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("reset()", () => {
|
||||
test("re-enables the guard after exhaustion", async () => {
|
||||
const onExhausted = vi.fn()
|
||||
const guard = createAuthRetryGuard(onExhausted)
|
||||
|
||||
// Exhaust
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
|
||||
guard.reset()
|
||||
|
||||
// Guard is usable again
|
||||
expect(await guard.execute(refreshSuccess)).toBe(true)
|
||||
})
|
||||
|
||||
test("resets the failure counter", async () => {
|
||||
const onExhausted = vi.fn()
|
||||
const guard = createAuthRetryGuard(onExhausted)
|
||||
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
|
||||
guard.reset()
|
||||
|
||||
// Need 3 fresh failures to exhaust again
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
expect(onExhausted).not.toHaveBeenCalled()
|
||||
|
||||
await guard.execute(refreshFailure)
|
||||
expect(onExhausted).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
test("is a no-op while onExhausted is in-flight", async () => {
|
||||
let resolveExhausted!: () => void
|
||||
const exhaustedPromise = new Promise<void>((resolve) => {
|
||||
resolveExhausted = resolve
|
||||
})
|
||||
const onExhausted = vi.fn(() => exhaustedPromise)
|
||||
const guard = createAuthRetryGuard(onExhausted)
|
||||
|
||||
await guard.execute(refreshFailure)
|
||||
await guard.execute(refreshFailure)
|
||||
|
||||
// Start the 3rd failure — onExhausted is now in-flight.
|
||||
// Don't await: we want to call reset() while it's still pending.
|
||||
const thirdCall = guard.execute(refreshFailure)
|
||||
|
||||
// Flush microtasks so execute() progresses past `await refreshFn()`
|
||||
// and sets exhaustionPromise before we call reset().
|
||||
await Promise.resolve()
|
||||
|
||||
// reset() while onExhausted hasn't resolved yet — should be a no-op
|
||||
guard.reset()
|
||||
|
||||
// Guard should still be exhausted
|
||||
const refreshFn = vi.fn(refreshSuccess)
|
||||
expect(await guard.execute(refreshFn)).toBe(false)
|
||||
expect(refreshFn).not.toHaveBeenCalled()
|
||||
|
||||
// Let onExhausted finish
|
||||
resolveExhausted()
|
||||
await thirdCall
|
||||
|
||||
// Now reset() should work
|
||||
guard.reset()
|
||||
expect(await guard.execute(refreshSuccess)).toBe(true)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -21,8 +21,9 @@ import * as E from "fp-ts/Either"
|
|||
import * as TE from "fp-ts/TaskEither"
|
||||
import { pipe, constVoid, flow } from "fp-ts/function"
|
||||
import { subscribe, pipe as wonkaPipe } from "wonka"
|
||||
import { filter, map, Subject } from "rxjs"
|
||||
import { filter, map, Subject, Subscription } from "rxjs"
|
||||
import { platform } from "~/platform"
|
||||
import { createAuthRetryGuard } from "~/helpers/retryAuthGuard"
|
||||
|
||||
// TODO: Implement caching
|
||||
|
||||
|
|
@ -65,6 +66,8 @@ const createSubscriptionClient = () => {
|
|||
})
|
||||
}
|
||||
|
||||
const authRetryGuard = createAuthRetryGuard(() => platform.auth.signOutUser())
|
||||
|
||||
const createHoppClient = () => {
|
||||
const exchanges = [
|
||||
// devtoolsExchange,
|
||||
|
|
@ -107,9 +110,9 @@ const createHoppClient = () => {
|
|||
},
|
||||
async refreshAuth() {
|
||||
const refresh = platform.auth.refreshAuthToken
|
||||
// should we logout if refreshAuthToken is not defined?
|
||||
if (!refresh) return
|
||||
await refresh()
|
||||
|
||||
await authRetryGuard.execute(() => refresh.call(platform.auth))
|
||||
},
|
||||
}
|
||||
}),
|
||||
|
|
@ -146,11 +149,23 @@ const createHoppClient = () => {
|
|||
}
|
||||
|
||||
let subscriptionClient: SubscriptionClient | null
|
||||
let authEventSubscription: Subscription | null = null
|
||||
export const client = ref<Client>()
|
||||
|
||||
export function initBackendGQLClient() {
|
||||
client.value = createHoppClient()
|
||||
|
||||
// Reset the retry guard only on successful login, not on every
|
||||
// client recreation (which also fires on logout/token_refresh).
|
||||
authEventSubscription?.unsubscribe()
|
||||
authEventSubscription = platform.auth
|
||||
.getAuthEventsStream()
|
||||
.subscribe((event) => {
|
||||
if (event.event === "login") {
|
||||
authRetryGuard.reset()
|
||||
}
|
||||
})
|
||||
|
||||
platform.auth.onBackendGQLClientShouldReconnect(() => {
|
||||
const currentUser = platform.auth.getCurrentUser()
|
||||
|
||||
|
|
|
|||
78
packages/hoppscotch-common/src/helpers/retryAuthGuard.ts
Normal file
78
packages/hoppscotch-common/src/helpers/retryAuthGuard.ts
Normal file
|
|
@ -0,0 +1,78 @@
|
|||
/**
|
||||
* Maximum number of consecutive auth refresh failures before signing out.
|
||||
* @see https://github.com/hoppscotch/hoppscotch/issues/5885
|
||||
*/
|
||||
const MAX_RETRIES = 3
|
||||
|
||||
/**
|
||||
* Creates an auth retry guard that tracks consecutive refresh failures
|
||||
* and triggers a sign-out after {@link MAX_RETRIES} consecutive failures.
|
||||
*
|
||||
* After exhaustion, subsequent calls short-circuit to `false` without
|
||||
* invoking `refreshFn` or `onExhausted` again. Call `reset()` on
|
||||
* successful login to re-enable refresh attempts.
|
||||
*
|
||||
* @see https://github.com/hoppscotch/hoppscotch/issues/5885
|
||||
*/
|
||||
export function createAuthRetryGuard(onExhausted: () => void | Promise<void>) {
|
||||
let failCount = 0
|
||||
let isExhausted = false
|
||||
let exhaustionPromise: Promise<void> | null = null
|
||||
|
||||
return {
|
||||
/**
|
||||
* Wraps an auth refresh attempt with retry tracking.
|
||||
* Resets on success. Calls `onExhausted` after {@link MAX_RETRIES}
|
||||
* consecutive failures and stays exhausted until `reset()` is called.
|
||||
*/
|
||||
async execute(refreshFn: () => Promise<boolean>): Promise<boolean> {
|
||||
// isExhausted covers the normal path; failCount >= MAX_RETRIES covers
|
||||
// the concurrent-call edge case where two callers both passed the check
|
||||
// at failCount = 2 before either could set isExhausted = true.
|
||||
if (isExhausted || failCount >= MAX_RETRIES) {
|
||||
return false
|
||||
}
|
||||
|
||||
let success: boolean
|
||||
try {
|
||||
success = await refreshFn()
|
||||
} catch (_) {
|
||||
// Treat thrown errors (network failures, etc.) as a failed refresh
|
||||
// so they count toward exhaustion and don't bypass the guard.
|
||||
success = false
|
||||
}
|
||||
|
||||
if (success) {
|
||||
failCount = 0
|
||||
return true
|
||||
}
|
||||
|
||||
failCount++
|
||||
if (failCount >= MAX_RETRIES && !isExhausted) {
|
||||
isExhausted = true
|
||||
try {
|
||||
exhaustionPromise = Promise.resolve().then(() => onExhausted())
|
||||
await exhaustionPromise
|
||||
} catch (_) {
|
||||
// Sign-out failed (e.g. network error), but the guard stays
|
||||
// exhausted so we don't re-enter the refresh loop.
|
||||
} finally {
|
||||
exhaustionPromise = null
|
||||
}
|
||||
}
|
||||
|
||||
return false
|
||||
},
|
||||
|
||||
/**
|
||||
* Reset the failure counter (e.g. on login or manual logout).
|
||||
* No-op while an exhaustion callback (sign-out) is still in-flight.
|
||||
*/
|
||||
reset() {
|
||||
if (exhaustionPromise) return
|
||||
|
||||
failCount = 0
|
||||
isExhausted = false
|
||||
},
|
||||
}
|
||||
}
|
||||
|
|
@ -222,7 +222,7 @@ async function refreshToken() {
|
|||
try {
|
||||
const refreshToken =
|
||||
await persistenceService.getLocalConfig("refresh_token")
|
||||
if (!refreshToken) return null
|
||||
if (!refreshToken) return false
|
||||
|
||||
const { response } = interceptorService.execute({
|
||||
id: Date.now(),
|
||||
|
|
@ -523,7 +523,7 @@ export const def: AuthPlatformDef = {
|
|||
|
||||
async refreshAuthToken() {
|
||||
const refreshed = await refreshToken()
|
||||
return refreshed ?? false
|
||||
return refreshed
|
||||
},
|
||||
|
||||
/**
|
||||
|
|
@ -553,6 +553,6 @@ export const def: AuthPlatformDef = {
|
|||
}
|
||||
|
||||
const refreshed = await refreshToken()
|
||||
return refreshed ?? false
|
||||
return refreshed
|
||||
},
|
||||
}
|
||||
|
|
|
|||
|
|
@ -339,8 +339,6 @@ export const def: AuthPlatformDef = {
|
|||
},
|
||||
|
||||
async signOutUser() {
|
||||
// if (!currentUser$.value) throw new Error("No user has logged in")
|
||||
|
||||
await logout()
|
||||
|
||||
probableUser$.next(null)
|
||||
|
|
|
|||
|
|
@ -56,12 +56,12 @@ export type OnboardingStatus = {
|
|||
const currentUser$ = new BehaviorSubject<HoppUser | null>(null);
|
||||
|
||||
const signOut = async (reloadWindow = false) => {
|
||||
await authQuery.logout();
|
||||
|
||||
// Reload the window if both `access_token` and `refresh_token`is invalid
|
||||
// there by the user is taken to the login page
|
||||
if (reloadWindow) {
|
||||
window.location.reload();
|
||||
// Best-effort backend logout — local state must be cleared regardless
|
||||
// so the UI never stays stuck in an authenticated state.
|
||||
try {
|
||||
await authQuery.logout();
|
||||
} catch (_) {
|
||||
// Backend unreachable — continue with local cleanup
|
||||
}
|
||||
|
||||
currentUser$.next(null);
|
||||
|
|
@ -70,6 +70,12 @@ const signOut = async (reloadWindow = false) => {
|
|||
authEvents$.next({
|
||||
event: 'logout',
|
||||
});
|
||||
|
||||
// Reload the window if both `access_token` and `refresh_token` are invalid
|
||||
// thereby the user is taken to the login page
|
||||
if (reloadWindow) {
|
||||
window.location.reload();
|
||||
}
|
||||
};
|
||||
|
||||
const getUserDetails = async () => {
|
||||
|
|
@ -132,10 +138,15 @@ const setInitialUser = async () => {
|
|||
const refreshToken = async () => {
|
||||
try {
|
||||
const res = await authQuery.refreshToken();
|
||||
authEvents$.next({
|
||||
event: 'token_refresh',
|
||||
});
|
||||
return res.status === 200;
|
||||
const isSuccessful = res.status === 200;
|
||||
|
||||
if (isSuccessful) {
|
||||
authEvents$.next({
|
||||
event: 'token_refresh',
|
||||
});
|
||||
}
|
||||
|
||||
return isSuccessful;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
|
|
|
|||
82
packages/hoppscotch-sh-admin/src/helpers/retryAuthGuard.ts
Normal file
82
packages/hoppscotch-sh-admin/src/helpers/retryAuthGuard.ts
Normal file
|
|
@ -0,0 +1,82 @@
|
|||
/**
|
||||
* Maximum number of consecutive auth refresh failures before signing out.
|
||||
* @see https://github.com/hoppscotch/hoppscotch/issues/5885
|
||||
*/
|
||||
const MAX_RETRIES = 3
|
||||
|
||||
/**
|
||||
* Creates an auth retry guard that tracks consecutive refresh failures
|
||||
* and triggers a sign-out after {@link MAX_RETRIES} consecutive failures.
|
||||
*
|
||||
* After exhaustion, subsequent calls short-circuit to `false` without
|
||||
* invoking `refreshFn` or `onExhausted` again. Call `reset()` on
|
||||
* successful login to re-enable refresh attempts.
|
||||
*
|
||||
* NOTE: This is a copy of `@hoppscotch/common/helpers/retryAuthGuard.ts`.
|
||||
* `sh-admin` cannot depend on `@hoppscotch/common`, so the utility is
|
||||
* duplicated here. Keep both copies in sync.
|
||||
*
|
||||
* @see https://github.com/hoppscotch/hoppscotch/issues/5885
|
||||
*/
|
||||
export function createAuthRetryGuard(onExhausted: () => void | Promise<void>) {
|
||||
let failCount = 0
|
||||
let isExhausted = false
|
||||
let exhaustionPromise: Promise<void> | null = null
|
||||
|
||||
return {
|
||||
/**
|
||||
* Wraps an auth refresh attempt with retry tracking.
|
||||
* Resets on success. Calls `onExhausted` after {@link MAX_RETRIES}
|
||||
* consecutive failures and stays exhausted until `reset()` is called.
|
||||
*/
|
||||
async execute(refreshFn: () => Promise<boolean>): Promise<boolean> {
|
||||
// isExhausted covers the normal path; failCount >= MAX_RETRIES covers
|
||||
// the concurrent-call edge case where two callers both passed the check
|
||||
// at failCount = 2 before either could set isExhausted = true.
|
||||
if (isExhausted || failCount >= MAX_RETRIES) {
|
||||
return false
|
||||
}
|
||||
|
||||
let success: boolean
|
||||
try {
|
||||
success = await refreshFn()
|
||||
} catch (_) {
|
||||
// Treat thrown errors (network failures, etc.) as a failed refresh
|
||||
// so they count toward exhaustion and don't bypass the guard.
|
||||
success = false
|
||||
}
|
||||
|
||||
if (success) {
|
||||
failCount = 0
|
||||
return true
|
||||
}
|
||||
|
||||
failCount++
|
||||
if (failCount >= MAX_RETRIES && !isExhausted) {
|
||||
isExhausted = true
|
||||
try {
|
||||
exhaustionPromise = Promise.resolve().then(() => onExhausted())
|
||||
await exhaustionPromise
|
||||
} catch (_) {
|
||||
// Sign-out failed (e.g. network error), but the guard stays
|
||||
// exhausted so we don't re-enter the refresh loop.
|
||||
} finally {
|
||||
exhaustionPromise = null
|
||||
}
|
||||
}
|
||||
|
||||
return false
|
||||
},
|
||||
|
||||
/**
|
||||
* Reset the failure counter (e.g. on login or manual logout).
|
||||
* No-op while an exhaustion callback (sign-out) is still in-flight.
|
||||
*/
|
||||
reset() {
|
||||
if (exhaustionPromise) return
|
||||
|
||||
failCount = 0
|
||||
isExhausted = false
|
||||
},
|
||||
}
|
||||
}
|
||||
|
|
@ -1,6 +1,7 @@
|
|||
import { authExchange } from '@urql/exchange-auth';
|
||||
import urql, { cacheExchange, createClient, fetchExchange } from '@urql/vue';
|
||||
import { createApp, h } from 'vue';
|
||||
import * as O from 'fp-ts/Option';
|
||||
import App from './App.vue';
|
||||
import ErrorComponent from './pages/_.vue';
|
||||
|
||||
|
|
@ -13,12 +14,24 @@ import '../assets/scss/styles.scss';
|
|||
import '../assets/scss/tailwind.scss';
|
||||
// END STYLES
|
||||
|
||||
import { pipe } from 'fp-ts/function';
|
||||
import * as O from 'fp-ts/Option';
|
||||
import { auth } from './helpers/auth';
|
||||
import { auth, authEvents$ } from './helpers/auth';
|
||||
import { GRAPHQL_UNAUTHORIZED } from './helpers/errors';
|
||||
import { createAuthRetryGuard } from './helpers/retryAuthGuard';
|
||||
import { HOPP_MODULES } from './modules';
|
||||
|
||||
/**
|
||||
* Auth retry guard — prevents infinite refreshAuth loops when tokens
|
||||
* are permanently invalid. Stays exhausted until the page reloads.
|
||||
* @see https://github.com/hoppscotch/hoppscotch/issues/5885
|
||||
*/
|
||||
const authRetryGuard = createAuthRetryGuard(() => auth.signOutUser(true));
|
||||
|
||||
authEvents$.subscribe((event) => {
|
||||
if (event.event === 'login') {
|
||||
authRetryGuard.reset();
|
||||
}
|
||||
});
|
||||
|
||||
(async () => {
|
||||
try {
|
||||
// Create URQL client
|
||||
|
|
@ -35,10 +48,10 @@ import { HOPP_MODULES } from './modules';
|
|||
return operation;
|
||||
},
|
||||
async refreshAuth() {
|
||||
pipe(
|
||||
await auth.performAuthRefresh(),
|
||||
O.getOrElseW(() => auth.signOutUser(true))
|
||||
);
|
||||
await authRetryGuard.execute(async () => {
|
||||
const result = await auth.performAuthRefresh();
|
||||
return O.isSome(result);
|
||||
});
|
||||
},
|
||||
didAuthError(error, _operation) {
|
||||
return error.message === GRAPHQL_UNAUTHORIZED;
|
||||
|
|
|
|||
Loading…
Reference in a new issue