[Rt-commit] rt branch, 4.4/add-ldap-email-authentication, created. rt-4.4.4-205-g7b8e0c5716

Blaine Motsinger blaine at bestpractical.com
Thu Jan 21 19:32:43 EST 2021


The branch, 4.4/add-ldap-email-authentication has been created
        at  7b8e0c571627d570f70aaaa2a4242a42d4be0350 (commit)

- Log -----------------------------------------------------------------
commit 30370812b567fa529e4bb68b292c6daeda973f7a
Author: Blaine Motsinger <blaine at bestpractical.com>
Date:   Thu Jan 21 17:44:30 2021 -0600

    Add LDAP email authentication
    
    This commit updates the LDAP authentication flow to allow users
    to authenticate using email address as well as username.
    
    The LDAP filter string was previously hardcoded to only query LDAP
    using the mapped Name field.  RT will now attempt to query LDAP
    using each of the defined keys in attr_match_list.
    
    After a match is found on the LDAP server, RT will now load the
    User object by either Name or EmailAddress, depending on what key
    returned the match.

diff --git a/lib/RT/Authen/ExternalAuth.pm b/lib/RT/Authen/ExternalAuth.pm
index bf08f8e761..51b069258a 100644
--- a/lib/RT/Authen/ExternalAuth.pm
+++ b/lib/RT/Authen/ExternalAuth.pm
@@ -403,7 +403,7 @@ sub DoAuth {
             # Don't continue unless the $username exists in the external service
 
             $RT::Logger->debug("Calling UserExists with \$username ($username) and \$service ($service)");
-            next unless RT::Authen::ExternalAuth::UserExists($username, $service);
+            next unless RT::Authen::ExternalAuth::UserExists($username, $service, $session);
         }
 
         ####################################################################
@@ -415,7 +415,15 @@ sub DoAuth {
 
         # Does user already exist internally to RT?
         $session->{'CurrentUser'} = RT::CurrentUser->new();
-        $session->{'CurrentUser'}->Load($username);
+
+        # if the LDAP search in LDAP::UserExists matched using EmailAddress instead of
+        # Name, load the RT user with that instead.
+        if ($session->{'_ldap_attr_match'} && $session->{'_ldap_attr_match'} eq 'EmailAddress') {
+            $session->{'CurrentUser'}->LoadByEmail($username);
+        }
+        else {
+            $session->{'CurrentUser'}->Load($username);
+        }
 
         # Unless we have loaded a valid user with a UserID create one.
         unless ($session->{'CurrentUser'}->Id) {
@@ -647,7 +655,7 @@ sub UserExists {
     # Request a username/password check from the specified service
     # This is only valid for non-SSO services.
 
-    my ($username,$service) = @_;
+    my ($username,$service,$session) = @_;
 
     my $success = 0;
 
@@ -659,7 +667,7 @@ sub UserExists {
     if ($config->{'type'} eq 'db') {
         $success = RT::Authen::ExternalAuth::DBI::UserExists($username,$service);
     } elsif ($config->{'type'} eq 'ldap') {
-        $success = RT::Authen::ExternalAuth::LDAP::UserExists($username,$service);
+        $success = RT::Authen::ExternalAuth::LDAP::UserExists($username,$service,$session);
     }
 
     return $success;
diff --git a/lib/RT/Authen/ExternalAuth/LDAP.pm b/lib/RT/Authen/ExternalAuth/LDAP.pm
index 2cc53f713a..f09bc602cc 100644
--- a/lib/RT/Authen/ExternalAuth/LDAP.pm
+++ b/lib/RT/Authen/ExternalAuth/LDAP.pm
@@ -206,6 +206,7 @@ sub GetAuth {
     my $group_attr_val  = $config->{'group_attr_value'} || 'dn';
     my $group_scope     = $config->{'group_scope'} || 'base';
     my $attr_map        = $config->{'attr_map'};
+    my $attr_match_list = $config->{'attr_match_list'};
     my @attrs           = ('dn');
 
     # Make sure we fetch the user attribute we'll need for the group check
@@ -221,44 +222,54 @@ sub GetAuth {
     my $ldap = _GetBoundLdapObj($config);
     return 0 unless ($ldap);
 
-    $filter = Net::LDAP::Filter->new(   '(&(' .
-                                        $attr_map->{'Name'} .
-                                        '=' .
-                                        escape_filter_value($username) .
-                                        ')' .
-                                        $filter .
-                                        ')'
-                                    );
+    # loop over each of the attr_match_list members for LDAP search
+    my $ldap_msg;
+    foreach my $attr_match ( @{$attr_match_list} ) {
+        unless ( defined $attr_map->{$attr_match} ) {
+            $RT::Logger->error( "Invalid LDAP mapping for $attr_match, no defined fields in attr_map" );
+            next;
+        }
 
-    $RT::Logger->debug( "LDAP Search === ",
-                        "Base:",
-                        $base,
-                        "== Filter:",
-                        $filter->as_string,
-                        "== Attrs:",
-                        join(',', at attrs));
+        my $net_ldap_filter = Net::LDAP::Filter->new(
+            '(&' .
+            $filter .
+            '(' .
+            $config->{'attr_map'}->{$attr_match} .
+            '=' .
+            escape_filter_value($username) .
+            '))'
+        );
 
-    my $ldap_msg = $ldap->search(   base   => $base,
-                                    filter => $filter,
-                                    attrs  => \@attrs);
+        $RT::Logger->debug( "LDAP Search === ",
+                            "Base:",
+                            $base,
+                            "== Filter:",
+                            ($net_ldap_filter ? $net_ldap_filter->as_string : ''),
+                            "== Attrs:",
+                            join(',', at attrs) );
 
-    unless ($ldap_msg->code == LDAP_SUCCESS || $ldap_msg->code == LDAP_PARTIAL_RESULTS) {
-        $RT::Logger->debug( "search for",
-                            $filter->as_string,
-                            "failed:",
-                            ldap_error_name($ldap_msg->code),
-                            $ldap_msg->code);
-        # Didn't even get a partial result - jump straight to the next external auth service
-        return 0;
-    }
+        $ldap_msg = $ldap->search( base   => $base,
+                                   filter => $filter,
+                                   attrs  => \@attrs );
 
-    unless ($ldap_msg->count == 1) {
-        $RT::Logger->info(  $service,
-                            "AUTH FAILED:",
-                            $username,
-                            "User not found or more than one user found");
-        # We got no user, or too many users.. jump straight to the next external auth service
-        return 0;
+        unless ( $ldap_msg->code == LDAP_SUCCESS || $ldap_msg->code == LDAP_PARTIAL_RESULTS ) {
+            $RT::Logger->debug( "search for",
+                                $filter->as_string,
+                                "failed:",
+                                ldap_error_name($ldap_msg->code),
+                                $ldap_msg->code );
+            # Didn't even get a partial result - jump straight to the next external auth service
+            return 0;
+        }
+
+        unless ( $ldap_msg->count == 1 ) {
+            $RT::Logger->info( $service,
+                               "AUTH FAILED:",
+                               $username,
+                               "User not found or more than one user found" );
+            # We got no user, or too many users.. try the next attr_match_list field.
+            next;
+        }
     }
 
     my $ldap_entry = $ldap_msg->first_entry;
@@ -523,7 +534,7 @@ sub CanonicalizeUserInfo {
 }
 
 sub UserExists {
-    my ($username,$service) = @_;
+    my ($username,$service,$session) = @_;
    $RT::Logger->debug("UserExists params:\nusername: $username , service: $service");
     my $config              = RT->Config->Get('ExternalSettings')->{$service};
 
@@ -536,57 +547,76 @@ sub UserExists {
     # but there's no harm in doing this to be sure
     undef $filter if defined $filter and $filter eq "()";
 
-    if (defined($config->{'attr_map'}->{'Name'})) {
-        # Construct the complex filter
-        $filter = Net::LDAP::Filter->new(           '(&' .
-                                                    $filter .
-                                                    '(' .
-                                                    $config->{'attr_map'}->{'Name'} .
-                                                    '=' .
-                                                    escape_filter_value($username) .
-                                                    '))'
-                                        );
-    }
-
-    my $ldap = _GetBoundLdapObj($config);
-    return unless $ldap;
+    my $attr_match_list = $config->{'attr_match_list'};
+    my @attrs = values( %{$config->{'attr_map'}} );
 
-    my @attrs = values(%{$config->{'attr_map'}});
+    # loop over each of the attr_match_list members for the initial lookup
+    foreach my $attr_match ( @{$attr_match_list} ) {
+        unless ( defined $config->{'attr_map'}->{$attr_match} ) {
+            $RT::Logger->error( "Invalid LDAP mapping for $attr_match, no defined fields in attr_map" );
+            next;
+        }
 
-    # Check that the user exists in the LDAP service
-    $RT::Logger->debug( "LDAP Search === ",
-                        "Base:",
-                        $base,
-                        "== Filter:",
-                        ($filter ? $filter->as_string : ''),
-                        "== Attrs:",
-                        join(',', at attrs));
+        my $net_ldap_filter = Net::LDAP::Filter->new(
+            '(&' .
+            $filter .
+            '(' .
+            $config->{'attr_map'}->{$attr_match} .
+            '=' .
+            escape_filter_value($username) .
+            '))'
+        );
 
-    my $user_found = $ldap->search( base    => $base,
-                                    filter  => $filter,
-                                    attrs   => \@attrs);
+        my $ldap = _GetBoundLdapObj($config);
+        return unless $ldap;
 
-    if($user_found->count < 1) {
-        # If 0 or negative integer, no user found or major failure
-        $RT::Logger->debug( "User Check Failed :: (",
-                            $service,
-                            ")",
-                            $username,
-                            "User not found");
-        return 0;
-    } elsif ($user_found->count > 1) {
-        # If more than one result returned, die because we the username field should be unique!
-        $RT::Logger->debug( "User Check Failed :: (",
-                            $service,
-                            ")",
-                            $username,
-                            "More than one user with that username!");
-        return 0;
+        # Check that the user exists in the LDAP service
+        $RT::Logger->debug( "LDAP Search === ",
+                            "Base:",
+                            $base,
+                            "== Filter:",
+                            ($net_ldap_filter ? $net_ldap_filter->as_string : ''),
+                            "== Attrs:",
+                            join(',', at attrs) );
+
+        my $user_found = $ldap->search( base   => $base,
+                                        filter => $net_ldap_filter,
+                                        attrs  => \@attrs );
+
+        if ( $user_found->count < 1 ) {
+            # If 0 or negative integer, no user found or major failure
+            $RT::Logger->debug( "User Check Failed :: (",
+                                $service,
+                                ")",
+                                $username,
+                                "User not found" );
+            next;
+        }
+        elsif ( $user_found->count > 1 ) {
+            # If more than one result returned, skip because we the username field should be unique!
+            $RT::Logger->debug( "User Check Failed :: (",
+                                $service,
+                                ")",
+                                $username,
+                                "More than one user with that username!" );
+            next;
+        }
+        else {
+            # User was found
+            $RT::Logger->debug( "User Check Succeeded :: (",
+                                $service,
+                                ")",
+                                $username );
+
+            # RT::Authen::ExternalAuth::DoAuth needs to be able to load by either User or EmailAddress.
+            # store the key that matched into the session so DoAuth can use the correct one.
+            $session->{'_ldap_attr_match'} = $attr_match;
+            return 1;
+        }
     }
-    undef $user_found;
 
-    # If we havent returned now, there must be a valid user.
-    return 1;
+    # No valid user was found using each of the search filters.
+    return 0;
 }
 
 sub UserDisabled {

commit 7b8e0c571627d570f70aaaa2a4242a42d4be0350
Author: Blaine Motsinger <blaine at bestpractical.com>
Date:   Tue Jan 19 18:46:43 2021 -0600

    Add test for LDAP attr search and match
    
    This test verifies LDAP authentication using to second entry in
    attr_match_list.  In this case EmailAddress instead of the
    previously hardcoded Name key.

diff --git a/t/externalauth/ldap_attr.t b/t/externalauth/ldap_attr.t
new file mode 100644
index 0000000000..5c3a07eca6
--- /dev/null
+++ b/t/externalauth/ldap_attr.t
@@ -0,0 +1,94 @@
+use strict;
+use warnings;
+
+use RT::Test tests => undef;
+
+eval { require RT::Authen::ExternalAuth; require Net::LDAP::Server::Test; 1; } or do {
+    plan skip_all => 'Unable to test without Net::LDAP and Net::LDAP::Server::Test';
+};
+
+my $ldap_port = RT::Test->find_idle_port;
+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();
+
+my $username = 'testldapuser';
+my $email    = "$username\@example.com";
+my $password = 'password';
+my $base     = 'dc=bestpractical,dc=com';
+my $dn       = "uid=$username,$base";
+my $entry    = {
+    cn           => $username,
+    mail         => $email,
+    uid          => $username,
+    objectClass  => 'User',
+    userPassword => $password,
+};
+
+$ldap->add( $base );
+$ldap->add( $dn, attr => [%$entry] );
+
+RT->Config->Set( ExternalAuthPriority        => ['My_LDAP'] );
+RT->Config->Set( ExternalInfoPriority        => ['My_LDAP'] );
+RT->Config->Set( AutoCreateNonExternalUsers  => 0 );
+RT->Config->Set( AutoCreate  => undef );
+RT->Config->Set(
+    ExternalSettings => {
+        'My_LDAP' => {
+            'type'            => 'ldap',
+            'server'          => "127.0.0.1:$ldap_port",
+            'base'            => $base,
+            'filter'          => '(objectClass=*)',
+            'd_filter'        => '()',
+            'tls'             => 0,
+            'net_ldap_args'   => [ version => 3 ],
+            'attr_match_list' => [ 'Name', 'EmailAddress' ],
+            'attr_map'        => {
+                'Name'         => 'uid',
+                'EmailAddress' => 'mail',
+            }
+        },
+    }
+);
+RT->Config->PostLoadCheck;
+
+my ( $baseurl, $m ) = RT::Test->started_ok();
+
+# create user, but don't set the email address.
+# after authentication, email address will be retrieved from LDAP and set for
+# the user by CanonicalizeUserInfo.
+my $testuser = RT::User->new( $RT::SystemUser );
+my ( $uid, $msg ) = $testuser->Create(
+    Name => $username,
+);
+ok( $uid, "created $username" );
+$testuser->SetEmailAddress('');
+ok( !$testuser->EmailAddress, "$username email address is not set" );
+
+diag 'test login with Name';
+$m->get_ok( $baseurl, 'base url' );
+$m->submit_form(
+    form_number => 1,
+    fields      => { user => $username, pass => 'password', },
+);
+$m->text_contains( 'Logout', 'logged in via form' );
+is( $m->uri, $baseurl . '/SelfService/' , 'selfservice page is displayed' );
+
+my $verifyuser = RT::User->new( $RT::SystemUser );
+$verifyuser->Load( $username );
+is( $verifyuser->EmailAddress, $email, "$username email address was retrieved from LDAP after authentication" );
+
+diag 'test login with EmailAddress';
+$m->logout;
+$m->get_ok( $baseurl, 'base url' );
+$m->submit_form(
+    form_number => 1,
+    fields      => { user => $email, pass => 'password', },
+);
+is( $m->uri, $baseurl . '/SelfService/' , 'selfservice page is displayed' );
+
+$ldap->unbind();
+
+done_testing;

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


More information about the rt-commit mailing list