Merge pull request #17255 from owncloud/fix-17119

[LDAP] Filter user groups obtained by memberof

9 files changed, 389 insertions, 1 deletions

diff --git a/apps/user_ldap/group_ldap.php b/apps/user_ldap/group_ldap.php
index 0395a4a80e3..a7a90c75832 100644
--- a/apps/user_ldap/group_ldap.php
+++ b/apps/user_ldap/group_ldap.php
@@ -378,9 +378,11 @@ class GROUP_LDAP extends BackendUtility implements \OCP\GroupInterface {
 			&& intval($this->access->connection->useMemberOfToDetectMembership) === 1
 		) {
 			$groupDNs = $this->access->readAttribute($userDN, 'memberOf');
+
 			if (is_array($groupDNs)) {
+				$groupDNs = $this->access->groupsMatchFilter($groupDNs);
 				foreach ($groupDNs as $dn) {
-					$groups[] = $this->access->dn2groupname($dn);;
+					$groups[] = $this->access->dn2groupname($dn);
 				}
 			}
 			if($primaryGroup !== false) {
diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php
index f38d11d4be3..b201220d725 100644
--- a/apps/user_ldap/lib/access.php
+++ b/apps/user_ldap/lib/access.php
@@ -347,6 +347,33 @@ class Access extends LDAPUtility implements user\IUserTools {
 	}
 
 	/**
+	 * accepts an array of group DNs and tests whether they match the user
+	 * filter by doing read operations against the group entries. Returns an
+	 * array of DNs that match the filter.
+	 *
+	 * @param string[] $groupDNs
+	 * @return string[]
+	 */
+	public function groupsMatchFilter($groupDNs) {
+		$validGroupDNs = [];
+		foreach($groupDNs as $dn) {
+			$cacheKey = 'groupsMatchFilter-'.$dn;
+			if($this->connection->isCached($cacheKey)) {
+				if($this->connection->getFromCache($cacheKey)) {
+					$validGroupDNs[] = $dn;
+				}
+				continue;
+			}
+
+			$result = $this->readAttribute($dn, 'cn', $this->connection->ldapGroupFilter);
+			if(is_array($result)) {
+				$validGroupDNs[] = $dn;
+			}
+		}
+		return $validGroupDNs;
+	}
+
+	/**
 	 * returns the internal ownCloud name for the given LDAP DN of the user, false on DN outside of search DN or failure
 	 * @param string $dn the dn of the user object
 	 * @param string $ldapName optional, the display name of the object
diff --git a/apps/user_ldap/tests/.htaccess b/apps/user_ldap/tests/.htaccess
new file mode 100755
index 00000000000..5e068d28661
--- /dev/null
+++ b/apps/user_ldap/tests/.htaccess
@@ -0,0 +1,14 @@
+# Generated by ownCloud on 2015-06-18 14:16:40
+# line below if for Apache 2.4
+<ifModule mod_authz_core.c>
+Require all denied
+</ifModule>
+
+# line below if for Apache 2.2
+<ifModule !mod_authz_core.c>
+deny from all
+Satisfy All
+</ifModule>
+
+# section for Apache 2.2 and 2.4
+IndexIgnore *
diff --git a/apps/user_ldap/tests/group_ldap.php b/apps/user_ldap/tests/group_ldap.php
index aeb306174f0..f716618ce48 100644
--- a/apps/user_ldap/tests/group_ldap.php
+++ b/apps/user_ldap/tests/group_ldap.php
@@ -404,6 +404,10 @@ class Test_Group_Ldap extends \Test\TestCase {
 			->method('dn2groupname')
 			->will($this->returnArgument(0));
 
+		$access->expects($this->once())
+			->method('groupsMatchFilter')
+			->will($this->returnArgument(0));
+
 		$groupBackend = new GroupLDAP($access);
 		$groups = $groupBackend->getUserGroups('userX');
 
diff --git a/apps/user_ldap/tests/integration/lib/IntegrationTestAccessGroupsMatchFilter.php b/apps/user_ldap/tests/integration/lib/IntegrationTestAccessGroupsMatchFilter.php
new file mode 100644
index 00000000000..6560153bb63
--- /dev/null
+++ b/apps/user_ldap/tests/integration/lib/IntegrationTestAccessGroupsMatchFilter.php
@@ -0,0 +1,158 @@
+<?php
+/**
+ * Created by PhpStorm.
+ * User: blizzz
+ * Date: 26.06.15
+ * Time: 18:13
+ */
+
+use OCA\user_ldap\lib\LDAP;
+
+require_once __DIR__  . '/../../../../../lib/base.php';
+
+class IntegrationTestAccessGroupsMatchFilter {
+	/** @var  LDAP */
+	protected $ldap;
+
+	/** @var  \OCA\user_ldap\lib\Connection */
+	protected $connection;
+
+	/** @var \OCA\user_ldap\lib\Access */
+	protected $access;
+
+	/** @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 environement and sets up a test configuration for
+	 * the LDAP backend.
+	 */
+	public function init() {
+		require('setup-scripts/createExplicitUsers.php');
+		require('setup-scripts/createExplicitGroups.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'];
+
+		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);
+	}
+
+	/**
+	 * tests whether the group filter works with one specific group, while the
+	 * input is the same.
+	 *
+	 * @return bool
+	 */
+	private function case1() {
+		$this->connection->setConfiguration(['ldapGroupFilter' => 'cn=RedGroup']);
+
+		$dns = ['cn=RedGroup,ou=Groups,' . $this->base];
+		$result = $this->access->groupsMatchFilter($dns);
+		return ($dns === $result);
+	}
+
+	/**
+	 * Tests whether a filter for limited groups is effective when more existing
+	 * groups were passed for validation.
+	 *
+	 * @return bool
+	 */
+	private function case2() {
+		$this->connection->setConfiguration(['ldapGroupFilter' => '(|(cn=RedGroup)(cn=PurpleGroup))']);
+
+		$dns = [
+			'cn=RedGroup,ou=Groups,' . $this->base,
+			'cn=BlueGroup,ou=Groups,' . $this->base,
+			'cn=PurpleGroup,ou=Groups,' . $this->base
+		];
+		$result = $this->access->groupsMatchFilter($dns);
+
+		$status =
+			count($result) === 2
+			&& in_array('cn=RedGroup,ou=Groups,' . $this->base, $result)
+			&& in_array('cn=PurpleGroup,ou=Groups,' . $this->base, $result);
+
+		return $status;
+	}
+
+	/**
+	 * 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);
+		$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' => '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');
+$test = new IntegrationTestAccessGroupsMatchFilter($host, $port, $adn, $apwd, $bdn);
+$test->init();
+$test->run();
diff --git a/apps/user_ldap/tests/integration/readme.md b/apps/user_ldap/tests/integration/readme.md
new file mode 100644
index 00000000000..e20efef8fdc
--- /dev/null
+++ b/apps/user_ldap/tests/integration/readme.md
@@ -0,0 +1,60 @@
+# Requirements #
+
+Have (as in do copy if not already done) the following files from https://github.com/owncloud/administration/tree/master/ldap-testing copied into the directory "setup-scripts":
+
+ * start.sh
+ * stop.sh
+ * config.php
+
+Configure config.php according to your needs, also have a look into the LDAP and network settings in start.sh and stop.sh.
+
+# Usage #
+
+The basic command to run a test is:
+
+```# ./run-test.sh [phpscript]```
+
+Yes, run it as root from within this directory.
+
+Example:
+
+```
+$ sudo ./run-test.sh lib/IntegrationTestAccessGroupsMatchFilter.php 
+71cbe88a4993e67066714d71c1cecc5ef26a54911a208103cb6294f90459e574
+c74dc0155db4efa7a0515d419528a8727bbc7596601cf25b0df05e348bd74895
+CONTAINER ID        IMAGE                       COMMAND             CREATED             STATUS                  PORTS                           NAMES
+c74dc0155db4        osixia/phpldapadmin:0.5.1   "/sbin/my_init"     1 seconds ago       Up Less than a second   80/tcp, 0.0.0.0:8443->443/tcp   docker-phpldapadmin   
+71cbe88a4993        nickstenning/slapd:latest   "/sbin/my_init"     1 seconds ago       Up Less than a second   127.0.0.1:7770->389/tcp         docker-slapd          
+
+LDAP server now available under 127.0.0.1:7770 (internal IP is 172.17.0.78)
+phpldapadmin now available under https://127.0.0.1:8443
+
+created user : Alice Ealic
+created group : RedGroup
+created group : BlueGroup
+created group : GreenGroup
+created group : PurpleGroup
+running case1 
+running case2 
+Tests succeeded
+Stopping and resetting containers
+docker-slapd
+docker-phpldapadmin
+docker-slapd
+docker-phpldapadmin
+```
+
+# How it works #
+
+1. start.sh is executed which brings up a fresh and clean OpenLDAP in Docker.
+2. The provided test script is executed. It also outputs results.
+3. stop.sh is executed to shut down OpenLDAP
+
+# Beware #
+
+This is quick solution for basically one test case. With expension this mechanism should be improved as well.
+
+It does not run automatically, unless you do it. No integration with any testing framework.
+
+exceptionOnLostConnection.php is not part of this mechanism. Read its source and run it isolated. While you're at it, port it :þ
+
diff --git a/apps/user_ldap/tests/integration/run-test.sh b/apps/user_ldap/tests/integration/run-test.sh
new file mode 100755
index 00000000000..e07e9b43408
--- /dev/null
+++ b/apps/user_ldap/tests/integration/run-test.sh
@@ -0,0 +1,17 @@
+#!/bin/sh
+
+if [ $1 ] ; then
+  TESTSCRIPT=$1
+else
+  echo "No test file given"                                                                                                                                                    exit
+fi
+
+if [ ! -e "$TESTSCRIPT" ] ; then
+    echo "Test file does not exist"
+    exit
+fi
+
+
+# sleep is necessary, otherwise the LDAP server cannot be connected to, yet.
+setup-scripts/start.sh && sleep 2 && php -f "$TESTSCRIPT"
+setup-scripts/stop.sh
diff --git a/apps/user_ldap/tests/integration/setup-scripts/createExplicitGroups.php b/apps/user_ldap/tests/integration/setup-scripts/createExplicitGroups.php
new file mode 100644
index 00000000000..68854de5571
--- /dev/null
+++ b/apps/user_ldap/tests/integration/setup-scripts/createExplicitGroups.php
@@ -0,0 +1,52 @@
+<?php
+
+if(php_sapi_name() !== 'cli') {
+	print('Only via CLI, please.');
+	exit(1);
+}
+
+include __DIR__ . '/config.php';
+
+$cr = ldap_connect($host, $port);
+ldap_set_option($cr, LDAP_OPT_PROTOCOL_VERSION, 3);
+$ok = ldap_bind($cr, $adn, $apwd);
+
+if (!$ok) {
+	die(ldap_error($cr));
+}
+
+$ouName = 'Groups';
+$ouDN = 'ou=' . $ouName . ',' . $bdn;
+
+//creates an OU
+if (true) {
+	$entry = [];
+	$entry['objectclass'][] = 'top';
+	$entry['objectclass'][] = 'organizationalunit';
+	$entry['ou'] = $ouName;
+	$b = ldap_add($cr, $ouDN, $entry);
+	if (!$b) {
+		die(ldap_error($cr));
+	}
+}
+
+$groups = ['RedGroup', 'BlueGroup', 'GreenGroup', 'PurpleGroup'];
+// groupOfNames requires groups to have at least one member
+// the member used is created by createExplicitUsers.php script
+$omniMember = 'uid=alice,ou=Users,' . $bdn;
+
+foreach ($groups as $cn) {
+	$newDN = 'cn=' . $cn . ',' . $ouDN;
+
+	$entry = [];
+	$entry['cn'] = $cn;
+	$entry['objectclass'][] = 'groupOfNames';
+	$entry['member'][] = $omniMember;
+
+	$ok = ldap_add($cr, $newDN, $entry);
+	if ($ok) {
+		echo('created group ' . ': ' . $entry['cn'] . PHP_EOL);
+	} else {
+		die(ldap_error($cr));
+	}
+}
diff --git a/apps/user_ldap/tests/integration/setup-scripts/createExplicitUsers.php b/apps/user_ldap/tests/integration/setup-scripts/createExplicitUsers.php
new file mode 100644
index 00000000000..ac21d48fd16
--- /dev/null
+++ b/apps/user_ldap/tests/integration/setup-scripts/createExplicitUsers.php
@@ -0,0 +1,54 @@
+<?php
+
+if(php_sapi_name() !== 'cli') {
+	print('Only via CLI, please.');
+	exit(1);
+}
+
+include __DIR__ . '/config.php';
+
+$cr = ldap_connect($host, $port);
+ldap_set_option($cr, LDAP_OPT_PROTOCOL_VERSION, 3);
+$ok = ldap_bind($cr, $adn, $apwd);
+
+if (!$ok) {
+	die(ldap_error($cr));
+}
+
+$ouName = 'Users';
+$ouDN = 'ou=' . $ouName . ',' . $bdn;
+
+//creates on OU
+if (true) {
+	$entry = [];
+	$entry['objectclass'][] = 'top';
+	$entry['objectclass'][] = 'organizationalunit';
+	$entry['ou'] = $ouName;
+	$b = ldap_add($cr, $ouDN, $entry);
+	if (!$b) {
+		die(ldap_error($cr));
+	}
+}
+
+$users = ['alice'];
+
+foreach ($users as $uid) {
+	$newDN = 'uid=' . $uid . ',' . $ouDN;
+	$fn = ucfirst($uid);
+	$sn = ucfirst(str_shuffle($uid));   // not so explicit but it's OK.
+
+	$entry = [];
+	$entry['cn'] = $fn . ' ' . $sn;
+	$entry['objectclass'][] = 'inetOrgPerson';
+	$entry['objectclass'][] = 'person';
+	$entry['sn'] = $sn;
+	$entry['userPassword'] = $uid;
+	$entry['displayName'] = $sn . ', ' . $fn;
+
+	$ok = ldap_add($cr, $newDN, $entry);
+	if ($ok) {
+		echo('created user ' . ': ' . $entry['cn'] . PHP_EOL);
+	} else {
+		die(ldap_error($cr));
+	}
+}