[Bps-public-commit] RT-Extension-LDAPImport branch, group-attribute-value, created. 0.31-5-gf7a50b5
Thomas Sibley
trs at bestpractical.com
Thu Feb 16 16:14:11 EST 2012
The branch, group-attribute-value has been created
at f7a50b5006a39d3701ce31cc000e82946c88d875 (commit)
- Log -----------------------------------------------------------------
commit 7128e064124423592bcc7daaf709777165c1b16e
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Feb 16 13:15:51 2012 -0500
Rename _dnlist to the generic _users
This is in preparation for using cache keys other than DN.
diff --git a/lib/RT/Extension/LDAPImport.pm b/lib/RT/Extension/LDAPImport.pm
index c6022d7..fa74866 100644
--- a/lib/RT/Extension/LDAPImport.pm
+++ b/lib/RT/Extension/LDAPImport.pm
@@ -5,7 +5,7 @@ our $VERSION = '0.31';
use warnings;
use strict;
use base qw(Class::Accessor);
-__PACKAGE__->mk_accessors(qw(_ldap _group screendebug _dnlist));
+__PACKAGE__->mk_accessors(qw(_ldap _group screendebug _users));
use Carp;
use Net::LDAP;
use Data::Dumper;
@@ -164,7 +164,7 @@ sub import_users {
my $mapping = $RT::LDAPMapping;
return unless $self->_check_ldap_mapping( mapping => $mapping );
- $self->_dnlist({});
+ $self->_users({});
my $done = 0; my $count = $results->count;
while (my $entry = $results->shift_entry) {
@@ -196,7 +196,7 @@ sub _import_user {
my %args = @_;
$self->_debug("Processing user $args{user}{Name}");
- $self->_dnlist->{lc $args{ldap_entry}->dn} = $args{user}{Name};
+ $self->_users->{lc $args{ldap_entry}->dn} = $args{user}{Name};
$args{user} = $self->create_rt_user( %args );
return unless $args{user};
@@ -714,23 +714,23 @@ sub add_group_members {
$self->_debug("No group in RT, would create with members:");
}
- my $dnlist = $self->_dnlist;
+ my $users = $self->_users;
foreach my $member (@$members) {
my $username;
- if (exists $dnlist->{lc $member}) {
- next unless $username = $dnlist->{lc $member};
+ if (exists $users->{lc $member}) {
+ next unless $username = $users->{lc $member};
} else {
my $ldap_users = $self->_run_search(
base => $member,
filter => $RT::LDAPFilter,
);
unless ( $ldap_users && $ldap_users->count ) {
- $dnlist->{lc $member} = undef;
+ $users->{lc $member} = undef;
$self->_error("No user found for $member who should be a member of $groupname");
next;
}
my $ldap_user = $ldap_users->shift_entry;
- $dnlist->{lc $member} = $username = $ldap_user->get_value($RT::LDAPMapping->{Name});
+ $users->{lc $member} = $username = $ldap_user->get_value($RT::LDAPMapping->{Name});
}
if ( delete $rt_group_members{$username} ) {
$self->_debug("\t$username\tin RT and LDAP");
commit c7c855ad1a32f78d2e0a8221b5febc675ffbe7d0
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Feb 16 15:20:00 2012 -0500
Add Member_Attr_Value to specify the user attribute contained in the Member_Attr of groups
By default we still assume Member_Attr contains a list of user DNs. If
you're running against OpenLDAP's default config, for instance, your
Member_Attr is memberUid which contains uids not DNs. Member_Attr_Value
gives you a way to configure what value is matched against.
Refactors user caching for consistency.
diff --git a/README b/README
index 1e37996..ad66c21 100644
--- a/README
+++ b/README
@@ -92,12 +92,17 @@ The search filter to apply (in this case, find all the bobs)
A mapping of
Attribute in RT => Attribute in LDAP
(this has changed since version 1, which was the other way around)
- Set($LDAPGroupMapping, {Name => 'cn',
- Member_Attr => 'member'});
+ Set($LDAPGroupMapping, {Name => 'cn',
+ Member_Attr => 'member',
+ Member_Attr_Value => 'dn' });
The mapping logic is the same as the LDAPMapping.
-There is one important special-case variable, Member_Attr
-Use this to tell the importer which attribute will contain DNs of group members
+There are two important special-case keys, Member_Attr and Member_Attr_Value.
+Member_Attr tells the importer which attribute contains group members.
+Member_Attr_Value, which defaults to 'dn', specifies what kind of user values
+are in Member_Attr. OpenLDAP, for example, often stores uid instead of dn in
+member.
+
If you do not specify a Description attribute, it will be filled with
'Imported from LDAP'
diff --git a/lib/RT/Extension/LDAPImport.pm b/lib/RT/Extension/LDAPImport.pm
index fa74866..a673733 100644
--- a/lib/RT/Extension/LDAPImport.pm
+++ b/lib/RT/Extension/LDAPImport.pm
@@ -8,6 +8,7 @@ use base qw(Class::Accessor);
__PACKAGE__->mk_accessors(qw(_ldap _group screendebug _users));
use Carp;
use Net::LDAP;
+use Net::LDAP::Util qw(escape_filter_value);
use Data::Dumper;
=head1 NAME
@@ -168,8 +169,7 @@ sub import_users {
my $done = 0; my $count = $results->count;
while (my $entry = $results->shift_entry) {
- my $user = $self->_build_object( ldap_entry => $entry, skip => qr/(?i)^CF\./, mapping => $mapping );
- $user->{Name} ||= $user->{EmailAddress};
+ my $user = $self->_build_user_object( ldap_entry => $entry );
unless ( $user->{Name} ) {
$self->_warn("No Name or Emailaddress for user, skipping ".Dumper $user);
next;
@@ -196,7 +196,7 @@ sub _import_user {
my %args = @_;
$self->_debug("Processing user $args{user}{Name}");
- $self->_users->{lc $args{ldap_entry}->dn} = $args{user}{Name};
+ $self->_cache_user( %args );
$args{user} = $self->create_rt_user( %args );
return unless $args{user};
@@ -207,6 +207,38 @@ sub _import_user {
return 1;
}
+=head2 _cache_user ldap_entry => Net::LDAP::Entry, [user => { ... }]
+
+Adds the user to a global cache which is used when importing groups later.
+
+Optionally takes a second argument which is a user data object returned by
+_build_user_object. If not given, _cache_user will call _build_user_object
+itself.
+
+Returns the user Name.
+
+=cut
+
+sub _cache_user {
+ my $self = shift;
+ my %args = (@_);
+ my $user = $args{user} || $self->_build_user_object( ldap_entry => $args{ldap_entry} );
+
+ my $group_map = $RT::LDAPGroupMapping || {};
+ my $member_attr_val = $group_map->{Member_Attr_Value} || 'dn';
+ my $membership_key = lc $member_attr_val eq 'dn'
+ ? $args{ldap_entry}->dn
+ : $args{ldap_entry}->get_value($member_attr_val);
+
+ # Fallback to the DN if the user record doesn't have a value
+ unless (defined $membership_key) {
+ $membership_key = $args{ldap_entry}->dn;
+ $self->_warn("User attribute '$member_attr_val' has no value for '$membership_key'; falling back to DN");
+ }
+
+ return $self->_users->{lc $membership_key} = $user->{Name};
+}
+
sub _show_user_info {
my $self = shift;
my %args = @_;
@@ -253,10 +285,28 @@ sub _check_ldap_mapping {
return 1;
}
+=head2 _build_user_object
+
+Utility method which wraps _build_object to provide sane defaults for building
+users. It also tries to ensure a Name exists in the returned object.
+
+=cut
+
+sub _build_user_object {
+ my $self = shift;
+ my $user = $self->_build_object(
+ skip => qr/(?i)^CF\./,
+ mapping => $RT::LDAPMapping,
+ @_
+ );
+ $user->{Name} ||= $user->{EmailAddress};
+ return $user;
+}
+
=head2 _build_object
Builds up data from LDAP for importing
-Returns a hash of user data ready for RT::User::Create
+Returns a hash of user or group data ready for RT::User::Create or RT::Group::Create
=cut
@@ -680,7 +730,7 @@ sub create_rt_group {
=head3 add_group_members
-Iterate over the list of DNs in the Member_Attr LDAP entry.
+Iterate over the list of values in the Member_Attr LDAP entry.
Look up the appropriate username from LDAP.
Add those users to the group.
Remove members of the RT Group who are no longer members
@@ -721,8 +771,9 @@ sub add_group_members {
next unless $username = $users->{lc $member};
} else {
my $ldap_users = $self->_run_search(
- base => $member,
- filter => $RT::LDAPFilter,
+ base => $RT::LDAPBase,
+ filter => "(&$RT::LDAPFilter($RT::LDAPGroupMapping->{Member_Attr_Value}="
+ . escape_filter_value($member) . "))"
);
unless ( $ldap_users && $ldap_users->count ) {
$users->{lc $member} = undef;
@@ -730,7 +781,7 @@ sub add_group_members {
next;
}
my $ldap_user = $ldap_users->shift_entry;
- $users->{lc $member} = $username = $ldap_user->get_value($RT::LDAPMapping->{Name});
+ $username = $self->_cache_user( ldap_entry => $ldap_user );
}
if ( delete $rt_group_members{$username} ) {
$self->_debug("\t$username\tin RT and LDAP");
commit f7a50b5006a39d3701ce31cc000e82946c88d875
Author: Thomas Sibley <trs at bestpractical.com>
Date: Thu Feb 16 16:08:57 2012 -0500
Tests for Member_Attr_Value
diff --git a/t/group-import.t b/t/group-import.t
index bc60565..6d28e80 100644
--- a/t/group-import.t
+++ b/t/group-import.t
@@ -1,7 +1,7 @@
use strict;
use warnings;
use lib 't/lib';
-use RT::Extension::LDAPImport::Test tests => 43;
+use RT::Extension::LDAPImport::Test tests => 66;
eval { require Net::LDAP::Server::Test; 1; } or do {
plan skip_all => 'Unable to test without Net::Server::LDAP::Test';
};
@@ -40,6 +40,7 @@ for ( 1 .. 4 ) {
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] );
@@ -89,40 +90,55 @@ ok( $importer->import_groups() );
is($groups->Count,0);
}
-ok( $importer->import_groups( import => 1 ) );
-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}++;
- }
+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" );
- foreach my $dn ( @{$entry->{members}} ) {
- my ($user) = grep { $_->{dn} eq $dn } @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;
+ 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};
}
- 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");
}
- is(keys %$idlist,0,"No dangling users");
-}
-my $group = RT::Group->new($RT::SystemUser);
-$group->LoadUserDefinedGroup( "42" );
-ok( !$group->Id );
+ my $group = RT::Group->new($RT::SystemUser);
+ $group->LoadUserDefinedGroup( "42" );
+ ok( !$group->Id );
-$group->LoadByCols(
- Domain => 'UserDefined',
- Name => "42",
-);
-ok( !$group->Id );
+ $group->LoadByCols(
+ Domain => 'UserDefined',
+ Name => "42",
+ );
+ ok( !$group->Id );
+}
-----------------------------------------------------------------------
More information about the Bps-public-commit
mailing list