fix: reload the folder after a silent token renewal

packages/web-pkg/src/composables/appDefaults/useAppFolderHandling.ts

@@ -134,7 +134,8 @@ export function useAppFolderHandling({
       }
     } catch (error) {
       if (error.statusCode === 401) {
-        return authService.handleAuthError(unref(currentRoute), error)
+        await authService.handleAuthError(unref(currentRoute), error)
+        return
       }
       resourcesStore.setCurrentFolder(null)
       folderLoadingError.value = error

packages/web-pkg/src/composables/authContext/useAuthService.ts

@@ -2,7 +2,10 @@ import { useService } from '../service'
 import { NavigationFailure } from 'vue-router'
 
 export interface AuthServiceInterface {
-  handleAuthError(route: any, error?: unknown): any
+  /**
+   * Resolves to `true` if the token got renewed, so the caller can retry its request.
+   */
+  handleAuthError(route: any, error?: unknown): Promise<boolean>
   signinSilent(): Promise<unknown>
   /**
    * Silent renewal that retries network errors. Resolves to whether it succeeded.

packages/web-pkg/src/services/folder/folderService.ts

@@ -106,28 +106,38 @@ export class FolderService {
       }
       resourcesStore.startResourcesLoading()
       try {
-        // cancel inner load with outer one so the loading state matches store writes
-        yield loader
-          .getTask(context)
-          .perform(...args)
-          .canceledOn(signal)
-      } catch (e) {
-        console.error(e)
-        const status = e?.statusCode || e?.response?.status
-        if (status === 401) {
-          // keep the load running until the auth error is handled, e.g. the token got renewed
+        // retry once after a token renewal
+        for (const isRetry of [false, true]) {
           try {
-            yield authService.handleAuthError(unref(router.currentRoute), e)
-          } catch (authError) {
-            console.error(authError)
+            // cancel inner load with outer one so the loading state matches store writes
+            yield loader
+              .getTask(context)
+              .perform(...args)
+              .canceledOn(signal)
+            return
+          } catch (e) {
+            console.error(e)
+            const status = e?.statusCode || e?.response?.status
+            if (status !== 401) {
+              // a missing resource is shown as such by the views
+              if (status !== 404) {
+                showErrorMessage({ title: $gettext('Failed to load files'), errors: [e] })
+              }
+              return
+            }
+
+            // keep the load running until the auth error is handled, e.g. the token got renewed
+            let renewed = false
+            try {
+              renewed = yield authService.handleAuthError(unref(router.currentRoute), e)
+            } catch (authError) {
+              console.error(authError)
+            }
+            if (!renewed || isRetry) {
+              return
+            }
           }
-          return
-        }
-        // a missing resource is shown as such by the views
-        if (status === 404) {
-          return
         }
-        showErrorMessage({ title: $gettext('Failed to load files'), errors: [e] })
       } finally {
         resourcesStore.finishResourcesLoading()
       }

packages/web-pkg/tests/unit/services/folder/folderService.spec.ts

@@ -22,6 +22,10 @@ import {
 } from '../../../../src/composables'
 
 describe('FolderService', () => {
+  const statusError = (statusCode: number, message = '') =>
+    Object.assign(new Error(message), { statusCode })
+  const unauthorized = () => statusError(401, 'Unauthorized')
+
   describe('resources loading state', () => {
     it('is set while the load runs and reset after it succeeds', async () => {
       const { task, resourcesStore, pendingLoads } = getWrapper()
@@ -85,7 +89,7 @@ describe('FolderService', () => {
 
       task.perform()
       await flushPromises()
-      const error = Object.assign(new Error('Internal Server Error'), { statusCode: 500 })
+      const error = statusError(500, 'Internal Server Error')
       pendingLoads[0].reject(error)
       await flushPromises()
 
@@ -108,6 +112,106 @@ describe('FolderService', () => {
     })
   })
 
+  describe('token renewal', () => {
+    beforeEach(() => {
+      vi.spyOn(console, 'error').mockImplementation(() => undefined)
+    })
+
+    it('loads the folder again after the token got renewed', async () => {
+      const { task, resourcesStore, messagesStore, authService, pendingLoads } = getWrapper()
+      authService.handleAuthError.mockResolvedValue(true)
+      const resource = mock<Resource>({ id: 'resource', name: 'resource' })
+
+      task.perform()
+      await flushPromises()
+      pendingLoads[0].reject(unauthorized())
+      await flushPromises()
+      expect(resourcesStore.resourcesLoading).toBe(true)
+
+      pendingLoads[1].resolve([resource])
+      await flushPromises()
+
+      expect(resourcesStore.resources).toEqual([resource])
+      expect(resourcesStore.resourcesLoading).toBe(false)
+      expect(authService.handleAuthError).toHaveBeenCalledTimes(1)
+      expect(messagesStore.showErrorMessage).not.toHaveBeenCalled()
+    })
+
+    it('does not load the folder again when the token did not get renewed', async () => {
+      const { task, resourcesStore, authService, pendingLoads } = getWrapper()
+      authService.handleAuthError.mockResolvedValue(false)
+
+      task.perform()
+      await flushPromises()
+      pendingLoads[0].reject(unauthorized())
+      await flushPromises()
+
+      expect(pendingLoads).toHaveLength(1)
+      expect(resourcesStore.resourcesLoading).toBe(false)
+    })
+
+    it('leaves a 401 of the retry to the auth service without loading the folder again', async () => {
+      const { task, resourcesStore, messagesStore, authService, pendingLoads } = getWrapper()
+      authService.handleAuthError.mockResolvedValue(true)
+
+      task.perform()
+      await flushPromises()
+      const firstError = unauthorized()
+      pendingLoads[0].reject(firstError)
+      await flushPromises()
+      const secondError = unauthorized()
+      pendingLoads[1].reject(secondError)
+      await flushPromises()
+
+      expect(authService.handleAuthError).toHaveBeenCalledTimes(2)
+      expect(authService.handleAuthError).toHaveBeenNthCalledWith(1, expect.anything(), firstError)
+      expect(authService.handleAuthError).toHaveBeenNthCalledWith(2, expect.anything(), secondError)
+      expect(pendingLoads).toHaveLength(2)
+      expect(resourcesStore.resourcesLoading).toBe(false)
+      expect(messagesStore.showErrorMessage).not.toHaveBeenCalled()
+    })
+
+    it.each([
+      { status: 404, errorMessages: 0 },
+      { status: 500, errorMessages: 1 }
+    ])(
+      'handles a $status of the retry like one of the first load',
+      async ({ status, errorMessages }) => {
+        const { task, messagesStore, authService, pendingLoads } = getWrapper()
+        authService.handleAuthError.mockResolvedValue(true)
+
+        task.perform()
+        await flushPromises()
+        pendingLoads[0].reject(unauthorized())
+        await flushPromises()
+        pendingLoads[1].reject(statusError(status))
+        await flushPromises()
+
+        expect(authService.handleAuthError).toHaveBeenCalledTimes(1)
+        expect(messagesStore.showErrorMessage).toHaveBeenCalledTimes(errorMessages)
+      }
+    )
+
+    it('does not retry when a new load started during the renewal', async () => {
+      const { task, authService, pendingLoads } = getWrapper()
+      let finishRenewal: (renewed: boolean) => void
+      authService.handleAuthError.mockReturnValue(
+        new Promise<boolean>((resolve) => (finishRenewal = resolve))
+      )
+
+      task.perform()
+      await flushPromises()
+      pendingLoads[0].reject(unauthorized())
+      await flushPromises()
+      task.perform()
+      await flushPromises()
+      finishRenewal(true)
+      await flushPromises()
+
+      expect(pendingLoads).toHaveLength(2)
+    })
+  })
+
   describe('overlapping loads', () => {
     beforeEach(() => {
       vi.spyOn(console, 'error').mockImplementation(() => undefined)
@@ -144,8 +248,8 @@ describe('FolderService', () => {
     })
 
     it.each([
-      { status: 500, error: Object.assign(new Error('load failed'), { statusCode: 500 }) },
-      { status: 401, error: Object.assign(new Error('Unauthorized'), { statusCode: 401 }) }
+      { status: 500, error: statusError(500, 'load failed') },
+      { status: 401, error: unauthorized() }
     ])(
       'stays silent when the older load fails with $status after the newer one finished',
       async ({ error }) => {
@@ -167,13 +271,12 @@ describe('FolderService', () => {
 
   describe('loaders', () => {
     const space = mock<SpaceResource>({ id: 'space-id', driveType: 'personal' })
-    const davError = (statusCode: number) => Object.assign(new Error(), { statusCode })
     const graphError = (status: number) => Object.assign(new Error(), { response: { status } })
 
     const loaders = [
-      { loader: new FolderLoaderSpace(), request: listFiles, getError: davError },
-      { loader: new FolderLoaderTrashbin(), request: listFiles, getError: davError },
-      { loader: new FolderLoaderFavorites(), request: search, getError: davError },
+      { loader: new FolderLoaderSpace(), request: listFiles, getError: statusError },
+      { loader: new FolderLoaderTrashbin(), request: listFiles, getError: statusError },
+      { loader: new FolderLoaderFavorites(), request: search, getError: statusError },
       { loader: new FolderLoaderSharedWithMe(), request: listSharedWithMe, getError: graphError },
       {
         loader: new FolderLoaderSharedWithOthers(),
@@ -241,7 +344,7 @@ describe('FolderService', () => {
           const { task, resourcesStore, messagesStore, clientService, authService } = getWrapper(
             new FolderLoaderSpace()
           )
-          listFiles(clientService).mockRejectedValue(davError(statusCode))
+          listFiles(clientService).mockRejectedValue(statusError(statusCode))
 
           await task.perform(space, '/folder')
 
@@ -257,9 +360,9 @@ describe('FolderService', () => {
         )
         let finishAuthHandling: () => void
         authService.handleAuthError.mockReturnValue(
-          new Promise<void>((resolve) => (finishAuthHandling = resolve))
+          new Promise<boolean>((resolve) => (finishAuthHandling = () => resolve(false)))
         )
-        listFiles(clientService).mockRejectedValue(davError(401))
+        listFiles(clientService).mockRejectedValue(statusError(401))
 
         task.perform(space, '/folder')
         await flushPromises()
@@ -276,7 +379,7 @@ describe('FolderService', () => {
           new FolderLoaderSpace()
         )
         authService.handleAuthError.mockRejectedValue(new Error('auth handling failed'))
-        listFiles(clientService).mockRejectedValue(davError(401))
+        listFiles(clientService).mockRejectedValue(statusError(401))
 
         await expect(task.perform(space, '/folder')).resolves.toBeUndefined()
 
@@ -296,7 +399,7 @@ describe('FolderService', () => {
         task.perform(space, '/old')
         task.perform(space, '/new')
         await flushPromises()
-        rejectFirstLoad(davError(401))
+        rejectFirstLoad(statusError(401))
         await flushPromises()
 
         expect(authService.handleAuthError).not.toHaveBeenCalled()

packages/web-runtime/src/services/auth/authService.ts

@@ -362,41 +362,48 @@ export class AuthService implements AuthServiceInterface {
     return '/?' + new URLSearchParams(currentQuery as Record<string, string>).toString()
   }
 
-  public async handleAuthError(route: RouteLocation, error?: unknown) {
+  /**
+   * Handles an auth error for the given route. Resolves to `true` if the token got renewed,
+   * so the caller can retry its request.
+   */
+  public async handleAuthError(route: RouteLocation, error?: unknown): Promise<boolean> {
     if (
       !this.authStore.userContextReady &&
       !isPublicLinkContextRequired(this.router, route) &&
       (isGuestContextRequired(this.router, route) || isGuestSessionInvalidError(error))
     ) {
       if (!isGuestSessionInvalidError(error)) {
-        return
+        return false
       }
       const permissionId =
         getGuestLinkError(error)?.permissionId || this.authStore.guestPermissionId
       this.guestSessionManager.expire()
       // a private link only resolves for an active guest session, anything else needs a user
       if (isGuestContextAllowed(route)) {
-        return this.router.replace({ path: '/login', query: { redirectUrl: route.fullPath } })
+        await this.router.replace({ path: '/login', query: { redirectUrl: route.fullPath } })
+        return false
       }
       if (!permissionId) {
-        return
+        return false
       }
-      return this.router.replace({
+      await this.router.replace({
         name: 'renewGuestLink',
         params: { permissionId },
         ...(isGuestContextRequired(this.router, route) && {
           query: { redirectUrl: route.fullPath }
         })
       })
+      return false
     }
     if (isPublicLinkContextRequired(this.router, route)) {
       const token = extractPublicLinkToken(route)
       this.publicLinkManager.clear(token)
-      return this.router.push({
+      await this.router.push({
         name: 'resolvePublicLink',
         params: { token },
         query: { redirectUrl: route.fullPath }
       })
+      return false
     }
     if (isUserContextRequired(this.router, route) || isIdpContextRequired(this.router, route)) {
       const user = await this.userManager.getUser()
@@ -410,17 +417,18 @@ export class AuthService implements AuthServiceInterface {
       // exhausted), so a brief interruption doesn't kill an otherwise-valid
       // session.
       if (user && (await this.renewToken())) {
-        return
+        return true
       }
 
       await this.userManager.removeUser('authError')
       this.tokenTimerWorker?.resetTokenTimer()
-      return
+      return false
     }
     // authGuard is taking care of redirecting the user to the
     // accessDenied page if hasAuthErrorOccurred is set to true
     // we can't push the route ourselves, see authGuard for details.
     this.hasAuthErrorOccurred = true
+    return false
   }
 
   /**