-
Notifications
You must be signed in to change notification settings - Fork 168
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
[full-ci] Fix unloading user with oc10 backend #7128
Conversation
This has essentially been backported from the in-progress PR #7072 which will not be ready for the upcoming release. |
@dschmidt having you as reviewer here would be awesome, since you're deep down in that rabbit hole already. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks okay, but really hard to tell 🙈
Results for oCISSharingPublic2 https://drone.owncloud.com/owncloud/web/26243/69/1 |
vueAuthInstance.clearLoginState() | ||
} | ||
// clear oidc client state | ||
vueAuthInstance.clearLoginState() |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds right!
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`) |
There was a problem hiding this comment.
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.
Kudos, SonarCloud Quality Gate passed! |
Description
While already being redirected to the logout URL of oc10 (killing the server side session) we had an intermediate issue of the user in oC Web not being unloaded. As a result, upon next login we still had the previous user available from the session storage. Fixed by not suppressing the user removal inside the oidc-client anymore, but instead deciding inside the
userUnloaded
event if we are in an oauth2 context and then redirect properly.Related Issue
Motivation and Context
Harden the oc10 integration.
Types of changes
Checklist: