[Rt-commit] rt branch, 4.4/add-ldap-email-authentication, repushed
Blaine Motsinger
blaine at bestpractical.com
Fri Jan 22 19:53:57 EST 2021
The branch 4.4/add-ldap-email-authentication was deleted and repushed:
was 7b8e0c571627d570f70aaaa2a4242a42d4be0350
now eaf25c59d8f966d987c0f6c285cadf85e785ccbd
1: 30370812b5 ! 1: 6bb0380d95 Add LDAP email authentication
@@ -39,6 +39,8 @@
+ 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) {
@@ -68,10 +70,17 @@
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 = ('dn');
++
++ 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
@@
my $ldap = _GetBoundLdapObj($config);
return 0 unless ($ldap);
@@ -87,7 +96,7 @@
+ # 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} ) {
++ 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;
+ }
@@ -103,7 +112,7 @@
+ '(&' .
+ $filter .
+ '(' .
-+ $config->{'attr_map'}->{$attr_match} .
++ $attr_map->{$attr_match} .
+ '=' .
+ escape_filter_value($username) .
+ '))'
@@ -130,7 +139,7 @@
- return 0;
- }
+ $ldap_msg = $ldap->search( base => $base,
-+ filter => $filter,
++ filter => $net_ldap_filter,
+ attrs => \@attrs );
- unless ($ldap_msg->count == 1) {
@@ -142,7 +151,7 @@
- return 0;
+ unless ( $ldap_msg->code == LDAP_SUCCESS || $ldap_msg->code == LDAP_PARTIAL_RESULTS ) {
+ $RT::Logger->debug( "search for",
-+ $filter->as_string,
++ $net_ldap_filter->as_string,
+ "failed:",
+ ldap_error_name($ldap_msg->code),
+ $ldap_msg->code );
@@ -150,7 +159,7 @@
+ return 0;
+ }
+
-+ unless ( $ldap_msg->count == 1 ) {
++ if ( $ldap_msg->count != 1 ) {
+ $RT::Logger->info( $service,
+ "AUTH FAILED:",
+ $username,
@@ -158,9 +167,55 @@
+ # 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;
+
+@@
+
+ # 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:",
+@@
+ "== 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);
+
+@@
+ 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);
@@
}
@@ -184,20 +239,34 @@
- 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;
-+ 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} ) {
++ 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 === ",
@@ -207,38 +276,9 @@
- ($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:",
@@ -247,27 +287,46 @@
+ ($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
+
+- 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" );
-+ 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!" );
++ "User not found or more than one user found with that username!" );
+ next;
+ }
+ else {
@@ -280,6 +339,7 @@
+ # 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;
+ }
}
@@ -288,8 +348,111 @@
- # 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 {
+@@
+ 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;
+
+- 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
+
2: 7b8e0c5716 ! 2: eaf25c59d8 Add test for LDAP attr search and match
@@ -2,7 +2,7 @@
Add test for LDAP attr search and match
- This test verifies LDAP authentication using to second entry in
+ This test verifies LDAP authentication using a second entry in
attr_match_list. In this case EmailAddress instead of the
previously hardcoded Name key.
More information about the rt-commit
mailing list