diff options
| -rw-r--r-- | ChangeLog | 1 | ||||
| -rw-r--r-- | app/Core/User/GroupSync.php | 50 | ||||
| -rw-r--r-- | app/Model/GroupMember.php | 2 | ||||
| -rw-r--r-- | tests/units/Core/User/GroupSyncTest.php | 61 |
4 files changed, 105 insertions, 9 deletions
@@ -11,6 +11,7 @@ New features: Improvements: +* Improve LDAP user group membership synchronization * Category and user filters do not append anymore in search field * Added more template hooks * Added tasks search with the API diff --git a/app/Core/User/GroupSync.php b/app/Core/User/GroupSync.php index 573acd47..4e08d574 100644 --- a/app/Core/User/GroupSync.php +++ b/app/Core/User/GroupSync.php @@ -16,16 +16,52 @@ class GroupSync extends Base * Synchronize group membership * * @access public - * @param integer $userId - * @param array $groupIds + * @param integer $userId + * @param array $externalGroupIds */ - public function synchronize($userId, array $groupIds) + public function synchronize($userId, array $externalGroupIds) { - foreach ($groupIds as $groupId) { - $group = $this->group->getByExternalId($groupId); + $userGroups = $this->groupMember->getGroups($userId); + $this->addGroups($userId, $userGroups, $externalGroupIds); + $this->removeGroups($userId, $userGroups, $externalGroupIds); + } + + /** + * Add missing groups to the user + * + * @access protected + * @param integer $userId + * @param array $userGroups + * @param array $externalGroupIds + */ + protected function addGroups($userId, array $userGroups, array $externalGroupIds) + { + $userGroupIds = array_column($userGroups, 'external_id', 'external_id'); - if (! empty($group) && ! $this->groupMember->isMember($group['id'], $userId)) { - $this->groupMember->addUser($group['id'], $userId); + foreach ($externalGroupIds as $externalGroupId) { + if (! isset($userGroupIds[$externalGroupId])) { + $group = $this->group->getByExternalId($externalGroupId); + + if (! empty($group)) { + $this->groupMember->addUser($group['id'], $userId); + } + } + } + } + + /** + * Remove groups from the user + * + * @access protected + * @param integer $userId + * @param array $userGroups + * @param array $externalGroupIds + */ + protected function removeGroups($userId, array $userGroups, array $externalGroupIds) + { + foreach ($userGroups as $userGroup) { + if (! empty($userGroup['external_id']) && ! in_array($userGroup['external_id'], $externalGroupIds)) { + $this->groupMember->removeUser($userGroup['id'], $userId); } } } diff --git a/app/Model/GroupMember.php b/app/Model/GroupMember.php index 14041704..baf303c4 100644 --- a/app/Model/GroupMember.php +++ b/app/Model/GroupMember.php @@ -119,7 +119,7 @@ class GroupMember extends Base public function getGroups($user_id) { return $this->db->table(self::TABLE) - ->columns(Group::TABLE.'.id', Group::TABLE.'.name') + ->columns(Group::TABLE.'.id', Group::TABLE.'.external_id', Group::TABLE.'.name') ->join(Group::TABLE, 'id', 'group_id') ->eq(self::TABLE.'.user_id', $user_id) ->asc(Group::TABLE.'.name') diff --git a/tests/units/Core/User/GroupSyncTest.php b/tests/units/Core/User/GroupSyncTest.php index e22b86d4..decb2fdc 100644 --- a/tests/units/Core/User/GroupSyncTest.php +++ b/tests/units/Core/User/GroupSyncTest.php @@ -8,7 +8,7 @@ use Kanboard\Model\GroupMember; class GroupSyncTest extends Base { - public function testSynchronize() + public function testAddGroups() { $group = new Group($this->container); $groupMember = new GroupMember($this->container); @@ -27,4 +27,63 @@ class GroupSyncTest extends Base $this->assertTrue($groupMember->isMember(1, 1)); $this->assertTrue($groupMember->isMember(2, 1)); } + + public function testRemoveGroups() + { + $group = new Group($this->container); + $groupMember = new GroupMember($this->container); + $groupSync = new GroupSync($this->container); + + $this->assertEquals(1, $group->create('My Group 1', 'externalId1')); + $this->assertEquals(2, $group->create('My Group 2', 'externalId2')); + + $this->assertTrue($groupMember->addUser(1, 1)); + + $this->assertTrue($groupMember->isMember(1, 1)); + $this->assertFalse($groupMember->isMember(2, 1)); + + $groupSync->synchronize(1, array('externalId3')); + + $this->assertFalse($groupMember->isMember(1, 1)); + $this->assertFalse($groupMember->isMember(2, 1)); + } + + public function testBoth() + { + $group = new Group($this->container); + $groupMember = new GroupMember($this->container); + $groupSync = new GroupSync($this->container); + + $this->assertEquals(1, $group->create('My Group 1', 'externalId1')); + $this->assertEquals(2, $group->create('My Group 2', 'externalId2')); + $this->assertEquals(3, $group->create('My Group 3', 'externalId3')); + + $this->assertTrue($groupMember->addUser(1, 1)); + $this->assertTrue($groupMember->addUser(2, 1)); + + $this->assertTrue($groupMember->isMember(1, 1)); + $this->assertTrue($groupMember->isMember(2, 1)); + $this->assertFalse($groupMember->isMember(3, 1)); + + $groupSync->synchronize(1, array('externalId1', 'externalId3')); + + $this->assertTrue($groupMember->isMember(1, 1)); + $this->assertFalse($groupMember->isMember(2, 1)); + $this->assertTrue($groupMember->isMember(3, 1)); + } + + public function testThatInternalGroupsAreNotTouched() + { + $group = new Group($this->container); + $groupMember = new GroupMember($this->container); + $groupSync = new GroupSync($this->container); + + $this->assertEquals(1, $group->create('My Group 1')); + $this->assertTrue($groupMember->addUser(1, 1)); + $this->assertTrue($groupMember->isMember(1, 1)); + + $groupSync->synchronize(1, array('externalId3')); + + $this->assertTrue($groupMember->isMember(1, 1)); + } } |