search

YunoHost-Apps / aeneria_ynh

æneria package for YunoHost
GNU Affero General Public License v3.0
10 stars 1 forks source link

Bug with admin_legacy user on YunoHost 11.1 #33

Closed tituspijean closed 1 year ago

tituspijean commented 1 year ago

While trying an upgrade with #32 on YunoHost 11.1 (with admins group created and creation of a admin_legacy user for the old non-LDAP admin account), I ended up with a failure.

2023-01-04 12:11:44,545: INFO - [#############++.....] > Upgrading aeneria...
2023-01-04 12:11:44,546: DEBUG - ++ whoami
2023-01-04 12:11:44,549: DEBUG - + [[ aeneria = root ]]
2023-01-04 12:11:44,549: DEBUG - + sudo -u aeneria php7.4 bin/console cache:clear -n
2023-01-04 12:11:45,677: DEBUG - 
2023-01-04 12:11:45,678: DEBUG -  // Clearing the cache for the prod environment with debug false
2023-01-04 12:11:45,678: DEBUG - 
2023-01-04 12:11:47,042: DEBUG -  [OK] Cache for the "prod" environment (debug=false) was successfully cleared.
2023-01-04 12:11:47,042: DEBUG - 
2023-01-04 12:11:47,066: DEBUG - + ynh_exec_as aeneria php7.4 bin/console doctrine:migrations:migrate -n
2023-01-04 12:11:47,066: DEBUG - + local user=aeneria
2023-01-04 12:11:47,066: DEBUG - + shift 1
2023-01-04 12:11:47,066: DEBUG - ++ whoami
2023-01-04 12:11:47,068: DEBUG - + [[ aeneria = root ]]
2023-01-04 12:11:47,069: DEBUG - + sudo -u aeneria php7.4 bin/console doctrine:migrations:migrate -n
2023-01-04 12:11:47,362: DEBUG - 
2023-01-04 12:11:47,362: DEBUG -  [OK] Already at the latest version ("App\Migrations\Version20210618095703")
2023-01-04 12:11:47,363: DEBUG - 
2023-01-04 12:11:47,379: DEBUG - ++ ynh_user_list
2023-01-04 12:11:47,380: DEBUG - ++ jq -r '.users | keys[]'
2023-01-04 12:11:47,383: DEBUG - ++ yunohost user list --output-as json --quiet
2023-01-04 12:11:47,810: DEBUG - + for username in $(ynh_user_list)
2023-01-04 12:11:47,811: DEBUG - ++ ynh_user_get_info --username=admin --key=mail
2023-01-04 12:11:47,861: DEBUG - ++ jq -r .mail
2023-01-04 12:11:47,865: DEBUG - ++ yunohost user info admin --output-as json --quiet
2023-01-04 12:11:48,834: DEBUG - + mail=admin_legacy
2023-01-04 12:11:48,834: DEBUG - ++ ynh_exec_as aeneria php7.4 bin/console aeneria:user:exist admin_legacy
2023-01-04 12:11:48,835: DEBUG - ++ local user=aeneria
2023-01-04 12:11:48,835: DEBUG - ++ shift 1
2023-01-04 12:11:48,836: DEBUG - +++ whoami
2023-01-04 12:11:48,838: DEBUG - ++ [[ aeneria = root ]]
2023-01-04 12:11:48,838: DEBUG - ++ sudo -u aeneria php7.4 bin/console aeneria:user:exist admin_legacy
2023-01-04 12:11:49,109: DEBUG - + user_exists=0
2023-01-04 12:11:49,111: DEBUG - ++ ynh_string_random
2023-01-04 12:11:49,112: DEBUG - ++ length=24
2023-01-04 12:11:49,113: DEBUG - ++ filter=A-Za-z0-9
2023-01-04 12:11:49,114: DEBUG - ++ sed --quiet 's/\(.\{24\}\).*/\1/p'
2023-01-04 12:11:49,116: DEBUG - ++ tr --complement --delete A-Za-z0-9
2023-01-04 12:11:49,117: DEBUG - ++ dd if=/dev/urandom bs=1 count=1000
2023-01-04 12:11:49,127: DEBUG - + user_pass=**********
2023-01-04 12:11:49,127: DEBUG - + ynh_exec_as aeneria php7.4 bin/console aeneria:user:add admin_legacy ********** -n
2023-01-04 12:11:49,127: DEBUG - + local user=aeneria
2023-01-04 12:11:49,127: DEBUG - + shift 1
2023-01-04 12:11:49,128: DEBUG - ++ whoami
2023-01-04 12:11:49,130: DEBUG - + [[ aeneria = root ]]
2023-01-04 12:11:49,130: DEBUG - + sudo -u aeneria php7.4 bin/console aeneria:user:add admin_legacy ********** -n
2023-01-04 12:11:49,363: DEBUG - 
2023-01-04 12:11:49,363: DEBUG -  [ERROR] admin_legacy is not a valid email

https://paste.yunohost.org/raw/uvefifayel

tituspijean commented 1 year ago

I am a bit worried by this piece of code:

https://github.com/YunoHost-Apps/aeneria_ynh/blob/39ac05428adcb09164d6bfcb004ed63dcf2d68db/scripts/upgrade#L162-L171

Is its aim to add all YunoHost users in Aeneria's user database?

At minimum it should exclude admin_legacy, since it will not have an email address. At best, we should rather have a hook that creates/deletes users when they are given/removed the aeneria.main permission, instead of looping over all the users. I somehow did not even check if the hooks existed sorry...

Is its aim to add all YunoHost users in Aeneria's user database?

Looks like it, from my own tests:

# ynh_exec_as $app php$phpversion bin/console aeneria:user:exist "real.email@domain.test"
1
# ynh_exec_as $app php$phpversion bin/console aeneria:user:exist "fake.email@domain.invalid"
0
SimonMellerin commented 1 year ago

Is its aim to add all YunoHost users in Aeneria's user database?

Yes that's it.

It was a way to easily synchronize æneria users with ynh users.

Normally, it should not be useful, the hook should do the job, I can't remember why I added this piece of code. May be for retro-compatibility issue.

Do you know if there is a better way to handle this kind of issue (syncro between ynh users and in-app users)?

arthurlutz commented 1 year ago

Go hit by this bug trying to install aeneria. Is there a way to go around this problem ?

sereinity commented 1 year ago

Also got hit by the 1.1.6~ynh3 release

tituspijean commented 1 year ago

We should rather rely on a post_app_addaccess hook to trigger aforementioned code and on-demand add users in Aeneria's database. Add a loop if the access is given to a group.

tituspijean commented 1 year ago

@SimonMellerin @arthurlutz @sereinity I have made the linked PR above to try to circumvent this issue. It's working on my test server, but I have not extensively tested out and since it uses relatively advanced Bash magic that I do not quite master, any additional test is welcome.

Basically, if you can try it that would be great.

You can install (yunohost app install https://github.com/YunoHost-Apps/aeneria_ynh/tree/perm-hook -f or upgrade to that branch (yunohost app upgrade aeneria -u https://github.com/YunoHost-Apps/aeneria_ynh/tree/perm-hook -F), and try to give/remove new users the aeneria permission and see what happens.

sereinity commented 1 year ago

The upgrade went well:

Info: Now upgrading aeneria...
Info: [+...................] > Loading installation settings...
Info: [#+..................] > Checking version...
Info: [##++................] > Backing up the app before upgrading (may take a while)...
Info: [####+...............] > Ensuring downward compatibility...
Info: [#####+..............] > Making sure dedicated system user exists...
Info: [######++............] > Upgrading NGINX web server configuration...
Info: [########+...........] > Upgrading dependencies...
Info: [#########+++........] > Upgrading PHP-FPM configuration...
Info: [############+.......] > Configuring aeneria...
Info: [#############+......] > Upgrading aeneria...
Info: [##############++....] > Setuping a cron job...
Warning: File /etc/cron.d/aeneria has been manually modified since the installation or last upgrade. So it has been duplicated in /var/ca
che/yunohost/appconfbackup//etc/cron.d/aeneria.backup.20230419.084621
Warning: --- /var/cache/yunohost/appconfbackup//etc/cron.d/aeneria.backup.20230419.084621       2022-12-03 13:42:33.805633783 +0000
Warning: +++ /etc/cron.d/aeneria        2023-04-19 08:46:22.947494642 +0100
Warning: @@ -1 +1,2 @@
Warning: -*/20 * * * * aeneria /usr/bin/php7.4 /var/www/aeneria/bin/console aeneria:fetch-data
Warning: +05 2-23/3 * * * aeneria /usr/bin/php7.4 /var/www/aeneria/bin/console aeneria:fetch-data
Warning: +*/15 * * * * aeneria /usr/bin/php7.4 /var/www/aeneria/bin/console aeneria:pending-action:process-expired
Info: [################+...] > Reloading NGINX web server...
Info: [####################] > Upgrade of aeneria completed
Success! aeneria upgraded
Success! Upgrade complete
sereinity commented 1 year ago

The latest release 1.1.6~ynh4 went well. I think we can close this issue.