[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