From 887c1dcbf8691eb8de14f09919732b0e4f03d82b Mon Sep 17 00:00:00 2001 From: amsprost Date: Fri, 4 Oct 2019 02:13:40 +0200 Subject: [PATCH] Fix inconsistencies, increase test coverage, update README --- README.md | 2 +- src/Services/Ldap/Client.php | 10 +- .../SetPassword/LdapSetPasswordAdapter.php | 6 +- tests/Unit/DTO/UserTest.php | 18 ++ tests/Unit/Services/Ldap/ClientTest.php | 57 ++++++ .../Services/Ldap/LdapParentHelperTest.php | 68 +++++++ .../Unit/Services/Ldap/UserDataMapperTest.php | 63 ++++++ .../SyncRemover/LdapSyncRemoverTest.php | 23 +++ .../AdapterFactory/LdapAdapterFactoryTest.php | 12 ++ .../GroupPush/LdapGroupPushAdapterTest.php | 186 ++++++++++++++++++ .../LdapSetPasswordAdapterTest.php | 98 +++++++++ 11 files changed, 536 insertions(+), 7 deletions(-) create mode 100644 tests/Unit/Services/Ldap/ClientTest.php create mode 100644 tests/Unit/Services/Ldap/LdapParentHelperTest.php create mode 100644 tests/Unit/Services/Ldap/UserDataMapperTest.php create mode 100644 tests/Unit/SynchronizationAdapter/GroupPush/LdapGroupPushAdapterTest.php create mode 100644 tests/Unit/SynchronizationAdapter/SetPassword/LdapSetPasswordAdapterTest.php diff --git a/README.md b/README.md index 48ba8c6..9b7682d 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ List of adapters: * [GitHub](https://github.com/) (WIP) * [Camunda](https://docs.camunda.org/manual/7.9/) -* LDAP (Planned) +* [LDAP](https://ldap.com/) * [MatterMost](https://mattermost.com/) (Planned) ## Installation diff --git a/src/Services/Ldap/Client.php b/src/Services/Ldap/Client.php index d718cdb..848e193 100644 --- a/src/Services/Ldap/Client.php +++ b/src/Services/Ldap/Client.php @@ -121,7 +121,7 @@ public function remove(string $dn): bool public function generateDn(array $rdn = []): string { - $dc = ''; + $dn = ''; foreach (array_merge($rdn, $this->target->getDomain()) as $key => $domainComponent) { $dnKey = is_string($key) ? $key : 'dc'; @@ -129,10 +129,14 @@ public function generateDn(array $rdn = []): string $domainComponent = is_array($domainComponent) ? $domainComponent : [$domainComponent]; foreach ($domainComponent as $domainComponentElement) { - $dc .= sprintf('%s=%s,', $dnKey, $domainComponentElement); + $dn .= sprintf('%s=%s,', $dnKey, $domainComponentElement); } } - return substr($dc, 0, -1); + if (empty($dn)) { + return $dn; + } + + return substr($dn, 0, -1); } } diff --git a/src/SynchronizationAdapter/SetPassword/LdapSetPasswordAdapter.php b/src/SynchronizationAdapter/SetPassword/LdapSetPasswordAdapter.php index fcb9102..eaa1e78 100644 --- a/src/SynchronizationAdapter/SetPassword/LdapSetPasswordAdapter.php +++ b/src/SynchronizationAdapter/SetPassword/LdapSetPasswordAdapter.php @@ -7,7 +7,7 @@ use LinkORB\OrgSync\Services\Ldap\LdapAssertionAwareTrait; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\LdapUserPushAdapter; -final class LdapSetPasswordAdapter implements SetPasswordInterface +class LdapSetPasswordAdapter implements SetPasswordInterface { use LdapAssertionAwareTrait; @@ -30,7 +30,7 @@ public function setPassword(User $user): SetPasswordInterface $userSearchFirstDn = $this->client->getDn( $this->client->first( $this->client->search( - sprintf('(cn=%s)', $user->getUsername()), + sprintf('(uid=%s)', $user->getUsername()), ['ou' => LdapUserPushAdapter::USERS_ORG_UNIT] ) ) @@ -46,7 +46,7 @@ public function setPassword(User $user): SetPasswordInterface return $this; } - private function encodePassword(string $password): string + protected function encodePassword(string $password): string { $salt = substr(str_shuffle( str_repeat('ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789', 4) diff --git a/tests/Unit/DTO/UserTest.php b/tests/Unit/DTO/UserTest.php index 7d591d5..1526c4a 100644 --- a/tests/Unit/DTO/UserTest.php +++ b/tests/Unit/DTO/UserTest.php @@ -42,4 +42,22 @@ public function getDtoClassName(): string { return User::class; } + + public function testSetPassword() + { + $somePass = '0000000'; + + $dto = (new User('user1', 'differentPass'))->setPassword($somePass); + + $this->assertSame($dto->getPassword(), $somePass); + } + + public function testSetPreviousPassword() + { + $somePass = '0000000'; + + $dto = (new User('user1', 'differentPass'))->setPreviousPassword($somePass); + + $this->assertSame($dto->getProperties()[User::PREVIOUS_PASSWORD], $somePass); + } } diff --git a/tests/Unit/Services/Ldap/ClientTest.php b/tests/Unit/Services/Ldap/ClientTest.php new file mode 100644 index 0000000..b81d5f1 --- /dev/null +++ b/tests/Unit/Services/Ldap/ClientTest.php @@ -0,0 +1,57 @@ +createPartialMock(Client::class, ['__destruct']); + $client->__construct($ldap); + + $this->assertEquals($expectedDn, $client->generateDn($rdn)); + } + + /** + * @return array + */ + public function getGenerateDnData(): array + { + return [ + [ + ['internal', 'com'], + ['cn' => ['test', 'somewhere'], 'ou' => 'org'], + 'cn=test,cn=somewhere,ou=org,dc=internal,dc=com' + ], + [ + ['com', 'internal'], + ['cn' => ['somewhere', 'test']], + 'cn=somewhere,cn=test,dc=com,dc=internal' + ], + [ + [], + ['uid' => 111222,'ou' => 'org'], + 'uid=111222,ou=org' + ], + [ + ['abc', 'xyz'], + [], + 'dc=abc,dc=xyz' + ], + [ + [], + [], + '' + ], + ]; + } +} diff --git a/tests/Unit/Services/Ldap/LdapParentHelperTest.php b/tests/Unit/Services/Ldap/LdapParentHelperTest.php new file mode 100644 index 0000000..c4e1210 --- /dev/null +++ b/tests/Unit/Services/Ldap/LdapParentHelperTest.php @@ -0,0 +1,68 @@ +parentHelper = new LdapParentHelper(); + } + + /** + * @dataProvider getParentData + */ + public function testGetParentGroups(Group $group, array $expectedParents, array $initialParents = []) + { + $this->assertEquals($expectedParents, $this->parentHelper->getParentGroups($initialParents, $group)); + } + + public function testGetParentsCircularReference() + { + $this->expectExceptionMessage('Circular reference detected'); + $this->expectException(\UnexpectedValueException::class); + + $group1 = new Group('test1', ''); + $group2 = new Group('test2', ''); + $group1->setParent($group2); + $group2->setParent($group1); + + $this->parentHelper->getParentGroups([], $group1); + } + + public function getParentData(): array + { + return [ + [ + new Group('hello', '', '', new Group('to', '', '', new Group('all', '', '', new Group('world', '')))), + ['to', 'all', 'world'], + ], + [ + new Group('empty', ''), + [], + ], + [ + new Group('empty', ''), + ['initial', 'set'], + ['initial', 'set'] + ], + [ + new Group('Earth', '', '', new Group('Eurasia', '', '', new Group('Europe', '',))), + ['space', 'MilkyWay', 'Eurasia', 'Europe'], + ['space', 'MilkyWay'] + ], + [ + new Group('Earth', ''), + [], + [] + ], + ]; + } +} diff --git a/tests/Unit/Services/Ldap/UserDataMapperTest.php b/tests/Unit/Services/Ldap/UserDataMapperTest.php new file mode 100644 index 0000000..253d965 --- /dev/null +++ b/tests/Unit/Services/Ldap/UserDataMapperTest.php @@ -0,0 +1,63 @@ +mapper = new UserDataMapper(); + } + + /** + * @dataProvider getMapData + */ + public function testMap(User $user, array $expectedUserData) + { + $this->assertEquals($expectedUserData, $this->mapper->map($user)); + } + + public function getMapData(): array + { + return [ + [ + new User(''), + [ + 'cn' => '', + 'uid' => '', + 'sn' => '', + 'objectClass' => ['inetOrgPerson', 'organizationalPerson', 'person', 'top'] + ], + ], + [ + new User('uadmin', 'p@ss', 'admin@example.com', 'superadm', '1.jpg', ['lastName' => 'Jong']), + [ + 'cn' => 'uadmin', + 'uid' => 'uadmin', + 'sn' => 'Jong', + 'objectClass' => ['inetOrgPerson', 'organizationalPerson', 'person', 'top'], + 'mail' => 'admin@example.com', + 'displayName' => 'superadm', + 'Photo' => '1.jpg', + ], + ], + [ + new User('uadmin', 'p@ss', '', 'superadm'), + [ + 'cn' => 'uadmin', + 'uid' => 'uadmin', + 'sn' => 'superadm', + 'objectClass' => ['inetOrgPerson', 'organizationalPerson', 'person', 'top'], + 'displayName' => 'superadm', + ], + ], + ]; + } +} diff --git a/tests/Unit/Services/SyncRemover/LdapSyncRemoverTest.php b/tests/Unit/Services/SyncRemover/LdapSyncRemoverTest.php index c321f0b..2f8b604 100644 --- a/tests/Unit/Services/SyncRemover/LdapSyncRemoverTest.php +++ b/tests/Unit/Services/SyncRemover/LdapSyncRemoverTest.php @@ -12,6 +12,7 @@ use LinkORB\OrgSync\SynchronizationAdapter\UserPush\LdapUserPushAdapter; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use UnexpectedValueException; class LdapSyncRemoverTest extends TestCase { @@ -124,6 +125,28 @@ public function testRemoveNonExistsGroups(array $orgGroups, array $existingGroup $this->assertNull($this->syncRemover->removeNonExists($organization)); } + public function testRemoveException() + { + $this->client + ->expects($this->exactly(2)) + ->method('search') + ->withConsecutive( + [sprintf('(ou=%s)', LdapUserPushAdapter::USERS_ORG_UNIT)], + [sprintf('(ou=%s)', LdapGroupPushAdapter::GROUPS_ORG_UNIT)] + ) + ->willReturnOnConsecutiveCalls(null, null); + $this->client + ->expects($this->exactly(2)) + ->method('count') + ->withConsecutive([null], [null]) + ->willReturnOnConsecutiveCalls(null, null); + + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage('Error during search!'); + + $this->syncRemover->removeNonExists(new Organization('test')); + } + public function getRemoveUsersData(): array { return [ diff --git a/tests/Unit/SynchronizationAdapter/AdapterFactory/LdapAdapterFactoryTest.php b/tests/Unit/SynchronizationAdapter/AdapterFactory/LdapAdapterFactoryTest.php index fd56c83..94b45f6 100644 --- a/tests/Unit/SynchronizationAdapter/AdapterFactory/LdapAdapterFactoryTest.php +++ b/tests/Unit/SynchronizationAdapter/AdapterFactory/LdapAdapterFactoryTest.php @@ -7,6 +7,8 @@ use LinkORB\OrgSync\Services\Ldap\Client; use LinkORB\OrgSync\Services\SyncRemover\LdapSyncRemover; use LinkORB\OrgSync\SynchronizationAdapter\AdapterFactory\LdapAdapterFactory; +use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\LdapGroupPushAdapter; +use LinkORB\OrgSync\SynchronizationAdapter\SetPassword\LdapSetPasswordAdapter; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\LdapUserPushAdapter; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -35,6 +37,16 @@ public function testCreateUserPushAdapter() $this->assertInstanceOf(LdapUserPushAdapter::class, $this->factory->createUserPushAdapter()); } + public function testCreateGroupPushAdapter() + { + $this->assertInstanceOf(LdapGroupPushAdapter::class, $this->factory->createGroupPushAdapter()); + } + + public function testCreateSetPasswordAdapter() + { + $this->assertInstanceOf(LdapSetPasswordAdapter::class, $this->factory->createSetPasswordAdapter()); + } + /** * @dataProvider getSupportsData */ diff --git a/tests/Unit/SynchronizationAdapter/GroupPush/LdapGroupPushAdapterTest.php b/tests/Unit/SynchronizationAdapter/GroupPush/LdapGroupPushAdapterTest.php new file mode 100644 index 0000000..031bb32 --- /dev/null +++ b/tests/Unit/SynchronizationAdapter/GroupPush/LdapGroupPushAdapterTest.php @@ -0,0 +1,186 @@ +client = $this->createMock(Client::class); + $this->parentHelper = $this->createMock(LdapParentHelper::class); + + $this->adapter = new LdapGroupPushAdapter($this->client, $this->parentHelper); + } + + /** + * @dataProvider getTestPushGroupData + */ + public function testPushGroup(Group $groupObj, array $groupArr, array $searchResults) + { + $parentGroups = ['some', 'parents']; + $this->parentHelper->expects($this->once())->method('getParentGroups')->willReturn($parentGroups); + $this->client + ->method('generateDn') + ->willReturnCallback(function (array $params) { + return json_encode($params); + }); + + $this->client + ->expects($this->exactly(2)) + ->method('search') + ->withConsecutive( + [$this->anything()], + [$this->callback(function (string $query) use ($groupObj) { + return strstr($query, $groupObj->getName()) !== false; + })] + ) + ->willReturnOnConsecutiveCalls(null, $searchResults); + + $this->client->expects($this->once())->method('count')->willReturn(1); + + $this->client + ->expects($this->once()) + ->method('first') + ->with($searchResults) + ->willReturnCallback(function (array $result) { + return reset($result); + }); + + if (count($searchResults) > 0) { + $dn = 'testDn'; + $this->client->expects($this->once())->method('getDn')->with(reset($searchResults))->willReturn($dn); + + $this->client + ->expects($this->once()) + ->method('modify') + ->with($groupArr, $dn) + ->willReturn(true); + } else { + $this->client + ->expects($this->once()) + ->method('add') + ->with( + $groupArr, + [ + 'cn' => array_merge([$groupObj->getName()], $parentGroups), + 'ou' => [LdapGroupPushAdapter::GROUPS_ORG_UNIT] + ] + ) + ->willReturn(true); + } + + $this->assertSame($this->adapter, $this->adapter->pushGroup($groupObj)); + } + + public function testPushUserOrgUnitCreation() + { + $group = new Group('test', ''); + + $this->client->method('add')->willReturn(true); + $this->client->method('modify')->willReturn(true); + + $this->client + ->expects($this->exactly(2)) + ->method('search') + ->withConsecutive([sprintf('(ou=%s)', LdapGroupPushAdapter::GROUPS_ORG_UNIT)], [$this->anything()]) + ->willReturnOnConsecutiveCalls([], [1]); + + $this->client->expects($this->once())->method('count')->willReturn(0); + + $this->client->expects($this->atLeastOnce())->method('add')->withConsecutive( + [ + [ + 'ou' => LdapGroupPushAdapter::GROUPS_ORG_UNIT, + 'objectClass' => ['top', 'organizationalUnit'], + ], + ['ou' => LdapGroupPushAdapter::GROUPS_ORG_UNIT] + ], + [$this->anything()] + ); + + $this->assertSame($this->adapter, $this->adapter->pushGroup($group)); + } + + public function testPushGroupSearchException() + { + $this->client->method('count')->willReturn(null); + + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage('Error during search!'); + + $this->adapter->pushGroup(new Group('', '')); + } + + public function testPushUserAddException() + { + $this->client->method('count')->willReturn(1); + $this->client->method('add')->willReturn(false); + $this->client->method('first')->willReturn(false); + + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage('Group \'undefined\' wasn\'t added'); + + $this->adapter->pushGroup(new Group('undefined', '')); + } + + public function getTestPushGroupData(): array + { + return [ + [ + new Group('temp123', '', null, null, [new User('temp1'), new User('temp2')]), + [ + 'cn' => 'temp123', + 'objectClass' => ['top', 'groupOfUniqueNames'], + 'uniqueMember' => [ + json_encode(['uid' => 'temp1', 'ou' => LdapUserPushAdapter::USERS_ORG_UNIT]), + json_encode(['uid' => 'temp2', 'ou' => LdapUserPushAdapter::USERS_ORG_UNIT]), + ], + ], + [], + ], + [ + new Group('temp000', ''), + [ + 'cn' => 'temp000', + 'objectClass' => ['top', 'groupOfUniqueNames'], + 'uniqueMember' => [], + ], + [1], + ], + [ + new Group('987', ''), + [ + 'cn' => '987', + 'objectClass' => ['top', 'groupOfUniqueNames'], + 'uniqueMember' => [], + ], + [1, 2, 3] + ] + ]; + } +} diff --git a/tests/Unit/SynchronizationAdapter/SetPassword/LdapSetPasswordAdapterTest.php b/tests/Unit/SynchronizationAdapter/SetPassword/LdapSetPasswordAdapterTest.php new file mode 100644 index 0000000..0fb20e7 --- /dev/null +++ b/tests/Unit/SynchronizationAdapter/SetPassword/LdapSetPasswordAdapterTest.php @@ -0,0 +1,98 @@ +client = $this->createMock(Client::class); + $this->adapter = $this->createPartialMock(LdapSetPasswordAdapter::class, ['encodePassword']); + $this->adapter->__construct($this->client); + } + + public function testSetPassword() + { + $user = new User('passTest', 'p@ssword'); + $dn = 'qwertyuio'; + $searchResult = [['dn' => $dn, 'found' => true]]; + $pass = '{SSHA}test123'; + + $this->client + ->expects($this->exactly(2)) + ->method('search') + ->withConsecutive( + ['(ou=' . LdapUserPushAdapter::USERS_ORG_UNIT . ')'], + ['(uid=' . $user->getUsername() . ')', ['ou' => LdapUserPushAdapter::USERS_ORG_UNIT]] + ) + ->willReturnOnConsecutiveCalls([1], $searchResult); + + $this->client + ->expects($this->once()) + ->method('count') + ->willReturnCallback(function (array $arg) { + return count($arg); + }); + $this->client + ->expects($this->once()) + ->method('first') + ->with($searchResult) + ->willReturnCallback(function (array $arg) { + return reset($arg); + }); + $this->client + ->expects($this->once()) + ->method('getDn') + ->with(reset($searchResult)) + ->willReturnCallback(function (array $arg) { + return $arg['dn']; + }); + + $this->adapter->expects($this->once())->method('encodePassword')->with('p@ssword')->willReturn($pass); + + $this->client + ->expects($this->once()) + ->method('modify') + ->with(['userPassword' => $pass], $dn) + ->willReturn(true); + + $this->assertSame($this->adapter, $this->adapter->setPassword($user)); + } + + public function testSetPasswordOuException() + { + $this->expectExceptionMessage('Error during search!'); + $this->expectException(UnexpectedValueException::class); + + $this->adapter->setPassword(new User('')); + } + + public function testSetPasswordException() + { + $this->client->method('count')->willReturn(1); + $this->client->method('getDn')->willReturn(''); + + $this->expectExceptionMessage('User \'\' password wasn\'t changed'); + $this->expectException(UnexpectedValueException::class); + + $this->adapter->setPassword(new User('', '')); + } +}