[Phpgroupware-users] Proposed LDAP patch (was Problems getting ldap auth working...)

[Izzy Blacklock]

On Tuesday 25 Mar 2003 9:50 pm, Lars Kneschke(priv.) wrote:
> Izzy Blacklock <address@hidden> schrieb:
> >On Tuesday 25 Mar 2003 12:25 am, Izzy Blacklock wrote:
> >
> >I figured out what was causing this error.  It seems you MUST HAVE
> >values for
> >the following fields when doing the initial setup. I had left them
> >blank as I
> >don't really want phpgw adding user accounts.  Just modifying accounts
> >I
> >create using idealx account manager.
> >
> >LDAP Default homedirectory prefix
> >LDAP Default shell
>
> phpGroupware is setting this values, when you set
>
> If using LDAP, do you want to manage homedirectory and loginshell
> attributes?:
>
> to
>
> yes

I had set this value to no and left the two fields blank, but in the process 
of troubleshooting the error I was getting when trying to add the demo 
accounts, I learned that these two fields are required by the posixAccount 
objectClass.  Without them, ldap_add returns an Object class violation error, 
which is what I was getting.  

I didn't take a close look at all the code involved, just enough to get a 
grasp of what was being done so I could do it manually and see the errors 
actually returned from the ldap server.  And of course find the bug! :)  I 
suspect that when you say no to the question above and don't provide any 
values then there isn't any defaults being set for these values.  Since they 
are needed, this obviousely wont work.  

If this is the case, then another patch is in order.  Sadly, I'm not likely 
going to be able to provide it.  I don't know php well enough.  It seems to 
me what is needed though is a change in which ldap elements are used when not 
managing these attributes.  Specifically, I think the following shouldn't be 
specified unless yes is the answer above:

objectClass: posixAccount
uidnumber:
gidnumber:

I double checked that ldapadd works when these elements are removed (using the 
elements from the created demo account as a template)  As long as these 
elements aren't needed by phpgw anywhere (I don't know), then I think this 
makes the most sense.  I just took another look at the source, and maybe I 
can create the patch.  It should be as simple as grouping these three 
elements together and wrapping them in an if statement.  Looking at the code 
again, it looks like this has already been done for the homedir and shell 
values if I'm reading this code snip correctly (reformated to accommodate 
e-mail line lengths):

---------------- 8< ----------------------
if ($GLOBALS['phpgw_info']['server']['ldap_extra_attributes'] &&
    $account_info['account_type'] != 'g')
    {
    $entry['homedirectory'] = $account_info['homedirectory'] &&
      $account_info['homedirectory'] != 
      $GLOBALS['phpgw_info']['server']['ldap_account_home'] ?
      $account_info['homedirectory'] :
$GLOBALS['phpgw_info']['server']['ldap_account_home'].SEP.$account_info['account_lid'];
    $entry['loginshell'] = $account_info['loginshell'] ?
      $account_info['loginshell'] :
      $GLOBALS['phpgw_info']['server']['ldap_account_shell'];
    }

--------------- 8< ---------------------------

Couldn't we just add the $entry lines for the three elements above in here.  
Of course that does create a problem with the array sequencing for the 
objectclass entries.  I'm not sure how best to resolve this issue.  Before I 
burn out my brain thinking on it (I'm new to php after all), perhaps I should 
get an answer to the question of whether these elements are needed anywhere 
else by phpgw!?  

...Izzy