[Bps-public-commit] RT-Extension-LDAPImport branch, master, updated. 0.35-10-gdfbae86

Alex Vandiver alexmv at bestpractical.com
Mon Sep 22 16:06:47 EDT 2014


The branch, master has been updated
       via  dfbae866054ced47f86c8b9c790a25016729ee45 (commit)
       via  f731f0c8f611e8b58d7cadcea4b4a1324a71bf0b (commit)
       via  ff51843c6d1ecb55eccb9f853427484d1260d54c (commit)
       via  b71444d0c44ee18d0c5e33c9f019f20ea7dca18d (commit)
       via  d20095ce0d1656d7f614e3f6fbd5b821b40615f8 (commit)
       via  c33d933d51c0fad4a0cc82fe236fbecf674b910d (commit)
       via  b7e5d5b936fde0a75367a8d004d7a839f2bf9a25 (commit)
      from  b16f80a985092a692cb3ca9c80f4b3cd4aaabd8f (commit)

Summary of changes:
 lib/RT/Extension/LDAPImport.pm               | 71 +++++++++++++++++++++++++---
 xt/{group-import.t => group-member-import.t} | 26 +++++-----
 2 files changed, 79 insertions(+), 18 deletions(-)
 copy xt/{group-import.t => group-member-import.t} (91%)

- Log -----------------------------------------------------------------
commit b7e5d5b936fde0a75367a8d004d7a839f2bf9a25
Author: Thomas Sibley <trsibley at uw.edu>
Date:   Wed Jan 22 10:02:11 2014 -0800

    Refactor the LDAP user entry fetching from the actual importing of those entries
    
    Internal code can now call _import_users separately with an arbitrary
    set of LDAP user entries.
    
    The cache reset is kept in import_users so that multiple internal calls
    to _import_users (not that multiples exist yet) will populate the same
    cache by default which should be safe and may save LDAP lookups during
    group membership sync.

diff --git a/lib/RT/Extension/LDAPImport.pm b/lib/RT/Extension/LDAPImport.pm
index f999a19..10ee3e9 100644
--- a/lib/RT/Extension/LDAPImport.pm
+++ b/lib/RT/Extension/LDAPImport.pm
@@ -526,9 +526,19 @@ sub import_users {
     my $self = shift;
     my %args = @_;
 
+    $self->_users({});
+
     my @results = $self->run_user_search;
-    unless ( @results ) {
-        $self->_debug("No results found, no import");
+    return $self->_import_users( %args, users => \@results );
+}
+
+sub _import_users {
+    my $self = shift;
+    my %args = @_;
+    my $users = $args{users};
+
+    unless ( @$users ) {
+        $self->_debug("No users found, no import");
         $self->disconnect_ldap;
         return;
     }
@@ -536,10 +546,8 @@ sub import_users {
     my $mapping = $RT::LDAPMapping;
     return unless $self->_check_ldap_mapping( mapping => $mapping );
 
-    $self->_users({});
-
-    my $done = 0; my $count = scalar @results;
-    while (my $entry = shift @results) {
+    my $done = 0; my $count = scalar @$users;
+    while (my $entry = shift @$users) {
         my $user = $self->_build_user_object( ldap_entry => $entry );
         $self->_import_user( user => $user, ldap_entry => $entry, import => $args{import} );
         $done++;

commit c33d933d51c0fad4a0cc82fe236fbecf674b910d
Author: Thomas Sibley <trsibley at uw.edu>
Date:   Wed Jan 22 12:26:23 2014 -0800

    Optionally accept a scope parameter for _run_search, defaulting to 'sub'
    
    When appropriate to the query, limiting the scope to 'base' or 'one' is
    more efficient.
    
    Subtree is the default of Net::LDAP and relied upon by all existing
    queries.  Making it the explicit default is good for forward compat if
    the Net::LDAP default changes.

diff --git a/lib/RT/Extension/LDAPImport.pm b/lib/RT/Extension/LDAPImport.pm
index 10ee3e9..5dbf1d6 100644
--- a/lib/RT/Extension/LDAPImport.pm
+++ b/lib/RT/Extension/LDAPImport.pm
@@ -437,6 +437,7 @@ sub _run_search {
     my %search = (
         base    => $args{base},
         filter  => $args{filter},
+        scope   => ($args{scope} || 'sub'),
     );
     my (@results, $page, $cookie);
 

commit d20095ce0d1656d7f614e3f6fbd5b821b40615f8
Author: Thomas Sibley <trsibley at uw.edu>
Date:   Wed Jan 22 12:29:31 2014 -0800

    Limit scope to 'base' when looking up a DN
    
    The DN is the base so we don't need to query the entire subtree (if
    any).

diff --git a/lib/RT/Extension/LDAPImport.pm b/lib/RT/Extension/LDAPImport.pm
index 5dbf1d6..c39ba34 100644
--- a/lib/RT/Extension/LDAPImport.pm
+++ b/lib/RT/Extension/LDAPImport.pm
@@ -1388,11 +1388,13 @@ sub add_group_members {
         } else {
             my $attr    = lc($RT::LDAPGroupMapping->{Member_Attr_Value} || 'dn');
             my $base    = $attr eq 'dn' ? $member : $RT::LDAPBase;
+            my $scope   = $attr eq 'dn' ? 'base'  : 'sub';
             my $filter  = $attr eq 'dn'
                             ? $RT::LDAPFilter
                             : "(&$RT::LDAPFilter($attr=" . escape_filter_value($member) . "))";
             my @results = $self->_run_search(
                 base   => $base,
+                scope  => $scope,
                 filter => $filter,
             );
             unless ( @results ) {

commit b71444d0c44ee18d0c5e33c9f019f20ea7dca18d
Author: Thomas Sibley <trsibley at uw.edu>
Date:   Wed Jan 22 12:57:42 2014 -0800

    Optionally import group members before syncing membership
    
    Adds the config option $LDAPImportGroupMembers.

diff --git a/lib/RT/Extension/LDAPImport.pm b/lib/RT/Extension/LDAPImport.pm
index c39ba34..c8ec1a7 100644
--- a/lib/RT/Extension/LDAPImport.pm
+++ b/lib/RT/Extension/LDAPImport.pm
@@ -280,6 +280,22 @@ up with two groups in RT.
 You can provide a C<Description> key which will be added as the group
 description in RT. The default description is 'Imported from LDAP'.
 
+=item C<< Set($LDAPImportGroupMembers, 1); >>
+
+When disabled, the default, LDAP group import expects that all LDAP members
+already exist as RT users.  Often the user import stage, which happens before
+groups, is used to create and/or update group members by using an
+C<$LDAPFilter> which includes a C<memberOf> attribute.
+
+When enabled, by setting to C<1>, LDAP group members are explicitly imported
+before membership is synced with RT.  This enables groups-only configurations
+to also import group members without specifying a potentially long and complex
+C<$LDAPFilter> using C<memberOf>.  It's particularly handy when C<memberOf>
+isn't available on user entries.
+
+Note that C<$LDAPFilter> still applies when this option is enabled, so some
+group members may be filtered out from the import.
+
 =item C<< Set($LDAPSizeLimit, 1000); >>
 
 You can set this value if your LDAP server has result size limits.
@@ -1370,6 +1386,37 @@ sub add_group_members {
         return;
     }
 
+    if ($RT::LDAPImportGroupMembers) {
+        $self->_debug("Importing members of group $groupname");
+        my @entries;
+        my $attr = lc($RT::LDAPGroupMapping->{Member_Attr_Value} || 'dn');
+
+        # Lookup each DN's full entry, or...
+        if ($attr eq 'dn') {
+            @entries = grep defined, map {
+                my @results = $self->_run_search(
+                    scope   => 'base',
+                    base    => $_,
+                    filter  => $RT::LDAPFilter,
+                );
+                $results[0]
+            } @$members;
+        }
+        # ...or find all the entries in a single search by attribute.
+        else {
+            # I wonder if this will run into filter length limits? -trs, 22 Jan 2014
+            my $members = join "", map { "($attr=" . escape_filter_value($_) . ")" } @$members;
+            @entries = $self->_run_search(
+                base   => $RT::LDAPBase,
+                filter => "(&$RT::LDAPFilter(|$members))",
+            );
+        }
+        $self->_import_users(
+            import  => $args{import},
+            users   => \@entries,
+        ) or $self->_debug("Importing group members failed");
+    }
+
     my %rt_group_members;
     if ($args{group} and not $args{new}) {
         my $user_members = $group->UserMembersObj( Recursively => 0);

commit ff51843c6d1ecb55eccb9f853427484d1260d54c
Author: Thomas Sibley <trsibley at uw.edu>
Date:   Wed Jan 22 14:56:36 2014 -0800

    Tests for $LDAPImportGroupMembers based on xt/group-import.t

diff --git a/xt/group-member-import.t b/xt/group-member-import.t
new file mode 100644
index 0000000..c8babda
--- /dev/null
+++ b/xt/group-member-import.t
@@ -0,0 +1,147 @@
+use strict;
+use warnings;
+use lib 'xt/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 $_ ".int rand(200),
+                    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,
+        members => [ map { $_->{dn} } @ldap_user_entries[($_-1),($_+3),($_+7)] ],
+        memberUid => [ map { $_->{uid} } @ldap_user_entries[($_+1),($_+3),($_+5)] ],
+        objectClass => 'Group',
+    };
+    $ldap->add( $dn, attr => [%$entry] );
+    push @ldap_group_entries, $entry;
+}
+$ldap->add(
+    "cn=42,ou=groups,dc=bestpractical,dc=com",
+    attr => [
+        cn => "42",
+        members => [ "uid=testuser1,ou=foo,dc=bestpractical,dc=com" ],
+        objectClass => 'Group',
+    ],
+);
+
+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  => 'members',
+                   });
+RT->Config->Set('LDAPImportGroupMembers',1);
+
+$importer->screendebug(1) if ($ENV{TEST_VERBOSE});
+
+# confirm that we skip the import
+ok( $importer->import_groups() );
+{
+    my $groups = RT::Groups->new($RT::SystemUser);
+    $groups->LimitToUserDefinedGroups;
+    is($groups->Count,0);
+}
+
+import_group_members_ok( members => 'dn' );
+
+RT->Config->Set('LDAPGroupMapping',
+                   {Name                => 'cn',
+                    Member_Attr         => 'memberUid',
+                    Member_Attr_Value   => 'uid',
+                   });
+import_group_members_ok( memberUid => 'uid' );
+
+sub import_group_members_ok {
+    my $attr = shift;
+    my $user_attr = shift;
+
+    ok( $importer->import_groups( import => 1 ), "imported groups" );
+
+    for my $entry (@ldap_user_entries) {
+        my $user = RT::User->new($RT::SystemUser);
+        $user->LoadByCols( EmailAddress => $entry->{mail},
+                           Realname => $entry->{cn},
+                           Name => $entry->{uid} );
+        ok($user->Id, "Found $entry->{cn} as ".$user->Id);
+    }
+
+    for my $entry (@ldap_group_entries) {
+        my $group = RT::Group->new($RT::SystemUser);
+        $group->LoadUserDefinedGroup( $entry->{cn} );
+        ok($group->Id, "Found $entry->{cn} as ".$group->Id);
+
+        my $idlist;
+        my $members = $group->MembersObj;
+        while (my $group_member = $members->Next) {
+            my $member = $group_member->MemberObj;
+            next unless $member->IsUser();
+            $idlist->{$member->Object->Id}++;
+        }
+
+        foreach my $member ( @{$entry->{$attr}} ) {
+            my ($user) = grep { $_->{$user_attr} eq $member } @ldap_user_entries;
+            my $rt_user = RT::User->new($RT::SystemUser);
+            my ($res,$msg) = $rt_user->Load($user->{uid});
+            unless ($res) {
+                diag("Couldn't load user $user->{uid}: $msg");
+                next;
+            }
+            ok($group->HasMember($rt_user->PrincipalObj->Id),"Correctly assigned $user->{uid} to $entry->{cn}");
+            delete $idlist->{$rt_user->Id};
+        }
+        is(keys %$idlist,0,"No dangling users");
+    }
+
+    my $group = RT::Group->new($RT::SystemUser);
+    $group->LoadUserDefinedGroup( "42" );
+    ok( !$group->Id );
+
+    $group->LoadByCols(
+        Domain => 'UserDefined',
+        Name   => "42",
+    );
+    ok( !$group->Id );
+}
+
+done_testing;

commit f731f0c8f611e8b58d7cadcea4b4a1324a71bf0b
Author: Thomas Sibley <trsibley at uw.edu>
Date:   Wed Jan 22 15:44:00 2014 -0800

    Unset _ldap object after disconnect
    
    Otherwise subsequent calls to _run_search will use the disconnected
    object (to no avail) instead of connecting again.

diff --git a/lib/RT/Extension/LDAPImport.pm b/lib/RT/Extension/LDAPImport.pm
index c8ec1a7..f500bd1 100644
--- a/lib/RT/Extension/LDAPImport.pm
+++ b/lib/RT/Extension/LDAPImport.pm
@@ -1544,6 +1544,7 @@ sub disconnect_ldap {
 
     $ldap->unbind;
     $ldap->disconnect;
+    $self->_ldap(undef);
     return;
 }
 

commit dfbae866054ced47f86c8b9c790a25016729ee45
Merge: b16f80a f731f0c
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Sep 22 16:05:11 2014 -0400

    Merge branch 'ldap-group-members'


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


More information about the Bps-public-commit mailing list