From 3d130260843de42d58caa26144e32eb63ee4941a Mon Sep 17 00:00:00 2001 From: Taras Machyshyn Date: Fri, 29 Jul 2016 17:53:49 +0300 Subject: [PATCH 1/2] LDAP fixes --- .../Espo/Core/Utils/Authentication/LDAP.php | 118 +++++++++------ .../Core/Utils/Authentication/LDAP/LDAP.php | 141 ------------------ .../Core/Utils/Authentication/LDAP/Utils.php | 22 ++- application/Espo/Core/defaults/config.php | 6 +- .../Espo/Resources/i18n/en_US/Settings.json | 6 +- .../layouts/Settings/authentication.json | 10 +- .../metadata/entityDefs/Settings.json | 25 ++-- client/src/views/admin/authentication.js | 4 +- 8 files changed, 116 insertions(+), 216 deletions(-) delete mode 100644 application/Espo/Core/Utils/Authentication/LDAP/LDAP.php diff --git a/application/Espo/Core/Utils/Authentication/LDAP.php b/application/Espo/Core/Utils/Authentication/LDAP.php index b07939670d..7105719fae 100644 --- a/application/Espo/Core/Utils/Authentication/LDAP.php +++ b/application/Espo/Core/Utils/Authentication/LDAP.php @@ -28,16 +28,18 @@ ************************************************************************/ namespace Espo\Core\Utils\Authentication; -use Espo\Core\Exceptions\Error, - Espo\Core\Utils\Config, - Espo\Core\ORM\EntityManager, - Espo\Core\Utils\Auth; + +use Espo\Core\Exceptions\Error; +use Espo\Core\Utils\Config; +use Espo\Core\ORM\EntityManager; +use Espo\Core\Utils\Auth; +use Zend\Ldap\Ldap as LdapClient; class LDAP extends Base { private $utils; - private $zendLdap; + private $ldapClient; /** * Espo => LDAP name @@ -50,27 +52,17 @@ class LDAP extends Base { parent::__construct($config, $entityManager, $auth); - $this->fields = array( - 'userName' => $this->getConfig()->get('ldapUserNameAttribute'), - 'firstName' => $this->getConfig()->get('ldapUserFirstNameAttribute'), - 'lastName' => $this->getConfig()->get('ldapUserLastNameAttribute'), - 'title' => $this->getConfig()->get('ldapUserTitleAttribute'), - 'emailAddress' => $this->getConfig()->get('ldapUserEmailAddressAttribute'), - 'phoneNumber' => $this->getConfig()->get('ldapUserPhoneNumberAttribute') - ); - $this->utils = new LDAP\Utils($config); - try { - $this->zendLdap = new LDAP\LDAP(); - } catch (\Exception $e) { - $GLOBALS['log']->error('LDAP Authentication error: ' . $e->getMessage()); - } - } - - protected function getZendLdap() - { - return $this->zendLdap; + $options = $this->getUtils()->getOptions(); + $this->fields = array( + 'userName' => $options['userNameAttribute'], + 'firstName' => $options['userTitleAttribute'], + 'lastName' => $options['userFirstNameAttribute'], + 'title' => $options['userLastNameAttribute'], + 'emailAddress' => $options['userEmailAddressAttribute'], + 'phoneNumber' => $options['userPhoneNumberAttribute'] + ); } protected function getUtils() @@ -78,6 +70,21 @@ class LDAP extends Base return $this->utils; } + protected function getLdapClient() + { + if (!isset($this->ldapClient)) { + $options = $this->getUtils()->getLdapClientOptions(); + + try { + $this->ldapClient = new LdapClient($options); + } catch (\Exception $e) { + $GLOBALS['log']->error('LDAP error: ' . $e->getMessage()); + } + } + + return $this->ldapClient; + } + /** * LDAP login * @@ -92,32 +99,35 @@ class LDAP extends Base return $this->loginByToken($username, $authToken); } - $options = $this->getUtils()->getZendOptions(); + $ldapClient = $this->getLdapClient(); - $ldap = $this->getZendLdap(); - $ldap = $ldap->setOptions($options); + //login LDAP admin user (ldapUsername, ldapPassword) + try { + $ldapClient->bind(); + } catch (\Exception $e) { + $options = $this->getUtils()->getLdapClientOptions(); + $GLOBALS['log']->error('LDAP: Authentication failed for user ['.$options['username'].'], details: ' . $e->getMessage()); + return; + } + + $userDn = $this->findLdapUserDnByUsername($username); + $GLOBALS['log']->debug('Found DN for ['.$username.']: ['.$userDn.'].'); + if (!isset($userDn)) { + $GLOBALS['log']->error('LDAP: Authentication failed for user ['.$username.'], details: user is not found.'); + return; + } try { - $ldap->bind($username, $password); - - $dn = $ldap->getDn($username); - $GLOBALS['log']->debug('Found DN for ['.$username.']: ['.$dn.']'); - - $loginFilter = $this->getUtils()->getOption('userLoginFilter'); - $GLOBALS['log']->debug('Found loginFilter: ['.$loginFilter.']'); - - $userData = $ldap->searchByLoginFilter($loginFilter, $dn, 3); - $GLOBALS['log']->debug('Found userData ... '); - - } catch (\Zend\Ldap\Exception\LdapException $zle) { + $ldapClient->bind($userDn, $password); + } catch (\Exception $e) { $admin = $this->adminLogin($username, $password); if (!isset($admin)) { - $GLOBALS['log']->error('LDAP Authentication: ' . $zle->getMessage()); + $GLOBALS['log']->error('LDAP: Authentication failed for user ['.$username.'], details: ' . $e->getMessage()); return null; } - $GLOBALS['log']->info('LDAP Authentication: Administrator login by username ['.$username.']'); + $GLOBALS['log']->info('LDAP: Administrator ['.$username.'] was logged in by Espo method.'); } $user = $this->getEntityManager()->getRepository('User')->findOne(array( @@ -129,6 +139,8 @@ class LDAP extends Base $isCreateUser = $this->getUtils()->getOption('createEspoUser'); if (!isset($user) && $isCreateUser) { $this->getAuth()->useNoAuth(); /** Required to fix Acl "isFetched()" error */ + + $userData = $ldapClient->getEntry($userDn); $user = $this->createUser($userData); } @@ -200,11 +212,12 @@ class LDAP extends Base $data = array(); // show full array of the LDAP user - $GLOBALS['log']->debug(print_r($userData, true)); + $GLOBALS['log']->debug('LDAP: user data: ' .print_r($userData, true)); foreach ($this->fields as $espo => $ldap) { + $ldap = strtolower($ldap); if (isset($userData[$ldap][0])) { - $GLOBALS['log']->debug('LDAP, createUser: ['.$espo.'] - ['.$userData[$ldap][0].']'); + $GLOBALS['log']->debug('LDAP: Create a user wtih ['.$espo.'] = ['.$userData[$ldap][0].'].'); $data[$espo] = $userData[$ldap][0]; } } @@ -217,4 +230,23 @@ class LDAP extends Base return $user; } + /** + * Find LDAP user DN by his username + * + * @param string $username + * + * @return string | null + */ + protected function findLdapUserDnByUsername($username) + { + $ldapClient = $this->getLdapClient(); + $options = $this->getUtils()->getOptions(); + + $result = $ldapClient->search('(&(objectClass=user)('.$options['userNameAttribute'].'='.$username.'))', null, LdapClient::SEARCH_SCOPE_ONE); + + foreach ($result as $item) { + return $item["dn"]; + } + } + } \ No newline at end of file diff --git a/application/Espo/Core/Utils/Authentication/LDAP/LDAP.php b/application/Espo/Core/Utils/Authentication/LDAP/LDAP.php deleted file mode 100644 index 882c65742d..0000000000 --- a/application/Espo/Core/Utils/Authentication/LDAP/LDAP.php +++ /dev/null @@ -1,141 +0,0 @@ -userNameAttribute = $name; - } - - /** - * Get DN depends on options, ex. "cn=test,ou=People,dc=maxcrc,dc=com" - * - * @return string DN format - */ - public function getDn($acctname) - { - return $this->getAccountDn($acctname, \Zend\Ldap\Ldap::ACCTNAME_FORM_DN); - } - - /** - * Fix a bug, ex. CN=Alice Baker,CN=Users,DC=example,DC=com - * - * @param string $acctname - * @return string - Account DN - */ - protected function getAccountDn($acctname) - { - $baseDn = $this->getBaseDn(); - - if ($this->getBindRequiresDn() && isset($baseDn)) { - try { - return parent::getAccountDn($acctname); - } catch (\Zend\Ldap\Exception\LdapException $zle) { - /*if ($zle->getCode() != \Zend\Ldap\Exception\LdapException::LDAP_NO_SUCH_OBJECT) { - throw $zle; - }*/ - } - - $acctname = $this->userNameAttribute . '=' . \Zend\Ldap\Filter\AbstractFilter::escapeValue($acctname) . ',' . $baseDn; - } - - return parent::getAccountDn($acctname); - } - - /** - * Search a user using userLoginFilter - * - * @param string $filter - * @param string $basedn - * @param int $scope - * @param array $attributes - * @return array - */ - public function searchByLoginFilter($filter, $basedn = null, $scope = self::SEARCH_SCOPE_SUB, array $attributes = array()) - { - $filter = $this->getLoginFilter($filter); - - $result = $this->search($filter, $basedn, $scope, $attributes); - - if ($result->count() > 0) { - return $result->getFirst(); - } - - throw new \Zend\Ldap\Exception\LdapException($this, 'searching: ' . $filter); - } - - /** - * Get login filter in LDAP format - * - * @param string $filter - * @return string - */ - protected function getLoginFilter($filter) - { - $baseFilter = '(objectClass=*)'; - - if (!empty($filter)) { - $baseFilter = '(&' . $baseFilter . $this->convertToFilterFormat($filter). ')'; - } - - return $baseFilter; - } - - /** - * Check and convert filter item in LDAP format - * - * @param string $filter [description] - * @return string - */ - protected function convertToFilterFormat($filter) - { - $filter = trim($filter); - if (substr($filter, 0, 1) != '(') { - $filter = '(' . $filter; - } - - if (substr($filter, -1) != ')') { - $filter = $filter . ')'; - } - - return $filter; - } -} \ No newline at end of file diff --git a/application/Espo/Core/Utils/Authentication/LDAP/Utils.php b/application/Espo/Core/Utils/Authentication/LDAP/Utils.php index 3a74248a96..dfb5b42a7b 100644 --- a/application/Espo/Core/Utils/Authentication/LDAP/Utils.php +++ b/application/Espo/Core/Utils/Authentication/LDAP/Utils.php @@ -57,7 +57,12 @@ class Utils 'tryUsernameSplit' => 'ldapTryUsernameSplit', 'networkTimeout' => 'ldapNetworkTimeout', 'createEspoUser' => 'ldapCreateEspoUser', - 'userLoginFilter' => 'ldapUserLoginFilter', + 'userNameAttribute' => 'ldapUserNameAttribute', + 'userTitleAttribute' => 'ldapUserTitleAttribute', + 'userFirstNameAttribute' => 'ldapUserFirstNameAttribute', + 'userLastNameAttribute' => 'ldapUserLastNameAttribute', + 'userEmailAddressAttribute' => 'ldapUserEmailAddressAttribute', + 'userPhoneNumberAttribute' => 'ldapUserPhoneNumberAttribute', ); /** @@ -66,8 +71,13 @@ class Utils * @var array */ protected $permittedEspoOptions = array( - 'createEspoUser' => false, - 'userLoginFilter' => null, + 'createEspoUser', + 'userNameAttribute', + 'userTitleAttribute', + 'userFirstNameAttribute', + 'userLastNameAttribute', + 'userEmailAddressAttribute', + 'userPhoneNumberAttribute', ); /** @@ -148,12 +158,10 @@ class Utils * * @return array */ - public function getZendOptions() + public function getLdapClientOptions() { $options = $this->getOptions(); - $espoOptions = array_keys($this->permittedEspoOptions); - - $zendOptions = array_diff_key($options, array_flip($espoOptions)); + $zendOptions = array_diff_key($options, array_flip($this->permittedEspoOptions)); return $zendOptions; } diff --git a/application/Espo/Core/defaults/config.php b/application/Espo/Core/defaults/config.php index 7db37cf787..1c7c9c661f 100644 --- a/application/Espo/Core/defaults/config.php +++ b/application/Espo/Core/defaults/config.php @@ -151,11 +151,11 @@ return array ( ], "calendarEntityList" => ["Meeting", "Call", "Task"], 'isInstalled' => false, - 'ldapUserNameAttribute' => 'samaccountname', - 'ldapUserFirstNameAttribute' => 'givenname', + 'ldapUserNameAttribute' => 'userPrincipalName', + 'ldapUserFirstNameAttribute' => 'givenName', 'ldapUserLastNameAttribute' => 'sn', 'ldapUserTitleAttribute' => 'title', 'ldapUserEmailAddressAttribute' => 'mail', - 'ldapUserPhoneNumberAttribute' => 'telephonenumber', + 'ldapUserPhoneNumberAttribute' => 'telephoneNumber', ); diff --git a/application/Espo/Resources/i18n/en_US/Settings.json b/application/Espo/Resources/i18n/en_US/Settings.json index 300e090ba3..e6ab56f767 100644 --- a/application/Espo/Resources/i18n/en_US/Settings.json +++ b/application/Espo/Resources/i18n/en_US/Settings.json @@ -33,10 +33,10 @@ "ldapHost": "Host", "ldapPort": "Port", "ldapAuth": "Auth", - "ldapUsername": "Username", + "ldapUsername": "Full User DN", "ldapPassword": "Password", - "ldapBindRequiresDn": "Bind Requires Dn", - "ldapBaseDn": "Base Dn", + "ldapBindRequiresDn": "Bind Requires DN", + "ldapBaseDn": "Base DN", "ldapAccountCanonicalForm": "Account Canonical Form", "ldapAccountDomainName": "Account Domain Name", "ldapTryUsernameSplit": "Try Username Split", diff --git a/application/Espo/Resources/layouts/Settings/authentication.json b/application/Espo/Resources/layouts/Settings/authentication.json index f96c7f3b8d..267de6f5c8 100644 --- a/application/Espo/Resources/layouts/Settings/authentication.json +++ b/application/Espo/Resources/layouts/Settings/authentication.json @@ -12,16 +12,16 @@ "rows": [ [{"name": "ldapHost"}, {"name": "ldapPort"}], [{"name": "ldapAuth"}, {"name": "ldapSecurity"}], - [{"name": "ldapUsername"}, false], + [{"name": "ldapUsername", "fullWidth": true}], [{"name": "ldapPassword"}, false], - [{"name": "ldapBindRequiresDn"}, {"name": "ldapUserLoginFilter"}], - [{"name": "ldapBaseDn"}, false], - [{"name": "ldapAccountCanonicalForm"}, false], + [{"name": "ldapUserNameAttribute"}, false], + [{"name": "ldapAccountCanonicalForm"}, {"name": "ldapBindRequiresDn"}], + [{"name": "ldapBaseDn", "fullWidth": true}], [{"name": "ldapAccountDomainName"}, {"name": "ldapAccountDomainNameShort"}], [{"name": "ldapTryUsernameSplit"}, {"name": "ldapOptReferrals"}], [{"name": "ldapCreateEspoUser"}, false], - [{"name": "ldapUserNameAttribute"}, {"name": "ldapUserTitleAttribute"}], [{"name": "ldapUserFirstNameAttribute"}, {"name": "ldapUserLastNameAttribute"}], + [{"name": "ldapUserTitleAttribute"}, false], [{"name": "ldapUserEmailAddressAttribute"}, {"name": "ldapUserPhoneNumberAttribute"}] ] } diff --git a/application/Espo/Resources/metadata/entityDefs/Settings.json b/application/Espo/Resources/metadata/entityDefs/Settings.json index dffb98f279..3b9599280e 100644 --- a/application/Espo/Resources/metadata/entityDefs/Settings.json +++ b/application/Espo/Resources/metadata/entityDefs/Settings.json @@ -154,9 +154,7 @@ "required": true }, "ldapPort": { - "type": "int", - "max": 999999, - "min": 0, + "type": "varchar", "default": 389 }, "ldapSecurity": { @@ -179,9 +177,6 @@ "ldapBaseDn": { "type": "varchar" }, - "ldapUserLoginFilter": { - "type": "varchar" - }, "ldapAccountCanonicalForm": { "type": "enum", "options": ["Dn", "Username", "Backslash", "Principal"] @@ -206,22 +201,28 @@ "default": true }, "ldapUserNameAttribute": { - "type": "varchar" + "type": "varchar", + "required": true }, "ldapUserFirstNameAttribute": { - "type": "varchar" + "type": "varchar", + "required": true }, "ldapUserLastNameAttribute": { - "type": "varchar" + "type": "varchar", + "required": true }, "ldapUserTitleAttribute": { - "type": "varchar" + "type": "varchar", + "required": true }, "ldapUserEmailAddressAttribute": { - "type": "varchar" + "type": "varchar", + "required": true }, "ldapUserPhoneNumberAttribute": { - "type": "varchar" + "type": "varchar", + "required": true }, "exportDisabled": { "type": "bool", diff --git a/client/src/views/admin/authentication.js b/client/src/views/admin/authentication.js index c1218cc32b..783025168d 100644 --- a/client/src/views/admin/authentication.js +++ b/client/src/views/admin/authentication.js @@ -76,14 +76,14 @@ Espo.define('views/admin/authentication', 'views/settings/record/edit', function true: [ { action: 'show', - fields: ['ldapUserNameAttribute', 'ldapUserTitleAttribute', 'ldapUserFirstNameAttribute', 'ldapUserLastNameAttribute', 'ldapUserEmailAddressAttribute', 'ldapUserPhoneNumberAttribute'] + fields: ['ldapUserTitleAttribute', 'ldapUserFirstNameAttribute', 'ldapUserLastNameAttribute', 'ldapUserEmailAddressAttribute', 'ldapUserPhoneNumberAttribute'] } ] }, default: [ { action: 'hide', - fields: ['ldapUserNameAttribute', 'ldapUserTitleAttribute', 'ldapUserFirstNameAttribute', 'ldapUserLastNameAttribute', 'ldapUserEmailAddressAttribute', 'ldapUserPhoneNumberAttribute'] + fields: ['ldapUserTitleAttribute', 'ldapUserFirstNameAttribute', 'ldapUserLastNameAttribute', 'ldapUserEmailAddressAttribute', 'ldapUserPhoneNumberAttribute'] } ] }, From 6c2cd93826112339c8d287dd90d0c91867ea3376 Mon Sep 17 00:00:00 2001 From: Taras Machyshyn Date: Mon, 1 Aug 2016 11:12:26 +0300 Subject: [PATCH 2/2] Default config --- application/Espo/Core/defaults/config.php | 6 ------ application/Espo/Core/defaults/systemConfig.php | 6 ++++++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/application/Espo/Core/defaults/config.php b/application/Espo/Core/defaults/config.php index d0296753bb..96b6068b8a 100644 --- a/application/Espo/Core/defaults/config.php +++ b/application/Espo/Core/defaults/config.php @@ -152,11 +152,5 @@ return array ( ], "calendarEntityList" => ["Meeting", "Call", "Task"], 'isInstalled' => false, - 'ldapUserNameAttribute' => 'userPrincipalName', - 'ldapUserFirstNameAttribute' => 'givenName', - 'ldapUserLastNameAttribute' => 'sn', - 'ldapUserTitleAttribute' => 'title', - 'ldapUserEmailAddressAttribute' => 'mail', - 'ldapUserPhoneNumberAttribute' => 'telephoneNumber', ); diff --git a/application/Espo/Core/defaults/systemConfig.php b/application/Espo/Core/defaults/systemConfig.php index c91450185f..13d031acfe 100644 --- a/application/Espo/Core/defaults/systemConfig.php +++ b/application/Espo/Core/defaults/systemConfig.php @@ -139,5 +139,11 @@ return array ( 'defaultPermissions' => 'authTokenMaxIdleTime' ), 'isInstalled' => false, + 'ldapUserNameAttribute' => 'sAMAccountName', + 'ldapUserFirstNameAttribute' => 'givenName', + 'ldapUserLastNameAttribute' => 'sn', + 'ldapUserTitleAttribute' => 'title', + 'ldapUserEmailAddressAttribute' => 'mail', + 'ldapUserPhoneNumberAttribute' => 'telephoneNumber', );