YunoHost-Apps / mumbleserver_ynh

Mumble server package for YunoHost
https://mumble.info
GNU Affero General Public License v3.0
10 stars 5 forks source link

mumble is breaking SSO for main domain #19

Closed julienmalik closed 6 years ago

julienmalik commented 6 years ago

The commit https://github.com/YunoHost-Apps/mumbleserver_ynh/commit/691b1abb774c7b927db4aced766d2e3099a2313a is breaking the sso behavior on my (jessie, 2.7.14.5) server. I don't believe this to be jessie-specific. Adding "domain" as an app setting changed ssowat behavior. I tracked it down to here : https://github.com/YunoHost/yunohost/blob/stretch-unstable/src/yunohost/app.py#L400-L408 and now my /etc/ssowat/conf.json has a "Mumble server" entry for each user pointing to the main domain, without path.

When opening https://maindomain.tld we get redirected to SSO Portal, and after successfull login, we get redirected back to https://maindomain.tld which shows the default NGINX webpage.

I don't remember how we support apps that we don't want to show on the portal. Looking at the code, the current support seems to be just that : don't set "domain" setting in the app conf, and it won't show up on the ssowat conf.

Jibec commented 6 years ago

I indeed had the same issue and assumed I did something wrong, but never found the issue! Thanks for pointing the source of this. I assume this is an issue coming from Core YunoHost. Inside Mumble, I may change the name of the variable storing the domain, but I don't like it, it feels like dirty fix.

Maybe we can do something with: "skipped_uris" "unprotected_uris" and "protected_uri"? See: https://yunohost.org/#/packaging_apps_helpers_fr

julienmalik commented 6 years ago

Waiting for a cleaner way to do this in the core, I think renaming the variable is the easiest workaround for now

Jibec commented 6 years ago

local tests says it works fine, please confirm :)

Psycojoker commented 6 years ago

Related PR https://github.com/YunoHost/yunohost/pull/507

julienmalik commented 6 years ago

The recommended way is now to set the "no_sso" app setting. I guess we should update this in mumble