-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unnecesarry warning on first LDAP user sync #374
Comments
Hi @pboguslawski . Thank you for taking the time to create this issue. It looks like that warning is there to be informative. I suppose we could change it from WARNING to INFO and possibly reword the message to be more accurate. What do you think @AltamashShaikh ? |
Problem is not only in message prio: now user is not synced from LDAP after first login (and synced from LDAP after further logins). Should be synced after first login too if LDAP sync is enabled. |
Just checking if I understand the ticket correctly:
Did I understand that correctly? If so, I see various options:
Keen to read your thoughts |
@snake14 Yes a warning is not required, we should change that for sure and @Stan-vw since the user gets synced after first login we could do the same at the time of login |
[...]
[...]
It's synced but not on the first login (here only warning is logged and account is set as LDAP account without syncing) but on further logins (here LDAP sync after every login).
I don't see any point in logging such message and different sync behaviour on first and further logins. |
@pboguslawski You are correct doesn't make sense to have the if else logic and we can remove the same as the existing user is marked as LDAP user on first login but not synced |
Is the message relevant in later logins if for some reason the sync failed? Or is there no downside to simply removing the message? To be honest, since the message doesn't have a clear "do this in case you see this error" I'm not even sure the message has value in case it would only show up in the exact scenario where it is relevant. This might be more relevant to show in a superuser's config screen than in the normal user's screen after logging in |
@Stan-vw Checking the code it doesn't seems useful to have this extra case just for 1st time login as the skipped part would be done next time you login. |
When LDAP user has already their account in matomo DB created manually, then on their first login warning is logged:
No such warning on further logins of same user.
Seems this warning makes no sense because after this warning user is marked as synced from LDAP and further updates from LDAP work fine for them.
Consider removing this warning and always update user from LDAP if LDAP user sync is enabled and user matches.
The text was updated successfully, but these errors were encountered: