diff options
author | Björn Schießle <bjoern@schiessle.org> | 2018-03-28 12:35:15 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-03-28 12:35:15 +0300 |
commit | d43bd330a1db438b4a975f4e4dca78a9b64f49fd (patch) | |
tree | 93881161f41b8a6dc8ce31bfc0875b3c362cb476 | |
parent | 17c57d6b4eeb88e805c4311c9860abd14da499d0 (diff) | |
parent | 423a76a84386d8147131418d1c74194f3b46f844 (diff) |
Merge pull request #185 from sergio91pt/group_mapping2
Group mapping
-rw-r--r-- | appinfo/app.php | 3 | ||||
-rw-r--r-- | lib/Settings/Admin.php | 5 | ||||
-rw-r--r-- | lib/UserBackend.php | 71 | ||||
-rw-r--r-- | tests/unit/Settings/AdminTest.php | 5 | ||||
-rw-r--r-- | tests/unit/UserBackendTest.php | 59 |
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() { |