| Subject: | Performing batch add/remove of users on a group object fails |
Performing multiple add_user/remove_user on a group object and then
performing a $group->update fails to properly update the group membership.
Specifically each add_user/remove_user invocation resets the internal
users list.
Attached is a test that demonstrates the issue as well as a patch I
believe address the issue.
Additionally, the action_for_update appears to have a side-effect in how
it manipulates the internal users list which can mask the behavior. The
attached files do not address this possible issue.
Thanks, if you require any further information please let me know.
| Subject: | batch-add-remove.t |
use Test::More tests => 46;
use strict;
use_ok('Net::LDAP::Class');
use_ok('Net::LDAP::Class::User::AD');
use_ok('Net::LDAP::Class::Group::AD');
use lib 't/lib';
use Net::LDAP;
use Data::Dump qw( dump );
use LDAPTestAD;
ok( my $server = LDAPTestAD::spawn_server(), "spawn server" );
ok( my $ldap = Net::LDAP->new(
LDAPTestAD::server_host(), port => LDAPTestAD::server_port()
),
"connect to server"
);
{
package MyLDAPUser;
use base 'Net::LDAP::Class::User::AD';
__PACKAGE__->metadata->setup(
base_dn => 'dc=test,dc=local',
attributes => __PACKAGE__->AD_attributes,
unique_attributes => __PACKAGE__->AD_unique_attributes,
);
sub init_group_class {'MyLDAPGroup'}
}
{
package MyLDAPGroup;
use base 'Net::LDAP::Class::Group::AD';
__PACKAGE__->metadata->setup(
base_dn => 'dc=test,dc=local',
attributes => __PACKAGE__->AD_attributes,
unique_attributes => __PACKAGE__->AD_unique_attributes,
);
sub init_user_class {'MyLDAPUser'}
}
ok( $ldap->bind, "bind to server" );
ok( my $group = MyLDAPGroup->new(
cn => 'foogroup',
ldap => $ldap,
),
"new group"
);
ok( $group->create, "create $group" );
### Test of removing multiple users.
my $R = 3;
for my $n ( 1 .. $R ) {
ok( my $u = MyLDAPUser->new(
sAMAccountName => $n,
ldap => $ldap,
groups => [$group],
cn => $n,
),
"new user $n"
);
ok( $u->create, "create user $u" );
}
ok( $group->read, "group update ok" );
for my $n ( 1 .. $R ){
ok( my $u = MyLDAPUser->new( ldap => $ldap, username => $n )->read, "user $n read ok");
ok( $group->has_user($u), "group has user $n");
}
for my $n ( 1 .. $R ){
my $u = MyLDAPUser->new( ldap => $ldap, username => $n );
ok( $group->remove_user( $u ), "removed user ok" );
}
ok( $group->update, "group update succeded" );
for my $n ( 1 .. $R ){
ok( my $u = MyLDAPUser->new( ldap => $ldap, username => $n )->read, "$n read ok");
ok( ! $group->has_user($u), "group doesn't have user $n");
}
### Test of adding multiple users.
ok( my $group2 = MyLDAPGroup->new(
cn => 'bargroup',
ldap => $ldap,
),
"new group2"
);
ok( $group2->create, "group2 create ok" );
for my $n ( 1 .. $R ){
ok( my $u = MyLDAPUser->new( ldap => $ldap, username => $n )->read, "user $n read ok");
ok( $group2->add_user( $u ), "added user $u ok");
}
ok( $group2->update, "group2 update ok");
for my $n ( 1 .. $R ){
ok( my $u = MyLDAPUser->new( ldap => $ldap, username => $n )->read, "user $n read ok");
ok( $group2->has_user( $u ), "has post-update user $u ok");
}
| Subject: | batch-user.patch |
diff --git a/lib/Net/LDAP/Class/Group/AD.pm b/lib/Net/LDAP/Class/Group/AD.pm
index c76bf47..117b0a9 100644
--- a/lib/Net/LDAP/Class/Group/AD.pm
+++ b/lib/Net/LDAP/Class/Group/AD.pm
@@ -390,12 +390,15 @@ sub add_user {
croak
"User object must have at least a username before adding to group $self";
}
- for my $u ( $self->secondary_users ) {
+ if( not defined $self->{users} ){
+ $self->{users} = $self->secondary_users;
+ }
+ my @users = @{ $self->{users} };
+ for my $u ( @users ) {
if ( "$u" eq "$user" ) {
croak "User $user is already a member of group $self";
}
}
- my @users = $self->secondary_users;
push( @users, $user );
$self->{users} = \@users;
}
@@ -418,7 +421,11 @@ sub remove_user {
croak
"User object must have at least a username before removing from group $self";
}
- my %users = map { $_->username => $_ } @{ $self->secondary_users };
+ if( not defined $self->{users} ){
+ $self->{users} = $self->secondary_users;
+ }
+ my @users = @{ $self->{users} };
+ my %users = map { $_->username => $_ } @users;
if ( !exists $users{ $user->username } ) {
croak "User $user is not a member of group $self";
}