diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index 9e7c894ac..56035044f 100644 --- a/docs/changelog/7.x.x.txt +++ b/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. diff --git a/lib/WebGUI/Auth.pm b/lib/WebGUI/Auth.pm index 363771176..222a59083 100644 --- a/lib/WebGUI/Auth.pm +++ b/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 $userId = $_[0] || $self->userId; + my $self = shift; + 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)); } diff --git a/lib/WebGUI/Auth/LDAP.pm b/lib/WebGUI/Auth/LDAP.pm index c80d5a0f9..c60d2bf84 100644 --- a/lib/WebGUI/Auth/LDAP.pm +++ b/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('
  • '.$i18n->get(2,'AuthLDAP').'
  • '); return 0; } - my $username = $self->session->form->get("authLDAP_ldapId") || $self->session->form->get("username"); - my $password = $self->session->form->get("authLDAP_identifier") || $self->session->form->get("identifier"); + my $username = $form->get("authLDAP_ldapId") || $form->get("username"); + my $password = $form->get("authLDAP_identifier") || $form->get("identifier"); my $uri = URI->new($connection->{ldapUrl}) or $error = '
  • '.$i18n->get(2,'AuthLDAP').'
  • '; @@ -102,27 +104,27 @@ sub _isValidLDAPUser { # Invalid login credentials, directory did not authenticate the user if ($auth->code == 48 || $auth->code == 49) { $error .= '
  • '.$i18n->get(68).'
  • '; - $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 .= '
  • LDAP error "'.$self->ldapStatusCode($auth->code).'" occured. '.$i18n->get(69).'
  • '; - $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 .= '
  • '.$i18n->get(68).'
  • '; - $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 = '
  • '.$i18n->get(2,'AuthLDAP').'
  • '; - $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 = '
  • '.$i18n->get(2,'AuthLDAP').'
  • '; - $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 .= '
  • LDAP error "'.$self->ldapStatusCode($auth->code).'" occured.'.$i18n->get(69).'
  • '; - $self->session->errorHandler->error("LDAP error: ".$self->ldapStatusCode($auth->code)); + $self->session->log->error("LDAP error: ".$self->ldapStatusCode($auth->code)); } $ldap->unbind; } else { $error .= '
  • '.$i18n->get(13,'AuthLDAP').'
  • '; - $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 $identifier = $self->session->form->process("identifier"); + my $username = $self->session->form->process("username"); + 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("

    ".$i18n->get(70)."

    ".$self->error); } diff --git a/lib/WebGUI/LDAPLink.pm b/lib/WebGUI/LDAPLink.pm index 49a97f2a9..2d6008f04 100644 --- a/lib/WebGUI/LDAPLink.pm +++ b/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 -cannot be established +Authenticates against the ldap server with the parameters stored in the +class, returning a valid ldap connection, or 0 if a connection cannot +be established =cut diff --git a/t/Auth/LDAP.t b/t/Auth/LDAP.t index 35971413c..24def1b75 100644 --- a/t/Auth/LDAP.t +++ b/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);