diff --git a/app/config.example.yml b/app/config.example.yml index 2e7d27e3..b0574a30 100644 --- a/app/config.example.yml +++ b/app/config.example.yml @@ -22,8 +22,18 @@ php-censor: comments: commit: false pull_request: false - authentication_settings: - state: false - user_id: 1 build: remove_builds: true + security: + disable_auth: false + default_user_id: 1 + auth_providers: + internal: + type: internal + ldap: + type: ldap + data: + host: 'ldap.php-censor.local' + port: 389 + base_dn: 'dc=php-censor,dc=local' + mail_attribute: mail diff --git a/docs/en/configuring.md b/docs/en/configuring.md index 85582e88..b72655c2 100644 --- a/docs/en/configuring.md +++ b/docs/en/configuring.md @@ -9,10 +9,12 @@ username/password pair and have forgotten the password, and if the server is on the `forgot password` email, then editing the config file manually would be handy. To do so, just edit the `php-censor` section in the config file (which is in [yaml format](https://en.wikipedia.org/wiki/YAML)), and add - php-censor: - authentication_settings: - state: 1 - user_id: 1 +```yml +php-censor: + security: + disable_auth: true + default_user_id: 1 +``` -where you can get the user_id by logging into the mysql database and selecting your user ID from the `users` table in +where you can get the `default_user_id by` logging into the mysql database and selecting your user ID from the `users` table in the PHP Censor database. diff --git a/phpunit.xml b/phpunit.xml index c2bbf131..5c1a9c76 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -40,6 +40,9 @@ ./tests/PHPCensor/ProcessControl + + ./tests/PHPCensor/Security + diff --git a/src/PHPCensor/Application.php b/src/PHPCensor/Application.php index af1563bb..c338c227 100644 --- a/src/PHPCensor/Application.php +++ b/src/PHPCensor/Application.php @@ -158,13 +158,13 @@ class Application extends b8\Application */ protected function shouldSkipAuth() { - $config = b8\Config::getInstance(); - $state = (bool)$config->get('php-censor.authentication_settings.state', false); - $userId = $config->get('php-censor.authentication_settings.user_id', 0); + $config = b8\Config::getInstance(); + $disableAuth = (bool)$config->get('php-censor.security.disable_auth', false); + $defaultUserId = (integer)$config->get('php-censor.security.default_user_id', 1); - if (false !== $state && 0 != (int)$userId) { + if ($disableAuth && $defaultUserId) { $user = b8\Store\Factory::getStore('User') - ->getByPrimaryKey($userId); + ->getByPrimaryKey($defaultUserId); if ($user) { $_SESSION['php-censor-user'] = $user; diff --git a/src/PHPCensor/Controller/SessionController.php b/src/PHPCensor/Controller/SessionController.php index 5d9f4661..6449ada6 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/ */ @@ -14,9 +14,12 @@ use b8; use PHPCensor\Helper\Email; use PHPCensor\Helper\Lang; use PHPCensor\Controller; +use PHPCensor\Security\Authentication\Service; +use PHPCensor\Store\UserStore; /** * Session Controller - Handles user login / logout. + * * @author Dan Cryer * @package PHPCI * @subpackage Web @@ -24,22 +27,29 @@ use PHPCensor\Controller; class SessionController extends Controller { /** - * @var \PHPCensor\Store\UserStore + * @var UserStore */ protected $userStore; + /** + * @var Service + */ + protected $authentication; + /** * Initialise the controller, set up stores and services. */ public function init() { $this->response->disableLayout(); - $this->userStore = b8\Store\Factory::getStore('User'); + + $this->userStore = b8\Store\Factory::getStore('User'); + $this->authentication = Service::getInstance(); } /** - * Handles user login (form and processing) - */ + * Handles user login (form and processing) + */ public function login() { $isLoginFailure = false; @@ -51,23 +61,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')); diff --git a/src/PHPCensor/Controller/SettingsController.php b/src/PHPCensor/Controller/SettingsController.php index fa227b79..5e274405 100644 --- a/src/PHPCensor/Controller/SettingsController.php +++ b/src/PHPCensor/Controller/SettingsController.php @@ -47,6 +47,7 @@ class SettingsController extends Controller /** * Display settings forms. + * * @return string */ public function index() @@ -72,18 +73,18 @@ class SettingsController extends Controller $emailSettings = $this->settings['php-censor']['email_settings']; } - $authSettings = []; - if (isset($this->settings['php-censor']['authentication_settings'])) { - $authSettings = $this->settings['php-censor']['authentication_settings']; + $securitySettings = []; + if (isset($this->settings['php-censor']['security'])) { + $securitySettings = $this->settings['php-censor']['security']; } - $this->view->configFile = APP_DIR . 'config.yml'; - $this->view->basicSettings = $this->getBasicForm($basicSettings); - $this->view->buildSettings = $this->getBuildForm($buildSettings); - $this->view->github = $this->getGithubForm(); - $this->view->emailSettings = $this->getEmailForm($emailSettings); - $this->view->authenticationSettings = $this->getAuthenticationForm($authSettings); - $this->view->isWriteable = $this->canWriteConfig(); + $this->view->configFile = APP_DIR . 'config.yml'; + $this->view->basicSettings = $this->getBasicForm($basicSettings); + $this->view->buildSettings = $this->getBuildForm($buildSettings); + $this->view->github = $this->getGithubForm(); + $this->view->emailSettings = $this->getEmailForm($emailSettings); + $this->view->securitySettings = $this->getAuthenticationForm($securitySettings); + $this->view->isWriteable = $this->canWriteConfig(); if (!empty($this->settings['php-censor']['github']['token'])) { $this->view->githubUser = $this->getGithubUser($this->settings['php-censor']['github']['token']); @@ -187,8 +188,8 @@ class SettingsController extends Controller { $this->requireAdmin(); - $this->settings['php-censor']['authentication_settings']['state'] = $this->getParam('disable_authentication', 0); - $this->settings['php-censor']['authentication_settings']['user_id'] = $_SESSION['php-censor-user-id']; + $this->settings['php-censor']['security']['disable_auth'] = (boolean)$this->getParam('disable_authentication', false); + $this->settings['php-censor']['security']['default_user_id'] = (integer)$_SESSION['php-censor-user-id']; $error = $this->storeSettings(); @@ -479,8 +480,8 @@ class SettingsController extends Controller $field->setContainerClass('form-group'); $field->setValue(0); - if (isset($values['state'])) { - $field->setValue((int)$values['state']); + if (isset($values['disable_auth'])) { + $field->setValue((integer)$values['disable_auth']); } $form->addField($field); diff --git a/src/PHPCensor/Helper/Email.php b/src/PHPCensor/Helper/Email.php index c9db49c5..eafecfff 100644 --- a/src/PHPCensor/Helper/Email.php +++ b/src/PHPCensor/Helper/Email.php @@ -119,10 +119,12 @@ class Email * * @return integer */ - public function send(Builder $builder) + public function send(Builder $builder = null) { $smtpServer = $this->config->get('php-censor.email_settings.smtp_address'); - $builder->logDebug(sprintf("SMTP: '%s'", !empty($smtpServer) ? 'true' : 'false')); + if (null !== $builder) { + $builder->logDebug(sprintf("SMTP: '%s'", !empty($smtpServer) ? 'true' : 'false')); + } $factory = new MailerFactory($this->config->get('php-censor')); $mailer = $factory->getSwiftMailerFromConfig(); diff --git a/src/PHPCensor/Helper/LoginIsDisabled.php b/src/PHPCensor/Helper/LoginIsDisabled.php index 9d07ef95..fca0c8fe 100644 --- a/src/PHPCensor/Helper/LoginIsDisabled.php +++ b/src/PHPCensor/Helper/LoginIsDisabled.php @@ -21,17 +21,19 @@ class LoginIsDisabled { /** * Checks if + * * @param $method * @param array $params + * * @return mixed|null */ public function __call($method, $params = []) { unset($method, $params); - $config = Config::getInstance(); - $state = (bool) $config->get('php-censor.authentication_settings.state', false); + $config = Config::getInstance(); + $disableAuth = (boolean)$config->get('php-censor.security.disable_auth', false); - return (false !== $state); + return $disableAuth; } } diff --git a/src/PHPCensor/Migrations/20150308074509_add_user_providers.php b/src/PHPCensor/Migrations/20150308074509_add_user_providers.php new file mode 100644 index 00000000..a01c626b --- /dev/null +++ b/src/PHPCensor/Migrations/20150308074509_add_user_providers.php @@ -0,0 +1,41 @@ +table('user') + // The provider name + ->addColumn('provider_key', 'string', [ + 'default' => 'internal', + 'limit' => MysqlAdapter::TEXT_SMALL + ]) + // A data used by the provider + ->addColumn('provider_data', 'string', [ + 'null' => true, + 'limit' => MysqlAdapter::TEXT_SMALL + ]) + ->save(); + } + + /** + * Migrate Down. + */ + public function down() + { + // Remove the provider columns + $this + ->table('user') + ->removeColumn('provider_key') + ->removeColumn('provider_data') + ->save(); + } +} diff --git a/src/PHPCensor/Migrations/20150324174958_unique_email_and_name_user_fields.php b/src/PHPCensor/Migrations/20150324174958_unique_email_and_name_user_fields.php index 8c1f4b7c..5a19ff16 100644 --- a/src/PHPCensor/Migrations/20150324174958_unique_email_and_name_user_fields.php +++ b/src/PHPCensor/Migrations/20150324174958_unique_email_and_name_user_fields.php @@ -22,11 +22,11 @@ class UniqueEmailAndNameUserFields extends AbstractMigration $user_table = $this->table('user'); if ($user_table->hasIndex('email', ['unique' => true])) { - $user_table->removeIndex('email', ['unique' => true])->save(); + $user_table->removeIndex(['email'], ['unique' => true])->save(); } if ($user_table->hasIndex('name', ['unique' => true])) { - $user_table->removeIndex('name', ['unique' => true])->save(); + $user_table->removeIndex(['name'], ['unique' => true])->save(); } } } diff --git a/src/PHPCensor/Model/Base/UserBase.php b/src/PHPCensor/Model/Base/UserBase.php index ff2e03b5..27006706 100644 --- a/src/PHPCensor/Model/Base/UserBase.php +++ b/src/PHPCensor/Model/Base/UserBase.php @@ -33,13 +33,15 @@ class UserBase extends Model * @var array */ protected $data = [ - 'id' => null, - 'email' => null, - 'hash' => null, - 'is_admin' => null, - 'name' => null, - 'language' => null, - 'per_page' => null, + 'id' => null, + 'email' => null, + 'hash' => null, + 'is_admin' => null, + 'name' => null, + 'language' => null, + 'per_page' => null, + 'provider_key' => null, + 'provider_data' => null, ]; /** @@ -47,13 +49,15 @@ class UserBase extends Model */ protected $getters = [ // Direct property getters: - 'id' => 'getId', - 'email' => 'getEmail', - 'hash' => 'getHash', - 'is_admin' => 'getIsAdmin', - 'name' => 'getName', - 'language' => 'getLanguage', - 'per_page' => 'getPerPage', + 'id' => 'getId', + 'email' => 'getEmail', + 'hash' => 'getHash', + 'is_admin' => 'getIsAdmin', + 'name' => 'getName', + 'language' => 'getLanguage', + 'per_page' => 'getPerPage', + 'provider_key' => 'getProviderKey', + 'provider_data' => 'getProviderData', // Foreign key getters: ]; @@ -62,13 +66,15 @@ class UserBase extends Model */ protected $setters = [ // Direct property setters: - 'id' => 'setId', - 'email' => 'setEmail', - 'hash' => 'setHash', - 'is_admin' => 'setIsAdmin', - 'name' => 'setName', - 'language' => 'setLanguage', - 'per_page' => 'setPerPage', + 'id' => 'setId', + 'email' => 'setEmail', + 'hash' => 'setHash', + 'is_admin' => 'setIsAdmin', + 'name' => 'setName', + 'language' => 'setLanguage', + 'per_page' => 'setPerPage', + 'provider_key' => 'setProviderKey', + 'provider_data' => 'setProviderData', // Foreign key setters: ]; @@ -112,6 +118,17 @@ class UserBase extends Model 'length' => 11, 'default' => null, ], + 'provider_key' => [ + 'type' => 'varchar', + 'length' => 255, + 'default' => 'internal', + ], + 'provider_data' => [ + 'type' => 'varchar', + 'length' => 255, + 'nullable' => true, + 'default' => null, + ], ]; /** @@ -165,6 +182,18 @@ class UserBase extends Model return $rtn; } + /** + * Get the value of Name / name. + * + * @return string + */ + public function getName() + { + $rtn = $this->data['name']; + + return $rtn; + } + /** * Get the value of IsAdmin / is_admin. * @@ -178,13 +207,25 @@ class UserBase extends Model } /** - * Get the value of Name / name. + * Get the value of ProviderKey / provider_key. * * @return string */ - public function getName() + public function getProviderKey() { - $rtn = $this->data['name']; + $rtn = $this->data['provider_key']; + + return $rtn; + } + + /** + * Get the value of ProviderData / provider_data. + * + * @return string + */ + public function getProviderData() + { + $rtn = $this->data['provider_data']; return $rtn; } @@ -273,6 +314,26 @@ class UserBase extends Model $this->setModified('hash'); } + /** + * Set the value of Name / name. + * + * Must not be null. + * @param $value string + */ + public function setName($value) + { + $this->validateNotNull('Name', $value); + $this->validateString('Name', $value); + + if ($this->data['name'] === $value) { + return; + } + + $this->data['name'] = $value; + + $this->setModified('name'); + } + /** * Set the value of IsAdmin / is_admin. * @@ -294,23 +355,41 @@ class UserBase extends Model } /** - * Set the value of Name / name. + * Set the value of ProviderKey / provider_key. * * Must not be null. * @param $value string */ - public function setName($value) + public function setProviderKey($value) { - $this->validateNotNull('Name', $value); - $this->validateString('Name', $value); + $this->validateNotNull('ProviderKey', $value); + $this->validateString('ProviderKey', $value); - if ($this->data['name'] === $value) { + if ($this->data['provider_key'] === $value) { return; } - $this->data['name'] = $value; + $this->data['provider_key'] = $value; - $this->setModified('name'); + $this->setModified('provider_key'); + } + + /** + * Set the value of ProviderData / provider_data. + * + * @param $value string + */ + public function setProviderData($value) + { + $this->validateString('ProviderData', $value); + + if ($this->data['provider_data'] === $value) { + return; + } + + $this->data['provider_data'] = $value; + + $this->setModified('provider_data'); } /** diff --git a/src/PHPCensor/Plugin/Option/PhpUnitOptions.php b/src/PHPCensor/Plugin/Option/PhpUnitOptions.php index 50a684cd..a091b13f 100644 --- a/src/PHPCensor/Plugin/Option/PhpUnitOptions.php +++ b/src/PHPCensor/Plugin/Option/PhpUnitOptions.php @@ -20,7 +20,7 @@ namespace PHPCensor\Plugin\Option; class PhpUnitOptions { protected $options; - protected $arguments = array(); + protected $arguments = []; public function __construct($options) { @@ -52,7 +52,7 @@ class PhpUnitOptions $prefix = $argumentName[0] == '-' ? '' : '--'; if (!is_array($argumentValues)) { - $argumentValues = array($argumentValues); + $argumentValues = [$argumentValues]; } foreach ($argumentValues as $argValue) { @@ -139,7 +139,7 @@ class PhpUnitOptions if (isset($this->arguments[$argumentName])) { if (!is_array($this->arguments[$argumentName])) { // Convert existing argument values into an array - $this->arguments[$argumentName] = array($this->arguments[$argumentName]); + $this->arguments[$argumentName] = [$this->arguments[$argumentName]]; } // Appends the new argument to the list @@ -160,14 +160,14 @@ class PhpUnitOptions $directories = $this->getOption('directory'); if (is_string($directories)) { - $directories = array($directories); + $directories = [$directories]; } else { if (is_null($directories)) { - $directories = array(); + $directories = []; } } - return is_array($directories) ? $directories : array($directories); + return is_array($directories) ? $directories : [$directories]; } /** @@ -240,10 +240,10 @@ class PhpUnitOptions if (isset($this->arguments[$argumentName])) { return is_array( $this->arguments[$argumentName] - ) ? $this->arguments[$argumentName] : array($this->arguments[$argumentName]); + ) ? $this->arguments[$argumentName] : [$this->arguments[$argumentName]]; } - return array(); + return []; } /** @@ -255,12 +255,12 @@ class PhpUnitOptions */ public static function findConfigFile($buildPath) { - $files = array( + $files = [ 'phpunit.xml', 'phpunit.xml.dist', 'tests/phpunit.xml', 'tests/phpunit.xml.dist', - ); + ]; foreach ($files as $file) { if (file_exists($buildPath . $file)) { diff --git a/src/PHPCensor/Plugin/PhpUnit.php b/src/PHPCensor/Plugin/PhpUnit.php index 014b6337..ec8c22b8 100644 --- a/src/PHPCensor/Plugin/PhpUnit.php +++ b/src/PHPCensor/Plugin/PhpUnit.php @@ -30,7 +30,7 @@ use PHPCensor\ZeroConfigPluginInterface; class PhpUnit extends Plugin implements ZeroConfigPluginInterface { /** @var string[] Raw options from the PHPCI config file */ - protected $options = array(); + protected $options = []; /** * @return string diff --git a/src/PHPCensor/Plugin/Util/PhpUnitResult.php b/src/PHPCensor/Plugin/Util/PhpUnitResult.php index a8264e21..ef95d66f 100644 --- a/src/PHPCensor/Plugin/Util/PhpUnitResult.php +++ b/src/PHPCensor/Plugin/Util/PhpUnitResult.php @@ -29,10 +29,10 @@ class PhpUnitResult const SEVERITY_SKIPPED = 'skipped'; protected $options; - protected $arguments = array(); + protected $arguments = []; protected $results; protected $failures = 0; - protected $errors = array(); + protected $errors = []; public function __construct($outputFile, $buildPath = '') { @@ -60,8 +60,8 @@ class PhpUnitResult } // Reset the parsing variables - $this->results = array(); - $this->errors = array(); + $this->results = []; + $this->errors = []; $this->failures = 0; if (is_array($events)) { @@ -88,13 +88,13 @@ class PhpUnitResult { list($pass, $severity) = $this->getStatus($event); - $data = array( + $data = [ 'pass' => $pass, 'severity' => $severity, 'message' => $this->buildMessage($event), - 'trace' => $pass ? array() : $this->buildTrace($event), + 'trace' => $pass ? [] : $this->buildTrace($event), 'output' => $event['output'], - ); + ]; if (!$pass) { $this->failures++; @@ -142,7 +142,7 @@ class PhpUnitResult break; } - return array($pass, $severity); + return [$pass, $severity]; } /** @@ -172,7 +172,7 @@ class PhpUnitResult */ protected function buildTrace($event) { - $formattedTrace = array(); + $formattedTrace = []; if (!empty($event['trace'])) { foreach ($event['trace'] as $step){ @@ -195,12 +195,12 @@ class PhpUnitResult $firstTrace = end($event['trace']); reset($event['trace']); - $this->errors[] = array( + $this->errors[] = [ 'message' => $data['message'], 'severity' => $data['severity'], 'file' => str_replace($this->buildPath, '', $firstTrace['file']), 'line' => $firstTrace['line'], - ); + ]; } /** diff --git a/src/PHPCensor/Security/Authentication/LoginPasswordProviderInterface.php b/src/PHPCensor/Security/Authentication/LoginPasswordProviderInterface.php new file mode 100644 index 00000000..ba0a1896 --- /dev/null +++ b/src/PHPCensor/Security/Authentication/LoginPasswordProviderInterface.php @@ -0,0 +1,30 @@ + + */ +interface LoginPasswordProviderInterface extends UserProviderInterface +{ + /** 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/src/PHPCensor/Security/Authentication/Service.php b/src/PHPCensor/Security/Authentication/Service.php new file mode 100644 index 00000000..5dd69cf4 --- /dev/null +++ b/src/PHPCensor/Security/Authentication/Service.php @@ -0,0 +1,109 @@ + + */ +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( + 'php-censor.security.auth_providers', + [ + 'internal' => [ + 'type' => '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 UserProviderInterface + */ + public static function buildProvider($key, $config) + { + $class = ucfirst($config['type']); + if (class_exists('\\PHPCensor\\Security\\Authentication\\UserProvider\\' . $class)) { + $class = '\\PHPCensor\\Security\\Authentication\\UserProvider\\' . $class; + } + + 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 UserProviderInterface[] + */ + public function getProviders() + { + return $this->providers; + } + + /** Return the user providers that allows password authentication. + * + * @return LoginPasswordProviderInterface[] + */ + public function getLoginPasswordProviders() + { + $providers = []; + foreach ($this->providers as $key => $provider) { + if ($provider instanceof LoginPasswordProviderInterface) { + $providers[$key] = $provider; + } + } + return $providers; + } +} diff --git a/src/PHPCensor/Security/Authentication/UserProvider/AbstractProvider.php b/src/PHPCensor/Security/Authentication/UserProvider/AbstractProvider.php new file mode 100644 index 00000000..6d8cf8de --- /dev/null +++ b/src/PHPCensor/Security/Authentication/UserProvider/AbstractProvider.php @@ -0,0 +1,43 @@ + + */ +abstract class AbstractProvider implements UserProviderInterface +{ + /** + * @var string + */ + protected $key; + + /** + * @var array + */ + protected $config; + + /** + * AbstractProvider constructor + * + * @param string $key + * @param array $config + */ + public function __construct($key, array $config) + { + $this->key = $key; + $this->config = $config; + } +} diff --git a/src/PHPCensor/Security/Authentication/UserProvider/Internal.php b/src/PHPCensor/Security/Authentication/UserProvider/Internal.php new file mode 100644 index 00000000..ff8c7853 --- /dev/null +++ b/src/PHPCensor/Security/Authentication/UserProvider/Internal.php @@ -0,0 +1,37 @@ + + */ +class Internal extends AbstractProvider implements LoginPasswordProviderInterface +{ + 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/src/PHPCensor/Security/Authentication/UserProvider/Ldap.php b/src/PHPCensor/Security/Authentication/UserProvider/Ldap.php new file mode 100644 index 00000000..142da14f --- /dev/null +++ b/src/PHPCensor/Security/Authentication/UserProvider/Ldap.php @@ -0,0 +1,78 @@ +config['data'])) { + $ldapData = $this->config['data']; + $ldapPort = !empty($ldapData['port']) ? $ldapData['port'] : null; + $ldapHost = !empty($ldapData['host']) ? $ldapData['host'] : 'localhost'; + $ldapBaseDn = !empty($ldapData['base_dn']) ? $ldapData['base_dn'] : 'dc=nodomain'; + $ldapMail = !empty($ldapData['mail_attribute']) ? $ldapData['mail_attribute'] : 'mail'; + + if ($ldapPort) { + $ldap = @ldap_connect($ldapHost, $ldapPort); + } else { + $ldap = @ldap_connect($ldapHost); + } + + if (false === $ldap) { + return false; + } + + ldap_set_option($ldap, LDAP_OPT_PROTOCOL_VERSION, 3); + + $ls = @ldap_search($ldap, $ldapBaseDn, $ldapMail . '=' . $user->getEmail()); + if (false === $ls) { + return false; + } + + $le = @ldap_get_entries($ldap, $ls); + if (!$le['count']) { + return false; + } + + $dn = $le[0]['dn']; + + return @ldap_bind($ldap, $dn, $password); + } + + return false; + } + + public function checkRequirements() + { + // Always fine + } + + public function provisionUser($identifier) + { + $userService = new UserService(Factory::getStore('User')); + + $parts = explode("@", $identifier); + $username = $parts[0]; + + return $userService->createUserWithProvider($username, $identifier, $this->key, null); + } +} diff --git a/src/PHPCensor/Security/Authentication/UserProviderInterface.php b/src/PHPCensor/Security/Authentication/UserProviderInterface.php new file mode 100644 index 00000000..4861a83b --- /dev/null +++ b/src/PHPCensor/Security/Authentication/UserProviderInterface.php @@ -0,0 +1,36 @@ + + */ +interface UserProviderInterface +{ + + /** 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/src/PHPCensor/Service/UserService.php b/src/PHPCensor/Service/UserService.php index bf0688e8..dfddda6d 100644 --- a/src/PHPCensor/Service/UserService.php +++ b/src/PHPCensor/Service/UserService.php @@ -57,6 +57,31 @@ class UserService return $this->store->save($user); } + /** + * Create a new user within PHPCI (with provider). + * + * @param $name + * @param $emailAddress + * @param $providerKey + * @param $providerData + * @param bool $isAdmin + * + * @return \PHPCI\Model\User + */ + + public function createUserWithProvider($name, $emailAddress, $providerKey, $providerData, $isAdmin = false) + { + $user = new User(); + $user->setName($name); + $user->setEmail($emailAddress); + $user->setHash(""); + $user->setProviderKey($providerKey); + $user->setProviderData($providerData); + $user->setIsAdmin(($isAdmin ? 1 : 0)); + + return $this->store->save($user); + } + /** * Update a user. * diff --git a/src/PHPCensor/View/Settings/index.phtml b/src/PHPCensor/View/Settings/index.phtml index ba9bb404..68621b27 100644 --- a/src/PHPCensor/View/Settings/index.phtml +++ b/src/PHPCensor/View/Settings/index.phtml @@ -126,7 +126,7 @@ Be careful: This setting disables authentication and uses your current admin account for all actions within PHP Censor with admin rights.

- + diff --git a/tests/PHPCensor/Helper/CommandExecutorTest.php b/tests/PHPCensor/Helper/CommandExecutorTest.php index efd8d443..0439ffa2 100644 --- a/tests/PHPCensor/Helper/CommandExecutorTest.php +++ b/tests/PHPCensor/Helper/CommandExecutorTest.php @@ -75,7 +75,7 @@ class CommandExecutorTest extends \PHPUnit_Framework_TestCase /bin/sh -c 'data="$(printf %%${length}s | tr " " "-")"; >&2 echo "\$data"; >&1 echo "\$data"' EOD; $data = str_repeat("-", $length); - $returnValue = $this->testedExecutor->executeCommand(array($script)); + $returnValue = $this->testedExecutor->executeCommand([$script]); $this->assertTrue($returnValue); $this->assertEquals($data, trim($this->testedExecutor->getLastOutput())); $this->assertEquals($data, trim($this->testedExecutor->getLastError())); diff --git a/tests/PHPCensor/Plugin/Option/PhpUnitOptionsTest.php b/tests/PHPCensor/Plugin/Option/PhpUnitOptionsTest.php index 3555d233..880cee22 100644 --- a/tests/PHPCensor/Plugin/Option/PhpUnitOptionsTest.php +++ b/tests/PHPCensor/Plugin/Option/PhpUnitOptionsTest.php @@ -21,80 +21,80 @@ class PhpUnitOptionsTest extends \PHPUnit_Framework_TestCase { public function validOptionsProvider() { - return array( - array( - array( + return [ + [ + [ 'config' => 'tests/phpunit.xml', 'args' => '--stop-on-error --log-junit /path/to/log/', - ), - array( + ], + [ 'stop-on-error' => '', 'log-junit' => '/path/to/log/', 'configuration' => 'tests/phpunit.xml', - ), - ), - array( - array( + ], + ], + [ + [ 'coverage' => '/path/to/coverage2/', - 'args' => array( + 'args' => [ 'coverage-html' => '/path/to/coverage1/', - ), - ), - array( - 'coverage-html' => array( + ], + ], + [ + 'coverage-html' => [ '/path/to/coverage1/', '/path/to/coverage2/', - ), - ), - ), - array( - array( - 'directory' => array( + ], + ], + ], + [ + [ + 'directory' => [ '/path/to/test1/', '/path/to/test2/', - ), - 'args' => array( + ], + 'args' => [ 'coverage-html' => '/path/to/coverage1/', - ), - ), - array( + ], + ], + [ 'coverage-html' => '/path/to/coverage1/', - ), - ), - array( - array( - 'config' => array('tests/phpunit.xml'), + ], + ], + [ + [ + 'config' => ['tests/phpunit.xml'], 'args' => "--testsuite=unit --bootstrap=vendor/autoload.php", - ), - array( + ], + [ 'testsuite' => 'unit', 'bootstrap' => 'vendor/autoload.php', - 'configuration' => array('tests/phpunit.xml'), - ), - ), - array( - array( - 'config' => array('tests/phpunit.xml'), + 'configuration' => ['tests/phpunit.xml'], + ], + ], + [ + [ + 'config' => ['tests/phpunit.xml'], 'args' => "--testsuite='unit' --bootstrap 'vendor/autoload.php'", - ), - array( + ], + [ 'testsuite' => 'unit', 'bootstrap' => 'vendor/autoload.php', - 'configuration' => array('tests/phpunit.xml'), - ), - ), - array( - array( - 'config' => array('tests/phpunit.xml'), + 'configuration' => ['tests/phpunit.xml'], + ], + ], + [ + [ + 'config' => ['tests/phpunit.xml'], 'args' => '--testsuite="unit" --bootstrap "vendor/autoload.php"', - ), - array( + ], + [ 'testsuite' => 'unit', 'bootstrap' => 'vendor/autoload.php', - 'configuration' => array('tests/phpunit.xml'), - ), - ), - ); + 'configuration' => ['tests/phpunit.xml'], + ], + ], + ]; } /** @@ -112,10 +112,10 @@ class PhpUnitOptionsTest extends \PHPUnit_Framework_TestCase public function testGetters() { $options = new PhpUnitOptions( - array( + [ 'run_from' => '/path/to/run/from', 'path' => 'subTest', - ) + ] ); $this->assertEquals('/path/to/run/from', $options->getRunFrom()); diff --git a/tests/PHPCensor/Plugin/PhpUnitTest.php b/tests/PHPCensor/Plugin/PhpUnitTest.php index 5882eaf6..11cab788 100644 --- a/tests/PHPCensor/Plugin/PhpUnitTest.php +++ b/tests/PHPCensor/Plugin/PhpUnitTest.php @@ -19,11 +19,11 @@ class PhpUnitTest extends \PHPUnit_Framework_TestCase { public function testSingleConfigFile() { - $options = array( + $options = [ 'config' => ROOT_DIR . 'phpunit.xml' - ); + ]; - $mockPlugin = $this->getPluginBuilder($options)->setMethods(array('runConfigFile'))->getMock(); + $mockPlugin = $this->getPluginBuilder($options)->setMethods(['runConfigFile'])->getMock(); $mockPlugin->expects($this->once())->method('runConfigFile')->with(ROOT_DIR . 'phpunit.xml'); $mockPlugin->execute(); @@ -31,16 +31,16 @@ class PhpUnitTest extends \PHPUnit_Framework_TestCase public function testMultiConfigFile() { - $options = array( - 'config' => array( + $options = [ + 'config' => [ ROOT_DIR . 'phpunit1.xml', ROOT_DIR . 'phpunit2.xml', - ) - ); + ] + ]; - $mockPlugin = $this->getPluginBuilder($options)->setMethods(array('runConfigFile'))->getMock(); + $mockPlugin = $this->getPluginBuilder($options)->setMethods(['runConfigFile'])->getMock(); $mockPlugin->expects($this->exactly(2))->method('runConfigFile')->withConsecutive( - array(ROOT_DIR . 'phpunit1.xml'), array(ROOT_DIR . 'phpunit2.xml') + [ROOT_DIR . 'phpunit1.xml'], [ROOT_DIR . 'phpunit2.xml'] ); $mockPlugin->execute(); @@ -53,30 +53,30 @@ class PhpUnitTest extends \PHPUnit_Framework_TestCase * * @return \PHPUnit_Framework_MockObject_MockBuilder */ - protected function getPluginBuilder($options = array()) + protected function getPluginBuilder($options = []) { $loggerMock = $this->getMockBuilder('\Monolog\Logger') - ->setConstructorArgs(array('Test')) - ->setMethods(array('addRecord')) + ->setConstructorArgs(['Test']) + ->setMethods(['addRecord']) ->getMock(); $mockBuild = $this->getMockBuilder('\PHPCensor\Model\Build')->getMock(); $mockBuilder = $this->getMockBuilder('\PHPCensor\Builder') - ->setConstructorArgs(array($mockBuild, $loggerMock)) - ->setMethods(array('executeCommand'))->getMock(); + ->setConstructorArgs([$mockBuild, $loggerMock]) + ->setMethods(['executeCommand'])->getMock(); return $this->getMockBuilder('PHPCensor\Plugin\PhpUnit')->setConstructorArgs( - array($mockBuilder, $mockBuild, $options) + [$mockBuilder, $mockBuild, $options] ); } public function testSingleDir() { - $options = array( + $options = [ 'directory' => '/test/directory/one' - ); + ]; - $mockPlugin = $this->getPluginBuilder($options)->setMethods(array('runDir'))->getMock(); + $mockPlugin = $this->getPluginBuilder($options)->setMethods(['runDir'])->getMock(); $mockPlugin->expects($this->once())->method('runDir')->with('/test/directory/one'); $mockPlugin->execute(); @@ -84,16 +84,16 @@ class PhpUnitTest extends \PHPUnit_Framework_TestCase public function testMultiDir() { - $options = array( - 'directory' => array( + $options = [ + 'directory' => [ '/test/directory/one', '/test/directory/two', - ) - ); + ] + ]; - $mockPlugin = $this->getPluginBuilder($options)->setMethods(array('runDir'))->getMock(); + $mockPlugin = $this->getPluginBuilder($options)->setMethods(['runDir'])->getMock(); $mockPlugin->expects($this->exactly(2))->method('runDir')->withConsecutive( - array('/test/directory/one'), array('/test/directory/two') + ['/test/directory/one'], ['/test/directory/two'] ); $mockPlugin->execute(); @@ -101,11 +101,11 @@ class PhpUnitTest extends \PHPUnit_Framework_TestCase public function testProcessResultsFromConfig() { - $options = array( + $options = [ 'config' => ROOT_DIR . 'phpunit.xml' - ); + ]; - $mockPlugin = $this->getPluginBuilder($options)->setMethods(array('processResults'))->getMock(); + $mockPlugin = $this->getPluginBuilder($options)->setMethods(['processResults'])->getMock(); $mockPlugin->expects($this->once())->method('processResults')->with($this->isType('string')); $mockPlugin->execute(); @@ -113,11 +113,11 @@ class PhpUnitTest extends \PHPUnit_Framework_TestCase public function testProcessResultsFromDir() { - $options = array( + $options = [ 'directory' => ROOT_DIR . 'Tests' - ); + ]; - $mockPlugin = $this->getPluginBuilder($options)->setMethods(array('processResults'))->getMock(); + $mockPlugin = $this->getPluginBuilder($options)->setMethods(['processResults'])->getMock(); $mockPlugin->expects($this->once())->method('processResults')->with($this->isType('string')); $mockPlugin->execute(); diff --git a/tests/PHPCensor/Security/Authentication/ServiceTest.php b/tests/PHPCensor/Security/Authentication/ServiceTest.php new file mode 100644 index 00000000..7e628e93 --- /dev/null +++ b/tests/PHPCensor/Security/Authentication/ServiceTest.php @@ -0,0 +1,86 @@ +assertInstanceOf('\PHPCensor\Security\Authentication\Service', Service::getInstance()); + } + + /** + * @covers Service::buildProvider + */ + public function testBuildBuiltinProvider() + { + $provider = Service::buildProvider('test', ['type' => 'internal']); + + $this->assertInstanceOf('\PHPCensor\Security\Authentication\UserProvider\Internal', $provider); + } + + /** + * @covers Service::buildProvider + */ + public function testBuildAnyProvider() + { + $config = ['type' => '\Tests\PHPCensor\Security\Authentication\DummyProvider']; + $provider = Service::buildProvider("test", $config); + + $this->assertInstanceOf('\Tests\PHPCensor\Security\Authentication\DummyProvider', $provider); + $this->assertEquals('test', $provider->key); + $this->assertEquals($config, $provider->config); + } + + /** + * @covers Service::getProviders + */ + public function testGetProviders() + { + $a = $this->prophesize('\PHPCensor\Security\Authentication\UserProviderInterface')->reveal(); + $b = $this->prophesize('\PHPCensor\Security\Authentication\UserProviderInterface')->reveal(); + $providers = ['a' => $a, 'b' => $b]; + + $service = new Service($providers); + + $this->assertEquals($providers, $service->getProviders()); + } + + /** + * @covers Service::getLoginPasswordProviders + */ + public function testGetLoginPasswordProviders() + { + $a = $this->prophesize('\PHPCensor\Security\Authentication\UserProviderInterface')->reveal(); + $b = $this->prophesize('\PHPCensor\Security\Authentication\LoginPasswordProviderInterface')->reveal(); + $providers = ['a' => $a, 'b' => $b]; + + $service = new Service($providers); + + $this->assertEquals(['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/PHPCensor/Security/Authentication/UserProvider/InternalTest.php b/tests/PHPCensor/Security/Authentication/UserProvider/InternalTest.php new file mode 100644 index 00000000..a8984e9f --- /dev/null +++ b/tests/PHPCensor/Security/Authentication/UserProvider/InternalTest.php @@ -0,0 +1,69 @@ +provider = new Internal('internal', [ + 'type' => 'internal', + ]); + } + + /** + * @covers Internal::verifyPassword + */ + public function testVerifyPassword() + { + $user = new User(); + $password = 'bla'; + $user->setHash(password_hash($password, PASSWORD_DEFAULT)); + + $this->assertTrue($this->provider->verifyPassword($user, $password)); + } + + /** + * @covers Internal::verifyPassword + */ + public function testVerifyInvaldPassword() + { + $user = new User(); + $password = 'foo'; + $user->setHash(password_hash($password, PASSWORD_DEFAULT)); + + $this->assertFalse($this->provider->verifyPassword($user, 'bar')); + } + + /** + * @covers Internal::checkRequirements + */ + public function testCheckRequirements() + { + $this->provider->checkRequirements(); + } + + /** + * @covers Internal::provisionUser + */ + public function testProvisionUser() + { + $this->assertNull($this->provider->provisionUser('john@doe.com')); + } +}