plugin-LoginLdap icon indicating copy to clipboard operation
plugin-LoginLdap copied to clipboard

LDAP user sync bugs in webserver auth mode

Open pboguslawski opened this issue 8 months ago • 10 comments

When matomo authenticates users with LDAP + webserver auth (users in LDAP do not use passwords)...

[LoginLdap]
servers[] = "myldap"
use_ldap_for_authentication = 0
synchronize_users_after_login = 1
enable_synchronize_access_from_ldap = 0
[...]
use_webserver_auth = 1
[...]
enable_password_confirmation = 0

...every successful UI request is updating users record in SQL

UPDATE `user` SET `email` = 'mail@here', `password` = 'new_random_pass_every_time_here', ts_password_modified = 'curr time here' WHERE `login` = 'login_here'
UPDATE user SET ts_password_modified = date_registered WHERE login = 'login_here'

Enabling DEBUG shows

DEBUG LoginLdap[2025-03-03 18:04:43 UTC] [7854e] UserMapper::getPiwikPasswordForLdapUser: Could not find LDAP password for user 'login_here', generating random one.
DEBUG LoginLdap[2025-03-03 18:04:43 UTC] [7854e] UserSynchronizer::synchronizeLdapUser: synchronizing user [ piwik login = login_here, ldap login = login_here ]

Random password generation is explained on

https://github.com/matomo-org/plugin-LoginLdap/issues/212 https://github.com/matomo-org/plugin-LoginLdap/issues/204

Every LDAP sync job execution from cron...

php /var/www/matomo/console loginldap:synchronize-users --no-interaction --no-ansi

...also generates SQL user update commands like above (but this time password in updates is not changing).

Problems to be resolved:

  1. Any user data sync from LDAP (i.e. after login if enabled, sync from cron job) should not generate any SQL update queries if users data in SQL and LDAP is the same to avoid wasting resources.
  2. Matomo should not assume there are passwords used for auth (i.e. client certs may be used) when webserver auth is enabled. Matomo API should not require password column in user table to be nonempty and should not generate dummy passwords nor try to copy hashes from LDAP even if available there.
  3. With synchronize_users_after_login = 1 user LDAP sync should not be performed on every UI page change, but as name says once after login (new session creation).

pboguslawski avatar Mar 03 '25 18:03 pboguslawski

Hi @pboguslawski . Thank you for taking the time to create this issue. It sounds like you are describing the designed/expected behaviour. I'll mark this issue to be reviewed by our Product team to take your enhancement suggestions under advisement.

snake14 avatar Mar 03 '25 19:03 snake14

@pboguslawski The setting of random password is to support our existing design, also the change to update the password after login was introduced in https://github.com/matomo-org/plugin-LoginLdap/pull/294, you can disable this by disabling "Enable User Access Synchronization from LDAP" setting in your LDAP config.

Image

AltamashShaikh avatar Mar 04 '25 03:03 AltamashShaikh

you can disable this by disabling "Enable User Access Synchronization from LDAP" setting in your LDAP config

Enable User Access Synchronization from LDAP turned on is required for new LDAP user to be able to login to Matomo UI (without it, user must wait for cron LDAP sync task to create their account in Matomo before login is possible). The problem (3) is not about syncing after login but syncing after each UI operation. To resolve (3) please consider syncing only after login (when UI session cookie is created) and not when UI session is already established.

pboguslawski avatar Mar 04 '25 07:03 pboguslawski

@pboguslawski I just check locally and it seems to be called only twice, can you explain how are you observing that it is trying to update the password multiple times ?

Also can you check if replacing this line with below code fixes the issue for you ?

if (Config::getShouldSynchronizeUsersAfterLogin() && empty($_SESSION['user.name'])) {

AltamashShaikh avatar Mar 04 '25 08:03 AltamashShaikh

can you explain how are you observing that it is trying to update the password multiple times

The problem was noticed in MariaDB bin log - there were multiple user record updates after user moving between pages in Matomo UI.

Just checked in test Matomo 5.2.1 installation that opening Matomo dashboard from other Matomo page generated many updates with random passwords (password in not used for auth in this scenario and Matomo does not have access to password attribute in LDAP). Matomo debug log said:

DEBUG LoginLdap[2025-03-04 08:45:32 UTC] [325ef] UserMapper::getPiwikPasswordForLdapUser: Could not find LDAP password for user 'my_login_here', generating random one.
DEBUG LoginLdap[2025-03-04 08:45:32 UTC] [325ef] UserSynchronizer::synchronizeLdapUser: synchronizing user [ piwik login = my_login_here, ldap login = my_login_here ]

DEBUG LoginLdap[2025-03-04 08:45:33 UTC] [6526e] UserMapper::getPiwikPasswordForLdapUser: Could not find LDAP password for user 'my_login_here', generating random one.
DEBUG LoginLdap[2025-03-04 08:45:33 UTC] [6526e] UserSynchronizer::synchronizeLdapUser: synchronizing user [ piwik login = my_login_here, ldap login = my_login_here ]

DEBUG LoginLdap[2025-03-04 08:45:33 UTC] [eda59] UserMapper::getPiwikPasswordForLdapUser: Could not find LDAP password for user 'my_login_here', generating random one.
DEBUG LoginLdap[2025-03-04 08:45:33 UTC] [eda59] UserSynchronizer::synchronizeLdapUser: synchronizing user [ piwik login = my_login_here, ldap login = my_login_here ]

DEBUG LoginLdap[2025-03-04 08:45:34 UTC] [0da4e] UserMapper::getPiwikPasswordForLdapUser: Could not find LDAP password for user 'my_login_here', generating random one.
DEBUG LoginLdap[2025-03-04 08:45:34 UTC] [0da4e] UserSynchronizer::synchronizeLdapUser: synchronizing user [ piwik login = my_login_here, ldap login = my_login_here ]

DEBUG LoginLdap[2025-03-04 08:45:34 UTC] [dc2f3] UserMapper::getPiwikPasswordForLdapUser: Could not find LDAP password for user 'my_login_here', generating random one.
DEBUG LoginLdap[2025-03-04 08:45:34 UTC] [dc2f3] UserSynchronizer::synchronizeLdapUser: synchronizing user [ piwik login = my_login_here, ldap login = my_login_here ]

[...]

MariaDB bin log said:

#Q> UPDATE `user` SET `email` = '[email protected]', `password` = '$2y$10$pf/XDKuvQbzGn1DpYDZXbuyMyIeREQQ13T6tL0JT1M.fROi0Tru1W', ts_password_modified = '2025-03-04 08:45:32' WHERE `login` = 'my_login_here'
#Q> UPDATE user SET ts_password_modified = date_registered WHERE login = 'my_login_here'

#Q> UPDATE `user` SET `email` = '[email protected]', `password` = '$2y$10$VzySWt5vJvbWfUekzmMWQe6wgovrgdymgJY5jbKuTvmWhOmPzYWuO', ts_password_modified = '2025-03-04 08:45:33' WHERE `login` = 'my_login_here'
#Q> UPDATE user SET ts_password_modified = date_registered WHERE login = 'my_login_here'

#Q> UPDATE `user` SET `email` = '[email protected]', `password` = '$2y$10$V/o9xSaYUXeAE4tEhVHxhuQCSJY8OkMSnxvMXh2cA48mtytHYFyGW', ts_password_modified = '2025-03-04 08:45:33' WHERE `login` = 'my_login_here'
#Q> UPDATE user SET ts_password_modified = date_registered WHERE login = 'my_login_here'

#Q> UPDATE `user` SET `email` = '[email protected]', `password` = '$2y$10$zNCZtIFrTXSyo/t.kYV63.XK/yJ5DiSy8lzbf6AkmTT/S6uHVVdzO', ts_password_modified = '2025-03-04 08:45:34' WHERE `login` = 'my_login_here'
#Q> UPDATE user SET ts_password_modified = date_registered WHERE login = 'my_login_here'

#Q> UPDATE `user` SET `email` = '[email protected]', `password` = '$2y$10$dNmwH9n8TYk/4LbHB4V0.ehLQgV4dtpNryDJ9eCE2XO2YPI0.VKpS', ts_password_modified = '2025-03-04 08:45:34' WHERE `login` = 'my_login_here'
#Q> UPDATE user SET ts_password_modified = date_registered WHERE login = 'my_login_here'

[...]

Also can you check if replacing this line with below code fixes the issue for you ?

Patching UserSynchronizer.php from 5.2.1 with

-                    if (Config::getShouldSynchronizeUsersAfterLogin()) {
+                    if (Config::getShouldSynchronizeUsersAfterLogin() && empty($_SESSION['user.name'])) {

does not resolve problem - still many updates on dashboard opening in UI.

pboguslawski avatar Mar 04 '25 09:03 pboguslawski

Just checked that $_SERVER['REMOTE_USER'] is set to correct user login every time in UserSynchronizer.php near if as above so web server authenticates correctly.

pboguslawski avatar Mar 04 '25 09:03 pboguslawski

Test env is based on Debian 12 (php v8.2.26, apache v2.4.62, php fpm). Matomo system check in UI does not contain any session handling related problems.

pboguslawski avatar Mar 04 '25 09:03 pboguslawski

@pboguslawski Strange, for me locally its only 2 times at the time of login, I will check within the team if we can do anything about this, I suspect your setup is trying to authenticate on every request, which could be the problem.

Reading this, Just checked that $_SERVER['REMOTE_USER'] is set to correct user login every time in UserSynchronizer.php near if as above so web server authenticates correctly.

@pboguslawski Does changing the patch line to below helps ?

if (Config::getShouldSynchronizeUsersAfterLogin() && empty($_SESSION['REMOTE_USER'])) {

AltamashShaikh avatar Mar 05 '25 00:03 AltamashShaikh

Changing to

if (Config::getShouldSynchronizeUsersAfterLogin() && empty($_SESSION['REMOTE_USER'])) {

does not help.

I can see Matomo has session handling using PHP session ID MATOMO_SESSID:

https://github.com/matomo-org/matomo/blob/5.x-dev/core/Session.php

https://github.com/matomo-org/matomo/blob/631a360bc7d71299cc5fbdca0f4020da6b3bd403/core/Session/SessionAuth.php#L92

I can see

Cookie:	MATOMO_SESSID=h56jv0n3da9k6r6h7b53i190ch

set in browser requests but I cannot see any session file in PHP session directory configured. Only record in session SQL table.

Should Matomo/PHP sessions (MATOMO_SESSID) be used for the authentication of all subsequent requests after successful auth of first request in session with this plugin? If so - where session data is stored (i.e. user login to be used for given MATOMO_SESSID: files in PHP session dir? SQL sessions table? elsewhere?).

pboguslawski avatar Mar 07 '25 18:03 pboguslawski

@pboguslawski Its stored in the database, you can view matomo_session or session table, the table name would depend on your prefix.

AltamashShaikh avatar Mar 10 '25 00:03 AltamashShaikh

@pboguslawski Any update ? Can we close this issue ?

AltamashShaikh avatar Apr 22 '25 04:04 AltamashShaikh

Its stored in the database, you can view matomo_session or session table, the table name would depend on your prefix.

Checked that login to Matomo 5.3.1 UI creates record in session table and this record is updated (modified column changes value) on every UI move so session stuff seems to work fine. In the same time redundant SQL update queries are sent to DB on every UI move (LDAP auth/sync should not be performed if session is already established).

Didn't find any session connected params in Matomo config related to session+LDAP handling that may be used to fix it.

So problem exists and should be resolved probably.

If you cannot reproduce it - please point me to code that should check whether session exists and skips LDAP auth if so and I will try to debug it.

pboguslawski avatar Apr 22 '25 09:04 pboguslawski

@pboguslawski Have you set any cron to sync exiting users separately ?

The code should not be called after login, I have checked the code again.

This is the code where you need to add the condition like && !SESSION_SET

AltamashShaikh avatar Apr 23 '25 08:04 AltamashShaikh

@pboguslawski Are you able to debug this on your own ?

AltamashShaikh avatar May 07 '25 03:05 AltamashShaikh

@pboguslawski could you please give us an update?

glitchhunter66 avatar Jun 06 '25 03:06 glitchhunter66

Will try to help but need more time. WIP added to subject.

pboguslawski avatar Jun 06 '25 05:06 pboguslawski