I don’t really understand why it was disabled (from the french [Mobilizon] Souci pour se connecter), but at least I don’t see any such errors in the log even after manually re-enabling the protection with:
Shouldn’t the package rather re-anable it, by now?
At least the remaining problem with mobilizon logins not working while logged into the yunohost SSO was not fixed by opening that security hole (it still works/don’t works the same).
ERROR Could not find https://github.com/YunoHost-Apps/mobilizon_ynh/tree/test_spoof in the list of installed apps:
- mobilizon
Guess upgrading would probably need to actively switch the protection ‘true’, though, or ideally do some kind of “migration” to unset the setting (yunohost app setting mobilizon protect_against_basic_auth_spoofing -d) if it currently is ‘false’ (e.g. from an override by previous packages) to use the defaut ‘true’.
Thanks! I had attempted to use yunohost upgrade in the same way as install and thus without the mobilizon -u part.
Today, with your fourth commit, the upgrade did switch the yunohost app setting mobilizon protect_against_basic_auth_spoofing from false to “forced” true. Maybe just simply deleting any override (-d on the CLI) could also work for the upgrade.
Found another source location that seems to suggest the protection setting is a bit of a misnomer, it’t seems not just about spoofing but simply disables all basic auth usage, and thus actually has to be set to ‘false’ to enable basic auth for apps that need it, e.g. for webdav or webcal clients connecting to “apps_that_need_external_auth_maybe” (yunohost/src/app.py at fc1e312f61cd4a4f416e9c0f08c2dbf6686d5cf0 · YunoHost/yunohost · GitHub).
Guess Mobilizon does not require basic auth, as it provides just simple ICS/WebCal downloads?
Seems apps that could need it should be added to above apps_that_need_external_auth_maybe to get a reasonable default for them and define an explicit install question in the manifest to disallow basic auth clients.
EDIT: But then, why this central override list complication?
Can’t just app config questions cover this?
Or was the intention to centrally generate a config question?
not sure if this would work in the manifest.toml:
(if mobilizon really has a need? webdav?)
[install.protect_against_basic_auth_spoofing]
ask.en = "Block all basic auth comming from clients (not SSO generated) ?"
help.en = "May have to be disabled for WebDav/CalDav clients, not for "randomized" ICS/webcal download links."
type = "bool"
default = nil
I think just stoping to set the override with the packages scripts and deleting any current override with the upgrade script for a while (to rely on the default “true”), would better avoid conflicts with any later custom admin settings (after removing the override delete from the upgrade script one or two upgrades later):
Well the current changes force-set protect_against_basic_auth_spoofing to true, instead of “just” (see below) removing the setting.
I also think it may not be disirable to just (silently) remove the # Migrate data to data_dir code within:
And isn’t that rather doing “removing the not yet functioning SSO”, not fixing any auth?
Same as with removing the old protect_against_basic_auth_spoofing change, all machines that are off/offline for longer and only upgraded or restored later will still be in need of that change. (It needlessly breaks the upgrade path.)
So, therefore also the my question above if apps do also have some migration helpers. Otherwise maybe just keep migration code in a separate file to run from upgrade and restore?
The problem I thought of when deleting the current protect_against_basic_auth_spoofing setting is that the app shouldn’t just keep deleting it on every update/restore, because that would prevent admins to keep the settings configured as desired with their local override.
That was the reason for the idea to only let the app actively remove the setting (that it once forced) for a short time, and then stop that as well. But another, better idea may be to just “touch” a file within the app dir to “take note” after successfully removing the setting, and not remove it a second time if the file is present.
Was this the output when you ran
Congratulations, think you can remove this now: