Skip to content
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

fix(LDAP): do not count mapped users x-times active configs #44767

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Apr 10, 2024

Summary

the method counts all rows in the mapping tables, which are not specific to single configurations, but contains users from all

So if you are operating on a one-click host with a user limit of 500, but have two LDAPs connected with altogether 300 users, without this fix they are interpreted as 600 users, and the 301st could not log in.

Checklist

- the method counts all rows in the mapping tables, which are not
  specific to single configurations, but contains users from all

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz blizzz added bug 3. to review Waiting for reviews labels Apr 10, 2024
@blizzz blizzz requested review from come-nc, a team, yemkareems and sorbaugh and removed request for a team April 10, 2024 10:49
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

Then the method should be moved out of User_LDAP and directly in User_Proxy if it’s not specific to a connection.

@blizzz
Copy link
Member Author

blizzz commented Apr 11, 2024

Then the method should be moved out of User_LDAP and directly in User_Proxy if it’s not specific to a connection.

Actually yes, and it crossed my mind yesterday as well. Revisiting this, I am not sure anymore why I decided against it – possibly to avoid injecting another dependency. But actually the abstract Proxy class pulls it in anyway (though on demand).

@blizzz blizzz added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Apr 11, 2024
@susnux susnux added this to the Nextcloud 30 milestone Apr 18, 2024
This was referenced Jul 30, 2024
This was referenced Aug 5, 2024
@skjnldsv skjnldsv mentioned this pull request Aug 13, 2024
@skjnldsv skjnldsv modified the milestones: Nextcloud 30, Nextcloud 31 Aug 14, 2024
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.

4 participants