From 6dc11a1d929a5c50df431aab10c6592688de730d Mon Sep 17 00:00:00 2001 From: amsprost Date: Sun, 25 Aug 2019 03:22:30 +0200 Subject: [PATCH 01/13] CARD-2527: Implement supports method, base github functionality + tests --- src/DTO/Target.php | 5 + src/DTO/Target/Github.php | 25 +++++ src/Services/InputHandler.php | 6 +- .../SyncRemover/GithubSyncRemover.php | 37 +++++++ .../AdapterFactoryInterface.php | 5 +- .../AdapterFactory/CamundaAdapterFactory.php | 16 +-- .../AdapterFactory/GithubAdapterFactory.php | 39 ++++--- .../GroupPush/CamundaGroupPushAdapter.php | 1 - .../GroupPush/GithubGroupPushAdapter.php | 40 +++++++ .../CamundaOrganizationPushAdapter.php | 14 --- .../OrganizationPushInterface.php | 10 -- .../UserPush/GithubUserPushAdapter.php | 25 ----- .../SynchronizationMediator.php | 28 ++++- tests/Unit/DTO/Target/GithubTest.php | 35 ++++++ .../SyncRemover/CamundaSyncRemoverTest.php | 1 + .../SyncRemover/GithubSyncRemoverTest.php | 101 ++++++++++++++++++ .../CamundaAdapterFactoryTest.php | 26 +++-- .../GithubAdapterFactoryTest.php | 90 ++++++++++++++++ .../GroupPush/GithubGroupPushAdapterTest.php | 89 +++++++++++++++ .../UserPush/GithubUserPushAdapterTest.php | 36 ------- .../SynchronizationMediatorTest.php | 92 +++++++++++++--- 21 files changed, 584 insertions(+), 137 deletions(-) create mode 100644 src/DTO/Target/Github.php create mode 100644 src/Services/SyncRemover/GithubSyncRemover.php create mode 100644 src/SynchronizationAdapter/GroupPush/GithubGroupPushAdapter.php delete mode 100644 src/SynchronizationAdapter/OrganizationPush/CamundaOrganizationPushAdapter.php delete mode 100644 src/SynchronizationAdapter/OrganizationPush/OrganizationPushInterface.php delete mode 100644 src/SynchronizationAdapter/UserPush/GithubUserPushAdapter.php create mode 100644 tests/Unit/DTO/Target/GithubTest.php create mode 100644 tests/Unit/Services/SyncRemover/GithubSyncRemoverTest.php create mode 100644 tests/Unit/SynchronizationAdapter/AdapterFactory/GithubAdapterFactoryTest.php create mode 100644 tests/Unit/SynchronizationAdapter/GroupPush/GithubGroupPushAdapterTest.php delete mode 100644 tests/Unit/SynchronizationAdapter/UserPush/GithubUserPushAdapterTest.php diff --git a/src/DTO/Target.php b/src/DTO/Target.php index 98acb33..c118a4c 100644 --- a/src/DTO/Target.php +++ b/src/DTO/Target.php @@ -11,6 +11,11 @@ */ abstract class Target { + public const USER_PUSH = 'push_user'; + public const GROUP_PUSH = 'push_group'; + public const SET_PASSWORD = 'set_password'; + public const PULL_ORGANIZATION = 'organization_pull'; + /** @var string */ private $baseUrl; diff --git a/src/DTO/Target/Github.php b/src/DTO/Target/Github.php new file mode 100644 index 0000000..f44ec10 --- /dev/null +++ b/src/DTO/Target/Github.php @@ -0,0 +1,25 @@ +token = $token; + + parent::__construct($baseUrl, $name); + } + + public function getToken(): string + { + return $this->token; + } +} diff --git a/src/Services/InputHandler.php b/src/Services/InputHandler.php index 12dee89..9c5ba09 100644 --- a/src/Services/InputHandler.php +++ b/src/Services/InputHandler.php @@ -4,6 +4,7 @@ use LinkORB\OrgSync\DTO\Group; use LinkORB\OrgSync\DTO\Organization; +use LinkORB\OrgSync\DTO\Target; use LinkORB\OrgSync\DTO\User; use LinkORB\OrgSync\Services\Target\TargetPool; use Symfony\Component\Serializer\Normalizer\DenormalizerInterface; @@ -67,6 +68,9 @@ public function handle(array $targets, array $organization): Organization return $organizationDto; } + /** + * @return Target[] + */ public function getTargets(): array { return $this->targetsPool->all(); @@ -120,4 +124,4 @@ private function handleGroupTargets(array $groupsTargets, Group $group): void ); } } -} \ No newline at end of file +} diff --git a/src/Services/SyncRemover/GithubSyncRemover.php b/src/Services/SyncRemover/GithubSyncRemover.php new file mode 100644 index 0000000..3335519 --- /dev/null +++ b/src/Services/SyncRemover/GithubSyncRemover.php @@ -0,0 +1,37 @@ +client = $client; + } + + public function removeNonExists(Organization $organization): void + { + $orgUsersToSync = []; + foreach ($organization->getUsers() as $user) { + $orgUsersToSync[$user->getUsername()] = $user; + } + + foreach ($this->client->teams() as $group) { + $members = $this->client->team()->members($group); + + foreach ($members as $member) { + if (!isset($orgUsersToSync[$member])) { + $this->client->team()->removeMember($group, $member); + } + } + } + } +} diff --git a/src/SynchronizationAdapter/AdapterFactory/AdapterFactoryInterface.php b/src/SynchronizationAdapter/AdapterFactory/AdapterFactoryInterface.php index c70e362..e868888 100644 --- a/src/SynchronizationAdapter/AdapterFactory/AdapterFactoryInterface.php +++ b/src/SynchronizationAdapter/AdapterFactory/AdapterFactoryInterface.php @@ -6,7 +6,6 @@ use LinkORB\OrgSync\Services\SyncRemover\SyncRemoverInterface; use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\GroupPushInterface; use LinkORB\OrgSync\SynchronizationAdapter\OrganizationPull\OrganizationPullInterface; -use LinkORB\OrgSync\SynchronizationAdapter\OrganizationPush\OrganizationPushInterface; use LinkORB\OrgSync\SynchronizationAdapter\SetPassword\SetPasswordInterface; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\UserPushInterface; @@ -18,11 +17,11 @@ public function createGroupPushAdapter(): GroupPushInterface; public function createOrganizationPullAdapter(): OrganizationPullInterface; - public function createOrganizationPushAdapter(): OrganizationPushInterface; - public function createSetPasswordAdapter(): SetPasswordInterface; public function createUserPushAdapter(): UserPushInterface; public function createSyncRemover(): SyncRemoverInterface; + + public function supports(string $action): bool; } diff --git a/src/SynchronizationAdapter/AdapterFactory/CamundaAdapterFactory.php b/src/SynchronizationAdapter/AdapterFactory/CamundaAdapterFactory.php index 48f101e..ed2630b 100644 --- a/src/SynchronizationAdapter/AdapterFactory/CamundaAdapterFactory.php +++ b/src/SynchronizationAdapter/AdapterFactory/CamundaAdapterFactory.php @@ -19,8 +19,6 @@ use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\CamundaGroupPushAdapter; use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\GroupPushInterface; use LinkORB\OrgSync\SynchronizationAdapter\OrganizationPull\OrganizationPullInterface; -use LinkORB\OrgSync\SynchronizationAdapter\OrganizationPush\CamundaOrganizationPushAdapter; -use LinkORB\OrgSync\SynchronizationAdapter\OrganizationPush\OrganizationPushInterface; use LinkORB\OrgSync\SynchronizationAdapter\SetPassword\CamundaSetPasswordAdapter; use LinkORB\OrgSync\SynchronizationAdapter\SetPassword\SetPasswordInterface; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\CamundaUserPushAdapter; @@ -89,11 +87,6 @@ public function createSetPasswordAdapter(): SetPasswordInterface ); } - public function createOrganizationPushAdapter(): OrganizationPushInterface - { - return new CamundaOrganizationPushAdapter(); - } - public function createSyncRemover(): SyncRemoverInterface { $userProvider = new CamundaUserProvider($this->camundaClient, new CamundaUserMapper()); @@ -104,6 +97,15 @@ public function createSyncRemover(): SyncRemoverInterface return new CamundaSyncRemover($userProvider, $groupProvider, $userGroupProvider, $this->camundaClient); } + public function supports(string $action): bool + { + return in_array($action, [ + Target::GROUP_PUSH, + Target::SET_PASSWORD, + Target::USER_PUSH, + ]); + } + protected function getClient(array $options): Client { return new Client($options); diff --git a/src/SynchronizationAdapter/AdapterFactory/GithubAdapterFactory.php b/src/SynchronizationAdapter/AdapterFactory/GithubAdapterFactory.php index 65d1134..95aba5a 100644 --- a/src/SynchronizationAdapter/AdapterFactory/GithubAdapterFactory.php +++ b/src/SynchronizationAdapter/AdapterFactory/GithubAdapterFactory.php @@ -2,50 +2,65 @@ namespace LinkORB\OrgSync\SynchronizationAdapter\AdapterFactory; +use BadMethodCallException; +use Github\Client; use LinkORB\OrgSync\DTO\Target; +use LinkORB\OrgSync\Services\SyncRemover\GithubSyncRemover; use LinkORB\OrgSync\Services\SyncRemover\SyncRemoverInterface; +use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\GithubGroupPushAdapter; use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\GroupPushInterface; use LinkORB\OrgSync\SynchronizationAdapter\OrganizationPull\OrganizationPullInterface; -use LinkORB\OrgSync\SynchronizationAdapter\OrganizationPush\OrganizationPushInterface; use LinkORB\OrgSync\SynchronizationAdapter\SetPassword\SetPasswordInterface; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\UserPushInterface; class GithubAdapterFactory implements AdapterFactoryInterface { - public const ADAPTER_KEY = 'github'; + /** + * @var Client + */ + private $client; - public function createOrganizationPullAdapter(): OrganizationPullInterface + public function __construct(Client $client) { - // TODO: Implement createOrganizationPullAdapter() method. + $this->client = $client; } public function setTarget(Target $target): AdapterFactoryInterface { - // TODO: Implement setTarget() method. + assert($target instanceof Target\Github); + + $this->client->authenticate($target->getToken()); + + return $this; + } + + public function createOrganizationPullAdapter(): OrganizationPullInterface + { + throw new BadMethodCallException('Not implemented yet'); } public function createGroupPushAdapter(): GroupPushInterface { - // TODO: Implement createGroupPushAdapter() method. + return new GithubGroupPushAdapter($this->client); } public function createUserPushAdapter(): UserPushInterface { - // TODO: Implement createUserPushAdapter() method. + throw new BadMethodCallException('Not implemented yet'); } public function createSetPasswordAdapter(): SetPasswordInterface { - // TODO: Implement createSetPasswordAdapter() method. + throw new BadMethodCallException('Not implemented yet'); } - public function createOrganizationPushAdapter(): OrganizationPushInterface + public function createSyncRemover(): SyncRemoverInterface { - // TODO: Implement createOrganizationPushAdapter() method. + return new GithubSyncRemover($this->client); } - public function createSyncRemover(): SyncRemoverInterface + public function supports(string $action): bool { - // TODO: Implement createSyncRemover() method. + return $action === Target::GROUP_PUSH; } } diff --git a/src/SynchronizationAdapter/GroupPush/CamundaGroupPushAdapter.php b/src/SynchronizationAdapter/GroupPush/CamundaGroupPushAdapter.php index b7ee9a9..90d480f 100644 --- a/src/SynchronizationAdapter/GroupPush/CamundaGroupPushAdapter.php +++ b/src/SynchronizationAdapter/GroupPush/CamundaGroupPushAdapter.php @@ -41,7 +41,6 @@ public function pushGroup(Group $group): GroupPushInterface $this->responseChecker->assertResponse($response); - // TODO: remove old members foreach ($group->getMembers() as $member) { $this->addMember($group->getName(), $member); } diff --git a/src/SynchronizationAdapter/GroupPush/GithubGroupPushAdapter.php b/src/SynchronizationAdapter/GroupPush/GithubGroupPushAdapter.php new file mode 100644 index 0000000..0ba295e --- /dev/null +++ b/src/SynchronizationAdapter/GroupPush/GithubGroupPushAdapter.php @@ -0,0 +1,40 @@ +client = $client; + } + + public function pushGroup(Group $group): GroupPushInterface + { + try { + $params = [ + 'name' => $group->getName(), + 'parent_team_id' => $group->getParent() ? $group->getParent()->getName() : null, + ]; + + $this->client->team()->update($group->getName(), $params); + } catch (Exception $e) { + $this->client->team()->create($group->getName(), $params ?? []); + } + + foreach ($group->getMembers() as $member) { + $this->client->team()->addMember($group->getName(), $member->getUsername()); + } + + return $this; + } +} diff --git a/src/SynchronizationAdapter/OrganizationPush/CamundaOrganizationPushAdapter.php b/src/SynchronizationAdapter/OrganizationPush/CamundaOrganizationPushAdapter.php deleted file mode 100644 index 4febcce..0000000 --- a/src/SynchronizationAdapter/OrganizationPush/CamundaOrganizationPushAdapter.php +++ /dev/null @@ -1,14 +0,0 @@ -client = $client; - } - - public function pushUser(User $user): UserPushInterface - { - - return $this; - } -} diff --git a/src/SynchronizationMediator/SynchronizationMediator.php b/src/SynchronizationMediator/SynchronizationMediator.php index 2f6847a..bc38cf3 100644 --- a/src/SynchronizationMediator/SynchronizationMediator.php +++ b/src/SynchronizationMediator/SynchronizationMediator.php @@ -3,6 +3,7 @@ namespace LinkORB\OrgSync\SynchronizationMediator; use LinkORB\OrgSync\DTO\Target; +use LinkORB\OrgSync\Exception\SyncTargetException; use LinkORB\OrgSync\Services\InputHandler; use LinkORB\OrgSync\SynchronizationAdapter\AdapterFactory\AdapterFactoryInterface; use LinkORB\OrgSync\SynchronizationAdapter\AdapterFactory\AdapterFactoryPoolInterface; @@ -46,15 +47,18 @@ public function setTarget(Target $target): SynchronizationMediatorInterface public function pushOrganization(Organization $organization): SynchronizationMediatorInterface { + foreach ($this->inputHandler->getTargets() as $target) { + $this->setTarget($target); + + $this->adapterFactory->createSyncRemover()->removeNonExists($organization); + } + foreach ($this->inputHandler->getTargets() as $target) { $this->setTarget($target); foreach ($organization->getUsers() as $user) { $this->pushUser($user); } - - $this->adapterFactory->createSyncRemover()->removeNonExists($organization); - $this->adapterFactory->createOrganizationPushAdapter()->pushOrganization($organization); } foreach ($organization->getGroups() as $group) { @@ -72,14 +76,18 @@ public function pushOrganization(Organization $organization): SynchronizationMed public function pushGroup(Group $group): SynchronizationMediatorInterface { - $this->adapterFactory->createGroupPushAdapter()->pushGroup($group); + if ($this->adapterFactory->supports(Target::GROUP_PUSH)) { + $this->adapterFactory->createGroupPushAdapter()->pushGroup($group); + } return $this; } public function pushUser(User $user): SynchronizationMediatorInterface { - $this->adapterFactory->createUserPushAdapter()->pushUser($user); + if ($this->adapterFactory->supports(Target::USER_PUSH)) { + $this->adapterFactory->createUserPushAdapter()->pushUser($user); + } return $this; } @@ -89,6 +97,10 @@ public function setPassword(User $user, string $password): SynchronizationMediat foreach ($this->inputHandler->getTargets() as $target) { $this->setTarget($target); + if (!$this->adapterFactory->supports(Target::SET_PASSWORD)) { + continue; + } + $this->adapterFactory->createSetPasswordAdapter()->setPassword($user, $password); } @@ -99,6 +111,12 @@ public function setPassword(User $user, string $password): SynchronizationMediat public function pullOrganization(): Organization { + if (!$this->adapterFactory->supports(Target::PULL_ORGANIZATION)) { + throw new SyncTargetException( + sprintf('Current target doesn\'t support \'%s\' action', Target::PULL_ORGANIZATION) + ); + } + return $this->adapterFactory->createOrganizationPullAdapter()->pullOrganization(); } } diff --git a/tests/Unit/DTO/Target/GithubTest.php b/tests/Unit/DTO/Target/GithubTest.php new file mode 100644 index 0000000..5eeccbe --- /dev/null +++ b/tests/Unit/DTO/Target/GithubTest.php @@ -0,0 +1,35 @@ + '', + 'name' => '', + 'token' => '', + ]; + } + + public function getDtoClassName(): string + { + return Github::class; + } +} diff --git a/tests/Unit/Services/SyncRemover/CamundaSyncRemoverTest.php b/tests/Unit/Services/SyncRemover/CamundaSyncRemoverTest.php index 64945b2..82d0725 100644 --- a/tests/Unit/Services/SyncRemover/CamundaSyncRemoverTest.php +++ b/tests/Unit/Services/SyncRemover/CamundaSyncRemoverTest.php @@ -4,6 +4,7 @@ use GuzzleHttp\Client; use GuzzleHttp\RequestOptions; +use LinkORB\OrgSync\DTO\Target; use LinkORB\OrgSync\DTO\User; use LinkORB\OrgSync\Services\Camunda\CamundaGroupMemberProvider; use LinkORB\OrgSync\Services\Camunda\CamundaGroupProvider; diff --git a/tests/Unit/Services/SyncRemover/GithubSyncRemoverTest.php b/tests/Unit/Services/SyncRemover/GithubSyncRemoverTest.php new file mode 100644 index 0000000..9b05685 --- /dev/null +++ b/tests/Unit/Services/SyncRemover/GithubSyncRemoverTest.php @@ -0,0 +1,101 @@ +client = $this->createMock(Client::class); + + $this->remover = new GithubSyncRemover($this->client); + + parent::setUp(); + } + + /** + * @dataProvider getRemoveData + */ + public function testRemoveNonExists(array $usersArray, array $teamMembers) + { + $users = array_map( + function (string $username) { + return new User($username); + }, + $usersArray + ); + + $expectations = []; + foreach ($teamMembers as $team => $members) { + foreach ($members as $member) { + if (!in_array($member, $usersArray)) { + $expectations[] = [$team, $member]; + } + } + } + + $team = $this->createMock(Teams::class); + + $this->client->method('__call') + ->willReturnMap([ + ['teams', [], array_keys($teamMembers)], + ['team', [], $team] + ]); + + $team + ->expects($this->exactly(count($teamMembers))) + ->method('members') + ->withConsecutive(...array_map(function (string $teamName) { + return [$teamName]; + }, array_keys($teamMembers))) + ->willReturnOnConsecutiveCalls(...array_values($teamMembers)); + + $team + ->expects($this->exactly(count($expectations))) + ->method('removeMember') + ->withConsecutive(...$expectations); + + $this->remover->removeNonExists(new Organization('', $users)); + } + + public function getRemoveData(): array + { + return [ + [ + ['test1', 'test2', 'test3'], + [ + 'org1' => [ + 'test1', + 'test5', + 'test2', + ], + 'org2' => [ + 'test3', + ], + 'org3' => [ + 'test5', + 'test6', + ] + ] + ] + ]; + } +} diff --git a/tests/Unit/SynchronizationAdapter/AdapterFactory/CamundaAdapterFactoryTest.php b/tests/Unit/SynchronizationAdapter/AdapterFactory/CamundaAdapterFactoryTest.php index ba3a9bc..9a2eb68 100644 --- a/tests/Unit/SynchronizationAdapter/AdapterFactory/CamundaAdapterFactoryTest.php +++ b/tests/Unit/SynchronizationAdapter/AdapterFactory/CamundaAdapterFactoryTest.php @@ -3,11 +3,11 @@ namespace LinkORB\OrgSync\Tests\Unit\SynchronizationAdapter\AdapterFactory; use GuzzleHttp\Client; +use LinkORB\OrgSync\DTO\Target; use LinkORB\OrgSync\DTO\Target\Camunda; use LinkORB\OrgSync\SynchronizationAdapter\AdapterFactory\CamundaAdapterFactory; use LinkORB\OrgSync\Services\PasswordHelper; use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\CamundaGroupPushAdapter; -use LinkORB\OrgSync\SynchronizationAdapter\OrganizationPush\CamundaOrganizationPushAdapter; use LinkORB\OrgSync\SynchronizationAdapter\SetPassword\CamundaSetPasswordAdapter; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\CamundaUserPushAdapter; use PHPUnit\Framework\MockObject\MockObject; @@ -86,11 +86,6 @@ public function testCreateSetPasswordAdapter() $this->assertInstanceOf(CamundaSetPasswordAdapter::class, $this->factory->createSetPasswordAdapter()); } - public function testCreateOrganizationPushAdapter() - { - $this->assertInstanceOf(CamundaOrganizationPushAdapter::class, $this->factory->createOrganizationPushAdapter()); - } - public function testCreateGroupPushAdapter() { $this->assertInstanceOf(CamundaGroupPushAdapter::class, $this->factory->createGroupPushAdapter()); @@ -105,4 +100,23 @@ public function getAdapterFactoryData(): array ['https://temp.nl', 'user', 'p@ssword'], ]; } + + /** + * @dataProvider getSupportsData + */ + public function testSupports(string $action, bool $expected) + { + $this->assertEquals($expected, $this->factory->supports($action)); + } + + public function getSupportsData(): array + { + return [ + [Target::GROUP_PUSH, true], + [Target::PULL_ORGANIZATION, false], + [Target::USER_PUSH, true], + [Target::SET_PASSWORD, true], + [Target::class, false], + ]; + } } diff --git a/tests/Unit/SynchronizationAdapter/AdapterFactory/GithubAdapterFactoryTest.php b/tests/Unit/SynchronizationAdapter/AdapterFactory/GithubAdapterFactoryTest.php new file mode 100644 index 0000000..89af959 --- /dev/null +++ b/tests/Unit/SynchronizationAdapter/AdapterFactory/GithubAdapterFactoryTest.php @@ -0,0 +1,90 @@ +client = $this->createMock(Client::class); + + $this->factory = new GithubAdapterFactory($this->client); + + parent::setUp(); + } + + public function testCreateUserPushAdapter() + { + $this->expectException(\BadMethodCallException::class); + + $this->factory->createUserPushAdapter(); + } + + public function testCreateOrganizationPullAdapter() + { + $this->expectException(\BadMethodCallException::class); + + $this->factory->createOrganizationPullAdapter(); + } + + public function testCreateSetPasswordAdapter() + { + $this->expectException(\BadMethodCallException::class); + + $this->factory->createSetPasswordAdapter(); + } + + public function testCreateGroupPushAdapter() + { + $this->assertInstanceOf(GithubGroupPushAdapter::class, $this->factory->createGroupPushAdapter()); + } + + public function testSetTarget() + { + $token = '123qwetest'; + + $this->client->expects($this->once())->method('authenticate')->with($token); + + $this->factory->setTarget(new Target\Github('', '', $token)); + } + + public function testCreateSyncRemover() + { + $this->assertInstanceOf(GithubSyncRemover::class, $this->factory->createSyncRemover()); + } + + /** + * @dataProvider getSupportsData + */ + public function testSupports(string $action, bool $expected) + { + $this->assertEquals($expected, $this->factory->supports($action)); + } + + public function getSupportsData(): array + { + return [ + [Target::GROUP_PUSH, true], + [Target::PULL_ORGANIZATION, false], + [Target::USER_PUSH, false], + ]; + } +} diff --git a/tests/Unit/SynchronizationAdapter/GroupPush/GithubGroupPushAdapterTest.php b/tests/Unit/SynchronizationAdapter/GroupPush/GithubGroupPushAdapterTest.php new file mode 100644 index 0000000..c489a75 --- /dev/null +++ b/tests/Unit/SynchronizationAdapter/GroupPush/GithubGroupPushAdapterTest.php @@ -0,0 +1,89 @@ +client = $this->createMock(Client::class); + + $this->groupPush = new GithubGroupPushAdapter($this->client); + + parent::setUp(); + } + + public function testCreateGroup() + { + $name = 'testing'; + $membersData = [ + 'Tom', + 'Jerry', + 'Spike', + ]; + + $params = [ + 'name' => $name, + 'parent_team_id' => null, + ]; + + $group = new Group($name, '', null, null, array_map(function (string $username) { + return new User($username); + }, $membersData)); + + $team = $this->createMock(Teams::class); + + $this->client->method('__call') + ->with('team', []) + ->willReturn($team); + + $team->expects($this->once()) + ->method('update') + ->with($name, $params) + ->willThrowException(new TransferException()); + $team->expects($this->once())->method('create')->with($name, $params); + + $team->expects($this->exactly(count($membersData))) + ->method('addMember') + ->withConsecutive(...array_map(function (string $username) use ($name) { + return [$name, $username]; + }, $membersData)); + + $this->assertSame($this->groupPush, $this->groupPush->pushGroup($group)); + } + + public function testUpdate() + { + $group = new Group('name', ''); + + $team = $this->createMock(Teams::class); + + $this->client->method('__call') + ->with('team', []) + ->willReturn($team); + + $team->expects($this->once())->method('update'); + $team->expects($this->never())->method('create'); + + $this->assertSame($this->groupPush, $this->groupPush->pushGroup($group)); + } +} diff --git a/tests/Unit/SynchronizationAdapter/UserPush/GithubUserPushAdapterTest.php b/tests/Unit/SynchronizationAdapter/UserPush/GithubUserPushAdapterTest.php deleted file mode 100644 index 704a0d6..0000000 --- a/tests/Unit/SynchronizationAdapter/UserPush/GithubUserPushAdapterTest.php +++ /dev/null @@ -1,36 +0,0 @@ -client = $this->createMock(Client::class); - $this->githubPushAdapter = new GithubUserPushAdapter($this->client); - - parent::setUp(); - } - - public function testPush() - { - $this->markTestSkipped(); - $user = $this->createMock(User::class); - -// $this->client->user()-> - - $this->assertSame($this->githubPushAdapter, $this->githubPushAdapter->pushUser($user)); - } -} diff --git a/tests/Unit/SynchronizationMediator/SynchronizationMediatorTest.php b/tests/Unit/SynchronizationMediator/SynchronizationMediatorTest.php index ef6bd5a..84426e8 100644 --- a/tests/Unit/SynchronizationMediator/SynchronizationMediatorTest.php +++ b/tests/Unit/SynchronizationMediator/SynchronizationMediatorTest.php @@ -2,7 +2,9 @@ namespace LinkORB\OrgSync\Tests\Unit\SynchronizationMediator; +use LinkORB\OrgSync\DTO\Target; use LinkORB\OrgSync\DTO\Target\Camunda; +use LinkORB\OrgSync\Exception\SyncTargetException; use LinkORB\OrgSync\Services\InputHandler; use LinkORB\OrgSync\Services\SyncRemover\SyncRemoverInterface; use LinkORB\OrgSync\SynchronizationAdapter\AdapterFactory\AdapterFactoryInterface; @@ -30,9 +32,6 @@ class SynchronizationMediatorTest extends TestCase /** @var AdapterFactoryPoolInterface|MockObject */ private $adapterFactoryPool; - /** @var OrganizationPushInterface|MockObject */ - private $organizationPushAdapter; - /** @var GroupPushInterface|MockObject */ private $groupPushAdapter; @@ -54,7 +53,6 @@ class SynchronizationMediatorTest extends TestCase protected function setUp(): void { $this->inputHandler = $this->createMock(InputHandler::class); - $this->organizationPushAdapter = $this->createMock(OrganizationPushInterface::class); $this->groupPushAdapter = $this->createMock(GroupPushInterface::class); $this->userPushAdapter = $this->createMock(UserPushInterface::class); $this->setPasswordAdapter = $this->createMock(SetPasswordInterface::class); @@ -62,7 +60,6 @@ protected function setUp(): void $this->syncRemover = $this->createMock(SyncRemoverInterface::class); $this->adapterFactory = $this->createConfiguredMock(AdapterFactoryInterface::class, [ - 'createOrganizationPushAdapter' => $this->organizationPushAdapter, 'createGroupPushAdapter' => $this->groupPushAdapter, 'createUserPushAdapter' => $this->userPushAdapter, 'createSetPasswordAdapter' => $this->setPasswordAdapter, @@ -102,7 +99,7 @@ public function testPushOrganization() new Camunda('local', 'adapter', 'test', 'user'), ]; - $this->inputHandler->expects($this->once())->method('getTargets')->willReturn($targets); + $this->inputHandler->expects($this->exactly(2))->method('getTargets')->willReturn($targets); $users = [$this->createMock(User::class), $this->createMock(User::class), $this->createMock(User::class)]; $groups = [$this->createConfiguredMock(Group::class, ['getTargets' => $targets])]; $organization = $this->createConfiguredMock(Organization::class, [ @@ -115,11 +112,11 @@ public function testPushOrganization() }, $targets); foreach ($groups as $group) { - $targetsConsecutive = array_merge($targetsConsecutive, $targetsConsecutive); + $targetsConsecutive = array_merge($targetsConsecutive, $targetsConsecutive, $targetsConsecutive); } $this->adapterFactoryPool - ->expects($this->exactly(count($targets) * (count($groups) + 1))) + ->expects($this->exactly(count($targets) * (count($groups) + 2))) ->method('get') ->withConsecutive(...$targetsConsecutive); @@ -137,26 +134,43 @@ public function testPushOrganization() ->withConsecutive(...$groupsConsecutive); $this->adapterFactory->expects($this->exactly(count($targets)))->method('createSyncRemover'); - $this->adapterFactory->expects($this->exactly(count($targets)))->method('createOrganizationPushAdapter'); $this->syncRemover ->expects($this->exactly(count($targets))) ->method('removeNonExists') ->with($organization); - $this->organizationPushAdapter - ->expects($this->exactly(count($targets))) - ->method('pushOrganization') - ->with($organization)->willReturnSelf(); - $this->assertSame($this->mediator, $this->mediator->pushOrganization($organization)); } + public function testOrganizationPushNotSupported() + { + $targets = [ + new Camunda('', ''), + new Camunda('', ''), + ]; + + $this->inputHandler->method('getTargets')->willReturn($targets); + + $users = [$this->createMock(User::class), $this->createMock(User::class), $this->createMock(User::class)]; + $groups = [$this->createConfiguredMock(Group::class, ['getTargets' => $targets])]; + + $this->adapterFactory->method('supports')->willReturn(false); + $this->adapterFactory->expects($this->never())->method('createUserPushAdapter'); + $this->adapterFactory->expects($this->never())->method('createGroupPushAdapter'); + + $this->mediator->pushOrganization($this->createConfiguredMock(Organization::class, [ + 'getUsers' => $users, + 'getGroups' => $groups, + ])); + } + public function testPushGroup() { $group = $this->createMock(Group::class); $this->adapterFactory->expects($this->once())->method('createGroupPushAdapter'); + $this->adapterFactory->expects($this->once())->method('supports')->with(Target::GROUP_PUSH)->willReturn(true); $this->groupPushAdapter->expects($this->once())->method('pushGroup')->with($group)->willReturnSelf(); @@ -171,6 +185,7 @@ public function testPushUser() $user = $this->createMock(User::class); $this->adapterFactory->expects($this->once())->method('createUserPushAdapter'); + $this->adapterFactory->expects($this->once())->method('supports')->with(Target::USER_PUSH)->willReturn(true); $this->userPushAdapter->expects($this->once())->method('pushUser')->with($user)->willReturnSelf(); @@ -185,14 +200,39 @@ public function testSetPassword() $user = $this->createMock(User::class); $password = '1234Qwer'; + $targets = [ + $this->createMock(Camunda::class), + $this->createMock(Camunda::class), + $this->createMock(Camunda::class), + $this->createMock(Camunda::class), + ]; + + $targetSupports = array_map(function (){return (bool) random_int(0, 1);}, $targets); + + $expectedTargets = []; + foreach ($targets as $key => $target) { + if ($targetSupports[$key]) { + $expectedTargets[] = $target; + } + } + $this->inputHandler ->expects($this->once()) ->method('getTargets') - ->willReturn([$this->createMock(Camunda::class)]); + ->willReturn($targets); - $this->adapterFactory->expects($this->once())->method('createSetPasswordAdapter'); + $this->adapterFactory->expects($this->exactly(count($expectedTargets)))->method('createSetPasswordAdapter'); + $this->adapterFactory + ->expects($this->exactly(count($targetSupports))) + ->method('supports') + ->with(Target::SET_PASSWORD) + ->willReturnOnConsecutiveCalls(...$targetSupports); - $this->setPasswordAdapter->expects($this->once())->method('setPassword')->with($user, $password)->willReturnSelf(); + $this->setPasswordAdapter + ->expects($this->exactly(count($expectedTargets))) + ->method('setPassword') + ->with($user, $password) + ->willReturnSelf(); $this->assertSame( $this->mediator, @@ -203,6 +243,11 @@ public function testSetPassword() public function testPullOrganization() { $this->adapterFactory->expects($this->once())->method('createOrganizationPullAdapter'); + $this->adapterFactory + ->expects($this->once()) + ->method('supports') + ->with(Target::PULL_ORGANIZATION) + ->willReturn(true); $organization = $this->createMock(Organization::class); @@ -213,4 +258,17 @@ public function testPullOrganization() $this->mediator->setTarget($this->createMock(Camunda::class))->pullOrganization() ); } + + public function testPullNotSupported() + { + $this->adapterFactory + ->expects($this->once()) + ->method('supports') + ->with(Target::PULL_ORGANIZATION) + ->willReturn(false); + + $this->expectException(SyncTargetException::class); + + $this->mediator->setTarget($this->createMock(Camunda::class))->pullOrganization(); + } } From dfb61ebae1b8cdc63d11ce2ea6355cbfe4691035 Mon Sep 17 00:00:00 2001 From: amsprost Date: Tue, 27 Aug 2019 00:52:28 +0200 Subject: [PATCH 02/13] Fix github integration, org-sync --- src/DTO/Group.php | 7 +++ src/DTO/Target.php | 1 + .../Github/AuthenticatedClientDecorator.php | 27 +++++++++++ src/Services/InputHandler.php | 4 ++ .../SyncRemover/GithubSyncRemover.php | 19 ++++++-- .../AdapterFactory/GithubAdapterFactory.php | 2 +- .../AdapterFactory/LdapAdapterFactory.php | 13 ++---- .../MatterMostAdapterFactory.php | 13 ++---- .../GroupPush/GithubGroupPushAdapter.php | 16 ++++--- tests/Helpers/OrganizationDataProvider.php | 15 +++++- tests/Unit/DTO/GroupTest.php | 23 ++++++++++ tests/Unit/Services/InputHandlerTest.php | 7 ++- .../SyncRemover/GithubSyncRemoverTest.php | 46 ++++++++++++++++--- .../GithubAdapterFactoryTest.php | 2 +- .../GroupPush/GithubGroupPushAdapterTest.php | 11 +++-- 15 files changed, 165 insertions(+), 41 deletions(-) create mode 100644 src/Services/Github/AuthenticatedClientDecorator.php diff --git a/src/DTO/Group.php b/src/DTO/Group.php index d6439a7..738f9fc 100644 --- a/src/DTO/Group.php +++ b/src/DTO/Group.php @@ -115,6 +115,13 @@ public function getProperties(): array return $this->properties; } + public function addProperty(string $key, string $value, bool $override = true): Group + { + $this->properties[$key] = ($override || !isset($this->properties[$key])) ? $value : $this->properties[$key]; + + return $this; + } + /** * @return Target[] */ diff --git a/src/DTO/Target.php b/src/DTO/Target.php index c118a4c..1f25c60 100644 --- a/src/DTO/Target.php +++ b/src/DTO/Target.php @@ -7,6 +7,7 @@ /** * @DiscriminatorMap(typeProperty="type", mapping={ * "camunda"="LinkORB\OrgSync\DTO\Target\Camunda", + * "github"="LinkORB\OrgSync\DTO\Target\Github", * }) */ abstract class Target diff --git a/src/Services/Github/AuthenticatedClientDecorator.php b/src/Services/Github/AuthenticatedClientDecorator.php new file mode 100644 index 0000000..189fc36 --- /dev/null +++ b/src/Services/Github/AuthenticatedClientDecorator.php @@ -0,0 +1,27 @@ +token, Client::AUTH_HTTP_TOKEN); + + return parent::__call($name, $args); + } + + public function authenticate($tokenOrLogin, $password = null, $authMethod = null) + { + $this->token = $tokenOrLogin; + + parent::authenticate($tokenOrLogin, $password, $authMethod); + } +} diff --git a/src/Services/InputHandler.php b/src/Services/InputHandler.php index 9c5ba09..ce862fb 100644 --- a/src/Services/InputHandler.php +++ b/src/Services/InputHandler.php @@ -11,6 +11,8 @@ class InputHandler { + public const GITHUB_ORGANIZATION = 'github_organization'; + /** @var TargetPool */ private $targetsPool; @@ -60,6 +62,8 @@ public function handle(array $targets, array $organization): Organization ); foreach ($organizationDto->getGroups() as $group) { + $group->addProperty(static::GITHUB_ORGANIZATION, $organizationDto->getName(), false); + $this->handleGroupParents($groupsParents, $group, $organizationDto); $this->handleGroupMembers($groupsMembers, $group, $organizationDto); $this->handleGroupTargets($groupsTargets, $group); diff --git a/src/Services/SyncRemover/GithubSyncRemover.php b/src/Services/SyncRemover/GithubSyncRemover.php index 3335519..a305913 100644 --- a/src/Services/SyncRemover/GithubSyncRemover.php +++ b/src/Services/SyncRemover/GithubSyncRemover.php @@ -19,17 +19,30 @@ public function __construct(Client $client) public function removeNonExists(Organization $organization): void { + $groupsToSync = []; + foreach ($organization->getGroups() as $group) { + $groupsToSync[$group->getName()] = $group; + } + $orgUsersToSync = []; foreach ($organization->getUsers() as $user) { $orgUsersToSync[$user->getUsername()] = $user; } - foreach ($this->client->teams() as $group) { - $members = $this->client->team()->members($group); + foreach ($this->client->teams()->all($organization->getName()) as $group) { + $teamName = $group['name']; + + if (!isset($groupsToSync[$teamName])) { + $this->client->team()->remove($teamName); + + continue; + } + + $members = $this->client->team()->members($teamName); foreach ($members as $member) { if (!isset($orgUsersToSync[$member])) { - $this->client->team()->removeMember($group, $member); + $this->client->team()->removeMember($teamName, $member); } } } diff --git a/src/SynchronizationAdapter/AdapterFactory/GithubAdapterFactory.php b/src/SynchronizationAdapter/AdapterFactory/GithubAdapterFactory.php index 95aba5a..8ed4d5b 100644 --- a/src/SynchronizationAdapter/AdapterFactory/GithubAdapterFactory.php +++ b/src/SynchronizationAdapter/AdapterFactory/GithubAdapterFactory.php @@ -29,7 +29,7 @@ public function setTarget(Target $target): AdapterFactoryInterface { assert($target instanceof Target\Github); - $this->client->authenticate($target->getToken()); + $this->client->authenticate($target->getToken(), Client::AUTH_HTTP_TOKEN); return $this; } diff --git a/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php b/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php index 4f6aec0..9e5fc7f 100644 --- a/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php +++ b/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php @@ -6,14 +6,11 @@ use LinkORB\OrgSync\Services\SyncRemover\SyncRemoverInterface; use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\GroupPushInterface; use LinkORB\OrgSync\SynchronizationAdapter\OrganizationPull\OrganizationPullInterface; -use LinkORB\OrgSync\SynchronizationAdapter\OrganizationPush\OrganizationPushInterface; use LinkORB\OrgSync\SynchronizationAdapter\SetPassword\SetPasswordInterface; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\UserPushInterface; class LdapAdapterFactory implements AdapterFactoryInterface { - public const ADAPTER_KEY = 'ldap'; - public function createOrganizationPullAdapter(): OrganizationPullInterface { // TODO: Implement createOrganizationPullAdapter() method. @@ -34,11 +31,6 @@ public function createSetPasswordAdapter(): SetPasswordInterface // TODO: Implement createSetPasswordAdapter() method. } - public function createOrganizationPushAdapter(): OrganizationPushInterface - { - // TODO: Implement createOrganizationPushAdapter() method. - } - public function setTarget(Target $target): AdapterFactoryInterface { // TODO: Implement setTarget() method. @@ -48,4 +40,9 @@ public function createSyncRemover(): SyncRemoverInterface { // TODO: Implement createSyncRemover() method. } + + public function supports(string $action): bool + { + return false; + } } diff --git a/src/SynchronizationAdapter/AdapterFactory/MatterMostAdapterFactory.php b/src/SynchronizationAdapter/AdapterFactory/MatterMostAdapterFactory.php index c733f1e..b8b5142 100644 --- a/src/SynchronizationAdapter/AdapterFactory/MatterMostAdapterFactory.php +++ b/src/SynchronizationAdapter/AdapterFactory/MatterMostAdapterFactory.php @@ -6,14 +6,11 @@ use LinkORB\OrgSync\Services\SyncRemover\SyncRemoverInterface; use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\GroupPushInterface; use LinkORB\OrgSync\SynchronizationAdapter\OrganizationPull\OrganizationPullInterface; -use LinkORB\OrgSync\SynchronizationAdapter\OrganizationPush\OrganizationPushInterface; use LinkORB\OrgSync\SynchronizationAdapter\SetPassword\SetPasswordInterface; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\UserPushInterface; class MatterMostAdapterFactory implements AdapterFactoryInterface { - public const ADAPTER_KEY = 'mattermost'; - public function createOrganizationPullAdapter(): OrganizationPullInterface { // TODO: Implement createOrganizationPullAdapter() method. @@ -34,11 +31,6 @@ public function createSetPasswordAdapter(): SetPasswordInterface // TODO: Implement createSetPasswordAdapter() method. } - public function createOrganizationPushAdapter(): OrganizationPushInterface - { - // TODO: Implement createOrganizationPushAdapter() method. - } - public function setTarget(Target $target): AdapterFactoryInterface { // TODO: Implement setTarget() method. @@ -48,4 +40,9 @@ public function createSyncRemover(): SyncRemoverInterface { // TODO: Implement createSyncRemover() method. } + + public function supports(string $action): bool + { + return false; + } } diff --git a/src/SynchronizationAdapter/GroupPush/GithubGroupPushAdapter.php b/src/SynchronizationAdapter/GroupPush/GithubGroupPushAdapter.php index 0ba295e..74a3675 100644 --- a/src/SynchronizationAdapter/GroupPush/GithubGroupPushAdapter.php +++ b/src/SynchronizationAdapter/GroupPush/GithubGroupPushAdapter.php @@ -5,6 +5,7 @@ use Github\Client; use Http\Client\Exception; use LinkORB\OrgSync\DTO\Group; +use LinkORB\OrgSync\Services\InputHandler; class GithubGroupPushAdapter implements GroupPushInterface { @@ -20,15 +21,18 @@ public function __construct(Client $client) public function pushGroup(Group $group): GroupPushInterface { + assert(!empty($group->getProperties()[InputHandler::GITHUB_ORGANIZATION])); + try { - $params = [ - 'name' => $group->getName(), - 'parent_team_id' => $group->getParent() ? $group->getParent()->getName() : null, - ]; + $params = ['name' => $group->getName()]; + + if ($group->getParent()) { + $params['parent_team_id'] = $group->getParent()->getName(); + } - $this->client->team()->update($group->getName(), $params); + $this->client->team()->update($group->getProperties()[InputHandler::GITHUB_ORGANIZATION], $params); } catch (Exception $e) { - $this->client->team()->create($group->getName(), $params ?? []); + $this->client->team()->create($group->getProperties()[InputHandler::GITHUB_ORGANIZATION], $params ?? []); } foreach ($group->getMembers() as $member) { diff --git a/tests/Helpers/OrganizationDataProvider.php b/tests/Helpers/OrganizationDataProvider.php index 5b20464..4cdd8d5 100644 --- a/tests/Helpers/OrganizationDataProvider.php +++ b/tests/Helpers/OrganizationDataProvider.php @@ -7,6 +7,7 @@ use LinkORB\OrgSync\DTO\Target; use LinkORB\OrgSync\DTO\Target\Camunda; use LinkORB\OrgSync\DTO\User; +use LinkORB\OrgSync\Services\InputHandler; class OrganizationDataProvider { @@ -128,19 +129,29 @@ public static function transformToTargets(array $targetsArray): array * @param Target[] $targets * @return Group[] */ - public static function transformToGroups(array $groupsArray, array $users, array $targets): array + public static function transformToGroups( + array $groupsArray, + array $users, + array $targets, + string $orgName = null + ): array { /** @var Group[] $groups */ $groups = []; $groupParents = []; foreach ($groupsArray as $name => $group) { + $props = $group['properties'] ?? []; + if ($orgName) { + $props[InputHandler::GITHUB_ORGANIZATION] = $orgName; + } + $groups[] = new Group( $name, $group['displayName'], $group['avatar'], null, [], - $group['properties'] ?? [] + $props ); foreach ($group['targets'] ?? [] as $linkedTarget) { diff --git a/tests/Unit/DTO/GroupTest.php b/tests/Unit/DTO/GroupTest.php index 3b59348..b104111 100644 --- a/tests/Unit/DTO/GroupTest.php +++ b/tests/Unit/DTO/GroupTest.php @@ -50,4 +50,27 @@ public function getDtoClassName(): string { return Group::class; } + + /** + * @dataProvider getAddPropertyData + */ + public function testAddProperty(string $key, $value, $expected, bool $override, array $defaultProps) + { + $group = new Group('', '', null, null, [], $defaultProps); + + $group->addProperty($key, $value, $override); + + $this->assertEquals($expected, $group->getProperties()[$key]); + } + + public function getAddPropertyData(): array + { + return [ + ['test', 1, 1, true, [],], + ['test1', false, false, true, ['test1' => true],], + ['test2', false, true, false, ['test2' => true, 'test1' => false,]], + ['qwerty', 'temp', 'temp', false, ['test1' => false,]], + ['qwerty', '123', 'temp', false, ['qwerty' => 'temp',]], + ]; + } } diff --git a/tests/Unit/Services/InputHandlerTest.php b/tests/Unit/Services/InputHandlerTest.php index 9dbd2a7..6304996 100644 --- a/tests/Unit/Services/InputHandlerTest.php +++ b/tests/Unit/Services/InputHandlerTest.php @@ -50,7 +50,12 @@ public function testDenormalization(array $targets, array $organization) $targetDtos = OrganizationDataProvider::transformToTargets($targets); /** @var User[] $users */ $userDtos = OrganizationDataProvider::transformToUsers($organization['users']); - $groups = OrganizationDataProvider::transformToGroups($organization['groups'], $userDtos, $targetDtos); + $groups = OrganizationDataProvider::transformToGroups( + $organization['groups'], + $userDtos, + $targetDtos, + $organization['name'] + ); $this->assertEquals( new Organization($organization['name'], $userDtos, $groups), diff --git a/tests/Unit/Services/SyncRemover/GithubSyncRemoverTest.php b/tests/Unit/Services/SyncRemover/GithubSyncRemoverTest.php index 9b05685..c08d4ad 100644 --- a/tests/Unit/Services/SyncRemover/GithubSyncRemoverTest.php +++ b/tests/Unit/Services/SyncRemover/GithubSyncRemoverTest.php @@ -4,6 +4,7 @@ use Github\Api\Organization\Teams; use Github\Client; +use LinkORB\OrgSync\DTO\Group; use LinkORB\OrgSync\DTO\Organization; use LinkORB\OrgSync\DTO\User; use LinkORB\OrgSync\Services\SyncRemover\GithubSyncRemover; @@ -34,8 +35,13 @@ protected function setUp(): void /** * @dataProvider getRemoveData */ - public function testRemoveNonExists(array $usersArray, array $teamMembers) + public function testRemoveNonExists(array $usersArray, array $teamMembers, array $orgGroups) { + $orgName = 'TempOrg'; + + $orgGroupDtos = array_map(function (string $name) { + return new Group($name, ''); + }, $orgGroups); $users = array_map( function (string $username) { return new User($username); @@ -45,6 +51,10 @@ function (string $username) { $expectations = []; foreach ($teamMembers as $team => $members) { + if (!in_array($team, $orgGroups)) { + continue; + } + foreach ($members as $member) { if (!in_array($member, $usersArray)) { $expectations[] = [$team, $member]; @@ -53,27 +63,45 @@ function (string $username) { } $team = $this->createMock(Teams::class); + $teams = $this->createConfiguredMock( + Teams::class, + [ + 'all' => array_map(function (string $name) { + return ['name' => $name]; + }, array_keys($teamMembers)) + ] + ); $this->client->method('__call') ->willReturnMap([ - ['teams', [], array_keys($teamMembers)], + ['teams', [], $teams], ['team', [], $team] ]); + $membersExpectations = array_intersect_key($teamMembers, array_flip($orgGroups)); $team - ->expects($this->exactly(count($teamMembers))) + ->expects($this->exactly(count($membersExpectations))) ->method('members') ->withConsecutive(...array_map(function (string $teamName) { return [$teamName]; - }, array_keys($teamMembers))) - ->willReturnOnConsecutiveCalls(...array_values($teamMembers)); + }, array_keys($membersExpectations))) + ->willReturnOnConsecutiveCalls(...array_values($membersExpectations)); $team ->expects($this->exactly(count($expectations))) ->method('removeMember') ->withConsecutive(...$expectations); - $this->remover->removeNonExists(new Organization('', $users)); + $groupsToDelete = array_map(function (string $name) { + return [$name]; + }, array_diff(array_keys($teamMembers), $orgGroups)); + + $team + ->expects($this->exactly(count($groupsToDelete))) + ->method('remove') + ->withConsecutive(...array_values($groupsToDelete)); + + $this->remover->removeNonExists(new Organization($orgName, $users, $orgGroupDtos)); } public function getRemoveData(): array @@ -93,8 +121,12 @@ public function getRemoveData(): array 'org3' => [ 'test5', 'test6', + ], + 'org4' => [ + 'test7', ] - ] + ], + ['org1', 'org2', 'org3'] ] ]; } diff --git a/tests/Unit/SynchronizationAdapter/AdapterFactory/GithubAdapterFactoryTest.php b/tests/Unit/SynchronizationAdapter/AdapterFactory/GithubAdapterFactoryTest.php index 89af959..bf0f083 100644 --- a/tests/Unit/SynchronizationAdapter/AdapterFactory/GithubAdapterFactoryTest.php +++ b/tests/Unit/SynchronizationAdapter/AdapterFactory/GithubAdapterFactoryTest.php @@ -61,7 +61,7 @@ public function testSetTarget() { $token = '123qwetest'; - $this->client->expects($this->once())->method('authenticate')->with($token); + $this->client->expects($this->once())->method('authenticate')->with($token, Client::AUTH_HTTP_TOKEN); $this->factory->setTarget(new Target\Github('', '', $token)); } diff --git a/tests/Unit/SynchronizationAdapter/GroupPush/GithubGroupPushAdapterTest.php b/tests/Unit/SynchronizationAdapter/GroupPush/GithubGroupPushAdapterTest.php index c489a75..213bf6a 100644 --- a/tests/Unit/SynchronizationAdapter/GroupPush/GithubGroupPushAdapterTest.php +++ b/tests/Unit/SynchronizationAdapter/GroupPush/GithubGroupPushAdapterTest.php @@ -7,6 +7,7 @@ use Http\Client\Exception\TransferException; use LinkORB\OrgSync\DTO\Group; use LinkORB\OrgSync\DTO\User; +use LinkORB\OrgSync\Services\InputHandler; use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\GithubGroupPushAdapter; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -34,6 +35,7 @@ protected function setUp(): void public function testCreateGroup() { + $orgName = 'test113'; $name = 'testing'; $membersData = [ 'Tom', @@ -43,12 +45,11 @@ public function testCreateGroup() $params = [ 'name' => $name, - 'parent_team_id' => null, ]; $group = new Group($name, '', null, null, array_map(function (string $username) { return new User($username); - }, $membersData)); + }, $membersData), [InputHandler::GITHUB_ORGANIZATION => $orgName]); $team = $this->createMock(Teams::class); @@ -58,9 +59,9 @@ public function testCreateGroup() $team->expects($this->once()) ->method('update') - ->with($name, $params) + ->with($orgName, $params) ->willThrowException(new TransferException()); - $team->expects($this->once())->method('create')->with($name, $params); + $team->expects($this->once())->method('create')->with($orgName, $params); $team->expects($this->exactly(count($membersData))) ->method('addMember') @@ -73,7 +74,9 @@ public function testCreateGroup() public function testUpdate() { + $orgName = 'test12'; $group = new Group('name', ''); + $group->addProperty(InputHandler::GITHUB_ORGANIZATION, $orgName); $team = $this->createMock(Teams::class); From 6a6f9b81eb5d6e2a910a9c86109d6a6e337261dc Mon Sep 17 00:00:00 2001 From: amsprost Date: Fri, 30 Aug 2019 02:00:57 +0200 Subject: [PATCH 03/13] Finish/fixed github integration, tests --- .../SyncRemover/GithubSyncRemover.php | 9 +-- .../GroupPush/GithubGroupPushAdapter.php | 46 ++++++++++++-- .../SyncRemover/GithubSyncRemoverTest.php | 63 +++++++++++++------ .../GroupPush/GithubGroupPushAdapterTest.php | 42 ++++++++++--- 4 files changed, 123 insertions(+), 37 deletions(-) diff --git a/src/Services/SyncRemover/GithubSyncRemover.php b/src/Services/SyncRemover/GithubSyncRemover.php index a305913..ea65fdb 100644 --- a/src/Services/SyncRemover/GithubSyncRemover.php +++ b/src/Services/SyncRemover/GithubSyncRemover.php @@ -31,18 +31,19 @@ public function removeNonExists(Organization $organization): void foreach ($this->client->teams()->all($organization->getName()) as $group) { $teamName = $group['name']; + $teamId = $group['id']; if (!isset($groupsToSync[$teamName])) { - $this->client->team()->remove($teamName); + $this->client->team()->remove($teamId); continue; } - $members = $this->client->team()->members($teamName); + $members = $this->client->team()->members($teamId); foreach ($members as $member) { - if (!isset($orgUsersToSync[$member])) { - $this->client->team()->removeMember($teamName, $member); + if (!isset($orgUsersToSync[$member['login']])) { + $this->client->team()->removeMember($teamId, $member['login']); } } } diff --git a/src/SynchronizationAdapter/GroupPush/GithubGroupPushAdapter.php b/src/SynchronizationAdapter/GroupPush/GithubGroupPushAdapter.php index 74a3675..7d7937a 100644 --- a/src/SynchronizationAdapter/GroupPush/GithubGroupPushAdapter.php +++ b/src/SynchronizationAdapter/GroupPush/GithubGroupPushAdapter.php @@ -4,8 +4,11 @@ use Github\Client; use Http\Client\Exception; +use InvalidArgumentException; use LinkORB\OrgSync\DTO\Group; use LinkORB\OrgSync\Services\InputHandler; +use stdClass; +use Throwable; class GithubGroupPushAdapter implements GroupPushInterface { @@ -27,18 +30,53 @@ public function pushGroup(Group $group): GroupPushInterface $params = ['name' => $group->getName()]; if ($group->getParent()) { - $params['parent_team_id'] = $group->getParent()->getName(); + $this->setParentId( + $group->getProperties()[InputHandler::GITHUB_ORGANIZATION], + $group->getParent()->getName(), + $params + ); } - $this->client->team()->update($group->getProperties()[InputHandler::GITHUB_ORGANIZATION], $params); + $responseData = $this->getGroupByName( + $group->getProperties()[InputHandler::GITHUB_ORGANIZATION], + $group->getName() + ); + + $groupData = $this->client->team()->update($responseData->id, $params); } catch (Exception $e) { - $this->client->team()->create($group->getProperties()[InputHandler::GITHUB_ORGANIZATION], $params ?? []); + $groupData = $this->client->team()->create($group->getProperties()[InputHandler::GITHUB_ORGANIZATION], $params ?? []); } foreach ($group->getMembers() as $member) { - $this->client->team()->addMember($group->getName(), $member->getUsername()); + $this->client->team()->addMember($groupData['id'], $member->getUsername()); } return $this; } + + private function getGroupByName(string $organization, string $name): stdClass + { + $response = $this->client + ->getHttpClient() + ->get( + sprintf( + 'orgs/%s/teams/%s', + $organization, + $name + ) + ); + + return json_decode($response->getBody()->getContents()); + } + + private function setParentId(string $organization, string $groupName, array &$params): void + { + try { + $parentResponseData = $this->getGroupByName($organization, $groupName); + + $params['parent_team_id'] = $parentResponseData->id; + } catch (Throwable $e) { + throw new InvalidArgumentException('Parent group not exists yet', 0, $e); + } + } } diff --git a/tests/Unit/Services/SyncRemover/GithubSyncRemoverTest.php b/tests/Unit/Services/SyncRemover/GithubSyncRemoverTest.php index c08d4ad..91b9a58 100644 --- a/tests/Unit/Services/SyncRemover/GithubSyncRemoverTest.php +++ b/tests/Unit/Services/SyncRemover/GithubSyncRemoverTest.php @@ -37,6 +37,10 @@ protected function setUp(): void */ public function testRemoveNonExists(array $usersArray, array $teamMembers, array $orgGroups) { + foreach ($teamMembers as $teamName => &$memberData) { + $memberData['teamName'] = $teamName; + } + $orgName = 'TempOrg'; $orgGroupDtos = array_map(function (string $name) { @@ -50,14 +54,14 @@ function (string $username) { ); $expectations = []; - foreach ($teamMembers as $team => $members) { + foreach ($teamMembers as $team => $teamData) { if (!in_array($team, $orgGroups)) { continue; } - foreach ($members as $member) { + foreach ($teamData['members'] as $member) { if (!in_array($member, $usersArray)) { - $expectations[] = [$team, $member]; + $expectations[] = [$teamData['id'], $member]; } } } @@ -66,9 +70,9 @@ function (string $username) { $teams = $this->createConfiguredMock( Teams::class, [ - 'all' => array_map(function (string $name) { - return ['name' => $name]; - }, array_keys($teamMembers)) + 'all' => array_map(function (array $team) { + return ['name' => $team['teamName'], 'id' => $team['id']]; + }, array_values($teamMembers)) ] ); @@ -82,18 +86,25 @@ function (string $username) { $team ->expects($this->exactly(count($membersExpectations))) ->method('members') - ->withConsecutive(...array_map(function (string $teamName) { - return [$teamName]; - }, array_keys($membersExpectations))) - ->willReturnOnConsecutiveCalls(...array_values($membersExpectations)); + ->withConsecutive(...array_map(function (array $team) { + return [$team['id']]; + }, array_values($membersExpectations))) + ->willReturnOnConsecutiveCalls(...array_values(array_map(function (array $teamData) { + $members = []; + foreach ($teamData['members'] as $member) { + $members[] = ['login' => $member]; + } + + return $members; + }, $membersExpectations))); $team ->expects($this->exactly(count($expectations))) ->method('removeMember') ->withConsecutive(...$expectations); - $groupsToDelete = array_map(function (string $name) { - return [$name]; + $groupsToDelete = array_map(function (string $name) use ($teamMembers) { + return [$teamMembers[$name]['id']]; }, array_diff(array_keys($teamMembers), $orgGroups)); $team @@ -111,20 +122,32 @@ public function getRemoveData(): array ['test1', 'test2', 'test3'], [ 'org1' => [ - 'test1', - 'test5', - 'test2', + 'id' => 31, + 'members' => [ + 'test1', + 'test5', + 'test2', + ], ], 'org2' => [ - 'test3', + 'id' => 91, + 'members' => [ + 'test3', + ], ], 'org3' => [ - 'test5', - 'test6', + 'id' => 87, + 'members' => [ + 'test5', + 'test6', + ], ], 'org4' => [ - 'test7', - ] + 'id' => 11, + 'members' => [ + 'test7', + ], + ], ], ['org1', 'org2', 'org3'] ] diff --git a/tests/Unit/SynchronizationAdapter/GroupPush/GithubGroupPushAdapterTest.php b/tests/Unit/SynchronizationAdapter/GroupPush/GithubGroupPushAdapterTest.php index 213bf6a..bf6986b 100644 --- a/tests/Unit/SynchronizationAdapter/GroupPush/GithubGroupPushAdapterTest.php +++ b/tests/Unit/SynchronizationAdapter/GroupPush/GithubGroupPushAdapterTest.php @@ -4,6 +4,7 @@ use Github\Api\Organization\Teams; use Github\Client; +use Http\Client\Common\HttpMethodsClient; use Http\Client\Exception\TransferException; use LinkORB\OrgSync\DTO\Group; use LinkORB\OrgSync\DTO\User; @@ -11,6 +12,8 @@ use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\GithubGroupPushAdapter; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\StreamInterface; class GithubGroupPushAdapterTest extends TestCase { @@ -24,9 +27,13 @@ class GithubGroupPushAdapterTest extends TestCase */ private $client; + /** @var HttpMethodsClient|MockObject */ + private $httpClient; + protected function setUp(): void { - $this->client = $this->createMock(Client::class); + $this->httpClient = $this->createMock(HttpMethodsClient::class); + $this->client = $this->createConfiguredMock(Client::class, ['getHttpClient' => $this->httpClient]); $this->groupPush = new GithubGroupPushAdapter($this->client); @@ -37,6 +44,7 @@ public function testCreateGroup() { $orgName = 'test113'; $name = 'testing'; + $id = 99; $membersData = [ 'Tom', 'Jerry', @@ -57,16 +65,17 @@ public function testCreateGroup() ->with('team', []) ->willReturn($team); - $team->expects($this->once()) - ->method('update') - ->with($orgName, $params) - ->willThrowException(new TransferException()); - $team->expects($this->once())->method('create')->with($orgName, $params); + $this->httpClient->expects($this->once())->method('get')->willThrowException(new TransferException()); + $this->client->expects($this->once())->method('getHttpClient'); + + $team->expects($this->never()) + ->method('update'); + $team->expects($this->once())->method('create')->with($orgName, $params)->willReturn(['id' => $id]); $team->expects($this->exactly(count($membersData))) ->method('addMember') - ->withConsecutive(...array_map(function (string $username) use ($name) { - return [$name, $username]; + ->withConsecutive(...array_map(function (string $username) use ($id) { + return [$id, $username]; }, $membersData)); $this->assertSame($this->groupPush, $this->groupPush->pushGroup($group)); @@ -75,6 +84,7 @@ public function testCreateGroup() public function testUpdate() { $orgName = 'test12'; + $id = 12; $group = new Group('name', ''); $group->addProperty(InputHandler::GITHUB_ORGANIZATION, $orgName); @@ -84,7 +94,21 @@ public function testUpdate() ->with('team', []) ->willReturn($team); - $team->expects($this->once())->method('update'); + $responseData = json_encode(['id' => $id]); + $response = $this->createConfiguredMock( + ResponseInterface::class, + [ + 'getBody' => $this->createConfiguredMock( + StreamInterface::class, + ['getContents' => $responseData] + ) + ] + ); + $this->httpClient->expects($this->once())->method('get')->willReturn($response); + $team->expects($this->once()) + ->method('update') + ->with($id, $this->anything()) + ->willReturn(json_encode($responseData)); $team->expects($this->never())->method('create'); $this->assertSame($this->groupPush, $this->groupPush->pushGroup($group)); From edd2050561fad96898aabe11acc81c18b4392cdd Mon Sep 17 00:00:00 2001 From: amsprost Date: Mon, 16 Sep 2019 01:18:10 +0200 Subject: [PATCH 04/13] CARD-2527: Finished Users synchronization --- README.md | 2 +- composer.json | 3 +- src/DTO/Target.php | 1 + src/DTO/Target/Ldap.php | 55 ++++++++ src/Services/Ldap/Client.php | 127 ++++++++++++++++++ src/Services/SyncRemover/LdapSyncRemover.php | 39 ++++++ .../AdapterFactory/LdapAdapterFactory.php | 23 +++- .../UserPush/LdapUserPushAdapter.php | 65 +++++++++ 8 files changed, 309 insertions(+), 6 deletions(-) create mode 100644 src/DTO/Target/Ldap.php create mode 100644 src/Services/Ldap/Client.php create mode 100644 src/Services/SyncRemover/LdapSyncRemover.php create mode 100644 src/SynchronizationAdapter/UserPush/LdapUserPushAdapter.php diff --git a/README.md b/README.md index 99a996a..48ba8c6 100644 --- a/README.md +++ b/README.md @@ -94,4 +94,4 @@ Array ``` ### Integrations -* [OrgSync CLI](https://github.com/linkorb/org-sync-cli) \ No newline at end of file +* [OrgSync CLI](https://github.com/linkorb/org-sync-cli) diff --git a/composer.json b/composer.json index 414d31a..9b2ab61 100644 --- a/composer.json +++ b/composer.json @@ -12,7 +12,8 @@ "symfony/property-access": "^4.3", "symfony/property-info": "^4.3", "symfony/serializer": "^4.3", - "ext-json": "*" + "ext-json": "*", + "ext-ldap": "*" }, "require-dev": { "phpunit/phpunit": "^8" diff --git a/src/DTO/Target.php b/src/DTO/Target.php index 1f25c60..983b91a 100644 --- a/src/DTO/Target.php +++ b/src/DTO/Target.php @@ -8,6 +8,7 @@ * @DiscriminatorMap(typeProperty="type", mapping={ * "camunda"="LinkORB\OrgSync\DTO\Target\Camunda", * "github"="LinkORB\OrgSync\DTO\Target\Github", + * "ldap"="LinkORB\OrgSync\DTO\Target\Ldap", * }) */ abstract class Target diff --git a/src/DTO/Target/Ldap.php b/src/DTO/Target/Ldap.php new file mode 100644 index 0000000..4fefc52 --- /dev/null +++ b/src/DTO/Target/Ldap.php @@ -0,0 +1,55 @@ +bindRdn = $usersBindRdn; + $this->password = $password; + $this->domain = $domain; + } + + public function getBindRdn(): string + { + return $this->bindRdn; + } + + public function getPassword(): string + { + return $this->password; + } + + /** + * @return string[] + */ + public function getDomain(): array + { + return $this->domain; + } +} diff --git a/src/Services/Ldap/Client.php b/src/Services/Ldap/Client.php new file mode 100644 index 0000000..11b3b9c --- /dev/null +++ b/src/Services/Ldap/Client.php @@ -0,0 +1,127 @@ +target = $target; + } + + public function __destruct() + { + ldap_unbind($this->connection); + } + + public function init(): self + { + if ($this->connection) { + return $this; + } + + $this->connection = ldap_connect($this->target->getBaseUrl()); + + ldap_set_option($this->connection, LDAP_OPT_PROTOCOL_VERSION, 3); + + return $this; + } + + public function bind(): self + { + if (!ldap_bind($this->connection, $this->target->getBindRdn(), $this->target->getPassword())) { + throw new UnexpectedValueException('Binding failed'); + } + + return $this; + } + + /** + * @return resource + */ + public function search(string $query) + { + return ldap_search($this->connection, $this->getDcString(), $query); + } + + /** + * @param resource $result + */ + public function count($result): ?int + { + assert(is_resource($result)); + + $count = ldap_count_entries($this->connection, $result); + + if ($count === false) { + return null; + } + + return $count; + } + + /** + * @param resource $result + */ + public function all($result): array + { + assert(is_resource($result)); + + return ldap_get_entries($this->connection, $result); + } + + public function add(array $data): bool + { + return ldap_add($this->connection, $this->getDn($data), $data); + } + + public function modify(array $data): bool + { + return ldap_modify($this->connection, $this->getDn($data), $data); + } + + public function remove(string $dn): bool + { + return ldap_delete($this->connection, $dn); + } + + private function getDn(array $data): string + { + $excludeKeys = ['dc', 'objectClass']; + $dn = ''; + + foreach ($data as $key => $item) { + if (in_array($key, $excludeKeys, true)) { + continue; + } + + $dn .= sprintf('%s=%s+', $key, $item); + } + + return substr($dn, 0, -1) . ',' . $this->getDcString(); + } + + private function getDcString(): string + { + $dc = ''; + + foreach ($this->target->getDomain() as $domainComponent) { + $dc .= sprintf('dc=%s,', $domainComponent); + } + + return substr($dc, 0, -1); + } +} diff --git a/src/Services/SyncRemover/LdapSyncRemover.php b/src/Services/SyncRemover/LdapSyncRemover.php new file mode 100644 index 0000000..ea08b5f --- /dev/null +++ b/src/Services/SyncRemover/LdapSyncRemover.php @@ -0,0 +1,39 @@ +client = $client; + } + + public function removeNonExists(Organization $organization): void + { + $existingUsers = $this->client->all( + $this->client->search('(objectClass=inetOrgPerson)') + ); + + // Remove `count` key from array + array_shift($existingUsers); + + // Removing users + $syncUsers = []; + foreach ($organization->getUsers() as $user) { + $syncUsers[$user->getUsername()] = $user; + } + + foreach ($existingUsers as $existingUser) { + if (!isset($syncUsers[$existingUser['cn'][0]])) { + $this->client->remove($existingUser['dn']); + } + } + } +} diff --git a/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php b/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php index 9e5fc7f..691d277 100644 --- a/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php +++ b/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php @@ -3,14 +3,20 @@ namespace LinkORB\OrgSync\SynchronizationAdapter\AdapterFactory; use LinkORB\OrgSync\DTO\Target; +use LinkORB\OrgSync\Services\Ldap\Client; +use LinkORB\OrgSync\Services\SyncRemover\LdapSyncRemover; use LinkORB\OrgSync\Services\SyncRemover\SyncRemoverInterface; use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\GroupPushInterface; use LinkORB\OrgSync\SynchronizationAdapter\OrganizationPull\OrganizationPullInterface; use LinkORB\OrgSync\SynchronizationAdapter\SetPassword\SetPasswordInterface; +use LinkORB\OrgSync\SynchronizationAdapter\UserPush\LdapUserPushAdapter; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\UserPushInterface; class LdapAdapterFactory implements AdapterFactoryInterface { + /** @var Client */ + private $client; + public function createOrganizationPullAdapter(): OrganizationPullInterface { // TODO: Implement createOrganizationPullAdapter() method. @@ -23,7 +29,7 @@ public function createGroupPushAdapter(): GroupPushInterface public function createUserPushAdapter(): UserPushInterface { - // TODO: Implement createUserPushAdapter() method. + return new LdapUserPushAdapter($this->client); } public function createSetPasswordAdapter(): SetPasswordInterface @@ -33,16 +39,25 @@ public function createSetPasswordAdapter(): SetPasswordInterface public function setTarget(Target $target): AdapterFactoryInterface { - // TODO: Implement setTarget() method. + $this->client = new Client($target); + $this->client + ->init() + ->bind(); + + return $this; } public function createSyncRemover(): SyncRemoverInterface { - // TODO: Implement createSyncRemover() method. + return new LdapSyncRemover($this->client); } public function supports(string $action): bool { - return false; + return in_array($action, [ + Target::GROUP_PUSH, + Target::SET_PASSWORD, + Target::USER_PUSH, + ], true); } } diff --git a/src/SynchronizationAdapter/UserPush/LdapUserPushAdapter.php b/src/SynchronizationAdapter/UserPush/LdapUserPushAdapter.php new file mode 100644 index 0000000..543754e --- /dev/null +++ b/src/SynchronizationAdapter/UserPush/LdapUserPushAdapter.php @@ -0,0 +1,65 @@ +client = $client; + } + + public function pushUser(User $user): UserPushInterface + { + $userInfo = $this->getUserInfo($user); + + // TODO: Use ldap_add with LDAP_CONTROL_SYNC for PHP 7.3 + $userSearchCount = $this->client->count( + $this->client->search(sprintf('(cn=%s)', $user->getUsername())) + ); + + if ($userSearchCount === null) { + throw new UnexpectedValueException('Error during search!'); + } + + if ($userSearchCount === 0) { + $res = $this->client->add($userInfo); + } else { + $res = $this->client->modify($userInfo); + } + + if (!$res) { + throw new UnexpectedValueException(sprintf('User \'%s\' wasn\'t added', $user->getUsername())); + } + + return $this; + } + + private function getUserInfo(User $user): array + { + $userInfo = [ + 'cn' => $user->getUsername(), + 'sn' => $user->getProperties()['lastName'] ?? array_pop(explode(' ', (string) $user->getDisplayName())), + 'mail' => $user->getEmail(), + ]; + + if ($user->getDisplayName()) { + $userInfo['displayName'] = $user->getDisplayName(); + } + + if ($user->getAvatar()) { + $userInfo['Photo'] = $user->getAvatar(); + } + + $userInfo['objectClass'] = ['inetOrgPerson', 'organizationalPerson', 'person', 'top']; + + return $userInfo; + } +} From b36ae62082bc646db424fd877b78909cf89de6d1 Mon Sep 17 00:00:00 2001 From: amsprost Date: Wed, 18 Sep 2019 01:33:26 +0200 Subject: [PATCH 05/13] Improved test coverage, fixed inconsistencies --- src/DTO/Target/Ldap.php | 4 +- .../AdapterFactory/LdapAdapterFactory.php | 7 +- tests/Unit/DTO/Target/LdapTest.php | 36 ++++++ .../SyncRemover/LdapSyncRemoverTest.php | 103 ++++++++++++++++ .../AdapterFactory/LdapAdapterFactoryTest.php | 68 +++++++++++ .../UserPush/LdapUserPushAdapterTest.php | 115 ++++++++++++++++++ 6 files changed, 330 insertions(+), 3 deletions(-) create mode 100644 tests/Unit/DTO/Target/LdapTest.php create mode 100644 tests/Unit/Services/SyncRemover/LdapSyncRemoverTest.php create mode 100644 tests/Unit/SynchronizationAdapter/AdapterFactory/LdapAdapterFactoryTest.php create mode 100644 tests/Unit/SynchronizationAdapter/UserPush/LdapUserPushAdapterTest.php diff --git a/src/DTO/Target/Ldap.php b/src/DTO/Target/Ldap.php index 4fefc52..df337ca 100644 --- a/src/DTO/Target/Ldap.php +++ b/src/DTO/Target/Ldap.php @@ -24,13 +24,13 @@ final class Ldap extends Target public function __construct( string $baseUrl, string $name, - string $usersBindRdn, + string $bindRdn, string $password, array $domain ) { parent::__construct($baseUrl, $name); - $this->bindRdn = $usersBindRdn; + $this->bindRdn = $bindRdn; $this->password = $password; $this->domain = $domain; } diff --git a/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php b/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php index 691d277..0ce1557 100644 --- a/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php +++ b/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php @@ -39,7 +39,7 @@ public function createSetPasswordAdapter(): SetPasswordInterface public function setTarget(Target $target): AdapterFactoryInterface { - $this->client = new Client($target); + $this->client = $this->getClient($target); $this->client ->init() ->bind(); @@ -60,4 +60,9 @@ public function supports(string $action): bool Target::USER_PUSH, ], true); } + + protected function getClient(Target\Ldap $target): Client + { + return new Client($target); + } } diff --git a/tests/Unit/DTO/Target/LdapTest.php b/tests/Unit/DTO/Target/LdapTest.php new file mode 100644 index 0000000..d5d2142 --- /dev/null +++ b/tests/Unit/DTO/Target/LdapTest.php @@ -0,0 +1,36 @@ + '', + 'name' => '', + 'bindRdn' => '', + 'password' => '', + 'domain' => [], + ]; + } + + public function getDtoClassName(): string + { + return Ldap::class; + } +} diff --git a/tests/Unit/Services/SyncRemover/LdapSyncRemoverTest.php b/tests/Unit/Services/SyncRemover/LdapSyncRemoverTest.php new file mode 100644 index 0000000..51001b0 --- /dev/null +++ b/tests/Unit/Services/SyncRemover/LdapSyncRemoverTest.php @@ -0,0 +1,103 @@ +client = $this->createMock(Client::class); + $this->syncRemover = new LdapSyncRemover($this->client); + } + + /** + * @dataProvider getRemoveData + */ + public function testRemoveNonExists(array $orgUsers, array $existingUsers,array $expectedToRemoveUsers) + { + $organization = new Organization('', $orgUsers); + + $searchResult = new \stdClass(); + $this->client + ->expects($this->once()) + ->method('search') + ->with('(objectClass=inetOrgPerson)') + ->willReturn($searchResult); + + $this->client + ->expects($this->once()) + ->method('all') + ->with($searchResult) + ->willReturn($existingUsers); + + $this->client + ->expects($this->exactly(count($expectedToRemoveUsers))) + ->method('remove') + ->withConsecutive(...$expectedToRemoveUsers); + + $this->assertNull($this->syncRemover->removeNonExists($organization)); + } + + public function getRemoveData(): array + { + return [ + [ + [ + new User('test11'), + new User('test33'), + ], + [ + 'count' => 3, + ['cn' => ['test011', 'test11'], 'dn' => 'test11del422'], + ['cn' => ['test11'], 'dn' => 'test11del22'], + ['cn' => ['test123'], 'dn' => 'test11del99'], + ], + [['test11del422'], ['test11del99']], + ], + [ + [], + [ + 'count' => 2, + ['cn' => ['test11'], 'dn' => 'test11del22'], + ['cn' => ['test123'], 'dn' => 'test11del99'], + ], + [['test11del22'], ['test11del99']], + ], + [ + [ + new User('test11'), + new User('test33'), + ], + [ + 'count' => 0, + ], + [], + ], + [ + [ + new User('test11'), + new User('test33'), + ], + [ + 'count' => 2, + ['cn' => ['test11'], 'dn' => 'test11del22'], + ['cn' => ['test33'], 'dn' => 'test11del99'], + ], + [], + ], + ]; + } +} diff --git a/tests/Unit/SynchronizationAdapter/AdapterFactory/LdapAdapterFactoryTest.php b/tests/Unit/SynchronizationAdapter/AdapterFactory/LdapAdapterFactoryTest.php new file mode 100644 index 0000000..511abbd --- /dev/null +++ b/tests/Unit/SynchronizationAdapter/AdapterFactory/LdapAdapterFactoryTest.php @@ -0,0 +1,68 @@ +client = $this->createMock(Client::class); + $this->client->method('bind')->willReturnSelf(); + $this->client->method('init')->willReturnSelf(); + $this->factory = $this->createPartialMock(LdapAdapterFactory::class, ['getClient']); + $this->factory->method('getClient')->willReturn($this->client); + $this->factory->setTarget(new Ldap('', '', '', '', [])); + } + + public function testCreateUserPushAdapter() + { + $this->assertInstanceOf(LdapUserPushAdapter::class, $this->factory->createUserPushAdapter()); + } + + /** + * @dataProvider getSupportsData + */ + public function testSupports(string $action, bool $isSupported) + { + $this->assertEquals($isSupported, $this->factory->supports($action)); + } + + public function testCreateSyncRemover() + { + $this->assertInstanceOf(LdapSyncRemover::class, $this->factory->createSyncRemover()); + } + + public function testSetTarget() + { + $this->client->expects($this->once())->method('bind'); + $this->client->expects($this->once())->method('init'); + + $this->assertSame($this->factory, $this->factory->setTarget(new Ldap('', '', '', '', []))); + } + + public function getSupportsData(): array + { + return [ + [Target::GROUP_PUSH, true], + [Target::SET_PASSWORD, true], + [Target::USER_PUSH, true], + [Target::PULL_ORGANIZATION, false], + ['', false], + ]; + } +} diff --git a/tests/Unit/SynchronizationAdapter/UserPush/LdapUserPushAdapterTest.php b/tests/Unit/SynchronizationAdapter/UserPush/LdapUserPushAdapterTest.php new file mode 100644 index 0000000..91836d9 --- /dev/null +++ b/tests/Unit/SynchronizationAdapter/UserPush/LdapUserPushAdapterTest.php @@ -0,0 +1,115 @@ +client = $this->createMock(Client::class); + $this->adapter = new LdapUserPushAdapter($this->client); + } + + /** + * @dataProvider getTestPushUserData + */ + public function testPushUser(User $userObj, array $userArr, array $searchResults) + { + $this->client + ->expects($this->once()) + ->method('search') + ->with($this->callback(function (string $query) use ($userObj) { + return strstr($query, $userObj->getUsername()) !== false; + })) + ->willReturn($searchResults); + + $this->client + ->expects($this->once()) + ->method('count') + ->with($searchResults) + ->willReturnCallback(function (array $result) { + return count($result); + }); + + $this->client + ->expects($this->once()) + ->method(count($searchResults) > 0 ? 'modify' : 'add') + ->with($userArr) + ->willReturn(true); + + $this->assertSame($this->adapter, $this->adapter->pushUser($userObj)); + } + + public function testPushUserSearchException() + { + $this->client->method('count')->willReturn(null); + + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage('Error during search!'); + + $this->adapter->pushUser(new User('', null, '')); + } + + public function testPushUserAddException() + { + $this->client->method('count')->willReturn(0); + $this->client->method('add')->willReturn(false); + + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessage('User \'undefined\' wasn\'t added'); + + $this->adapter->pushUser(new User('undefined', null, '')); + } + + public function getTestPushUserData(): array + { + return [ + [ + new User('temp123', null, 'temp@c.com', 'star', null, ['lastName' => 'Testenko']), + [ + 'cn' => 'temp123', + 'sn' => 'Testenko', + 'mail' => 'temp@c.com', + 'displayName' => 'star', + 'objectClass' => ['inetOrgPerson', 'organizationalPerson', 'person', 'top'], + ], + [], + ], + [ + new User('temp000', null, 'temp99@a.com', 'super mega star', 'ava.gif'), + [ + 'cn' => 'temp000', + 'sn' => 'star', + 'mail' => 'temp99@a.com', + 'displayName' => 'super mega star', + 'Photo' => 'ava.gif', + 'objectClass' => ['inetOrgPerson', 'organizationalPerson', 'person', 'top'], + ], + [1], + ], + [ + new User('987', null, 'a@a.nl'), + [ + 'cn' => '987', + 'sn' => '', + 'mail' => 'a@a.nl', + 'objectClass' => ['inetOrgPerson', 'organizationalPerson', 'person', 'top'], + ], + [1,2,3] + ] + ]; + } +} From def2054eace1e172a12e7a0314e7d92c536febd2 Mon Sep 17 00:00:00 2001 From: amsprost Date: Sun, 22 Sep 2019 02:31:26 +0200 Subject: [PATCH 06/13] CARD-2527: Finish group LDAP synchronization --- src/Services/Ldap/Client.php | 30 ++++--- src/Services/Ldap/LdapAssertionAwareTrait.php | 15 ++++ src/Services/Ldap/LdapParentHelper.php | 24 +++++ src/Services/Ldap/UserDataMapper.php | 29 +++++++ src/Services/SyncRemover/LdapSyncRemover.php | 81 ++++++++++++++++- .../AdapterFactory/LdapAdapterFactory.php | 21 ++++- .../GroupPush/LdapGroupPushAdapter.php | 87 +++++++++++++++++++ .../UserPush/LdapUserPushAdapter.php | 62 +++++++------ 8 files changed, 298 insertions(+), 51 deletions(-) create mode 100644 src/Services/Ldap/LdapAssertionAwareTrait.php create mode 100644 src/Services/Ldap/LdapParentHelper.php create mode 100644 src/Services/Ldap/UserDataMapper.php create mode 100644 src/SynchronizationAdapter/GroupPush/LdapGroupPushAdapter.php diff --git a/src/Services/Ldap/Client.php b/src/Services/Ldap/Client.php index 11b3b9c..acf588f 100644 --- a/src/Services/Ldap/Client.php +++ b/src/Services/Ldap/Client.php @@ -52,9 +52,9 @@ public function bind(): self /** * @return resource */ - public function search(string $query) + public function search(string $query, array $additionalDn = []) { - return ldap_search($this->connection, $this->getDcString(), $query); + return ldap_search($this->connection, $this->getDcString($additionalDn), $query); } /** @@ -83,14 +83,14 @@ public function all($result): array return ldap_get_entries($this->connection, $result); } - public function add(array $data): bool + public function add(array $data, array $additionalDn = []): bool { - return ldap_add($this->connection, $this->getDn($data), $data); + return ldap_add($this->connection, $this->getDn($data, $additionalDn), $data); } - public function modify(array $data): bool + public function modify(array $data, array $additionalDn = []): bool { - return ldap_modify($this->connection, $this->getDn($data), $data); + return ldap_modify($this->connection, $this->getDn($data, $additionalDn), $data); } public function remove(string $dn): bool @@ -98,9 +98,9 @@ public function remove(string $dn): bool return ldap_delete($this->connection, $dn); } - private function getDn(array $data): string + public function getDn(array $data, array $additionalDn = []): string { - $excludeKeys = ['dc', 'objectClass']; + $excludeKeys = ['dc', 'objectClass', 'uniqueMember']; $dn = ''; foreach ($data as $key => $item) { @@ -111,15 +111,21 @@ private function getDn(array $data): string $dn .= sprintf('%s=%s+', $key, $item); } - return substr($dn, 0, -1) . ',' . $this->getDcString(); + return substr($dn, 0, -1) . ',' . $this->getDcString($additionalDn); } - private function getDcString(): string + private function getDcString(array $additionalDn = []): string { $dc = ''; - foreach ($this->target->getDomain() as $domainComponent) { - $dc .= sprintf('dc=%s,', $domainComponent); + foreach (array_merge($additionalDn, $this->target->getDomain()) as $key => $domainComponent) { + $dnKey = is_string($key) ? $key : 'dc'; + + $domainComponent = is_array($domainComponent) ? $domainComponent : [$domainComponent]; + + foreach ($domainComponent as $domainComponentElement) { + $dc .= sprintf('%s=%s,', $dnKey, $domainComponentElement); + } } return substr($dc, 0, -1); diff --git a/src/Services/Ldap/LdapAssertionAwareTrait.php b/src/Services/Ldap/LdapAssertionAwareTrait.php new file mode 100644 index 0000000..1093e64 --- /dev/null +++ b/src/Services/Ldap/LdapAssertionAwareTrait.php @@ -0,0 +1,15 @@ +getParent() === null) { + return $parents; + } else { + // to prevent circular reference + $this->assertResult(!in_array($group->getParent()->getName(), $parents), 'Circular reference detected'); + + $parents[] = $group->getParent()->getName(); + + return $this->getParentGroups($parents, $group->getParent()); + } + } +} diff --git a/src/Services/Ldap/UserDataMapper.php b/src/Services/Ldap/UserDataMapper.php new file mode 100644 index 0000000..0207917 --- /dev/null +++ b/src/Services/Ldap/UserDataMapper.php @@ -0,0 +1,29 @@ + $user->getUsername(), + 'sn' => $user->getProperties()['lastName'] ?? array_pop(explode(' ', (string) $user->getDisplayName())), + 'mail' => $user->getEmail(), + ]; + + if ($user->getDisplayName()) { + $userInfo['displayName'] = $user->getDisplayName(); + } + + if ($user->getAvatar()) { + $userInfo['Photo'] = $user->getAvatar(); + } + + $userInfo['objectClass'] = ['inetOrgPerson', 'organizationalPerson', 'person', 'top']; + + return $userInfo; + } +} diff --git a/src/Services/SyncRemover/LdapSyncRemover.php b/src/Services/SyncRemover/LdapSyncRemover.php index ea08b5f..3001ce2 100644 --- a/src/Services/SyncRemover/LdapSyncRemover.php +++ b/src/Services/SyncRemover/LdapSyncRemover.php @@ -2,23 +2,55 @@ namespace LinkORB\OrgSync\Services\SyncRemover; +use LinkORB\OrgSync\DTO\Group; use LinkORB\OrgSync\DTO\Organization; +use LinkORB\OrgSync\DTO\User; use LinkORB\OrgSync\Services\Ldap\Client; +use LinkORB\OrgSync\Services\Ldap\LdapAssertionAwareTrait; +use LinkORB\OrgSync\Services\Ldap\LdapParentHelper; +use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\LdapGroupPushAdapter; +use LinkORB\OrgSync\SynchronizationAdapter\UserPush\LdapUserPushAdapter; class LdapSyncRemover implements SyncRemoverInterface { + use LdapAssertionAwareTrait; + /** @var Client */ private $client; - public function __construct(Client $client) + /** @var LdapParentHelper */ + private $parentHelper; + + public function __construct(Client $client, LdapParentHelper $parentHelper) { $this->client = $client; + $this->parentHelper = $parentHelper; } public function removeNonExists(Organization $organization): void + { + $usersOrgUnit = $this->client->count( + $this->client->search(sprintf('(ou=%s)', LdapUserPushAdapter::USERS_ORG_UNIT)) + ); + $groupsOrgUnit = $this->client->count( + $this->client->search(sprintf('(ou=%s)', LdapGroupPushAdapter::GROUPS_ORG_UNIT)) + ); + + $this->assertResult($usersOrgUnit !== null && $groupsOrgUnit !== null, 'Error during search!'); + if ($usersOrgUnit !== 0) { + $this->removeExcessUsers($organization->getUsers()); + } + + if ($groupsOrgUnit !== 0) { + $this->removeExcessGroups($organization->getGroups()); + } + } + + /** @param User[] $users */ + private function removeExcessUsers(array $users): void { $existingUsers = $this->client->all( - $this->client->search('(objectClass=inetOrgPerson)') + $this->client->search('(objectClass=inetOrgPerson)', ['ou' => LdapUserPushAdapter::USERS_ORG_UNIT]) ); // Remove `count` key from array @@ -26,7 +58,7 @@ public function removeNonExists(Organization $organization): void // Removing users $syncUsers = []; - foreach ($organization->getUsers() as $user) { + foreach ($users as $user) { $syncUsers[$user->getUsername()] = $user; } @@ -36,4 +68,47 @@ public function removeNonExists(Organization $organization): void } } } + + /** + * @param Group[] $groups + */ + private function removeExcessGroups(array $groups): void + { + $existingGroups = $existingUsers = $this->client->all( + $this->client->search('(objectClass=groupOfUniqueNames)', ['ou' => LdapGroupPushAdapter::GROUPS_ORG_UNIT]) + ); + + // Remove `count` key from array + array_shift($existingGroups); + + // Removing users + /** @var Group[] $syncGroups */ + $syncGroups = []; + foreach ($groups as $group) { + $syncGroups[$group->getName()] = $group; + } + + foreach ($existingGroups as $existingGroup) { + if (!isset($syncGroups[$existingGroup['cn'][0]])) { + $this->client->remove($existingGroup['dn']); + + continue; + } + + $syncGroup = $syncGroups[$existingGroup['cn'][0]]; + $groupDn = $this->client->getDn( + ['cn' => $syncGroup->getName()], + [ + 'cn' => $this->parentHelper->getParentGroups([], $syncGroup), + 'ou' => LdapGroupPushAdapter::GROUPS_ORG_UNIT, + ] + ); + + if ($groupDn !== $existingGroup['dn']) { + $this->client->remove($existingGroup['dn']); + + continue; + } + } + } } diff --git a/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php b/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php index 0ce1557..4bced00 100644 --- a/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php +++ b/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php @@ -4,9 +4,12 @@ use LinkORB\OrgSync\DTO\Target; use LinkORB\OrgSync\Services\Ldap\Client; +use LinkORB\OrgSync\Services\Ldap\LdapParentHelper; +use LinkORB\OrgSync\Services\Ldap\UserDataMapper; use LinkORB\OrgSync\Services\SyncRemover\LdapSyncRemover; use LinkORB\OrgSync\Services\SyncRemover\SyncRemoverInterface; use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\GroupPushInterface; +use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\LdapGroupPushAdapter; use LinkORB\OrgSync\SynchronizationAdapter\OrganizationPull\OrganizationPullInterface; use LinkORB\OrgSync\SynchronizationAdapter\SetPassword\SetPasswordInterface; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\LdapUserPushAdapter; @@ -17,6 +20,18 @@ class LdapAdapterFactory implements AdapterFactoryInterface /** @var Client */ private $client; + /** @var UserDataMapper */ + private $userMapper; + + /** @var LdapParentHelper */ + private $parentHelper; + + public function __construct() + { + $this->userMapper = new UserDataMapper(); + $this->parentHelper = new LdapParentHelper(); + } + public function createOrganizationPullAdapter(): OrganizationPullInterface { // TODO: Implement createOrganizationPullAdapter() method. @@ -24,12 +39,12 @@ public function createOrganizationPullAdapter(): OrganizationPullInterface public function createGroupPushAdapter(): GroupPushInterface { - // TODO: Implement createGroupPushAdapter() method. + return new LdapGroupPushAdapter($this->client, $this->userMapper, $this->parentHelper); } public function createUserPushAdapter(): UserPushInterface { - return new LdapUserPushAdapter($this->client); + return new LdapUserPushAdapter($this->client,$this->userMapper); } public function createSetPasswordAdapter(): SetPasswordInterface @@ -49,7 +64,7 @@ public function setTarget(Target $target): AdapterFactoryInterface public function createSyncRemover(): SyncRemoverInterface { - return new LdapSyncRemover($this->client); + return new LdapSyncRemover($this->client, $this->parentHelper); } public function supports(string $action): bool diff --git a/src/SynchronizationAdapter/GroupPush/LdapGroupPushAdapter.php b/src/SynchronizationAdapter/GroupPush/LdapGroupPushAdapter.php new file mode 100644 index 0000000..943ddf3 --- /dev/null +++ b/src/SynchronizationAdapter/GroupPush/LdapGroupPushAdapter.php @@ -0,0 +1,87 @@ +client = $client; + $this->mapper = $mapper; + $this->parentHelper = $parentHelper; + } + + public function pushGroup(Group $group): GroupPushInterface + { + $groupInfo = $this->getGroupInfo($group); + + $groupsOrgUnit = $this->client->count( + $this->client->search(sprintf('(ou=%s)', static::GROUPS_ORG_UNIT)) + ); + + $this->assertResult($groupsOrgUnit !== null, 'Error during search!'); + if ($groupsOrgUnit === 0) { + $this->client->add([ + 'ou' => static::GROUPS_ORG_UNIT, + 'objectClass' => ['top', 'organizationalUnit'], + ]); + } + + $groupRn = ['cn' => $this->parentHelper->getParentGroups([], $group), 'ou' => array_merge([static::GROUPS_ORG_UNIT])]; + + // TODO: Use ldap_add with LDAP_CONTROL_SYNC for PHP 7.3 + $groupSearchCount = $this->client->count( + $this->client->search(sprintf('(cn=%s)', $group->getName()), $groupRn) + ); + + $this->assertResult($groupSearchCount !== null, 'Error during search!'); + + if ($groupSearchCount === 0) { + $res = $this->client->add($groupInfo, $groupRn); + } else { + $res = $this->client->modify($groupInfo, $groupRn); + } + + $this->assertResult((bool) $res, sprintf('Group \'%s\' wasn\'t added', $group->getName())); + + return $this; + } + + private function getGroupInfo(Group $group): array + { + $groupInfo = [ + 'cn' => $group->getName(), + 'objectClass' => ['top', 'groupOfUniqueNames'], + 'uniqueMember' => [], + ]; + + foreach ($group->getMembers() as $member) { + $groupInfo['uniqueMember'][] = $this->client->getDn( + $this->mapper->map($member), + ['ou' => LdapUserPushAdapter::USERS_ORG_UNIT] + ); + } + + return $groupInfo; + } +} diff --git a/src/SynchronizationAdapter/UserPush/LdapUserPushAdapter.php b/src/SynchronizationAdapter/UserPush/LdapUserPushAdapter.php index 543754e..53137f9 100644 --- a/src/SynchronizationAdapter/UserPush/LdapUserPushAdapter.php +++ b/src/SynchronizationAdapter/UserPush/LdapUserPushAdapter.php @@ -4,62 +4,58 @@ use LinkORB\OrgSync\DTO\User; use LinkORB\OrgSync\Services\Ldap\Client; -use UnexpectedValueException; +use LinkORB\OrgSync\Services\Ldap\LdapAssertionAwareTrait; +use LinkORB\OrgSync\Services\Ldap\UserDataMapper; class LdapUserPushAdapter implements UserPushInterface { + public const USERS_ORG_UNIT = 'users'; + + use LdapAssertionAwareTrait; + /** @var Client */ private $client; - public function __construct(Client $client) + /** @var UserDataMapper */ + private $mapper; + + public function __construct(Client $client, UserDataMapper $mapper) { $this->client = $client; + $this->mapper = $mapper; } public function pushUser(User $user): UserPushInterface { - $userInfo = $this->getUserInfo($user); + $userInfo = $this->mapper->map($user); + + $usersOrgUnit = $this->client->count( + $this->client->search(sprintf('(ou=%s)', static::USERS_ORG_UNIT)) + ); + + $this->assertResult($usersOrgUnit !== null, 'Error during search!'); + if ($usersOrgUnit === 0) { + $this->client->add([ + 'ou' => static::USERS_ORG_UNIT, + 'objectClass' => ['top', 'organizationalUnit'], + ]); + } // TODO: Use ldap_add with LDAP_CONTROL_SYNC for PHP 7.3 $userSearchCount = $this->client->count( - $this->client->search(sprintf('(cn=%s)', $user->getUsername())) + $this->client->search(sprintf('(cn=%s)', $user->getUsername()), ['ou' => static::USERS_ORG_UNIT]) ); - if ($userSearchCount === null) { - throw new UnexpectedValueException('Error during search!'); - } + $this->assertResult($userSearchCount !== null, 'Error during search!'); if ($userSearchCount === 0) { - $res = $this->client->add($userInfo); + $res = $this->client->add($userInfo, ['ou' => static::USERS_ORG_UNIT]); } else { - $res = $this->client->modify($userInfo); + $res = $this->client->modify($userInfo, ['ou' => static::USERS_ORG_UNIT]); } - if (!$res) { - throw new UnexpectedValueException(sprintf('User \'%s\' wasn\'t added', $user->getUsername())); - } + $this->assertResult((bool) $res, sprintf('User \'%s\' wasn\'t added', $user->getUsername())); return $this; } - - private function getUserInfo(User $user): array - { - $userInfo = [ - 'cn' => $user->getUsername(), - 'sn' => $user->getProperties()['lastName'] ?? array_pop(explode(' ', (string) $user->getDisplayName())), - 'mail' => $user->getEmail(), - ]; - - if ($user->getDisplayName()) { - $userInfo['displayName'] = $user->getDisplayName(); - } - - if ($user->getAvatar()) { - $userInfo['Photo'] = $user->getAvatar(); - } - - $userInfo['objectClass'] = ['inetOrgPerson', 'organizationalPerson', 'person', 'top']; - - return $userInfo; - } } From 4bd287bce60b1c3b42068de665fb9c4de181ba7a Mon Sep 17 00:00:00 2001 From: amsprost Date: Mon, 30 Sep 2019 02:55:10 +0200 Subject: [PATCH 07/13] Finished SetPassword, tests, Changed setPassword signature, generate uid based rdn --- src/DTO/User.php | 22 ++- src/Services/Ldap/Client.php | 47 +++--- src/Services/Ldap/UserDataMapper.php | 6 +- src/Services/SyncRemover/LdapSyncRemover.php | 7 +- .../AdapterFactory/LdapAdapterFactory.php | 8 +- .../GroupPush/LdapGroupPushAdapter.php | 37 ++--- .../SetPassword/CamundaSetPasswordAdapter.php | 7 +- .../SetPassword/LdapSetPasswordAdapter.php | 57 +++++++ .../SetPassword/SetPasswordInterface.php | 2 +- .../UserPush/LdapUserPushAdapter.php | 24 +-- .../SynchronizationMediator.php | 4 +- .../SynchronizationMediatorInterface.php | 2 +- .../SyncRemover/LdapSyncRemoverTest.php | 149 +++++++++++++++++- .../CamundaAdapterFactoryTest.php | 4 +- .../AdapterFactory/LdapAdapterFactoryTest.php | 1 + .../CamundaSetPasswordAdapterTest.php | 4 +- .../UserPush/LdapUserPushAdapterTest.php | 83 ++++++++-- .../SynchronizationMediatorTest.php | 5 +- 18 files changed, 368 insertions(+), 101 deletions(-) create mode 100644 src/SynchronizationAdapter/SetPassword/LdapSetPasswordAdapter.php diff --git a/src/DTO/User.php b/src/DTO/User.php index a139ecb..fd9fedf 100644 --- a/src/DTO/User.php +++ b/src/DTO/User.php @@ -4,6 +4,8 @@ class User { + public const PREVIOUS_PASSWORD = 'previousPassword'; + /** * @var string */ @@ -76,10 +78,26 @@ public function getPassword(): ?string return $this->password; } + public function setPassword(?string $password): self + { + $this->password = $password; + + return $this; + } + + public function setPreviousPassword(?string $password): self + { + if ($password !== null) { + $this->properties[static::PREVIOUS_PASSWORD] = $password; + } + + return $this; + } + /** - * @return string + * @return string|null */ - public function getEmail(): string + public function getEmail(): ?string { return $this->email; } diff --git a/src/Services/Ldap/Client.php b/src/Services/Ldap/Client.php index acf588f..d718cdb 100644 --- a/src/Services/Ldap/Client.php +++ b/src/Services/Ldap/Client.php @@ -54,7 +54,7 @@ public function bind(): self */ public function search(string $query, array $additionalDn = []) { - return ldap_search($this->connection, $this->getDcString($additionalDn), $query); + return ldap_search($this->connection, $this->generateDn($additionalDn), $query); } /** @@ -83,42 +83,47 @@ public function all($result): array return ldap_get_entries($this->connection, $result); } - public function add(array $data, array $additionalDn = []): bool + /** + * @param resource $result + * @return resource + */ + public function first($result) { - return ldap_add($this->connection, $this->getDn($data, $additionalDn), $data); + assert(is_resource($result)); + + return ldap_first_entry($this->connection, $result); } - public function modify(array $data, array $additionalDn = []): bool + /** + * @param resource $entry + */ + public function getDn($entry): ?string { - return ldap_modify($this->connection, $this->getDn($data, $additionalDn), $data); + assert(is_resource($entry)); + + return ldap_get_dn($this->connection, $entry) ?: null; } - public function remove(string $dn): bool + public function add(array $data, array $rdn = []): bool { - return ldap_delete($this->connection, $dn); + return ldap_add($this->connection, $this->generateDn($rdn), $data); } - public function getDn(array $data, array $additionalDn = []): string + public function modify(array $data, string $dn): bool { - $excludeKeys = ['dc', 'objectClass', 'uniqueMember']; - $dn = ''; - - foreach ($data as $key => $item) { - if (in_array($key, $excludeKeys, true)) { - continue; - } - - $dn .= sprintf('%s=%s+', $key, $item); - } + return ldap_modify($this->connection, $dn, $data); + } - return substr($dn, 0, -1) . ',' . $this->getDcString($additionalDn); + public function remove(string $dn): bool + { + return ldap_delete($this->connection, $dn); } - private function getDcString(array $additionalDn = []): string + public function generateDn(array $rdn = []): string { $dc = ''; - foreach (array_merge($additionalDn, $this->target->getDomain()) as $key => $domainComponent) { + foreach (array_merge($rdn, $this->target->getDomain()) as $key => $domainComponent) { $dnKey = is_string($key) ? $key : 'dc'; $domainComponent = is_array($domainComponent) ? $domainComponent : [$domainComponent]; diff --git a/src/Services/Ldap/UserDataMapper.php b/src/Services/Ldap/UserDataMapper.php index 0207917..610df04 100644 --- a/src/Services/Ldap/UserDataMapper.php +++ b/src/Services/Ldap/UserDataMapper.php @@ -10,10 +10,14 @@ public function map(User $user): array { $userInfo = [ 'cn' => $user->getUsername(), + 'uid' => $user->getUsername(), 'sn' => $user->getProperties()['lastName'] ?? array_pop(explode(' ', (string) $user->getDisplayName())), - 'mail' => $user->getEmail(), ]; + if ($user->getEmail()) { + $userInfo['mail'] = $user->getEmail(); + } + if ($user->getDisplayName()) { $userInfo['displayName'] = $user->getDisplayName(); } diff --git a/src/Services/SyncRemover/LdapSyncRemover.php b/src/Services/SyncRemover/LdapSyncRemover.php index 3001ce2..aab88d3 100644 --- a/src/Services/SyncRemover/LdapSyncRemover.php +++ b/src/Services/SyncRemover/LdapSyncRemover.php @@ -96,18 +96,15 @@ private function removeExcessGroups(array $groups): void } $syncGroup = $syncGroups[$existingGroup['cn'][0]]; - $groupDn = $this->client->getDn( - ['cn' => $syncGroup->getName()], + $groupDn = $this->client->generateDn( [ - 'cn' => $this->parentHelper->getParentGroups([], $syncGroup), + 'cn' => $this->parentHelper->getParentGroups([$syncGroup->getName()], $syncGroup), 'ou' => LdapGroupPushAdapter::GROUPS_ORG_UNIT, ] ); if ($groupDn !== $existingGroup['dn']) { $this->client->remove($existingGroup['dn']); - - continue; } } } diff --git a/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php b/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php index 4bced00..a82f782 100644 --- a/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php +++ b/src/SynchronizationAdapter/AdapterFactory/LdapAdapterFactory.php @@ -2,6 +2,7 @@ namespace LinkORB\OrgSync\SynchronizationAdapter\AdapterFactory; +use BadMethodCallException; use LinkORB\OrgSync\DTO\Target; use LinkORB\OrgSync\Services\Ldap\Client; use LinkORB\OrgSync\Services\Ldap\LdapParentHelper; @@ -11,6 +12,7 @@ use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\GroupPushInterface; use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\LdapGroupPushAdapter; use LinkORB\OrgSync\SynchronizationAdapter\OrganizationPull\OrganizationPullInterface; +use LinkORB\OrgSync\SynchronizationAdapter\SetPassword\LdapSetPasswordAdapter; use LinkORB\OrgSync\SynchronizationAdapter\SetPassword\SetPasswordInterface; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\LdapUserPushAdapter; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\UserPushInterface; @@ -34,12 +36,12 @@ public function __construct() public function createOrganizationPullAdapter(): OrganizationPullInterface { - // TODO: Implement createOrganizationPullAdapter() method. + throw new BadMethodCallException('Not implemented yet'); } public function createGroupPushAdapter(): GroupPushInterface { - return new LdapGroupPushAdapter($this->client, $this->userMapper, $this->parentHelper); + return new LdapGroupPushAdapter($this->client, $this->parentHelper); } public function createUserPushAdapter(): UserPushInterface @@ -49,7 +51,7 @@ public function createUserPushAdapter(): UserPushInterface public function createSetPasswordAdapter(): SetPasswordInterface { - // TODO: Implement createSetPasswordAdapter() method. + return new LdapSetPasswordAdapter($this->client); } public function setTarget(Target $target): AdapterFactoryInterface diff --git a/src/SynchronizationAdapter/GroupPush/LdapGroupPushAdapter.php b/src/SynchronizationAdapter/GroupPush/LdapGroupPushAdapter.php index 943ddf3..c092192 100644 --- a/src/SynchronizationAdapter/GroupPush/LdapGroupPushAdapter.php +++ b/src/SynchronizationAdapter/GroupPush/LdapGroupPushAdapter.php @@ -6,7 +6,6 @@ use LinkORB\OrgSync\Services\Ldap\Client; use LinkORB\OrgSync\Services\Ldap\LdapAssertionAwareTrait; use LinkORB\OrgSync\Services\Ldap\LdapParentHelper; -use LinkORB\OrgSync\Services\Ldap\UserDataMapper; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\LdapUserPushAdapter; class LdapGroupPushAdapter implements GroupPushInterface @@ -18,16 +17,12 @@ class LdapGroupPushAdapter implements GroupPushInterface /** @var Client */ private $client; - /** @var UserDataMapper */ - private $mapper; - /** @var LdapParentHelper */ private $parentHelper; - public function __construct(Client $client, UserDataMapper $mapper, LdapParentHelper $parentHelper) + public function __construct(Client $client, LdapParentHelper $parentHelper) { $this->client = $client; - $this->mapper = $mapper; $this->parentHelper = $parentHelper; } @@ -41,28 +36,29 @@ public function pushGroup(Group $group): GroupPushInterface $this->assertResult($groupsOrgUnit !== null, 'Error during search!'); if ($groupsOrgUnit === 0) { - $this->client->add([ - 'ou' => static::GROUPS_ORG_UNIT, - 'objectClass' => ['top', 'organizationalUnit'], - ]); + $this->client->add( + [ + 'ou' => static::GROUPS_ORG_UNIT, + 'objectClass' => ['top', 'organizationalUnit'], + ], + ['ou' => static::GROUPS_ORG_UNIT] + ); } - $groupRn = ['cn' => $this->parentHelper->getParentGroups([], $group), 'ou' => array_merge([static::GROUPS_ORG_UNIT])]; + $groupRn = ['cn' => $this->parentHelper->getParentGroups([], $group), 'ou' => [static::GROUPS_ORG_UNIT]]; - // TODO: Use ldap_add with LDAP_CONTROL_SYNC for PHP 7.3 - $groupSearchCount = $this->client->count( + $groupFirst = $this->client->first( $this->client->search(sprintf('(cn=%s)', $group->getName()), $groupRn) ); - $this->assertResult($groupSearchCount !== null, 'Error during search!'); - - if ($groupSearchCount === 0) { + if (!$groupFirst) { + array_unshift($groupRn['cn'], $group->getName()); $res = $this->client->add($groupInfo, $groupRn); } else { - $res = $this->client->modify($groupInfo, $groupRn); + $res = $this->client->modify($groupInfo, $this->client->getDn($groupFirst)); } - $this->assertResult((bool) $res, sprintf('Group \'%s\' wasn\'t added', $group->getName())); + $this->assertResult((bool)$res, sprintf('Group \'%s\' wasn\'t added', $group->getName())); return $this; } @@ -76,9 +72,8 @@ private function getGroupInfo(Group $group): array ]; foreach ($group->getMembers() as $member) { - $groupInfo['uniqueMember'][] = $this->client->getDn( - $this->mapper->map($member), - ['ou' => LdapUserPushAdapter::USERS_ORG_UNIT] + $groupInfo['uniqueMember'][] = $this->client->generateDn( + ['uid' => $member->getUsername(), 'ou' => LdapUserPushAdapter::USERS_ORG_UNIT] ); } diff --git a/src/SynchronizationAdapter/SetPassword/CamundaSetPasswordAdapter.php b/src/SynchronizationAdapter/SetPassword/CamundaSetPasswordAdapter.php index 4076a3f..13746d5 100644 --- a/src/SynchronizationAdapter/SetPassword/CamundaSetPasswordAdapter.php +++ b/src/SynchronizationAdapter/SetPassword/CamundaSetPasswordAdapter.php @@ -26,15 +26,16 @@ public function __construct(Client $httpClient, PasswordHelper $passwordHelper, $this->responseChecker = $responseChecker; } - public function setPassword(User $user, string $password): SetPasswordInterface + public function setPassword(User $user): SetPasswordInterface { - $authPassword = $user->getPassword() ?? $this->passwordHelper->getDefaultPassword($user->getUsername()); + $authPassword = $user->getProperties()[User::PREVIOUS_PASSWORD] + ?? $this->passwordHelper->getDefaultPassword($user->getUsername()); $response = $this->httpClient->put( sprintf('user/%s/credentials', $user->getUsername()), [ RequestOptions::JSON => [ - 'password' => $password, + 'password' => $user->getPassword(), 'authenticatedUserPassword' => $authPassword, ], ] diff --git a/src/SynchronizationAdapter/SetPassword/LdapSetPasswordAdapter.php b/src/SynchronizationAdapter/SetPassword/LdapSetPasswordAdapter.php new file mode 100644 index 0000000..fcb9102 --- /dev/null +++ b/src/SynchronizationAdapter/SetPassword/LdapSetPasswordAdapter.php @@ -0,0 +1,57 @@ +client = $client; + } + + public function setPassword(User $user): SetPasswordInterface + { + $usersOrgUnit = $this->client->count( + $this->client->search(sprintf('(ou=%s)', LdapUserPushAdapter::USERS_ORG_UNIT)) + ); + + $this->assertResult($usersOrgUnit !== null && $usersOrgUnit > 0, 'Error during search!'); + + $userSearchFirstDn = $this->client->getDn( + $this->client->first( + $this->client->search( + sprintf('(cn=%s)', $user->getUsername()), + ['ou' => LdapUserPushAdapter::USERS_ORG_UNIT] + ) + ) + ); + + $res = $this->client->modify( + ['userPassword' => $this->encodePassword($user->getPassword())], + $userSearchFirstDn + ); + + $this->assertResult((bool)$res, sprintf('User \'%s\' password wasn\'t changed', $user->getUsername())); + + return $this; + } + + private function encodePassword(string $password): string + { + $salt = substr(str_shuffle( + str_repeat('ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789', 4) + ), 0, 4); + + return '{SSHA}' . base64_encode(sha1($password . $salt, true) . $salt); + } +} diff --git a/src/SynchronizationAdapter/SetPassword/SetPasswordInterface.php b/src/SynchronizationAdapter/SetPassword/SetPasswordInterface.php index 275991d..8bfe5a0 100644 --- a/src/SynchronizationAdapter/SetPassword/SetPasswordInterface.php +++ b/src/SynchronizationAdapter/SetPassword/SetPasswordInterface.php @@ -6,5 +6,5 @@ interface SetPasswordInterface { - public function setPassword(User $user, string $password): self; + public function setPassword(User $user): self; } diff --git a/src/SynchronizationAdapter/UserPush/LdapUserPushAdapter.php b/src/SynchronizationAdapter/UserPush/LdapUserPushAdapter.php index 53137f9..d8228eb 100644 --- a/src/SynchronizationAdapter/UserPush/LdapUserPushAdapter.php +++ b/src/SynchronizationAdapter/UserPush/LdapUserPushAdapter.php @@ -35,26 +35,26 @@ public function pushUser(User $user): UserPushInterface $this->assertResult($usersOrgUnit !== null, 'Error during search!'); if ($usersOrgUnit === 0) { - $this->client->add([ - 'ou' => static::USERS_ORG_UNIT, - 'objectClass' => ['top', 'organizationalUnit'], - ]); + $this->client->add( + [ + 'ou' => static::USERS_ORG_UNIT, + 'objectClass' => ['top', 'organizationalUnit'], + ], + ['ou' => static::USERS_ORG_UNIT] + ); } - // TODO: Use ldap_add with LDAP_CONTROL_SYNC for PHP 7.3 - $userSearchCount = $this->client->count( + $userFirst = $this->client->first( $this->client->search(sprintf('(cn=%s)', $user->getUsername()), ['ou' => static::USERS_ORG_UNIT]) ); - $this->assertResult($userSearchCount !== null, 'Error during search!'); - - if ($userSearchCount === 0) { - $res = $this->client->add($userInfo, ['ou' => static::USERS_ORG_UNIT]); + if (!$userFirst) { + $res = $this->client->add($userInfo, ['uid' => $user->getUsername(), 'ou' => static::USERS_ORG_UNIT]); } else { - $res = $this->client->modify($userInfo, ['ou' => static::USERS_ORG_UNIT]); + $res = $this->client->modify($userInfo, $this->client->getDn($userFirst)); } - $this->assertResult((bool) $res, sprintf('User \'%s\' wasn\'t added', $user->getUsername())); + $this->assertResult((bool)$res, sprintf('User \'%s\' wasn\'t pushed', $user->getUsername())); return $this; } diff --git a/src/SynchronizationMediator/SynchronizationMediator.php b/src/SynchronizationMediator/SynchronizationMediator.php index bc38cf3..ed13e78 100644 --- a/src/SynchronizationMediator/SynchronizationMediator.php +++ b/src/SynchronizationMediator/SynchronizationMediator.php @@ -92,7 +92,7 @@ public function pushUser(User $user): SynchronizationMediatorInterface return $this; } - public function setPassword(User $user, string $password): SynchronizationMediatorInterface + public function setPassword(User $user): SynchronizationMediatorInterface { foreach ($this->inputHandler->getTargets() as $target) { $this->setTarget($target); @@ -101,7 +101,7 @@ public function setPassword(User $user, string $password): SynchronizationMediat continue; } - $this->adapterFactory->createSetPasswordAdapter()->setPassword($user, $password); + $this->adapterFactory->createSetPasswordAdapter()->setPassword($user); } $this->adapterFactory = null; diff --git a/src/SynchronizationMediator/SynchronizationMediatorInterface.php b/src/SynchronizationMediator/SynchronizationMediatorInterface.php index 95483e5..3f152c7 100644 --- a/src/SynchronizationMediator/SynchronizationMediatorInterface.php +++ b/src/SynchronizationMediator/SynchronizationMediatorInterface.php @@ -15,7 +15,7 @@ public function pushGroup(Group $organization): self; public function pushUser(User $user): self; - public function setPassword(User $user, string $password): self; + public function setPassword(User $user): self; public function pullOrganization(): Organization; diff --git a/tests/Unit/Services/SyncRemover/LdapSyncRemoverTest.php b/tests/Unit/Services/SyncRemover/LdapSyncRemoverTest.php index 51001b0..c321f0b 100644 --- a/tests/Unit/Services/SyncRemover/LdapSyncRemoverTest.php +++ b/tests/Unit/Services/SyncRemover/LdapSyncRemoverTest.php @@ -2,10 +2,14 @@ namespace LinkORB\OrgSync\Tests\Unit\Services\SyncRemover; +use LinkORB\OrgSync\DTO\Group; use LinkORB\OrgSync\DTO\Organization; use LinkORB\OrgSync\DTO\User; use LinkORB\OrgSync\Services\Ldap\Client; +use LinkORB\OrgSync\Services\Ldap\LdapParentHelper; use LinkORB\OrgSync\Services\SyncRemover\LdapSyncRemover; +use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\LdapGroupPushAdapter; +use LinkORB\OrgSync\SynchronizationAdapter\UserPush\LdapUserPushAdapter; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -17,25 +21,38 @@ class LdapSyncRemoverTest extends TestCase /** @var Client|MockObject */ private $client; + /** @var LdapParentHelper|MockObject */ + private $parentHelper; + protected function setUp(): void { $this->client = $this->createMock(Client::class); - $this->syncRemover = new LdapSyncRemover($this->client); + $this->parentHelper = $this->createMock(LdapParentHelper::class); + $this->syncRemover = new LdapSyncRemover($this->client, $this->parentHelper); } /** - * @dataProvider getRemoveData + * @dataProvider getRemoveUsersData */ - public function testRemoveNonExists(array $orgUsers, array $existingUsers,array $expectedToRemoveUsers) + public function testRemoveNonExistsUsers(array $orgUsers, array $existingUsers, array $expectedToRemoveUsers) { $organization = new Organization('', $orgUsers); $searchResult = new \stdClass(); $this->client - ->expects($this->once()) + ->expects($this->exactly(3)) ->method('search') - ->with('(objectClass=inetOrgPerson)') - ->willReturn($searchResult); + ->withConsecutive( + [sprintf('(ou=%s)', LdapUserPushAdapter::USERS_ORG_UNIT)], + [sprintf('(ou=%s)', LdapGroupPushAdapter::GROUPS_ORG_UNIT)], + ['(objectClass=inetOrgPerson)'] + ) + ->willReturnOnConsecutiveCalls(true, null, $searchResult); + $this->client + ->expects($this->exactly(2)) + ->method('count') + ->withConsecutive([true], [null]) + ->willReturnOnConsecutiveCalls(1, 0); $this->client ->expects($this->once()) @@ -51,7 +68,63 @@ public function testRemoveNonExists(array $orgUsers, array $existingUsers,array $this->assertNull($this->syncRemover->removeNonExists($organization)); } - public function getRemoveData(): array + /** + * @param Group[] $orgGroups + * @dataProvider getRemoveGroupsData + */ + public function testRemoveNonExistsGroups(array $orgGroups, array $existingGroups, array $expectedToRemoveGroups) + { + $organization = new Organization('', [], $orgGroups); + + $searchResult = new \stdClass(); + $this->client + ->expects($this->exactly(3)) + ->method('search') + ->withConsecutive( + [sprintf('(ou=%s)', LdapUserPushAdapter::USERS_ORG_UNIT)], + [sprintf('(ou=%s)', LdapGroupPushAdapter::GROUPS_ORG_UNIT)], + ['(objectClass=groupOfUniqueNames)', ['ou' => LdapGroupPushAdapter::GROUPS_ORG_UNIT]] + ) + ->willReturnOnConsecutiveCalls(null, true, $searchResult); + $this->client + ->expects($this->exactly(2)) + ->method('count') + ->withConsecutive([null], [true]) + ->willReturnOnConsecutiveCalls(0, 1); + + $this->client + ->expects($this->once()) + ->method('all') + ->with($searchResult) + ->willReturn($existingGroups); + + $this->parentHelper + ->method('getParentGroups') + ->willReturnCallback(function (array $initialParent, Group $group) { + return [$group->getName()]; + }); + + $this->client + ->method('generateDn') + ->willReturnCallback(function (array $rdn) use ($orgGroups) { + foreach ($orgGroups as $group) { + if ($group->getName() === $rdn['cn'][0]) { + return $group->getProperties()['dn']; + } + } + + return ''; + }); + + $this->client + ->expects($this->exactly(count($expectedToRemoveGroups))) + ->method('remove') + ->withConsecutive(...$expectedToRemoveGroups); + + $this->assertNull($this->syncRemover->removeNonExists($organization)); + } + + public function getRemoveUsersData(): array { return [ [ @@ -100,4 +173,66 @@ public function getRemoveData(): array ], ]; } + + public function getRemoveGroupsData(): array + { + return [ + [ + [ + new Group('test011', '', null, null, [], ['dn' => 'test11del22']), + new Group('test033', '', null, null, [], ['dn' => 'test91del62']), + ], + [ + 'count' => 3, + ['cn' => ['test0011', 'test011'], 'dn' => 'test11del422'], + ['cn' => ['test011'], 'dn' => 'test11del22'], + ['cn' => ['test123'], 'dn' => 'test11del99'], + ], + [['test11del422'], ['test11del99']], + ], + [ + [], + [ + 'count' => 2, + ['cn' => ['test11'], 'dn' => 'test121del22'], + ['cn' => ['test123'], 'dn' => 'test121del99'], + ], + [['test121del22'], ['test121del99']], + ], + [ + [ + new Group('test011', '', null, null, [], ['dn' => 'rty']), + new Group('test033', '', null, null, [], ['dn' => 'qwe']), + ], + [ + 'count' => 0, + ], + [], + ], + [ + [ + new Group('group11', '', null, null, [], ['dn' => '1rty']), + new Group('group33', '', null, null, [], ['dn' => 'rty3']), + ], + [ + 'count' => 2, + ['cn' => ['group11'], 'dn' => '1rty'], + ['cn' => ['group33'], 'dn' => 'rty3'], + ], + [], + ], + [ + [ + new Group('group11', '', null, null, [], ['dn' => 'different']), + new Group('group33', '', null, null, [], ['dn' => 'rty3']), + ], + [ + 'count' => 2, + ['cn' => ['group11'], 'dn' => '1rty'], + ['cn' => ['group33'], 'dn' => 'rty3'], + ], + [['1rty']], + ], + ]; + } } diff --git a/tests/Unit/SynchronizationAdapter/AdapterFactory/CamundaAdapterFactoryTest.php b/tests/Unit/SynchronizationAdapter/AdapterFactory/CamundaAdapterFactoryTest.php index 9a2eb68..d6c5e16 100644 --- a/tests/Unit/SynchronizationAdapter/AdapterFactory/CamundaAdapterFactoryTest.php +++ b/tests/Unit/SynchronizationAdapter/AdapterFactory/CamundaAdapterFactoryTest.php @@ -78,7 +78,9 @@ public function testCreateUserPushAdapter() public function testCreateOrganizationPullAdapter() { - $this->markTestSkipped('Need to implement'); + $this->expectException(\BadMethodCallException::class); + + $this->factory->createOrganizationPullAdapter(); } public function testCreateSetPasswordAdapter() diff --git a/tests/Unit/SynchronizationAdapter/AdapterFactory/LdapAdapterFactoryTest.php b/tests/Unit/SynchronizationAdapter/AdapterFactory/LdapAdapterFactoryTest.php index 511abbd..fd56c83 100644 --- a/tests/Unit/SynchronizationAdapter/AdapterFactory/LdapAdapterFactoryTest.php +++ b/tests/Unit/SynchronizationAdapter/AdapterFactory/LdapAdapterFactoryTest.php @@ -27,6 +27,7 @@ protected function setUp(): void $this->factory = $this->createPartialMock(LdapAdapterFactory::class, ['getClient']); $this->factory->method('getClient')->willReturn($this->client); $this->factory->setTarget(new Ldap('', '', '', '', [])); + $this->factory->__construct(); } public function testCreateUserPushAdapter() diff --git a/tests/Unit/SynchronizationAdapter/SetPassword/CamundaSetPasswordAdapterTest.php b/tests/Unit/SynchronizationAdapter/SetPassword/CamundaSetPasswordAdapterTest.php index c7911ba..c1868a1 100644 --- a/tests/Unit/SynchronizationAdapter/SetPassword/CamundaSetPasswordAdapterTest.php +++ b/tests/Unit/SynchronizationAdapter/SetPassword/CamundaSetPasswordAdapterTest.php @@ -41,8 +41,8 @@ public function testSetPassword() { $id = 'testId'; - $user = $this->createConfiguredMock(User::class, ['getUsername' => $id]); $password = '0123456789'; + $user = $this->createConfiguredMock(User::class, ['getUsername' => $id, 'getPassword' => $password]); $this->httpClient ->expects($this->once()) @@ -61,6 +61,6 @@ public function testSetPassword() ) ->willReturn($this->createConfiguredMock(ResponseInterface::class, ['getStatusCode' => 200])); - $this->assertSame($this->adapter, $this->adapter->setPassword($user, $password)); + $this->assertSame($this->adapter, $this->adapter->setPassword($user)); } } diff --git a/tests/Unit/SynchronizationAdapter/UserPush/LdapUserPushAdapterTest.php b/tests/Unit/SynchronizationAdapter/UserPush/LdapUserPushAdapterTest.php index 91836d9..fb8a045 100644 --- a/tests/Unit/SynchronizationAdapter/UserPush/LdapUserPushAdapterTest.php +++ b/tests/Unit/SynchronizationAdapter/UserPush/LdapUserPushAdapterTest.php @@ -4,6 +4,7 @@ use LinkORB\OrgSync\DTO\User; use LinkORB\OrgSync\Services\Ldap\Client; +use LinkORB\OrgSync\Services\Ldap\UserDataMapper; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\LdapUserPushAdapter; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -20,7 +21,7 @@ class LdapUserPushAdapterTest extends TestCase protected function setUp(): void { $this->client = $this->createMock(Client::class); - $this->adapter = new LdapUserPushAdapter($this->client); + $this->adapter = new LdapUserPushAdapter($this->client, new UserDataMapper()); } /** @@ -29,30 +30,76 @@ protected function setUp(): void public function testPushUser(User $userObj, array $userArr, array $searchResults) { $this->client - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('search') - ->with($this->callback(function (string $query) use ($userObj) { - return strstr($query, $userObj->getUsername()) !== false; - })) - ->willReturn($searchResults); + ->withConsecutive( + [$this->anything()], + [$this->callback(function (string $query) use ($userObj) { + return strstr($query, $userObj->getUsername()) !== false; + })] + ) + ->willReturnOnConsecutiveCalls(null, $searchResults); + + $this->client->expects($this->once())->method('count')->willReturn(1); $this->client ->expects($this->once()) - ->method('count') + ->method('first') ->with($searchResults) ->willReturnCallback(function (array $result) { - return count($result); + return reset($result); }); - $this->client - ->expects($this->once()) - ->method(count($searchResults) > 0 ? 'modify' : 'add') - ->with($userArr) - ->willReturn(true); + 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($userArr, $dn) + ->willReturn(true); + } else { + $this->client + ->expects($this->once()) + ->method('add') + ->with($userArr, ['uid' => $userObj->getUsername(), 'ou' => LdapUserPushAdapter::USERS_ORG_UNIT]) + ->willReturn(true); + } + $this->assertSame($this->adapter, $this->adapter->pushUser($userObj)); } + public function testPushUserOrgUnitCreation() + { + $user = new User('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)', LdapUserPushAdapter::USERS_ORG_UNIT)], [$this->anything()]) + ->willReturnOnConsecutiveCalls([], [1]); + + $this->client->expects($this->once())->method('count')->willReturn(0); + + $this->client->expects($this->atLeastOnce())->method('add')->withConsecutive( + [ + [ + 'ou' => LdapUserPushAdapter::USERS_ORG_UNIT, + 'objectClass' => ['top', 'organizationalUnit'], + ], + ['ou' => LdapUserPushAdapter::USERS_ORG_UNIT] + ], + [$this->anything()] + ); + + $this->assertSame($this->adapter, $this->adapter->pushUser($user)); + } + public function testPushUserSearchException() { $this->client->method('count')->willReturn(null); @@ -65,11 +112,12 @@ public function testPushUserSearchException() public function testPushUserAddException() { - $this->client->method('count')->willReturn(0); + $this->client->method('count')->willReturn(1); $this->client->method('add')->willReturn(false); + $this->client->method('first')->willReturn(false); $this->expectException(UnexpectedValueException::class); - $this->expectExceptionMessage('User \'undefined\' wasn\'t added'); + $this->expectExceptionMessage('User \'undefined\' wasn\'t pushed'); $this->adapter->pushUser(new User('undefined', null, '')); } @@ -81,6 +129,7 @@ public function getTestPushUserData(): array new User('temp123', null, 'temp@c.com', 'star', null, ['lastName' => 'Testenko']), [ 'cn' => 'temp123', + 'uid' => 'temp123', 'sn' => 'Testenko', 'mail' => 'temp@c.com', 'displayName' => 'star', @@ -92,6 +141,7 @@ public function getTestPushUserData(): array new User('temp000', null, 'temp99@a.com', 'super mega star', 'ava.gif'), [ 'cn' => 'temp000', + 'uid' => 'temp000', 'sn' => 'star', 'mail' => 'temp99@a.com', 'displayName' => 'super mega star', @@ -104,11 +154,12 @@ public function getTestPushUserData(): array new User('987', null, 'a@a.nl'), [ 'cn' => '987', + 'uid' => '987', 'sn' => '', 'mail' => 'a@a.nl', 'objectClass' => ['inetOrgPerson', 'organizationalPerson', 'person', 'top'], ], - [1,2,3] + [1, 2, 3] ] ]; } diff --git a/tests/Unit/SynchronizationMediator/SynchronizationMediatorTest.php b/tests/Unit/SynchronizationMediator/SynchronizationMediatorTest.php index 84426e8..61e0cc9 100644 --- a/tests/Unit/SynchronizationMediator/SynchronizationMediatorTest.php +++ b/tests/Unit/SynchronizationMediator/SynchronizationMediatorTest.php @@ -198,7 +198,6 @@ public function testPushUser() public function testSetPassword() { $user = $this->createMock(User::class); - $password = '1234Qwer'; $targets = [ $this->createMock(Camunda::class), @@ -231,12 +230,12 @@ public function testSetPassword() $this->setPasswordAdapter ->expects($this->exactly(count($expectedTargets))) ->method('setPassword') - ->with($user, $password) + ->with($user) ->willReturnSelf(); $this->assertSame( $this->mediator, - $this->mediator->setPassword($user, $password) + $this->mediator->setPassword($user) ); } From 887c1dcbf8691eb8de14f09919732b0e4f03d82b Mon Sep 17 00:00:00 2001 From: amsprost Date: Fri, 4 Oct 2019 02:13:40 +0200 Subject: [PATCH 08/13] 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('', '')); + } +} From 9d3e52146bd81ca90bb17cac84ca764fd662272e Mon Sep 17 00:00:00 2001 From: amsprost Date: Mon, 7 Oct 2019 01:30:10 +0200 Subject: [PATCH 09/13] CARD-2527: Finished User push & user removal --- composer.json | 7 +- src/DTO/Target.php | 3 +- src/DTO/Target/Mattermost.php | 59 ++++++++++ src/Services/Camunda/ResponseChecker.php | 10 +- .../Github/AuthenticatedClientDecorator.php | 27 ----- .../SyncRemover/MattermostSyncRemover.php | 53 +++++++++ .../MatterMostAdapterFactory.php | 48 -------- .../MattermostAdapterFactory.php | 104 ++++++++++++++++++ .../UserPush/MattermostUserPushAdapter.php | 64 +++++++++++ 9 files changed, 293 insertions(+), 82 deletions(-) create mode 100644 src/DTO/Target/Mattermost.php delete mode 100644 src/Services/Github/AuthenticatedClientDecorator.php create mode 100644 src/Services/SyncRemover/MattermostSyncRemover.php delete mode 100644 src/SynchronizationAdapter/AdapterFactory/MatterMostAdapterFactory.php create mode 100644 src/SynchronizationAdapter/AdapterFactory/MattermostAdapterFactory.php create mode 100644 src/SynchronizationAdapter/UserPush/MattermostUserPushAdapter.php diff --git a/composer.json b/composer.json index 9b2ab61..447ce92 100644 --- a/composer.json +++ b/composer.json @@ -5,15 +5,16 @@ "description": "Organization sync library", "require": { "php": "^7.2", + "ext-json": "*", + "ext-ldap": "*", "doctrine/annotations": "^1.6", + "gnello/php-mattermost-driver": "^2.9", "knplabs/github-api": "^2.11", "php-http/guzzle6-adapter": "^1.1", "symfony/cache": "^4.3", "symfony/property-access": "^4.3", "symfony/property-info": "^4.3", - "symfony/serializer": "^4.3", - "ext-json": "*", - "ext-ldap": "*" + "symfony/serializer": "^4.3" }, "require-dev": { "phpunit/phpunit": "^8" diff --git a/src/DTO/Target.php b/src/DTO/Target.php index 983b91a..19001ea 100644 --- a/src/DTO/Target.php +++ b/src/DTO/Target.php @@ -9,6 +9,7 @@ * "camunda"="LinkORB\OrgSync\DTO\Target\Camunda", * "github"="LinkORB\OrgSync\DTO\Target\Github", * "ldap"="LinkORB\OrgSync\DTO\Target\Ldap", + * "mattermost"="LinkORB\OrgSync\DTO\Target\Mattermost", * }) */ abstract class Target @@ -19,7 +20,7 @@ abstract class Target public const PULL_ORGANIZATION = 'organization_pull'; /** @var string */ - private $baseUrl; + protected $baseUrl; /** @var string */ private $name; diff --git a/src/DTO/Target/Mattermost.php b/src/DTO/Target/Mattermost.php new file mode 100644 index 0000000..0f7e904 --- /dev/null +++ b/src/DTO/Target/Mattermost.php @@ -0,0 +1,59 @@ +getBaseUrl(), 2); + + $this->baseUrl = end($urlParts); + $this->scheme = reset($urlParts); + + $this->token = $token; + $this->login = $login; + $this->password = $password; + } + + public function getToken(): ?string + { + return $this->token; + } + + public function getLogin(): ?string + { + return $this->login; + } + + public function getPassword(): ?string + { + return $this->password; + } + + public function getScheme(): string + { + return $this->scheme; + } +} diff --git a/src/Services/Camunda/ResponseChecker.php b/src/Services/Camunda/ResponseChecker.php index e6e01b7..ee465f1 100644 --- a/src/Services/Camunda/ResponseChecker.php +++ b/src/Services/Camunda/ResponseChecker.php @@ -18,17 +18,21 @@ class ResponseChecker /** @var string */ private $contextException; - public function __construct(string $contextDto) + /** @var string[] */ + private $allowedCodes; + + public function __construct(string $contextDto, array $allowedCodes = []) { assert(array_key_exists($contextDto, static::CONTEXT_MAP)); $this->contextException = static::CONTEXT_MAP[$contextDto]; + $this->allowedCodes = $allowedCodes; } public function assertResponse(ResponseInterface $response): void { - if ($response->getStatusCode() >= 400) { + if ($response->getStatusCode() >= 400 && !in_array($response->getStatusCode(), $this->allowedCodes)) { throw new $this->contextException((string)$response->getBody(), $response->getStatusCode()); } } -} \ No newline at end of file +} diff --git a/src/Services/Github/AuthenticatedClientDecorator.php b/src/Services/Github/AuthenticatedClientDecorator.php deleted file mode 100644 index 189fc36..0000000 --- a/src/Services/Github/AuthenticatedClientDecorator.php +++ /dev/null @@ -1,27 +0,0 @@ -token, Client::AUTH_HTTP_TOKEN); - - return parent::__call($name, $args); - } - - public function authenticate($tokenOrLogin, $password = null, $authMethod = null) - { - $this->token = $tokenOrLogin; - - parent::authenticate($tokenOrLogin, $password, $authMethod); - } -} diff --git a/src/Services/SyncRemover/MattermostSyncRemover.php b/src/Services/SyncRemover/MattermostSyncRemover.php new file mode 100644 index 0000000..95765ea --- /dev/null +++ b/src/Services/SyncRemover/MattermostSyncRemover.php @@ -0,0 +1,53 @@ +driver = $driver; + } + + public function removeNonExists(Organization $organization): void + { + $existingUsers = $this->getExistingUsers(); + + // Removing users + $syncUsers = []; + foreach ($organization->getUsers() as $user) { + $syncUsers[$user->getUsername()] = $user; + } + + foreach ($existingUsers as $existingUser) { + if (!isset($syncUsers[$existingUser['username']])) { + $this->driver->getUserModel()->deactivateUserAccount($existingUser['id']); + } + } + } + + private function getExistingUsers(): array + { + $users = json_decode( + (string) $this->driver->getUserModel()->getUsers(['per_page' => 200])->getBody(), + true + ); + + $users = array_filter($users, function (array $user) { + return $user['delete_at'] === 0; + }); + + return array_map(function (array $user) { + return [ + 'username' => $user['username'] ?? '', + 'id' => $user['id'] ?? '', + ]; + }, $users); + } +} diff --git a/src/SynchronizationAdapter/AdapterFactory/MatterMostAdapterFactory.php b/src/SynchronizationAdapter/AdapterFactory/MatterMostAdapterFactory.php deleted file mode 100644 index b8b5142..0000000 --- a/src/SynchronizationAdapter/AdapterFactory/MatterMostAdapterFactory.php +++ /dev/null @@ -1,48 +0,0 @@ -defaultPassSalt = $defaultPassSalt; + } + + public function createOrganizationPullAdapter(): OrganizationPullInterface + { + // TODO: Implement createOrganizationPullAdapter() method. + } + + public function createGroupPushAdapter(): GroupPushInterface + { + throw new BadMethodCallException('Not implemented yet'); + } + + public function createUserPushAdapter(): UserPushInterface + { + return new MattermostUserPushAdapter( + new ResponseChecker(User::class, [404]), + $this->driver, + $this->passwordHelper + ); + } + + public function createSetPasswordAdapter(): SetPasswordInterface + { + // TODO: Implement createSetPasswordAdapter() method. + } + + public function setTarget(Target $target): AdapterFactoryInterface + { + assert($target instanceof Mattermost); + + if (!empty($target->getToken())) { + $driverOpts = [ + 'token' => $target->getToken() + ]; + } else { + $driverOpts = [ + 'login_id' => $target->getLogin(), + 'password' => $target->getPassword(), + ]; + } + + $driverOpts['url'] = $target->getBaseUrl(); + $driverOpts['scheme'] = $target->getScheme(); + + $container = new Container([ + 'driver' => $driverOpts + ]); + + $this->driver = new Driver($container); + $this->driver->authenticate(); + $this->passwordHelper = $this->getPasswordHelper($this->defaultPassSalt . $target->getName()); + + return $this; + } + + public function createSyncRemover(): SyncRemoverInterface + { + return new MattermostSyncRemover($this->driver); + } + + public function supports(string $action): bool + { + return false; + } + + protected function getPasswordHelper(?string $salt): PasswordHelper + { + return new PasswordHelper($salt); + } +} diff --git a/src/SynchronizationAdapter/UserPush/MattermostUserPushAdapter.php b/src/SynchronizationAdapter/UserPush/MattermostUserPushAdapter.php new file mode 100644 index 0000000..5c2ab8c --- /dev/null +++ b/src/SynchronizationAdapter/UserPush/MattermostUserPushAdapter.php @@ -0,0 +1,64 @@ +checker = $checker; + $this->driver = $driver; + $this->passwordHelper = $passwordHelper; + } + + public function pushUser(User $user): UserPushInterface + { + $response = $this->driver->getUserModel()->getUserByUsername($user->getUsername()); + $this->checker->assertResponse($response); + + $requestOptions = $this->getRequestOptions($user); + + if ($response->getStatusCode() === 200) { + $responseBody = json_decode((string) $response->getBody(), true); + + if ($responseBody['delete_at'] !== 0) { + $this->driver->getUserModel()->updateUserActive($responseBody['id'] ?? '', ['active' => true]); + } + + $response = $this->driver->getUserModel()->patchUser($responseBody['id'] ?? '', $requestOptions); + } else { + $requestOptions['password'] = $this->passwordHelper->getDefaultPassword($user->getUsername()); + + $response = $this->driver->getUserModel()->createUser($requestOptions); + } + + $this->checker->assertResponse($response); + + return $this; + } + + private function getRequestOptions(User $user): array + { + return [ + 'email' => $user->getEmail(), + 'username' => $user->getUsername(), + 'first_name' => $user->getProperties()['firstName'] ?? null, + 'last_name' => $user->getProperties()['lastName'] ?? null, + 'nickname' => $user->getDisplayName(), + ]; + } +} From 220e9e4a377b217d4e95e46a0d275ff5aacff4ed Mon Sep 17 00:00:00 2001 From: amsprost Date: Mon, 14 Oct 2019 00:09:13 +0200 Subject: [PATCH 10/13] CARD-2527: Finished Mattermost implementation --- .../Mattermost/BaseEntriesProvider.php | 72 ++++++++++++ .../SyncRemover/MattermostSyncRemover.php | 60 ++++++---- .../MattermostAdapterFactory.php | 21 +++- .../GroupPush/MattermostGroupPushAdapter.php | 108 ++++++++++++++++++ .../MattermostSetPasswordAdapter.php | 49 ++++++++ 5 files changed, 285 insertions(+), 25 deletions(-) create mode 100644 src/Services/Mattermost/BaseEntriesProvider.php create mode 100644 src/SynchronizationAdapter/GroupPush/MattermostGroupPushAdapter.php create mode 100644 src/SynchronizationAdapter/SetPassword/MattermostSetPasswordAdapter.php diff --git a/src/Services/Mattermost/BaseEntriesProvider.php b/src/Services/Mattermost/BaseEntriesProvider.php new file mode 100644 index 0000000..27795aa --- /dev/null +++ b/src/Services/Mattermost/BaseEntriesProvider.php @@ -0,0 +1,72 @@ +driver = $driver; + } + + public function getExistingUsers(): array + { + return array_map(function (array $user) { + return [ + 'username' => $user['username'] ?? '', + 'id' => $user['id'] ?? '', + ]; + }, $this->getBaseEntries(function (int $limit) { + return $this->driver->getUserModel()->getUsers(['per_page' => $limit]); + })); + } + + public function getExistingGroups(): array + { + return array_map(function (array $group) { + return [ + 'name' => $group['name'] ?? '', + 'id' => $group['id'] ?? '', + ]; + }, $this->getBaseEntries(function (int $limit) { + return $this->driver->getTeamModel()->getTeams(['per_page' => $limit]); + })); + } + + public function getTeamMembers(string $teamId): array + { + return array_map(function (array $member) { + return $member['user_id'] ?? ''; + }, $this->getBaseEntries(function (int $limit) use ($teamId) { + return $this->driver->getTeamModel()->getTeamMembers($teamId, ['per_page' => $limit]); + })); + } + + private function getBaseEntries(callable $driverFn): array + { + $entries = []; + + $limit = 200; + do { + /** @var ResponseInterface $response */ + $response = $driverFn($limit); + + $responseData = json_decode( + (string)$response->getBody(), + true + ); + + $entries = array_merge($entries, $responseData); + } while (count($responseData) === $limit); + + return array_filter($entries, function (array $entry) { + return $entry['delete_at'] === 0; + }); + } +} diff --git a/src/Services/SyncRemover/MattermostSyncRemover.php b/src/Services/SyncRemover/MattermostSyncRemover.php index 95765ea..5536343 100644 --- a/src/Services/SyncRemover/MattermostSyncRemover.php +++ b/src/Services/SyncRemover/MattermostSyncRemover.php @@ -3,21 +3,28 @@ namespace LinkORB\OrgSync\Services\SyncRemover; use Gnello\Mattermost\Driver; +use LinkORB\OrgSync\DTO\Group; use LinkORB\OrgSync\DTO\Organization; +use LinkORB\OrgSync\Services\Mattermost\BaseEntriesProvider; class MattermostSyncRemover implements SyncRemoverInterface { /** @var Driver */ private $driver; - public function __construct(Driver $driver) + /** @var BaseEntriesProvider */ + private $provider; + + public function __construct(Driver $driver, BaseEntriesProvider $provider) { $this->driver = $driver; + $this->provider = $provider; } public function removeNonExists(Organization $organization): void { - $existingUsers = $this->getExistingUsers(); + $existingUsers = $this->provider->getExistingUsers(); + $existingGroups = $this->provider->getExistingGroups(); // Removing users $syncUsers = []; @@ -25,29 +32,42 @@ public function removeNonExists(Organization $organization): void $syncUsers[$user->getUsername()] = $user; } + $existingUsersIds = []; foreach ($existingUsers as $existingUser) { + $existingUsersIds[$existingUser['username']] = $existingUser['id']; + if (!isset($syncUsers[$existingUser['username']])) { $this->driver->getUserModel()->deactivateUserAccount($existingUser['id']); } } - } - private function getExistingUsers(): array - { - $users = json_decode( - (string) $this->driver->getUserModel()->getUsers(['per_page' => 200])->getBody(), - true - ); - - $users = array_filter($users, function (array $user) { - return $user['delete_at'] === 0; - }); - - return array_map(function (array $user) { - return [ - 'username' => $user['username'] ?? '', - 'id' => $user['id'] ?? '', - ]; - }, $users); + // Removing groups + $syncGroups = []; + foreach ($organization->getGroups() as $group) { + $syncGroups[$group->getName()] = $group; + } + + foreach ($existingGroups as $existingGroup) { + if (!isset($syncGroups[$existingGroup['name']])) { + $this->driver->getTeamModel()->deleteTeam($existingGroup['id']); + + continue; + } + + $membersMap = []; + /** @var Group $group */ + $group = $syncGroups[$existingGroup['name']]; + foreach ($group->getMembers() as $member) { + $membersMap[$existingUsersIds[$member->getUsername()]] = true; + } + + foreach ($this->provider->getTeamMembers($existingGroup['id']) as $member) { + if (!isset($membersMap[$member])) { + $this->driver->getTeamModel()->removeUser($existingGroup['id'], $member, []); + } + } + } } + + } diff --git a/src/SynchronizationAdapter/AdapterFactory/MattermostAdapterFactory.php b/src/SynchronizationAdapter/AdapterFactory/MattermostAdapterFactory.php index d9f1566..84474fe 100644 --- a/src/SynchronizationAdapter/AdapterFactory/MattermostAdapterFactory.php +++ b/src/SynchronizationAdapter/AdapterFactory/MattermostAdapterFactory.php @@ -8,11 +8,14 @@ use LinkORB\OrgSync\DTO\Target\Mattermost; use LinkORB\OrgSync\DTO\User; use LinkORB\OrgSync\Services\Camunda\ResponseChecker; +use LinkORB\OrgSync\Services\Mattermost\BaseEntriesProvider; use LinkORB\OrgSync\Services\PasswordHelper; use LinkORB\OrgSync\Services\SyncRemover\MattermostSyncRemover; use LinkORB\OrgSync\Services\SyncRemover\SyncRemoverInterface; use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\GroupPushInterface; +use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\MattermostGroupPushAdapter; use LinkORB\OrgSync\SynchronizationAdapter\OrganizationPull\OrganizationPullInterface; +use LinkORB\OrgSync\SynchronizationAdapter\SetPassword\MattermostSetPasswordAdapter; use LinkORB\OrgSync\SynchronizationAdapter\SetPassword\SetPasswordInterface; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\MattermostUserPushAdapter; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\UserPushInterface; @@ -36,12 +39,12 @@ public function __construct(?string $defaultPassSalt) public function createOrganizationPullAdapter(): OrganizationPullInterface { - // TODO: Implement createOrganizationPullAdapter() method. + throw new BadMethodCallException('Not implemented yet'); } public function createGroupPushAdapter(): GroupPushInterface { - throw new BadMethodCallException('Not implemented yet'); + return new MattermostGroupPushAdapter($this->driver); } public function createUserPushAdapter(): UserPushInterface @@ -55,7 +58,11 @@ public function createUserPushAdapter(): UserPushInterface public function createSetPasswordAdapter(): SetPasswordInterface { - // TODO: Implement createSetPasswordAdapter() method. + return new MattermostSetPasswordAdapter( + $this->driver, + $this->passwordHelper, + new ResponseChecker(User::class) + ); } public function setTarget(Target $target): AdapterFactoryInterface @@ -89,12 +96,16 @@ public function setTarget(Target $target): AdapterFactoryInterface public function createSyncRemover(): SyncRemoverInterface { - return new MattermostSyncRemover($this->driver); + return new MattermostSyncRemover($this->driver, new BaseEntriesProvider($this->driver)); } public function supports(string $action): bool { - return false; + return in_array($action, [ + Target::GROUP_PUSH, + Target::SET_PASSWORD, + Target::USER_PUSH, + ]); } protected function getPasswordHelper(?string $salt): PasswordHelper diff --git a/src/SynchronizationAdapter/GroupPush/MattermostGroupPushAdapter.php b/src/SynchronizationAdapter/GroupPush/MattermostGroupPushAdapter.php new file mode 100644 index 0000000..de3d368 --- /dev/null +++ b/src/SynchronizationAdapter/GroupPush/MattermostGroupPushAdapter.php @@ -0,0 +1,108 @@ +driver = $driver; + } + + public function pushGroup(Group $group): GroupPushInterface + { + $existsResponse = $this->driver->getTeamModel()->getTeamByName($group->getName()); + + if ($this->exists($existsResponse)) { + $response = $this->update($this->getTeamId($existsResponse), $group); + } else { + $response = $this->create($group); + } + + if ($response->getStatusCode() >= 400) { + throw new BadMethodCallException('Unable to perform action. Try hard remove teams by CLI. See https://docs.mattermost.com/administration/command-line-tools.html'); + } + + $teamId = $this->getTeamId($response); + $membersIds = []; + foreach ($group->getMembers() as $member) { + $membersIds[] = $this->getMemberId($member); + } + + $this->addMembers($teamId, $membersIds); + + return $this; + } + + private function exists(ResponseInterface $response): bool + { + if ($response->getStatusCode() !== 200) { + return false; + } + + $teamData = json_decode((string) $response->getBody(), true); + + if ($teamData['delete_at'] !== 0) { + return false; + } + + return true; + } + + private function create(Group $group): ResponseInterface + { + return $this->driver->getTeamModel()->createTeam([ + 'name' => $group->getName(), + 'display_name' => $group->getDisplayName(), + 'type' => 'I', + ]); + } + + private function update(string $teamId, Group $group): ResponseInterface + { + return $this->driver->getTeamModel()->updateTeam($teamId, ['id' => $teamId, 'display_name' => $group->getDisplayName()]); + } + + /** + * @param string $teamId + * @param string[] $userIds + */ + private function addMembers(string $teamId, array $userIds): void + { + $entries = []; + foreach ($userIds as $userId) { + $entries[] = [ + 'team_id' => $teamId, + 'user_id' => $userId, + ]; + } + + $this->driver->getTeamModel()->addMultipleUsers($teamId, $entries); + } + + private function getTeamId(ResponseInterface $response): string + { + $teamData = json_decode((string) $response->getBody(), true); + + return $teamData['id'] ?? ''; + } + + private function getMemberId(User $user): string + { + $response = json_decode( + (string) $this->driver->getUserModel()->getUserByUsername($user->getUsername())->getBody(), + true + ); + + return $response['id'] ?? ''; + } +} diff --git a/src/SynchronizationAdapter/SetPassword/MattermostSetPasswordAdapter.php b/src/SynchronizationAdapter/SetPassword/MattermostSetPasswordAdapter.php new file mode 100644 index 0000000..dd55cf8 --- /dev/null +++ b/src/SynchronizationAdapter/SetPassword/MattermostSetPasswordAdapter.php @@ -0,0 +1,49 @@ +driver = $driver; + $this->passwordHelper = $passwordHelper; + $this->responseChecker = $responseChecker; + } + public function setPassword(User $user): SetPasswordInterface + { + $currentPassword = $user->getProperties()[User::PREVIOUS_PASSWORD] + ?? $this->passwordHelper->getDefaultPassword($user->getUsername()); + + $userInfoResponse = json_decode( + (string) $this->driver->getUserModel()->getUserByUsername($user->getUsername())->getBody(), + true + ); + + $response = $this->driver->getUserModel()->updateUserPassword( + $userInfoResponse['id'] ?? '', + [ + 'current_password' => $currentPassword, + 'new_password' => $user->getPassword() + ] + ); + + $this->responseChecker->assertResponse($response); + + return $this; + } +} From c4b862438fccdbe26b4fd746b6acfb4180b5803b Mon Sep 17 00:00:00 2001 From: amsprost Date: Mon, 14 Oct 2019 01:42:00 +0200 Subject: [PATCH 11/13] CARD-2527: Add tests --- .../{Camunda => }/ResponseChecker.php | 0 tests/Unit/DTO/Target/MattermostTest.php | 45 +++++ .../Mattermost/BaseEntriesProviderTest.php | 163 ++++++++++++++++++ .../MattermostAdapterFactoryTest.php | 142 +++++++++++++++ .../GroupPush/CamundaGroupPushAdapterTest.php | 2 +- .../CamundaSetPasswordAdapterTest.php | 2 +- .../UserPush/CamundaUserPushAdapterTest.php | 2 +- 7 files changed, 353 insertions(+), 3 deletions(-) rename src/Services/{Camunda => }/ResponseChecker.php (100%) create mode 100644 tests/Unit/DTO/Target/MattermostTest.php create mode 100644 tests/Unit/Services/Mattermost/BaseEntriesProviderTest.php create mode 100644 tests/Unit/SynchronizationAdapter/AdapterFactory/MattermostAdapterFactoryTest.php diff --git a/src/Services/Camunda/ResponseChecker.php b/src/Services/ResponseChecker.php similarity index 100% rename from src/Services/Camunda/ResponseChecker.php rename to src/Services/ResponseChecker.php diff --git a/tests/Unit/DTO/Target/MattermostTest.php b/tests/Unit/DTO/Target/MattermostTest.php new file mode 100644 index 0000000..2db6577 --- /dev/null +++ b/tests/Unit/DTO/Target/MattermostTest.php @@ -0,0 +1,45 @@ + '', + 'name' => '', + 'token' => '', + 'login' => '', + 'password' => '', + ]; + } + + public function getDtoClassName(): string + { + return Mattermost::class; + } + + public function testScheme() + { + $dto = new Mattermost('http://matterhost.com/test', 'test'); + + $this->assertSame('http', $dto->getScheme()); + $this->assertSame('matterhost.com/test', $dto->getBaseUrl()); + } +} diff --git a/tests/Unit/Services/Mattermost/BaseEntriesProviderTest.php b/tests/Unit/Services/Mattermost/BaseEntriesProviderTest.php new file mode 100644 index 0000000..b42ff01 --- /dev/null +++ b/tests/Unit/Services/Mattermost/BaseEntriesProviderTest.php @@ -0,0 +1,163 @@ +driver = $this->createMock(Driver::class); + $this->provider = new BaseEntriesProvider($this->driver); + } + + /** + * @dataProvider getTestMembersData + */ + public function testGetTeamMembers(array $serverResponse, array $expectedMembers) + { + $teamId = '11df4f4l'; + + $teamModel = $this->createMock(TeamModel::class); + $this->driver->method('getTeamModel')->willReturn($teamModel); + + $teamModel + ->expects($this->exactly(1)) + ->method('getTeamMembers') + ->with($teamId, ['per_page' => 200]) + ->willReturn( + $this->createConfiguredMock(ResponseInterface::class, ['getBody' => json_encode($serverResponse)]) + ); + + $this->assertEquals($expectedMembers, $this->provider->getTeamMembers($teamId)); + } + + /** + * @dataProvider getTestUsersData + */ + public function testGetExistingUsers(array $serverResponse, array $expectedUsers) + { + $userModel = $this->createMock(UserModel::class); + $this->driver->method('getUserModel')->willReturn($userModel); + + $userModel + ->expects($this->exactly(1)) + ->method('getUsers') + ->with(['per_page' => 200]) + ->willReturn( + $this->createConfiguredMock(ResponseInterface::class, ['getBody' => json_encode($serverResponse)]) + ); + + $this->assertEquals($expectedUsers, $this->provider->getExistingUsers()); + } + + /** + * @dataProvider getTestGroupsData + */ + public function testGetExistingGroups(array $serverResponse, array $expectedGroups) + { + $teamModel = $this->createMock(TeamModel::class); + $this->driver->method('getTeamModel')->willReturn($teamModel); + + $teamModel + ->expects($this->exactly(1)) + ->method('getTeams') + ->with(['per_page' => 200]) + ->willReturn( + $this->createConfiguredMock(ResponseInterface::class, ['getBody' => json_encode($serverResponse)]) + ); + + $this->assertEquals($expectedGroups, $this->provider->getExistingGroups()); + } + + public function getTestMembersData(): array + { + return [ + [ + [ + ['user_id' => 'a12s3d', 'delete_at' => 0], + ['user_id' => '5g8jf4', 'delete_at' => 100], + ['user_id' => '5gtyy', 'delete_at' => 0], + ], + ['a12s3d', '5gtyy'], + ], + [ + [ + ['user_id' => 'a12s3d', 'delete_at' => 400], + ['user_id' => '5g8jf4', 'delete_at' => 100], + ['user_id' => '5gtyy', 'delete_at' => 1000], + ], + [], + ], + [ + [], + [], + ], + ]; + } + + public function getTestUsersData(): array + { + return [ + [ + [ + ['username' => 'test11', 'id' => 'a12s3d', 'delete_at' => 0], + ['username' => 'qwe', 'id' => '5g8jf4', 'delete_at' => 100], + ['username' => 'fjjt', 'id' => '5gtyy', 'delete_at' => 0], + ], + [['username' => 'test11', 'id' => 'a12s3d'], ['username' => 'fjjt', 'id' => '5gtyy']], + ], + [ + [ + ['username' => 'test11', 'id' => 'a12s3d', 'delete_at' => 400], + ['username' => 'qwe', 'id' => '5g8jf4', 'delete_at' => 100], + ['username' => 'fjjt', 'id' => '5gtyy', 'delete_at' => 1000], + ], + [], + ], + [ + [], + [], + ], + ]; + } + + public function getTestGroupsData(): array + { + return [ + [ + [ + ['name' => 'test11', 'id' => 'a12s3d', 'delete_at' => 0], + ['name' => 'qwe', 'id' => '5g8jf4', 'delete_at' => 100], + ['name' => 'fjjt', 'id' => '5gtyy', 'delete_at' => 0], + ], + [['name' => 'test11', 'id' => 'a12s3d'], ['name' => 'fjjt', 'id' => '5gtyy']], + ], + [ + [ + ['name' => 'test11', 'id' => 'a12s3d', 'delete_at' => 400], + ['name' => 'qwe', 'id' => '5g8jf4', 'delete_at' => 100], + ['name' => 'fjjt', 'id' => '5gtyy', 'delete_at' => 1000], + ], + [], + ], + [ + [], + [], + ], + ]; + } +} diff --git a/tests/Unit/SynchronizationAdapter/AdapterFactory/MattermostAdapterFactoryTest.php b/tests/Unit/SynchronizationAdapter/AdapterFactory/MattermostAdapterFactoryTest.php new file mode 100644 index 0000000..b559c5d --- /dev/null +++ b/tests/Unit/SynchronizationAdapter/AdapterFactory/MattermostAdapterFactoryTest.php @@ -0,0 +1,142 @@ +driver = $this->createMock(Driver::class); + $this->passwordHelper = $this->createMock(PasswordHelper::class); + + $this->factory = $this->createPartialMock(MattermostAdapterFactory::class, ['getDriver']); + $this->factory->method('getDriver')->willReturn($this->driver); + $this->factory->__construct(null); + $this->factory->setTarget(new Mattermost('http://test', '')); + } + + public function testCreateSetPasswordAdapter() + { + $this->assertInstanceOf(MattermostSetPasswordAdapter::class, $this->factory->createSetPasswordAdapter()); + } + + public function testCreateSyncRemover() + { + $this->assertInstanceOf(MattermostSyncRemover::class, $this->factory->createSyncRemover()); + } + + public function testCreateGroupPushAdapter() + { + $this->assertInstanceOf(MattermostGroupPushAdapter::class, $this->factory->createGroupPushAdapter()); + } + + public function testCreateUserPushAdapter() + { + $this->assertInstanceOf(MattermostUserPushAdapter::class, $this->factory->createUserPushAdapter()); + } + + /** + * @dataProvider getAdapterFactoryData + */ + public function testSetTarget( + string $scheme, + string $baseUri, + ?string $authUsername, + ?string $authPassword, + ?string $authToken + ) { + $salt = 'some test salt'; + + $this->factory = $this->createPartialMock( + MattermostAdapterFactory::class, + ['getDriver', 'getPasswordHelper'] + ); + + $this->factory + ->expects($this->once()) + ->method('getPasswordHelper') + ->with($salt) + ->willReturn($this->passwordHelper); + + $options = ['url' => $baseUri, 'scheme' => $scheme]; + + if ($authToken) { + $options['token'] = $authToken; + } else { + $options['login_id'] = $authUsername; + $options['password'] = $authPassword; + } + + $container = new Container(['driver' => $options]); + + $this->factory + ->expects($this->once()) + ->method('getDriver') + ->with($container) + ->willReturn($this->driver); + + $this->factory->__construct($salt); + $this->factory->setTarget( + new Mattermost($scheme . '://' . $baseUri, '', $authToken, $authUsername, $authPassword) + ); + } + + public function testCreateOrganizationPullAdapter() + { + $this->expectException(BadMethodCallException::class); + + $this->factory->createOrganizationPullAdapter(); + } + + /** + * @dataProvider getSupportsData + */ + public function testSupports(string $action, bool $expected) + { + $this->assertEquals($expected, $this->factory->supports($action)); + } + + public function getAdapterFactoryData(): array + { + return [ + ['http', 'test.com', 'tt11', null, null], + ['http', 'test.com', '4t57u', 'name123', null], + ['http', 'test.com', null, null, '123qwe'], + ['https', 'temp.nl', null, 'user', 'p@ssword'], + ]; + } + + public function getSupportsData(): array + { + return [ + [Target::GROUP_PUSH, true], + [Target::PULL_ORGANIZATION, false], + [Target::USER_PUSH, true], + [Target::SET_PASSWORD, true], + [Target::class, false], + ]; + } +} diff --git a/tests/Unit/SynchronizationAdapter/GroupPush/CamundaGroupPushAdapterTest.php b/tests/Unit/SynchronizationAdapter/GroupPush/CamundaGroupPushAdapterTest.php index 66e3bd5..c2cf08c 100644 --- a/tests/Unit/SynchronizationAdapter/GroupPush/CamundaGroupPushAdapterTest.php +++ b/tests/Unit/SynchronizationAdapter/GroupPush/CamundaGroupPushAdapterTest.php @@ -8,7 +8,7 @@ use LinkORB\OrgSync\DTO\Group; use LinkORB\OrgSync\DTO\User; use LinkORB\OrgSync\Exception\GroupSyncException; -use LinkORB\OrgSync\Services\Camunda\ResponseChecker; +use LinkORB\OrgSync\Services\ResponseChecker; use LinkORB\OrgSync\SynchronizationAdapter\GroupPush\CamundaGroupPushAdapter; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; diff --git a/tests/Unit/SynchronizationAdapter/SetPassword/CamundaSetPasswordAdapterTest.php b/tests/Unit/SynchronizationAdapter/SetPassword/CamundaSetPasswordAdapterTest.php index c1868a1..5c84981 100644 --- a/tests/Unit/SynchronizationAdapter/SetPassword/CamundaSetPasswordAdapterTest.php +++ b/tests/Unit/SynchronizationAdapter/SetPassword/CamundaSetPasswordAdapterTest.php @@ -5,7 +5,7 @@ use GuzzleHttp\Client; use GuzzleHttp\RequestOptions; use LinkORB\OrgSync\DTO\User; -use LinkORB\OrgSync\Services\Camunda\ResponseChecker; +use LinkORB\OrgSync\Services\ResponseChecker; use LinkORB\OrgSync\Services\PasswordHelper; use LinkORB\OrgSync\SynchronizationAdapter\SetPassword\CamundaSetPasswordAdapter; use PHPUnit\Framework\MockObject\MockObject; diff --git a/tests/Unit/SynchronizationAdapter/UserPush/CamundaUserPushAdapterTest.php b/tests/Unit/SynchronizationAdapter/UserPush/CamundaUserPushAdapterTest.php index 0393073..81d4350 100644 --- a/tests/Unit/SynchronizationAdapter/UserPush/CamundaUserPushAdapterTest.php +++ b/tests/Unit/SynchronizationAdapter/UserPush/CamundaUserPushAdapterTest.php @@ -7,7 +7,7 @@ use GuzzleHttp\RequestOptions; use LinkORB\OrgSync\DTO\User; use LinkORB\OrgSync\Exception\UserSyncException; -use LinkORB\OrgSync\Services\Camunda\ResponseChecker; +use LinkORB\OrgSync\Services\ResponseChecker; use LinkORB\OrgSync\Services\PasswordHelper; use LinkORB\OrgSync\SynchronizationAdapter\UserPush\CamundaUserPushAdapter; use PHPUnit\Framework\MockObject\MockObject; From 88b415cca05cdb065babf29fa5a22dc7f0d1f2fc Mon Sep 17 00:00:00 2001 From: amsprost Date: Sat, 19 Oct 2019 14:48:39 +0200 Subject: [PATCH 12/13] Minor improvements --- src/Services/Mattermost/BaseEntriesProvider.php | 4 ++-- src/Services/ResponseChecker.php | 2 +- .../AdapterFactory/CamundaAdapterFactory.php | 2 +- .../AdapterFactory/MattermostAdapterFactory.php | 9 +++++++-- .../GroupPush/CamundaGroupPushAdapter.php | 2 +- .../SetPassword/CamundaSetPasswordAdapter.php | 2 +- .../SetPassword/MattermostSetPasswordAdapter.php | 2 +- .../UserPush/CamundaUserPushAdapter.php | 2 +- .../UserPush/MattermostUserPushAdapter.php | 2 +- 9 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/Services/Mattermost/BaseEntriesProvider.php b/src/Services/Mattermost/BaseEntriesProvider.php index 27795aa..f4b07be 100644 --- a/src/Services/Mattermost/BaseEntriesProvider.php +++ b/src/Services/Mattermost/BaseEntriesProvider.php @@ -65,8 +65,8 @@ private function getBaseEntries(callable $driverFn): array $entries = array_merge($entries, $responseData); } while (count($responseData) === $limit); - return array_filter($entries, function (array $entry) { + return array_values(array_filter($entries, function (array $entry) { return $entry['delete_at'] === 0; - }); + })); } } diff --git a/src/Services/ResponseChecker.php b/src/Services/ResponseChecker.php index ee465f1..38db4ce 100644 --- a/src/Services/ResponseChecker.php +++ b/src/Services/ResponseChecker.php @@ -1,6 +1,6 @@ $driverOpts ]); - $this->driver = new Driver($container); + $this->driver = $this->getDriver($container); $this->driver->authenticate(); $this->passwordHelper = $this->getPasswordHelper($this->defaultPassSalt . $target->getName()); @@ -112,4 +112,9 @@ protected function getPasswordHelper(?string $salt): PasswordHelper { return new PasswordHelper($salt); } + + protected function getDriver(Container $container): Driver + { + return new Driver($container); + } } diff --git a/src/SynchronizationAdapter/GroupPush/CamundaGroupPushAdapter.php b/src/SynchronizationAdapter/GroupPush/CamundaGroupPushAdapter.php index 90d480f..816ee5c 100644 --- a/src/SynchronizationAdapter/GroupPush/CamundaGroupPushAdapter.php +++ b/src/SynchronizationAdapter/GroupPush/CamundaGroupPushAdapter.php @@ -6,7 +6,7 @@ use GuzzleHttp\RequestOptions; use LinkORB\OrgSync\DTO\Group; use LinkORB\OrgSync\DTO\User; -use LinkORB\OrgSync\Services\Camunda\ResponseChecker; +use LinkORB\OrgSync\Services\ResponseChecker; final class CamundaGroupPushAdapter implements GroupPushInterface { diff --git a/src/SynchronizationAdapter/SetPassword/CamundaSetPasswordAdapter.php b/src/SynchronizationAdapter/SetPassword/CamundaSetPasswordAdapter.php index 13746d5..22bf8b9 100644 --- a/src/SynchronizationAdapter/SetPassword/CamundaSetPasswordAdapter.php +++ b/src/SynchronizationAdapter/SetPassword/CamundaSetPasswordAdapter.php @@ -5,7 +5,7 @@ use GuzzleHttp\Client; use GuzzleHttp\RequestOptions; use LinkORB\OrgSync\DTO\User; -use LinkORB\OrgSync\Services\Camunda\ResponseChecker; +use LinkORB\OrgSync\Services\ResponseChecker; use LinkORB\OrgSync\Services\PasswordHelper; final class CamundaSetPasswordAdapter implements SetPasswordInterface diff --git a/src/SynchronizationAdapter/SetPassword/MattermostSetPasswordAdapter.php b/src/SynchronizationAdapter/SetPassword/MattermostSetPasswordAdapter.php index dd55cf8..59148ea 100644 --- a/src/SynchronizationAdapter/SetPassword/MattermostSetPasswordAdapter.php +++ b/src/SynchronizationAdapter/SetPassword/MattermostSetPasswordAdapter.php @@ -4,7 +4,7 @@ use Gnello\Mattermost\Driver; use LinkORB\OrgSync\DTO\User; -use LinkORB\OrgSync\Services\Camunda\ResponseChecker; +use LinkORB\OrgSync\Services\ResponseChecker; use LinkORB\OrgSync\Services\PasswordHelper; class MattermostSetPasswordAdapter implements SetPasswordInterface diff --git a/src/SynchronizationAdapter/UserPush/CamundaUserPushAdapter.php b/src/SynchronizationAdapter/UserPush/CamundaUserPushAdapter.php index f1c5b07..c1c9b29 100644 --- a/src/SynchronizationAdapter/UserPush/CamundaUserPushAdapter.php +++ b/src/SynchronizationAdapter/UserPush/CamundaUserPushAdapter.php @@ -5,7 +5,7 @@ use GuzzleHttp\Client; use GuzzleHttp\RequestOptions; use LinkORB\OrgSync\DTO\User; -use LinkORB\OrgSync\Services\Camunda\ResponseChecker; +use LinkORB\OrgSync\Services\ResponseChecker; use LinkORB\OrgSync\Services\PasswordHelper; final class CamundaUserPushAdapter implements UserPushInterface diff --git a/src/SynchronizationAdapter/UserPush/MattermostUserPushAdapter.php b/src/SynchronizationAdapter/UserPush/MattermostUserPushAdapter.php index 5c2ab8c..dc9d7f8 100644 --- a/src/SynchronizationAdapter/UserPush/MattermostUserPushAdapter.php +++ b/src/SynchronizationAdapter/UserPush/MattermostUserPushAdapter.php @@ -4,7 +4,7 @@ use Gnello\Mattermost\Driver; use LinkORB\OrgSync\DTO\User; -use LinkORB\OrgSync\Services\Camunda\ResponseChecker; +use LinkORB\OrgSync\Services\ResponseChecker; use LinkORB\OrgSync\Services\PasswordHelper; class MattermostUserPushAdapter implements UserPushInterface From b23718b1015ec9a9f417383a03589f81fe31b916 Mon Sep 17 00:00:00 2001 From: amsprost Date: Mon, 21 Oct 2019 00:28:19 +0200 Subject: [PATCH 13/13] CARD-2527: Increased test coverage, minor improvements, updated README --- README.md | 2 +- src/DTO/User.php | 2 + src/Services/Camunda/CamundaUserMapper.php | 4 +- src/Services/Ldap/UserDataMapper.php | 3 +- .../SyncRemover/MattermostSyncRemover.php | 2 - .../GroupPush/MattermostGroupPushAdapter.php | 8 +- .../UserPush/CamundaUserPushAdapter.php | 8 +- .../UserPush/MattermostUserPushAdapter.php | 8 +- .../SyncRemover/MattermostSyncRemoverTest.php | 157 ++++++++++++++++++ .../MattermostGroupPushAdapterTest.php | 147 ++++++++++++++++ .../MattermostSetPasswordAdapterTest.php | 98 +++++++++++ .../MattermostUserPushAdapterTest.php | 123 ++++++++++++++ 12 files changed, 546 insertions(+), 16 deletions(-) create mode 100644 tests/Unit/Services/SyncRemover/MattermostSyncRemoverTest.php create mode 100644 tests/Unit/SynchronizationAdapter/GroupPush/MattermostGroupPushAdapterTest.php create mode 100644 tests/Unit/SynchronizationAdapter/SetPassword/MattermostSetPasswordAdapterTest.php create mode 100644 tests/Unit/SynchronizationAdapter/UserPush/MattermostUserPushAdapterTest.php diff --git a/README.md b/README.md index 9b7682d..25b99d6 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ List of adapters: * [GitHub](https://github.com/) (WIP) * [Camunda](https://docs.camunda.org/manual/7.9/) * [LDAP](https://ldap.com/) -* [MatterMost](https://mattermost.com/) (Planned) +* [MatterMost](https://mattermost.com/) ## Installation diff --git a/src/DTO/User.php b/src/DTO/User.php index fd9fedf..6043ed0 100644 --- a/src/DTO/User.php +++ b/src/DTO/User.php @@ -5,6 +5,8 @@ class User { public const PREVIOUS_PASSWORD = 'previousPassword'; + public const FIRST_NAME = 'firstName'; + public const LAST_NAME = 'lastName'; /** * @var string diff --git a/src/Services/Camunda/CamundaUserMapper.php b/src/Services/Camunda/CamundaUserMapper.php index 1a43622..04f37cc 100644 --- a/src/Services/Camunda/CamundaUserMapper.php +++ b/src/Services/Camunda/CamundaUserMapper.php @@ -15,8 +15,8 @@ public function map(array $data): User null, null, [ - 'firstName' => $data['firstName'] ?? null, - 'lastName' => $data['lastName'] ?? null, + User::FIRST_NAME => $data['firstName'] ?? null, + User::LAST_NAME => $data['lastName'] ?? null, ] ); } diff --git a/src/Services/Ldap/UserDataMapper.php b/src/Services/Ldap/UserDataMapper.php index 610df04..c9ecc2e 100644 --- a/src/Services/Ldap/UserDataMapper.php +++ b/src/Services/Ldap/UserDataMapper.php @@ -11,7 +11,8 @@ public function map(User $user): array $userInfo = [ 'cn' => $user->getUsername(), 'uid' => $user->getUsername(), - 'sn' => $user->getProperties()['lastName'] ?? array_pop(explode(' ', (string) $user->getDisplayName())), + 'sn' => $user->getProperties()[User::LAST_NAME] + ?? array_pop(explode(' ', (string) $user->getDisplayName())), ]; if ($user->getEmail()) { diff --git a/src/Services/SyncRemover/MattermostSyncRemover.php b/src/Services/SyncRemover/MattermostSyncRemover.php index 5536343..4c25137 100644 --- a/src/Services/SyncRemover/MattermostSyncRemover.php +++ b/src/Services/SyncRemover/MattermostSyncRemover.php @@ -68,6 +68,4 @@ public function removeNonExists(Organization $organization): void } } } - - } diff --git a/src/SynchronizationAdapter/GroupPush/MattermostGroupPushAdapter.php b/src/SynchronizationAdapter/GroupPush/MattermostGroupPushAdapter.php index de3d368..762e074 100644 --- a/src/SynchronizationAdapter/GroupPush/MattermostGroupPushAdapter.php +++ b/src/SynchronizationAdapter/GroupPush/MattermostGroupPushAdapter.php @@ -32,6 +32,10 @@ public function pushGroup(Group $group): GroupPushInterface throw new BadMethodCallException('Unable to perform action. Try hard remove teams by CLI. See https://docs.mattermost.com/administration/command-line-tools.html'); } + if (empty($group->getMembers())) { + return $this; + } + $teamId = $this->getTeamId($response); $membersIds = []; foreach ($group->getMembers() as $member) { @@ -93,7 +97,7 @@ private function getTeamId(ResponseInterface $response): string { $teamData = json_decode((string) $response->getBody(), true); - return $teamData['id'] ?? ''; + return $teamData['id']; } private function getMemberId(User $user): string @@ -103,6 +107,6 @@ private function getMemberId(User $user): string true ); - return $response['id'] ?? ''; + return $response['id']; } } diff --git a/src/SynchronizationAdapter/UserPush/CamundaUserPushAdapter.php b/src/SynchronizationAdapter/UserPush/CamundaUserPushAdapter.php index c1c9b29..915e1a0 100644 --- a/src/SynchronizationAdapter/UserPush/CamundaUserPushAdapter.php +++ b/src/SynchronizationAdapter/UserPush/CamundaUserPushAdapter.php @@ -50,8 +50,8 @@ protected function create(User $user): void RequestOptions::JSON => [ 'profile' => [ 'id' => $user->getUsername(), - 'firstName' => $user->getProperties()['firstName'] ?? null, - 'lastName' => $user->getProperties()['lastName'] ?? null, + 'firstName' => $user->getProperties()[User::FIRST_NAME] ?? null, + 'lastName' => $user->getProperties()[User::LAST_NAME] ?? null, 'email' => $user->getEmail(), ], 'credentials' => [ @@ -71,8 +71,8 @@ protected function update(User $user): void [ RequestOptions::JSON => [ 'id' => $user->getUsername(), - 'firstName' => $user->getProperties()['firstName'] ?? null, - 'lastName' => $user->getProperties()['lastName'] ?? null, + 'firstName' => $user->getProperties()[User::FIRST_NAME] ?? null, + 'lastName' => $user->getProperties()[User::LAST_NAME] ?? null, 'email' => $user->getEmail(), ], ] diff --git a/src/SynchronizationAdapter/UserPush/MattermostUserPushAdapter.php b/src/SynchronizationAdapter/UserPush/MattermostUserPushAdapter.php index dc9d7f8..0561307 100644 --- a/src/SynchronizationAdapter/UserPush/MattermostUserPushAdapter.php +++ b/src/SynchronizationAdapter/UserPush/MattermostUserPushAdapter.php @@ -36,10 +36,10 @@ public function pushUser(User $user): UserPushInterface $responseBody = json_decode((string) $response->getBody(), true); if ($responseBody['delete_at'] !== 0) { - $this->driver->getUserModel()->updateUserActive($responseBody['id'] ?? '', ['active' => true]); + $this->driver->getUserModel()->updateUserActive($responseBody['id'], ['active' => true]); } - $response = $this->driver->getUserModel()->patchUser($responseBody['id'] ?? '', $requestOptions); + $response = $this->driver->getUserModel()->patchUser($responseBody['id'], $requestOptions); } else { $requestOptions['password'] = $this->passwordHelper->getDefaultPassword($user->getUsername()); @@ -56,8 +56,8 @@ private function getRequestOptions(User $user): array return [ 'email' => $user->getEmail(), 'username' => $user->getUsername(), - 'first_name' => $user->getProperties()['firstName'] ?? null, - 'last_name' => $user->getProperties()['lastName'] ?? null, + 'first_name' => $user->getProperties()[User::FIRST_NAME] ?? null, + 'last_name' => $user->getProperties()[User::LAST_NAME] ?? null, 'nickname' => $user->getDisplayName(), ]; } diff --git a/tests/Unit/Services/SyncRemover/MattermostSyncRemoverTest.php b/tests/Unit/Services/SyncRemover/MattermostSyncRemoverTest.php new file mode 100644 index 0000000..cfae5a5 --- /dev/null +++ b/tests/Unit/Services/SyncRemover/MattermostSyncRemoverTest.php @@ -0,0 +1,157 @@ +teamModel = $this->createMock(TeamModel::class); + $this->userModel = $this->createMock(UserModel::class); + $this->provider = $this->createMock(BaseEntriesProvider::class); + + $this->remover = new MattermostSyncRemover( + $this->createConfiguredMock( + Driver::class, + ['getTeamModel' => $this->teamModel, 'getUserModel' => $this->userModel] + ), + $this->provider + ); + } + + /** + * @dataProvider getRemoveDataProvider + */ + public function testRemoveNonExists( + array $existingUsers, + array $existingGroups, + array $existingMembers, + Organization $organization, + array $expectedRemoveUserIds, + array $expectedRemoveGroupIds, + array $expectedRemoveMembers + ) + { + $this->provider->expects($this->once())->method('getExistingUsers')->willReturn($existingUsers); + $this->provider->expects($this->once())->method('getExistingGroups')->willReturn($existingGroups); + $this->provider + ->expects($this->exactly(count($existingMembers))) + ->method('getTeamMembers') + ->withConsecutive(...array_map(function (string $key) { + return [$key]; + }, array_keys($existingMembers))) + ->willReturnOnConsecutiveCalls(...array_values($existingMembers)); + + $this->userModel + ->expects($this->exactly(count($expectedRemoveUserIds))) + ->method('deactivateUserAccount') + ->withConsecutive(...array_map(function (string $id) { + return [$id]; + }, $expectedRemoveUserIds)); + + $this->teamModel + ->expects($this->exactly(count($expectedRemoveGroupIds))) + ->method('deleteTeam') + ->withConsecutive(...array_map(function (string $id) { + return [$id]; + }, $expectedRemoveGroupIds)); + + $this->teamModel + ->expects($this->exactly(count($expectedRemoveMembers))) + ->method('removeUser') + ->withConsecutive(...$expectedRemoveMembers); + + $this->assertNull($this->remover->removeNonExists($organization)); + } + + public function getRemoveDataProvider(): array + { + return [ + [ + [ + [ + 'username' => 'test11', + 'id' => 'trhl56', + ], + [ + 'username' => 'user56', + 'id' => 'khkjsf4', + ], + [ + 'username' => 'admin', + 'id' => '1', + ], + ], + [ + [ + 'name' => 'first', + 'id' => '24rb', + ], + [ + 'name' => 'secret', + 'id' => '658jhg', + ], + [ + 'name' => 'qwer', + 'id' => 'gnt6', + ], + [ + 'name' => 'rty', + 'id' => 'thyj8', + ], + [ + 'name' => 'uio', + 'id' => 'hdtr56', + ], + ], + [ + '24rb' => ['trhl56',], + 'gnt6' => [], + 'thyj8' => ['1'], + ], + new Organization( + 'test', + [ + new User('test11'), + new User('user567'), + new User('temp'), + new User('admin') + ], + [ + new Group('first', '', null, null, [new User('test11')]), + new Group('qwerty', '', null, null, [new User('test11'), new User('user567')]), + new Group('secretqwer', '', null, null, [new User('admin'), new User('qw')]), + new Group('qwer', '', null, null, []), + new Group('rty', '', null, null, []), + ] + ), + ['khkjsf4'], + ['658jhg', 'hdtr56'], + [['thyj8', '1', []]] + ] + ]; + } +} diff --git a/tests/Unit/SynchronizationAdapter/GroupPush/MattermostGroupPushAdapterTest.php b/tests/Unit/SynchronizationAdapter/GroupPush/MattermostGroupPushAdapterTest.php new file mode 100644 index 0000000..39d498a --- /dev/null +++ b/tests/Unit/SynchronizationAdapter/GroupPush/MattermostGroupPushAdapterTest.php @@ -0,0 +1,147 @@ +userModel = $this->createMock(UserModel::class); + $this->teamModel = $this->createMock(TeamModel::class); + + $this->adapter = new MattermostGroupPushAdapter( + $this->createConfiguredMock( + Driver::class, ['getTeamModel' => $this->teamModel, 'getUserModel' => $this->userModel] + ) + ); + } + + public function testCreateGroupWithMembers() + { + $group = new Group( + 'grOne', + 'First', + null, + null, + [new User('temp'), new User('test1'), new User('user')] + ); + $id = '1444'; + + $this->teamModel + ->expects($this->once()) + ->method('getTeamByName') + ->with($group->getName()) + ->willReturn( + $this->createConfiguredMock( + ResponseInterface::class, + ['getStatusCode' => 200, 'getBody' => json_encode(['delete_at' => 1])] + ) + ); + + $this->teamModel + ->expects($this->once()) + ->method('createTeam') + ->with([ + 'name' => $group->getName(), + 'display_name' => $group->getDisplayName(), + 'type' => 'I', + ]) + ->willReturn( + $this->createConfiguredMock( + ResponseInterface::class, + ['getStatusCode' => 200, 'getBody' => json_encode(['id' => $id])] + ) + ); + + $this->userModel + ->expects($this->exactly(count($group->getMembers()))) + ->method('getUserByUsername') + ->withConsecutive(...array_map(function (User $user) { + return [$user->getUsername()]; + }, $group->getMembers())) + ->willReturnCallback(function (string $username) { + return $this->createConfiguredMock( + ResponseInterface::class, + ['getBody' => json_encode(['id' => $username . '_user'])] + ); + }); + + $this->teamModel + ->expects($this->once()) + ->method('addMultipleUsers') + ->with($id, array_map(function (User $user) use ($id) { + return [ + 'team_id' => $id, + 'user_id' => $user->getUsername() . '_user', + ]; + }, $group->getMembers())); + + $this->assertSame($this->adapter, $this->adapter->pushGroup($group)); + } + + public function testUpdateGroup() + { + $group = new Group('grOne', 'First'); + $id = '1001'; + + $this->teamModel + ->method('getTeamByName') + ->with($group->getName()) + ->willReturn( + $this->createConfiguredMock( + ResponseInterface::class, + ['getStatusCode' => 200, 'getBody' => json_encode(['delete_at' => 0, 'id' => $id])] + ) + ); + + $this->teamModel + ->expects($this->once()) + ->method('updateTeam') + ->with( + $id, + ['id' => $id, 'display_name' => $group->getDisplayName()] + ) + ->willReturn($this->createConfiguredMock(ResponseInterface::class, ['getStatusCode' => 200])); + + $this->assertSame($this->adapter, $this->adapter->pushGroup($group)); + } + + public function testPushGroupException() + { + $this->teamModel->method('getTeamByName') + ->willReturn( + $this->createConfiguredMock( + ResponseInterface::class, + ['getStatusCode' => 400] + ) + ); + + $this->teamModel + ->method('createTeam') + ->willReturn($this->createConfiguredMock(ResponseInterface::class, ['getStatusCode' => 400])); + + $this->expectException(BadMethodCallException::class); + + $this->adapter->pushGroup(new Group('', '')); + } +} diff --git a/tests/Unit/SynchronizationAdapter/SetPassword/MattermostSetPasswordAdapterTest.php b/tests/Unit/SynchronizationAdapter/SetPassword/MattermostSetPasswordAdapterTest.php new file mode 100644 index 0000000..a74450d --- /dev/null +++ b/tests/Unit/SynchronizationAdapter/SetPassword/MattermostSetPasswordAdapterTest.php @@ -0,0 +1,98 @@ +helper = $this->createMock(PasswordHelper::class); + $this->driver = $this->createMock(Driver::class); + + $this->adapter = new MattermostSetPasswordAdapter( + $this->driver, + $this->helper, + new ResponseChecker(User::class) + ); + } + + public function testSetPasswordWithoutPrevious() + { + $user = new User('testUser', 'newSecurePass'); + + $defaultPassword = 'fbifhuoj;i'; + $this->helper + ->expects($this->once()) + ->method('getDefaultPassword') + ->with($user->getUsername()) + ->willReturn($defaultPassword); + + $userModel = $this->createMock(UserModel::class); + $this->driver->method('getUserModel')->willReturn($userModel); + + $userId = 123; + + $userModel + ->expects($this->once()) + ->method('getUserByUsername') + ->with($user->getUsername()) + ->willReturn( + $this->createConfiguredMock(ResponseInterface::class, ['getBody' => json_encode(['id' => $userId])]) + ); + + $userModel + ->expects($this->once()) + ->method('updateUserPassword') + ->with($userId, ['current_password' => $defaultPassword, 'new_password' => $user->getPassword()]) + ->willReturn($this->createConfiguredMock(ResponseInterface::class, ['getStatusCode' => 200])); + + $this->assertSame($this->adapter, $this->adapter->setPassword($user)); + } + + public function testSetPasswordWithPrevious() + { + $user = new User('firstOne', 'p@sssss', null, null, null, [User::PREVIOUS_PASSWORD => 'prevP@ss']); + + $this->helper->expects($this->never())->method('getDefaultPassword'); + + $userModel = $this->createMock(UserModel::class); + $this->driver->method('getUserModel')->willReturn($userModel); + + $userModel->method('getUserByUsername')->willReturn( + $this->createConfiguredMock(ResponseInterface::class, ['getBody' => json_encode(['id' => 132])]) + ); + + $userModel + ->expects($this->once()) + ->method('updateUserPassword') + ->with( + $this->anything(), + [ + 'current_password' => $user->getProperties()[User::PREVIOUS_PASSWORD], + 'new_password' => $user->getPassword() + ] + ) + ->willReturn($this->createConfiguredMock(ResponseInterface::class, ['getStatusCode' => 200])); + + $this->assertSame($this->adapter, $this->adapter->setPassword($user)); + } +} diff --git a/tests/Unit/SynchronizationAdapter/UserPush/MattermostUserPushAdapterTest.php b/tests/Unit/SynchronizationAdapter/UserPush/MattermostUserPushAdapterTest.php new file mode 100644 index 0000000..dcfb6ec --- /dev/null +++ b/tests/Unit/SynchronizationAdapter/UserPush/MattermostUserPushAdapterTest.php @@ -0,0 +1,123 @@ +model = $this->createMock(UserModel::class); + $this->helper = $this->createMock(PasswordHelper::class); + + $this->adapter = new MattermostUserPushAdapter( + new ResponseChecker(User::class, [404]), + $this->createConfiguredMock(Driver::class, ['getUserModel' => $this->model]), + $this->helper + ); + } + + public function testCreateUser() + { + $email = 'a@a.com'; + $username = 'ttest'; + $firstName = 'User'; + $lastName = 'Test'; + $nickname = 'tuser'; + $defaultPass = 'p@ssword'; + + $user = new User( + $username, + null, + $email, + $nickname, + null, + [User::FIRST_NAME => $firstName, User::LAST_NAME => $lastName] + ); + + $this->model + ->expects($this->once()) + ->method('getUserByUsername') + ->with($user->getUsername()) + ->willReturn($this->createConfiguredMock(ResponseInterface::class, ['getStatusCode' => 404])); + + $this->helper->expects($this->once())->method('getDefaultPassword')->with($username)->willReturn($defaultPass); + + $this->model + ->expects($this->once()) + ->method('createUser') + ->with([ + 'email' => $email, + 'username' => $username, + 'first_name' => $firstName, + 'last_name' => $lastName, + 'nickname' => $nickname, + 'password' => $defaultPass, + ]) + ->willReturn($this->createConfiguredMock(ResponseInterface::class, ['getStatusCode' => 200])); + + $this->assertSame($this->adapter, $this->adapter->pushUser($user)); + } + + public function testUpdateUser() + { + $user = new User('test', null); + $id = 111; + + $this->model + ->method('getUserByUsername') + ->with($user->getUsername()) + ->willReturn($this->createConfiguredMock( + ResponseInterface::class, + ['getStatusCode' => 200, 'getBody' => json_encode(['delete_at' => 0, 'id' => $id])] + )); + + $this->helper->expects($this->never())->method('getDefaultPassword'); + + $this->model + ->expects($this->once()) + ->method('patchUser') + ->with($id, $this->anything()) + ->willReturn($this->createConfiguredMock(ResponseInterface::class, ['getStatusCode' => 200])); + + $this->assertSame($this->adapter, $this->adapter->pushUser($user)); + } + + public function testUpdateDeletedUser() + { + $user = new User('test', null); + $id = 2317; + + $this->model + ->method('getUserByUsername') + ->willReturn($this->createConfiguredMock( + ResponseInterface::class, + ['getStatusCode' => 200, 'getBody' => json_encode(['delete_at' => 1, 'id' => $id])] + )); + + $this->model->expects($this->once())->method('updateUserActive')->with($id, ['active' => true]); + $this->model + ->method('patchUser') + ->willReturn($this->createConfiguredMock(ResponseInterface::class, ['getStatusCode' => 200])); + + $this->assertSame($this->adapter, $this->adapter->pushUser($user)); + } +}