[fix] redirect logout - Githubissues

YunoHost-Apps / nextcloud_ynh

Nextcloud package for YunoHost

https://nextcloud.com/

GNU Affero General Public License v3.0

147 stars 63 forks source link

[fix] redirect logout #636

Closed zamentur closed 5 months ago

zamentur commented 10 months ago

Problem

Logout on sso was broken and don't return on the nextcloud login form.

Solution

Fix all the things and logout on sso only if it's a ldap user (it's possible to create internal nextcloud user side to the ldap users...)

PR Status

[x] Code finished and ready to be reviewed/tested
[x] The fix/enhancement were manually tested (if applicable)

Automatic tests

Automatic tests can be triggered on https://ci-apps-dev.yunohost.org/ after creating the PR, by commenting "!testme", "!gogogadgetoci" or "By the power of systemd, I invoke The Great App CI to test this Pull Request!". (N.B. : for this to work you need to be a member of the Yunohost-Apps organization)

ericgaspar commented 10 months ago

!testme

yunohost-bot commented 10 months ago

:stuck_out_tongue_winking_eye:

ericgaspar commented 10 months ago

Any way to test this? I have upgraded my server with this PR but can't find any noticable difference.

zamentur commented 10 months ago

Any way to test this? I have upgraded my server with this PR but can't find any noticable difference.

What do you mean exactly ?

With this PR, when i click on nextcloud logout:

If i am a LDAP user
- I am logout from YunoHost SSO
- I am logout from nextcloud
- I am redirected on nextcloud login form (but this could be configured with logout_url...)
If i am a nextcloud internal user:
- I am logout from nextcloud
- I am redirected on nextcloud login form

Without this PR, but with the Testing PR instead:

the logout patch failed (see https://github.com/YunoHost/issues/issues/2296 )
logout from nextcloud doesn't work at all, if you are logged in with the SSO...

ericgaspar commented 10 months ago

During my test, when connected to Nextcloud with LDAP user, when I disconnect from Nextcloud there is no effect, I stay on Nextcloud page. The patch should be reapplied during upgrade right?

zamentur commented 10 months ago

I stay on Nextcloud page

without being logged out ?

ericgaspar commented 10 months ago

I stay on Nextcloud page

without being logged out ?

Yes, without being logged out.

https://github.com/YunoHost-Apps/nextcloud_ynh/assets/46165813/d18ae263-17fd-4589-949d-f256a4095c52

zamentur commented 10 months ago

Now it works if we are in UPGRADE_APP mode, but we probably should be better.

ericgaspar commented 10 months ago

I have upgraded with latest changes but still have the same behaviors as before. What do you mean by UPGRADE_APP mode?

zamentur commented 10 months ago

UPGRADE_APP mode is triggered if the sources change or if we run the upgrade with --force.

zamentur commented 10 months ago

https://github.com/YunoHost/yunohost/blob/dev/src/app.py#L669

ericgaspar commented 10 months ago

@zamentur Is it working on your side?

zamentur commented 9 months ago

[x] Fresh install with subpath: yes
[ ] Upgrade with subpath: no (The upgrade work only if you upgrade from a smaller upstream version :/ )

zamentur commented 9 months ago

Note that 27.1.5rc1 is on the road...

ericgaspar commented 9 months ago

strangely #644 PR has solved my redirect logout issue

zamentur commented 9 months ago

I know we have a oldstable branch, however, i think we probably should avoid to upgrade on recent new major version, there are always bugs with X.0.x version...

lapineige commented 9 months ago

In that case we are already in version 28.0.1

ericgaspar commented 8 months ago

!testme

yunohost-bot commented 8 months ago

:carousel_horse: