Skip to content

Commit

Permalink
fix: skip reusing old token in embed mode with delegated auth
Browse files Browse the repository at this point in the history
  • Loading branch information
LukasHirt committed Dec 4, 2023
1 parent 9d40302 commit 2494d73
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 2 deletions.
6 changes: 6 additions & 0 deletions changelog/unreleased/bugfix-skip-token-reuse-in-embed-mode
Original file line number Diff line number Diff line change
@@ -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
8 changes: 7 additions & 1 deletion packages/web-runtime/src/pages/oidcCallback.vue
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ export default defineComponent({
return
}
console.info(
'[page:oidcCallback:handleRequestedTokenEvent] - received delegated access_token'
)
authService.signInCallback(event.data.data.access_token)
}
Expand All @@ -60,8 +63,10 @@ export default defineComponent({
}
if (unref(isDelegatingAuthentication)) {
postMessage<void>('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<void>('owncloud-embed:request-token')
return
}
Expand All @@ -78,6 +83,7 @@ export default defineComponent({
return
}
console.info('[page:oidcCallback:hook:beforeUnmount] - removing update-token event listener')
window.removeEventListener('message', handleRequestedTokenEvent)
})
Expand Down
14 changes: 14 additions & 0 deletions packages/web-runtime/src/services/auth/authService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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())
Expand Down Expand Up @@ -303,6 +316,7 @@ export class AuthService {
return
}

console.info('[authService:handleDelegatedTokenUpdate] - going to update the access_token')
this.userManager.updateContext(event.data, false)
}
}
Expand Down
129 changes: 128 additions & 1 deletion packages/web-runtime/tests/unit/services/auth/authService.spec.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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)
})
})
})

0 comments on commit 2494d73

Please sign in to comment.