Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion changelog/unreleased/enhancement-add-auth-delegation
Original file line number Diff line number Diff line change
Expand Up @@ -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
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.debug(
'[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.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
}
Expand All @@ -78,6 +83,7 @@ export default defineComponent({
return
}
console.debug('[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.debug('[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.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())
Expand Down Expand Up @@ -303,6 +316,7 @@ export class AuthService {
return
}

console.debug('[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)
})
})
})