handle group renames with new option

README

@@ -139,6 +139,13 @@ CONFIGURATION
         identifier (dn or other LDAP field) on a user record means the user
         will be added to that group in RT.
 
+        "id" is the field in LDAP group record that uniquely identifies the
+        group. This is optional and shouldn't be equal to mapping for Name
+        field. Group names in RT must be distinct and you don't need another
+        unique identifier in common situation. However, when you rename a
+        group in LDAP, without this option set properly you end up with two
+        groups in RT.
+
         You can provide a "Description" key which will be added as the group
         description in RT. The default description is 'Imported from LDAP'.
 

lib/RT/Extension/LDAPImport.pm

@@ -174,6 +174,13 @@ A match between the member field on the group record and this
 identifier (dn or other LDAP field) on a user record means the
 user will be added to that group in RT.
 
+C<id> is the field in LDAP group record that uniquely identifies
+the group. This is optional and shouldn't be equal to mapping for
+Name field. Group names in RT must be distinct and you don't need
+another unique identifier in common situation. However, when you
+rename a group in LDAP, without this option set properly you end
+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'.
 
@@ -1004,8 +1011,10 @@ sub create_rt_group {
     my %args = @_;
     my $group = $args{group};
 
-    my $group_obj = RT::Group->new($RT::SystemUser);
-    $group_obj->LoadUserDefinedGroup( $group->{Name} );
+    my $group_obj = $self->find_rt_group(%args);
+    return unless defined $group_obj;
+
+    my $id = delete $group->{'id'};
 
     my $created;
     if ($group_obj->Id) {
@@ -1031,6 +1040,15 @@ sub create_rt_group {
             }
             $created = $val;
             $self->_debug("Created group for $group->{Name} with id ".$group_obj->Id);
+
+            if ( $id ) {
+                my ($val, $msg) = $group_obj->SetAttribute( Name => 'LDAPImport-gid-'.$id, Content => 1 );
+                unless ($val) {
+                    $self->_error("couldn't set attribute: $msg");
+                    return;
+                }
+            }
+
         } else {
             print "Found new group $group->{Name} to create in RT\n";
             $self->_show_group_info( %args );
@@ -1045,6 +1063,88 @@ sub create_rt_group {
 
 }
 
+sub find_rt_group {
+    my $self = shift;
+    my %args = @_;
+    my $group = $args{group};
+
+    my $group_obj = RT::Group->new($RT::SystemUser);
+    $group_obj->LoadUserDefinedGroup( $group->{Name} );
+    return $group_obj unless $group->{'id'};
+
+    unless ( $group_obj->id ) {
+        $self->_debug("No group in RT named $group->{Name}. Looking by $group->{id} LDAP id.");
+        $group_obj = $self->find_rt_group_by_ldap_id( $group->{'id'} );
+        unless ( $group_obj ) {
+            $self->_debug("No group in RT with LDAP id $group->{id}. Creating a new one.");
+            return RT::Group->new($RT::SystemUser);
+        }
+
+        $self->_debug("No group in RT named $group->{Name}, but found group by LDAP id $group->{id}. Renaming the group.");
+        # $group->Update will take care of the name
+        return $group_obj;
+    }
+
+    my $attr_name = 'LDAPImport-gid-'. $group->{'id'};
+    my $rt_gid = $group_obj->FirstAttribute( $attr_name );
+    return $group_obj if $rt_gid;
+
+    my $other_group = $self->find_rt_group_by_ldap_id( $group->{'id'} );
+    if ( $other_group ) {
+        $self->_debug("Group with LDAP id $group->{id} exists, as well as group named $group->{Name}. Renaming both.");
+    }
+    elsif ( grep $_->Name =~ /^LDAPImport-gid-/, @{ $group_obj->Attributes->ItemsArrayRef } ) {
+        $self->_debug("No group in RT with LDAP id $group->{id}, but group $group->{Name} has id. Renaming the group and creating a new one.");
+    }
+    else {
+        $self->_debug("No group in RT with LDAP id $group->{id}, but group $group->{Name} exists and has no LDAP id. Assigning the id to the group.");
+        if ( $args{import} ) {
+            my ($status, $msg) = $group_obj->SetAttribute( Name => $attr_name, Content => 1 );
+            unless ( $status ) {
+                $self->_error("Couldn't set attribute: $msg");
+                return undef;
+            }
+            $self->_debug("Assigned $group->{id} LDAP group id to $group->{Name}");
+        }
+        else {
+            print "Group $group->{'Name'} gets LDAP id $group->{id}\n";
+        }
+
+        return $group_obj;
+    }
+
+    # rename existing group to move it out of our way
+    {
+        my ($old, $new) = ($group_obj->Name, $group_obj->Name .' (LDAPImport '. time . ')');
+        if ( $args{import} ) {
+            my ($status, $msg) = $group_obj->SetName( $new );
+            unless ( $status ) {
+                $self->_error("Couldn't rename group from $old to $new: $msg");
+                return undef;
+            }
+            $self->_debug("Renamed group $old to $new");
+        }
+        else {
+            print "Group $old to be renamed to $new\n";
+        }
+    }
+
+    return $other_group || RT::Group->new($RT::SystemUser);
+}
+
+sub find_rt_group_by_ldap_id {
+    my $self = shift;
+    my $id = shift;
+
+    my $groups = RT::Groups->new( RT->SystemUser );
+    $groups->LimitToUserDefinedGroups;
+    my $attr_alias = $groups->Join( FIELD1 => 'id', TABLE2 => 'Attributes', FIELD2 => 'ObjectId' );
+    $groups->Limit( ALIAS => $attr_alias, FIELD => 'ObjectType', VALUE => 'RT::Group' );
+    $groups->Limit( ALIAS => $attr_alias, FIELD => 'Name', VALUE => 'LDAPImport-gid-'. $id );
+    return $groups->First;
+}
+
+
 =head3 add_group_members
 
 Iterate over the list of values in the C<Member_Attr> LDAP entry.

t/group-rename.t

+use strict;
+use warnings;
+use lib 't/lib';
+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';
+};
+
+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 { $_->{dn} } @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  => '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');
+    ok !get_group('Test Group 1')->FirstAttribute('LDAPImport-gid-1');
+}
+
+# map id
+{
+    RT->Config->Get('LDAPGroupMapping')->{'id'} = 'gid';
+    ok( $importer->import_groups( import => 1 ), "imported groups" );
+
+    is_member_of('testuser1', 'Test Group 1');
+    ok get_group('Test Group 1')->FirstAttribute('LDAPImport-gid-1');
+}
+
+# rename a group
+{
+    $ldap->modify(
+        "cn=Test Group 1,ou=groups,dc=bestpractical,dc=com",
+        replace => { 'cn' => 'Test Group 1 Renamed' },
+    );
+    ok( $importer->import_groups( import => 1 ), "imported groups" );
+    ok !get_group('Test Group 1')->id;
+    is_member_of('testuser1', 'Test Group 1 Renamed');
+    ok get_group('Test Group 1 Renamed')->FirstAttribute('LDAPImport-gid-1');
+}
+
+# swap two groups
+{
+    is_member_of('testuser2', 'Test Group 2');
+    is_member_of('testuser3', 'Test Group 3');
+    $ldap->modify(
+        "cn=Test Group 2,ou=groups,dc=bestpractical,dc=com",
+        replace => { 'cn' => 'Test Group 3' },
+    );
+    $ldap->modify(
+        "cn=Test Group 3,ou=groups,dc=bestpractical,dc=com",
+        replace => { 'cn' => 'Test Group 2' },
+    );
+    ok( $importer->import_groups( import => 1 ), "imported groups" );
+    is_member_of('testuser2', 'Test Group 3');
+    is_member_of('testuser3', 'Test Group 2');
+    ok get_group('Test Group 2')->FirstAttribute('LDAPImport-gid-3');
+    ok get_group('Test Group 3')->FirstAttribute('LDAPImport-gid-2');
+}
+
+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;
+}
+
+