mirror of
https://github.com/lldap/lldap.git
synced 2026-04-25 08:15:52 +03:00
[GH-ISSUE #666] LLDAP group names are case sensitive when searching, they shouldn't be #241
Labels
No labels
backend
blocked
bug
cleanup
dependencies
docker
documentation
duplicate
enhancement
enhancement
frontend
github_actions
good first issue
help wanted
help wanted
integration
invalid
ldap
pull-request
question
rust
rust
tests
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/lldap-lldap#241
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @ArVar on GitHub (Sep 7, 2023).
Original GitHub issue: https://github.com/lldap/lldap/issues/666
I'm following the instructions as explained here: https://github.com/lldap/lldap/blob/main/example_configs/nextcloud.md
Unfortunately, I'm getting both:
A message, telling me, that the LDAP server does not support memberOf (which is obviously not true).
And yes, I've read this in the example:
the only from these groups option is not functional, but would like to mention this in this context.Trying to use the described filter:
(&(objectclass=person)(memberOf=cn=nextcloud_users,ou=groups,dc=example,dc=com))I'm getting following warning in lldap logs:
and the users are not discovered.
Of course I have groups and Users assigned to them.
I'm using the most recent image for the container, as well as an up to date Nextcloud installation. I've found similar errors in the issues but no solution for this particular problem so far.
Apperently, there also was a discussion on discord a while ago, but without a proper solution.
Does anyone have similar experiences?
For me it looks somehow related to case-(in)sensitivity.
@nitnelave commented on GitHub (Sep 7, 2023):
Sorry for the silly question, but: could you have somehow gotten mixed up
between the groups and users filters?
The warning you get is from querying groups (which also explains why member
of doesn't work, since it only works with users)
@nitnelave commented on GitHub (Sep 7, 2023):
Just leaving a note: I do not think it's due to case sensitivity. We lowercase every keyword, and we have tests checking that we do.
@ArVar commented on GitHub (Sep 11, 2023):
I need to recheck. But I'm explicitly doing as described in the example. I'll report.
@ArVar commented on GitHub (Sep 11, 2023):
So, I played around with different group names. I created different groups in lldap. The filter in nextcloud as described in the example only works for existing groups with all-lowercase names. E.g. for 'test' as an lldap-group it worked, where for 'Test' the result was always empty until I created the group 'test' in lldap (with all lowercase). Therefore I think, there really might be a case issue. The request might be transformed to lowercase in the nextcloud LDAP-plugin but lldap seems to be case-sensitive at least for groups. LDAP is mostly case-insensitve by default.
@nitnelave commented on GitHub (Sep 11, 2023):
Hmm, yeah, it does look like a case sensitivity now (I still think there was something wrong with the filters at the beginning, that was not a case sensitivity problem).
Indeed, I think the group names are case sensitive in LLDAP, where they should not be. I'll rename the issue to refocus on that. Can you work around this for now?
@ArVar commented on GitHub (Sep 11, 2023):
After further thinking about the warning:
Ignoring unknown group attribute ""memberof"" in filter.\n\This warning should not appear in the logs, since my filter is for users and not for groups:
ldapUserFilter "(&(objectclass=person)(memberOf=cn=nextcloud_users,ou=groups,dc=example,dc=com))"This might or might not be related to how the LDAP-plugin of nextcloud works but mislead me to wrong conclusions earlier. But it definetely is not the source of this problem, since I checked lldaps source, which handles this memberOf attribute well, I think.
Is it possible that convert_group_filter is applied on each LDAP-search even if there is only a user filter? (Would be another, completely different issue).
@nitnelave commented on GitHub (Sep 11, 2023):
Could you run LLDAP in verbose mode and paste the logs here? It's hard to debug without knowing what query was sent.
@ArVar commented on GitHub (Sep 21, 2023):
Here are the logs for following request:
(&(objectclass=person)(memberOf=cn=Mitarbeiter,ou=groups,dc=***,dc=de))Logs:
The group "Mitarbeiter" is transformed to "mitarbeiter" and the Result is
return: []"Mitarbeiter" has members where "mitarbeiter" does not exist.
I hope this helps to dig a bit deeper.
@nitnelave commented on GitHub (Sep 21, 2023):
Ah, yes, I think I see what's happening. This is a half-baked attempt at making comparison case insensitive: the searched group is lowercased, which would help if the stored groups were lowercased as well, but it's not the case (haha).
Yep, that's a very legitimate bug
@nitnelave commented on GitHub (Sep 21, 2023):
In the meantime, I recommend only using lowercase groups
@ArVar commented on GitHub (Sep 21, 2023):
Wow, what a response time, @nitnelave. 🥇 :-D
@ArVar commented on GitHub (Oct 18, 2023):
Hi @nitnelave, if you point me to the relevant code snippets you have in mind, maybe I can help and create a pull request.
@nitnelave commented on GitHub (Oct 20, 2023):
Sorry for the delay, I was thinking how to best approach it. I'd still like to be able to have capital letters in group names (so we have to store them in capital), but there's no portable (and indexable) way to have a case insensitive search in SQL.
One way would be to write an SQL migration, and do something different for each engine to add a case insensitive
collate.Sea-QL doesn't support
collatecross-engine yet (there's https://docs.rs/sea-query/latest/sea_query/table/struct.TableCreateStatement.html#method.collate and https://github.com/SeaQL/sea-orm/issues/1901 but they're not there yet), so we'd have to check the type of the connection and write raw SQL for each engine.@nitnelave commented on GitHub (Oct 20, 2023):
As for the specifics, it'd be something like adding a
migrate_to_v6method here: https://github.com/lldap/lldap/blob/main/server/src/domain/sql_migrations.rs#L495@nitnelave commented on GitHub (Dec 15, 2023):
Alright, I fixed the casing of group names, attribute names and user emails while I was at it :)
@a1730 commented on GitHub (Jan 11, 2024):
Fantastic!
This will be a big win for us. May I plead for a point release with these features, please?
Thank you for this excellent project.