From 343036e55c0b41891fc86aafc0cbb3077503ab64 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 4 Jul 2018 00:10:43 +0200 Subject: [PATCH] allow admin to disable fetching of avatars as well as a specific attribute Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/Configuration.php | 40 ++++++++++++++++++++ apps/user_ldap/lib/Connection.php | 10 +++++ apps/user_ldap/lib/User/Manager.php | 9 +++-- apps/user_ldap/lib/User/User.php | 16 +++++--- apps/user_ldap/lib/User_LDAP.php | 6 ++- apps/user_ldap/tests/ConfigurationTest.php | 44 ++++++++++++++++++++-- apps/user_ldap/tests/User/ManagerTest.php | 34 ++++++++--------- 7 files changed, 126 insertions(+), 33 deletions(-) diff --git a/apps/user_ldap/lib/Configuration.php b/apps/user_ldap/lib/Configuration.php index 6240e8d8f5..3871003fae 100644 --- a/apps/user_ldap/lib/Configuration.php +++ b/apps/user_ldap/lib/Configuration.php @@ -35,8 +35,13 @@ namespace OCA\User_LDAP; /** * @property int ldapPagingSize holds an integer + * @property string ldapUserAvatarRule */ class Configuration { + const AVATAR_PREFIX_DEFAULT = 'default'; + const AVATAR_PREFIX_NONE = 'none'; + const AVATAR_PREFIX_DATA_ATTRIBUTE = 'data:'; + protected $configPrefix = null; protected $configRead = false; /** @@ -61,6 +66,7 @@ class Configuration { 'ldapIgnoreNamingRules' => null, 'ldapUserDisplayName' => null, 'ldapUserDisplayName2' => null, + 'ldapUserAvatarRule' => null, 'ldapGidNumber' => null, 'ldapUserFilterObjectclass' => null, 'ldapUserFilterGroups' => null, @@ -472,6 +478,7 @@ class Configuration { 'ldap_experienced_admin' => 0, 'ldap_dynamic_group_member_url' => '', 'ldap_default_ppolicy_dn' => '', + 'ldap_user_avatar_rule' => 'default', ); } @@ -495,6 +502,7 @@ class Configuration { 'ldap_userfilter_groups' => 'ldapUserFilterGroups', 'ldap_userlist_filter' => 'ldapUserFilter', 'ldap_user_filter_mode' => 'ldapUserFilterMode', + 'ldap_user_avatar_rule' => 'ldapUserAvatarRule', 'ldap_login_filter' => 'ldapLoginFilter', 'ldap_login_filter_mode' => 'ldapLoginFilterMode', 'ldap_loginfilter_email' => 'ldapLoginFilterEmail', @@ -536,4 +544,36 @@ class Configuration { return $array; } + /** + * @param string $rule + * @return array + * @throws \RuntimeException + */ + public function resolveRule($rule) { + if($rule === 'avatar') { + return $this->getAvatarAttributes(); + } + throw new \RuntimeException('Invalid rule'); + } + + public function getAvatarAttributes() { + $value = $this->ldapUserAvatarRule ?: self::AVATAR_PREFIX_DEFAULT; + $defaultAttributes = ['jpegphoto', 'thumbnailphoto']; + + if($value === self::AVATAR_PREFIX_NONE) { + return []; + } + if(strpos($value, self::AVATAR_PREFIX_DATA_ATTRIBUTE) === 0) { + $attribute = trim(substr($value, strlen(self::AVATAR_PREFIX_DATA_ATTRIBUTE))); + if($attribute === '') { + return $defaultAttributes; + } + return [$attribute]; + } + if($value !== self::AVATAR_PREFIX_DEFAULT) { + \OC::$server->getLogger()->warning('Invalid config value to ldapUserAvatarRule; falling back to default.'); + } + return $defaultAttributes; + } + } diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php index 977b7c5442..85e6ad6fd9 100644 --- a/apps/user_ldap/lib/Connection.php +++ b/apps/user_ldap/lib/Connection.php @@ -48,6 +48,7 @@ use OCP\ILogger; * @property string ldapUserFilter * @property string ldapUserDisplayName * @property string ldapUserDisplayName2 + * @property string ldapUserAvatarRule * @property boolean turnOnPasswordChange * @property boolean hasPagedResultSupport * @property string[] ldapBaseUsers @@ -169,6 +170,15 @@ class Connection extends LDAPUtility { } } + /** + * @param string $rule + * @return array + * @throws \RuntimeException + */ + public function resolveRule($rule) { + return $this->configuration->resolveRule($rule); + } + /** * sets whether the result of the configuration validation shall * be ignored when establishing the connection. Used by the Wizard diff --git a/apps/user_ldap/lib/User/Manager.php b/apps/user_ldap/lib/User/Manager.php index 55fc7499be..c48193c7ad 100644 --- a/apps/user_ldap/lib/User/Manager.php +++ b/apps/user_ldap/lib/User/Manager.php @@ -163,6 +163,7 @@ class Manager { /** * returns a list of attributes that will be processed further, e.g. quota, * email, displayname, or others. + * * @param bool $minimal - optional, set to true to skip attributes with big * payload * @return string[] @@ -190,10 +191,10 @@ class Manager { if(!$minimal) { // attributes that are not really important but may come with big // payload. - $attributes = array_merge($attributes, array( - 'jpegphoto', - 'thumbnailphoto' - )); + $attributes = array_merge( + $attributes, + $this->access->getConnection()->resolveRule('avatar') + ); } return $attributes; diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index f64c0b4b44..02764a72ec 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -245,10 +245,12 @@ class User { $this->connection->writeToCache($cacheKey, $groups); //Avatar - $attrs = array('jpegphoto', 'thumbnailphoto'); - foreach ($attrs as $attr) { - if(isset($ldapEntry[$attr])) { - $this->avatarImage = $ldapEntry[$attr][0]; + /** @var Connection $connection */ + $connection = $this->access->getConnection(); + $attributes = $connection->resolveRule('avatar'); + foreach ($attributes as $attribute) { + if(isset($ldapEntry[$attribute])) { + $this->avatarImage = $ldapEntry[$attribute][0]; // the call to the method that saves the avatar in the file // system must be postponed after the login. It is to ensure // external mounts are mounted properly (e.g. with login @@ -348,7 +350,9 @@ class User { } $this->avatarImage = false; - $attributes = array('jpegPhoto', 'thumbnailPhoto'); + /** @var Connection $connection */ + $connection = $this->access->getConnection(); + $attributes = $connection->resolveRule('avatar'); foreach($attributes as $attribute) { $result = $this->access->readAttribute($this->dn, $attribute); if($result !== false && is_array($result) && isset($result[0])) { @@ -575,7 +579,7 @@ class User { */ private function setOwnCloudAvatar() { if(!$this->image->valid()) { - $this->log->log('jpegPhoto data invalid for '.$this->dn, ILogger::ERROR); + $this->log->log('avatar image data from LDAP invalid for '.$this->dn, ILogger::ERROR); return false; } //make sure it is a square and not bigger than 128x128 diff --git a/apps/user_ldap/lib/User_LDAP.php b/apps/user_ldap/lib/User_LDAP.php index 1b0c07f0ca..ca7e0b304e 100644 --- a/apps/user_ldap/lib/User_LDAP.php +++ b/apps/user_ldap/lib/User_LDAP.php @@ -103,6 +103,10 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn return $this->userPluginManager->canChangeAvatar($uid); } + if(!$this->implementsActions(Backend::PROVIDE_AVATAR)) { + return true; + } + $user = $this->access->userManager->get($uid); if(!$user instanceof User) { return false; @@ -550,7 +554,7 @@ class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn return (bool)((Backend::CHECK_PASSWORD | Backend::GET_HOME | Backend::GET_DISPLAYNAME - | Backend::PROVIDE_AVATAR + | (($this->access->connection->ldapUserAvatarRule !== 'none') ? Backend::PROVIDE_AVATAR : 0) | Backend::COUNT_USERS | (((int)$this->access->connection->turnOnPasswordChange === 1)? Backend::SET_PASSWORD :0) | $this->userPluginManager->getImplementedActions()) diff --git a/apps/user_ldap/tests/ConfigurationTest.php b/apps/user_ldap/tests/ConfigurationTest.php index 797d2598be..26217ea130 100644 --- a/apps/user_ldap/tests/ConfigurationTest.php +++ b/apps/user_ldap/tests/ConfigurationTest.php @@ -23,7 +23,16 @@ namespace OCA\User_LDAP\Tests; +use OCA\User_LDAP\Configuration; + class ConfigurationTest extends \Test\TestCase { + /** @var Configuration */ + protected $configuration; + + public function setUp() { + parent::setUp(); + $this->configuration = new Configuration('t01', false); + } public function configurationDataProvider() { $inputWithDN = array( @@ -84,6 +93,10 @@ class ConfigurationTest extends \Test\TestCase { // default behaviour, one case is enough, special needs must be tested // individually 'set string value' => array('ldapHost', $inputString, $expectedString), + + 'set avatar rule, default' => ['ldapUserAvatarRule', 'default', 'default'], + 'set avatar rule, none' => ['ldapUserAvatarRule', 'none', 'none'], + 'set avatar rule, data attribute' => ['ldapUserAvatarRule', 'data:jpegPhoto', 'data:jpegPhoto'], ); } @@ -91,10 +104,35 @@ class ConfigurationTest extends \Test\TestCase { * @dataProvider configurationDataProvider */ public function testSetValue($key, $input, $expected) { - $configuration = new \OCA\User_LDAP\Configuration('t01', false); + $this->configuration->setConfiguration([$key => $input]); + $this->assertSame($this->configuration->$key, $expected); + } - $configuration->setConfiguration([$key => $input]); - $this->assertSame($configuration->$key, $expected); + public function avatarRuleValueProvider() { + return [ + ['none', []], + ['data:selfie', ['selfie']], + ['data:', ['jpegphoto', 'thumbnailphoto']], + ['default', ['jpegphoto', 'thumbnailphoto']], + ['invalid#', ['jpegphoto', 'thumbnailphoto']], + ]; + } + + /** + * @dataProvider avatarRuleValueProvider + */ + public function testGetAvatarAttributes($setting, $expected) { + $this->configuration->setConfiguration(['ldapUserAvatarRule' => $setting]); + $this->assertSame($expected, $this->configuration->getAvatarAttributes()); + } + + /** + * @dataProvider avatarRuleValueProvider + */ + public function testResolveRule($setting, $expected) { + $this->configuration->setConfiguration(['ldapUserAvatarRule' => $setting]); + // so far the only thing that can get resolved :) + $this->assertSame($expected, $this->configuration->resolveRule('avatar')); } } diff --git a/apps/user_ldap/tests/User/ManagerTest.php b/apps/user_ldap/tests/User/ManagerTest.php index da30b210b1..5399aa95a6 100644 --- a/apps/user_ldap/tests/User/ManagerTest.php +++ b/apps/user_ldap/tests/User/ManagerTest.php @@ -238,7 +238,17 @@ class ManagerTest extends \Test\TestCase { $this->assertNull($user); } - public function testGetAttributesAll() { + public function attributeRequestProvider() { + return [ + [ false ], + [ true ], + ]; + } + + /** + * @dataProvider attributeRequestProvider + */ + public function testGetAttributes($minimal) { list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) = $this->getTestInstances(); @@ -246,28 +256,14 @@ class ManagerTest extends \Test\TestCase { $manager->setLdapAccess($access); $connection = $access->getConnection(); - $connection->setConfiguration(array('ldapEmailAttribute' => 'mail')); + $connection->setConfiguration(['ldapEmailAttribute' => 'mail', 'ldapUserAvatarRule' => 'default']); - $attributes = $manager->getAttributes(); + $attributes = $manager->getAttributes($minimal); $this->assertTrue(in_array('dn', $attributes)); $this->assertTrue(in_array($access->getConnection()->ldapEmailAttribute, $attributes)); - $this->assertTrue(in_array('jpegphoto', $attributes)); - $this->assertTrue(in_array('thumbnailphoto', $attributes)); - } - - public function testGetAttributesMinimal() { - list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) = - $this->getTestInstances(); - - $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc, $userMgr, $notiMgr); - $manager->setLdapAccess($access); - - $attributes = $manager->getAttributes(true); - - $this->assertTrue(in_array('dn', $attributes)); - $this->assertTrue(!in_array('jpegphoto', $attributes)); - $this->assertTrue(!in_array('thumbnailphoto', $attributes)); + $this->assertSame(!$minimal, in_array('jpegphoto', $attributes)); + $this->assertSame(!$minimal, in_array('thumbnailphoto', $attributes)); } }