Merge branch 'bugfix/auth-fix-4340'

fixes #4340

application/controllers/AuthenticationController.php

@@ -6,8 +6,9 @@
 # namespace Icinga\Application\Controllers;
 
 use Icinga\Web\ActionController;
-use Icinga\Authentication\Auth;
+use Icinga\Authentication\Credentials as Credentials;
-use Icinga\Web\Notification;
+use Icinga\Authentication\Manager as AuthManager;
+use Icinga\Form\Builder as FormBuilder;
 
 /**
  * Class AuthenticationController
@@ -25,13 +26,67 @@ class AuthenticationController extends ActionController
      */
     protected $modifiesSession = true;
 
+    private function getAuthForm()
+    {
+        return array(
+            'username' => array(
+                'text',
+                array(
+                    'label' => t('Username'),
+                    'required' => true,
+                )
+            ),
+            'password' => array(
+                'password',
+                array(
+                    'label' => t('Password'),
+                    'required' => true
+                )
+            ),
+            'submit' => array(
+                'submit',
+                array(
+                    'label' => t('Login'),
+                    'class' => 'pull-right'
+                )
+            )
+        );
+    }
+
     /**
      *
      */
     public function loginAction()
     {
         $this->replaceLayout = true;
-        $this->view->form = $this->widget('form', array('name' => 'login'));
+        $credentials = new Credentials();
+        $this->view->form = FormBuilder::fromArray(
+            $this->getAuthForm(),
+            array(
+                "CSRFProtection" => false, // makes no sense here
+                "model" => &$credentials
+            )
+        );
+        try {
+            $auth = AuthManager::getInstance(null, array(
+                "writeSession" => true 
+            ));
+            if ($auth->isAuthenticated()) {
+                $this->redirectNow('index?_render=body');
+            }
+            if ($this->getRequest()->isPost() && $this->view->form->isSubmitted()) {
+                $this->view->form->repopulate();
+                if ($this->view->form->isValid()) {
+                    if (!$auth->authenticate($credentials)) {
+                        $this->view->form->getElement('password')->addError(t('Please provide a valid username and password'));
+                    } else {
+                        $this->redirectNow('index?_render=body');
+                    }
+                }
+            }
+        } catch (\Icinga\Exception\ConfigurationError $configError) {
+            $this->view->errorInfo = $configError->getMessage();
+        }
     }
 
     /**
@@ -39,9 +94,11 @@ class AuthenticationController extends ActionController
      */
     public function logoutAction()
     {
+        $auth = AuthManager::getInstance(null, array(
+            "writeSession" => true 
+        ));
         $this->replaceLayout = true;
-        Auth::getInstance()->forgetAuthentication();
+        $auth->removeAuthorization();
-        Notification::success('You have been logged out');
         $this->_forward('login');
     }
 }

application/layouts/scripts/parts/topbar.phtml

@@ -12,7 +12,7 @@
     </li>
    <li class="dropdown">
     <a href="#" class="dropdown-toggle" data-toggle="dropdown"><?php echo
-      $this->escape($this->auth()->getUsername())
+      $this->escape($this->auth()->getUser()->getUsername())
        ?> <i class="icon-user icon-white" style="margin-top:0.2em"></i>
     <b class="caret"></b>
     </a>

application/views/scripts/authentication/login.phtml

@@ -1 +1,10 @@
-<?php echo $this->form ?>
+<div class="well" style="margin:auto;width: 40%;max-width:25em;margin-top:10%;">
+<h1> Login </h1>
+<?php echo $this->form->render() ?>
+<?php if (isset($this->errorInfo)): ?>
+    <div class="alert alert-error">
+        <?= $this->errorInfo ?> 
+    </div>
+<?php endif ?>
+
+</div>

config/authentication.ini

@@ -1,2 +1,9 @@
 [users]
+backend=ldap
+hostname=localhost
+root_dn="ou=people,dc=icinga,dc=org"
+bind_dn="cn=admin,cn=config"
+bind_pw=admin
+user_class=inetOrgPerson
+user_name_attribute=uid
  

library/Icinga/Application/Web.php

@@ -38,7 +38,6 @@ class Web extends ApplicationBootstrap
         return $this->loadConfig()
             ->configureErrorHandling()
             ->setTimezone()
-            ->configureSession()
             ->configureCache()
             ->prepareZendMvc()
             ->loadTranslations()
@@ -80,16 +79,6 @@ class Web extends ApplicationBootstrap
         $this->dispatchFrontController();
     }
 
-    /**
-     * Configure web session settings
-     *
-     * @return self
-     */
-    protected function configureSession()
-    {
-        Manager::getInstance();
-        return $this;
-    }
 
     protected function loadTranslations()
     {

library/Icinga/Authentication/Backend.php

@@ -6,6 +6,9 @@ namespace Icinga\Authentication;
 
 class Backend
 {
+    /**
+     * @var UserBackend
+     */
     protected $userBackend;
 
     public function __construct($config)

library/Icinga/Authentication/Backend/LdapUserBackend.php

@@ -5,6 +5,8 @@
 namespace Icinga\Authentication\Backend;
 
 use Icinga\Authentication\User as User;
+use Icinga\Authentication\UserBackend;
+use Icinga\Authentication\Credentials;
 use Icinga\Protocol\Ldap;
 
 class LdapUserBackend implements UserBackend
@@ -16,14 +18,11 @@ class LdapUserBackend implements UserBackend
         $this->connection = new Ldap\Connection($config);
     }
 
-    public function hasUsername($username)
+    public function hasUsername(Credentials $credential)
     {
-        if (!$username) {
-            return false;
-        }
         return $this->connection->fetchOne(
-            $this->selectUsername($username)
+            $this->selectUsername($credential->getUsername())
-        ) === $username;
+        ) === $credential->getUsername();
     }
 
     protected function stripAsterisks($string)
@@ -38,19 +37,15 @@ class LdapUserBackend implements UserBackend
             ->where('sAMAccountName', $this->stripAsterisks($username));
     }
 
-    public function authenticate($username, $password = null)
+    public function authenticate(Credentials $credentials)
     {
-        if (empty($username) || empty($password)) {
-            return false;
-        }
-
         if (!$this->connection->testCredentials(
-            $this->connection->fetchDN($this->selectUsername($username)),
+            $this->connection->fetchDN($this->selectUsername($credentials->getUsername())),
-            $password
+            $credentials->getPassword()
         )     ) {
             return false;
         }
-        $user = new User($username);
+        $user = new User($credentials->getUsername());
 
         return $user;
     }

library/Icinga/Authentication/Manager.php

@@ -6,6 +6,7 @@ namespace Icinga\Authentication;
 
 use Icinga\Application\Logger as Logger;
 use Icinga\Application\Config as Config;
+use Icinga\Exception\ConfigurationError as ConfigError;
 
 class Manager
 {
@@ -25,7 +26,6 @@ class Manager
         if ($config === null) {
             $config = Config::getInstance()->authentication;
         }
-
         if (isset($options["userBackendClass"])) {
             $this->userBackend = $options["userBackendClass"];
         } elseif ($config->users !== null) {
@@ -77,6 +77,15 @@ class Manager
 
     public function authenticate(Credentials $credentials, $persist = true)
     {
+        if (!$this->userBackend) {
+            Logger::error("No authentication backend provided, your users will never be able to login.");
+            throw new ConfigError(
+                "No authentication backend set - login will never succeed as icinga-web ".
+                "doesn't know how to determine your user. \n".
+                "To fix this error, setup your authentication.ini with a valid authentication backend."
+            );
+            return false;
+        }
         if (!$this->userBackend->hasUsername($credentials)) {
             Logger::info("Unknown user %s tried to log in", $credentials->getUsername());
             return false;
@@ -115,7 +124,7 @@ class Manager
     public function removeAuthorization()
     {
         $this->user = null;
-        $this->session->delete();
+        $this->session->purge();
     }
 
     public function getUser()

library/Icinga/Authentication/PhpSession.php

@@ -28,7 +28,6 @@ class PhpSession extends Session
     private static $DEFAULT_COOKIEOPTIONS = array(
         'use_trans_sid'           => false,
         'use_cookies'             => true,
-        'use_only_cooies'         => true,
         'cookie_httponly'         => true,
         'use_only_cookies'        => true,
         'hash_function'           => true,
@@ -51,6 +50,9 @@ class PhpSession extends Session
                 );
             }
         }
+        if (!is_writable(session_save_path())) {
+            throw new \Icinga\Exception\ConfigurationError("Can't save session");
+        }
     }
 
     private function sessionCanBeChanged()
@@ -78,12 +80,7 @@ class PhpSession extends Session
         }
 
         session_name(PhpSession::SESSION_NAME);
-
+        session_start();
-        /*
-         * @todo This is not right
-         */
-        \Zend_Session::start();
-        // session_start();
         $this->isOpen = true;
         $this->setAll($_SESSION);
         return true;
@@ -138,16 +135,18 @@ class PhpSession extends Session
 
     public function purge()
     {
-        if ($this->ensureOpen() && !$this->isFlushed) {
+        if ($this->ensureOpen()) {
             $_SESSION = array();
             session_destroy();
             $this->clearCookies();
+            $this->close();
         }
     }
 
     private function clearCookies()
     {
         if (ini_get("session.use_cookies")) {
+            Logger::debug("Clearing cookies");
             $params = session_get_cookie_params();
             setcookie(
                 session_name(),

library/Icinga/Authentication/User.php

@@ -20,15 +20,15 @@ namespace Icinga\Authentication;
  */
 class User
 {
-    private $username = "";
+    public $username = "";
-    private $firstname = "";
+    public $firstname = "";
-    private $lastname = "";
+    public $lastname = "";
-    private $email = "";
+    public $email = "";
-    private $domain = "";
+    public $domain = "";
-    private $additionalInformation = array();
+    public $additionalInformation = array();
 
-    private $permissions = array();
+    public $permissions = array();
-    private $groups = array();
+    public $groups = array();
 
     public function __construct($username, $firstname = null, $lastname = null, $email = null)
     {

library/Icinga/Authentication/UserBackend.php

@@ -1,15 +1,28 @@
 <?php
 // {{{ICINGA_LICENSE_HEADER}}}
-// {{{ICINGA_LICENSE_HEADER}}// {{{ICINGA_LICENSE_HEADER}}}
 // {{{ICINGA_LICENSE_HEADER}}}}
 
 namespace Icinga\Authentication;
 
 interface UserBackend
 {
+    /**
+     * Creates a new object
+     * @param $config
+     */
     public function __construct($config);
 
+    /**
+     * Test if the username exists
+     * @param Credentials $credentials
+     * @return boolean
+     */
     public function hasUsername(Credentials $credentials);
 
+    /**
+     * Authenticate
+     * @param Credentials $credentials
+     * @return User
+     */
     public function authenticate(Credentials $credentials);
 }

library/Icinga/Protocol/Ldap/Connection.php

@@ -4,6 +4,7 @@
 
 namespace Icinga\Protocol\Ldap;
 
+use Icinga\Exception\ConfigurationError as ConfigError;
 use Icinga\Application\Platform;
 use Icinga\Application\Config;
 use Icinga\Application\Logger as Log;
@@ -69,6 +70,10 @@ class Connection
         // '1.3.6.1.1.8' => '8', // Cancel Extended Request
     );
 
+    protected $use_tls = false;
+    protected $force_tls = false;
+
+
     /**
      * @var array
      */
@@ -116,7 +121,8 @@ class Connection
         $this->bind_dn = $config->bind_dn;
         $this->bind_pw = $config->bind_pw;
         $this->root_dn = $config->root_dn;
-
+        $this->use_tls = isset($config->tls) ? $config->tls : false;
+        $this->force_tls = $this->use_tls;
     }
 
     /**
@@ -266,6 +272,7 @@ class Connection
         // We do not support pagination right now, and there is no chance to
         // do so for PHP < 5.4. Warnings about "Sizelimit exceeded" will
         // therefore not be hidden right now.
+        Log::debug("Query: %s", $query->__toString());
         $results = ldap_search(
             $this->ds,
             $this->root_dn,
@@ -405,7 +412,6 @@ class Connection
         if (isset($result->supportedExtension)) {
             foreach ($result->supportedExtension as $oid) {
                 if (array_key_exists($oid, $this->ldap_extension)) {
-                    echo $this->ldap_extension[$oid] . "\n";
                     // STARTTLS -> läuft mit OpenLDAP
                 }
             }
@@ -429,17 +435,16 @@ class Connection
         if ($this->ds !== null) {
             return;
         }
-        $use_tls = true;
-        $force_tls = true;
 
-        if ($use_tls) {
+        if ($this->use_tls) {
             $this->prepareTlsEnvironment();
         }
         Log::debug("Trying to connect to %s", $this->hostname);
         $this->ds = ldap_connect($this->hostname, 389);
         $this->discoverCapabilities();
+        if ($this->use_tls) {
             Log::debug("Trying ldap_start_tls()");
-        if (ldap_start_tls($this->ds)) {
+            if (@ldap_start_tls($this->ds)) {
                 Log::debug("Trying ldap_start_tls() succeeded");
             } else {
                 Log::warn(
@@ -447,6 +452,7 @@ class Connection
                     ldap_error($this->ds)
                 );
             }
+        }
 
 
         // ldap_rename requires LDAPv3:
@@ -470,12 +476,10 @@ class Connection
                 '***',
                 ldap_error($this->ds)
             );
-            throw new Exception(
+            throw new ConfigError(
                 sprintf(
-                    'LDAP connection (%s / %s) failed: %s',
+                    'Could not connect to the authentication server, please '.
-                    $this->bind_dn,
+                    'review your LDAP connection settings.'
-                    '***' /* $this->bind_pw */,
-                    ldap_error($this->ds)
                 )
             );
         }

library/Icinga/Web/ActionController.php

@@ -110,10 +110,6 @@ class ActionController extends ZfController
          * @todo remove this!
          */
 
-        $this->allowAccess = true;
-        $this->init();
-
-        return null;
 
         if ($this->handlesAuthentication() ||
                 Manager::getInstance(
@@ -338,48 +334,4 @@ class ActionController extends ZfController
         }*/
 
     }
-
-    /**
-     * Whether the token parameter is valid
-     *
-     * TODO: Could this make use of Icinga\Web\Session once done?
-     *
-     * @param int $maxAge    Max allowed token age
-     * @param string $sessionId A specific session id (useful for tests?)
-     * @return bool
-     */
-    public function hasValidToken($maxAge = 600, $sessionId = null)
-    {
-        $sessionId = $sessionId ? $sessionId : session_id();
-        $seed = $this->_getParam('seed');
-        if (!is_numeric($seed)) {
-            return false;
-        }
-
-        // Remove quantitized timestamp portion so maxAge applies
-        $seed -= (intval(time() / $maxAge) * $maxAge);
-        $token = $this->_getParam('token');
-        return $token === hash('sha256', $sessionId . $seed);
-    }
-
-    /**
-     * Get a new seed/token pair
-     *
-     * TODO: Could this make use of Icinga\Web\Session once done?
-     *
-     * @param int $maxAge    Max allowed token age
-     * @param string $sessionId A specific session id (useful for tests?)
-     *
-     * @return array
-     */
-    public function getSeedTokenPair($maxAge = 600, $sessionId = null)
-    {
-        $sessionId = $sessionId ? $sessionId : session_id();
-        $seed = mt_rand();
-        $hash = hash('sha256', $sessionId . $seed);
-
-        // Add quantitized timestamp portion to apply maxAge
-        $seed += (intval(time() / $maxAge) * $maxAge);
-        return array($seed, $hash);
-    }
 }

library/Icinga/Web/Notification.php

@@ -7,7 +7,6 @@ namespace Icinga\Web;
 use Icinga\Exception\ProgrammingError;
 use Icinga\Application\Platform;
 use Icinga\Application\Logger as Log;
-use Zend_Session_Namespace as SessionNamespace;
 
 /**
  * Class Notification
@@ -149,10 +148,10 @@ class Notification
      */
     final private function __construct()
     {
-        $this->session = new SessionNamespace('IcingaNotification');
+        //$this->session = new SessionNamespace('IcingaNotification');
-        if (!is_array($this->session->messages)) {
+        //if (!is_array($this->session->messages)) {
             $this->session->messages = array();
-        }
+        //}
 
         if (Platform::isCli()) {
             $this->cliFlag = true;

test/php/application/views/helpers/QlinkTest.php

@@ -4,7 +4,7 @@
 require_once('Zend/View/Helper/Abstract.php');
 require_once('Zend/View.php');
 
-require('../../application/views/helpers/Qlink.php');
+require_once('../../application/views/helpers/Qlink.php');
 
 
 /**

test/php/library/Icinga/Authentication/PhpSessionTest.php

@@ -7,6 +7,7 @@ namespace Tests\Icinga\Authentication;
 require_once("../../library/Icinga/Authentication/Session.php");
 require_once("../../library/Icinga/Authentication/PhpSession.php");
 require_once("../../library/Icinga/Application/Logger.php");
+require_once("../../library/Icinga/Exception/ConfigurationError.php");
 require_once("Zend/Log.php");
 
 use Icinga\Authentication\PhpSession as PhpSession;

test/php/library/Icinga/Web/ActionControllerTest.php

@@ -1,80 +1,6 @@
 <?php
 
 namespace Tests\Icinga\Web\ActionController;
-use Icinga\Web\ActionController as Action;
 
-require_once('Zend/Controller/Action.php');
-require_once('../../library/Icinga/Web/ActionController.php');
 
-/**
- * This is not a nice hack, but doesn't affect the behaviour of
- * the tested methods, allowing us to avoid bootstrapping
- * the request/response System for every test
- *
- * Class ActionTestWrap
- * @package Tests\Icinga\Mvc\Controller
- */
-class ActionTestWrap extends Action {
-    private $args;
-    public function __construct(\Zend_Controller_Request_Abstract $request = null,
-                                \Zend_Controller_Response_Abstract $response = null, array $invokeArgs = array())
-    {}
 
-    public function setArguments($args) {
-        $this->args = $args;
-    }
-
-    protected function _getParam($paramName, $default = null) {
-        if(isset($this->args[$paramName]))
-            return $this->args[$paramName];
-        return $default;
-    }
-}
-
-class ActionTest extends \PHPUnit_Framework_TestCase
-{
-    public function testSeedGeneration()
-    {
-        $action = new ActionTestWrap();
-        list($seed1,$token1) = $action->getSeedTokenPair(600,"test");
-        list($seed2,$token2) = $action->getSeedTokenPair(600,"test");
-        list($seed3,$token3) = $action->getSeedTokenPair(600,"test");
-        $this->assertTrue($seed1 != $seed2 && $seed2 != $seed3 && $seed1 != $seed3);
-        $this->assertTrue($token1 != $token2 && $token2 != $token3 && $token1 != $token3);
-    }
-
-    public function testSeedValidation()
-    {
-        $action = new ActionTestWrap();
-        list($seed,$token) = $action->getSeedTokenPair(600,"test");
-        $action->setArguments(array(
-            "seed" => $seed,
-            "token" => $token
-        ));
-        $this->assertTrue($action->hasValidToken(600,"test"));
-        $this->assertFalse($action->hasValidToken(600,"test2"));
-        $action->setArguments(array(
-            "seed" => $seed."ds",
-            "token" => $token
-        ));
-        $this->assertFalse($action->hasValidToken(600,"test"));
-        $action->setArguments(array(
-            "seed" => $seed,
-            "token" => $token."afs"
-        ));
-        $this->assertFalse($action->hasValidToken(600,"test"));
-    }
-
-    public function testMaxAge()
-    {
-        $action = new ActionTestWrap();
-        list($seed,$token) = $action->getSeedTokenPair(1,"test");
-        $action->setArguments(array(
-            "seed" => $seed,
-            "token" => $token
-        ));
-        $this->assertTrue($action->hasValidToken(1,"test"));
-        sleep(1);
-        $this->assertFalse($action->hasValidToken(1,"test"));
-    }
-}