fix: skip reusing old token in embed mode with delegated auth (#10113)

* fix: skip reusing old token in embed mode with delegated auth --------- Co-authored-by: Benedikt Kulmann <[email protected]>
owncloud · Dec 13, 2023 · 04e7495 · 04e7495
1 parent 07d270b
commit 04e7495
Show file tree

Hide file tree

Showing 4 changed files with 151 additions and 3 deletions.
diff --git a/changelog/unreleased/enhancement-add-auth-delegation b/changelog/unreleased/enhancement-add-auth-delegation
@@ -3,5 +3,6 @@ Enhancement: Add authentication delegation in the Embed mode
 We've added authentication delegation so that the user does not need to reauthenticate when the parent application already holds a
 valid access token for the user.
 
-https://github.com/owncloud/web/pull/10082
 https://github.com/owncloud/web/issues/10072
+https://github.com/owncloud/web/pull/10082
+https://github.com/owncloud/web/pull/10113
diff --git a/packages/web-runtime/src/pages/oidcCallback.vue b/packages/web-runtime/src/pages/oidcCallback.vue
@@ -47,6 +47,9 @@ export default defineComponent({
         return
       }
 
+      console.debug(
+        '[page:oidcCallback:handleRequestedTokenEvent] - received delegated access_token'
+      )
       authService.signInCallback(event.data.data.access_token)
     }
 
@@ -60,8 +63,10 @@ export default defineComponent({
       }
 
       if (unref(isDelegatingAuthentication)) {
-        postMessage<void>('owncloud-embed:request-token')
+        console.debug('[page:oidcCallback:hook:mounted] - adding update-token event listener')
         window.addEventListener('message', handleRequestedTokenEvent)
+        console.debug('[page:oidcCallback:hook:mounted] - requesting delegated access_token')
+        postMessage<void>('owncloud-embed:request-token')
 
         return
       }
@@ -78,6 +83,7 @@ export default defineComponent({
         return
       }
 
+      console.debug('[page:oidcCallback:hook:beforeUnmount] - removing update-token event listener')
       window.removeEventListener('message', handleRequestedTokenEvent)
     })
 

diff --git a/packages/web-runtime/src/services/auth/authService.ts b/packages/web-runtime/src/services/auth/authService.ts
@@ -161,10 +161,21 @@ export class AuthService {
         this.userManager.areEventHandlersRegistered = true
       }
 
+      // This is to prevent issues in embed mode when the expired token is still saved but already expired
+      // If the following code gets executed, it would toggle errorOccurred var which would then lead to redirect to the access denied screen
+      if (
+        this.configurationManager.options.embed?.enabled &&
+        this.configurationManager.options.embed.delegateAuthentication
+      ) {
+        return
+      }
+
       // relevant for page reload: token is already in userStore
       // no userLoaded event and no signInCallback gets triggered
       const accessToken = await this.userManager.getAccessToken()
       if (accessToken) {
+        console.debug('[authService:initializeContext] - updating context with saved access_token')
+
         try {
           await this.userManager.updateContext(accessToken, fetchUserData)
         } catch (e) {
@@ -190,9 +201,11 @@ export class AuthService {
         this.configurationManager.options.embed.delegateAuthentication &&
         accessToken
       ) {
+        console.debug('[authService:signInCallback] - setting access_token and fetching user')
         await this.userManager.updateContext(accessToken, true)
 
         // Setup a listener to handle token refresh
+        console.debug('[authService:signInCallback] - adding listener to update-token event')
         window.addEventListener('message', this.handleDelegatedTokenUpdate)
       } else {
         await this.userManager.signinRedirectCallback(this.buildSignInCallbackUrl())
@@ -303,6 +316,7 @@ export class AuthService {
       return
     }
 
+    console.debug('[authService:handleDelegatedTokenUpdate] - going to update the access_token')
     this.userManager.updateContext(event.data, false)
   }
 }

diff --git a/packages/web-runtime/tests/unit/services/auth/authService.spec.ts b/packages/web-runtime/tests/unit/services/auth/authService.spec.ts
@@ -1,6 +1,11 @@
 import { ConfigurationManager } from '@ownclouders/web-pkg'
+import { mock } from 'jest-mock-extended'
+import { Store } from 'vuex'
 import { AuthService } from 'web-runtime/src/services/auth/authService'
-import { createRouter } from 'web-test-helpers/src'
+import { UserManager } from 'web-runtime/src/services/auth/userManager'
+import { RouteLocation, createRouter } from 'web-test-helpers/src'
+
+const mockUpdateContext = jest.fn()
 
 describe('AuthService', () => {
   describe('signInCallback', () => {
@@ -45,4 +50,126 @@ describe('AuthService', () => {
       }
     )
   })
+
+  describe('initializeContext', () => {
+    it('when embed mode is disabled and access_token is present, should call updateContext', async () => {
+      const authService = new AuthService()
+      const configurationManager = new ConfigurationManager()
+
+      jest.replaceProperty(
+        authService as any,
+        'userManager',
+        mock<UserManager>({
+          getAccessToken: jest.fn().mockResolvedValue('access-token'),
+          updateContext: mockUpdateContext
+        })
+      )
+
+      configurationManager.initialize({
+        server: 'http://server/address/',
+        options: { embed: { enabled: false } }
+      })
+      authService.initialize(configurationManager, null, mock<Store<any>>({}), null, null, null)
+
+      await authService.initializeContext(mock<RouteLocation>({}))
+
+      expect(mockUpdateContext).toHaveBeenCalledWith('access-token', true)
+    })
+
+    it('when embed mode is disabled and access_token is not present, should not call updateContext', async () => {
+      const authService = new AuthService()
+      const configurationManager = new ConfigurationManager()
+
+      jest.replaceProperty(
+        authService as any,
+        'userManager',
+        mock<UserManager>({
+          getAccessToken: jest.fn().mockResolvedValue(null),
+          updateContext: mockUpdateContext
+        })
+      )
+
+      configurationManager.initialize({
+        server: 'http://server/address/',
+        options: { embed: { enabled: false } }
+      })
+      authService.initialize(configurationManager, null, mock<Store<any>>({}), null, null, null)
+
+      await authService.initializeContext(mock<RouteLocation>({}))
+
+      expect(mockUpdateContext).not.toHaveBeenCalled()
+    })
+
+    it('when embed mode is enabled, access_token is present but auth is not delegated, should call updateContext', async () => {
+      const authService = new AuthService()
+      const configurationManager = new ConfigurationManager()
+
+      jest.replaceProperty(
+        authService as any,
+        'userManager',
+        mock<UserManager>({
+          getAccessToken: jest.fn().mockResolvedValue('access-token'),
+          updateContext: mockUpdateContext
+        })
+      )
+
+      configurationManager.initialize({
+        server: 'http://server/address/',
+        options: { embed: { enabled: true, delegateAuthentication: false } }
+      })
+      authService.initialize(configurationManager, null, mock<Store<any>>({}), null, null, null)
+
+      await authService.initializeContext(mock<RouteLocation>({}))
+
+      expect(mockUpdateContext).toHaveBeenCalledWith('access-token', true)
+    })
+
+    it('when embed mode is enabled, access_token is present and auth is delegated, should not call updateContext', async () => {
+      const authService = new AuthService()
+      const configurationManager = new ConfigurationManager()
+
+      jest.replaceProperty(
+        authService as any,
+        'userManager',
+        mock<UserManager>({
+          getAccessToken: jest.fn().mockResolvedValue('access-token'),
+          updateContext: mockUpdateContext
+        })
+      )
+
+      configurationManager.initialize({
+        server: 'http://server/address/',
+        options: { embed: { enabled: true, delegateAuthentication: true } }
+      })
+      authService.initialize(configurationManager, null, mock<Store<any>>({}), null, null, null)
+
+      await authService.initializeContext(mock<RouteLocation>({}))
+
+      expect(mockUpdateContext).not.toHaveBeenCalled()
+    })
+
+    it('when embed mode is disabled, access_token is present and auth is delegated, should call updateContext', async () => {
+      const authService = new AuthService()
+      const configurationManager = new ConfigurationManager()
+
+      jest.replaceProperty(
+        authService as any,
+        'userManager',
+        mock<UserManager>({
+          getAccessToken: jest.fn().mockResolvedValue('access-token'),
+          updateContext: mockUpdateContext
+        })
+      )
+
+      configurationManager.initialize({
+        server: 'http://server/address/',
+        options: { embed: { enabled: false, delegateAuthentication: true } }
+      })
+      authService.initialize(configurationManager, null, mock<Store<any>>({}), null, null, null)
+
+      await authService.initializeContext(mock<RouteLocation>({}))
+
+      expect(mockUpdateContext).toHaveBeenCalledWith('access-token', true)
+    })
+  })
 })