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] 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'