group membership checked against login instead of DN

[victort]

Hi , sorry if this is out of the blue.

After weeks of messing with why Friendica won't work with LDAP, i finally narrowed my particular problem down to Group Membership not parsing the attribute value correctly.

for my LDAP groups, the memberUid attribute is not filled with DNs of my users, but the uids of my users.

So after changing $dn to $res, suddenly all my group members would resolve!

It's a very minor change, but it might affect someone else.

I didn't see an issues tab on the friendica-addons repo, so I thought I'd just submit this change the pull request way. Sorry if this is out of sequence or something. While not a coder at all, I am happy to help resolve this any other way you'd like.

[MrPetovan]

Hi @victort and thanks for your submission. Addons issues are centralized with core issues here: https://github.com/friendica/friendica/issues

I'm not qualified to judge if your change is applicable to all cases but I'm glad it solved yours. I'm going to let people with more familiarity with LDAP to take a better glance than me.

[MrPetovan]

CC @nupplaphil

[annando]

I'm unsure about this. I have the feeling as if we should make it an option, but I don't have any LDAP server to test it.

[victort]

I agree it should be an option. Also, I'm confused why $res works and $username doesn't. but I am not an expert in such things.

In my otherwise uninformed opinion, I too agree it should be an option, like ldap_group_member_attribute or something. It's just beyond my skills to do so.

anyway, thanks for considering.
US$0.02++

[victort]

hi again,

Just for the record, I've experienced a regression when I upgraded to 2019.12, which reintroduced the checking-for-member-of-ldap-group problem, which i fixed with the same one liner.

[MrPetovan]

Hi @victort , thanks for the follow-up, this pull request hasn't been merged yet because of @annando 's concerns above, so it hasn't been part of the latest release and your one-liner patch is still warranted for your specific case.

[annando]

I totally forgot about this. I guess we should make it configurable. Then the responsibility is in the hands of the administrator.

[victort]

Hi again. No, I don't think you necessarily merge this, BUT.. I did want to mention that after upgrading to 2020.7, had to make this adjustment again before I could log in.

Thanks for playing!

[annando]

Like last year, I totally forgot. Can you enhance the PR so that this change is configurable? I'm not totally sure if the change would work with all systems. So having a configuration for that would be the best.