From 9ac28b12b4af1c403ea7f2ac7de7a6feda3b940f Mon Sep 17 00:00:00 2001 From: Adirelle Date: Sun, 8 Mar 2015 18:53:08 +0100 Subject: [PATCH] First version of the pluggable authentication. --- .../Authentication/LoginPasswordProvider.php | 30 +++++ PHPCI/Security/Authentication/Service.php | 111 ++++++++++++++++++ .../Security/Authentication/UserProvider.php | 36 ++++++ .../UserProvider/AbstractProvider.php | 40 +++++++ .../Authentication/UserProvider/Internal.php | 37 ++++++ .../Security/Authentication/ServiceTest.php | 89 ++++++++++++++ .../UserProvider/InternalTest.php | 70 +++++++++++ .../Controller/SessionController.php | 46 ++++++-- 8 files changed, 449 insertions(+), 10 deletions(-) create mode 100644 PHPCI/Security/Authentication/LoginPasswordProvider.php create mode 100644 PHPCI/Security/Authentication/Service.php create mode 100644 PHPCI/Security/Authentication/UserProvider.php create mode 100644 PHPCI/Security/Authentication/UserProvider/AbstractProvider.php create mode 100644 PHPCI/Security/Authentication/UserProvider/Internal.php create mode 100644 Tests/PHPCI/Security/Authentication/ServiceTest.php create mode 100644 Tests/PHPCI/Security/Authentication/UserProvider/InternalTest.php diff --git a/PHPCI/Security/Authentication/LoginPasswordProvider.php b/PHPCI/Security/Authentication/LoginPasswordProvider.php new file mode 100644 index 00000000..5faef3dd --- /dev/null +++ b/PHPCI/Security/Authentication/LoginPasswordProvider.php @@ -0,0 +1,30 @@ + + */ +interface LoginPasswordProvider extends UserProvider +{ + /** Verify if the supplied password matches the user's one. + * + * @param User $user + * @param string $password + * + * @return bool + */ + public function verifyPassword(User $user, $password); +} diff --git a/PHPCI/Security/Authentication/Service.php b/PHPCI/Security/Authentication/Service.php new file mode 100644 index 00000000..7b3ce929 --- /dev/null +++ b/PHPCI/Security/Authentication/Service.php @@ -0,0 +1,111 @@ + + */ +class Service +{ + /** + * + * @var Service + */ + static private $instance; + + /** Return the service singletion. + * + * @return Service + */ + public static function getInstance() + { + if (self::$instance === null) { + $config = Config::getInstance()->get( + 'phpci.security.authentication', + array('internal' => 'internal') + ); + + $providers = []; + foreach ($config as $key => $providerConfig) { + $providers[$key] = self::buildProvider($key, $providerConfig); + } + self::$instance = new self($providers); + } + return self::$instance; + } + + /** Create a provider from a given configuration. + * + * @param string $key + * @param string|array $config + * @return UserProvider + */ + public static function buildProvider($key, $config) + { + if (is_string($config)) { + $config = array('type' => $config); + } + + $type = $config['type']; + if (class_exists($type)) { + $class = $type; + } elseif (class_exists('PHPCI\\Security\\Authentication\\UserProvider\\' . $type)) { + $class = 'PHPCI\\Security\\Authentication\\UserProvider\\' . $type; + } else { + // TODO: error + } + + return new $class($key, $config); + } + + /** The table of providers. + * + * @var array + */ + private $providers; + + /** Initialize the service. + * + * @param array $providers + */ + public function __construct(array $providers) + { + $this->providers = $providers; + } + + /** Return all providers. + * + * @return UserProvider[] + */ + public function getProviders() + { + return $this->providers; + } + + /** Return the user providers that allows password authentication. + * + * @return LoginPasswordProvider[] + */ + public function getLoginPasswordProviders() + { + $providers = []; + foreach ($this->providers as $key => $provider) { + if ($provider instanceof LoginPasswordProvider) { + $providers[$key] = $provider; + } + } + return $providers; + } +} diff --git a/PHPCI/Security/Authentication/UserProvider.php b/PHPCI/Security/Authentication/UserProvider.php new file mode 100644 index 00000000..b72777eb --- /dev/null +++ b/PHPCI/Security/Authentication/UserProvider.php @@ -0,0 +1,36 @@ + + */ +interface UserProvider +{ + + /** Check if all software requirements are met (libraries, extensions, ...) + * + * @throws Exception + */ + public function checkRequirements(); + + /** Provision an new user for the given identifier. + * + * @param string $identifier The user identifier. + * + * @return User|null The new user or null if the provider does not know the user. + */ + public function provisionUser($identifier); +} diff --git a/PHPCI/Security/Authentication/UserProvider/AbstractProvider.php b/PHPCI/Security/Authentication/UserProvider/AbstractProvider.php new file mode 100644 index 00000000..6c745d62 --- /dev/null +++ b/PHPCI/Security/Authentication/UserProvider/AbstractProvider.php @@ -0,0 +1,40 @@ + + */ +abstract class AbstractProvider implements UserProvider +{ + /** + * @var string + */ + private $key; + + public function __construct($key) + { + $this->key = $key; + } + + /** + * + * @return string + */ + public function getKey() + { + return $this->key; + } +} diff --git a/PHPCI/Security/Authentication/UserProvider/Internal.php b/PHPCI/Security/Authentication/UserProvider/Internal.php new file mode 100644 index 00000000..ca521bf2 --- /dev/null +++ b/PHPCI/Security/Authentication/UserProvider/Internal.php @@ -0,0 +1,37 @@ + + */ +class Internal extends AbstractProvider implements LoginPasswordProvider +{ + + public function verifyPassword(User $user, $password) + { + return password_verify($password, $user->getHash()); + } + + public function checkRequirements() + { + // Always fine + } + + public function provisionUser($identifier) + { + return null; + } +} diff --git a/Tests/PHPCI/Security/Authentication/ServiceTest.php b/Tests/PHPCI/Security/Authentication/ServiceTest.php new file mode 100644 index 00000000..79de61ea --- /dev/null +++ b/Tests/PHPCI/Security/Authentication/ServiceTest.php @@ -0,0 +1,89 @@ +assertInstanceOf('PHPCI\Security\Authentication\Service', Service::getInstance()); + } + + /** + * @covers PHPCI\Security\Authentication\Service::buildProvider + */ + public function testBuildBuiltinProvider() + { + $provider = Service::buildProvider("test", array('type' => 'internal')); + + $this->assertInstanceOf('PHPCI\Security\Authentication\UserProvider\Internal', $provider); + } + + /** + * @covers PHPCI\Security\Authentication\Service::buildProvider + */ + public function testBuildAnyProvider() + { + $config = array('type' => 'PHPCI\Security\Authentication\Tests\DummyProvider'); + $provider = Service::buildProvider("test", $config); + + $this->assertInstanceOf('PHPCI\Security\Authentication\Tests\DummyProvider', $provider); + $this->assertEquals('test', $provider->key); + $this->assertEquals($config, $provider->config); + } + + /** + * @covers PHPCI\Security\Authentication\Service::getProviders + */ + public function testGetProviders() + { + $a = $this->prophesize('PHPCI\Security\Authentication\UserProvider')->reveal(); + $b = $this->prophesize('PHPCI\Security\Authentication\UserProvider')->reveal(); + $providers = array('a' => $a, 'b' => $b); + + $service = new Service($providers); + + $this->assertEquals($providers, $service->getProviders()); + } + + /** + * @covers PHPCI\Security\Authentication\Service::getLoginPasswordProviders + * @todo Implement testGetLoginPasswordProviders(). + */ + public function testGetLoginPasswordProviders() + { + $a = $this->prophesize('PHPCI\Security\Authentication\UserProvider')->reveal(); + $b = $this->prophesize('PHPCI\Security\Authentication\LoginPasswordProvider')->reveal(); + $providers = array('a' => $a, 'b' => $b); + + $service = new Service($providers); + + $this->assertEquals(array('b' => $b), $service->getLoginPasswordProviders()); + } +} + +class DummyProvider +{ + public $key; + public $config; + public function __construct($key, array $config) + { + $this->key = $key; + $this->config = $config; + } +} diff --git a/Tests/PHPCI/Security/Authentication/UserProvider/InternalTest.php b/Tests/PHPCI/Security/Authentication/UserProvider/InternalTest.php new file mode 100644 index 00000000..5068616a --- /dev/null +++ b/Tests/PHPCI/Security/Authentication/UserProvider/InternalTest.php @@ -0,0 +1,70 @@ +provider = new Internal("internal"); + } + + /** + * @covers PHPCI\Security\Authentication\UserProvider\Internal::verifyPassword + */ + public function testVerifyPassword() + { + $user = new \PHPCI\Model\User; + $password = 'bla'; + $user->setHash(password_hash($password, PASSWORD_DEFAULT)); + + $this->assertTrue($this->provider->verifyPassword($user, $password)); + } + + /** + * @covers PHPCI\Security\Authentication\UserProvider\Internal::verifyPassword + */ + public function testVerifyInvaldPassword() + { + $user = new \PHPCI\Model\User; + $password = 'foo'; + $user->setHash(password_hash($password, PASSWORD_DEFAULT)); + + $this->assertFalse($this->provider->verifyPassword($user, 'bar')); + } + + /** + * @covers PHPCI\Security\Authentication\UserProvider\Internal::checkRequirements + */ + public function testCheckRequirements() + { + $this->provider->checkRequirements(); + } + + /** + * @covers PHPCI\Security\Authentication\UserProvider\Internal::provisionUser + */ + public function testProvisionUser() + { + $this->assertNull($this->provider->provisionUser('john@doe.com')); + } +} diff --git a/src/PHPCensor/Controller/SessionController.php b/src/PHPCensor/Controller/SessionController.php index 5d9f4661..db0e3a36 100644 --- a/src/PHPCensor/Controller/SessionController.php +++ b/src/PHPCensor/Controller/SessionController.php @@ -3,7 +3,7 @@ /** * PHPCI - Continuous Integration for PHP * - * @copyright Copyright 2014, Block 8 Limited. + * @copyright Copyright 2015, Block 8 Limited. * @license https://github.com/Block8/PHPCI/blob/master/LICENSE.md * @link https://www.phptesting.org/ */ @@ -17,6 +17,7 @@ use PHPCensor\Controller; /** * Session Controller - Handles user login / logout. + * * @author Dan Cryer * @package PHPCI * @subpackage Web @@ -28,6 +29,11 @@ class SessionController extends Controller */ protected $userStore; + /** + * @var \PHPCI\Security\Authentication\Service + */ + protected $authentication; + /** * Initialise the controller, set up stores and services. */ @@ -35,11 +41,12 @@ class SessionController extends Controller { $this->response->disableLayout(); $this->userStore = b8\Store\Factory::getStore('User'); + $this->authentication = \PHPCI\Security\Authentication\Service::getInstance(); } /** - * Handles user login (form and processing) - */ + * Handles user login (form and processing) + */ public function login() { $isLoginFailure = false; @@ -51,23 +58,42 @@ class SessionController extends Controller } else { unset($_SESSION['login_token']); - $user = $this->userStore->getByEmailOrName($this->getParam('email')); + $email = $this->getParam('email'); + $password = $this->getParam('password', ''); + $isLoginFailure = true; - if ($user && password_verify($this->getParam('password', ''), $user->getHash())) { - session_regenerate_id(true); - $_SESSION['php-censor-user-id'] = $user->getId(); + $user = $this->userStore->getByEmailOrName($email); + + $providers = $this->authentication->getLoginPasswordProviders(); + + if (null !== $user) { + // Delegate password verification to the user provider, if found + $key = $user->getProviderKey(); + $isLoginFailure = !isset($providers[$key]) || !$providers[$key]->verifyPassword($user, $password); + } else { + // Ask each providers to provision the user + foreach ($providers as $provider) { + $user = $provider->provisionUser($email); + if ($user !== null && $provider->verifyPassword($user, $password)) { + $this->userStore->save($user); + $isLoginFailure = false; + break; + } + } + } + + if (!$isLoginFailure) { + $_SESSION['php-censor-user-id'] = $user->getId(); $response = new b8\Http\Response\RedirectResponse(); $response->setHeader('Location', $this->getLoginRedirect()); return $response; - } else { - $isLoginFailure = true; } } } $form = new b8\Form(); $form->setMethod('POST'); - $form->setAction(APP_URL.'session/login'); + $form->setAction(APP_URL . 'session/login'); $email = new b8\Form\Element\Text('email'); $email->setLabel(Lang::get('login'));