andrew at morphoss
May 14, 2012, 12:29 AM
Post #2 of 3
(579 views)
Permalink
|
On Fri, 2012-05-11 at 15:21 +0200, Rasmus Borup Hansen wrote:
> Hi! I've been setting up DAViCal and ran into some problems with LDAP
> synchronization. The following patch got me up and running. The first
> couple of edits clean up the code and solve a problem with groups with
> no members at all getting the same members as the previous group with
> members. The last two changes format the timestamps in the right way.
> Note that I haven't looked at the else-parts following
>
> $valid[$mapping['modified']] = "$Y-$m-$d $H:$M:$S";
>
> as I don't understand why they are there at all, since in two other
> places in the code timestamps are converted in a similar manner
> without testing if $c->authenticate_hook['config']['format_updated']
> is empty.
Hi Rasmus,
Thanks for your patch. It seems that the original author of the LDAP
driver wrote the date-handling in some possibly over-complicated way
that let you specify a format for the date that was returned from the
LDAP server...
Given that I've never actually got around to setting up an LDAP server
myself, either for testing, or for some real life need, I just have to
depend on whatever people send me for this so the code quality gets
pretty uneven. It seems to me that it might be possible different LDAP
servers to return dates in different formats though, so that code might
be needed. Maybe.
Of course if I've made it this far through life without desperately
needing an LDAP server for something the odds of me actually running one
end up being pretty low, so unless someone volunteers to look after this
code (including responding to the mailing list and IRC) it'll continue
to be a second class citizen.
As a result I'm quite tempted to apply your patch, but I'm not sure that
I believe moving an assignment like:
$row = array();
into a loop that does not consume $row seems either necessary or
expected. Could you perhaps provide a better justification than " it
works for me", or a more carefully scoped patch?
Cheers,
Andrew.
>
>
> ---
> inc/drivers_ldap.php | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/inc/drivers_ldap.php b/inc/drivers_ldap.php
> index a784e48..c4eb46c 100644
> --- a/inc/drivers_ldap.php
> +++ b/inc/drivers_ldap.php
> @@ -127,7 +127,6 @@ class ldapDrivers
> join(', ', $attributes),
> $baseDNUsers);
> }
> - $row = array();
> for($i = ldap_first_entry($this->connect,$entry);
> $i && $arr = ldap_get_attributes($this->connect,$i);
> $i = ldap_next_entry($this->connect,$i) ) {
> @@ -158,10 +157,10 @@ class ldapDrivers
> join(', ', $attributes),
> $baseDNGroups);
> }
> - $row = array();
> for($i = ldap_first_entry($this->connect,$entry);
> $i && $arr = ldap_get_attributes($this->connect,$i);
> $i = ldap_next_entry($this->connect,$i) ) {
> + $row = array();
> for ($j=0; $j < $arr['count']; $j++) {
> $row[$arr[$j]] = count($arr[$arr[$j]])>2?$arr[$arr[$j]]:$arr[$arr[$j]][0];
> }
> @@ -549,6 +548,7 @@ function sync_LDAP(){
> foreach($c->authenticate_hook['config']['format_updated'] as $k => $v)
> $$k = substr($ldap_timestamp,$v[0],$v[1]);
> $ldap_timestamp = $Y.$m.$d.$H.$M.$S;
> + $valid[$mapping['modified']] = "$Y-$m-$d $H:$M:$S";
> }
> else if ( preg_match('{^(\d{8})(\d{6})(Z)?$', $ldap_timestamp, $matches ) ) {
> $ldap_timestamp = $matches[1].'T'.$matches[2].$matches[3];
> @@ -556,7 +556,6 @@ function sync_LDAP(){
> else if ( empty($ldap_timestamp) ) {
> $ldap_timestamp = date('c');
> }
> - $valid[$mapping['modified']] = $ldap_timestamp;
>
> sync_user_from_LDAP( $principal, $mapping, $valid );
> }
--
------------------------------------------------------------------------
andrew (AT) morphoss (DOT) com +64(272)DEBIAN
Building more free and open source software for everyone.
------------------------------------------------------------------------
|
signature.asc
