Kipjr / Ldap_Login

piwigo plugin ldap login
http://piwigo.org/ext/extension_view.php?eid=650
6 stars 15 forks source link

check_ldap_group_membership whith openldap result in bad filter for memberUid #142

Open lours974 opened 7 months ago

lours974 commented 7 months ago

Hello,

Bad search filter when using openldap and memberUid="USER", so login not working.

Not working : DEBUG: [check_ldap_group_membership]> @ldap_search($this->cnx,'dc=xxxx,dc=xxx', '(&(objectclass=posixGroup)(cn=piwigo_users)(memberUid=uid=xxx,ou=xxx,dc=xxx,dc=xxx)(cn=*))','memberUid') for piwigo_users

Must be : DEBUG: [check_ldap_group_membership]> @ldap_search($this->cnx,'dc=xxx,dc=xxx', '(&(objectclass=posixGroup)(cn=piwigo_users)(memberUid=xxx)(cn=*))','memberUid') for piwigo_users

Correction for made it work in Ldap_Login/main.inc.php

399 $search_filter = "(&(objectclass=$group_class)(cn=$group_cn)($member_attr=$user_dn)($group_filter))"; changed to 399 $search_filter = "(&(objectclass=$group_class)(cn=$group_cn)($member_attr=$user_login)($group_filter))";

Authentication now working with openldap and memberUid=USER

I don't know if there is other change to make, i just want to share this one with you, if you can't do a more beautiful change in the code. Thanks for reading (my bad english :-)

Kipjr commented 7 months ago

with objectClass group, member as attribute type and dn as member object attribute , this seems to work:

ldap_search_s(ld, "dc=planetexpress,dc=com", 2, "(&(objectclass=group)(cn=admin_staff)(member=cn=Hermes Conrad,ou=people,dc=planetexpress,dc=com)(cn=*))", attrList,  0, &msg)
Getting 1 entries:
Dn: cn=admin_staff,ou=people,dc=planetexpress,dc=com
cn: admin_staff; 
groupType: 0x80000002 = ( ACCOUNT_GROUP | SECURITY_ENABLED ); 
member (2): cn=Hubert J. Farnsworth,ou=people,dc=planetexpress,dc=com; cn=Hermes Conrad,ou=people,dc=planetexpress,dc=com; 
objectClass (2): Group; top; 

But I noticed you're using memberUid as attribute, which does not uses the DN, but the regular username. I will try to build a check to check for both the DN and the username in the search filter

(&(objectclass=$group_class)(cn=$group_cn)(|($member_attr=$user_login)($member_attr=$user_dn))($group_filter))

_"Give me an object of the class "$group_class", with the name "$group_cn" and the "$member_att" either be "$user_login" or "$user_dn", provided that "$groupfilter" is a match.

eamathw commented 7 months ago

I'd suggest adding a config option to toggle whether the group membership filter uses the user's DN or login name. Example:

diff -x .git -uprN ./admin/configuration.php /var/www/piwigo/plugins/Ldap_Login/admin/configuration.php
--- ./admin/configuration.php   2024-01-18 14:02:26.209653762 -0600
+++ /var/www/piwigo/plugins/Ldap_Login/admin/configuration.php  2024-01-18 13:43:32.466699388 -0600
@@ -50,6 +50,7 @@ if (isset($_POST['save']) or isset($_POS
    $me->config['ld_group_desc']   = $_POST['LD_GROUP_DESC'];

    $me->config['ld_group_member_attr']   = $_POST['LD_GROUP_MEMBER_ATTR'];
+   $me->config['ld_group_member_attr_is_dn']   = $_POST['LD_GROUP_MEMBER_ATTR_IS_DN'];
    $me->config['ld_user_member_attr']   = $_POST['LD_USER_MEMBER_ATTR'];

    $me->config['ld_group_user']   = $_POST['LD_GROUP_USER'];
@@ -152,6 +153,7 @@ $template->assign('LD_GROUP_ATTR',$me->c
 $template->assign('LD_GROUP_DESC',$me->check_config('ld_group_desc'));

 $template->assign('LD_GROUP_MEMBER_ATTR',$me->check_config('ld_group_member_attr'));
+$template->assign('LD_GROUP_MEMBER_ATTR_IS_DN',$me->check_config('ld_group_member_attr_is_dn'));
 $template->assign('LD_USER_MEMBER_ATTR',$me->check_config('ld_user_member_attr'));
 $template->assign('LD_MEMBERSHIP_USER',$me->check_config('ld_membership_user'));

diff -x .git -uprN ./admin/configuration.tpl /var/www/piwigo/plugins/Ldap_Login/admin/configuration.tpl
--- ./admin/configuration.tpl   2024-01-18 14:02:26.209653762 -0600
+++ /var/www/piwigo/plugins/Ldap_Login/admin/configuration.tpl  2024-01-18 13:53:27.861456534 -0600
@@ -158,6 +158,13 @@
    <i style="margin:15%;">{'The attribute field to use when loading the group members from the group.'|@translate}</i> 
    </p>
    <p>
+   <label style="display:inline-block; width:15%;" for="ld_group_member_attr_is_dn">
+        <input type='hidden' value='0' name='LD_GROUP_MEMBER_ATTR_IS_DN'>
+        <input type="checkbox" id="ld_group_member_attr_is_dn" name="LD_GROUP_MEMBER_ATTR_IS_DN" value="1" {if $LD_GROUP_MEMBER_ATTR_IS_DN== 1}checked{/if}>
+   {'Group membership attribute is a DN'|@translate}</label>
+   {if isset($LD_GROUP_MEMBER_ATTR_IS_DN) and $LD_GROUP_MEMBER_ATTR_IS_DN}<i style="color:red;">{$LD_GROUP_MEMBER_ATTR_IS_DN}</i>{/if}
+   </p>
+   <p>
    <label style="display:inline-block; width:15%;" for="ld_user_member_attr">{'User Membership Attribute:'|@translate}</label>
    <input size="70" type="text" id="ld_user_member_attr" name="LD_USER_MEMBER_ATTR" value="{$LD_USER_MEMBER_ATTR}" />
    {if isset($WARN_LD_USER_MEMBER_ATTR) and $WARN_LD_USER_MEMBER_ATTR}<i style="color:red;">{$WARN_LD_USER_MEMBER_ATTR}</i>{/if}<br>
diff -x .git -uprN ./class.ldap.php /var/www/piwigo/plugins/Ldap_Login/class.ldap.php
--- ./class.ldap.php    2024-01-18 14:02:26.213653942 -0600
+++ /var/www/piwigo/plugins/Ldap_Login/class.ldap.php   2024-01-18 13:58:11.726215439 -0600
@@ -25,6 +25,7 @@ class Ldap {
        'ld_group_desc' => 'description',
        'ld_group_basedn' => 'cn=groups,dc=domain,dc=tld',
        'ld_group_member_attr' => 'member',
+       'ld_group_member_attr_is_dn' => 1,
        'ld_user_member_attr' => 'memberOf',
        'ld_group_webmaster' => 'cn=piwigo_webmasters,cn=groups,dc=domain,dc=tld',
        'ld_group_admin' => 'cn=piwigo_admins,cn=groups,dc=domain,dc=tld',
@@ -396,7 +397,12 @@ class Ldap {
                         return false;
                 }

-       $search_filter = "(&(objectclass=$group_class)(cn=$group_cn)($member_attr=$user_dn)($group_filter))"; 
+       if ( $this->config['ld_group_member_attr_is_dn'] ) {
+           $user_member_attr_val = $user_dn;
+       } else {
+           $user_member_attr_val = $user_login;
+       }
+       $search_filter = "(&(objectclass=$group_class)(cn=$group_cn)($member_attr=$user_member_attr_val)($group_filter))"; 
        $this->write_log("[check_ldap_group_membership]> @ldap_search(\$this->cnx,'$base_dn', '$search_filter','$member_attr') for $group_cn");
        $search = ldap_search($this->cnx, $base_dn, $search_filter,array($member_attr),0,0,5); //search for group
        if($search){