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: remove user locally if no logout url in IdP #10974

Merged
merged 3 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
8 changes: 8 additions & 0 deletions changelog/unreleased/bugfix-local-logout
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Bugfix: Local logout if IdP has no logout support

Some IdPs don't support a logout endpoint. In those cases the web UI ran into a fatal error an showed an empty screen without
kulmann marked this conversation as resolved.
Show resolved Hide resolved
further redirects. Fixed by forgetting the currently authenticated user when the OpenID Connect configuration doesn't contain
an `endSessionEndpoint` url.

https://github.com/owncloud/web/pull/10974
https://github.com/owncloud/enterprise/issues/6631
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { useService } from '../service'
import { NavigationFailure } from 'vue-router'

export interface AuthServiceInterface {
handleAuthError(route: any): any
signinSilent(): Promise<unknown>
logoutUser(): Promise<void | NavigationFailure>
}

export const useAuthService = (): AuthServiceInterface => {
Expand Down
16 changes: 8 additions & 8 deletions packages/web-runtime/src/components/Topbar/UserMenu.vue
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,13 @@
import { storeToRefs } from 'pinia'
import { defineComponent, PropType, ComponentPublicInstance, computed, unref } from 'vue'
import { filesize } from 'filesize'
import { authService } from '../../services/auth'
import {
useRoute,
useSpacesStore,
useThemeStore,
useUserStore,
routeToContextQuery
routeToContextQuery,
useAuthService
} from '@ownclouders/web-pkg'
import { OcDrop } from 'design-system/src/components'
import { MenuItem } from '../../helpers/menuItems'
Expand All @@ -167,6 +167,7 @@ export default defineComponent({
const userStore = useUserStore()
const themeStore = useThemeStore()
const spacesStore = useSpacesStore()
const authService = useAuthService()

const { user } = storeToRefs(userStore)

Expand All @@ -181,6 +182,9 @@ export default defineComponent({
query: { redirectUrl: unref(route).fullPath }
}
})
const logout = () => {
authService.logoutUser()
}

const imprintUrl = computed(() => themeStore.currentTheme.common.urls.imprint)
const privacyUrl = computed(() => themeStore.currentTheme.common.urls.privacy)
Expand All @@ -195,7 +199,8 @@ export default defineComponent({
loginLink,
imprintUrl,
privacyUrl,
quota
quota,
logout
}
},
computed: {
Expand Down Expand Up @@ -249,11 +254,6 @@ export default defineComponent({
onShown: () =>
(this.$refs.menu as ComponentPublicInstance).$el.querySelector('a:first-of-type').focus()
})
},
methods: {
logout() {
authService.logoutUser()
}
}
})
</script>
Expand Down
1 change: 0 additions & 1 deletion packages/web-runtime/src/pages/accessDenied.vue
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ export default defineComponent({
}
}
return {
name: 'login',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea why we had the name key here... maybe just by mistake and it was really meant for the to route?!

type: 'router-link',
to: {
name: 'login',
Expand Down
68 changes: 47 additions & 21 deletions packages/web-runtime/src/pages/logout.vue
Original file line number Diff line number Diff line change
@@ -1,42 +1,66 @@
<template>
<div class="oc-login-card oc-position-center">
<img class="oc-login-logo" :src="logoImg" alt="" :aria-hidden="true" />
<div class="oc-login-card-body oc-width-medium">
<h2 class="oc-login-card-title" v-text="cardTitle" />
<p v-text="cardHint" />
</div>
<div class="oc-login-card-footer oc-pt-rm">
<p>
{{ footerSlogan }}
</p>
<div class="oc-height-viewport oc-flex oc-flex-column oc-flex-center oc-flex-middle">
<div class="oc-login-card">
<img class="oc-login-logo" :src="logoImg" alt="" :aria-hidden="true" />
<div class="oc-login-card-body oc-width-medium">
<h2 class="oc-login-card-title" v-text="cardTitle" />
<p v-text="cardHint" />
</div>
<div class="oc-login-card-footer oc-pt-rm">
<p>
{{ footerSlogan }}
</p>
</div>
</div>
<oc-button
id="exitAnchor"
class="oc-mt-m oc-width-medium"
size="large"
appearance="filled"
variation="primary"
v-bind="loginButtonAttrs"
>
{{ loginButtonText }}
</oc-button>
</div>
</template>
<script lang="ts">
import { computed, defineComponent } from 'vue'
import { useRouter, useThemeStore } from '@ownclouders/web-pkg'
import { authService } from 'web-runtime/src/services/auth'
import { useConfigStore, useThemeStore } from '@ownclouders/web-pkg'
import { useGettext } from 'vue3-gettext'
import { storeToRefs } from 'pinia'

export default defineComponent({
name: 'LogoutPage',
setup() {
const router = useRouter()
const { $gettext } = useGettext()
const themeStore = useThemeStore()

const { currentTheme } = storeToRefs(themeStore)

authService.logoutUser().then(() => {
router.push({ name: 'login' })
})
const configStore = useConfigStore()

const cardTitle = computed(() => {
return $gettext('Logout in progress')
return $gettext('Logged out')
})
const cardHint = computed(() => {
return $gettext("Please wait while you're being logged out.")
return $gettext('You have been logged out successfully.')
})
const loginButtonText = computed(() => {
return $gettext('Log in again')
})
const loginButtonAttrs = computed(() => {
if (configStore.options.loginUrl) {
const configLoginURL = new URL(encodeURI(configStore.options.loginUrl))
return {
type: 'a',
href: configLoginURL.toString()
}
}
return {
type: 'router-link',
to: {
name: 'login'
}
}
})

const footerSlogan = computed(() => currentTheme.value.common.slogan)
Expand All @@ -46,7 +70,9 @@ export default defineComponent({
logoImg,
cardTitle,
cardHint,
footerSlogan
footerSlogan,
loginButtonText,
loginButtonAttrs
}
}
})
Expand Down
22 changes: 14 additions & 8 deletions packages/web-runtime/src/services/auth/authService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import {
UserStore,
CapabilityStore,
ConfigStore,
useTokenTimerWorker
useTokenTimerWorker,
AuthServiceInterface
} from '@ownclouders/web-pkg'
import { RouteLocation, Router } from 'vue-router'
import {
Expand All @@ -22,7 +23,7 @@ import { Language } from 'vue3-gettext'
import { PublicLinkType } from '@ownclouders/web-client'
import { WebWorkersStore } from '@ownclouders/web-pkg'

export class AuthService {
export class AuthService implements AuthServiceInterface {
private clientService: ClientService
private configStore: ConfigStore
private router: Router
Expand Down Expand Up @@ -151,7 +152,7 @@ export class AuthService {
this.userManager.events.addUserUnloaded(async () => {
console.log('user unloaded…')
this.tokenTimerWorker?.resetTokenTimer()
await this.resetStateAfterUserLogout()
this.resetStateAfterUserLogout()

if (this.userManager.unloadReason === 'authError') {
this.hasAuthErrorOccurred = true
Expand All @@ -167,7 +168,6 @@ export class AuthService {
if (oAuth2.logoutUrl) {
return (window.location = oAuth2.logoutUrl as any)
}
return (window.location = `${this.configStore.serverUrl}/index.php/logout` as any)
}
})
this.userManager.events.addSilentRenewError(async (error) => {
Expand Down Expand Up @@ -314,12 +314,18 @@ export class AuthService {
}

public async logoutUser() {
const endSessionEndpoint = await this.userManager.metadataService?.getEndSessionEndpoint()
if (!endSessionEndpoint) {
await this.userManager.removeUser()
return this.router.push({ name: 'logout' })
}

const u = await this.userManager.getUser()
if (u && u.id_token) {
return this.userManager.signoutRedirect({ id_token_hint: u.id_token })
} else {
await this.userManager.removeUser()
}

return await this.userManager.removeUser()
}

private resetStateAfterUserLogout() {
Expand All @@ -328,7 +334,7 @@ export class AuthService {
this.authStore.clearUserContext()
}

private handleDelegatedTokenUpdate(event: MessageEvent): void {
private handleDelegatedTokenUpdate(event: MessageEvent) {
if (
this.configStore.options.embed?.delegateAuthenticationOrigin &&
event.origin !== this.configStore.options.embed.delegateAuthenticationOrigin
Expand All @@ -341,7 +347,7 @@ export class AuthService {
}

console.debug('[authService:handleDelegatedTokenUpdate] - going to update the access_token')
this.userManager.updateContext(event.data, false)
return this.userManager.updateContext(event.data, false)
}
}

Expand Down
2 changes: 0 additions & 2 deletions packages/web-runtime/src/services/auth/userManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ export interface UserManagerOptions {
}

export class UserManager extends OidcUserManager {
private readonly storePrefix
private clientService: ClientService
private configStore: ConfigStore
private userStore: UserStore
Expand Down Expand Up @@ -111,7 +110,6 @@ export class UserManager extends OidcUserManager {
Log.setLevel(Log.WARN)

super(openIdConfig)
this.storePrefix = storePrefix
this.browserStorage = browserStorage
this.clientService = options.clientService
this.configStore = options.configStore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ exports[`access denied page > renders component 1`] = `
<div class="oc-login-card-footer oc-pt-rm">
<p>ownCloud – A safe home for all your data</p>
</div>
</div> <a attrs="[object Object]" class="oc-button oc-rounded oc-button-l oc-button-justify-content-center oc-button-gap-m oc-button-primary oc-button-primary-filled oc-mt-m oc-width-medium" id="exitAnchor" name="login"></a>
</div> <a attrs="[object Object]" class="oc-button oc-rounded oc-button-l oc-button-justify-content-center oc-button-gap-m oc-button-primary oc-button-primary-filled oc-mt-m oc-width-medium" id="exitAnchor"></a>
</div>"
`;