[Pkg-owncloud-commits] [owncloud] 03/79: Backport of #17464, fix uncaught exception on not permitted file types when setting avatar

David Prévot taffit at moszumanska.debian.org
Tue Sep 1 20:55:32 UTC 2015


This is an automated email from the git hooks/post-receive script.

taffit pushed a commit to branch stable8
in repository owncloud.

commit 9bfbed5e0d725d6a5040efcc9ccf6d5fcf18a57a
Author: Arthur Schiwon <blizzz at owncloud.com>
Date:   Mon Jul 6 23:07:33 2015 +0200

    Backport of #17464, fix uncaught exception on not permitted file types when setting avatar
    
    refactor integration tests to make it easier to add new ones
    
    add integration tests for avatar update
    
    fix uncaught exception on not permitted file types when setting avatar, fixes #17232
    
    fix test after rebasing
---
 apps/user_ldap/lib/user/user.php                   |   8 +-
 .../tests/integration/abstractintegrationtest.php  | 137 +++++++++++++++++++++
 .../tests/integration/data/avatar-invalid.gif      | Bin 0 -> 48702 bytes
 .../tests/integration/data/avatar-valid.jpg        | Bin 0 -> 95495 bytes
 apps/user_ldap/tests/integration/fakemanager.php   |  33 +++++
 .../lib/IntegrationTestAccessGroupsMatchFilter.php | 128 +++++--------------
 .../lib/user/IntegrationTestUserAvatar.php         | 128 +++++++++++++++++++
 apps/user_ldap/tests/integration/run-test.sh       |   2 +-
 .../setup-scripts/createExplicitUsers.php          |   2 +-
 9 files changed, 340 insertions(+), 98 deletions(-)

diff --git a/apps/user_ldap/lib/user/user.php b/apps/user_ldap/lib/user/user.php
index 54ee7a9..ac5d8f5 100644
--- a/apps/user_ldap/lib/user/user.php
+++ b/apps/user_ldap/lib/user/user.php
@@ -342,7 +342,13 @@ class User {
 		}
 
 		$avatar = $this->avatarManager->getAvatar($this->uid);
-		$avatar->set($this->image);
+		try {
+			$avatar->set($this->image);
+		} catch (\Exception $e) {
+			\OC::$server->getLogger()->notice(
+				'Could not set avatar for ' . $this->dn	. ', because: ' . $e->getMessage(),
+				['app' => 'user_ldap']);
+		}
 	}
 
 }
diff --git a/apps/user_ldap/tests/integration/abstractintegrationtest.php b/apps/user_ldap/tests/integration/abstractintegrationtest.php
new file mode 100644
index 0000000..f0f5e2d
--- /dev/null
+++ b/apps/user_ldap/tests/integration/abstractintegrationtest.php
@@ -0,0 +1,137 @@
+<?php
+/**
+ * @author Arthur Schiwon <blizzz at owncloud.com>
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCA\user_ldap\tests\integration;
+
+use OCA\user_ldap\lib\Access;
+use OCA\user_ldap\lib\Connection;
+use OCA\user_ldap\lib\LDAP;
+use OCA\user_ldap\lib\user\Manager;
+
+abstract class AbstractIntegrationTest {
+	/** @var  LDAP */
+	protected $ldap;
+
+	/** @var  Connection */
+	protected $connection;
+
+	/** @var Access */
+	protected $access;
+
+	/** @var Manager */
+	protected $userManager;
+
+	/** @var  string */
+	protected $base;
+
+	/** @var string[] */
+	protected $server;
+
+	public function __construct($host, $port, $bind, $pwd, $base) {
+		$this->base = $base;
+		$this->server = [
+			'host' => $host,
+			'port' => $port,
+			'dn'   => $bind,
+			'pwd'  => $pwd
+		];
+	}
+
+	/**
+	 * prepares the LDAP environment and sets up a test configuration for
+	 * the LDAP backend.
+	 */
+	public function init() {
+		$this->initLDAPWrapper();
+		$this->initConnection();
+		$this->initUserManager();
+		$this->initAccess();
+
+	}
+
+	/**
+	 * initializes the test LDAP wrapper
+	 */
+	protected function initLDAPWrapper() {
+		$this->ldap = new LDAP();
+	}
+
+	/**
+	 * sets up the LDAP configuration to be used for the test
+	 */
+	protected function initConnection() {
+		$this->connection = new Connection($this->ldap, '', null);
+		$this->connection->setConfiguration([
+			'ldapHost' => $this->server['host'],
+			'ldapPort' => $this->server['port'],
+			'ldapBase' => $this->base,
+			'ldapAgentName' => $this->server['dn'],
+			'ldapAgentPassword' => $this->server['pwd'],
+			'ldapUserFilter' => 'objectclass=inetOrgPerson',
+			'ldapUserDisplayName' => 'cn',
+			'ldapGroupDisplayName' => 'cn',
+			'ldapLoginFilter' => '(|(uid=%uid)(samaccountname=%uid))',
+			'ldapCacheTTL' => 0,
+			'ldapConfigurationActive' => 1,
+		]);
+	}
+
+	/**
+	 * initializes an LDAP user manager instance
+	 * @return Manager
+	 */
+	protected function initUserManager() {
+		$this->userManager = new FakeManager();
+	}
+
+	/**
+	 * initializes the Access test instance
+	 */
+	protected function initAccess() {
+		$this->access = new Access($this->connection, $this->ldap, $this->userManager);
+	}
+
+	/**
+	 * runs the test cases while outputting progress and result information
+	 *
+	 * If a test failed, the script is exited with return code 1.
+	 */
+	public function run() {
+		$methods = get_class_methods($this);
+		$atLeastOneCaseRan = false;
+		foreach($methods as $method) {
+			if(strpos($method, 'case') === 0) {
+				print("running $method " . PHP_EOL);
+				if(!$this->$method()) {
+					print(PHP_EOL . '>>> !!! Test ' . $method . ' FAILED !!! <<<' . PHP_EOL . PHP_EOL);
+					exit(1);
+				}
+				$atLeastOneCaseRan = true;
+			}
+		}
+		if($atLeastOneCaseRan) {
+			print('Tests succeeded' . PHP_EOL);
+		} else {
+			print('No Test was available.' . PHP_EOL);
+			exit(1);
+		}
+	}
+}
diff --git a/apps/user_ldap/tests/integration/data/avatar-invalid.gif b/apps/user_ldap/tests/integration/data/avatar-invalid.gif
new file mode 100644
index 0000000..0001088
Binary files /dev/null and b/apps/user_ldap/tests/integration/data/avatar-invalid.gif differ
diff --git a/apps/user_ldap/tests/integration/data/avatar-valid.jpg b/apps/user_ldap/tests/integration/data/avatar-valid.jpg
new file mode 100644
index 0000000..61b5ec2
Binary files /dev/null and b/apps/user_ldap/tests/integration/data/avatar-valid.jpg differ
diff --git a/apps/user_ldap/tests/integration/fakemanager.php b/apps/user_ldap/tests/integration/fakemanager.php
new file mode 100644
index 0000000..afc9c55
--- /dev/null
+++ b/apps/user_ldap/tests/integration/fakemanager.php
@@ -0,0 +1,33 @@
+<?php
+/**
+ * @author Arthur Schiwon <blizzz at owncloud.com>
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCA\user_ldap\tests\integration;
+
+/**
+ * Class FakeManager
+ *
+ * this is a mock of \OCA\user_ldap\lib\user\Manager which is a dependency of
+ * Access, that pulls plenty more things in. Because it is not needed in the
+ * scope of these tests, we replace it with a mock.
+ */
+class FakeManager extends \OCA\user_ldap\lib\user\Manager {
+	public function __construct() {}
+}
diff --git a/apps/user_ldap/tests/integration/lib/IntegrationTestAccessGroupsMatchFilter.php b/apps/user_ldap/tests/integration/lib/IntegrationTestAccessGroupsMatchFilter.php
index 92035d9..17d0461 100644
--- a/apps/user_ldap/tests/integration/lib/IntegrationTestAccessGroupsMatchFilter.php
+++ b/apps/user_ldap/tests/integration/lib/IntegrationTestAccessGroupsMatchFilter.php
@@ -1,72 +1,42 @@
 <?php
 /**
- * Created by PhpStorm.
- * User: blizzz
- * Date: 26.06.15
- * Time: 18:13
+ * @author Arthur Schiwon <blizzz at owncloud.com>
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
  */
 
-use OCA\user_ldap\lib\LDAP;
-
-require_once __DIR__  . '/../../../../../lib/base.php';
-
-class IntegrationTestAccessGroupsMatchFilter {
-	/** @var  LDAP */
-	protected $ldap;
+namespace OCA\user_ldap\tests\integration\lib;
 
-	/** @var  \OCA\user_ldap\lib\Connection */
-	protected $connection;
+use OCA\user_ldap\lib\Connection;
+use OCA\user_ldap\tests\integration\AbstractIntegrationTest;
 
-	/** @var \OCA\user_ldap\lib\Access */
-	protected $access;
-
-	/** @var  string */
-	protected $base;
-
-	/** @var string[] */
-	protected $server;
+require_once __DIR__  . '/../../../../../lib/base.php';
 
-	public function __construct($host, $port, $bind, $pwd, $base) {
-		$this->base = $base;
-		$this->server = [
-			'host' => $host,
-			'port' => $port,
-			'dn'   => $bind,
-			'pwd'  => $pwd
-		];
-	}
+class IntegrationTestAccessGroupsMatchFilter extends AbstractIntegrationTest {
 
 	/**
-	 * prepares the LDAP environement and sets up a test configuration for
+	 * prepares the LDAP environment and sets up a test configuration for
 	 * the LDAP backend.
 	 */
 	public function init() {
-		require('setup-scripts/createExplicitUsers.php');
-		require('setup-scripts/createExplicitGroups.php');
-		require('setup-scripts/createExplicitGroupsDifferentOU.php');
-
-		$this->initLDAPWrapper();
-		$this->initConnection();
-		$this->initAccess();
-	}
-
-	/**
-	 * runs the test cases while outputting progress and result information
-	 *
-	 * If a test failed, the script is exited with return code 1.
-	 */
-	public function run() {
-		$cases = ['case1', 'case2', 'case3'];
-
-		foreach ($cases as $case) {
-			print("running $case " . PHP_EOL);
-			if (!$this->$case()) {
-				print(PHP_EOL . '>>> !!! Test ' . $case . ' FAILED !!! <<<' . PHP_EOL . PHP_EOL);
-				exit(1);
-			}
-		}
-
-		print('Tests succeeded' . PHP_EOL);
+		require(__DIR__ . '/../setup-scripts/createExplicitUsers.php');
+		require(__DIR__ . '/../setup-scripts/createExplicitGroups.php');
+		require(__DIR__ . '/../setup-scripts/createExplicitGroupsDifferentOU.php');
+		parent::init();
 	}
 
 	/**
@@ -75,7 +45,7 @@ class IntegrationTestAccessGroupsMatchFilter {
 	 *
 	 * @return bool
 	 */
-	private function case1() {
+	protected function case1() {
 		$this->connection->setConfiguration(['ldapGroupFilter' => 'cn=RedGroup']);
 
 		$dns = ['cn=RedGroup,ou=Groups,' . $this->base];
@@ -89,7 +59,7 @@ class IntegrationTestAccessGroupsMatchFilter {
 	 *
 	 * @return bool
 	 */
-	private function case2() {
+	protected function case2() {
 		$this->connection->setConfiguration(['ldapGroupFilter' => '(|(cn=RedGroup)(cn=PurpleGroup))']);
 
 		$dns = [
@@ -113,7 +83,7 @@ class IntegrationTestAccessGroupsMatchFilter {
 	 *
 	 * @return bool
 	 */
-	private function case3() {
+	protected function case3() {
 		$this->connection->setConfiguration(['ldapGroupFilter' => '(objectclass=groupOfNames)']);
 
 		$dns = [
@@ -132,53 +102,21 @@ class IntegrationTestAccessGroupsMatchFilter {
 	}
 
 	/**
-	 * initializes the Access test instance
-	 */
-	private function initAccess() {
-		$this->access = new \OCA\user_ldap\lib\Access($this->connection, $this->ldap, new FakeManager());
-	}
-
-	/**
-	 * initializes the test LDAP wrapper
-	 */
-	private function initLDAPWrapper() {
-		$this->ldap = new LDAP();
-	}
-
-	/**
 	 * sets up the LDAP configuration to be used for the test
 	 */
-	private function initConnection() {
-		$this->connection = new \OCA\user_ldap\lib\Connection($this->ldap, '', null);
+	protected function initConnection() {
+		parent::initConnection();
 		$this->connection->setConfiguration([
-			'ldapHost' => $this->server['host'],
-			'ldapPort' => $this->server['port'],
-			'ldapBase' => $this->base,
 			'ldapBaseGroups' => 'ou=Groups,' . $this->base,
-			'ldapAgentName' => $this->server['dn'],
-			'ldapAgentPassword' => $this->server['pwd'],
 			'ldapUserFilter' => 'objectclass=inetOrgPerson',
 			'ldapUserDisplayName' => 'displayName',
 			'ldapGroupDisplayName' => 'cn',
 			'ldapLoginFilter' => 'uid=%uid',
-			'ldapCacheTTL' => 0,
-			'ldapConfigurationActive' => 1,
 		]);
 	}
 }
 
-/**
- * Class FakeManager
- *
- * this is a mock of \OCA\user_ldap\lib\user\Manager which is a dependency of
- * Access, that pulls plenty more things in. Because it is not needed in the
- * scope of these tests, we replace it with a mock.
- */
-class FakeManager extends \OCA\user_ldap\lib\user\Manager {
-	public function __construct() {}
-}
-
-require_once('setup-scripts/config.php');
+require_once(__DIR__ . '/../setup-scripts/config.php');
 $test = new IntegrationTestAccessGroupsMatchFilter($host, $port, $adn, $apwd, $bdn);
 $test->init();
 $test->run();
diff --git a/apps/user_ldap/tests/integration/lib/user/IntegrationTestUserAvatar.php b/apps/user_ldap/tests/integration/lib/user/IntegrationTestUserAvatar.php
new file mode 100644
index 0000000..a03d6b0
--- /dev/null
+++ b/apps/user_ldap/tests/integration/lib/user/IntegrationTestUserAvatar.php
@@ -0,0 +1,128 @@
+<?php
+
+use OCA\user_ldap\lib\user\User;
+use OCA\User_LDAP\Mapping\UserMapping;
+use OCA\user_ldap\tests\integration\AbstractIntegrationTest;
+
+require_once __DIR__  . '/../../../../../../lib/base.php';
+
+class IntegrationTestUserAvatar extends AbstractIntegrationTest {
+	/** @var  UserMapping */
+	protected $mapping;
+
+	/**
+	 * prepares the LDAP environment and sets up a test configuration for
+	 * the LDAP backend.
+	 */
+	public function init() {
+		require(__DIR__ . '/../../setup-scripts/createExplicitUsers.php');
+		parent::init();
+		$this->mapping = new UserMapping(\OC::$server->getDatabaseConnection());
+		$this->mapping->clear();
+		$this->access->setUserMapper($this->mapping);
+		$userBackend  = new OCA\user_ldap\USER_LDAP($this->access, \OC::$server->getConfig());
+		\OC_User::useBackend($userBackend);
+	}
+
+	/**
+	 * A method that does the common steps of test cases 1 and 2. The evaluation
+	 * is not happening here.
+	 *
+	 * @param string $dn
+	 * @param string $username
+	 * @param string $image
+	 */
+	private function execFetchTest($dn, $username, $image) {
+		$this->setJpegPhotoAttribute($dn, $image);
+
+		// assigns our self-picked oc username to the dn
+		$this->mapping->map($dn, $username, 'fakeUUID-' . $username);
+
+		// initialize home folder and make sure that the user will update
+		// also remove an possibly existing avatar
+		\OC_Util::tearDownFS();
+		\OC_Util::setupFS($username);
+		\OC::$server->getUserFolder($username);
+		\OC::$server->getConfig()->deleteUserValue($username, 'user_ldap', User::USER_PREFKEY_LASTREFRESH);
+		if(\OC::$server->getAvatarManager()->getAvatar($username)->exists()) {
+			\OC::$server->getAvatarManager()->getAvatar($username)->remove();
+		}
+
+		// finally attempt to get the avatar set
+		$user = $this->userManager->get($dn);
+		$user->updateAvatar();
+	}
+
+	/**
+	 * tests whether an avatar can be retrieved from LDAP and stored correctly
+	 *
+	 * @return bool
+	 */
+	protected function case1() {
+		$image = file_get_contents(__DIR__ . '/../../data/avatar-valid.jpg');
+		$dn = 'uid=alice,ou=Users,' . $this->base;
+		$username = 'alice1337';
+
+		$this->execFetchTest($dn, $username, $image);
+
+		return \OC::$server->getAvatarManager()->getAvatar($username)->exists();
+	}
+
+	/**
+	 * tests whether an image received from LDAP which is of an invalid file
+	 * type is dealt with properly (i.e. not set and not dying).
+	 *
+	 * @return bool
+	 */
+	protected function case2() {
+		// gif by Pmspinner from https://commons.wikimedia.org/wiki/File:Avatar2469_3.gif
+		$image = file_get_contents(__DIR__ . '/../../data/avatar-invalid.gif');
+		$dn = 'uid=boris,ou=Users,' . $this->base;
+		$username = 'boris7844';
+
+		$this->execFetchTest($dn, $username, $image);
+
+		return !\OC::$server->getAvatarManager()->getAvatar($username)->exists();
+	}
+
+	/**
+	 * This writes an image to the 'jpegPhoto' attribute on LDAP.
+	 *
+	 * @param string $dn
+	 * @param string $image An image read via file_get_contents
+	 * @throws \OC\ServerNotAvailableException
+	 */
+	private function setJpegPhotoAttribute($dn, $image) {
+		$changeSet = ['jpegphoto' => $image];
+		ldap_mod_add($this->connection->getConnectionResource(), $dn, $changeSet);
+	}
+
+	protected function initUserManager() {
+		$this->userManager = new \OCA\user_ldap\lib\user\Manager(
+			\OC::$server->getConfig(),
+			new \OCA\user_ldap\lib\FilesystemHelper(),
+			new \OCA\user_ldap\lib\LogWrapper(),
+			\OC::$server->getAvatarManager(),
+			new \OCP\Image(),
+			\OC::$server->getDatabaseConnection()
+		);
+	}
+
+	/**
+	 * sets up the LDAP configuration to be used for the test
+	 */
+	protected function initConnection() {
+		parent::initConnection();
+		$this->connection->setConfiguration([
+			'ldapUserFilter' => 'objectclass=inetOrgPerson',
+			'ldapUserDisplayName' => 'displayName',
+			'ldapGroupDisplayName' => 'cn',
+			'ldapLoginFilter' => 'uid=%uid',
+		]);
+	}
+}
+
+require_once(__DIR__ . '/../../setup-scripts/config.php');
+$test = new IntegrationTestUserAvatar($host, $port, $adn, $apwd, $bdn);
+$test->init();
+$test->run();
diff --git a/apps/user_ldap/tests/integration/run-test.sh b/apps/user_ldap/tests/integration/run-test.sh
index e07e9b4..7a29db2 100755
--- a/apps/user_ldap/tests/integration/run-test.sh
+++ b/apps/user_ldap/tests/integration/run-test.sh
@@ -13,5 +13,5 @@ fi
 
 
 # sleep is necessary, otherwise the LDAP server cannot be connected to, yet.
-setup-scripts/start.sh && sleep 2 && php -f "$TESTSCRIPT"
+setup-scripts/start.sh && sleep 5 && php -f "$TESTSCRIPT"
 setup-scripts/stop.sh
diff --git a/apps/user_ldap/tests/integration/setup-scripts/createExplicitUsers.php b/apps/user_ldap/tests/integration/setup-scripts/createExplicitUsers.php
index ac21d48..bb784d6 100644
--- a/apps/user_ldap/tests/integration/setup-scripts/createExplicitUsers.php
+++ b/apps/user_ldap/tests/integration/setup-scripts/createExplicitUsers.php
@@ -30,7 +30,7 @@ if (true) {
 	}
 }
 
-$users = ['alice'];
+$users = ['alice', 'boris'];
 
 foreach ($users as $uid) {
 	$newDN = 'uid=' . $uid . ',' . $ouDN;

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git



More information about the Pkg-owncloud-commits mailing list