wbrawner/server
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #1729 from nextcloud/downstream-ldap-3

Browse source 
[downstream] LDAP empty hardening
This commit is contained in:
Lukas Reschke 2016-10-18 17:20:15 +02:00 committed by GitHub
commit b8eea5fcab
12 changed files with 108 additions and 90 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
apps/user_ldap/lib/Access.php
View file

@ -184,14 +184,14 @@ class Access extends LDAPUtility implements IUserTools {
$dn = $this->helper->DNasBaseParameter($dn);
$rr = @$this->ldap->read($cr, $dn, $filter, array($attr));
if(!$this->ldap->isResource($rr)) {
if(!empty($attr)) {
if ($attr !== '') {
//do not throw this message on userExists check, irritates
\OCP\Util::writeLog('user_ldap', 'readAttribute failed for DN '.$dn, \OCP\Util::DEBUG);
}
//in case an error occurs , e.g. object does not exist
return false;
}
if (empty($attr) && ($filter === 'objectclass=*' || $this->ldap->countEntries($cr, $rr) === 1)) {
if ($attr === '' && ($filter === 'objectclass=*' || $this->ldap->countEntries($cr, $rr) === 1)) {
\OCP\Util::writeLog('user_ldap', 'readAttribute: '.$dn.' found', \OCP\Util::DEBUG);
return array();
}
@ -422,8 +422,8 @@ class Access extends LDAPUtility implements IUserTools {
}
if($isUser) {
$usernameAttribute = $this->connection->ldapExpertUsernameAttr;
if(!empty($usernameAttribute)) {
$usernameAttribute = strval($this->connection->ldapExpertUsernameAttr);
if ($usernameAttribute !== '') {
$username = $this->readAttribute($fdn, $usernameAttribute);
$username = $username[0];
} else {
@ -1128,7 +1128,7 @@ class Access extends LDAPUtility implements IUserTools {
private function combineFilter($filters, $operator) {
$combinedFilter = '('.$operator;
foreach($filters as $filter) {
if(!empty($filter) && $filter[0] !== '(') {
if ($filter !== '' && $filter[0] !== '(') {
$filter = '('.$filter.')';
}
$combinedFilter.=$filter;
@ -1211,7 +1211,7 @@ class Access extends LDAPUtility implements IUserTools {
$search = $this->prepareSearchTerm($search);
if(!is_array($searchAttributes) || count($searchAttributes) === 0) {
if(empty($fallbackAttribute)) {
if ($fallbackAttribute === '') {
return '';
}
$filter[] = $fallbackAttribute . '=' . $search;
@ -1237,8 +1237,12 @@ class Access extends LDAPUtility implements IUserTools {
$allowEnum = $config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes');
$result = empty($term) ? '*' :
$allowEnum !== 'no' ? $term . '*' : $term;
$result = $term;
if ($term === '') {
$result = '*';
} else if ($allowEnum !== 'no') {
$result = $term . '*';
}
return $result;
}
@ -1285,7 +1289,7 @@ class Access extends LDAPUtility implements IUserTools {
$filter = $this->connection->ldapUserFilter;
$base = $this->connection->ldapBaseUsers;
if($this->connection->ldapUuidUserAttribute === 'auto' && empty($uuidOverride)) {
if ($this->connection->ldapUuidUserAttribute === 'auto' && $uuidOverride === '') {
// Sacrebleu! The UUID attribute is unknown :( We need first an
// existing DN to be able to reliably detect it.
$result = $this->search($filter, $base, ['dn'], 1);
@ -1341,7 +1345,7 @@ class Access extends LDAPUtility implements IUserTools {
return true;
}
if(!empty($uuidOverride) && !$force) {
if ($uuidOverride !== '' && !$force) {
$this->connection->$uuidAttr = $uuidOverride;
return true;
}
@ -1384,7 +1388,7 @@ class Access extends LDAPUtility implements IUserTools {
if($this->detectUuidAttribute($dn, $isUser)) {
$uuid = $this->readAttribute($dn, $this->connection->$uuidAttr);
if( !is_array($uuid)
&& !empty($uuidOverride)
&& $uuidOverride !== ''
&& $this->detectUuidAttribute($dn, $isUser, true)) {
$uuid = $this->readAttribute($dn,
$this->connection->$uuidAttr);

4
apps/user_ldap/lib/Configuration.php
View file

@ -161,7 +161,7 @@ class Configuration {
break;
case 'homeFolderNamingRule':
$trimmedVal = trim($val);
if(!empty($trimmedVal) && strpos($val, 'attr:') === false) {
if ($trimmedVal !== '' && strpos($val, 'attr:') === false) {
$val = 'attr:'.$trimmedVal;
}
break;
@ -309,7 +309,7 @@ class Configuration {
foreach($value as $key => $val) {
if(is_string($val)) {
$val = trim($val);
if(!empty($val)) {
if ($val !== '') {
//accidental line breaks are not wanted and can cause
// odd behaviour. Thus, away with them.
$finalValue[] = $val;

13
apps/user_ldap/lib/Connection.php
View file

@ -137,7 +137,7 @@ class Connection extends LDAPUtility {
$this->configuration->$name = $value;
$after = $this->configuration->$name;
if($before !== $after) {
if(!empty($this->configID)) {
if ($this->configID !== '') {
$this->configuration->saveConfiguration();
}
$this->validateConfiguration();
@ -358,8 +358,8 @@ class Connection extends LDAPUtility {
}
}
$backupPort = $this->configuration->ldapBackupPort;
if(empty($backupPort)) {
$backupPort = intval($this->configuration->ldapBackupPort);
if ($backupPort <= 0) {
$this->configuration->backupPort = $this->configuration->ldapPort;
}
@ -427,7 +427,10 @@ class Connection extends LDAPUtility {
//combinations
$agent = $this->configuration->ldapAgentName;
$pwd = $this->configuration->ldapAgentPassword;
if((empty($agent) && !empty($pwd)) || (!empty($agent) && empty($pwd))) {
if (
($agent === '' && $pwd !== '')
|| ($agent !== '' && $pwd === '')
) {
\OCP\Util::writeLog('user_ldap',
$errorStr.'either no password is given for the'.
'user agent or a password is given, but not an'.
@ -568,7 +571,7 @@ class Connection extends LDAPUtility {
* @throws \OC\ServerNotAvailableException
*/
private function doConnect($host, $port) {
if(empty($host)) {
if ($host === '') {
return false;
}
$this->ldapConnectionRes = $this->ldap->connect($host, $port);

11
apps/user_ldap/lib/Group_LDAP.php
View file

@ -360,7 +360,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface {
$filterParts = [];
$filterParts[] = $this->access->getFilterForUserCount();
if(!empty($search)) {
if ($search !== '') {
$filterParts[] = $this->access->getFilterPartForUserSearch($search);
}
$filterParts[] = 'primaryGroupID=' . $groupID;
@ -658,7 +658,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface {
$groupUsers[] = $this->access->dn2username($ldap_users[0]['dn'][0]);
} else {
//we got DNs, check if we need to filter by search or we can give back all of them
if(!empty($search)) {
if ($search !== '') {
if(!$this->access->readAttribute($member,
$this->access->connection->ldapUserDisplayName,
$this->access->getFilterPartForUserSearch($search))) {
@ -714,7 +714,7 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface {
return false;
}
if(empty($search)) {
if ($search === '') {
$groupUsers = count($members) + $primaryUserCount;
$this->access->connection->writeToCache($cacheKey, $groupUsers);
return $groupUsers;
@ -826,9 +826,8 @@ class Group_LDAP extends BackendUtility implements \OCP\GroupInterface {
return array();
}
$search = $this->access->escapeFilterPart($search, true);
$pagingSize = $this->access->connection->ldapPagingSize;
if ((! $this->access->connection->hasPagedResultSupport)
|| empty($pagingSize)) {
$pagingSize = intval($this->access->connection->ldapPagingSize);
if (!$this->access->connection->hasPagedResultSupport || $pagingSize <= 0) {
return $this->getGroupsChunk($search, $limit, $offset);
}
$maxGroups = 100000; // limit max results (just for safety reasons)

27
apps/user_ldap/lib/User/User.php
View file

@ -183,13 +183,13 @@ class User {
$displayName = $displayName2 = '';
$attr = strtolower($this->connection->ldapUserDisplayName);
if(isset($ldapEntry[$attr])) {
$displayName = $ldapEntry[$attr][0];
$displayName = strval($ldapEntry[$attr][0]);
}
$attr = strtolower($this->connection->ldapUserDisplayName2);
if(isset($ldapEntry[$attr])) {
$displayName2 = $ldapEntry[$attr][0];
$displayName2 = strval($ldapEntry[$attr][0]);
}
if(!empty($displayName)) {
if ($displayName !== '') {
$this->composeAndStoreDisplayName($displayName);
$this->access->cacheUserDisplayName(
$this->getUsername(),
@ -261,10 +261,10 @@ class User {
* @throws \Exception
*/
public function getHomePath($valueFromLDAP = null) {
$path = $valueFromLDAP;
$path = strval($valueFromLDAP);
$attr = null;
if( is_null($path)
if (is_null($valueFromLDAP)
&& strpos($this->access->connection->homeFolderNamingRule, 'attr:') === 0
&& $this->access->connection->homeFolderNamingRule !== 'attr:')
{
@ -276,7 +276,7 @@ class User {
}
}
if(!empty($path)) {
if ($path !== '') {
//if attribute's value is an absolute path take this, otherwise append it to data dir
//check for / at the beginning or pattern c:\ resp. c:/
if( '/' !== $path[0]
@ -393,7 +393,8 @@ class User {
* @returns string the effective display name
*/
public function composeAndStoreDisplayName($displayName, $displayName2 = '') {
if(!empty($displayName2)) {
$displayName2 = strval($displayName2);
if($displayName2 !== '') {
$displayName .= ' (' . $displayName2 . ')';
}
$this->store('displayName', $displayName);
@ -432,20 +433,20 @@ class User {
if($this->wasRefreshed('email')) {
return;
}
$email = $valueFromLDAP;
$email = strval($valueFromLDAP);
if(is_null($valueFromLDAP)) {
$emailAttribute = $this->connection->ldapEmailAttribute;
if(!empty($emailAttribute)) {
if ($emailAttribute !== '') {
$aEmail = $this->access->readAttribute($this->dn, $emailAttribute);
if(is_array($aEmail) && (count($aEmail) > 0)) {
$email = $aEmail[0];
$email = strval($aEmail[0]);
}
}
}
if(!is_null($email)) {
if ($email !== '') {
$user = $this->userManager->get($this->uid);
if (!is_null($user)) {
$currentEmail = $user->getEMailAddress();
$currentEmail = strval($user->getEMailAddress());
if ($currentEmail !== $email) {
$user->setEMailAddress($email);
}
@ -470,7 +471,7 @@ class User {
if(is_null($valueFromLDAP)) {
$quotaAttribute = $this->connection->ldapQuotaAttribute;
if(!empty($quotaAttribute)) {
if ($quotaAttribute !== '') {
$aQuota = $this->access->readAttribute($this->dn, $quotaAttribute);
if($aQuota && (count($aQuota) > 0)) {
$quota = $aQuota[0];

6
apps/user_ldap/lib/User_LDAP.php
View file

@ -385,7 +385,7 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
//Check whether the display name is configured to have a 2nd feature
$additionalAttribute = $this->access->connection->ldapUserDisplayName2;
$displayName2 = '';
if(!empty($additionalAttribute)) {
if ($additionalAttribute !== '') {
$displayName2 = $this->access->readAttribute(
$this->access->username2dn($uid),
$additionalAttribute);
@ -398,8 +398,8 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
if($displayName && (count($displayName) > 0)) {
$displayName = $displayName[0];
if(is_array($displayName2) && (count($displayName2) > 0)) {
$displayName2 = $displayName2[0];
if (is_array($displayName2)){
$displayName2 = count($displayName2) > 0 ? $displayName2[0] : '';
}
$user = $this->access->userManager->get($uid);

22
apps/user_ldap/lib/Wizard.php
View file

@ -220,7 +220,7 @@ class Wizard extends LDAPUtility {
}
$attr = $this->configuration->ldapUserDisplayName;
if($attr !== 'displayName' && !empty($attr)) {
if ($attr !== '' && $attr !== 'displayName') {
// most likely not the default value with upper case N,
// verify it still produces a result
$count = intval($this->countUsersWithAttribute($attr, true));
@ -262,7 +262,7 @@ class Wizard extends LDAPUtility {
}
$attr = $this->configuration->ldapEmailAttribute;
if(!empty($attr)) {
if ($attr !== '') {
$count = intval($this->countUsersWithAttribute($attr, true));
if($count > 0) {
return false;
@ -552,7 +552,7 @@ class Wizard extends LDAPUtility {
}
//make sure the use display name is set
$displayName = $this->configuration->ldapGroupDisplayName;
if(empty($displayName)) {
if ($displayName === '') {
$d = $this->configuration->getDefaults();
$this->applyFind('ldap_group_display_name',
$d['ldap_group_display_name']);
@ -576,7 +576,7 @@ class Wizard extends LDAPUtility {
}
//make sure the use display name is set
$displayName = $this->configuration->ldapUserDisplayName;
if(empty($displayName)) {
if ($displayName === '') {
$d = $this->configuration->getDefaults();
$this->applyFind('ldap_display_name', $d['ldap_display_name']);
}
@ -904,7 +904,7 @@ class Wizard extends LDAPUtility {
$er = $this->ldap->firstEntry($cr, $rr);
$attrs = $this->ldap->getAttributes($cr, $er);
$dn = $this->ldap->getDN($cr, $er);
if(empty($dn)) {
if ($dn == false || $dn === '') {
continue;
}
$filterPart = '(memberof=' . $dn . ')';
@ -923,7 +923,7 @@ class Wizard extends LDAPUtility {
if($parts > 1) {
$filter = '(&' . $filter . ')';
}
if(empty($filter)) {
if ($filter === '') {
$filter = '(objectclass=*)';
}
break;
@ -973,7 +973,7 @@ class Wizard extends LDAPUtility {
//fallback
$attr = 'cn';
}
if(!empty($attr)) {
if ($attr !== '') {
$filterUsername = '(' . $attr . $loginpart . ')';
$parts++;
}
@ -1098,8 +1098,10 @@ class Wizard extends LDAPUtility {
$agent = $this->configuration->ldapAgentName;
$pwd = $this->configuration->ldapAgentPassword;
return ( (!empty($agent) && !empty($pwd))
|| (empty($agent) && empty($pwd)));
return
($agent !== '' && $pwd !== '')
|| ($agent === '' && $pwd === '')
;
}
/**
@ -1236,7 +1238,7 @@ class Wizard extends LDAPUtility {
if(is_array($setFeatures) && !empty($setFeatures)) {
//something is already configured? pre-select it.
$this->result->addChange($dbkey, $setFeatures);
} else if($po && !empty($maxEntryObjC)) {
} else if ($po && $maxEntryObjC !== '') {
//pre-select objectclass with most result entries
$maxEntryObjC = str_replace($p, '', $maxEntryObjC);
$this->applyFind($dbkey, $maxEntryObjC);

27
apps/user_ldap/tests/AccessTest.php
View file

@ -27,9 +27,12 @@
namespace OCA\User_LDAP\Tests;
use OCA\User_LDAP\Access;
use OCA\User_LDAP\Connection;
use OCA\User_LDAP\FilesystemHelper;
use OCA\User_LDAP\Helper;
use OCA\User_LDAP\ILDAPWrapper;
use OCA\User_LDAP\LogWrapper;
use OCA\User_LDAP\User\Manager;
use OCP\IAvatarManager;
use OCP\IConfig;
use OCP\IDBConnection;
@ -45,29 +48,21 @@ use OCP\IUserManager;
*/
class AccessTest extends \Test\TestCase {
private function getConnectorAndLdapMock() {
static $conMethods;
static $accMethods;
static $umMethods;
if(is_null($conMethods) || is_null($accMethods)) {
$conMethods = get_class_methods('\OCA\User_LDAP\Connection');
$accMethods = get_class_methods('\OCA\User_LDAP\Access');
$umMethods = get_class_methods('\OCA\User_LDAP\User\Manager');
}
$lw = $this->createMock(ILDAPWrapper::class);
$connector = $this->getMock('\OCA\User_LDAP\Connection',
$conMethods,
array($lw, null, null));
$um = $this->getMock('\OCA\User_LDAP\User\Manager',
$umMethods, array(
$connector = $this->getMockBuilder(Connection::class)
->setConstructorArgs([$lw, null, null])
->getMock();
$um = $this->getMockBuilder(Manager::class)
->setConstructorArgs([
$this->createMock(IConfig::class),
$this->createMock(FilesystemHelper::class),
$this->createMock(LogWrapper::class),
$this->createMock(IAvatarManager::class),
$this->createMock(Image::class),
$this->createMock(IDBConnection::class),
$this->createMock(IUserManager::class)));
$helper = new \OCA\User_LDAP\Helper(\OC::$server->getConfig());
$this->createMock(IUserManager::class)])
->getMock();
$helper = new Helper(\OC::$server->getConfig());
return array($lw, $connector, $um, $helper);
}

6
apps/user_ldap/tests/ConnectionTest.php
View file

@ -47,9 +47,9 @@ class ConnectionTest extends \Test\TestCase {
$this->ldap = $this->createMock(ILDAPWrapper::class);
// we use a mock here to replace the cache mechanism, due to missing DI in LDAP backend.
$this->connection = $this->getMockBuilder('OCA\User_LDAP\Connection')
->setMethods(['getFromCache', 'writeToCache'])
->setConstructorArgs([$this->ldap, '', null])
->getMock();
->setMethods(['getFromCache', 'writeToCache'])
->setConstructorArgs([$this->ldap, '', null])
->getMock();
$this->ldap->expects($this->any())
->method('areLDAPFunctionsAvailable')

14
apps/user_ldap/tests/Group_LDAPTest.php
View file

@ -48,16 +48,18 @@ class Group_LDAPTest extends \Test\TestCase {
$accMethods = get_class_methods('\OCA\User_LDAP\Access');
}
$lw = $this->createMock(ILDAPWrapper::class);
$connector = $this->getMock('\OCA\User_LDAP\Connection',
$conMethods,
array($lw, null, null));
$connector = $this->getMockBuilder('\OCA\User_LDAP\Connection')
->setMethods($conMethods)
->setConstructorArgs([$lw, null, null])
->getMock();
$um = $this->getMockBuilder('\OCA\User_LDAP\User\Manager')
->disableOriginalConstructor()
->getMock();
$helper = new \OCA\User_LDAP\Helper(\OC::$server->getConfig());
$access = $this->getMock('\OCA\User_LDAP\Access',
$accMethods,
array($connector, $lw, $um, $helper));
$access = $this->getMockBuilder('\OCA\User_LDAP\Access')
->setMethods($accMethods)
->setConstructorArgs([$connector, $lw, $um, $helper])
->getMock();
$access->expects($this->any())
->method('getConnection')

18
apps/user_ldap/tests/User/UserTest.php
View file

@ -77,13 +77,19 @@ class UserTest extends \Test\TestCase {
if (is_null($userMgr)) {
$userMgr = $this->createMock(IUserManager::class);
}
$um = $this->getMock('\OCA\User_LDAP\User\Manager',
$umMethods, array($cfMock, $fsMock, $logMock, $avaMgr, $im, $dbc, $userMgr));
$connector = $this->getMock('\OCA\User_LDAP\Connection',
$conMethods, array($lw, null, null));
$um = $this->getMockBuilder('\OCA\User_LDAP\User\Manager')
->setMethods($umMethods)
->setConstructorArgs([$cfMock, $fsMock, $logMock, $avaMgr, $im, $dbc, $userMgr])
->getMock();
$helper = new \OCA\User_LDAP\Helper(\OC::$server->getConfig());
$access = $this->getMock('\OCA\User_LDAP\Access',
$accMethods, array($connector, $lw, $um, $helper));
$connector = $this->getMockBuilder('\OCA\User_LDAP\Connection')
->setMethods($conMethods)
->setConstructorArgs([$lw, null, null])
->getMock();
$access = $this->getMockBuilder('\OCA\User_LDAP\Access')
->setMethods($accMethods)
->setConstructorArgs([$connector, $lw, $um, $helper])
->getMock();
return array($access, $connector);
}

24
apps/user_ldap/tests/WizardTest.php
View file

@ -61,18 +61,24 @@ class WizardTest extends \Test\TestCase {
$accMethods = get_class_methods('\OCA\User_LDAP\Access');
}
$lw = $this->createMock(ILDAPWrapper::class);
$conf = $this->getMock('\OCA\User_LDAP\Configuration',
$confMethods,
array($lw, null, null));
$conf = $this->getMockBuilder('\OCA\User_LDAP\Configuration')
->setMethods($confMethods)
->setConstructorArgs([$lw, null, null])
->getMock();
$connector = $this->getMock('\OCA\User_LDAP\Connection',
$connMethods, array($lw, null, null));
$connector = $this->getMockBuilder('\OCA\User_LDAP\Connection')
->setMethods($connMethods)
->setConstructorArgs([$lw, null, null])
->getMock();
$um = $this->getMockBuilder('\OCA\User_LDAP\User\Manager')
->disableOriginalConstructor()
->getMock();
->disableOriginalConstructor()
->getMock();
$helper = new \OCA\User_LDAP\Helper(\OC::$server->getConfig());
$access = $this->getMock('\OCA\User_LDAP\Access',
$accMethods, array($connector, $lw, $um, $helper));
$access = $this->getMockBuilder('\OCA\User_LDAP\Access')
->setMethods($accMethods)
->setConstructorArgs([$connector, $lw, $um, $helper])
->getMock();
return array(new Wizard($conf, $lw, $access), $conf, $lw, $access);
}