Skip to content

Conversation

@AltamashShaikh
Copy link
Contributor

Description

Adds code to delete all the access if no access returned from Ldap and enable_synchronize_access_from_ldap=1

Issue No

#PG-4868

Steps to Replicate the Issue

  1. Enable sync access from ldap
  2. Map the access in Ldap for a user
  3. Login via that user
  4. Now remove all the access for that user in LDAP and try logging in, it will work
  5. Checkout this PR
  6. Logout and Login again with that user.

Checklist

  • [✔] Tested locally or on demo2/demo3?
  • [✔] New test case added/updated?
  • [NA] Are all newly added texts included via translation?
  • [NA] Are text sanitized properly? (Eg use of v-text v/s v-html for vue)
  • [✔] Version bumped?
  • [NA] I have understood, reviewed, and tested all AI outputs before use
  • [NA] All AI instructions respect security, IP, and privacy rules

Comment on lines 204 to 208
} else {
$this->logger->log('warning', "UserSynchronizer::{func}: User '{user}' has no access in LDAP, but access synchronization is enabled.", array(
'func' => __FUNCTION__,
'user' => $piwikLogin
));
Copy link
Contributor

@james-hill-matomo james-hill-matomo Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AltamashShaikh Not sure if this is ready for review yet, but this error message is now wrong. Is there any case where accessSynchronization is disabled and this function is called? I don't think there should be - having the if is nice, but maybe it should be further up the stack?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@james-hill-matomo Updated logic.

@james-hill-matomo
Copy link
Contributor

#416

@AltamashShaikh
Copy link
Contributor Author

@james-hill-matomo Thanks for the suggestion, I have applied them here and updated the test case to be check the behaviour more accurately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants