From f3bdeb2493216de052ef54865debe377817e8b1f Mon Sep 17 00:00:00 2001 From: Dmitry Khomutov Date: Sun, 22 Jan 2017 18:05:24 +0700 Subject: [PATCH] Fixed LDAP authentication + unified app config options: authenticate_settings, security and ldap --- app/config.example.yml | 16 +++- docs/en/configuring.md | 12 +-- src/PHPCensor/Application.php | 10 +-- .../Command/RegisterLdapUserCommand.php | 87 ------------------- src/PHPCensor/Command/RegisterUserCommand.php | 87 ------------------- .../Controller/SessionController.php | 9 +- .../Controller/SettingsController.php | 29 ++++--- src/PHPCensor/Helper/LoginIsDisabled.php | 8 +- ...php => LoginPasswordProviderInterface.php} | 2 +- .../Security/Authentication/Service.php | 19 ++-- .../UserProvider/AbstractProvider.php | 4 +- .../Authentication/UserProvider/Internal.php | 4 +- .../Authentication/UserProvider/Ldap.php | 45 ++++++---- ...Provider.php => UserProviderInterface.php} | 2 +- src/PHPCensor/Service/UserService.php | 6 +- src/PHPCensor/View/Settings/index.phtml | 2 +- .../Security/Authentication/ServiceTest.php | 8 +- 17 files changed, 105 insertions(+), 245 deletions(-) delete mode 100644 src/PHPCensor/Command/RegisterLdapUserCommand.php delete mode 100644 src/PHPCensor/Command/RegisterUserCommand.php rename src/PHPCensor/Security/Authentication/{LoginPasswordProvider.php => LoginPasswordProviderInterface.php} (89%) rename src/PHPCensor/Security/Authentication/{UserProvider.php => UserProviderInterface.php} (96%) 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/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/Command/RegisterLdapUserCommand.php b/src/PHPCensor/Command/RegisterLdapUserCommand.php deleted file mode 100644 index 27cea119..00000000 --- a/src/PHPCensor/Command/RegisterLdapUserCommand.php +++ /dev/null @@ -1,87 +0,0 @@ -userStore = $userStore; - } - - protected function configure() - { - $this - ->setName('php-censor:register-ldap-user') - ->setDescription(Lang::get('register_ldap_user')); - } - - /** - * Creates an admin user in the existing PHPCI database - * - * {@inheritDoc} - */ - protected function execute(InputInterface $input, OutputInterface $output) - { - $userService = new UserService($this->userStore); - - /** @var $dialog \Symfony\Component\Console\Helper\DialogHelper */ - $dialog = $this->getHelperSet()->get('dialog'); - - // Function to validate mail address. - $mailValidator = function ($answer) { - if (!filter_var($answer, FILTER_VALIDATE_EMAIL)) { - throw new \InvalidArgumentException(Lang::get('must_be_valid_email')); - } - - return $answer; - }; - - $email = $dialog->askAndValidate($output, Lang::get('enter_email'), $mailValidator, false); - $name = $dialog->ask($output, Lang::get('enter_name')); - $providerKey = "ldap"; - $providerData = null; - $isAdmin = ($dialog->ask($output, Lang::get('enter_isadmin'))); - $isAdmin = !empty($isAdmin); - $password = ""; - - try { - $userService->createUserWithProvider($name, $email, $password, $providerKey, $providerData, $isAdmin); - $output->writeln(Lang::get('user_created')); - } catch (\Exception $e) { - $output->writeln(sprintf('%s', Lang::get('failed_to_create'))); - $output->writeln(sprintf('%s', $e->getMessage())); - } - } -} diff --git a/src/PHPCensor/Command/RegisterUserCommand.php b/src/PHPCensor/Command/RegisterUserCommand.php deleted file mode 100644 index 2360b4d1..00000000 --- a/src/PHPCensor/Command/RegisterUserCommand.php +++ /dev/null @@ -1,87 +0,0 @@ -userStore = $userStore; - } - - protected function configure() - { - $this - ->setName('php-censor:register-user') - ->setDescription(Lang::get('register_user')); - } - - /** - * Creates an admin user in the existing PHPCI database - * - * {@inheritDoc} - */ - protected function execute(InputInterface $input, OutputInterface $output) - { - $userService = new UserService($this->userStore); - - /** @var $dialog \Symfony\Component\Console\Helper\DialogHelper */ - $dialog = $this->getHelperSet()->get('dialog'); - - // Function to validate mail address. - $mailValidator = function ($answer) { - if (!filter_var($answer, FILTER_VALIDATE_EMAIL)) { - throw new \InvalidArgumentException(Lang::get('must_be_valid_email')); - } - - return $answer; - }; - - $id = $dialog->ask($output, Lang::get('enter_id')); - $password = $dialog->askHiddenResponse($output, Lang::get('enter_password')); - $emailAddress = $dialog->askAndValidate($output, Lang::get('enter_email'), $mailValidator, false); - $providerKey = $dialog->ask($output, Lang::get('enter_providerkey')); - $providerData = $dialog->ask($output, Lang::get('enter_providerdata')); - $isAdmin = $dialog->ask($output, Lang::get('enter_isadmin')); - $isAdmin = !empty($isAdmin); - $name = $dialog->ask($output, Lang::get('enter_name')); - - try { - $userService->createUserWithProvider($name, $emailAddress, $id, $password, $providerKey, $providerData, $isAdmin = false); - $output->writeln(Lang::get('user_created')); - } catch (\Exception $e) { - $output->writeln(sprintf('%s', Lang::get('failed_to_create'))); - $output->writeln(sprintf('%s', $e->getMessage())); - } - } -} diff --git a/src/PHPCensor/Controller/SessionController.php b/src/PHPCensor/Controller/SessionController.php index 7c4807fb..6449ada6 100644 --- a/src/PHPCensor/Controller/SessionController.php +++ b/src/PHPCensor/Controller/SessionController.php @@ -42,8 +42,9 @@ class SessionController extends Controller public function init() { $this->response->disableLayout(); - $this->userStore = b8\Store\Factory::getStore('User'); - $this->authentication = Service::getInstance(); + + $this->userStore = b8\Store\Factory::getStore('User'); + $this->authentication = Service::getInstance(); } /** @@ -60,8 +61,8 @@ class SessionController extends Controller } else { unset($_SESSION['login_token']); - $email = $this->getParam('email'); - $password = $this->getParam('password', ''); + $email = $this->getParam('email'); + $password = $this->getParam('password', ''); $isLoginFailure = true; $user = $this->userStore->getByEmailOrName($email); 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/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/Security/Authentication/LoginPasswordProvider.php b/src/PHPCensor/Security/Authentication/LoginPasswordProviderInterface.php similarity index 89% rename from src/PHPCensor/Security/Authentication/LoginPasswordProvider.php rename to src/PHPCensor/Security/Authentication/LoginPasswordProviderInterface.php index e5a2fc1c..ba0a1896 100644 --- a/src/PHPCensor/Security/Authentication/LoginPasswordProvider.php +++ b/src/PHPCensor/Security/Authentication/LoginPasswordProviderInterface.php @@ -17,7 +17,7 @@ use PHPCensor\Model\User; * * @author Adirelle */ -interface LoginPasswordProvider extends UserProvider +interface LoginPasswordProviderInterface extends UserProviderInterface { /** Verify if the supplied password matches the user's one. * diff --git a/src/PHPCensor/Security/Authentication/Service.php b/src/PHPCensor/Security/Authentication/Service.php index e58be8b8..5dd69cf4 100644 --- a/src/PHPCensor/Security/Authentication/Service.php +++ b/src/PHPCensor/Security/Authentication/Service.php @@ -33,8 +33,12 @@ class Service { if (self::$instance === null) { $config = Config::getInstance()->get( - 'php-censor.security.authentication', - ['internal' => ['type' => 'internal']] + 'php-censor.security.auth_providers', + [ + 'internal' => [ + 'type' => 'internal' + ] + ] ); $providers = []; @@ -49,9 +53,10 @@ class Service /** Create a provider from a given configuration. * - * @param string $key + * @param string $key * @param string|array $config - * @return UserProvider + * + * @return UserProviderInterface */ public static function buildProvider($key, $config) { @@ -80,7 +85,7 @@ class Service /** Return all providers. * - * @return UserProvider[] + * @return UserProviderInterface[] */ public function getProviders() { @@ -89,13 +94,13 @@ class Service /** Return the user providers that allows password authentication. * - * @return LoginPasswordProvider[] + * @return LoginPasswordProviderInterface[] */ public function getLoginPasswordProviders() { $providers = []; foreach ($this->providers as $key => $provider) { - if ($provider instanceof LoginPasswordProvider) { + if ($provider instanceof LoginPasswordProviderInterface) { $providers[$key] = $provider; } } diff --git a/src/PHPCensor/Security/Authentication/UserProvider/AbstractProvider.php b/src/PHPCensor/Security/Authentication/UserProvider/AbstractProvider.php index d9f0835e..02695b1b 100644 --- a/src/PHPCensor/Security/Authentication/UserProvider/AbstractProvider.php +++ b/src/PHPCensor/Security/Authentication/UserProvider/AbstractProvider.php @@ -10,14 +10,14 @@ namespace PHPCensor\Security\Authentication\UserProvider; -use PHPCensor\Security\Authentication\UserProvider; +use PHPCensor\Security\Authentication\UserProviderInterface; /** * Abstract user provider. * * @author Adirelle */ -abstract class AbstractProvider implements UserProvider +abstract class AbstractProvider implements UserProviderInterface { /** * @var string diff --git a/src/PHPCensor/Security/Authentication/UserProvider/Internal.php b/src/PHPCensor/Security/Authentication/UserProvider/Internal.php index 79f8f52d..85d66250 100644 --- a/src/PHPCensor/Security/Authentication/UserProvider/Internal.php +++ b/src/PHPCensor/Security/Authentication/UserProvider/Internal.php @@ -11,14 +11,14 @@ namespace PHPCensor\Security\Authentication\UserProvider; use PHPCensor\Model\User; -use PHPCensor\Security\Authentication\LoginPasswordProvider; +use PHPCensor\Security\Authentication\LoginPasswordProviderInterface; /** * Internal user provider * * @author Adirelle */ -class Internal extends AbstractProvider implements LoginPasswordProvider +class Internal extends AbstractProvider implements LoginPasswordProviderInterface { public function verifyPassword(User $user, $password) diff --git a/src/PHPCensor/Security/Authentication/UserProvider/Ldap.php b/src/PHPCensor/Security/Authentication/UserProvider/Ldap.php index 996fb690..61889f37 100644 --- a/src/PHPCensor/Security/Authentication/UserProvider/Ldap.php +++ b/src/PHPCensor/Security/Authentication/UserProvider/Ldap.php @@ -11,36 +11,44 @@ namespace PHPCensor\Security\Authentication\UserProvider; use b8\Config; +use b8\Store\Factory; use PHPCensor\Model\User; -use PHPCensor\Security\Authentication\LoginPasswordProvider; +use PHPCensor\Security\Authentication\LoginPasswordProviderInterface; +use PHPCensor\Service\UserService; /** * Ldap user provider. * * @author Dmitrii Zolotov (@itherz) */ -class Ldap extends AbstractProvider implements LoginPasswordProvider +class Ldap extends AbstractProvider implements LoginPasswordProviderInterface { - public function verifyPassword(User $user, $password) { - $config = Config::getInstance()->get('php-censor.security.ldap', []); - $server = $config["server"]; - $mailAttribute = $config["mailAttribute"]; - $ldap = ldap_connect($server); + $providers = Config::getInstance()->get('php-censor.security.auth_providers', []); + if ($providers) { + foreach ($providers as $provider) { + if (isset($provider['type']) && 'ldap' === $provider['type']) { + $ldapData = $provider['data']; - ldap_set_option($ldap, LDAP_OPT_PROTOCOL_VERSION, 3); + $ldap = ldap_connect($ldapData['host'], $ldapData['port']); - $ls = ldap_search($ldap, $config["base"], $mailAttribute . "=" . $user->getEmail()); - $le = ldap_get_entries($ldap, $ls); + ldap_set_option($ldap, LDAP_OPT_PROTOCOL_VERSION, 3); - if ($le["count"] == 0) { - return false; + $ls = ldap_search($ldap, $ldapData['base_dn'], $ldapData['mail_attribute'] . '=' . $user->getEmail()); + $le = ldap_get_entries($ldap, $ls); + if (!$le['count']) { + continue; + } + + $dn = $le[0]['dn']; + + return @ldap_bind($ldap, $dn, $password); + } + } } - $dn = $le[0]["dn"]; - - return ldap_bind($ldap, $dn, $password); + return false; } public function checkRequirements() @@ -50,6 +58,11 @@ class Ldap extends AbstractProvider implements LoginPasswordProvider public function provisionUser($identifier) { - return null; + $userService = new UserService(Factory::getStore('User')); + + $parts = explode("@", $identifier); + $username = $parts[0]; + + return $userService->createUserWithProvider($username, $identifier, 'ldap', null); } } diff --git a/src/PHPCensor/Security/Authentication/UserProvider.php b/src/PHPCensor/Security/Authentication/UserProviderInterface.php similarity index 96% rename from src/PHPCensor/Security/Authentication/UserProvider.php rename to src/PHPCensor/Security/Authentication/UserProviderInterface.php index 5a5eab2e..4861a83b 100644 --- a/src/PHPCensor/Security/Authentication/UserProvider.php +++ b/src/PHPCensor/Security/Authentication/UserProviderInterface.php @@ -17,7 +17,7 @@ use PHPCensor\Model\User; * * @author Adirelle */ -interface UserProvider +interface UserProviderInterface { /** Check if all software requirements are met (libraries, extensions, ...) diff --git a/src/PHPCensor/Service/UserService.php b/src/PHPCensor/Service/UserService.php index 75f6cf3f..dfddda6d 100644 --- a/src/PHPCensor/Service/UserService.php +++ b/src/PHPCensor/Service/UserService.php @@ -59,17 +59,17 @@ class UserService /** * Create a new user within PHPCI (with provider). + * * @param $name * @param $emailAddress - * @param $id - * @param $password * @param $providerKey * @param $providerData * @param bool $isAdmin + * * @return \PHPCI\Model\User */ - public function createUserWithProvider($name, $emailAddress, $id, $password, $providerKey, $providerData, $isAdmin = false) + public function createUserWithProvider($name, $emailAddress, $providerKey, $providerData, $isAdmin = false) { $user = new User(); $user->setName($name); 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/Security/Authentication/ServiceTest.php b/tests/PHPCensor/Security/Authentication/ServiceTest.php index 6a6d1244..7e628e93 100644 --- a/tests/PHPCensor/Security/Authentication/ServiceTest.php +++ b/tests/PHPCensor/Security/Authentication/ServiceTest.php @@ -50,8 +50,8 @@ class ServiceTest extends \PHPUnit_Framework_TestCase */ public function testGetProviders() { - $a = $this->prophesize('\PHPCensor\Security\Authentication\UserProvider')->reveal(); - $b = $this->prophesize('\PHPCensor\Security\Authentication\UserProvider')->reveal(); + $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); @@ -64,8 +64,8 @@ class ServiceTest extends \PHPUnit_Framework_TestCase */ public function testGetLoginPasswordProviders() { - $a = $this->prophesize('\PHPCensor\Security\Authentication\UserProvider')->reveal(); - $b = $this->prophesize('\PHPCensor\Security\Authentication\LoginPasswordProvider')->reveal(); + $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);