Merge pull request #185 from sergio91pt/group_mapping2

Group mapping

5 files changed, 131 insertions, 12 deletions

diff --git a/appinfo/app.php b/appinfo/app.php
index 841bb548..d1ef6712 100644
--- a/appinfo/app.php
+++ b/appinfo/app.php
@@ -44,7 +44,8 @@ $userBackend = new \OCA\User_SAML\UserBackend(
 	$urlGenerator,
 	\OC::$server->getSession(),
 	\OC::$server->getDatabaseConnection(),
-	\OC::$server->getUserManager()
+	\OC::$server->getUserManager(),
+	\OC::$server->getGroupManager()
 );
 $userBackend->registerBackends(\OC::$server->getUserManager()->getBackends());
 OC_User::useBackend($userBackend);
diff --git a/lib/Settings/Admin.php b/lib/Settings/Admin.php
index 7ae9e9c0..919f4bde 100644
--- a/lib/Settings/Admin.php
+++ b/lib/Settings/Admin.php
@@ -112,6 +112,11 @@ class Admin implements ISettings {
 				'type' => 'line',
 				'required' => false,
 			],
+			'group_mapping' => [
+				'text' => $this->l10n->t('Attribute to map the users groups to.'),
+				'type' => 'line',
+				'required' => true,
+			],
 		];
 
 		$type = $this->config->getAppValue('user_saml', 'type');
diff --git a/lib/UserBackend.php b/lib/UserBackend.php
index d1f6bd11..17e437ae 100644
--- a/lib/UserBackend.php
+++ b/lib/UserBackend.php
@@ -25,6 +25,7 @@ use OCP\Authentication\IApacheBackend;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IDBConnection;
 use OCP\IUserManager;
+use OCP\IGroupManager;
 use OCP\UserInterface;
 use OCP\IUserBackend;
 use OCP\IConfig;
@@ -42,6 +43,8 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend {
 	private $db;
 	/** @var IUserManager */
 	private $userManager;
+	/** @var IGroupManager */
+	private $groupManager;
 	/** @var \OCP\UserInterface[] */
 	private static $backends = [];
 
@@ -51,17 +54,20 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend {
 	 * @param ISession $session
 	 * @param IDBConnection $db
 	 * @param IUserManager $userManager
+	 * @param IGroupManager $groupManager
 	 */
 	public function __construct(IConfig $config,
 								IURLGenerator $urlGenerator,
 								ISession $session,
 								IDBConnection $db,
-								IUserManager $userManager) {
+								IUserManager $userManager,
+								IGroupManager $groupManager) {
 		$this->config = $config;
 		$this->urlGenerator = $urlGenerator;
 		$this->session = $session;
 		$this->db = $db;
 		$this->userManager = $userManager;
+		$this->groupManager = $groupManager;
 	}
 
 	/**
@@ -415,21 +421,29 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend {
 		self::$backends = $backends;
 	}
 
-	private function getAttributeValue($name, array $attributes) {
+	private function getAttributeKeys($name)
+	{
 		$keys = explode(' ', $this->config->getAppValue('user_saml', $name, ''));
 
-		if(count($keys) === 1 && $keys[0] === '') {
+		if (count($keys) === 1 && $keys[0] === '') {
 			throw new \InvalidArgumentException('Attribute is not configured');
 		}
+		return $keys;
+	}
+
+	private function getAttributeValue($name, array $attributes) {
+		$keys = $this->getAttributeKeys($name);
 
 		$value = '';
 		foreach($keys as $key) {
 			if (isset($attributes[$key])) {
 				if (is_array($attributes[$key])) {
-					if($value !== '') {
-						$value .= ' ';
+					foreach ($attributes[$key] as $attribute_part_value) {
+						if($value !== '') {
+							$value .= ' ';
+						}
+						$value .= $attribute_part_value;
 					}
-					$value .= $attributes[$key][0];
 				} else {
 					if($value !== '') {
 						$value .= ' ';
@@ -442,6 +456,23 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend {
 		return $value;
 	}
 
+	private function getAttributeArrayValue($name, array $attributes) {
+		$keys = $this->getAttributeKeys($name);
+
+		$value = array();
+		foreach($keys as $key) {
+			if (isset($attributes[$key])) {
+				if (is_array($attributes[$key])) {
+					$value = array_merge($value, array_values($attributes[$key]));
+				} else {
+					$value[] = $attributes[$key];
+				}
+			}
+		}
+
+		return $value;
+	}
+
 	public function updateAttributes($uid,
 									 array $attributes) {
 		$user = $this->userManager->get($uid);
@@ -464,6 +495,13 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend {
 			$newQuota = null;
 		}
 
+		try {
+			$newGroups = $this->getAttributeArrayValue('saml-attribute-mapping-group_mapping', $attributes);
+		} catch (\InvalidArgumentException $e) {
+			$newGroups = null;
+		}
+
+
 		if ($user !== null) {
 			$currentEmail = (string)$user->getEMailAddress();
 			if ($newEmail !== null
@@ -471,7 +509,7 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend {
 				$user->setEMailAddress($newEmail);
 			}
 			$currentDisplayname = (string)$this->getDisplayName($uid);
-			if($newDisplayname !== null
+			if ($newDisplayname !== null
 				&& $currentDisplayname !== $newDisplayname) {
 				\OC_Hook::emit('OC_User', 'changeUser',
 					[
@@ -486,6 +524,25 @@ class UserBackend implements IApacheBackend, UserInterface, IUserBackend {
 			if ($newQuota !== null) {
 				$user->setQuota($newQuota);
 			}
+
+			if ($newGroups !== null) {
+				$groupManager = $this->groupManager;
+				$oldGroups = $groupManager->getUserGroupIds($user);
+
+				$groupsToAdd = array_unique(array_diff($newGroups, $oldGroups));
+				$groupsToRemove = array_diff($oldGroups, $newGroups);
+
+				foreach ($groupsToAdd as $group) {
+					if (!($groupManager->groupExists($group))) {
+						$groupManager->createGroup($group);
+					}
+					$groupManager->get($group)->addUser($user);
+				}
+
+				foreach ($groupsToRemove as $group) {
+					$groupManager->get($group)->removeUser($user);
+				}
+			}
 		}
 	}
 }
diff --git a/tests/unit/Settings/AdminTest.php b/tests/unit/Settings/AdminTest.php
index e42ebcd9..8a189d48 100644
--- a/tests/unit/Settings/AdminTest.php
+++ b/tests/unit/Settings/AdminTest.php
@@ -120,6 +120,11 @@ class AdminTest extends \Test\TestCase  {
 				'type' => 'line',
 				'required' => false,
 			],
+			'group_mapping' => [
+				'text' => $this->l10n->t('Attribute to map the users groups to.'),
+				'type' => 'line',
+				'required' => true,
+			],
 		];
 
 		$params = [
diff --git a/tests/unit/UserBackendTest.php b/tests/unit/UserBackendTest.php
index 81ea0660..7844a52d 100644
--- a/tests/unit/UserBackendTest.php
+++ b/tests/unit/UserBackendTest.php
@@ -24,10 +24,11 @@ namespace OCA\User_SAML\Tests\Settings;
 use OCA\User_SAML\UserBackend;
 use OCP\IConfig;
 use OCP\IDBConnection;
+use OCP\IGroup;
+use OCP\IGroupManager;
 use OCP\ISession;
 use OCP\IURLGenerator;
 use OCP\IUser;
-use OCP\IUserBackend;
 use OCP\IUserManager;
 use Test\TestCase;
 
@@ -42,6 +43,8 @@ class UserBackendTest extends TestCase   {
 	private $db;
 	/** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
 	private $userManager;
+	/** @var IGroupManager|\PHPUnit_Framework_MockObject_MockObject */
+	private $groupManager;
 	/** @var UserBackend|\PHPUnit_Framework_MockObject_MockObject */
 	private $userBackend;
 
@@ -53,6 +56,7 @@ class UserBackendTest extends TestCase   {
 		$this->session = $this->createMock(ISession::class);
 		$this->db = $this->createMock(IDBConnection::class);
 		$this->userManager = $this->createMock(IUserManager::class);
+		$this->groupManager = $this->createMock(IGroupManager::class);
 	}
 
 	public function getMockedBuilder(array $mockedFunctions = []) {
@@ -63,7 +67,8 @@ class UserBackendTest extends TestCase   {
 					$this->urlGenerator,
 					$this->session,
 					$this->db,
-					$this->userManager
+					$this->userManager,
+					$this->groupManager
 				])
 				->setMethods($mockedFunctions)
 				->getMock();
@@ -73,7 +78,8 @@ class UserBackendTest extends TestCase   {
 				$this->urlGenerator,
 				$this->session,
 				$this->db,
-				$this->userManager
+				$this->userManager,
+				$this->groupManager
 			);
 		}
 	}
@@ -122,6 +128,8 @@ class UserBackendTest extends TestCase   {
 		$this->getMockedBuilder(['getDisplayName', 'setDisplayName']);
 		/** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */
 		$user = $this->createMock(IUser::class);
+		$groupA = $this->createMock(IGroup::class);
+		$groupC = $this->createMock(IGroup::class);
 
 		$this->config
 			->expects($this->at(0))
@@ -138,6 +146,11 @@ class UserBackendTest extends TestCase   {
 			->method('getAppValue')
 			->with('user_saml', 'saml-attribute-mapping-quota_mapping', '')
 			->willReturn('quota');
+		$this->config
+			->expects($this->at(3))
+			->method('getAppValue')
+			->with('user_saml', 'saml-attribute-mapping-group_mapping', '')
+			->willReturn('groups');
 
 		$this->userManager
 			->expects($this->once())
@@ -165,7 +178,45 @@ class UserBackendTest extends TestCase   {
 			->expects($this->once())
 			->method('setDisplayName')
 			->with('ExistingUser', 'New Displayname');
-		$this->userBackend->updateAttributes('ExistingUser', ['email' => 'new@example.com', 'displayname' => 'New Displayname', 'quota' => '50MB']);
+
+		$this->groupManager
+			->expects($this->once())
+			->method('getUserGroupIds')
+			->with($user)
+			->willReturn(['groupA', 'groupB']);
+		$this->groupManager
+			->expects($this->once())
+			->method('groupExists')
+			->with('groupC')
+			->willReturn(false);
+		$this->groupManager
+			->expects($this->once())
+			->method('createGroup')
+			->with('groupC');
+
+		// updateAttributes first adds new groups, then removes old ones
+		// In this test groupA is removed from the user, groupB is unchanged
+		// and groupC is added
+		$this->groupManager
+			->expects($this->exactly(2))
+			->method('get')
+			->withConsecutive(['groupC'], ['groupA'])
+			->willReturnOnConsecutiveCalls($groupC, $groupA);
+		$groupA
+			->expects($this->once())
+			->method('removeUser')
+			->with($user);
+		$groupC
+			->expects($this->once())
+			->method('addUser')
+			->with($user);
+
+		$this->userBackend->updateAttributes('ExistingUser', [
+			'email' => 'new@example.com',
+			'displayname' => 'New Displayname',
+			'quota' => '50MB',
+			'groups' => ['groupB', 'groupC'],
+		]);
 	}
 
 	public function testUpdateAttributesQuotaDefaultFallback() {