[Rt-commit] rt branch, 4.4/add-ldap-email-authentication, repushed
Blaine Motsinger
blaine at bestpractical.com
Fri Jan 29 15:03:22 EST 2021
The branch 4.4/add-ldap-email-authentication was deleted and repushed:
was 0d25844552b4b92481edcdf24dab918578981847
now f0a107b699e5f9eaf1abe87a9ace08cbe71655a0
1: 2fe0f25870 ! 1: ed3ec68cfc Add LDAP email authentication
@@ -17,11 +17,21 @@
--- a/lib/RT/Authen/ExternalAuth.pm
+++ b/lib/RT/Authen/ExternalAuth.pm
@@
+ # 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 $exists;
+ if(defined($username)) {
+ $RT::Logger->debug("Pass not going to be checked, attempting SSO");
+ $pass_bypass = 1;
+@@
# 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);
++ $exists = RT::Authen::ExternalAuth::UserExists($username, $service) or next;
++
++ # TODO: we should last here if UserExists was a success
}
####################################################################
@@ -31,52 +41,34 @@
$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') {
++ if ( ref $exists && defined $exists->{'EmailAddress'} && $exists->{'EmailAddress'} eq $username ) {
+ $session->{'CurrentUser'}->LoadByEmail($username);
+ }
+ else {
+ $session->{'CurrentUser'}->Load($username);
+ }
+
-+ delete $session->{'_ldap_attr_match'};
++ # If LDAP search found the user, and Name was returned, ensure $username is set to Name.
++ # We want to try and ensure the autocreated user below has Name as name and not EmailAddress.
++ if ( ref $exists && $exists->{'Name'} ) {
++ $username = $exists->{'Name'};
++ }
# Unless we have loaded a valid user with a UserID create one.
unless ($session->{'CurrentUser'}->Id) {
-@@
- # 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;
-
-@@
- 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
--- a/lib/RT/Authen/ExternalAuth/LDAP.pm
+++ b/lib/RT/Authen/ExternalAuth/LDAP.pm
@@
- 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 @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};
++ if exists $attr_map->{$attr_match} && defined $attr_map->{$attr_match};
+ }
# Make sure we fetch the user attribute we'll need for the group check
@@ -96,7 +88,7 @@
+ # 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} ) {
++ unless ( exists $attr_map->{$attr_match} && defined $attr_map->{$attr_match} ) {
+ $RT::Logger->error( "Invalid LDAP mapping for $attr_match, no defined fields in attr_map" );
+ next;
+ }
@@ -212,15 +204,6 @@
ldap_error_name($ldap_msg->code),
$ldap_msg->code);
@@
- }
-
- 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};
-
-@@
# but there's no harm in doing this to be sure
undef $filter if defined $filter and $filter eq "()";
@@ -240,19 +223,34 @@
+ 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};
++ if exists $attr_map->{$attr_match} && defined $attr_map->{$attr_match};
}
- my $ldap = _GetBoundLdapObj($config);
- return unless $ldap;
++ # Ensure we try to get back a Name value from LDAP on the initial LDAP search.
++ my $name_attr;
++ if ( defined $attr_map->{'Name'} ) {
++ push @attrs, $attr_map->{'Name'};
++ $name_attr = $attr_map->{'Name'};
++ }
+
+- 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 ( exists $attr_map->{$attr_match} && $attr_map->{$attr_match} ) {
++ unless ( exists $attr_map->{$attr_match} && defined $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'}});
+- # 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 .
@@ -263,28 +261,11 @@
+ '))'
+ );
-- # 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;
-
- my $user_found = $ldap->search( base => $base,
- filter => $filter,
- attrs => \@attrs);
-+ # 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 $ldap = _GetBoundLdapObj($config);
++ return unless $ldap;
- if($user_found->count < 1) {
- # If 0 or negative integer, no user found or major failure
@@ -302,6 +283,15 @@
- $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:",
++ ($search_filter ? $search_filter->as_string : ''),
++ "== Attrs:",
++ join(',', at attrs) );
++
+ my $user_found = $ldap->search( base => $base,
+ filter => $search_filter,
+ attrs => \@attrs );
@@ -336,11 +326,18 @@
+ }
+ else {
+ # User was found
-+ # 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;
++ my $match = {
++ $attr_match => $username,
++ };
++
++ if ( $attr_match ne 'Name' && $name_attr ) {
++ my $ldap_entry = $user_found->first_entry;
++ my $name_value = $ldap_entry->get_value($name_attr);
++
++ $match->{'Name'} = $name_value;
++ }
++
++ return $match;
+ }
}
- undef $user_found;
@@ -353,106 +350,4 @@
}
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
-@@
- 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;
--
-- # We only need the UID for confirmation now,
-- # the other information would waste time and bandwidth
-- my @attrs = ('uid');
-+ my $ldap = _GetBoundLdapObj($config);
-+ next unless $ldap;
-
-- $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
-
2: 0d25844552 = 2: f0a107b699 Add test for LDAP attr search and match
More information about the rt-commit
mailing list