[Rt-commit] rt branch, 4.4/add-ldap-email-authentication, repushed
? sunnavy
sunnavy at bestpractical.com
Wed Feb 3 16:20:24 EST 2021
The branch 4.4/add-ldap-email-authentication was deleted and repushed:
was e4e26e8a031f33262dee929b2ad5cde7166750b1
now 6d978cb6f30325dd961bd5bd9c3a95f37d0f8d2e
1: 57b60e5c77 ! 1: 909b322ee4 Add LDAP email authentication
@@ -17,10 +17,18 @@
--- a/lib/RT/Authen/ExternalAuth.pm
+++ b/lib/RT/Authen/ExternalAuth.pm
@@
+ 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
+@@
# 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;
++ my $field;
if(defined($username)) {
$RT::Logger->debug("Pass not going to be checked, attempting SSO");
$pass_bypass = 1;
@@ -29,9 +37,9 @@
$RT::Logger->debug("Calling UserExists with \$username ($username) and \$service ($service)");
- next unless RT::Authen::ExternalAuth::UserExists($username, $service);
-+ $exists = RT::Authen::ExternalAuth::UserExists($username, $service) or next;
-+
-+ # TODO: we should last here if UserExists was a success
++ $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';
}
####################################################################
@@ -40,22 +48,83 @@
# Does user already exist internally to RT?
$session->{'CurrentUser'} = RT::CurrentUser->new();
- $session->{'CurrentUser'}->Load($username);
-+
-+ # If a user was found during the LDAP search in UserExists, we need to ensure
-+ # $username is Name instead of EmailAddress, if the user used that to auth.
-+ if ( ref $exists eq 'RT::User' ) {
-+ $username = $exists->Name;
-+ }
-+ # This check is strange, but we need to also allow for other ExternalAuth types to return 1
-+ # for UserExists, while still checking for a valid username from LDAP.
-+ elsif ( $exists !~ /^1$/ ) {
-+ $username = $exists;
-+ }
-+
-+ $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;
+@@
+ $RT::Logger->debug("Loading new user (",
+ $username,
+ ") into current session");
+- $session->{'CurrentUser'}->Load($username);
++ $session->{'CurrentUser'}->Load($UserObj->Id);
+ }
+
+ ####################################################################
+@@
+
+ # 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
+@@
+ 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,
+@@
+ }
+
+ sub UpdateUserInfo {
+- my $username = shift;
++ my $username = shift;
++ my $field = shift || 'Name';
+
+ # Prepare for the worst...
+ my $found = 0;
+@@
+ 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
+@@
+ # 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
--- a/lib/RT/Authen/ExternalAuth/LDAP.pm
@@ -65,11 +134,6 @@
my $attr_map = $config->{'attr_map'};
my @attrs = ('dn');
+ my $attr_match_list = $config->{'attr_match_list'};
-+
-+ foreach my $attr_match (@{$attr_match_list}) {
-+ push @attrs, $attr_map->{$attr_match}
-+ if defined $attr_map->{$attr_match};
-+ }
# Make sure we fetch the user attribute we'll need for the group check
push @attrs, $group_attr_val
@@ -117,7 +181,7 @@
+ "Base:",
+ $base,
+ "== Filter:",
-+ ($search_filter ? $search_filter->as_string : ''),
++ $search_filter->as_string,
+ "== Attrs:",
+ join(',', at attrs) );
+
@@ -226,10 +290,11 @@
+ if 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.
+ 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
@@ -256,29 +321,7 @@
+ 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:",
@@ -287,11 +330,30 @@
+ ($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,
@@ -322,23 +384,7 @@
+ }
+ else {
+ # User was found in LDAP
-+ my $match = RT::User->new($RT::SystemUser);
-+ if ( $attr_match eq 'EmailAddress' ) {
-+ $match->LoadByEmail($username);
-+ }
-+ else {
-+ $match->Load($username);
-+ }
-+
-+ # If the user doesn't exist in RT, return the Name value we got from LDAP
-+ # incase we need to create the user in RT.
-+ unless ( $match->Id ) {
-+ my $ldap_entry = $user_found->first_entry;
-+ my $name_value = $ldap_entry->get_value($attr_map->{'Name'});
-+ $match = $name_value;
-+ }
-+
-+ return $match;
++ return $attr_match;
+ }
}
- undef $user_found;
@@ -351,4 +397,95 @@
}
sub UserDisabled {
+@@
+ return 0;
+ }
+
+- if (defined($config->{'attr_map'}->{'Name'})) {
+- # Construct the complex filter
+- $search_filter = Net::LDAP::Filter->new( '(&' .
+- $filter .
+- $d_filter .
+- '(' .
+- $config->{'attr_map'}->{'Name'} .
+- '=' .
+- 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;
++ return unless $ldap;
+
+- # We only need the UID for confirmation now,
+- # the other information would waste time and bandwidth
+- my @attrs = ('uid');
++ my $attr_map = $config->{'attr_map'};
++ my $attr_match_list = $config->{'attr_match_list'};
++ my @attrs = 'uid';
+
+- $RT::Logger->debug( "LDAP Search === ",
+- "Base:",
+- $base,
+- "== Filter:",
+- ($search_filter ? $search_filter->as_string : ''),
+- "== Attrs:",
+- join(',', at attrs));
++ 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;
++ }
+
+- 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 $search_filter = Net::LDAP::Filter->new(
++ '(&' . $filter . $d_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( ',', @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;
++ }
+ }
++ return 0;
+ }
+ # {{{ sub _GetBoundLdapObj
+
2: 64f64c1521 < -: ------- Add test for LDAP attr search and match
3: e4e26e8a03 < -: ------- Add test for user autocreation with LDAP attr
-: ------- > 2: 6d978cb6f3 Add tests for user email login
More information about the rt-commit
mailing list