From 2494d730326a64de13234a911c6548353083c9a0 Mon Sep 17 00:00:00 2001 From: Lukas Hirt Date: Mon, 4 Dec 2023 18:57:05 +0100 Subject: [PATCH] fix: skip reusing old token in embed mode with delegated auth --- .../bugfix-skip-token-reuse-in-embed-mode | 6 + .../web-runtime/src/pages/oidcCallback.vue | 8 +- .../src/services/auth/authService.ts | 14 ++ .../unit/services/auth/authService.spec.ts | 129 +++++++++++++++++- 4 files changed, 155 insertions(+), 2 deletions(-) create mode 100644 changelog/unreleased/bugfix-skip-token-reuse-in-embed-mode diff --git a/changelog/unreleased/bugfix-skip-token-reuse-in-embed-mode b/changelog/unreleased/bugfix-skip-token-reuse-in-embed-mode new file mode 100644 index 00000000000..5d1d84bc293 --- /dev/null +++ b/changelog/unreleased/bugfix-skip-token-reuse-in-embed-mode @@ -0,0 +1,6 @@ +Bugfix: Skip reusing saved token in Embed mode with delegated authentication + +We've stopped reusing saved token in the AuthService when Web is included in Embed mode with delegated authentication. +The parent application is responsible for handling the access_token. + +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 index 8b74783b476..6e251cc3a79 100644 --- 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.info( + '[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('owncloud-embed:request-token') + console.info('[page:oidcCallback:hook:mounted] - adding update-token event listener') window.addEventListener('message', handleRequestedTokenEvent) + console.info('[page:oidcCallback:hook:mounted] - requesting delegated access_token') + postMessage('owncloud-embed:request-token') return } @@ -78,6 +83,7 @@ export default defineComponent({ return } + console.info('[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 index 7deb6cfbe60..394d6d54725 100644 --- 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.info('[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.info('[authService:signInCallback] - setting access_token and fetching user') await this.userManager.updateContext(accessToken, true) // Setup a listener to handle token refresh + console.info('[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.info('[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 index 7e19a490f8e..26cd9a0993d 100644 --- 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({ + getAccessToken: jest.fn().mockResolvedValue('access-token'), + updateContext: mockUpdateContext + }) + ) + + configurationManager.initialize({ + server: 'http://server/address/', + options: { embed: { enabled: false } } + }) + authService.initialize(configurationManager, null, mock>({}), null, null, null) + + await authService.initializeContext(mock({})) + + 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({ + getAccessToken: jest.fn().mockResolvedValue(null), + updateContext: mockUpdateContext + }) + ) + + configurationManager.initialize({ + server: 'http://server/address/', + options: { embed: { enabled: false } } + }) + authService.initialize(configurationManager, null, mock>({}), null, null, null) + + await authService.initializeContext(mock({})) + + 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({ + 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>({}), null, null, null) + + await authService.initializeContext(mock({})) + + 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({ + 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>({}), null, null, null) + + await authService.initializeContext(mock({})) + + 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({ + 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>({}), null, null, null) + + await authService.initializeContext(mock({})) + + expect(mockUpdateContext).toHaveBeenCalledWith('access-token', true) + }) + }) })