From db61cfafe126189046c561f4115e7c3e8b81fbc8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jannis=20Mo=C3=9Fhammer?= <jannis.mosshammer@netways.de>
Date: Mon, 10 Jun 2013 13:28:54 +0200
Subject: [PATCH] Update and test Auth/Manager implementation

- remove Storable inheritance from User and make it a plain DAO
- remove Authorization methods from User

refs #4265
refs #4250
---
 library/Icinga/Authentication/Credentials.php |  49 +++++++
 library/Icinga/Authentication/Manager.php     |  37 ++++--
 library/Icinga/Authentication/PHPSession.php  |   6 +-
 library/Icinga/Authentication/User.php        | 107 +++++++++++----
 library/Icinga/Authentication/UserBackend.php |  24 +---
 .../Icinga/Authentication/BackendMock.php     |  59 +++++++++
 .../Icinga/Authentication/ManagerTest.php     | 122 ++++++++++++++++++
 .../Icinga/Authentication/SessionMock.php     |  49 +++++++
 8 files changed, 395 insertions(+), 58 deletions(-)
 create mode 100644 library/Icinga/Authentication/Credentials.php
 create mode 100644 test/php/library/Icinga/Authentication/BackendMock.php
 create mode 100644 test/php/library/Icinga/Authentication/ManagerTest.php
 create mode 100644 test/php/library/Icinga/Authentication/SessionMock.php

diff --git a/library/Icinga/Authentication/Credentials.php b/library/Icinga/Authentication/Credentials.php
new file mode 100644
index 000000000..8bba08d87
--- /dev/null
+++ b/library/Icinga/Authentication/Credentials.php
@@ -0,0 +1,49 @@
+<?php
+
+namespace Icinga\Authentication;
+
+
+class Credentials
+{
+    protected $username;
+    protected $password;
+    protected $domain;
+
+
+    public function __construct($username = "", $password = null, $domain = null)
+    {
+        $this->username = $username;
+        $this->password = $password;
+        $this->domain = $domain;
+    }
+
+    public function getUsername()
+    {
+        return $this->username;
+    }
+
+    public function setUsername($username)
+    {
+        return $this->username = $username;
+    }
+
+    public function getPassword()
+    {
+        return $this->password;
+    }
+
+    public function setPassword($password)
+    {
+        return $this->password = $password;
+    }
+
+    public function getDomain()
+    {
+        return $this->domain;
+    }
+
+    public function setDomain($domain)
+    {
+        return $this->domain = $domain;
+    }
+}
diff --git a/library/Icinga/Authentication/Manager.php b/library/Icinga/Authentication/Manager.php
index 98f4520e1..309cdaf6a 100644
--- a/library/Icinga/Authentication/Manager.php
+++ b/library/Icinga/Authentication/Manager.php
@@ -9,7 +9,9 @@ class Manager
 {
     const BACKEND_TYPE_USER = "User";
     const BACKEND_TYPE_GROUP = "Group";
-    
+
+    private static $instance = null;
+
     private $user = null;
     private $groups = array();
     private $userBackend = null;
@@ -29,7 +31,7 @@ class Manager
         }
 
         if (isset($options["groupBackendClass"])) {
-            $this->userBackend = $options["groupBackendClass"];
+            $this->groupBackend = $options["groupBackendClass"];
         } elseif ($config->groups != null) {
             $this->groupBackend = initBackend(BACKEND_TYPE_GROUP, $config->groups);
         }
@@ -37,18 +39,28 @@ class Manager
         if (!isset($options["sessionClass"])) {
             $this->session = new PhpSession($config->session);
         } else {
-            $options["sessionClass"];
+            $this->session = $options["sessionClass"];
+        }
+        if (isset($options["writeSession"]) && $options["writeSession"] === true) {
+            $this->session->read(true);
+        } else {
+            $this->session->read();
         }
     }
 
     public static function getInstance($config = null, array $options = array())
     {
         if (self::$instance === null) {
-            self::$instance = new Auth($config, $options);
+            self::$instance = new Manager($config, $options);
         }
         return self::$instance;
     }
 
+    public static function clearInstance()
+    {
+        self::$instance = null;
+    }
+
     private function initBackend($authenticationTarget, $authenticationSource)
     {
         $userbackend = ucwords(strtolower($authenticationSource->backend));
@@ -56,7 +68,7 @@ class Manager
         return new $class($authenticationSource);
     }
 
-    public function authenticate(Credentials $credentials)
+    public function authenticate(Credentials $credentials, $persist = true)
     {
         if (!$this->userBackend->hasUsername($credentials)) {
             Logger::info("Unknown user %s tried to log in", $credentials->getUsername());
@@ -68,26 +80,29 @@ class Manager
             return false;
         }
         
-        persistCurrentUser();
+        if ($persist == true) {
+            $this->persistCurrentUser();
+            $this->session->write();
+        }
         return true;
     }
 
     public function persistCurrentUser()
     {
-        $this->session->set("user", $this->user->toSession());
+        $this->session->set("user", $this->user);
     }
 
     public function authenticateFromSession()
     {
-        $this->user = User::fromSession($this->session->get("user", null));
+        $this->user = $this->session->get("user", null);
     }
 
-    public function isAuthenticated()
+    public function isAuthenticated($ignoreSession = false)
     {
-        if ($this->user === null) {
+        if ($this->user === null && !$ignoreSession) {
             $this->authenticateFromSession();
         }
-        return is_object($this->user) && !empty($this->user->username);
+        return is_object($this->user);
     }
 
     public function removeAuthorization()
diff --git a/library/Icinga/Authentication/PHPSession.php b/library/Icinga/Authentication/PHPSession.php
index 230971092..7c4821536 100644
--- a/library/Icinga/Authentication/PHPSession.php
+++ b/library/Icinga/Authentication/PHPSession.php
@@ -28,7 +28,11 @@ class PhpSession extends Session
         }
         foreach ($options as $sessionVar => $value) {
             if (ini_set("session.".$sessionVar, $value) === false) {
-                Logger::warn("Could not set php.ini settint %s = %s", $sessionVar, $value);
+                Logger::warn(
+                    "Could not set php.ini setting %s = %s. This might affect your sessions behaviour.",
+                    $sessionVar,
+                    $value
+                );
             }
         }
     }
diff --git a/library/Icinga/Authentication/User.php b/library/Icinga/Authentication/User.php
index ccabfcb3e..a739394a5 100644
--- a/library/Icinga/Authentication/User.php
+++ b/library/Icinga/Authentication/User.php
@@ -10,61 +10,116 @@ namespace Icinga\Authentication;
 /**
  * This class represents a user object
  *
- * TODO: Show some use cases
  *
  * @copyright  Copyright (c) 2013 Icinga-Web Team <info@icinga.org>
  * @author     Icinga-Web Team <info@icinga.org>
  * @package    Icinga\Application
  * @license    http://www.gnu.org/copyleft/gpl.html GNU General Public License
  */
-class User extends Storable
+class User
 {
-    protected $defaultProps = array(
-        'username'   => null,
-        'password'   => null,
-        'first_name' => null,
-        'last_name'  => null,
-        'email'      => null,
-    );
-    protected $permissions = array();
-    protected $backend;
-    protected $groups;
-    protected $key = 'username';
+    private $username = "";
+    private $firstname = "";
+    private $lastname = "";
+    private $email = "";
+    private $domain = "";
+    private $additionalInformation = array();
 
-    public function listGroups()
+    private $permissions = array();
+    private $groups = array();
+
+    public function __construct($username, $firstname, $lastname, $email)
     {
-        if ($this->groups === null) {
-            $this->loadGroups();
-        }
+        $this->setUsername($username);
+        $this->setFirstname($firstname);
+        $this->setLastname($lastname);
+        $this->setEmail($email);
     }
 
-    protected function loadGroups()
+    public function getGroups()
     {
-        // Whatever
+        return $this->groups;
+    }
+
+    public function setGroups(array $groups)
+    {
+        $this->groups = $groups;
     }
 
     public function isMemberOf(Group $group)
     {
-        
+        return in_array($group, $this->groups);
     }
 
-    public function getPermissionList()
+    public function getPermissions()
     {
         return $this->permissions;
     }
 
-    public function hasPermission($uri, $permission)
+    public function getUsername()
     {
-
+        return $this->username;
     }
 
-    public function grantPermission($uri, $permission)
+    public function setUsername($name)
     {
-
+        $this->username = $name;
     }
 
-    public function revokePermission($uri, $permission)
+    public function getFirstname()
     {
+        return $this->firstname;
+    }
 
+    public function setFirstname($name)
+    {
+        $this->firstname = $name;
+    }
+
+    public function getLastname()
+    {
+        return $this->lastname;
+    }
+
+    public function setLastname($name)
+    {
+        $this->lastname = $name;
+    }
+
+    public function getEmail()
+    {
+        return $this->email;
+    }
+
+    public function setEmail($mail)
+    {
+        if (filter_var($mail, FILTER_VALIDATE_EMAIL)) {
+            $this->mail = $mail;
+        } else {
+            throw new InvalidArgumentException("Invalid mail given for user $this->username: $mail");
+        }
+    }
+
+    public function setDomain($domain)
+    {
+        $this->domain = $domain;
+    }
+
+    public function getDomain()
+    {
+        return $this->domain;
+    }
+
+    public function setAdditional($key, $value)
+    {
+        $this->additionalInformation[$key] = $value;
+    }
+
+    public function getAdditional($key)
+    {
+        if (isset($this->additionalInformation[$key])) {
+            return $this->additionalInformation[$key];
+        }
+        return null;
     }
 }
diff --git a/library/Icinga/Authentication/UserBackend.php b/library/Icinga/Authentication/UserBackend.php
index 259310ce1..fa9792fc0 100644
--- a/library/Icinga/Authentication/UserBackend.php
+++ b/library/Icinga/Authentication/UserBackend.php
@@ -2,27 +2,11 @@
 
 namespace Icinga\Authentication;
 
-class UserBackend
+interface UserBackend
 {
-    protected $config;
+    public function __construct($config);
 
-    public function __construct($config)
-    {
-        $this->config = $config;
-        $this->init();
-    }
+    public function hasUsername(Credentials $credentials);
 
-    protected function init()
-    {
-    }
-
-    public function hasUsername($username)
-    {
-        return false;
-    }
-
-    public function authenticate($username, $password = null)
-    {
-        return false;
-    }
+    public function authenticate(Credentials $credentials);
 }
diff --git a/test/php/library/Icinga/Authentication/BackendMock.php b/test/php/library/Icinga/Authentication/BackendMock.php
new file mode 100644
index 000000000..4f885bca3
--- /dev/null
+++ b/test/php/library/Icinga/Authentication/BackendMock.php
@@ -0,0 +1,59 @@
+<?php
+
+
+namespace Tests\Icinga\Authentication;
+
+require_once("../../library/Icinga/Authentication/Credentials.php");
+require_once("../../library/Icinga/Authentication/UserBackend.php");
+require_once("../../library/Icinga/Authentication/User.php");
+
+use Icinga\Authentication\Credentials as Credentials;
+use Icinga\Authentication\UserBackend as UserBackend;
+use Icinga\Authentication\User as User;
+
+/**
+*   Simple backend mock that takes an config object  
+*   with the property "credentials", which is an array 
+*   of Credentials this backend authenticates
+**/
+class BackendMock implements UserBackend
+{
+    public $allowedCredentials = array();
+    public function __construct($config = null)
+    {
+        if ($config === null) {
+            return;
+        }
+        if (isset ($config->credentials)) {
+            $this->allowedCredentials = $config->credentials;
+        }
+    }
+
+    public function hasUsername(Credentials $userCredentials)
+    {
+        foreach ($this->allowedCredentials as $credential) {
+            if ($credential->getUsername() == $userCredentials->getUsername()) {
+                return true;
+            }
+        }
+        return false;
+    }
+    
+    public static function getDummyUser()
+    {
+        return new User(
+            "Username",
+            "Firstname",
+            "Lastname",
+            "user@test.local"
+        );
+    }
+
+    public function authenticate(Credentials $credentials)
+    {
+        if (!in_array($credentials, $this->allowedCredentials)) {
+            return false;
+        }
+        return self::getDummyUser();
+    }
+}
diff --git a/test/php/library/Icinga/Authentication/ManagerTest.php b/test/php/library/Icinga/Authentication/ManagerTest.php
new file mode 100644
index 000000000..4af29555c
--- /dev/null
+++ b/test/php/library/Icinga/Authentication/ManagerTest.php
@@ -0,0 +1,122 @@
+<?php
+
+
+namespace Tests\Icinga\Authentication;
+
+require_once("../../library/Icinga/Application/Logger.php");
+require_once("../../library/Icinga/Authentication/Manager.php");
+require_once("../../library/Icinga/Authentication/Credentials.php");
+require_once("Zend/Log.php");
+require_once("BackendMock.php");
+require_once("SessionMock.php");
+
+use Icinga\Authentication\Manager as AuthManager;
+use Icinga\Authentication\Credentials as Credentials;
+
+/**
+*
+* Test class for Manager 
+* Created Mon, 10 Jun 2013 07:54:34 +0000 
+*
+**/
+class ManagerTest extends \PHPUnit_Framework_TestCase
+{
+    public function getTestCredentials()
+    {
+        return (object) array("credentials" => array(
+            new Credentials("jdoe", "passjdoe"),
+            new Credentials("root", "passroot"),
+            new Credentials("test", "passtest")
+        ));
+    }
+
+    public function getManagerInstance(&$session = null, $write = false)
+    {
+        if ($session == null) {
+            $session = new SessionMock();
+        }
+        return  AuthManager::getInstance(
+            (object) array(),
+            array(
+                "userBackendClass" => new BackendMock(
+                    $this->getTestCredentials()
+                ),
+                "groupBackendClass" => new BackendMock(),
+                "sessionClass" => $session,
+                "writeSession" => $write
+            )
+        );
+    }
+
+    public function testManagerInstanciation()
+    {
+        AuthManager::clearInstance();
+        $this->setPreserveGlobalState(false);
+        $authMgr = $this->getManagerInstance();
+        $auth = $this->assertEquals($authMgr, AuthManager::getInstance());
+        AuthManager::clearInstance();
+    }
+
+
+    public function testAuthentication()
+    {
+        AuthManager::clearInstance();
+        $auth = $this->getManagerInstance();
+        $this->assertFalse(
+            $auth->authenticate(
+                new Credentials("jhoe", "passjdoe"),
+                false
+            )
+        );
+        $this->assertFalse(
+            $auth->authenticate(
+                new Credentials("joe", "passjhoe"),
+                false
+            )
+        );
+        $this->assertTrue(
+            $auth->authenticate(
+                new Credentials("jdoe", "passjdoe"),
+                false
+            )
+        );
+        AuthManager::clearInstance();
+    }
+
+    public function testPersistAuthInSession()
+    {
+        AuthManager::clearInstance();
+        $session = new SessionMock();
+        $auth = $this->getManagerInstance($session, true);
+        $this->assertFalse($auth->isAuthenticated(true));
+        $auth->authenticate(new Credentials("jdoe", "passjdoe"));
+        $this->assertNotEquals(null, $session->get("user"));
+        $user = $session->get("user");
+        $this->assertEquals("Username", $user->getUsername());
+        $this->assertTrue($auth->isAuthenticated(true));
+        AuthManager::clearInstance();
+    }
+
+    public function testAuthenticateFromSession()
+    {
+        AuthManager::clearInstance();
+        $session = new SessionMock();
+        $session->set("user", BackendMock::getDummyUser());
+        $auth = $this->getManagerInstance($session, false);
+        $this->assertFalse($auth->isAuthenticated(true));
+        $this->assertTrue($auth->isAuthenticated());
+        $this->assertTrue($auth->isAuthenticated());
+    }
+
+    /**
+    *
+    *   @expectedException \Exception
+    **/
+    public function testWriteSessionTwice()
+    {
+        $auth = $this->getManagerInstance($session, false);
+        $this->assertFalse($auth->isAuthenticated(true));
+        $auth->authenticate(new Credentials("jdoe", "passjdoe"));
+        
+    }
+}
diff --git a/test/php/library/Icinga/Authentication/SessionMock.php b/test/php/library/Icinga/Authentication/SessionMock.php
new file mode 100644
index 000000000..7b2eca730
--- /dev/null
+++ b/test/php/library/Icinga/Authentication/SessionMock.php
@@ -0,0 +1,49 @@
+<?php
+
+
+namespace Tests\Icinga\Authentication;
+
+require_once("../../library/Icinga/Authentication/Session.php");
+
+use Icinga\Authentication\Session as Session;
+
+class SessionMock extends Session
+{
+    public $isOpen = false;
+    public $isWritten = false;
+    
+    public function open()
+    {
+        if (!$this->isOpen && $this->isWritten) {
+            throw new \Exception("Session write after close");
+        }
+        $this->isOpen = true;
+    }
+    
+    public function read($keepOpen = false)
+    {
+        $this->open();
+        if (!$keepOpen) {
+            $this->close();
+        }
+    }
+    
+    public function write($keepOpen = false)
+    {
+        
+        $this->open();
+        if (!$keepOpen) {
+            $this->close();
+        }
+    }
+    
+    public function close()
+    {
+        $this->isOpen = false;
+        $this->isWritten = true;
+    }
+
+    public function purge()
+    {
+    }
+}