[Rt-commit] rt branch, 4.4/add-ldap-email-authentication, created. rt-4.4.4-205-geaf25c59d8
Blaine Motsinger
blaine at bestpractical.com
Fri Jan 22 19:53:55 EST 2021
The branch, 4.4/add-ldap-email-authentication has been created
at eaf25c59d8f966d987c0f6c285cadf85e785ccbd (commit)
- Log -----------------------------------------------------------------
commit 6bb0380d958a3d6c604c9280c90180c87c3b68b2
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..64d2ba4075 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,17 @@ 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);
+ }
+
+ delete $session->{'_ldap_attr_match'};
# Unless we have loaded a valid user with a UserID create one.
unless ($session->{'CurrentUser'}->Id) {
@@ -647,7 +657,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 +669,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..7b3c5b1849 100644
--- a/lib/RT/Authen/ExternalAuth/LDAP.pm
+++ b/lib/RT/Authen/ExternalAuth/LDAP.pm
@@ -206,7 +206,13 @@ 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 @attrs = ('dn');
+ my $attr_match_list = $config->{'attr_match_list'};
+
+ my @attrs;
+ foreach my $attr_match (@{$attr_match_list}) {
+ push @attrs, $attr_map->{$attr_match}
+ if exists $attr_map->{$attr_match} && $attr_map->{$attr_match};
+ }
# Make sure we fetch the user attribute we'll need for the group check
push @attrs, $group_attr_val
@@ -221,46 +227,67 @@ 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 ( exists $attr_map->{$attr_match} && $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 .
+ '(' .
+ $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 => $net_ldap_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",
+ $net_ldap_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;
+ }
+
+ if ( $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;
+ }
+ else {
+ # User was found
+ $RT::Logger->debug( "User Check Succeeded :: (",
+ $service,
+ ")",
+ $username );
+ last;
+ }
}
+ # if we didn't match anything, go to the next external auth service
+ return 0 unless $ldap_msg->first_entry;
+
my $ldap_entry = $ldap_msg->first_entry;
my $ldap_dn = $ldap_entry->dn;
@@ -298,7 +325,7 @@ sub GetAuth {
# We only need the dn for the actual group since all we care about is existence
@attrs = qw(dn);
- $filter = Net::LDAP::Filter->new("(${group_attr}=" . escape_filter_value($group_val) . ")");
+ my $net_ldap_filter = Net::LDAP::Filter->new("(${group_attr}=" . escape_filter_value($group_val) . ")");
$RT::Logger->debug( "LDAP Search === ",
"Base:",
@@ -306,12 +333,12 @@ sub GetAuth {
"== Scope:",
$group_scope,
"== Filter:",
- $filter->as_string,
+ $net_ldap_filter->as_string,
"== Attrs:",
join(',', at attrs));
$ldap_msg = $ldap->search( base => $group,
- filter => $filter,
+ filter => $net_ldap_filter,
attrs => \@attrs,
scope => $group_scope);
@@ -319,7 +346,7 @@ sub GetAuth {
unless ($ldap_msg->code == LDAP_SUCCESS ||
$ldap_msg->code == LDAP_PARTIAL_RESULTS) {
$RT::Logger->critical( "Search for",
- $filter->as_string,
+ $net_ldap_filter->as_string,
"failed:",
ldap_error_name($ldap_msg->code),
$ldap_msg->code);
@@ -523,7 +550,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 +563,84 @@ 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 $attr_map = $config->{'attr_map'};
+ my $attr_match_list = $config->{'attr_match_list'};
+
+ my @attrs;
+ foreach my $attr_match (@{$attr_match_list}) {
+ push @attrs, $attr_map->{$attr_match}
+ if exists $attr_map->{$attr_match} && $attr_map->{$attr_match};
}
- my $ldap = _GetBoundLdapObj($config);
- return unless $ldap;
+ # loop over each of the attr_match_list members for the initial lookup
+ foreach my $attr_match ( @{$attr_match_list} ) {
+ unless ( exists $attr_map->{$attr_match} && $attr_map->{$attr_match} ) {
+ $RT::Logger->error( "Invalid LDAP mapping for $attr_match, no defined fields in attr_map" );
+ next;
+ }
- my @attrs = values(%{$config->{'attr_map'}});
+ my $net_ldap_filter = Net::LDAP::Filter->new(
+ '(&' .
+ $filter .
+ '(' .
+ $attr_map->{$attr_match} .
+ '=' .
+ escape_filter_value($username) .
+ '))'
+ );
- # 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 $ldap = _GetBoundLdapObj($config);
+ return unless $ldap;
+
+ # 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 => $filter,
- attrs => \@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");
- 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;
+ unless ( $user_found->code == LDAP_SUCCESS || $user_found->code == LDAP_PARTIAL_RESULTS ) {
+ $RT::Logger->debug( "search for",
+ $filter->as_string,
+ "failed:",
+ ldap_error_name($user_found->code),
+ $user_found->code );
+ # Didn't even get a partial result - jump straight to the next external auth service
+ return 0;
+ }
+
+ if ( $user_found->count != 1 ) {
+ $RT::Logger->debug( "User Check Failed :: (",
+ $service,
+ ")",
+ $username,
+ "User not found or more than one user found 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.
+ # go to the next external auth service.
+ return 0;
}
sub UserDisabled {
@@ -603,7 +657,6 @@ sub UserDisabled {
my $base = $config->{'base'};
my $filter = $config->{'filter'};
my $d_filter = $config->{'d_filter'};
- my $search_filter;
# While LDAP filters must be surrounded by parentheses, an empty set
# of parentheses is an invalid filter and will cause failure
@@ -620,54 +673,57 @@ sub UserDisabled {
return 0;
}
- if (defined($config->{'attr_map'}->{'Name'})) {
+ my $attr_map = $config->{'attr_map'};
+ my $attr_match_list = $config->{'attr_match_list'};
+
+ my @attrs;
+ foreach my $attr_match (@{$attr_match_list}) {
+ push @attrs, $attr_map->{$attr_match}
+ if exists $attr_map->{$attr_match} && $attr_map->{$attr_match};
+ }
+
+ # loop over each of the attr_match_list members for the initial lookup
+ foreach my $attr_match ( @{$attr_match_list} ) {
+ unless ( exists $attr_map->{$attr_match} && $attr_map->{$attr_match} ) {
+ $RT::Logger->debug( "You haven't specified an LDAP attribute to match the RT $attr_match attribute for this service ($service), so it's impossible look up the disabled status of this user ($username)" );
+ next;
+ }
+
# Construct the complex filter
- $search_filter = Net::LDAP::Filter->new( '(&' .
+ my $search_filter = Net::LDAP::Filter->new( '(&' .
$filter .
$d_filter .
'(' .
- $config->{'attr_map'}->{'Name'} .
+ $attr_map->{$attr_match} .
'=' .
escape_filter_value($username) .
'))'
);
- } else {
- $RT::Logger->debug("You haven't specified an LDAP attribute to match the RT \"Name\" attribute for this service (",
- $service,
- "), so it's impossible look up the disabled status of this user (",
- $username,
- ") so I'm just going to assume the user is not disabled");
- return 0;
- }
+ my $ldap = _GetBoundLdapObj($config);
+ next unless $ldap;
- my $ldap = _GetBoundLdapObj($config);
- next unless $ldap;
-
- # We only need the UID for confirmation now,
- # the other information would waste time and bandwidth
- my @attrs = ('uid');
-
- $RT::Logger->debug( "LDAP Search === ",
- "Base:",
- $base,
- "== Filter:",
- ($search_filter ? $search_filter->as_string : ''),
- "== Attrs:",
- join(',', at attrs));
+ $RT::Logger->debug( "LDAP Search === ",
+ "Base:",
+ $base,
+ "== Filter:",
+ ($search_filter ? $search_filter->as_string : ''),
+ "== Attrs:",
+ join(',', at attrs));
- my $disabled_users = $ldap->search(base => $base,
- filter => $search_filter,
- attrs => \@attrs);
- # If ANY results are returned,
- # we are going to assume the user should be disabled
- if ($disabled_users->count) {
- undef $disabled_users;
- return 1;
- } else {
- undef $disabled_users;
- return 0;
+ my $disabled_users = $ldap->search(base => $base,
+ filter => $search_filter,
+ attrs => \@attrs);
+ # If ANY results are returned,
+ # we are going to assume the user should be disabled
+ if ($disabled_users->count) {
+ return 1;
+ } else {
+ next;
+ }
}
+
+ return 0;
}
# {{{ sub _GetBoundLdapObj
commit eaf25c59d8f966d987c0f6c285cadf85e785ccbd
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 a 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