Merge pull request #9784 from nocodb/nc-fix/cognito-signin

refactor: Token generation improvements and corrections

packages/nc-gui/composables/useApi/interceptors.ts

@@ -1,13 +1,7 @@
 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
@@ -18,8 +12,9 @@ export function addAxiosInterceptors(api: Api<any>) {
   axiosInstance.interceptors.request.use((config) => {
     config.headers['xc-gui'] = 'true'
 
-    // Add auth header only if signed in and if `xc-short-token` header is not present (for short-lived tokens used for token generation)
-    if (state.token.value && !config.headers['xc-short-token']) config.headers['xc-auth'] = state.token.value
+    if (state.token.value && !config.headers['xc-short-token']) {
+      config.headers['xc-auth'] = state.token.value
+    }
 
     if (!config.url?.endsWith('/user/me') && !config.url?.endsWith('/admin/roles') && state.previewAs?.value) {
       config.headers['xc-preview'] = state.previewAs.value
@@ -42,27 +37,22 @@ export function addAxiosInterceptors(api: Api<any>) {
     return config
   })
 
-  // Return a successful response back to the calling service
   axiosInstance.interceptors.response.use(
-    (response) => {
-      return response
-    },
-    // Handle Error
+    (response) => response,
     async (error) => {
       const isSharedPage =
         route.value?.params?.typeOrId === 'base' || route.value?.params?.typeOrId === 'ERD' || route.value.meta.public
 
-      // 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')
+      if (error.response?.data?.msg === DbNotFoundMsg) {
+        return router.replace('/base/0')
+      }
 
-      // Return any error which is not due to authentication back to the calling service
       if (!error.response || error.response.status !== 401) {
         return Promise.reject(error)
       }
 
-      // Logout user if token refresh didn't work or user is disabled
       if (error.config.url === '/auth/token/refresh') {
         await state.signOut({
           redirectToSignin: !route.value.meta.public,
@@ -71,105 +61,19 @@ export function addAxiosInterceptors(api: Api<any>) {
         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: 5000 })
-        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)
+      try {
+        const token = await state.refreshToken({
+          axiosInstance,
+          skipLogout: true,
         })
-          })
-        }
-      }
 
-      let refreshTokenPromiseRes: (token: string) => void
-      let refreshTokenPromiseRej: (e: Error) => void
-
-      // avoid multiple refresh token requests by multiple requests at the same time
-      // wait for the first request to finish and then retry the failed requests
-      if (refreshTokenPromise) {
-        // if previous refresh token request succeeds use the token and retry request
-        return refreshTokenPromise
-          .then((token) => {
-            // New request with new token
-            return new Promise((resolve, reject) => {
         const config = error.config
         config.headers['xc-auth'] = token
-              axiosInstance
-                .request(config)
-                .then((response) => {
-                  resolve(response)
-                })
-                .catch((error) => {
-                  reject(error)
-                })
-            })
-          })
-          .catch(() => {
-            // 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
-          })
-          .finally(() => {
-            isTokenRefreshInProgress.value = false
-          })
-      }
-
-      // Try request again with new token
-      return axiosInstance
-        .post('/auth/token/refresh', null, {
-          withCredentials: true,
-          cancelToken: undefined,
-        })
-        .then((token) => {
-          // New request with new token
-          const config = error.config
-          config.headers['xc-auth'] = token.data.token
-          state.signIn(token.data.token, true)
-
-          // resolve the refresh token promise and reset
-          refreshTokenPromiseRes(token.data.token)
-          refreshTokenPromise = null
-
-          return new Promise((resolve, reject) => {
-            axiosInstance
-              .request(config)
-              .then((response) => {
-                resolve(response)
-              })
-              .catch((error) => {
-                reject(error)
-              })
-          })
-        })
-        .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
+        const response = await axiosInstance.request(config)
+        return response
+      } catch (refreshTokenError) {
+        if ((refreshTokenError as any)?.code === 'ERR_CANCELED') {
           return Promise.reject(refreshTokenError)
         }
 
@@ -178,12 +82,8 @@ export function addAxiosInterceptors(api: Api<any>) {
           skipApiCall: true,
         })
 
-          // reject the refresh token promise and reset
-          refreshTokenPromiseRej(refreshTokenError)
-          refreshTokenPromise = null
-
         return Promise.reject(error)
-        })
+      }
     },
   )
 

packages/nc-gui/composables/useGlobal/actions.ts

@@ -1,8 +1,8 @@
 import { getActivePinia } from 'pinia'
-import type { Actions, AppInfo, State } from './types'
+import type { Actions, AppInfo, Getters, State } from './types'
 import type { NcProjectType } from '#imports'
 
-export function useGlobalActions(state: State): Actions {
+export function useGlobalActions(state: State, _getters: Getters): Actions {
   const isTokenUpdatedTab = useState('isTokenUpdatedTab', () => false)
 
   const setIsMobileMode = (isMobileMode: boolean) => {
@@ -45,7 +45,7 @@ export function useGlobalActions(state: State): Actions {
   /** Sign in by setting the token in localStorage
    * keepProps - is for keeping any existing role info if user id is same as previous user
    * */
-  const signIn: Actions['signIn'] = async (newToken, keepProps = false) => {
+  const signIn: Actions['signIn'] = (newToken, keepProps = false) => {
     isTokenUpdatedTab.value = true
     state.token.value = newToken
 
@@ -63,32 +63,45 @@ export function useGlobalActions(state: State): Actions {
   }
 
   /** manually try to refresh token */
-  const refreshToken = async () => {
+  const _refreshToken = async ({
+    axiosInstance,
+    skipSignOut = false,
+  }: {
+    axiosInstance?: any
+    skipSignOut?: boolean
+  } = {}) => {
     const nuxtApp = useNuxtApp()
     const t = nuxtApp.vueApp.i18n.global.t
 
-    return new Promise((resolve) => {
-      nuxtApp.$api.instance
-        .post('/auth/token/refresh', null, {
+    if (!axiosInstance) {
+      axiosInstance = nuxtApp.$api?.instance
+    }
+
+    try {
+      const response = await axiosInstance.post('/auth/token/refresh', null, {
         withCredentials: true,
       })
-        .then((response) => {
       if (response.data?.token) {
         signIn(response.data.token, true)
+        return response.data.token
       }
-        })
-        .catch(async () => {
-          if (state.token.value && state.user.value) {
+      return null
+    } catch (e) {
+      if (state.token.value && state.user.value && !skipSignOut) {
         await signOut({
           skipApiCall: true,
         })
         message.error(t('msg.error.youHaveBeenSignedOut'))
       }
-        })
-        .finally(() => resolve(true))
-    })
+      return null
+    }
   }
 
+  const refreshToken = useSharedExecutionFn('refreshToken', _refreshToken, {
+    timeout: 10000,
+    storageDelay: 1000,
+  })
+
   const loadAppInfo = async () => {
     try {
       const nuxtApp = useNuxtApp()

packages/nc-gui/composables/useGlobal/index.ts

@@ -40,7 +40,7 @@ export const useGlobal = createGlobalState((): UseGlobalReturn => {
 
   const getters = useGlobalGetters(state)
 
-  const actions = useGlobalActions(state)
+  const actions = useGlobalActions(state, getters)
 
   watch(
     state.jwtPayload,

packages/nc-gui/composables/useGlobal/types.ts

@@ -2,8 +2,8 @@ import type { ComputedRef, Ref, ToRefs } from 'vue'
 import type { WritableComputedRef } from '@vue/reactivity'
 import type { JwtPayload } from 'jwt-decode'
 import type { ProjectRoles } from 'nocodb-sdk'
+import type { AxiosInstance } from 'axios'
 import type { NcProjectType } from '#imports'
-
 export interface AppInfo {
   ncSiteUrl: string
   authType: 'jwt' | 'none'
@@ -91,8 +91,8 @@ export interface SignOutParams {
 
 export interface Actions {
   signOut: (signOutParams?: SignOutParams) => Promise<void>
-  signIn: (token: string, keepProps?: boolean) => Promise<void>
-  refreshToken: () => void
+  signIn: (token: string, keepProps?: boolean) => void
+  refreshToken: (params: { axiosInstance?: AxiosInstance; skipLogout?: boolean; cognitoOnly?: boolean }) => Promise<void>
   loadAppInfo: () => void
   setIsMobileMode: (isMobileMode: boolean) => void
   navigateToProject: (params: { workspaceId?: string; baseId?: string; type?: NcProjectType; query?: any }) => void

packages/nc-gui/middleware/02.auth.global.ts

@@ -75,8 +75,12 @@ export default defineNuxtRouteMiddleware(async (to, from) => {
       })
     }
 
+    try {
       /** try generating access token using refresh token */
-    await state.refreshToken()
+      await state.refreshToken({})
+    } catch (e) {
+      console.info('Refresh token failed', (e as Error)?.message)
+    }
 
     /** if user is still not signed in, redirect to signin page */
     if (!state.signedIn.value) {

packages/nc-gui/store/notification.ts

@@ -6,8 +6,6 @@ import { useStorage } from '@vueuse/core'
 const CancelToken = axios.CancelToken
 
 export const useNotification = defineStore('notificationStore', () => {
-  const isTokenRefreshInProgress = useStorage(TOKEN_REFRESH_PROGRESS_KEY, false)
-
   const readNotifications = ref<NotificationType[]>([])
 
   const unreadNotifications = ref<NotificationType[]>([])
@@ -195,7 +193,6 @@ export const useNotification = defineStore('notificationStore', () => {
     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()
   }
 

packages/nc-gui/utils/ncUtils.ts

@@ -1,2 +1,2 @@
 export const isEeUI = false
-export const TOKEN_REFRESH_PROGRESS_KEY = 'nc_token_refresh_progress'
+export const TOKEN_REFRESH_PROGRESS_KEY = 'nc-token-refresh-progress'

packages/nocodb/src/constants/index.ts

@@ -6,3 +6,10 @@ export const NC_ATTACHMENT_FIELD_SIZE =
   +process.env['NC_ATTACHMENT_FIELD_SIZE'] || 20 * 1024 * 1024; // 20 MB
 export const NC_MAX_ATTACHMENTS_ALLOWED =
   +process.env['NC_MAX_ATTACHMENTS_ALLOWED'] || 10;
+export const NC_REFRESH_TOKEN_EXP_IN_DAYS =
+  parseInt(process.env.NC_REFRESH_TOKEN_EXP_IN_DAYS, 10) || 30;
+
+// throw error if user provided invalid value
+if (!NC_REFRESH_TOKEN_EXP_IN_DAYS || NC_REFRESH_TOKEN_EXP_IN_DAYS <= 0) {
+  throw new Error('NC_REFRESH_TOKEN_EXP_IN_DAYS must be a positive number');
+}

packages/nocodb/src/db/CustomKnex.ts

@@ -990,7 +990,12 @@ function parseNestedCondition(obj, qb, pKey?, table?, tableAlias?) {
         break;
       default:
         // if object handle recursively
-        if (typeof val === 'object' && !Array.isArray(val)) {
+        if (
+          val &&
+          typeof val === 'object' &&
+          !(val instanceof Date) &&
+          !Array.isArray(val)
+        ) {
           qb = parseNestedCondition.call(self, val, qb, key, tn, tableAlias);
         } else {
           // handle based on operator
@@ -1247,7 +1252,12 @@ function parseNestedConditionv2(obj, qb, pKey?, table?, tableAlias?) {
         break;
       default:
         // if object handle recursively
-        if (typeof val === 'object' && !Array.isArray(val)) {
+        if (
+          val &&
+          typeof val === 'object' &&
+          !(val instanceof Date) &&
+          !Array.isArray(val)
+        ) {
           qb = parseNestedCondition.call(self, val, qb, key, tn, tableAlias);
         } else {
           // handle based on operator

packages/nocodb/src/models/UserRefreshToken.ts

@@ -3,14 +3,7 @@ import Noco from '~/Noco';
 import { extractProps } from '~/helpers/extractProps';
 import { MetaTable, RootScopes } from '~/utils/globals';
 import { parseMetaProp, stringifyMetaProp } from '~/utils/modelUtils';
-
-const NC_REFRESH_TOKEN_EXP_IN_DAYS =
-  parseInt(process.env.NC_REFRESH_TOKEN_EXP_IN_DAYS, 10) || 90;
-
-// throw error if user provided invalid value
-if (NC_REFRESH_TOKEN_EXP_IN_DAYS <= 0) {
-  throw new Error('NC_REFRESH_TOKEN_EXP_IN_DAYS must be a positive number');
-}
+import { NC_REFRESH_TOKEN_EXP_IN_DAYS } from '~/constants';
 
 export default class UserRefreshToken {
   fk_user_id: string;

packages/nocodb/src/services/users/helpers.ts

@@ -3,6 +3,7 @@ import * as jwt from 'jsonwebtoken';
 import type User from '~/models/User';
 import type { NcConfig } from '~/interface/config';
 import type { Response } from 'express';
+import { NC_REFRESH_TOKEN_EXP_IN_DAYS } from '~/constants';
 
 export function genJwt(
   user: User & { extra?: Record<string, any> },
@@ -30,7 +31,9 @@ export function setTokenCookie(res: Response, token): void {
   // create http only cookie with refresh token that expires in 7 days
   const cookieOptions = {
     httpOnly: true,
-    expires: new Date(Date.now() + 7 * 24 * 60 * 60 * 1000),
+    expires: new Date(
+      Date.now() + NC_REFRESH_TOKEN_EXP_IN_DAYS * 24 * 60 * 60 * 1000,
+    ),
     domain: process.env.NC_BASE_HOST_NAME || undefined,
   };
   res.cookie('refresh_token', token, cookieOptions);

packages/nocodb/src/services/users/users.service.ts

@@ -599,11 +599,11 @@ export class UsersService {
 
     if (!user['token_version']) {
       user['token_version'] = randomTokenString();
-    }
 
       await User.update(user.id, {
         token_version: user['token_version'],
       });
+    }
 
     await UserRefreshToken.insert({
       token: refreshToken,