Auto update user's DN if it changes on the LDAP server. Fixes bug #11217

docs/changelog/7.x.x.txt

@@ -14,6 +14,7 @@
  - fixed #11226: New stylesheet (wg-base.css), new style templates (from the TWG)
  - fixed #11216: LDAP Connections status incorrect
  - fixed #11229: ProgressBar throws errors on some messages.
+ - fixed #11217: LDAP authentication fails if user DN changes
 
 7.8.4
  - Fixed a compatibility problem between WRE and new Spectre code.

lib/WebGUI/Auth.pm

@@ -701,8 +701,8 @@ Returns a hash reference with the user's authentication information.  This metho
 =cut
 
 sub getParams {
-	my $self = shift;
+	my $self       = shift;
-	my $userId = $_[0] || $self->userId;
+	my $userId     = $_[0] || $self->userId;
 	my $authMethod = $_[1] || $self->authMethod;
 	return $self->session->db->buildHashRef("select fieldName, fieldData from authentication where userId=".$self->session->db->quote($userId)." and authMethod=".$self->session->db->quote($authMethod));
 }

lib/WebGUI/Auth/LDAP.pm

@@ -40,9 +40,11 @@ i.e., it does not validate their username or ensure their account is active.
 =cut
 
 sub _isValidLDAPUser {
-    my $self = shift;
+    my $self    = shift;
+    my $session = $self->session;
+    my $form    = $session->form;
     my ($error, $ldap, $search, $auth, $connectDN);
-    my $i18n = WebGUI::International->new($self->session);
+    my $i18n = WebGUI::International->new($session);
 
     my $connection = $self->getLDAPConnection;
     return 0 unless $connection;
@@ -53,8 +55,8 @@ sub _isValidLDAPUser {
         $self->error('<li>'.$i18n->get(2,'AuthLDAP').'</li>');
         return 0;
     }
-    my $username = $self->session->form->get("authLDAP_ldapId") || $self->session->form->get("username");
+    my $username = $form->get("authLDAP_ldapId")     || $form->get("username");
-    my $password = $self->session->form->get("authLDAP_identifier") || $self->session->form->get("identifier");
+    my $password = $form->get("authLDAP_identifier") || $form->get("identifier");
 
     my $uri = URI->new($connection->{ldapUrl}) or $error = '<li>'.$i18n->get(2,'AuthLDAP').'</li>';
 
@@ -102,27 +104,27 @@ sub _isValidLDAPUser {
                 # Invalid login credentials, directory did not authenticate the user
                 if ($auth->code == 48 || $auth->code == 49) {
                     $error .= '<li>'.$i18n->get(68).'</li>';
-                    $self->session->errorHandler->warn("Invalid LDAP information for registration of LDAP ID: ".$self->session->form->process('authLDAP_ldapId'));
+                    $session->log->warn("Invalid LDAP information for registration of LDAP ID: ".$self->session->form->process('authLDAP_ldapId'));
                 }
                 elsif ($auth->code > 0) {  # Some other LDAP error occured
                     $error .= '<li>LDAP error "'.$self->ldapStatusCode($auth->code).'" occured. '.$i18n->get(69).'</li>';
-                    $self->session->errorHandler->error("LDAP error: ".$self->ldapStatusCode($auth->code));
+                    $session->log->error("LDAP error: ".$self->ldapStatusCode($auth->code));
                 }
                 $ldap->unbind;
             }
             else { # Could not find the user in the directory to build a DN
                 $error .= '<li>'.$i18n->get(68).'</li>';
-                $self->session->errorHandler->warn("Invalid LDAP information for registration of LDAP ID: ".$self->session->form->process("authLDAP_ldapId"));
+                $session->log->warn("Invalid LDAP information for registration of LDAP ID: ".$self->session->form->process("authLDAP_ldapId"));
             }
         }
         else { # Unable to bind with proxy user credentials or anonymously for our search
             $error = '<li>'.$i18n->get(2,'AuthLDAP').'</li>';
-            $self->session->errorHandler->error("Couldn't bind to LDAP server: ".$connection->{ldapUrl});
+            $session->log->error("Couldn't bind to LDAP server: ".$connection->{ldapUrl});
         }
    }
    else { # Could not create our LDAP object
       $error = '<li>'.$i18n->get(2,'AuthLDAP').'</li>';
-      $self->session->errorHandler->error("Couldn't create LDAP object: ".$connection->{ldapUrl});
+      $session->log->error("Couldn't create LDAP object: ".$connection->{ldapUrl});
    }
 
    $self->error($error);
@@ -176,21 +178,32 @@ sub authenticate {
 
         # Try to bind using the users dn and password
         $auth = $ldap->bind(dn=>$userData->{connectDN}, password=>$identifier);
+
+        # Failure to bind could have resulted from change to in DN on LDAP server.
+        # Test for new DN and update user account as needed
+        if ($auth->code > 0 && $self->_isValidLDAPUser()) {
+            # Update user profile and log change
+            # _isValidLDAPUser will set _connectDN to new correct value
+            $auth = $ldap->bind(dn=>$self->{_connectDN}, password=>$identifier);
+            my $message = "DN has been changed for user ".$_[0]." from \"".$userData->{connectDN}."\" to \"".$self->{_connectDN}."\"";
+            $self->saveParams($self->user->userId, $self->authMethod, { connectDN => $self->{_connectDN} });
+            $self->session->errorHandler->warn($message);
+        }
         
         # Authentication failed
-        if ($auth->code == 48 || $auth->code == 49){
+        if ($auth->code == 48 || $auth->code == 49 || $auth->code == 32){
             $error .= $self->SUPER::authenticationError;
         }
         elsif ($auth->code > 0) { # Some other LDAP error happened
             $error .= '<li>LDAP error "'.$self->ldapStatusCode($auth->code).'" occured.'.$i18n->get(69).'</li>';
-            $self->session->errorHandler->error("LDAP error: ".$self->ldapStatusCode($auth->code));
+            $self->session->log->error("LDAP error: ".$self->ldapStatusCode($auth->code));
         }
 
         $ldap->unbind;
     }
     else { 
         $error .= '<li>'.$i18n->get(13,'AuthLDAP').'</li>';
-        $self->session->errorHandler->error("Could not process this LDAP URL: ".$userData->{ldapUrl});
+        $self->session->log->error("Could not process this LDAP URL: ".$userData->{ldapUrl});
     }
 
     if($error ne ""){
@@ -645,8 +658,8 @@ Process the login form. Create a new account if auto registration is enabled.
 sub login {
     my $self = shift;
     my $i18n = WebGUI::International->new($self->session);
-    my $username = $self->session->form->process("username");
+    my $username         = $self->session->form->process("username");
-    my $identifier = $self->session->form->process("identifier");
+    my $identifier       = $self->session->form->process("identifier");
     my $autoRegistration = $self->session->setting->get("automaticLDAPRegistration");
     my $hasAuthenticated = 0;
 
@@ -684,7 +697,7 @@ sub login {
    }
    return $self->SUPER::login() if $hasAuthenticated;  #Standard login routine for login
 
-   $self->session->errorHandler->security("login to account ".$self->session->form->process("username")." with invalid information.");
+   $self->session->log->security("login to account ".$self->session->form->process("username")." with invalid information.");
    return $self->displayLogin("<h1>".$i18n->get(70)."</h1>".$self->error);
 }
 

lib/WebGUI/LDAPLink.pm

@@ -49,8 +49,9 @@ These subroutines are available from this package:
 
 =head2 bind ( )
 
-Authenticates against the ldap server with the parameters stored in the class, returning a valid ldap connection, or 0 if a connection
+Authenticates against the ldap server with the parameters stored in the
-cannot be established
+class, returning a valid ldap connection, or 0 if a connection cannot
+be established
 
 =cut
 

t/Auth/LDAP.t

@@ -19,6 +19,7 @@ use lib "$FindBin::Bin/../lib";
 use Test::More;
 use WebGUI::Test; # Must use this before any other WebGUI modules
 use WebGUI::Session;
+use Test::Deep;
 use Scope::Guard;
 
 #----------------------------------------------------------------------------
@@ -36,7 +37,8 @@ my $ldapProps   = {
     ldapLinkId      => sprintf( '%022s', "testlink" ),
 };
 $session->db->setRow("ldapLink","ldapLinkId",$ldapProps, $ldapProps->{ldapLinkId});
-my $ldap            = WebGUI::LDAPLink->new( $session, $ldapProps->{ldapLinkId} );
+my $ldapLink        = WebGUI::LDAPLink->new( $session, $ldapProps->{ldapLinkId} );
+my $ldap            = $ldapLink->bind;
 $session->setting->set('ldapConnection', $ldapProps->{ldapLinkId} );
 
 # Cleanup
@@ -50,7 +52,7 @@ my @cleanup = (
 #----------------------------------------------------------------------------
 # Tests
 
-plan tests => 3;        # Increment this number for each test you create
+plan tests => 8;        # Increment this number for each test you create
 
 #----------------------------------------------------------------------------
 # Test Login of existing user
@@ -110,5 +112,76 @@ is( $session->user->get('username'), 'Bogs Diamond', 'Bogs was created' )
 or diag( $auth->error );
 WebGUI::Test->addToCleanup( $session->user );
 
-$session->user({ userId => 1 }); # Restore Visitor
+$session->setting->set('automaticLDAPRegistration', 0);
+$session->user({ userId => 1 }); # Restore Visitor
+
+#----------------------------------------------------------------------------
+# Test DN reset from LDAP
+
+$session->setting->set('automaticLDAPRegistration', 1);
+my $result = $ldap->add( 'cn=Brooks Hatley,ou=Convicts,o=shawshank',
+    attr    => [
+        cn              => 'Brooks Hatley',
+        givenName       => 'Brooks',
+        sn              => 'Hatley',
+        ou              => 'Convicts',
+        o               => 'shawshank',
+        objectClass     => [ qw( top inetOrgPerson ) ],
+        userPassword    => 'BrooksHatley',
+    ]
+);
+
+$session->request->setup_body({
+    username        => 'Brooks Hatley',
+    identifier      => 'BrooksHatley',
+});
+$auth   = WebGUI::Auth::LDAP->new( $session, 'LDAP' );
+$out    = $auth->login;
+is $session->user->get('username'), 'Brooks Hatley', 'Brooks was created';
+cmp_deeply(
+    $auth->getParams,
+    {
+        connectDN      => 'cn=Brooks Hatley,ou=Convicts,o=shawshank',
+        ldapConnection => '00000000000000testlink',
+        ldapUrl        => 'ldaps://smoke.plainblack.com/ou=Convicts,o=shawshank',
+    },
+    'authentication information set after creating account'
+);
+WebGUI::Test->addToCleanup( $session->user, );
+$out    = $auth->logout;
+is $session->user->get('username'), 'Visitor', 'Brooks was logged out';
+
+$ldap->moddn( 'cn=Brooks Hatley,ou=Convicts,o=shawshank',
+    newrdn => 'cn=Brooks Hatlen',
+);
+
+$ldap->modify( 'cn=Brooks Hatlen,ou=Convicts,o=shawshank',
+    replace    => {
+        sn              => 'Hatlen',
+        userPassword    => 'BrooksHatlen',
+    },
+);
+
+$session->request->setup_body({
+    username        => 'Brooks Hatley',
+    identifier      => 'BrooksHatlen',
+});
+
+$auth   = WebGUI::Auth::LDAP->new( $session, 'LDAP' );
+$out    = $auth->login;
+is $session->user->get('username'), 'Brooks Hatley', 'Brooks was logged in after name change';
+cmp_deeply(
+    $auth->getParams,
+    {
+        connectDN      => 'cn=Brooks Hatlen,ou=Convicts,o=shawshank',
+        ldapConnection => '00000000000000testlink',
+        ldapUrl        => 'ldaps://smoke.plainblack.com/ou=Convicts,o=shawshank',
+    },
+    'authentication information updated after name change'
+);
+
+
+$ldap->delete( 'cn=Brooks Hatlen,ou=Convicts,o=shawshank' );
+$ldap->delete( 'cn=Brooks Hatley,ou=Convicts,o=shawshank' );
+
 $session->setting->set('automaticLDAPRegistration', 0);