[Rt-commit] rt branch, 4.4/add-ldap-email-authentication, created. rt-4.4.4-205-gd2a595fcdb
? sunnavy
sunnavy at bestpractical.com
Wed Feb 3 16:57:08 EST 2021
The branch, 4.4/add-ldap-email-authentication has been created
at d2a595fcdb58713e0596e89317e4fe11b3b4b632 (commit)
- Log -----------------------------------------------------------------
commit 63c81c99b99b34474c6b3423d500fdf8885a4de9
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..95d63daa7a 100644
--- a/lib/RT/Authen/ExternalAuth.pm
+++ b/lib/RT/Authen/ExternalAuth.pm
@@ -357,6 +357,7 @@ sub DoAuth {
return (0, "User already logged in!") if ($session->{'CurrentUser'} && $session->{'CurrentUser'}->Id);
# For each of those services..
+ my ( $matched_field, $matched_username );
foreach my $service (@auth_services) {
# Get the full configuration for that service as a hashref
@@ -381,6 +382,7 @@ sub DoAuth {
# safely bypass the password checking later on; primarily because
# it's VERY unlikely we even have a password to check if an SSO succeeded.
my $pass_bypass = 0;
+ my $field;
if(defined($username)) {
$RT::Logger->debug("Pass not going to be checked, attempting SSO");
$pass_bypass = 1;
@@ -403,7 +405,9 @@ 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);
+ $field = RT::Authen::ExternalAuth::UserExists( $username, $service ) or next;
+ # 1 means no field info, which will fall back to Name
+ undef $field if $field && $field eq '1';
}
####################################################################
@@ -415,18 +419,16 @@ sub DoAuth {
# Does user already exist internally to RT?
$session->{'CurrentUser'} = RT::CurrentUser->new();
- $session->{'CurrentUser'}->Load($username);
+ $session->{CurrentUser}->LoadByCols( $field || 'Name', $username );
# Unless we have loaded a valid user with a UserID create one.
unless ($session->{'CurrentUser'}->Id) {
my $UserObj = RT::User->new($RT::SystemUser);
my $create = RT->Config->Get('UserAutocreateDefaultsOnLogin')
|| RT->Config->Get('AutoCreate');
- my ($val, $msg) =
- $UserObj->Create(%{ref($create) ? $create : {}},
- Name => $username,
- Gecos => $username,
- );
+
+ my ( $val, $msg ) = $UserObj->Create( %{ ref($create) ? $create : {} },
+ $field ? ( $field => $username ) : ( 'Name' => $username, Gecos => $username, ) );
unless ($val) {
$RT::Logger->error( "Couldn't create user $username: $msg" );
next;
@@ -440,7 +442,7 @@ sub DoAuth {
$RT::Logger->debug("Loading new user (",
$username,
") into current session");
- $session->{'CurrentUser'}->Load($username);
+ $session->{'CurrentUser'}->Load($UserObj->Id);
}
####################################################################
@@ -462,7 +464,11 @@ sub DoAuth {
# If the password check succeeded then this is our authoritative service
# and we proceed to user information update and login.
- last if $success;
+ if ( $success ) {
+ $matched_field = $field if $field;
+ $matched_username = $username;
+ last;
+ }
}
# If we got here and don't have a user loaded we must have failed to
@@ -502,7 +508,7 @@ sub DoAuth {
if ( @{ RT->Config->Get('ExternalInfoPriority') } ) {
# Note that UpdateUserInfo does not care how we authenticated the user
# It will look up user info from whatever is specified in $RT::ExternalInfoPriority
- ($info_updated,$info_updated_msg) = RT::Authen::ExternalAuth::UpdateUserInfo($session->{'CurrentUser'}->Name);
+ ($info_updated,$info_updated_msg) = RT::Authen::ExternalAuth::UpdateUserInfo($matched_username, $matched_field || 'Name');
}
# Now that we definitely have up-to-date user information,
@@ -540,7 +546,8 @@ sub DoAuth {
}
sub UpdateUserInfo {
- my $username = shift;
+ my $username = shift;
+ my $field = shift || 'Name';
# Prepare for the worst...
my $found = 0;
@@ -550,7 +557,7 @@ sub UpdateUserInfo {
my $user_disabled = RT::Authen::ExternalAuth::UserDisabled($username);
my $UserObj = RT::User->new(RT->SystemUser);
- $UserObj->Load($username);
+ $UserObj->LoadByCols($field => $username);
# If user is disabled, set the RT::Principal to disabled and return out of the function.
# I think it's a waste of time and energy to update a user's information if they are disabled
@@ -592,7 +599,7 @@ 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);
+ my %args = ($field => $username);
$UserObj->CanonicalizeUserInfo(\%args);
# For each piece of information returned by CanonicalizeUserInfo,
diff --git a/lib/RT/Authen/ExternalAuth/LDAP.pm b/lib/RT/Authen/ExternalAuth/LDAP.pm
index 2cc53f713a..2782504381 100644
--- a/lib/RT/Authen/ExternalAuth/LDAP.pm
+++ b/lib/RT/Authen/ExternalAuth/LDAP.pm
@@ -207,6 +207,7 @@ sub GetAuth {
my $group_scope = $config->{'group_scope'} || 'base';
my $attr_map = $config->{'attr_map'};
my @attrs = ('dn');
+ my $attr_match_list = $config->{'attr_match_list'};
# Make sure we fetch the user attribute we'll need for the group check
push @attrs, $group_attr_val
@@ -221,45 +222,62 @@ 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 $search_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:",
+ $search_filter->as_string,
+ "== Attrs:",
+ join(',', at attrs) );
+
+ $ldap_msg = $ldap->search( base => $base,
+ filter => $search_filter,
+ attrs => \@attrs );
+
+ unless ( $ldap_msg->code == LDAP_SUCCESS || $ldap_msg->code == LDAP_PARTIAL_RESULTS ) {
+ $RT::Logger->critical( "search for",
+ $search_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->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;
+ 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
+ last;
+ }
}
- 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;
- }
+ # 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 +316,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 $search_filter = Net::LDAP::Filter->new("(${group_attr}=" . escape_filter_value($group_val) . ")");
$RT::Logger->debug( "LDAP Search === ",
"Base:",
@@ -306,12 +324,12 @@ sub GetAuth {
"== Scope:",
$group_scope,
"== Filter:",
- $filter->as_string,
+ $search_filter->as_string,
"== Attrs:",
join(',', at attrs));
$ldap_msg = $ldap->search( base => $group,
- filter => $filter,
+ filter => $search_filter,
attrs => \@attrs,
scope => $group_scope);
@@ -319,7 +337,7 @@ sub GetAuth {
unless ($ldap_msg->code == LDAP_SUCCESS ||
$ldap_msg->code == LDAP_PARTIAL_RESULTS) {
$RT::Logger->critical( "Search for",
- $filter->as_string,
+ $search_filter->as_string,
"failed:",
ldap_error_name($ldap_msg->code),
$ldap_msg->code);
@@ -536,57 +554,88 @@ 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 defined $attr_map->{$attr_match};
}
+ # Ensure we try to get back a Name value from LDAP on the initial LDAP search.
+ push @attrs, $attr_map->{'Name'};
+
my $ldap = _GetBoundLdapObj($config);
return unless $ldap;
- 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 $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 $search_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:",
+ ($search_filter ? $search_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 => $search_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 ) {
+ # 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, jump to the next attr because 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 in LDAP
+ return $attr_match;
+ }
}
- 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 {
@@ -594,7 +643,9 @@ sub UserDisabled {
my ($username,$service) = @_;
# FIRST, check that the user exists in the LDAP service
- unless(UserExists($username,$service)) {
+ my $field = UserExists( $username, $service );
+
+ unless($field) {
$RT::Logger->debug("User (",$username,") doesn't exist! - Assuming not disabled for the purposes of disable checking");
return 0;
}
@@ -620,19 +671,19 @@ sub UserDisabled {
return 0;
}
- if (defined($config->{'attr_map'}->{'Name'})) {
+ if (defined($config->{'attr_map'}->{$field})) {
# Construct the complex filter
$search_filter = Net::LDAP::Filter->new( '(&' .
$filter .
$d_filter .
'(' .
- $config->{'attr_map'}->{'Name'} .
+ $config->{'attr_map'}->{$field} .
'=' .
escape_filter_value($username) .
'))'
);
} else {
- $RT::Logger->debug("You haven't specified an LDAP attribute to match the RT \"Name\" attribute for this service (",
+ $RT::Logger->debug("You haven't specified an LDAP attribute to match the RT \"$field\" attribute for this service (",
$service,
"), so it's impossible look up the disabled status of this user (",
$username,
commit d2a595fcdb58713e0596e89317e4fe11b3b4b632
Author: Blaine Motsinger <blaine at bestpractical.com>
Date: Tue Feb 2 12:24:41 2021 -0600
Add tests for user email login
diff --git a/lib/RT/Test/Web.pm b/lib/RT/Test/Web.pm
index 2b0d33ce03..4093009658 100644
--- a/lib/RT/Test/Web.pm
+++ b/lib/RT/Test/Web.pm
@@ -127,6 +127,14 @@ sub logged_in_as {
my $self = shift;
my $user = shift || '';
+ if ( $user =~ /\@/ ) {
+ my $user_object = RT::User->new( RT->SystemUser );
+ $user_object->LoadByEmail($user);
+ if ( $user_object->Id ) {
+ $user = $user_object->Name;
+ }
+ }
+
unless ( $self->status == HTTP::Status::HTTP_OK ) {
Test::More::diag( "error: status is ". $self->status );
return 0;
diff --git a/t/externalauth/ldap_email_login.t b/t/externalauth/ldap_email_login.t
new file mode 100644
index 0000000000..ffb726f736
--- /dev/null
+++ b/t/externalauth/ldap_email_login.t
@@ -0,0 +1,93 @@
+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 $base = 'dc=bestpractical,dc=com';
+
+RT->Config->Set( ExternalAuthPriority => ['My_LDAP'] );
+RT->Config->Set( ExternalInfoPriority => ['My_LDAP'] );
+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',
+ 'RealName' => 'cn',
+ 'Gecos' => 'uid',
+ 'NickName' => 'nick',
+ }
+ },
+ }
+);
+RT->Config->PostLoadCheck;
+
+my ( $baseurl, $m ) = RT::Test->started_ok();
+
+my $username = 'testuser';
+my $email = "$username\@example.com";
+my $name = 'Test LDAP User';
+my $nick = 'test';
+my $password = 'password';
+my $dn = "uid=$username,$base";
+my $entry = {
+ cn => $name,
+ mail => $email,
+ uid => $username,
+ objectClass => 'User',
+ userPassword => $password,
+ nick => $nick,
+};
+
+$ldap->add($base);
+$ldap->add( $dn, attr => [%$entry] );
+
+diag 'test autocreate';
+
+ok( $m->login( $email, 'password' ), 'Logged in with auto-created user' );
+is( $m->uri, $baseurl . '/SelfService/', 'selfservice page is displayed' );
+
+my $user = RT::User->new($RT::SystemUser);
+$user->Load($username);
+
+is( $user->Name, $username, "$username was autocreated with Name retrieved from LDAP" );
+is( $user->EmailAddress, $email, "$username was autocreated with EmailAddress retrieved from LDAP" );
+is( $user->RealName, $name, "$username was autocreated with RealName retrieved from LDAP" );
+is( $user->Gecos, $username, "$username was autocreated with Gecos retrieved from LDAP" );
+is( $user->NickName, $nick, "$username was autocreated with NickName retrieved from LDAP" );
+
+diag "test user update on login";
+
+ok( $user->SetName("testldapuser") );
+ok( $user->SetNickName("testldapuser") );
+
+ok( $m->login( $email, 'password', logout => 1 ), 'Logged in again' );
+ok( $m->logout, 'logged out' );
+
+$user->Load( $user->Id );
+is( $user->Name, $username, 'Username is updated' );
+is( $user->NickName, $nick, 'Username is updated' );
+
+$ldap->unbind();
+
+done_testing;
-----------------------------------------------------------------------
More information about the rt-commit
mailing list