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

[Izzy Blacklock]

{-- This isn't a crosspost; at lest I hope it isn't.  I sent it in reply to a 
message on the users list but it hasn't shown up and posts I made after have.  
I'm reposting it here because this is really where it belongs anyway.  --}

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['acco
unt_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