group membership checked against login instead of DN

victort commented

2019-11-17 01:25:07 +01:00

(Migrated from github.com)

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.

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 `uid`s 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 commented

2019-11-17 09:38:35 +01:00

(Migrated from github.com)

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.

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 commented

2019-11-17 09:39:08 +01:00

(Migrated from github.com)

CC @nupplaphil

annando commented

2019-11-17 19:19:15 +01:00

(Migrated from github.com)

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 commented

2019-11-18 01:53:05 +01:00

(Migrated from github.com)

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++

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 commented

2019-12-26 20:39:35 +01:00

(Migrated from github.com)

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.

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 commented

2019-12-27 00:30:00 +01:00

(Migrated from github.com)

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 commented

2019-12-27 01:51:09 +01:00

(Migrated from github.com)

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

victort commented

2020-09-14 17:58:47 +02:00

(Migrated from github.com)

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!

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 commented

2020-09-14 19:18:21 +02:00

(Migrated from github.com)

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.

This pull request has changes conflicting with the target branch.

ldapauth/ldapauth.php

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin victort/LDAP_Group_memebership_tweak:victort/LDAP_Group_memebership_tweak

git checkout victort/LDAP_Group_memebership_tweak

Merge

Merge the changes and update on Forgejo.

git checkout develop

git merge --no-ff victort/LDAP_Group_memebership_tweak

git checkout develop

git merge --ff-only victort/LDAP_Group_memebership_tweak

git checkout victort/LDAP_Group_memebership_tweak

git rebase develop

git checkout develop

git merge --no-ff victort/LDAP_Group_memebership_tweak

git checkout develop

git merge --squash victort/LDAP_Group_memebership_tweak

git checkout develop

git merge --ff-only victort/LDAP_Group_memebership_tweak

git checkout develop

git merge victort/LDAP_Group_memebership_tweak

git push origin develop

group membership checked against login instead of DN #911

Checkout

Merge