diff --git a/application/controllers/AuthenticationController.php b/application/controllers/AuthenticationController.php index da687e36c..bf6f0db39 100644 --- a/application/controllers/AuthenticationController.php +++ b/application/controllers/AuthenticationController.php @@ -1,5 +1,4 @@ view->form = new LoginForm(); $this->view->form->setRequest($this->_request); $this->view->title = 'Icinga Web Login'; + try { $redirectUrl = Url::fromPath($this->_request->getParam('redirect', 'dashboard')); + if ($this->_request->isXmlHttpRequest()) { $redirectUrl->setParam('_render', 'layout'); } + $auth = AuthManager::getInstance(); if ($auth->isAuthenticated()) { $this->redirectNow($redirectUrl); } + if ($this->view->form->isSubmittedAndValid()) { - $user = new User( - $this->view->form->getValue('username') - ); + $user = new User($this->view->form->getValue('username')); + try { $config = Config::app('authentication'); } catch (NotReadableError $e) { @@ -85,37 +87,33 @@ class AuthenticationController extends ActionController . ' up. Please contact your Icinga Web administrator' ); } - $backendsWithError = 0; + // TODO(el): Currently the user is only notified about authentication backend problems when all backends // have errors. It may be the case that the authentication backend which provides the user has errors // but other authentication backends work. In that scenario the user is presented an error message // saying "Incorrect username or password". We must inform the user that not all authentication methods // are available. $backendsTried = 0; + $backendsWithError = 0; $chain = new AuthChain($config); foreach ($chain as $backend) { - ++$backendsTried; - try { - if ($backend->authenticate($user, $this->view->form->getValue('password'))) { - $auth->setAuthenticated($user); - $this->redirectNow($redirectUrl); - } - } catch (Exception $e) { - Logger::error( - new Exception( - 'Cannot authenticate against backend "' . $backend->getName() . '".' - . ' An exception was thrown:', 0, $e - ) - ); - ++$backendsWithError; + $authenticated = $backend->authenticate($user, $this->view->form->getValue('password')); + if ($authenticated === true) { + $auth->setAuthenticated($user); + $this->redirectNow($redirectUrl); + } elseif ($authenticated === null) { + $backendsWithError += 1; } + $backendsTried += 1; } + if ($backendsWithError === $backendsTried) { throw new ConfigurationError( 'No authentication methods available. It seems that all set up authentication methods have' . ' errors. Please contact your Icinga Web administrator' ); } + $this->view->form->getElement('password')->addError(t('Incorrect username or password')); } } catch (Exception $e) { @@ -140,4 +138,3 @@ class AuthenticationController extends ActionController } } } -// @codingStandardsIgnoreEnd diff --git a/library/Icinga/Authentication/Backend/DbUserBackend.php b/library/Icinga/Authentication/Backend/DbUserBackend.php index 409fa4d06..03464adc7 100644 --- a/library/Icinga/Authentication/Backend/DbUserBackend.php +++ b/library/Icinga/Authentication/Backend/DbUserBackend.php @@ -29,12 +29,13 @@ namespace Icinga\Authentication\Backend; -use Exception; -use Zend_Db_Expr; +use \Exception; +use \Zend_Db_Expr; +use \Zend_Db_Select; use Icinga\Authentication\UserBackend; use Icinga\Data\Db\Connection; +use Icinga\Logger\Logger; use Icinga\User; -use \Zend_Db_Select; class DbUserBackend extends UserBackend { @@ -68,32 +69,42 @@ class DbUserBackend extends UserBackend } /** - * Authenticate + * Authenticate the given user and return true on success, false on failure and null on error * * @param User $user * @param string $password * - * @return bool - * @throws \Exception If we can not fetch the salt + * @return bool|null */ public function authenticate(User $user, $password) { - $salt = $this->getSalt($user->getUsername()); - if ($salt === null) { - return false; - } - if ($salt === '') { - throw new Exception(); - } + try { + $salt = $this->getSalt($user->getUsername()); + if ($salt === null) { + return false; + } + if ($salt === '') { + throw new Exception('Cannot find salt for user ' . $user->getUsername()); + } - $select = new Zend_Db_Select($this->conn->getConnection()); - $row = $select->from('account', array(new Zend_Db_Expr(1))) - ->where('username = ?', $user->getUsername()) - ->where('active = ?', true) - ->where('password = ?', $this->hashPassword($password, $salt)) - ->query()->fetchObject(); + $select = new Zend_Db_Select($this->conn->getConnection()); + $row = $select->from('account', array(new Zend_Db_Expr(1))) + ->where('username = ?', $user->getUsername()) + ->where('active = ?', true) + ->where('password = ?', $this->hashPassword($password, $salt)) + ->query()->fetchObject(); - return ($row !== false) ? true : false; + return ($row !== false) ? true : false; + } catch (Exception $e) { + Logger::error( + sprintf( + 'Failed to authenticate user "%s" with backend "%s". Exception occured: %s', + $user->getUsername(), + $this->getName(), + $e->getMessage() + ) + ); + } } /** diff --git a/library/Icinga/Authentication/Backend/LdapUserBackend.php b/library/Icinga/Authentication/Backend/LdapUserBackend.php index 2e80302f1..f0922e092 100644 --- a/library/Icinga/Authentication/Backend/LdapUserBackend.php +++ b/library/Icinga/Authentication/Backend/LdapUserBackend.php @@ -29,7 +29,9 @@ namespace Icinga\Authentication\Backend; +use \Exception; use Icinga\User; +use Icinga\Logger\Logger; use Icinga\Authentication\UserBackend; use Icinga\Protocol\Ldap\Connection; @@ -87,23 +89,30 @@ class LdapUserBackend extends UserBackend } /** - * Authenticate + * Authenticate the given user and return true on success, false on failure and null on error * * @param User $user * @param string $password * - * @return bool + * @return bool|null */ public function authenticate(User $user, $password) { - if ($this->conn->testCredentials( + try { + return $this->conn->testCredentials( $this->conn->fetchDN($this->createQuery($user->getUsername())), $password - ) - ) { - return true; + ); + } catch (Exception $e) { + Logger::error( + sprintf( + 'Failed to authenticate user "%s" with backend "%s". Exception occured: %s', + $user->getUsername(), + $this->getName(), + $e->getMessage() + ) + ); } - return false; } /**