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.

Sign up for GitHub

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

[full-ci] Fix unloading user with oc10 backend #7128

Merged
merged 1 commit into from
Jun 15, 2022
Merged

[full-ci] Fix unloading user with oc10 backend #7128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions changelog/unreleased/bugfix-not-logged-out-if-oC10-backend
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ We've fixed an issue, where the user won't be logged out if the backend is ownCl

https://github.com/owncloud/web/pull/6939
https://github.com/owncloud/web/issues/5886
https://github.com/owncloud/web/pull/7128
35 changes: 16 additions & 19 deletions packages/web-runtime/src/store/user.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,38 +24,26 @@ const state = {
}

const actions = {
cleanUpLoginState(context, options = { clearOIDCLoginState: true }) {
cleanUpLoginState(context) {
if (context.state.id === '') {
return
}

// reset user
this.reset({ self: false, nested: false, modules: { user: { self: true } } })

// clear dynamic navItems
context.dispatch('clearDynamicNavItems')

if (options.clearOIDCLoginState) {
// clear oidc client state
vueAuthInstance.clearLoginState()
}
// clear oidc client state
vueAuthInstance.clearLoginState()
Copy link
Contributor Author

@kulmann kulmann Jun 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@dschmidt for more clarity: this function removes the user from the oidc user manager (boils down to being removed from the session storage). So it must be used unconditionally on logout (before it was only used for OIDC but not for OAuth2). The only issue was that the user unload event itself had a redirect to the login page, which was then faster than the intended redirect to the oc10 logout url which is called further down in the logout function. I moved the code that determines the logout url to the user unload event and removed it from the logout function. That's the gist of this PR.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds right!

},
async logout(context) {
const logoutFinalizer = (isOauth2 = false) => {
const logoutFinalizer = () => {
// Remove signed in user
context.dispatch('cleanUpLoginState', { clearOIDCLoginState: !isOauth2 })
context.dispatch('cleanUpLoginState')
context.dispatch('hideModal')
context.dispatch('loadSettingsValues')

// Force redirect
if (isOauth2) {
if (context.getters?.configuration?.auth?.logoutUrl) {
return (window.location = context.getters?.configuration?.auth?.logoutUrl)
} else if (context.getters?.configuration?.server) {
return (window.location = `${context.getters?.configuration?.server}/index.php/logout`)
}

router.push({ name: 'login' })
}
}
const u = await vueAuthInstance.getStoredUserObject()

Expand All @@ -73,7 +61,7 @@ const actions = {
})
} else {
// Oauth2 logout
logoutFinalizer(true)
logoutFinalizer()
}
},
async initAuth(context, payload = { autoRedirect: false }) {
Expand Down Expand Up @@ -172,6 +160,15 @@ const actions = {
vueAuthInstance.events().addUserUnloaded(() => {
console.log('user unloaded…')
context.dispatch('cleanUpLoginState')

if (context.getters?.configuration?.auth) {
// => OAuth2
if (context.getters?.configuration?.auth?.logoutUrl) {
return (window.location = context.getters?.configuration?.auth?.logoutUrl)
} else if (context.getters?.configuration?.server) {
return (window.location = `${context.getters?.configuration?.server}index.php/logout`)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also made sure that there is no double / in the logout url here. the server url already is ensured to have a trailing slash.

}
}
router.push({ name: 'login' })
})
vueAuthInstance.events().addSilentRenewError((error) => {
Expand Down