From 0e7a9dce6b9772331556bebcbf3ae24eb4352011 Mon Sep 17 00:00:00 2001
From: Pranav C <pranavxc@gmail.com>
Date: Tue, 3 Sep 2024 20:20:42 +0000
Subject: [PATCH 1/3] fix: refresh-token based token generation issue

---
 .../components/dlg/TableDescriptionUpdate.vue |  1 -
 .../nc-gui/composables/useApi/interceptors.ts | 54 ++++++++++++++++---
 .../nc-gui/composables/useGlobal/actions.ts   | 11 ++--
 .../nc-gui/composables/useGlobal/index.ts     | 16 ------
 packages/nc-gui/store/notification.ts         |  8 ++-
 packages/nc-gui/utils/ncUtils.ts              |  1 +
 6 files changed, 63 insertions(+), 28 deletions(-)

diff --git a/packages/nc-gui/components/dlg/TableDescriptionUpdate.vue b/packages/nc-gui/components/dlg/TableDescriptionUpdate.vue
index 3d35e74630..5f85a44bc3 100644
--- a/packages/nc-gui/components/dlg/TableDescriptionUpdate.vue
+++ b/packages/nc-gui/components/dlg/TableDescriptionUpdate.vue
@@ -1,6 +1,5 @@
 <script setup lang="ts">
 import type { TableType } from 'nocodb-sdk'
-import type { ComponentPublicInstance } from '@vue/runtime-core'
 
 interface Props {
   modelValue?: boolean
diff --git a/packages/nc-gui/composables/useApi/interceptors.ts b/packages/nc-gui/composables/useApi/interceptors.ts
index 29d9208bbb..8f979c7f4b 100644
--- a/packages/nc-gui/composables/useApi/interceptors.ts
+++ b/packages/nc-gui/composables/useApi/interceptors.ts
@@ -1,10 +1,13 @@
 import type { Api } from 'nocodb-sdk'
+import { useStorage } from '@vueuse/core'
 
 const DbNotFoundMsg = 'Database config not found'
 
 let refreshTokenPromise: Promise<string> | null = null
 
 export function addAxiosInterceptors(api: Api<any>) {
+  const isTokenRefreshInProgress = useStorage(TOKEN_REFRESH_PROGRESS_KEY, false)
+
   const state = useGlobal()
   const router = useRouter()
   const route = router.currentRoute
@@ -45,7 +48,10 @@ export function addAxiosInterceptors(api: Api<any>) {
       return response
     },
     // Handle Error
-    (error) => {
+    async (error) => {
+      // if cancel request then throw error
+      if (error.code === 'ERR_CANCELED') return Promise.reject(error)
+
       if (error.response && error.response.data && error.response.data.msg === DbNotFoundMsg) return router.replace('/base/0')
 
       // Return any error which is not due to authentication back to the calling service
@@ -55,10 +61,33 @@ export function addAxiosInterceptors(api: Api<any>) {
 
       // Logout user if token refresh didn't work or user is disabled
       if (error.config.url === '/auth/token/refresh') {
-        state.signOut()
+        state.signOut(undefined, true)
         return Promise.reject(error)
       }
 
+      // if no active refresh token request in the current session then check the local storage
+      if (!refreshTokenPromise && isTokenRefreshInProgress.value) {
+        // if token refresh is already in progress, wait for it to finish and then retry the request if token is available
+        await until(isTokenRefreshInProgress).toMatch((v) => !v, { timeout: 10000 })
+        isTokenRefreshInProgress.value = false
+
+        // check if the user is signed in by checking the token presence and retry the request with the new token
+        if (state.token.value) {
+          return new Promise((resolve, reject) => {
+            const config = error.config
+            config.headers['xc-auth'] = state.token.value
+            axiosInstance
+              .request(config)
+              .then((response) => {
+                resolve(response)
+              })
+              .catch((error) => {
+                reject(error)
+              })
+          })
+        }
+      }
+
       let refreshTokenPromiseRes: (token: string) => void
       let refreshTokenPromiseRej: (e: Error) => void
 
@@ -86,15 +115,20 @@ export function addAxiosInterceptors(api: Api<any>) {
             // ignore since it could have already been handled and redirected to sign in
           })
       } else {
+        isTokenRefreshInProgress.value = true
         refreshTokenPromise = new Promise<string>((resolve, reject) => {
           refreshTokenPromiseRes = resolve
           refreshTokenPromiseRej = reject
         })
 
         // set a catch on the promise to avoid unhandled promise rejection
-        refreshTokenPromise.catch(() => {
-          // ignore
-        })
+        refreshTokenPromise
+          .catch(() => {
+            // ignore
+          })
+          .finally(() => {
+            isTokenRefreshInProgress.value = false
+          })
       }
 
       // Try request again with new token
@@ -124,9 +158,17 @@ export function addAxiosInterceptors(api: Api<any>) {
           })
         })
         .catch(async (refreshTokenError) => {
+          // skip signout call if request cancelled
+          if (refreshTokenError.code === 'ERR_CANCELED') {
+            // reject the refresh token promise and reset
+            refreshTokenPromiseRej(refreshTokenError)
+            refreshTokenPromise = null
+            return Promise.reject(refreshTokenError)
+          }
+
           await state.signOut({
             redirectToSignin: !route.value.meta.public,
-          })
+          }, true);
 
           // reject the refresh token promise and reset
           refreshTokenPromiseRej(refreshTokenError)
diff --git a/packages/nc-gui/composables/useGlobal/actions.ts b/packages/nc-gui/composables/useGlobal/actions.ts
index 94f5363573..cbd6621227 100644
--- a/packages/nc-gui/composables/useGlobal/actions.ts
+++ b/packages/nc-gui/composables/useGlobal/actions.ts
@@ -1,5 +1,5 @@
 import { getActivePinia } from 'pinia'
-import type { Actions, AppInfo, SignOutParams, State } from './types'
+import type { Actions, AppInfo, State } from './types'
 import type { NcProjectType } from '#imports'
 
 export function useGlobalActions(state: State): Actions {
@@ -8,10 +8,13 @@ export function useGlobalActions(state: State): Actions {
   }
 
   /** Sign out by deleting the token from localStorage */
-  const signOut: Actions['signOut'] = async ({ redirectToSignin, signinUrl = '/signin' }: SignOutParams = {}) => {
+  const signOut: Actions['signOut'] = async ({ redirectToSignin, signinUrl = '/signin', skipApiCall = false }: SignOutParams = {}) => {
     try {
-      const nuxtApp = useNuxtApp()
-      await nuxtApp.$api.auth.signout()
+      // call and invalidate refresh token only if user manually triggered logout
+      if (!skipApiCall) {
+        const nuxtApp = useNuxtApp()
+        await nuxtApp.$api.auth.signout()
+      }
     } catch {
       // ignore error
     } finally {
diff --git a/packages/nc-gui/composables/useGlobal/index.ts b/packages/nc-gui/composables/useGlobal/index.ts
index ed51d255d6..16a9d34805 100644
--- a/packages/nc-gui/composables/useGlobal/index.ts
+++ b/packages/nc-gui/composables/useGlobal/index.ts
@@ -42,22 +42,6 @@ export const useGlobal = createGlobalState((): UseGlobalReturn => {
 
   const actions = useGlobalActions(state)
 
-  /** try to refresh token before expiry (5 min before expiry) */
-  watch(
-    () =>
-      !!(
-        state.jwtPayload.value &&
-        state.jwtPayload.value.exp &&
-        state.jwtPayload.value.exp - 5 * 60 < state.timestamp.value / 1000
-      ),
-    async (expiring: boolean) => {
-      if (getters.signedIn.value && state.jwtPayload.value && expiring) {
-        await actions.refreshToken()
-      }
-    },
-    { immediate: true },
-  )
-
   watch(
     state.jwtPayload,
     (nextPayload) => {
diff --git a/packages/nc-gui/store/notification.ts b/packages/nc-gui/store/notification.ts
index dc2e7bd251..1d9f21cd7f 100644
--- a/packages/nc-gui/store/notification.ts
+++ b/packages/nc-gui/store/notification.ts
@@ -2,8 +2,11 @@ import { defineStore } from 'pinia'
 import type { NotificationType } from 'nocodb-sdk'
 import axios, { type CancelTokenSource } from 'axios'
 import { CancelToken } from 'axios'
+import { useStorage } from '@vueuse/core'
 
 export const useNotification = defineStore('notificationStore', () => {
+  const isTokenRefreshInProgress = useStorage(TOKEN_REFRESH_PROGRESS_KEY, false)
+
   const readNotifications = ref<NotificationType[]>([])
 
   const unreadNotifications = ref<NotificationType[]>([])
@@ -198,7 +201,10 @@ export const useNotification = defineStore('notificationStore', () => {
       try {
         if (newToken && newToken !== oldToken) {
           await init()
-        } else if (!newToken) {
+        }
+        // clear polling if there is no refresh token request in progress
+        // and access token is removed
+        else if (!newToken && !isTokenRefreshInProgress.value) {
           clearPolling()
         }
       } catch (e) {
diff --git a/packages/nc-gui/utils/ncUtils.ts b/packages/nc-gui/utils/ncUtils.ts
index d5cffd508e..c2a581984f 100644
--- a/packages/nc-gui/utils/ncUtils.ts
+++ b/packages/nc-gui/utils/ncUtils.ts
@@ -1 +1,2 @@
 export const isEeUI = false
+export const TOKEN_REFRESH_PROGRESS_KEY = 'nc_token_refresh_progress'

From ebf3941de97dd88bfb101514f2e4005c94135435 Mon Sep 17 00:00:00 2001
From: Pranav C <pranavxc@gmail.com>
Date: Tue, 3 Sep 2024 20:20:42 +0000
Subject: [PATCH 2/3] refactor: improvements in notification polling

---
 .../nc-gui/composables/useApi/interceptors.ts |  3 ++-
 packages/nc-gui/store/notification.ts         | 19 +++++++++++--------
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/packages/nc-gui/composables/useApi/interceptors.ts b/packages/nc-gui/composables/useApi/interceptors.ts
index 8f979c7f4b..7fa0c8a54c 100644
--- a/packages/nc-gui/composables/useApi/interceptors.ts
+++ b/packages/nc-gui/composables/useApi/interceptors.ts
@@ -68,7 +68,7 @@ export function addAxiosInterceptors(api: Api<any>) {
       // if no active refresh token request in the current session then check the local storage
       if (!refreshTokenPromise && isTokenRefreshInProgress.value) {
         // if token refresh is already in progress, wait for it to finish and then retry the request if token is available
-        await until(isTokenRefreshInProgress).toMatch((v) => !v, { timeout: 10000 })
+        await until(isTokenRefreshInProgress).toMatch((v) => !v, { timeout: 5000 })
         isTokenRefreshInProgress.value = false
 
         // check if the user is signed in by checking the token presence and retry the request with the new token
@@ -135,6 +135,7 @@ export function addAxiosInterceptors(api: Api<any>) {
       return axiosInstance
         .post('/auth/token/refresh', null, {
           withCredentials: true,
+          cancelToken: undefined,
         })
         .then((token) => {
           // New request with new token
diff --git a/packages/nc-gui/store/notification.ts b/packages/nc-gui/store/notification.ts
index 1d9f21cd7f..13f9ff1650 100644
--- a/packages/nc-gui/store/notification.ts
+++ b/packages/nc-gui/store/notification.ts
@@ -175,13 +175,19 @@ export const useNotification = defineStore('notificationStore', () => {
   })
 
   // function to clear polling and cancel any pending requests
-  const clearPolling = () => {
+  const clearPolling = async () => {
     if (timeOutId) {
       clearTimeout(timeOutId)
       timeOutId = null
     }
-    cancelTokenSource?.cancel()
+    // take a reference of the cancel token source and set the current one to null
+    // so that we can cancel the polling request even if token changes
+    const source = cancelTokenSource
     cancelTokenSource = null
+    // wait if refresh token generation is in progress and cancel the polling after that
+    // set a timeout of 10 seconds to avoid hanging
+    await until(isTokenRefreshInProgress).toMatch((v) => !v, { timeout: 10000 })
+    source?.cancel()
   }
 
   const init = async () => {
@@ -189,7 +195,7 @@ export const useNotification = defineStore('notificationStore', () => {
     // For playwright, polling will cause the test to hang indefinitely
     // as we wait for the networkidle event. So, we disable polling for playwright
     if (!(window as any).isPlaywright) {
-      clearPolling()
+      clearPolling().catch((e) => console.log(e))
       pollNotifications().catch((e) => console.log(e))
     }
   }
@@ -201,11 +207,8 @@ export const useNotification = defineStore('notificationStore', () => {
       try {
         if (newToken && newToken !== oldToken) {
           await init()
-        }
-        // clear polling if there is no refresh token request in progress
-        // and access token is removed
-        else if (!newToken && !isTokenRefreshInProgress.value) {
-          clearPolling()
+        } else if (!newToken) {
+          clearPolling().catch((e) => console.log(e))
         }
       } catch (e) {
         console.error(e)

From 439b51dabda152ebd00031c899d475c83c78e898 Mon Sep 17 00:00:00 2001
From: Pranav C <pranavxc@gmail.com>
Date: Tue, 3 Sep 2024 20:20:42 +0000
Subject: [PATCH 3/3] chore: lint

---
 packages/nc-gui/composables/useApi/interceptors.ts |  3 ++-
 packages/nc-gui/composables/useGlobal/actions.ts   | 10 ++++++++--
 packages/nc-gui/composables/useGlobal/types.ts     |  1 +
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/packages/nc-gui/composables/useApi/interceptors.ts b/packages/nc-gui/composables/useApi/interceptors.ts
index 7fa0c8a54c..ae5e1a04cd 100644
--- a/packages/nc-gui/composables/useApi/interceptors.ts
+++ b/packages/nc-gui/composables/useApi/interceptors.ts
@@ -169,7 +169,8 @@ export function addAxiosInterceptors(api: Api<any>) {
 
           await state.signOut({
             redirectToSignin: !route.value.meta.public,
-          }, true);
+            skipApiCall: true,
+          })
 
           // reject the refresh token promise and reset
           refreshTokenPromiseRej(refreshTokenError)
diff --git a/packages/nc-gui/composables/useGlobal/actions.ts b/packages/nc-gui/composables/useGlobal/actions.ts
index cbd6621227..1b077f9aa2 100644
--- a/packages/nc-gui/composables/useGlobal/actions.ts
+++ b/packages/nc-gui/composables/useGlobal/actions.ts
@@ -8,7 +8,11 @@ export function useGlobalActions(state: State): Actions {
   }
 
   /** Sign out by deleting the token from localStorage */
-  const signOut: Actions['signOut'] = async ({ redirectToSignin, signinUrl = '/signin', skipApiCall = false }: SignOutParams = {}) => {
+  const signOut: Actions['signOut'] = async ({
+    redirectToSignin,
+    signinUrl = '/signin',
+    skipApiCall = false,
+  }: SignOutParams = {}) => {
     try {
       // call and invalidate refresh token only if user manually triggered logout
       if (!skipApiCall) {
@@ -72,7 +76,9 @@ export function useGlobalActions(state: State): Actions {
         })
         .catch(async () => {
           if (state.token.value && state.user.value) {
-            await signOut()
+            await signOut({
+              skipApiCall: true,
+            })
             message.error(t('msg.error.youHaveBeenSignedOut'))
           }
         })
diff --git a/packages/nc-gui/composables/useGlobal/types.ts b/packages/nc-gui/composables/useGlobal/types.ts
index 4e5d412d18..ccc1141dff 100644
--- a/packages/nc-gui/composables/useGlobal/types.ts
+++ b/packages/nc-gui/composables/useGlobal/types.ts
@@ -84,6 +84,7 @@ export interface SignOutParams {
   redirectToSignin?: boolean
   signinUrl?: string
   skipRedirect?: boolean
+  skipApiCall?: boolean
 }
 
 export interface Actions {