[Bps-public-commit] r16680 - in RT-Authen-ExternalAuth/trunk: html/Callbacks/ExternalAuth/autohandler lib/RT/Authen/ExternalAuth

zordrak at bestpractical.com zordrak at bestpractical.com
Wed Nov 5 10:02:34 EST 2008


Author: zordrak
Date: Wed Nov  5 10:02:31 2008
New Revision: 16680

Modified:
   RT-Authen-ExternalAuth/trunk/html/Callbacks/ExternalAuth/autohandler/Auth
   RT-Authen-ExternalAuth/trunk/lib/RT/Authen/ExternalAuth/DBI.pm
   RT-Authen-ExternalAuth/trunk/lib/RT/Authen/ExternalAuth/LDAP.pm
   RT-Authen-ExternalAuth/trunk/lib/RT/User_Vendor.pm

Log:
zordrak | 2008-11-05 15:01:00 +0000
 * Initial refactoring of ExternalAuth done
 * Next step potentially is to remove the User_Vendor.pm entirely

Modified: RT-Authen-ExternalAuth/trunk/html/Callbacks/ExternalAuth/autohandler/Auth
==============================================================================
--- RT-Authen-ExternalAuth/trunk/html/Callbacks/ExternalAuth/autohandler/Auth	(original)
+++ RT-Authen-ExternalAuth/trunk/html/Callbacks/ExternalAuth/autohandler/Auth	Wed Nov  5 10:02:31 2008
@@ -36,22 +36,23 @@
 
         # Unless we have loaded a valid user with a UserID
         unless ($session{'CurrentUser'}->Id) {
-            # Start with a new SystemUser
-            my $UserObj = RT::User->new($RT::SystemUser);
-            # Set the user's name to the one we were given
-            my ($val, $msg) = $UserObj->SetName($user);
-
-            # If a password was given on the login page, validate it
-            if (defined($pass)) {
-                $password_validated = $UserObj->IsPassword($pass);
+            # Check if user exists externally - autocreate user if it does
+            my $user_exists_externally = 0;
+            my @auth_services = @$RT::ExternalAuthPriority;
+            foreach my $service (@auth_services) {
+                my $config = $RT::Settings->{$service};
+                if ($config->{'type'} eq 'db') {    
+                    $user_exists_externally = RT::Authen::ExternalAuth::DBI->UserExists($service,$user);
+                    last if $user_exists_externally;
+                } elsif ($config->{'type'} eq 'ldap') {
+                    $user_exists_externally = RT::Authen::ExternalAuth::LDAP->UserExists($service,$user);
+                    last if $user_exists_externally;
+                } else {
+                    $RT::Logger->error("Invalid type specification in config for service:",$service);
+                }
             }
             
-            # If the password was validated successfully
-            # start the autocreation process to create the user
-            # permanently in RT
-            if ($password_validated) {
-                    ### If there were a standard param to check for whether or not we
-                    ### should autocreate authenticated users, we'd check it here.
+            if($user_exists_externally){
                     my ($val, $msg) = 
                       $UserObj->Create(%{ref($RT::AutoCreate) ? $RT::AutoCreate : {}},
                                        Name   => $user,
@@ -69,12 +70,21 @@
                     $user_autocreated = 1;
             }
 
-            # If we autocreated a user, then load the user as the CurrentUser in $session
-            # To RT, this means we have a valid, authenticated user
-            if ($UserObj->Id) {
-                my ($ret, $msg) = $session{'CurrentUser'}->Load($user);
-                unless ($ret) {
-                    $RT::Logger->error("Couldn't load user $user: $msg");
+            my ($val, $msg) = $UserObj->SetName($user);
+
+            # If a password was given on the login page, validate it
+            if (defined($pass)) {
+                $password_validated = $UserObj->IsPassword($pass);
+            }
+            
+            if($password_validated) {
+                # If we autocreated a user, then load the user as the CurrentUser in $session
+                # To RT, this means we have a valid, authenticated user
+                if ($UserObj->Id) {
+                    my ($ret, $msg) = $session{'CurrentUser'}->Load($user);
+                    unless ($ret) {
+                        $RT::Logger->error("Couldn't load user $user: $msg");
+                    }
                 }
             }
         }

Modified: RT-Authen-ExternalAuth/trunk/lib/RT/Authen/ExternalAuth/DBI.pm
==============================================================================
--- RT-Authen-ExternalAuth/trunk/lib/RT/Authen/ExternalAuth/DBI.pm	(original)
+++ RT-Authen-ExternalAuth/trunk/lib/RT/Authen/ExternalAuth/DBI.pm	Wed Nov  5 10:02:31 2008
@@ -1,7 +1,7 @@
 package RT::Authen::ExternalAuth::DBI;
 use DBI;
 
-sub getAuth {
+sub GetAuth {
 
     my ($service, $username, $password) = @_;
     
@@ -102,7 +102,7 @@
     return 1;   
 }
 
-sub FindUserThenReturnInfo {
+sub CanonicalizeUSerInfo {
     
     my ($service, $key, $value) = @_;
 
@@ -186,6 +186,128 @@
     return ($found, %params);
 }
 
+sub UserExists {
+    
+    my ($username,$service) = @_;
+    my $config              = $RT::ExternalSettings->{$service};
+    my $table    	        = $config->{'table'};
+    my $u_field	            = $config->{'u_field'};
+    my $query               = "SELECT $u_field FROM $table WHERE $u_field=?";
+    my @bind_params         = ($username);
+
+    # Uncomment this to do a basic trace on DBI information and log it
+    # DBI->trace(1,'/tmp/dbi.log');
+    
+    # Get DBI Object, do the query, disconnect
+    my $dbh = $self->_GetBoundDBIObj($config);
+    my $results_hashref = $dbh->selectall_hashref($query,$u_field,{}, at bind_params);
+    $dbh->disconnect();
+
+    my $num_of_results = scalar keys %$results_hashref;
+        
+    if ($num_of_results > 1) { 
+        # If more than one result returned, die because we the username field should be unique!
+        $RT::Logger->debug( "Disable Check Failed :: (",
+                            $service,
+                            ")",
+                            $username,
+                            "More than one user with that username!");
+        return 0;
+    } elsif ($num_of_results < 1) { 
+        # If 0 or negative integer, no user found or major failure
+        $RT::Logger->debug( "Disable Check Failed :: (",
+                            $service,
+                            ")",
+                            $username,
+                            "User not found");   
+        return 0; 
+    }
+    
+    # Number of results is exactly one, so we found the user we were looking for
+    return 1;            
+}
+
+sub UserDisabled {
+
+    my ($username,$service) = @_;
+    
+    # FIRST, check that the user exists in the DBI service
+    unless UserExists($username,$service) {
+        $RT::Logger->debug("User (",$username,") doesn't exist! - Assuming not disabled for the purposes of disable checking";
+        return 0;
+    }
+    
+    # Get the necessary config info
+    my $config              = $RT::ExternalSettings->{$service};
+    my $table    	        = $config->{'table'};
+    my $u_field	            = $config->{'u_field'};
+    my $disable_field       = $config->{'d_field'};
+    my $disable_values_list = $config->{'d_values'};
+
+    unless ($disable_field) {
+        # If we don't know how to check for disabled users, consider them all enabled.
+        $RT::Logger->debug("No d_field specified for this DBI service (",
+                            $service,
+                            "), so considering all users enabled");
+        return 0;
+    } 
+    
+    my $query = "SELECT $u_field,$disable_field FROM $table WHERE $u_field=?";
+    my @bind_params = ($username);
+
+    # Uncomment this to do a basic trace on DBI information and log it
+    # DBI->trace(1,'/tmp/dbi.log');
+    
+    # Get DBI Object, do the query, disconnect
+    my $dbh = $self->_GetBoundDBIObj($config);
+    my $results_hashref = $dbh->selectall_hashref($query,$u_field,{}, at bind_params);
+    $dbh->disconnect();
+
+    my $num_of_results = scalar keys %$results_hashref;
+        
+    if ($num_of_results > 1) { 
+        # If more than one result returned, die because we the username field should be unique!
+        $RT::Logger->debug( "Disable Check Failed :: (",
+                            $service,
+                            ")",
+                            $username,
+                            "More than one user with that username! - Assuming not disabled");
+        # Drop out to next service for an info check
+        return 0;
+    } elsif ($num_of_results < 1) { 
+        # If 0 or negative integer, no user found or major failure
+        $RT::Logger->debug( "Disable Check Failed :: (",
+                            $service,
+                            ")",
+                            $username,
+                            "User not found - Assuming not disabled");   
+        # Drop out to next service for an info check
+        return 0;             
+    } else { 
+        # otherwise all should be well
+        
+        # $user_db_disable_value = The value for "disabled" returned from the DB
+        my $user_db_disable_value = $results_hashref->{$username}->{$disable_field};
+        
+        # For each of the values in the (list of values that we consider to mean the user is disabled)..
+        foreach my $disable_value (@{$disable_values_list}){
+            $RT::Logger->debug( "DB Disable Check:", 
+                                "User's Val is $user_db_disable_value,",
+                                "Checking against: $disable_value");
+            
+            # If the value from the DB matches a value from the list, the user is disabled.
+            if ($user_db_disable_value eq $disable_value) {
+                return 1;
+            }
+        }
+        
+        # If we've not returned yet, the user can't be disabled
+        return 0;
+    }
+    $RT::Logger->crit("It is seriously not possible to run this code.. what the hell did you do?!");
+    return 0;
+}
+
 # {{{ sub _GetBoundDBIObj
 
 sub _GetBoundDBIObj {

Modified: RT-Authen-ExternalAuth/trunk/lib/RT/Authen/ExternalAuth/LDAP.pm
==============================================================================
--- RT-Authen-ExternalAuth/trunk/lib/RT/Authen/ExternalAuth/LDAP.pm	(original)
+++ RT-Authen-ExternalAuth/trunk/lib/RT/Authen/ExternalAuth/LDAP.pm	Wed Nov  5 10:02:31 2008
@@ -3,7 +3,7 @@
 use Net::LDAP::Util qw(ldap_error_name);
 use Net::LDAP::Filter;
 
-sub getAuth {
+sub GetAuth {
     
     my ($service, $username, $password) = @_;
     
@@ -264,6 +264,153 @@
     return ($found, %params);
 }
 
+sub UserExists {
+    
+    my ($username,$service) = @_;
+    my $config              = $RT::ExternalSettings->{$service};
+    
+    my $base                = $config->{'base'};
+    my $filter              = $config->{'filter'};
+
+    # While LDAP filters must be surrounded by parentheses, an empty set
+    # of parentheses is an invalid filter and will cause failure
+    # This shouldn't matter since we are now using Net::LDAP::Filter below,
+    # but there's no harm in doing this to be sure
+    if ($filter eq "()") { undef($filter) };
+
+    if (defined($config->{'attr_map'}->{'Name'})) {
+        # Construct the complex filter
+        $filter = Net::LDAP::Filter->new(           '(&' . 
+                                                    $filter . 
+                                                    '(' . 
+                                                    $config->{'attr_map'}->{'Name'} . 
+                                                    '=' . 
+                                                    $username . 
+                                                    '))'
+                                        );
+    }
+
+    my $ldap = $self->_GetBoundLdapObj($config);
+    next unless $ldap;
+
+    my @attrs = values(%{$config->{'attr_map'}});
+
+    # Check that the user exists in the LDAP service
+    $RT::Logger->debug( "LDAP Search === ",
+                        "Base:",
+                        $base,
+                        "== Filter:", 
+                        $filter->as_string,
+                        "== Attrs:", 
+                        join(',', at attrs));
+    
+    my $user_found = $ldap->search( base    => $base,
+                                    filter  => $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;
+    }
+    undef $user_found;
+    
+    # If we havent returned now, there must be a valid user.
+    return 1;
+}
+
+sub UserDisabled {
+
+    my ($username,$service) = @_;
+
+    # FIRST, check that the user exists in the LDAP service
+    unless UserExists($username,$service) {
+        $RT::Logger->debug("User (",$username,") doesn't exist! - Assuming not disabled for the purposes of disable checking";
+        return 0;
+    }
+    
+    my $config          = $RT::ExternalSettings->{$service};
+    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
+    # This shouldn't matter since we are now using Net::LDAP::Filter below,
+    # but there's no harm in doing this to be sure
+    if ($filter eq "()") { undef($filter) };
+    if ($d_filter eq "()") { undef($d_filter) };
+
+    unless ($d_filter) {
+        # If we don't know how to check for disabled users, consider them all enabled.
+        $RT::Logger->debug("No d_filter specified for this LDAP service (",
+                            $service,
+                            "), so considering all users enabled");
+        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'} . 
+                                                    '=' . 
+                                                    $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 = $self->_GetBoundLdapObj($config);
+    next unless $ldap;
+
+    # We only need the UID for confirmation now, 
+    # the other information would waste time and bandwidth
+    @attrs = ('uid'); 
+    
+    $RT::Logger->debug( "LDAP Search === ",
+                        "Base:",
+                        $base,
+                        "== 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;
+    }
+}
 # {{{ sub _GetBoundLdapObj
 
 sub _GetBoundLdapObj {

Modified: RT-Authen-ExternalAuth/trunk/lib/RT/User_Vendor.pm
==============================================================================
--- RT-Authen-ExternalAuth/trunk/lib/RT/User_Vendor.pm	(original)
+++ RT-Authen-ExternalAuth/trunk/lib/RT/User_Vendor.pm	Wed Nov  5 10:02:31 2008
@@ -1,36 +1,9 @@
-### User_Local.pm overlay for External Service authentication and information
-###
-### CREDITS
+### User_Vendor.pm
+# Overlay for RT::User object as part of RT::Authen::ExternalAuth
 #
-# Based on User_Local.pm for LDAP created by JimMeyer and found at:
+# Originally based on User_Local.pm for LDAP created by Jim Meyer (purp at acm.org) and found at:
 #   http://wiki.bestpractical.com/view/LdapUserLocalOverlay
-#
-# His Credits:
-#
-#   IsLDAPPassword() based on implementation of IsPassword() found at:
-#
-#   http://www.justatheory.com/computers/programming/perl/rt/User_Local.pm.ldap
-#
-#   Author's credits:
-#   Modification Originally by Marcelo Bartsch <bartschm_cl at hotmail.com>
-#   Update by Stewart James <stewart.james at vu.edu.au for rt3.
-#   Update by David Wheeler <david at kineticode.com> for TLS and 
-#      Group membership support.
-#
-#
-#   CaonicalizeEmailAddress(), CanonicalizeUserInfo(), and LookupExternalInfo()
-#   based on work by Phillip Cole (phillip d cole @ uk d coltgroup d com)
-#   found at:
-#
-#   http://wiki.bestpractical.com/view/AutoCreateAndCanonicalizeUserInfo
-#
-#   His credits:
-#     based on CurrentUser_Local.pm and much help from the mailing lists 
-#
-#   All integrated, refactored, and updated by Jim Meyer (purp at acm.org)
-#
-# Modified to provide alternate external services authentication and information for rt3
-# as part of RT::Authen::ExternalAuth by Mike Peachey (mike.peachey at jennic.com)
+
 
 no warnings qw(redefine);
 use strict;
@@ -64,12 +37,12 @@
         # And then act accordingly depending on what type of service it is.
         # Right now, there is only code for DBI and LDAP services
         if ($config->{'type'} eq 'db') {    
-            my $success = RT::Authen::ExternalAuth::DBI->getAuth($service,$name_to_auth,$pass_to_auth);
+            my $success = RT::Authen::ExternalAuth::DBI->GetAuth($service,$name_to_auth,$pass_to_auth);
             return 1 if $success;
             next;
             
         } elsif ($config->{'type'} eq 'ldap') {
-            my $success = RT::Authen::ExternalAuth::LDAP->getAuth($service,$name_to_auth,$pass_to_auth);
+            my $success = RT::Authen::ExternalAuth::LDAP->GetAuth($service,$name_to_auth,$pass_to_auth);
             return 1 if $success;
             next;
                     
@@ -179,8 +152,10 @@
     my $args = shift;
 
     my $found = 0;
-    my %params = ();
-
+    my %params = (Name         => undef,
+                  EmailAddress => undef,
+                  RealName     => undef);
+    
     $RT::Logger->debug( (caller(0))[3], 
                         "called by", 
                         caller, 
@@ -201,15 +176,52 @@
         
         # For each attr we've been told to canonicalize in the match list
         foreach my $rt_attr (@{$config->{'attr_match_list'}}) {
-            # Jump to the next attr if it's not an RT attr passed in $args
-            $RT::Logger->debug( "Attempting to use this canonicalization key:",
-                                $rt_attr);
-            next unless defined($args->{$rt_attr});
-                                
-            # Else, use it as a key for GetUserInfoHash    
-            ($found, %params) = 
-                $self->GetUserInfoHash($service,$config->{'attr_map'}->{$rt_attr},$args->{$rt_attr});
-         
+            # Jump to the next attr in $args if this one isn't in the attr_match_list
+            $RT::Logger->debug( "Attempting to use this canonicalization key:",$rt_attr);
+            unless defined($args->{$rt_attr}) {
+                $RT::Logger->debug("This attribute (",
+                                    $rt_attr,
+                                    ") is not defined in the attr_match_list for this service (",
+                                    $service,
+                                    ")");
+                next;
+            }
+                               
+            # Else, use it as a canonicalization key and lookup the user info    
+            my $key = $config->{'attr_map'}->{$rt_attr};
+            my $value = $args->{$rt_attr};
+            
+            # Check to see that the key being asked for is defined in the config's attr_map
+            my $valid = 0;
+            my ($attr_key, $attr_value);
+            my $attr_map = $config->{'attr_map'};
+            while (($attr_key, $attr_value) = each %$attr_map) {
+                $valid = 1 if ($key eq $attr_value);
+            }
+            unless ($valid){
+                $RT::Logger->debug( "This key (",
+                                    $key,
+                                    "is not a valid attribute key (",
+                                    $service,
+                                    ")");
+                next;
+            }
+            
+            # Use an if/elsif structure to do a lookup with any custom code needed 
+            # for any given type of external service, or die if no code exists for
+            # the service requested.
+            
+            if($config->{'type'} eq 'ldap'){    
+                ($found, %params) = RT::Authen::ExternalAuth::LDAP->CanonicalizeUserInfo($service,$key,$value);
+            } elsif ($config->{'type'} eq 'db') {
+                ($found, %params) = RT::Authen::ExternalAuth::DBI->CanonicalizeUserInfo($service,$key,$value);
+            } else {
+                $RT::Logger->debug( (caller(0))[3],
+                                    "does not consider",
+                                    $service,
+                                    "a valid information service");
+            }
+       
             # Don't Check any more attributes
             last if $found;
         }
@@ -242,85 +254,6 @@
 }
 # }}}
 
-# {{{ sub LookupExternalUserInfo
-
-=head2 GetUserInfoHash SERVICE KEY VALUE
-
-LookupExternalUserInfo takes a key/value pair and an external service
-to look it up in; looks it up and returns a params hash containing 
-all attrs listed in the source's attr_map, suitable for creating 
-an RT::User object.
-
-Returns a tuple, ($found, %params)
-
-=cut
-
-sub GetUserInfoHash {
-    my $self = shift;
-    my ($service,$key, $value) = @_;
-    
-    # Haven't got anything yet..
-    my $found = 0;
-    my %params = (Name         => undef,
-                  EmailAddress => undef,
-                  RealName     => undef);
-    
-    # Get the full configuration for the service in question
-    my $config = $RT::ExternalSettings->{$service};
-    
-    # Check to see that the key being asked for is defined in the config's attr_map
-    my $valid = 0;
-    my ($attr_key, $attr_value);
-    my $attr_map = $config->{'attr_map'};
-    while (($attr_key, $attr_value) = each %$attr_map) {
-        $valid = 1 if ($key eq $attr_value);
-    }
-    unless ($valid){
-        $RT::Logger->debug( $key,
-                            "is not valid attribute key (",
-                            $service,
-                            ")");
-        return ($found, %params);
-    }
-    
-    # Use an if/elsif structure to do a lookup with any custom code needed 
-    # for any given type of external service, or die if no code exists for
-    # the service requested.
-    
-    if($config->{'type'} eq 'ldap'){    
-        ($found, %params) = RT::Authen::ExternalAuth::LDAP->CanonicalizeUserInfo($service,$key,$value);
-    } elsif ($config->{'type'} eq 'db') {
-        ($found, %params) = RT::Authen::ExternalAuth::DBI->CanonicalizeUserInfo($service,$key,$value);
-    } else {
-        $RT::Logger->debug( (caller(0))[3],
-                            "does not consider",
-                            $service,
-                            "a valid information service");
-    }
-
-
-    # Why on earth do we return the same RealName, just quoted?!
-    # Seconded by Mike Peachey - I'd like to know that too!!
-    # Sod it, until it breaks something, I'm removing this line forever!
-    # $params{'RealName'} = "\"$params{'RealName'}\"";
-    
-    
-    # Log the info found and return it
-    $RT::Logger->info(  (caller(0))[3],
-                        ": Returning: ",
-                        join(", ", map {sprintf("%s: %s", $_, $params{$_})}
-                            sort(keys(%params))));
-    
-    $RT::Logger->debug( (caller(0))[3],
-                        "No user was found this time"
-                      ) if ($found == 0);
-
-    return ($found, %params);
-}
-
-# }}}
-
-
 sub UpdateFromExternal {
     my $self = shift;
 
@@ -329,7 +262,7 @@
     my $updated = 0;
     my $msg = "User NOT updated";
     
-    my $name_to_update  	= $self->Name;
+    my $username  	= $self->Name;
     my $user_disabled 	    = 0;
     
     # Get the list of information service names requested by user.    
@@ -346,177 +279,37 @@
         my $config = $RT::ExternalSettings->{$service};
         
         # If the config doesn't exist, don't bother doing anything, skip to next in list.
-        next unless defined($config);
+        unless defined($config) {
+            $RT::Logger->debug("You haven't defined a configuration for the service named \"",
+                                $service,
+                                "\" so I'm not going to try to get user information from it. Skipping...");
+            next;
+        }
         
         # If it's a DBI config:
         if ($config->{'type'} eq 'db') {
-            # Get the necessary config info
-            my $table    	        = $config->{'table'};
-    	    my $u_field	            = $config->{'u_field'};
-            my $disable_field       = $config->{'d_field'};
-            my $disable_values_list = $config->{'d_values'};
-
-            # Only lookup disable information from the DB if a disable_field has been set
-            if ($disable_field) { 
-                my $query = "SELECT $u_field,$disable_field FROM $table WHERE $u_field=?";
-        	    my @bind_params = ($name_to_update);
-
-                # Uncomment this to do a basic trace on DBI information and log it
-                # DBI->trace(1,'/tmp/dbi.log');
-                
-                # Get DBI Object, do the query, disconnect
-                my $dbh = $self->_GetBoundDBIObj($config);
-                my $results_hashref = $dbh->selectall_hashref($query,$u_field,{}, at bind_params);
-                $dbh->disconnect();
-
-                my $num_of_results = scalar keys %$results_hashref;
-                    
-                if ($num_of_results > 1) { 
-                    # If more than one result returned, die because we the username field should be unique!
-                    $RT::Logger->debug( "Disable Check Failed :: (",
-                                        $service,
-                                        ")",
-                                        $name_to_update,
-                                        "More than one user with that username!");
-                    # Drop out to next service for an info check
-                    next;
-                } elsif ($num_of_results < 1) { 
-                    # If 0 or negative integer, no user found or major failure
-                    $RT::Logger->debug( "Disable Check Failed :: (",
-                                        $service,
-                                        ")",
-                                        $name_to_update,
-                                        "User not found");   
-                    # Drop out to next service for an info check
-                    next;             
-                } else { 
-                    # otherwise all should be well
-                    
-                    # $user_db_disable_value = The value for "disabled" returned from the DB
-                    my $user_db_disable_value = $results_hashref->{$name_to_update}->{$disable_field};
-                    
-                    # For each of the values in the (list of values that we consider to mean the user is disabled)..
-                    foreach my $disable_value (@{$disable_values_list}){
-                        $RT::Logger->debug( "DB Disable Check:", 
-                                            "User's Val is $user_db_disable_value,",
-                                            "Checking against: $disable_value");
-                        
-                        # If the value from the DB matches a value from the list, the user is disabled.
-                        if ($user_db_disable_value eq $disable_value) {
-                            $user_disabled = 1;
-                        }
-                    }
-                }
-            }
-            
-            # If we havent been dropped out by a "next;" by now, 
-            # then this will be the authoritative service
-            
-        } elsif ($config->{'type'} eq 'ldap') {
-            
-            my $base            = $config->{'base'};
-            my $filter          = $config->{'filter'};
-            my $disable_filter  = $config->{'d_filter'};
-            
-            my ($u_filter,$d_filter);
-
-            # While LDAP filters must be surrounded by parentheses, an empty set
-            # of parentheses is an invalid filter and will cause failure
-            # This shouldn't matter since we are now using Net::LDAP::Filter below,
-            # but there's no harm in doing this to be sure
-            if ($filter eq "()") { undef($filter) };
-            if ($disable_filter eq "()") { undef($disable_filter) };
-
-            unless ($disable_filter) {
-                $RT::Logger->error("You haven't specified a d_filter in your configuration.  Not specifying a d_filter usually results in all users being marked as disabled and being unable to log in");
-            }
-
-            if (defined($config->{'attr_map'}->{'Name'})) {
-                # Construct the complex filter
-                $disable_filter = Net::LDAP::Filter->new(   '(&' . 
-                                                            $filter . 
-                                                            $disable_filter . 
-                                                            '(' . 
-                                                            $config->{'attr_map'}->{'Name'} . 
-                                                            '=' . 
-                                                            $self->Name . 
-                                                            '))'
-                                                        );
-                $filter = Net::LDAP::Filter->new(           '(&' . 
-                                                            $filter . 
-                                                            '(' . 
-                                                            $config->{'attr_map'}->{'Name'} . 
-                                                            '=' . 
-                                                            $self->Name . 
-                                                            '))'
-                                                );
-            }
- 
-            my $ldap = $self->_GetBoundLdapObj($config);
-            next unless $ldap;
-
-            my @attrs = values(%{$config->{'attr_map'}});
-
-            # FIRST, check that the user exists in the LDAP service
-            $RT::Logger->debug( "LDAP Search === ",
-                                "Base:",
-                                $base,
-                                "== Filter:", 
-                                $filter->as_string,
-                                "== Attrs:", 
-                                join(',', at attrs));
             
-            my $user_found = $ldap->search( base    => $base,
-                                            filter  => $filter,
-                                            attrs   => \@attrs);
-
-            if($user_found->count < 1) {
-                # If 0 or negative integer, no user found or major failure
-                $RT::Logger->debug( "Disable Check Failed :: (",
+            unless RT::Authen::ExternalAuth::DBI->UserExists($username,$service) {
+                $RT::Logger->debug("User (",
+                                    $username,
+                                    ") doesn't exist in service (",
                                     $service,
-                                    ")",
-                                    $name_to_update,
-                                    "User not found");   
-                # Drop out to next service for an info check
-                next;  
-            } elsif ($user_found->count > 1) {
-                # If more than one result returned, die because we the username field should be unique!
-                $RT::Logger->debug( "Disable Check Failed :: (",
-                                    $service,
-                                    ")",
-                                    $name_to_update,
-                                    "More than one user with that username!");
-                # Drop out to next service for an info check
+                                    ") - Cannot update information - Skipping...");
                 next;
             }
-            undef $user_found;
-                        
-            # SECOND, now we know the user exists in the service, 
-            # check if they are returned in a search for disabled users 
+            $user_disabled = RT::Authen::ExternalAuth::DBI->UserDisabled($username,$service);
             
-            # We only need the UID for confirmation now, 
-            # the other information would waste time and bandwidth
-            @attrs = ('uid'); 
+        } elsif ($config->{'type'} eq 'ldap') {
             
-            $RT::Logger->debug( "LDAP Search === ",
-                                "Base:",
-                                $base,
-                                "== Filter:", 
-                                $disable_filter->as_string,
-                                "== Attrs:", 
-                                join(',', at attrs));
-                  
-            my $disabled_users = $ldap->search(base   => $base, 
-                                               filter => $disable_filter, 
-                                               attrs  => \@attrs);
-            # If ANY results are returned, 
-            # we are going to assume the user should be disabled
-            if ($disabled_users->count) {
-               $user_disabled = 1;
+            unless RT::Authen::ExternalAuth::LDAP->UserExists($username,$service) {
+                $RT::Logger->debug("User (",
+                                    $username,
+                                    ") doesn't exist in service (",
+                                    $service,
+                                    ") - Cannot update information - Skipping...");
+                next;
             }
-            
-            # If we havent been dropped out by a "next;" by now, 
-            # then this will be the authoritative service
+            $user_disabled = RT::Authen::ExternalAuth::LDAP->UserDisabled($username,$service);
             
         } else {
             # The type of external service doesn't currently have any methods associated with it. Or it's a typo.
@@ -541,7 +334,7 @@
         # Load the user inside an RT::SystemUser so you can  set their 
         # information no matter who they are or what permissions they have
         my $UserObj = RT::User->new($RT::SystemUser);
-        $UserObj->Load($name_to_update);        
+        $UserObj->Load($username);        
 
         # If user is disabled, set the RT::Principle 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
@@ -556,24 +349,24 @@
             # Make sure principle is disabled in RT
             my ($val, $message) = $UserObj->SetDisabled(1);
             # Log what has happened
-            $RT::Logger->info("DISABLED user ",
-                                $name_to_update,
-                                "per External Service", 
+            $RT::Logger->info("User marked as DISABLED (",
+                                $username,
+                                ") per External Service", 
                                 "($val, $message)\n");
             $msg = "User disabled";
         } else {
             # Make sure principle is not disabled in RT
             my ($val, $message) = $UserObj->SetDisabled(0);
             # Log what has happened
-            $RT::Logger->info("ENABLED user ",
-                                $name_to_update,
-                                "per External Service",
+            $RT::Logger->info("User marked as ENABLED (",
+                                $username,
+                                ") per External Service",
                                 "($val, $message)\n");
 
             # 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 => $name_to_update);
+            my %args = (Name => $username);
             $self->CanonicalizeUserInfo(\%args);
 
             # For each piece of information returned by CanonicalizeUserInfo,
@@ -598,9 +391,9 @@
 
             # Confirm update success
             $updated = 1;
-            $RT::Logger->debug( "UPDATED user ",
-                                $name_to_update,
-                                "from External Service\n");
+            $RT::Logger->debug( "UPDATED user (",
+                                $username,
+                                ") from External Service\n");
             $msg = 'User updated';
             
             # Just in case we're not the last iteration of the foreach,



More information about the Bps-public-commit mailing list