Merge pull request #10115 from nextcloud/backport/10098/ldap-image-attribute

[stable13] allow admin to disable fetching of avatars as well as a specific attribute
This commit is contained in:
Morris Jobke 2018-07-11 11:45:26 +02:00 committed by GitHub
commit 73c1fa6b64
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 239 additions and 50 deletions

View File

@ -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',
@ -535,4 +543,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 [strtolower($attribute)];
}
if($value !== self::AVATAR_PREFIX_DEFAULT) {
\OC::$server->getLogger()->warning('Invalid config value to ldapUserAvatarRule; falling back to default.');
}
return $defaultAttributes;
}
}

View File

@ -47,6 +47,7 @@ use OC\ServerNotAvailableException;
* @property string ldapUserFilter
* @property string ldapUserDisplayName
* @property string ldapUserDisplayName2
* @property string ldapUserAvatarRule
* @property boolean turnOnPasswordChange
* @property boolean hasPagedResultSupport
* @property string[] ldapBaseUsers
@ -168,6 +169,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

View File

@ -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;

View File

@ -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, Util::ERROR);
$this->log->log('avatar image data from LDAP invalid for '.$this->dn, Util::ERROR);
return false;
}
//make sure it is a square and not bigger than 128x128

View File

@ -102,6 +102,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;
@ -549,7 +553,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
| ((intval($this->access->connection->turnOnPasswordChange) === 1)?(Backend::SET_PASSWORD):0)
| $this->userPluginManager->getImplementedActions())

View File

@ -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,36 @@ 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: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'));
}
}

View File

@ -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));
}
}

View File

@ -622,7 +622,7 @@ class UserTest extends \Test\TestCase {
$this->access->expects($this->once())
->method('readAttribute')
->with($this->equalTo('uid=alice,dc=foo,dc=bar'),
$this->equalTo('jpegPhoto'))
$this->equalTo('jpegphoto'))
->will($this->returnValue(array('this is a photo')));
$image->expects($this->once())
@ -657,6 +657,10 @@ class UserTest extends \Test\TestCase {
$uid = 'alice';
$dn = 'uid=alice,dc=foo,dc=bar';
$this->access->connection->expects($this->any())
->method('resolveRule')
->with('avatar')
->willReturn(['jpegphoto', 'thumbnailphoto']);
$user = new User(
$uid, $dn, $this->access, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr);
@ -672,11 +676,11 @@ class UserTest extends \Test\TestCase {
->method('readAttribute')
->willReturnCallback(function($dn, $attr) {
if($dn === 'uid=alice,dc=foo,dc=bar'
&& $attr === 'jpegPhoto')
&& $attr === 'jpegphoto')
{
return false;
} elseif($dn === 'uid=alice,dc=foo,dc=bar'
&& $attr === 'thumbnailPhoto')
&& $attr === 'thumbnailphoto')
{
return ['this is a photo'];
}
@ -715,6 +719,10 @@ class UserTest extends \Test\TestCase {
$uid = 'alice';
$dn = 'uid=alice,dc=foo,dc=bar';
$this->access->connection->expects($this->any())
->method('resolveRule')
->with('avatar')
->willReturn(['jpegphoto', 'thumbnailphoto']);
$user = new User(
$uid, $dn, $this->access, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr);
@ -730,11 +738,11 @@ class UserTest extends \Test\TestCase {
->method('readAttribute')
->willReturnCallback(function($dn, $attr) {
if($dn === $dn
&& $attr === 'jpegPhoto')
&& $attr === 'jpegphoto')
{
return false;
} elseif($dn === $dn
&& $attr === 'thumbnailPhoto')
&& $attr === 'thumbnailphoto')
{
return ['this is a photo'];
}
@ -765,6 +773,10 @@ class UserTest extends \Test\TestCase {
$uid = 'alice';
$dn = 'uid=alice,dc=foo,dc=bar';
$this->access->connection->expects($this->any())
->method('resolveRule')
->with('avatar')
->willReturn(['jpegphoto', 'thumbnailphoto']);
$user = new User(
$uid, $dn, $this->access, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr);
@ -783,11 +795,11 @@ class UserTest extends \Test\TestCase {
->method('readAttribute')
->willReturnCallback(function($dn, $attr) {
if($dn === $dn
&& $attr === 'jpegPhoto')
&& $attr === 'jpegphoto')
{
return false;
} elseif($dn === $dn
&& $attr === 'thumbnailPhoto')
&& $attr === 'thumbnailphoto')
{
return ['this is a photo'];
}
@ -825,6 +837,11 @@ class UserTest extends \Test\TestCase {
->with($this->equalTo($uid))
->will($this->returnValue($avatar));
$this->access->connection->expects($this->any())
->method('resolveRule')
->with('avatar')
->willReturn(['jpegphoto', 'thumbnailphoto']);
$user = new User(
$uid, $dn, $this->access, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr);
@ -867,6 +884,10 @@ class UserTest extends \Test\TestCase {
$uid = 'alice';
$dn = 'uid=alice,dc=foo,dc=bar';
$this->access->connection->expects($this->any())
->method('resolveRule')
->with('avatar')
->willReturn(['jpegphoto', 'thumbnailphoto']);
$user = new User(
$uid, $dn, $this->access, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr);
@ -931,6 +952,10 @@ class UserTest extends \Test\TestCase {
$uid = 'alice';
$dn = 'uid=alice,dc=foo,dc=bar';
$this->access->connection->expects($this->any())
->method('resolveRule')
->with('avatar')
->willReturn(['jpegphoto', 'thumbnailphoto']);
$user = new User(
$uid, $dn, $this->access, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr);
@ -1003,8 +1028,12 @@ class UserTest extends \Test\TestCase {
$this->access->expects($this->once())
->method('readAttribute')
->with($this->equalTo('uid=alice,dc=foo,dc=bar'),
$this->equalTo('jpegPhoto'))
$this->equalTo('jpegphoto'))
->will($this->returnValue(array('this is a photo')));
$this->connection->expects($this->any())
->method('resolveRule')
->with('avatar')
->willReturn(['jpegphoto', 'thumbnailphoto']);
$uid = 'alice';
$dn = 'uid=alice,dc=foo,dc=bar';
@ -1016,7 +1045,28 @@ class UserTest extends \Test\TestCase {
$this->assertSame('this is a photo', $photo);
//make sure readAttribute is not called again but the already fetched
//photo is returned
$photo = $user->getAvatarImage();
$user->getAvatarImage();
}
public function testGetAvatarImageDisabled() {
list(, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr) =
$this->getTestInstances();
$uid = 'alice';
$dn = 'uid=alice,dc=foo,dc=bar';
$this->access->expects($this->never())
->method('readAttribute')
->with($this->equalTo($dn), $this->anything());
$this->access->connection->expects($this->any())
->method('resolveRule')
->with('avatar')
->willReturn([]);
$user = new User(
$uid, $dn, $this->access, $config, $filesys, $image, $log, $avaMgr, $userMgr, $notiMgr);
$this->assertFalse($user->getAvatarImage());
}
public function imageDataProvider() {
@ -1062,16 +1112,20 @@ class UserTest extends \Test\TestCase {
}
return $name;
}));
$this->connection->expects($this->any())
->method('resolveRule')
->with('avatar')
->willReturn(['jpegphoto', 'thumbnailphoto']);
$record = array(
strtolower($this->connection->ldapQuotaAttribute) => array('4096'),
strtolower($this->connection->ldapEmailAttribute) => array('alice@wonderland.org'),
strtolower($this->connection->ldapUserDisplayName) => array('Aaaaalice'),
$record = [
strtolower($this->connection->ldapQuotaAttribute) => ['4096'],
strtolower($this->connection->ldapEmailAttribute) => ['alice@wonderland.org'],
strtolower($this->connection->ldapUserDisplayName) => ['Aaaaalice'],
'uid' => array($uid),
'homedirectory' => array('Alice\'s Folder'),
'memberof' => array('cn=groupOne', 'cn=groupTwo'),
'jpegphoto' => array('here be an image')
);
'homedirectory' => ['Alice\'s Folder'],
'memberof' => ['cn=groupOne', 'cn=groupTwo'],
'jpegphoto' => ['here be an image']
];
foreach($requiredMethods as $method) {
$userMock->expects($this->once())

View File

@ -1663,4 +1663,45 @@ class User_LDAPTest extends TestCase {
$this->assertFalse($ldap->createUser('uid', 'password'));
}
public function actionProvider() {
return [
[ 'ldapUserAvatarRule', 'default', Backend::PROVIDE_AVATAR, true] ,
[ 'ldapUserAvatarRule', 'data:selfiePhoto', Backend::PROVIDE_AVATAR, true],
[ 'ldapUserAvatarRule', 'none', Backend::PROVIDE_AVATAR, false],
[ 'turnOnPasswordChange', 0, Backend::SET_PASSWORD, false],
[ 'turnOnPasswordChange', 1, Backend::SET_PASSWORD, true],
];
}
/**
* @dataProvider actionProvider
*/
public function testImplementsAction($configurable, $value, $actionCode, $expected) {
$pluginManager = $this->createMock(UserPluginManager::class);
$pluginManager->expects($this->once())
->method('getImplementedActions')
->willReturn(0);
$access = $this->getAccessMock();
$access->connection->expects($this->any())
->method('__get')
->willReturnMap([
[$configurable, $value],
]);
$config = $this->createMock(IConfig::class);
$noti = $this->createMock(INotificationManager::class);
$session = $this->createMock(Session::class);
$backend = new User_LDAP(
$access,
$config,
$noti,
$session,
$pluginManager
);
$this->assertSame($expected, $backend->implementsActions($actionCode));
}
}