[Bps-public-commit] RT-Extension-LDAPImport branch, useful-callbacks, created. 0.34-7-gc6accae

Ruslan Zakirov ruz at bestpractical.com
Thu Feb 14 13:06:24 EST 2013


The branch, useful-callbacks has been created
        at  c6accae169591aca63d57ef2a944c43fabf752c1 (commit)

- Log -----------------------------------------------------------------
commit bb27e95492577ca5938f37853a7eae047bf0cb25
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Thu Feb 14 03:33:58 2013 +0400

    make callbacks (subrefs) in maps useful
    
    How callbacks were called before was completly useless -
    no context and ldap attribute expected not computed value.
    It's hardly backwards incompatible change as there was no
    good reason to use them.
    
    Drop flattening nested arrays in maps. Only one level deep
    arrays are supported now.
    
    New callbacks are called with enough context to change almost
    everything and should return computed value.

diff --git a/lib/RT/Extension/LDAPImport.pm b/lib/RT/Extension/LDAPImport.pm
index 421ebe6..ba3b445 100644
--- a/lib/RT/Extension/LDAPImport.pm
+++ b/lib/RT/Extension/LDAPImport.pm
@@ -584,29 +584,13 @@ C<RT::User::Create> or C<RT::Group::Create>.
 sub _build_object {
     my $self = shift;
     my %args = @_;
-    my $mapping = $args{mapping};
-
-    my $object = {};
-    foreach my $rtfield ( keys %{$mapping} ) {
-        next if $rtfield =~ $args{skip};
-        my $ldap_attribute = $mapping->{$rtfield};
 
-        my @attributes = $self->_parse_ldap_mapping($ldap_attribute);
-        unless (@attributes) {
-            $self->_error("Invalid LDAP mapping for $rtfield ".Dumper($ldap_attribute));
-            next;
-        }
-        my @values;
-        foreach my $attribute (@attributes) {
-            #$self->_debug("fetching value for $attribute and storing it in $rtfield");
-            # otherwise we'll pull 7 alternate names out of the Name field
-            # this may want to be configurable
-            push @values, scalar $args{ldap_entry}->get_value($attribute);
-        }
-        $object->{$rtfield} = join(' ',grep {defined} @values);
+    my $res = $self->_parse_ldap_mapping( %args );
+    foreach my $value ( values %$res ) {
+        @$value = map { ref $_ eq 'ARRAY'? $_->[0] : $_ } @$value;
+        $value = join ' ', grep defined && length, @$value;
     }
-
-    return $object;
+    return $res;
 }
 
 =head3 _parse_ldap_mapping
@@ -629,18 +613,45 @@ together.
 
 sub _parse_ldap_mapping {
     my $self = shift;
-    my $mapping = shift;
-
-    if (ref $mapping eq 'ARRAY') {
-        return map { $self->_parse_ldap_mapping($_) } @$mapping;
-    } elsif (ref $mapping eq 'CODE') {
-        return map { $self->_parse_ldap_mapping($_) } $mapping->()
-    } elsif (ref $mapping) {
-        $self->_error("Invalid type of LDAPMapping [$mapping]");
-        return;
-    } else {
-        return $mapping;
+    my %args = @_;
+
+    my $mapping = $args{mapping};
+
+    my %res;
+    foreach my $rtfield ( keys %$mapping ) {
+        next if $args{'skip'} && $rtfield =~ $args{'skip'};
+        next if $args{'only'} && $rtfield !~ $args{'only'};
+
+        my $ldap_field = $mapping->{$rtfield};
+        my @list = grep defined && length, ref $ldap_field eq 'ARRAY'? @$ldap_field : ($ldap_field);
+        unless (@list) {
+            $self->_error("Invalid LDAP mapping for $rtfield, no defined fields");
+            next;
+        }
+
+        my @values;
+        foreach my $e (@list) {
+            if (ref $e eq 'CODE') {
+                push @values, $e->(
+                    %args,
+                    self => $self,
+                    rt_field => $rtfield,
+                    ldap_field => $ldap_field,
+                    result => \%res,
+                );
+            } elsif (ref $e) {
+                $self->_error("Invalid type of LDAP mapping for $rtfield, value is $e");
+                next;
+            } else {
+                # XXX: get_value asref returns undef if there is no such field on
+                # the entry, should we warn?
+                push @values, grep defined, $args{'ldap_entry'}->get_value( $e, asref => 1 );
+            }
+        }
+        $res{ $rtfield } = \@values;
     }
+
+    return \%res;
 }
 
 =head2 create_rt_user
@@ -799,25 +810,18 @@ sub add_custom_field_value {
     my %args = @_;
     my $user = $args{user};
 
-    foreach my $rtfield ( keys %{$RT::LDAPMapping} ) {
+    my $data = $self->_build_object(
+        %args,
+        only => qr/^CF\.(.+)$/i,
+        mapping => $RT::LDAPMapping,
+    );
+
+    foreach my $rtfield ( keys %$data ) {
         next unless $rtfield =~ /^CF\.(.+)$/i;
         my $cf_name = $1;
-        my $ldap_attribute = $RT::LDAPMapping->{$rtfield};
 
-        my @attributes = $self->_parse_ldap_mapping($ldap_attribute);
-        unless (@attributes) {
-            $self->_error("Invalid LDAP mapping for $rtfield ".Dumper($ldap_attribute));
-            next;
-        }
-        my @values;
-        foreach my $attribute (@attributes) {
-            #$self->_debug("fetching value for $attribute and storing it in $rtfield");
-            # otherwise we'll pull 7 alternate names out of the Name field
-            # this may want to be configurable
-            push @values, scalar $args{ldap_entry}->get_value($attribute);
-        }
-        my $cfv_name = join(' ', at values); 
-        next unless $cfv_name;
+        my $cfv_name = $data->{ $rtfield }
+            or next;
 
         my $cf = RT::CustomField->new($RT::SystemUser);
         my ($status, $msg) = $cf->Load($cf_name);
@@ -866,21 +870,18 @@ sub update_object_custom_field_values {
     my %args = @_;
     my $obj  = $args{object};
 
-    foreach my $rtfield ( keys %{$RT::LDAPMapping} ) {
+    my $data = $self->_build_object(
+        %args,
+        only => qr/^UserCF\.(.+)$/i,
+        mapping => $RT::LDAPMapping,
+    );
+
+    foreach my $rtfield ( keys %$data ) {
         # XXX TODO: accept GroupCF when we call this from group_import too
         next unless $rtfield =~ /^UserCF\.(.+)$/i;
         my $cf_name = $1;
-        my $ldap_attribute = $RT::LDAPMapping->{$rtfield};
-
-        my @attributes = $self->_parse_ldap_mapping($ldap_attribute);
-        unless (@attributes) {
-            $self->_error("Invalid LDAP mapping for $rtfield ".Dumper($ldap_attribute));
-            next;
-        }
-        my $value = join ' ',
-                    grep { defined and length }
-                     map { scalar $args{ldap_entry}->get_value($_) }
-                         @attributes;
+        # XXX TODO: value can not be undefined, but empty string
+        my $value = $data->{$rtfield};
 
         my $current = $obj->FirstCustomFieldValue($cf_name);
 

commit c6accae169591aca63d57ef2a944c43fabf752c1
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Thu Feb 14 22:00:18 2013 +0400

    make it possible to use sub in Member_Attr
    
    allows to convert members or do other things

diff --git a/lib/RT/Extension/LDAPImport.pm b/lib/RT/Extension/LDAPImport.pm
index ba3b445..912756b 100644
--- a/lib/RT/Extension/LDAPImport.pm
+++ b/lib/RT/Extension/LDAPImport.pm
@@ -934,7 +934,18 @@ sub import_groups {
 
     my $done = 0; my $count = scalar @results;
     while (my $entry = shift @results) {
-        my $group = $self->_build_object( ldap_entry => $entry, skip => qr/(?i)^Member_Attr/, mapping => $mapping );
+        my $group = $self->_parse_ldap_mapping(
+            %args,
+            ldap_entry => $entry,
+            skip => qr/^Member_Attr_Value$/i,
+            mapping => $mapping,
+        );
+        foreach my $key ( grep !/^Member_Attr/, keys %$group ) {
+            @{ $group->{$key} } = map { ref $_ eq 'ARRAY'? $_->[0] : $_ } @{ $group->{$key} };
+            $group->{$key} = join ' ', grep defined && length, @{ $group->{$key} };
+        }
+        @{ $group->{'Member_Attr'} } = map { ref $_ eq 'ARRAY'? @$_ : $_  } @{ $group->{'Member_Attr'} }
+            if $group->{'Member_Attr'};
         $group->{Description} ||= 'Imported from LDAP';
         unless ( $group->{Name} ) {
             $self->_warn("No Name for group, skipping ".Dumper $group);
@@ -987,7 +998,14 @@ sub _import_group {
     $self->_debug("Processing group $group->{Name}");
     my ($group_obj, $created) = $self->create_rt_group( %args, group => $group );
     return if $args{import} and not $group_obj;
-    $self->add_group_members( %args, name => $group->{Name}, group => $group_obj, ldap_entry => $ldap_entry, new => $created );
+    $self->add_group_members(
+        %args,
+        name => $group->{Name},
+        info => $group,
+        group => $group_obj,
+        ldap_entry => $ldap_entry,
+        new => $created,
+    );
     # XXX TODO: support OCFVs for groups too
     return;
 }
@@ -1015,6 +1033,8 @@ sub create_rt_group {
     my $group_obj = $self->find_rt_group(%args);
     return unless defined $group_obj;
 
+    $group = { map { $_ => $group->{$_} } qw(id Name Description) };
+
     my $id = delete $group->{'id'};
 
     my $created;
@@ -1165,8 +1185,7 @@ sub add_group_members {
 
     $self->_debug("Processing group membership for $groupname");
 
-    my $members = $self->_get_group_members_from_ldap(%args);
-
+    my $members = $args{'info'}{'Member_Attr'};
     unless (defined $members) {
         $self->_warn("No members found for $groupname in Member_Attr");
         return;
@@ -1235,17 +1254,6 @@ sub add_group_members {
     }
 }
 
-sub _get_group_members_from_ldap {
-    my $self = shift;
-    my %args = @_;
-    my $ldap_entry = $args{ldap_entry};
-
-    my $mapping = $RT::LDAPGroupMapping;
-
-    my $members = $ldap_entry->get_value($mapping->{Member_Attr}, asref => 1);
-}
-
-
 =head2 _show_group
 
 Show debugging information about the group record we're going to import
diff --git a/t/group-callbacks.t b/t/group-callbacks.t
new file mode 100644
index 0000000..e485c83
--- /dev/null
+++ b/t/group-callbacks.t
@@ -0,0 +1,108 @@
+use strict;
+use warnings;
+use lib 't/lib';
+use RT::Extension::LDAPImport::Test tests => undef;
+eval { require Net::LDAP::Server::Test; 1; } or do {
+    plan skip_all => 'Unable to test without Net::Server::LDAP::Test';
+};
+
+use Net::LDAP::Entry;
+use RT::User;
+
+my $importer = RT::Extension::LDAPImport->new;
+isa_ok($importer,'RT::Extension::LDAPImport');
+
+my $ldap_port = 1024 + int rand(10000) + $$ % 1024;
+ok( my $server = Net::LDAP::Server::Test->new( $ldap_port, auto_schema => 1 ),
+    "spawned test LDAP server on port $ldap_port");
+my $ldap = Net::LDAP->new("localhost:$ldap_port");
+$ldap->bind();
+$ldap->add("dc=bestpractical,dc=com");
+
+my @ldap_user_entries;
+for ( 1 .. 12 ) {
+    my $username = "testuser$_";
+    my $dn = "uid=$username,ou=foo,dc=bestpractical,dc=com";
+    my $entry = {
+                    dn   => $dn,
+                    cn   => "Test User $_",
+                    mail => "$username\@invalid.tld",
+                    uid  => $username,
+                    objectClass => 'User',
+                };
+    push @ldap_user_entries, $entry;
+    $ldap->add( $dn, attr => [%$entry] );
+}
+
+my @ldap_group_entries;
+for ( 1 .. 4 ) {
+    my $groupname = "Test Group $_";
+    my $dn = "cn=$groupname,ou=groups,dc=bestpractical,dc=com";
+    my $entry = {
+        cn          => $groupname,
+        gid         => $_,
+        members     => [ map { 'mail="'. $_->{'mail'} .'"' } @ldap_user_entries[($_-1),($_+3),($_+7)] ],
+        objectClass => 'Group',
+    };
+    $ldap->add( $dn, attr => [%$entry] );
+    push @ldap_group_entries, $entry;
+}
+
+RT->Config->Set('LDAPHost',"ldap://localhost:$ldap_port");
+RT->Config->Set('LDAPMapping',
+                   {Name         => 'uid',
+                    EmailAddress => 'mail',
+                    RealName     => 'cn'});
+RT->Config->Set('LDAPBase','dc=bestpractical,dc=com');
+RT->Config->Set('LDAPFilter','(objectClass=User)');
+RT->Config->Set('LDAPSkipAutogeneratedGroup',1);
+
+RT->Config->Set('LDAPGroupBase','dc=bestpractical,dc=com');
+RT->Config->Set('LDAPGroupFilter','(objectClass=Group)');
+RT->Config->Set('LDAPGroupMapping', {
+    Name         => 'cn',
+    Member_Attr  => sub {
+        my %args = @_;
+        my $self = $args{'self'};
+        my $members = $args{ldap_entry}->get_value('members', asref => 1);
+        foreach my $record ( @$members ) {
+            my $user = RT::User->new( RT->SystemUser );
+            $user->LoadByEmail($record =~ /mail="(.*)"/);
+            $self->_users->{ lc $record } = $user->Name;
+        }
+        return @$members;
+    },
+});
+
+ok( $importer->import_users( import => 1 ), 'imported users');
+# no id mapping
+{
+    ok( $importer->import_groups( import => 1 ), "imported groups" );
+
+    is_member_of('testuser1', 'Test Group 1');
+}
+
+done_testing;
+
+sub is_member_of {
+    my $uname = shift;
+    my $gname = shift;
+
+    my $group = get_group($gname);
+    return ok(0, "found group $gname") unless $group->id;
+
+    my $user = RT::User->new($RT::SystemUser);
+    $user->Load( $uname );
+    return ok(0, "found user $uname") unless $user->id;
+
+    return ok($group->HasMember($user->id), "$uname is member of $gname");
+}
+
+sub get_group {
+    my $gname = shift;
+    my $group = RT::Group->new($RT::SystemUser);
+    $group->LoadUserDefinedGroup( $gname );
+    return $group;
+}
+
+

-----------------------------------------------------------------------



More information about the Bps-public-commit mailing list