fix(web): preserve current URL on 401 redirect and harden token refresh lock

- Build signin URL with redirect param so users return to their
  original page after re-authentication
- Replace naive refresh lock with token-based ownership to prevent
  cross-tab lock release conflicts
- Add stale lock detection with max age to avoid deadlocks from
  crashed tabs
- Add timeout to waitUntilTokenRefreshed to prevent infinite polling
- Add tests for signin redirect URL building and refresh token logic

web/service/base.signin-redirect.spec.ts

@@ -0,0 +1,61 @@
+import { describe, expect, it } from 'vitest'
+import { buildSigninUrlWithRedirect } from './base'
+
+describe('buildSigninUrlWithRedirect', () => {
+  describe('OAuth callback preservation', () => {
+    it('should include the full current OAuth authorize URL in oauth_redirect_url', () => {
+      // Arrange
+      const currentLocation = {
+        origin: 'https://skills.bash-is-all-you-need.dify.dev',
+        pathname: '/account/oauth/authorize',
+        search: '?client_id=dcfcd6a4-5799-405a-a6d7-04261b24dd02&redirect_uri=https%3A%2F%2Fcreators.dify.dev%2Fapi%2Fv1%2Foauth%2Fcallback%2Fdify&response_type=code',
+      } as const
+
+      // Act
+      const signinUrl = buildSigninUrlWithRedirect(currentLocation, '')
+      const parsedSigninUrl = new URL(signinUrl)
+
+      // Assert
+      expect(parsedSigninUrl.pathname).toBe('/signin')
+      const encodedRedirect = parsedSigninUrl.searchParams.get('oauth_redirect_url')
+      expect(encodedRedirect).toBeTruthy()
+      expect(decodeURIComponent(encodedRedirect!)).toBe(`${currentLocation.origin}${currentLocation.pathname}${currentLocation.search}`)
+    })
+  })
+
+  describe('Signin self-redirect guard', () => {
+    it('should return plain signin URL when current path is already signin', () => {
+      // Arrange
+      const currentLocation = {
+        origin: 'https://skills.bash-is-all-you-need.dify.dev',
+        pathname: '/signin',
+        search: '?oauth_redirect_url=https%3A%2F%2Fskills.bash-is-all-you-need.dify.dev%2Faccount%2Foauth%2Fauthorize',
+      } as const
+
+      // Act
+      const signinUrl = buildSigninUrlWithRedirect(currentLocation, '')
+
+      // Assert
+      expect(signinUrl).toBe('https://skills.bash-is-all-you-need.dify.dev/signin')
+    })
+  })
+
+  describe('basePath support', () => {
+    it('should respect basePath when building signin URL', () => {
+      // Arrange
+      const currentLocation = {
+        origin: 'https://example.com',
+        pathname: '/console/apps',
+        search: '?tab=all',
+      } as const
+
+      // Act
+      const signinUrl = buildSigninUrlWithRedirect(currentLocation, '/console')
+
+      // Assert
+      expect(signinUrl.startsWith('https://example.com/console/signin?')).toBe(true)
+      const encodedRedirect = new URL(signinUrl).searchParams.get('oauth_redirect_url')
+      expect(decodeURIComponent(encodedRedirect || '')).toBe('https://example.com/console/apps?tab=all')
+    })
+  })
+})

web/service/base.ts

@@ -37,6 +37,7 @@ import { refreshAccessTokenOrReLogin } from './refresh-token'
 import { getWebAppPassport } from './webapp-auth'
 
 const TIME_OUT = 100000
+const SIGNIN_REDIRECT_URL_KEY = 'oauth_redirect_url'
 
 export type IconObject = {
   background: string
@@ -156,6 +157,20 @@ function jumpTo(url: string) {
   globalThis.location.href = url
 }
 
+export function buildSigninUrlWithRedirect(currentLocation: Pick<Location, 'origin' | 'pathname' | 'search'>, currentBasePath: string) {
+  const signinPath = `${currentBasePath}/signin`
+  const signinUrl = `${currentLocation.origin}${signinPath}`
+
+  // Keep signin as-is to avoid redirect loops.
+  if (currentLocation.pathname === signinPath)
+    return signinUrl
+
+  const currentUrl = `${currentLocation.origin}${currentLocation.pathname}${currentLocation.search}`
+  const params = new URLSearchParams()
+  params.set(SIGNIN_REDIRECT_URL_KEY, encodeURIComponent(currentUrl))
+  return `${signinUrl}?${params.toString()}`
+}
+
 function unicodeToChar(text: string) {
   if (!text)
     return ''
@@ -781,7 +796,7 @@ export const request = async<T>(url: string, options = {}, otherOptions?: IOther
     const errResp: Response = err as any
     if (errResp.status === 401) {
       const [parseErr, errRespData] = await asyncRunSafe<ResponseError>(errResp.json())
-      const loginUrl = `${globalThis.location.origin}${basePath}/signin`
+      const loginUrl = buildSigninUrlWithRedirect(globalThis.location, basePath)
       if (parseErr) {
         globalThis.location.href = loginUrl
         return Promise.reject(err)

web/service/refresh-token.spec.ts

@@ -0,0 +1,86 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { refreshAccessTokenOrReLogin } from './refresh-token'
+
+const mockFetchWithRetry = vi.fn()
+
+vi.mock('@/utils', () => ({
+  fetchWithRetry: (...args: unknown[]) => mockFetchWithRetry(...args),
+}))
+
+function createDeferred<T>() {
+  let resolve!: (value: T) => void
+  let reject!: (reason?: unknown) => void
+  const promise = new Promise<T>((res, rej) => {
+    resolve = res
+    reject = rej
+  })
+
+  return { promise, resolve, reject }
+}
+
+describe('refreshAccessTokenOrReLogin', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    vi.useRealTimers()
+    localStorage.clear()
+    globalThis.fetch = vi.fn()
+  })
+
+  describe('stale cross-tab lock handling', () => {
+    it('should clean stale lock and execute refresh request', async () => {
+      // Arrange
+      localStorage.setItem('is_other_tab_refreshing', '1')
+      localStorage.setItem('last_refresh_time', `${Date.now() - 30_000}`)
+      mockFetchWithRetry.mockResolvedValue([null, new Response(null, { status: 200 })])
+
+      // Act
+      await refreshAccessTokenOrReLogin(5_000)
+
+      // Assert
+      expect(mockFetchWithRetry).toHaveBeenCalledTimes(1)
+      expect(localStorage.getItem('is_other_tab_refreshing')).toBeNull()
+      expect(localStorage.getItem('last_refresh_time')).toBeNull()
+    })
+  })
+
+  describe('concurrent refresh requests', () => {
+    it('should avoid duplicate refresh calls when a refresh is already in progress', async () => {
+      // Arrange
+      const deferredRefresh = createDeferred<[null, Response]>()
+      mockFetchWithRetry.mockImplementation(() => deferredRefresh.promise)
+
+      // Act
+      const firstRefresh = refreshAccessTokenOrReLogin(5_000)
+      const secondRefresh = refreshAccessTokenOrReLogin(5_000)
+      deferredRefresh.resolve([null, new Response(null, { status: 200 })])
+
+      // Assert
+      await expect(firstRefresh).resolves.toBeUndefined()
+      await expect(secondRefresh).resolves.toBeUndefined()
+      expect(mockFetchWithRetry).toHaveBeenCalledTimes(1)
+      expect(localStorage.getItem('is_other_tab_refreshing')).toBeNull()
+      expect(localStorage.getItem('last_refresh_time')).toBeNull()
+    })
+  })
+
+  describe('waiter behavior', () => {
+    it('should wait for another tab lock to release without issuing duplicate refresh', async () => {
+      // Arrange
+      vi.useFakeTimers()
+      localStorage.setItem('is_other_tab_refreshing', 'other-tab-token')
+      localStorage.setItem('last_refresh_time', `${Date.now()}`)
+
+      // Act
+      const waitingRefresh = refreshAccessTokenOrReLogin(5_000)
+      setTimeout(() => {
+        localStorage.removeItem('is_other_tab_refreshing')
+        localStorage.removeItem('last_refresh_time')
+      }, 300)
+      await vi.advanceTimersByTimeAsync(1_000)
+
+      // Assert
+      await expect(waitingRefresh).resolves.toBeUndefined()
+      expect(mockFetchWithRetry).not.toHaveBeenCalled()
+    })
+  })
+})

web/service/refresh-token.ts

@@ -2,16 +2,55 @@ import { API_PREFIX } from '@/config'
 import { fetchWithRetry } from '@/utils'
 
 const LOCAL_STORAGE_KEY = 'is_other_tab_refreshing'
+const LAST_REFRESH_TIME_KEY = 'last_refresh_time'
+const REFRESH_LOCK_MAX_AGE_MS = 10 * 1000
+const REFRESH_LOCK_POLL_INTERVAL_MS = 200
 
 let isRefreshing = false
-function waitUntilTokenRefreshed() {
+let refreshLockToken: string | null = null
+
+const getCurrentTime = () => Date.now()
+
+function getRefreshLockAge() {
+  const lastTime = globalThis.localStorage.getItem(LAST_REFRESH_TIME_KEY) || '0'
+  const parsedLastTime = Number.parseInt(lastTime, 10)
+  if (Number.isNaN(parsedLastTime) || parsedLastTime <= 0)
+    return Number.POSITIVE_INFINITY
+
+  return getCurrentTime() - parsedLastTime
+}
+
+function hasValidRefreshLock() {
+  const refreshLock = globalThis.localStorage.getItem(LOCAL_STORAGE_KEY)
+  if (!refreshLock)
+    return false
+
+  if (getRefreshLockAge() <= REFRESH_LOCK_MAX_AGE_MS)
+    return true
+
+  // stale lock from another tab/session: clean it up to avoid deadlock
+  globalThis.localStorage.removeItem(LOCAL_STORAGE_KEY)
+  globalThis.localStorage.removeItem(LAST_REFRESH_TIME_KEY)
+  return false
+}
+
+function waitUntilTokenRefreshed(maxWaitMs: number) {
+  const startedAt = getCurrentTime()
   return new Promise<void>((resolve) => {
     function _check() {
-      const isRefreshingSign = globalThis.localStorage.getItem(LOCAL_STORAGE_KEY)
-      if ((isRefreshingSign && isRefreshingSign === '1') || isRefreshing) {
+      if (getCurrentTime() - startedAt >= maxWaitMs) {
+        if (!isRefreshing) {
+          globalThis.localStorage.removeItem(LOCAL_STORAGE_KEY)
+          globalThis.localStorage.removeItem(LAST_REFRESH_TIME_KEY)
+        }
+        resolve()
+        return
+      }
+
+      if (hasValidRefreshLock() || isRefreshing) {
         setTimeout(() => {
           _check()
-        }, 1000)
+        }, REFRESH_LOCK_POLL_INTERVAL_MS)
       }
       else {
         resolve()
@@ -21,45 +60,46 @@ function waitUntilTokenRefreshed() {
   })
 }
 
-const isRefreshingSignAvailable = function (delta: number) {
-  const nowTime = new Date().getTime()
-  const lastTime = globalThis.localStorage.getItem('last_refresh_time') || '0'
-  return nowTime - Number.parseInt(lastTime) <= delta
+function acquireRefreshLock() {
+  refreshLockToken = `${getCurrentTime()}-${Math.random().toString(36).slice(2)}`
+  globalThis.localStorage.setItem(LOCAL_STORAGE_KEY, refreshLockToken)
+  globalThis.localStorage.setItem(LAST_REFRESH_TIME_KEY, getCurrentTime().toString())
 }
 
 // only one request can send
 async function getNewAccessToken(timeout: number): Promise<void> {
+  let lockAcquired = false
+
   try {
-    const isRefreshingSign = globalThis.localStorage.getItem(LOCAL_STORAGE_KEY)
-    if ((isRefreshingSign && isRefreshingSign === '1' && isRefreshingSignAvailable(timeout)) || isRefreshing) {
-      await waitUntilTokenRefreshed()
+    if (hasValidRefreshLock() || isRefreshing) {
+      await waitUntilTokenRefreshed(Math.min(timeout, REFRESH_LOCK_MAX_AGE_MS))
+      return
+    }
+
+    isRefreshing = true
+    acquireRefreshLock()
+    lockAcquired = true
+    globalThis.addEventListener('beforeunload', releaseRefreshLock)
+
+    // Do not use baseFetch to refresh tokens.
+    // If a 401 response occurs and baseFetch itself attempts to refresh the token,
+    // it can lead to an infinite loop if the refresh attempt also returns 401.
+    // To avoid this, handle token refresh separately in a dedicated function
+    // that does not call baseFetch and uses a single retry mechanism.
+    const [error, ret] = await fetchWithRetry(globalThis.fetch(`${API_PREFIX}/refresh-token`, {
+      method: 'POST',
+      credentials: 'include', // Important: include cookies in the request
+      headers: {
+        'Content-Type': 'application/json;utf-8',
+      },
+      // No body needed - refresh token is in cookie
+    }))
+    if (error) {
+      return Promise.reject(error)
     }
     else {
-      isRefreshing = true
-      globalThis.localStorage.setItem(LOCAL_STORAGE_KEY, '1')
-      globalThis.localStorage.setItem('last_refresh_time', new Date().getTime().toString())
-      globalThis.addEventListener('beforeunload', releaseRefreshLock)
-
-      // Do not use baseFetch to refresh tokens.
-      // If a 401 response occurs and baseFetch itself attempts to refresh the token,
-      // it can lead to an infinite loop if the refresh attempt also returns 401.
-      // To avoid this, handle token refresh separately in a dedicated function
-      // that does not call baseFetch and uses a single retry mechanism.
-      const [error, ret] = await fetchWithRetry(globalThis.fetch(`${API_PREFIX}/refresh-token`, {
-        method: 'POST',
-        credentials: 'include', // Important: include cookies in the request
-        headers: {
-          'Content-Type': 'application/json;utf-8',
-        },
-        // No body needed - refresh token is in cookie
-      }))
-      if (error) {
-        return Promise.reject(error)
-      }
-      else {
-        if (ret.status === 401)
-          return Promise.reject(ret)
-      }
+      if (ret.status === 401)
+        return Promise.reject(ret)
     }
   }
   catch (error) {
@@ -67,16 +107,21 @@ async function getNewAccessToken(timeout: number): Promise<void> {
     return Promise.reject(error)
   }
   finally {
-    releaseRefreshLock()
+    if (lockAcquired)
+      releaseRefreshLock()
   }
 }
 
 function releaseRefreshLock() {
-  // Always clear the refresh lock to avoid cross-tab deadlocks.
-  // This is safe to call multiple times and from tabs that were only waiting.
+  const currentLockToken = globalThis.localStorage.getItem(LOCAL_STORAGE_KEY)
+
   isRefreshing = false
-  globalThis.localStorage.removeItem(LOCAL_STORAGE_KEY)
-  globalThis.localStorage.removeItem('last_refresh_time')
+  if (refreshLockToken && currentLockToken === refreshLockToken) {
+    globalThis.localStorage.removeItem(LOCAL_STORAGE_KEY)
+    globalThis.localStorage.removeItem(LAST_REFRESH_TIME_KEY)
+  }
+
+  refreshLockToken = null
   globalThis.removeEventListener('beforeunload', releaseRefreshLock)
 }