[Bps-public-commit] rt-authen-externalauth branch, multiple-emails, updated. 0.09-31-g782946a

Ruslan Zakirov ruz at bestpractical.com
Wed May 11 10:23:20 EDT 2011


The branch, multiple-emails has been updated
       via  782946aa504ab5dd65caca22d0d2a1f4f24265e9 (commit)
       via  6e9e0d36c32f920b4b78c45efc5cbd2d705cd489 (commit)
       via  0b600c7b1de2de296fc76e2b30f08f2f68c2fad2 (commit)
       via  823d3f9e3419a2a09879ca7e8cc4531c33f7c838 (commit)
       via  f79db1ca9053f3d7fa7ba786b720ca3a4b070b0a (commit)
       via  75d16d71a3cbad19eeebaf96c5773411e77a59d3 (commit)
       via  463563a75d4290820a0fb4261150ce48a1e11526 (commit)
       via  caee808ea57977408ebbf929acce541b67c98732 (commit)
       via  85696d98fc9413b919c77046edac51db3e89d8ea (commit)
       via  0f9739a61763b3768de1a657740add4f7f9c8c2b (commit)
      from  a5a4f3ca58d257008ac62bdad94f0746a28285c0 (commit)

Summary of changes:
 lib/RT/Authen/ExternalAuth.pm      |  173 ++++++++++++++++++++++++++++++------
 lib/RT/Authen/ExternalAuth/LDAP.pm |   81 +++++++++--------
 lib/RT/Authen/ExternalAuth/Test.pm |   17 ++++
 xt/ldap/multiple-emails.t          |  164 ++++++++++++++++++++++++++++++++++
 xt/ldap/privileged.t               |   72 +++++++++------
 5 files changed, 411 insertions(+), 96 deletions(-)
 create mode 100644 xt/ldap/multiple-emails.t

- Log -----------------------------------------------------------------
commit 0f9739a61763b3768de1a657740add4f7f9c8c2b
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed May 11 01:48:58 2011 +0400

    get rid of $found and params

diff --git a/lib/RT/Authen/ExternalAuth/LDAP.pm b/lib/RT/Authen/ExternalAuth/LDAP.pm
index ba35a71..3290be0 100644
--- a/lib/RT/Authen/ExternalAuth/LDAP.pm
+++ b/lib/RT/Authen/ExternalAuth/LDAP.pm
@@ -110,11 +110,6 @@ sub CanonicalizeUserInfo {
     
     my ($service, $key, $value) = @_;
 
-    my $found = 0;
-    my %params = (Name         => undef,
-                  EmailAddress => undef,
-                  RealName     => undef);
-
     # Load the config
     my $config = $RT::ExternalSettings->{$service};
    
@@ -141,7 +136,7 @@ sub CanonicalizeUserInfo {
         $RT::Logger->critical(  (caller(0))[3],
                                 "LDAP baseDN not defined");
         # Drop out to the next external information service
-        return ($found, %params);
+        return (0);
     }
 
     # Get a Net::LDAP object based on the config we provide
@@ -149,7 +144,7 @@ sub CanonicalizeUserInfo {
 
     # Jump to the next external information service if we can't get one, 
     # errors should be logged by _GetBoundLdapObj so we don't have to.
-    return ($found, %params) unless ($ldap);
+    return (0) unless ($ldap);
 
     my $ldap_msg = PerformSearch(
         $ldap,
@@ -160,21 +155,24 @@ sub CanonicalizeUserInfo {
     
     # If there's only one match, we're good; more than one and
     # we don't know which is the right one so we skip it.
-    if ($ldap_msg && $ldap_msg->count == 1) {
-        my $entry = $ldap_msg->first_entry();
-        foreach my $key (keys(%{$config->{'attr_map'}})) {
-            if ($RT::LdapAttrMap->{$key} eq 'dn') {
-                $params{$key} = $entry->dn();
-            } else {
-                $params{$key} = 
-                  ($entry->get_value($config->{'attr_map'}->{$key}))[0];
-            }
+    unless ($ldap_msg && $ldap_msg->count == 1) {
+        Unbind( $ldap );
+        return (0);
+    }
+
+    my %res;
+    my $entry = $ldap_msg->first_entry();
+    foreach my $key (keys(%{$config->{'attr_map'}})) {
+        if ($RT::LdapAttrMap->{$key} eq 'dn') {
+            $res{$key} = $entry->dn();
+        } else {
+            $res{$key} = 
+              ($entry->get_value($config->{'attr_map'}->{$key}))[0];
         }
-        $found = 1;
     }
-    Unbind( $ldap );
 
-    return ($found, %params);
+    Unbind( $ldap );
+    return (1, %res);
 }
 
 sub UserExists {

commit 85696d98fc9413b919c77046edac51db3e89d8ea
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed May 11 02:42:50 2011 +0400

    support not only Name matching on update

diff --git a/lib/RT/Authen/ExternalAuth.pm b/lib/RT/Authen/ExternalAuth.pm
index d5fab69..52ebc3a 100644
--- a/lib/RT/Authen/ExternalAuth.pm
+++ b/lib/RT/Authen/ExternalAuth.pm
@@ -279,8 +279,8 @@ sub UpdateUserInfo {
     # Update their info from external service using the username as the lookup key
     # CanonicalizeUserInfo will work out for itself which service to use
     # Passing it a service instead could break other RT code
-    my %args = (Name => $username);
-    $UserObj->CanonicalizeUserInfo(\%args);
+    my %args;
+    $UserObj->CanonicalizeUserInfo( \%args );
 
     # For each piece of information returned by CanonicalizeUserInfo,
     # run the Set method for that piece of info to change it for the user
@@ -443,6 +443,14 @@ sub CanonicalizeUserInfo {
                   EmailAddress => undef,
                   RealName     => undef);
     
+    my $current_value = sub {
+        my $field = shift;
+        return $args->{ $field } if keys %$args;
+
+        return undef unless $UserObj->can( $field );
+        return $UserObj->$field();
+    };
+
     $RT::Logger->debug( (caller(0))[3], 
                         "called by", 
                         caller, 
@@ -470,7 +478,7 @@ sub CanonicalizeUserInfo {
         foreach my $rt_attr (@{$config->{'attr_match_list'}}) {
             # Jump to the next attr in $args if this one isn't in the attr_match_list
             $RT::Logger->debug( "Attempting to use this canonicalization key:",$rt_attr);
-            my $value = $args->{$rt_attr};
+            my $value = $current_value->( $rt_attr );
             unless( defined $value && length $value ) {
                 $RT::Logger->debug("This attribute (",
                                     $rt_attr,

commit caee808ea57977408ebbf929acce541b67c98732
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed May 11 02:50:19 2011 +0400

    in CanonicalizeUserInfo deal with attr_map outside implementations

diff --git a/lib/RT/Authen/ExternalAuth.pm b/lib/RT/Authen/ExternalAuth.pm
index 52ebc3a..5cc5b41 100644
--- a/lib/RT/Authen/ExternalAuth.pm
+++ b/lib/RT/Authen/ExternalAuth.pm
@@ -437,12 +437,7 @@ sub CanonicalizeUserInfo {
     
     my $UserObj = shift;
     my $args    = shift;
-    
-    my $found   = 0;
-    my %params  = (Name         => undef,
-                  EmailAddress => undef,
-                  RealName     => undef);
-    
+
     my $current_value = sub {
         my $field = shift;
         return $args->{ $field } if keys %$args;
@@ -451,6 +446,8 @@ sub CanonicalizeUserInfo {
         return $UserObj->$field();
     };
 
+    my ($found, $config, %params) = (0);
+
     $RT::Logger->debug( (caller(0))[3], 
                         "called by", 
                         caller, 
@@ -467,13 +464,19 @@ sub CanonicalizeUserInfo {
                             $service);
         
         # Get the config for the service so that we know what attrs we can canonicalize
-        my $config = $RT::ExternalSettings->{$service};
+        $config = $RT::ExternalSettings->{$service};
 
         if($config->{'type'} eq 'cookie'){
             $RT::Logger->debug("You cannot use SSO cookies as an information service!");
             next;
         }  
         
+        # Get the list of unique attrs we need
+        my @service_attrs = do {
+            my %seen;
+            grep !$seen{$_}++, map ref($_)? @$_ : ($_), values %{ $config->{'attr_map'} }
+        };
+
         # For each attr we've been told to canonicalize in the match list
         foreach my $rt_attr (@{$config->{'attr_match_list'}}) {
             # Jump to the next attr in $args if this one isn't in the attr_match_list
@@ -502,9 +505,9 @@ sub CanonicalizeUserInfo {
             # the service requested.
             
             if($config->{'type'} eq 'ldap'){    
-                ($found, %params) = RT::Authen::ExternalAuth::LDAP::CanonicalizeUserInfo($service,$key,$value, $args);
+                ($found, %params) = RT::Authen::ExternalAuth::LDAP::CanonicalizeUserInfo($service,$key,$value, \@service_attrs);
             } elsif ($config->{'type'} eq 'db') {
-                ($found, %params) = RT::Authen::ExternalAuth::DBI::CanonicalizeUserInfo($service,$key,$value, $args);
+                ($found, %params) = RT::Authen::ExternalAuth::DBI::CanonicalizeUserInfo($service,$key,$value, \@service_attrs);
             } else {
                 $RT::Logger->debug( (caller(0))[3],
                                     "does not consider",
@@ -518,29 +521,40 @@ sub CanonicalizeUserInfo {
         # Don't Check any more services
         last if $found;
     }
-    
-    # If found, Canonicalize Email Address and 
-    # update the args hash that we were given the hashref for
-    if ($found) {
-        # It's important that we always have a canonical email address
-        if ($params{'EmailAddress'}) {
-            $params{'EmailAddress'} = $UserObj->CanonicalizeEmailAddress($params{'EmailAddress'});
-        } 
-        %$args = (%$args, %params);
+
+    unless ( $found ) {
+        ### HACK: The config var below is to overcome the (IMO) bug in
+        ### RT::User::Create() which expects this function to always
+        ### return true or rejects the user for creation. This should be
+        ### a different config var (CreateUncanonicalizedUsers) and 
+        ### should be honored in RT::User::Create()
+        return($RT::AutoCreateNonExternalUsers);
+    }
+
+    # If found let's build back RT's fields
+    my %res;
+    while ( my ($k, $v) = each %{ $config->{'attr_map'} } ) {
+        if ( keys %$args ) {
+            $res{ $k } = $params{ $v };
+        } else {
+            $res{ $k } = $params{ $v };
+        }
     }
 
+    # It's important that we always have a canonical email address
+    if ($res{'EmailAddress'}) {
+        $res{'EmailAddress'} = $UserObj->CanonicalizeEmailAddress($res{'EmailAddress'});
+    } 
+
+    # update the args hash that we were given the hashref for
+    %$args = (%$args, %res);
+
     $RT::Logger->info(  (caller(0))[3], 
                         "returning", 
                         join(", ", map {sprintf("%s: %s", $_, $args->{$_})} 
                             sort(keys(%$args))));
 
-    ### HACK: The config var below is to overcome the (IMO) bug in
-    ### RT::User::Create() which expects this function to always
-    ### return true or rejects the user for creation. This should be
-    ### a different config var (CreateUncanonicalizedUsers) and 
-    ### should be honored in RT::User::Create()
-    return($found || $RT::AutoCreateNonExternalUsers);
-   
+    return $found;
 }
 
 {
diff --git a/lib/RT/Authen/ExternalAuth/LDAP.pm b/lib/RT/Authen/ExternalAuth/LDAP.pm
index 3290be0..9c44033 100644
--- a/lib/RT/Authen/ExternalAuth/LDAP.pm
+++ b/lib/RT/Authen/ExternalAuth/LDAP.pm
@@ -108,7 +108,7 @@ sub GetAuth {
 
 sub CanonicalizeUserInfo {
     
-    my ($service, $key, $value) = @_;
+    my ($service, $key, $value, $attrs) = @_;
 
     # Load the config
     my $config = $RT::ExternalSettings->{$service};
@@ -117,9 +117,6 @@ sub CanonicalizeUserInfo {
     my $base            = $config->{'base'};
     my $filter          = $config->{'filter'};
 
-    # Get the list of unique attrs we need
-    my @attrs = values(%{$config->{'attr_map'}});
-
     # This is a bit confusing and probably broken. Something to revisit..
     my $filter_addition = ($key && $value) ? "(". $key . "=$value)" : "";
     if(defined($filter) && ($filter ne "()")) {
@@ -150,7 +147,7 @@ sub CanonicalizeUserInfo {
         $ldap,
         base   => $base,
         filter => $filter,
-        attrs  => \@attrs
+        attrs  => $attrs
     );
     
     # If there's only one match, we're good; more than one and
@@ -161,16 +158,14 @@ sub CanonicalizeUserInfo {
     }
 
     my %res;
-    my $entry = $ldap_msg->first_entry();
-    foreach my $key (keys(%{$config->{'attr_map'}})) {
-        if ($RT::LdapAttrMap->{$key} eq 'dn') {
-            $res{$key} = $entry->dn();
+    my $entry = $ldap_msg->first_entry;
+    foreach my $attr ( @$attrs ) {
+        if ( $attr eq 'dn' ) {
+            $res{ $attr } = $entry->dn;
         } else {
-            $res{$key} = 
-              ($entry->get_value($config->{'attr_map'}->{$key}))[0];
+            $res{ $attr } = ($entry->get_value( $attr ))[0];
         }
     }
-
     Unbind( $ldap );
     return (1, %res);
 }

commit 463563a75d4290820a0fb4261150ce48a1e11526
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed May 11 17:04:14 2011 +0400

    ::Test->add_ldap_user_simple method

diff --git a/lib/RT/Authen/ExternalAuth/Test.pm b/lib/RT/Authen/ExternalAuth/Test.pm
index 5e9a440..2d433d5 100644
--- a/lib/RT/Authen/ExternalAuth/Test.pm
+++ b/lib/RT/Authen/ExternalAuth/Test.pm
@@ -113,4 +113,21 @@ sub add_ldap_user {
     return $ldap{'client'}->add( $dn, attr => [%args] );
 }
 
+{ my $i = 0;
+sub add_ldap_user_simple {
+    my $self = shift;
+    my %args = @_;
+
+    my $name = "testuser". ++$i;
+
+    s/\%name\b/$name/g foreach grep defined, values %args;
+
+    $self->add_ldap_user(
+        cn    => $name,
+        mail  => "$name\@invalid.tld",
+        %args,
+    );
+    return $name;
+} }
+
 1;

commit 75d16d71a3cbad19eeebaf96c5773411e77a59d3
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed May 11 17:03:28 2011 +0400

    test that AutoCreate works on submission by email

diff --git a/xt/ldap/privileged.t b/xt/ldap/privileged.t
index 3e14631..1b4ae78 100644
--- a/xt/ldap/privileged.t
+++ b/xt/ldap/privileged.t
@@ -1,51 +1,65 @@
 use strict;
 use warnings;
 
-use RT::Authen::ExternalAuth::Test ldap => 1, tests => 13;
+use RT::Authen::ExternalAuth::Test ldap => 1, tests => 18;
 my $class = 'RT::Authen::ExternalAuth::Test';
 
-
 my ($server, $client) = $class->bootstrap_ldap_basics;
 ok( $server, "spawned test LDAP server" );
 
-my $username = "testuser";
-$class->add_ldap_user(
-    uid  => $username,
-    mail => "$username\@invalid.tld",
-);
-
 RT->Config->Set( AutoCreate                  => { Privileged => 1 } );
 
+RT::Test->set_rights(
+    { Principal => 'Everyone', Right => [qw(SeeQueue ShowTicket CreateTicket)] },
+);
+
 my ( $baseurl, $m ) = RT::Test->started_ok();
 
-diag "test uri login";
+
+diag "login, make sure user privileged";
 {
-    ok( !$m->login( 'fakeuser', 'password' ), 'not logged in with fake user' );
-    ok( $m->login( 'testuser', 'password' ), 'logged in' );
+    my $username = $class->add_ldap_user_simple;
+    ok( $m->login( $username, 'password' ), 'logged in' );
+
+    {
+        my $user = RT::User->new($RT::SystemUser);
+        my ($ok,$msg) = $user->Load( $username );
+        ok($user->id);
+        is($user->EmailAddress, "$username\@invalid.tld");
+        ok($user->Privileged, 'privileged user');
+    }
+
+    unlike( $m->uri, qr!SelfService!, 'privileged home page' );
 }
 
-diag "test user creation";
+diag "send mail, make sure user privileged";
 {
-    my $testuser = RT::User->new($RT::SystemUser);
-    my ($ok,$msg) = $testuser->Load( 'testuser' );
-    ok($ok,$msg);
-    is($testuser->EmailAddress,'testuser at invalid.tld');
-}
+    my $username = $class->add_ldap_user_simple;
+    {
+        my $mail = << "MAIL";
+Subject: Test
+From: $username\@invalid.tld
 
+test
+MAIL
 
-diag "test form login";
-{
-    $m->logout;
-    $m->get_ok( $baseurl, 'base url' );
-    $m->submit_form(
-        form_number => 1,
-        fields      => { user => 'testuser', pass => 'password', },
-    );
-    $m->text_contains( 'Logout', 'logged in via form' );
-}
+        my ($status, $id) = RT::Test->send_via_mailgate($mail);
+        is ($status >> 8, 0, "The mail gateway exited normally");
+        ok ($id, "got id of a newly created ticket - $id");
 
-like( $m->uri, qr!$baseurl/(index\.html)?!, 'privileged home page' );
+        my $ticket = RT::Ticket->new( $RT::SystemUser );
+        $ticket->Load( $id );
+        ok ($ticket->id, 'loaded ticket');
 
-$client->unbind();
+        my $user = $ticket->CreatorObj;
+        is($user->EmailAddress, "$username\@invalid.tld");
+        ok($user->Privileged, 'privileged user');
+    }
+    {
+        ok( $m->login( $username, 'password' ), 'logged in' );
+        unlike( $m->uri, qr!SelfService!, 'privileged home page' );
+    }
+}
 
+$client->unbind();
 $m->get_warnings;

commit f79db1ca9053f3d7fa7ba786b720ca3a4b070b0a
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed May 11 17:11:39 2011 +0400

    tests for setups with multiple emails stored in LDAP

diff --git a/xt/ldap/multiple-emails.t b/xt/ldap/multiple-emails.t
new file mode 100644
index 0000000..5ebe609
--- /dev/null
+++ b/xt/ldap/multiple-emails.t
@@ -0,0 +1,164 @@
+use strict;
+use warnings;
+
+use RT::Authen::ExternalAuth::Test ldap => 1, tests => 43;
+my $class = 'RT::Authen::ExternalAuth::Test';
+
+my ($server, $client) = $class->bootstrap_ldap_basics;
+ok( $server, "spawned test LDAP server" );
+
+my $queue = RT::Test->load_or_create_queue(Name => 'General');
+ok($queue->id, "loaded the General queue");
+
+RT->Config->Set( AutoCreate                  => { Privileged => 1 } );
+
+RT->Config->Get('ExternalSettings')->{'My_LDAP'}{'attr_map'}{'EmailAddress'}
+    = ['mail', 'alias'];
+
+RT::Test->set_rights(
+    { Principal => 'Everyone', Right => [qw(SeeQueue ShowTicket CreateTicket)] },
+);
+
+my ( $baseurl, $m ) = RT::Test->started_ok();
+
+diag "login then send emails from different addresses";
+{
+    my $username = new_user();
+    my $first_user;
+    {
+        ok( $m->login( $username, 'password' ), 'logged in' );
+
+        ok( $m->goto_create_ticket( $queue ), "go to create ticket" );
+        $m->form_name('TicketCreate');
+        $m->submit;
+
+        my ($id) = ($m->content =~ /.*Ticket (\d+) created.*/g);
+        ok $id, "created a ticket";
+
+        my $ticket = RT::Ticket->new( $RT::SystemUser );
+        $ticket->Load( $id );
+        ok ($ticket->id, 'loaded ticket');
+
+        my $user = $first_user = $ticket->CreatorObj;
+        is( $user->Name, $username );
+        is( $user->EmailAddress, "$username\@invalid.tld" );
+    }
+
+    {
+        my $mail = << "MAIL";
+Subject: Test
+From: $username\@invalid.tld
+
+test
+MAIL
+
+        my ($status, $id) = RT::Test->send_via_mailgate($mail);
+        is ($status >> 8, 0, "The mail gateway exited normally");
+        ok ($id, "got id of a newly created ticket - $id");
+
+        my $ticket = RT::Ticket->new( $RT::SystemUser );
+        $ticket->Load( $id );
+        ok ($ticket->id, 'loaded ticket');
+
+        my $user = $ticket->CreatorObj;
+        is( $user->id, $first_user->id );
+        is( $user->Name, $username );
+        is( $user->EmailAddress, "$username\@invalid.tld" );
+    }
+
+    {
+        my $mail = << "MAIL";
+Subject: Test
+From: $username\@alternative.tld
+
+test
+MAIL
+
+        my ($status, $id) = RT::Test->send_via_mailgate($mail);
+        is ($status >> 8, 0, "The mail gateway exited normally");
+        ok ($id, "got id of a newly created ticket - $id");
+
+        my $ticket = RT::Ticket->new( $RT::SystemUser );
+        $ticket->Load( $id );
+        ok ($ticket->id, 'loaded ticket');
+
+        my $user = $ticket->CreatorObj;
+        is( $user->id, $first_user->id );
+        is( $user->Name, $username );
+        is( $user->EmailAddress, "$username\@invalid.tld" );
+    }
+}
+
+diag "send a mail from alternative address, then try other credentials";
+{
+    my $username = new_user();
+    my $first_user;
+    {
+        my $mail = << "MAIL";
+Subject: Test
+From: $username\@alternative.tld
+
+test
+MAIL
+
+        my ($status, $id) = RT::Test->send_via_mailgate($mail);
+        is ($status >> 8, 0, "The mail gateway exited normally");
+        ok ($id, "got id of a newly created ticket - $id");
+
+        my $ticket = RT::Ticket->new( $RT::SystemUser );
+        $ticket->Load( $id );
+        ok ($ticket->id, 'loaded ticket');
+
+        my $user = $first_user = $ticket->CreatorObj;
+        is( $user->Name, $username );
+        is( $user->EmailAddress, "$username\@alternative.tld" );
+    }
+
+    {
+        ok( $m->login( $username, 'password' ), 'logged in' );
+
+        ok( $m->goto_create_ticket( $queue ), "go to create ticket" );
+        $m->form_name('TicketCreate');
+        $m->submit;
+
+        my ($id) = ($m->content =~ /.*Ticket (\d+) created.*/g);
+        ok $id, "created a ticket";
+
+        my $ticket = RT::Ticket->new( $RT::SystemUser );
+        $ticket->Load( $id );
+        ok ($ticket->id, 'loaded ticket');
+
+        my $user = $ticket->CreatorObj;
+        is( $user->id, $first_user->id );
+        is( $user->Name, $username );
+        is( $user->EmailAddress, "$username\@alternative.tld" );
+    }
+
+    {
+        my $mail = << "MAIL";
+Subject: Test
+From: $username\@invalid.tld
+
+test
+MAIL
+
+        my ($status, $id) = RT::Test->send_via_mailgate($mail);
+        is ($status >> 8, 0, "The mail gateway exited normally");
+        ok ($id, "got id of a newly created ticket - $id");
+
+        my $ticket = RT::Ticket->new( $RT::SystemUser );
+        $ticket->Load( $id );
+        ok ($ticket->id, 'loaded ticket');
+
+        my $user = $ticket->CreatorObj;
+        is( $user->id, $user->id );
+        is( $user->Name, $username );
+        is( $user->EmailAddress, "$username\@alternative.tld" );
+    }
+}
+
+$client->unbind();
+$m->get_warnings;
+
+sub new_user { return $class->add_ldap_user_simple( alias => '%name at alternative.tld' ) }
+

commit 823d3f9e3419a2a09879ca7e8cc4531c33f7c838
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed May 11 17:15:00 2011 +0400

    method to join zero or more LDAP filters with '&' or '|' op

diff --git a/lib/RT/Authen/ExternalAuth/LDAP.pm b/lib/RT/Authen/ExternalAuth/LDAP.pm
index 9c44033..9dfb36e 100644
--- a/lib/RT/Authen/ExternalAuth/LDAP.pm
+++ b/lib/RT/Authen/ExternalAuth/LDAP.pm
@@ -404,6 +404,25 @@ sub PerformSearch {
     return $res;
 }
 
+sub JoinFilters {
+    my $op = shift;
+    my @list =
+        grep defined && length && $_ ne '()',
+        map ref $_? $_->as_string : $_,
+        @_;
+    return undef unless @list;
+
+    my $str = @list > 1
+        ? "($op". join( '', @list ) .')'
+        : $list[0]
+    ;
+    my $obj = Net::LDAP::Filter->new( $str );
+    $RT::Logger->error("'$str' is not valid LDAP filter")
+        unless $obj;
+
+    return $obj;
+}
+
 # }}}
 
 1;

commit 0b600c7b1de2de296fc76e2b30f08f2f68c2fad2
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed May 11 18:07:18 2011 +0400

    change how we build main filter in LDAP::CanonicalizeUserInfo
    
    * support key to be an array to mean any of these LDAP attributes
    * key and value are always defined by the caller
    * use JoinFilters
    ** support situation when filter in the config is empty

diff --git a/lib/RT/Authen/ExternalAuth/LDAP.pm b/lib/RT/Authen/ExternalAuth/LDAP.pm
index 9dfb36e..7848bec 100644
--- a/lib/RT/Authen/ExternalAuth/LDAP.pm
+++ b/lib/RT/Authen/ExternalAuth/LDAP.pm
@@ -112,23 +112,14 @@ sub CanonicalizeUserInfo {
 
     # Load the config
     my $config = $RT::ExternalSettings->{$service};
-   
-    # Figure out what's what
-    my $base            = $config->{'base'};
-    my $filter          = $config->{'filter'};
 
-    # This is a bit confusing and probably broken. Something to revisit..
-    my $filter_addition = ($key && $value) ? "(". $key . "=$value)" : "";
-    if(defined($filter) && ($filter ne "()")) {
-        $filter = Net::LDAP::Filter->new(   "(&" . 
-                                            $filter . 
-                                            $filter_addition . 
-                                            ")"
-                                        ); 
-    } else {
-        $RT::Logger->debug( "LDAP Filter invalid or not present.");
-    }
+    my $filter = JoinFilters(
+        '&',
+        JoinFilters('|', map "($_=$value)", ref $key? @$key: ($key) ),
+        $config->{'filter'},
+    ) or return (0);
 
+    my $base = $config->{'base'};
     unless (defined($base)) {
         $RT::Logger->critical(  (caller(0))[3],
                                 "LDAP baseDN not defined");

commit 6e9e0d36c32f920b4b78c45efc5cbd2d705cd489
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed May 11 18:11:06 2011 +0400

    change how we process hash from XXX::CanonicalizeUserInfo
    
    support the following config when we map hash returned by
    CanonicalizeUserInfo to RT user's fields.
    
    attr_map => {
        EmailAddress => ['mail', 'alias'],
        ...
    },

diff --git a/lib/RT/Authen/ExternalAuth.pm b/lib/RT/Authen/ExternalAuth.pm
index 5cc5b41..62f2525 100644
--- a/lib/RT/Authen/ExternalAuth.pm
+++ b/lib/RT/Authen/ExternalAuth.pm
@@ -534,10 +534,18 @@ sub CanonicalizeUserInfo {
     # If found let's build back RT's fields
     my %res;
     while ( my ($k, $v) = each %{ $config->{'attr_map'} } ) {
-        if ( keys %$args ) {
+        unless ( ref $v ) {
             $res{ $k } = $params{ $v };
+            next;
+        }
+
+        my $current = $current_value->( $k );
+        unless ( defined $current ) {
+            $res{ $k } = (grep defined && length, map $params{ $_ }, @$v)[0];
         } else {
-            $res{ $k } = $params{ $v };
+            unless ( grep defined && length && $_ eq $current, map $params{ $_ }, @$v ) {
+                $res{ $k } = (grep defined && length, map $params{ $_ }, @$v)[0];
+            }
         }
     }
 

commit 782946aa504ab5dd65caca22d0d2a1f4f24265e9
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed May 11 18:18:49 2011 +0400

    extend ::User->LoadByCols to support alternative values
    
    RT's default ::User->LoadByCols failed, but user record may be
    still there if we have info providing service with alternative
    values for fields from 'attr_match_list' list

diff --git a/lib/RT/Authen/ExternalAuth.pm b/lib/RT/Authen/ExternalAuth.pm
index 62f2525..1296c5e 100644
--- a/lib/RT/Authen/ExternalAuth.pm
+++ b/lib/RT/Authen/ExternalAuth.pm
@@ -574,4 +574,91 @@ sub CanonicalizeUserInfo {
     };
 }
 
+{
+    no warnings 'redefine';
+    my $orig = RT::User->can('LoadByCols');
+    *RT::User::LoadByCols = sub {
+        my $self = shift;
+        my %args = @_;
+
+        my $rv = $orig->( $self, %args );
+        return $rv if $self->id;
+
+# we couldn't load a user. ok, but user may exist anyway. It may happen when
+# RT fields in attr_match_list are mapped to multiple attributes in an external
+# source, for example: attr_map => { EmailAddress => [qw(mail alias1 alias2 alias3)], }
+
+        # find services that may have alternative values for a field we search by
+        my @info_services = $RT::ExternalInfoPriority ? @{$RT::ExternalInfoPriority} : ();
+        foreach my $service ( splice @info_services ) {
+            my $config = $RT::ExternalSettings->{ $service };
+            next if $config->{'type'} eq 'cookie';
+            next unless
+                grep ref $_,
+                map $config->{'attr_map'}{ $_ },
+                @{ $config->{'attr_match_list'} };
+
+            push @info_services, $service;
+        }
+        return $rv unless @info_services;
+
+        # find user in external service and fetch alternative values
+        # for a field
+        my ($found, $search_by, @alternatives);
+        foreach my $service (@info_services) {
+            my $config = $RT::ExternalSettings->{$service};
+
+            # Get the list of unique attrs we need
+            my @service_attrs = do {
+                my %seen;
+                grep !$seen{$_}++, map ref($_)? @$_ : ($_), values %{ $config->{'attr_map'} }
+            };
+
+            foreach my $rt_attr ( @{ $config->{'attr_match_list'} } ) {
+                next unless exists $args{ $rt_attr }
+                    && defined $args{ $rt_attr }
+                    && length $args{ $rt_attr };
+                next unless ref $config->{'attr_map'}{ $rt_attr };
+
+                $search_by = $rt_attr;
+                last;
+            }
+            next unless $search_by;
+
+            my @search_args = (
+                $service,
+                $config->{'attr_map'}{ $search_by },
+                $args{ $search_by },
+                $config->{'attr_map'}{ $search_by },
+            );
+
+            my %params;
+            if($config->{'type'} eq 'ldap') {
+                ($found, %params) = RT::Authen::ExternalAuth::LDAP::CanonicalizeUserInfo( @search_args );
+            } elsif ($config->{'type'} eq 'db') {
+                ($found, %params) = RT::Authen::ExternalAuth::DBI::CanonicalizeUserInfo( @search_args );
+            } else {
+                $RT::Logger->debug( (caller(0))[3],
+                                    "does not consider",
+                                    $service,
+                                    "a valid information service");
+            }
+            next unless $found;
+
+            @alternatives = grep defined && length && $_ ne $args{ $search_by }, values %params;
+
+            # Don't Check any more services
+            last;
+        }
+        return $rv unless $found;
+
+        # lookup by alternatives
+        foreach my $value ( @alternatives ) {
+            my $rv = $orig->( $self, %args, $search_by => $value );
+            return $rv if $self->id;
+        }
+        return $rv;
+    };
+}
+
 1;

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



More information about the Bps-public-commit mailing list