From 37d80682825e3cb4e272d1e6a46dd2f30f561599 Mon Sep 17 00:00:00 2001 From: Cristian Scheid Date: Fri, 24 Jul 2026 11:09:59 -0300 Subject: [PATCH 1/2] fix(local-controller): validate when trying adding member of type contact Signed-off-by: Cristian Scheid --- lib/Controller/LocalController.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/Controller/LocalController.php b/lib/Controller/LocalController.php index 4cce953ff..ae16afce7 100644 --- a/lib/Controller/LocalController.php +++ b/lib/Controller/LocalController.php @@ -226,7 +226,7 @@ public function memberAdd(string $circleId, string $userId, int $type): DataResp if (!$this->configService->isLocalInstance($currentUser->getInstance())) { throw new OCSException('works only from local instance', 404); } - + // prefix with current user id to scope contact lookup to this user $userId = $currentUser->getUserId() . '/' . $userId; } @@ -263,6 +263,15 @@ public function membersAdd(string $circleId, array $members): DataResponse { $userId = $this->get('id', $member); $type = $this->getInt('type', $member); + if ($type === Member::TYPE_CONTACT) { + $currentUser = $this->federatedUserService->getCurrentUser(); + if (!$this->configService->isLocalInstance($currentUser->getInstance())) { + throw new OCSException('works only from local instance', 404); + } + // prefix with current user id to scope contact lookup to this user + $userId = $currentUser->getUserId() . '/' . $userId; + } + if ($type === Member::TYPE_CIRCLE) { $this->circleService->getCircle($userId); } From 489993b6e075b29db809fb24084cda877cd101d9 Mon Sep 17 00:00:00 2001 From: Louis Chmn Date: Fri, 31 Jul 2026 15:53:09 +0200 Subject: [PATCH 2/2] test: Add coverage for memberAdd and membersAdd methods Signed-off-by: Louis Chmn --- .../lib/Controller/LocalControllerTest.php | 263 +++++++++++++----- 1 file changed, 188 insertions(+), 75 deletions(-) diff --git a/tests/unit/lib/Controller/LocalControllerTest.php b/tests/unit/lib/Controller/LocalControllerTest.php index 188e93f13..aa210e8fb 100644 --- a/tests/unit/lib/Controller/LocalControllerTest.php +++ b/tests/unit/lib/Controller/LocalControllerTest.php @@ -1,4 +1,5 @@ getContainer()->get(IUserManager::class); + + foreach ([self::TEST_USER_1, self::TEST_USER_2, self::TEST_USER_3, self::TEST_USER_4] as $userId) { + $user = $userManager->get($userId); + if ($user === null) { + $userManager->createUser($userId, 'test-pwd'); + self::$usersToCleanup[] = $userId; + } + } + } - /** @var IUserSession|MockObject */ - private $userSession; + public function setUp(): void { + parent::setUp(); - /** @var FederatedUserService|MockObject */ - private $federatedUserService; + $this->app = new Application(); + $this->container = $this->app->getContainer(); + $this->userManager = $this->container->get(IUserManager::class); + $this->userSession = $this->container->get(IUserSession::class); - /** @var CircleService|MockObject */ - private $circleService; + $user1 = $this->userManager->get(self::TEST_USER_1); + $this->userSession->setUser($user1); - /** @var MemberService|MockObject */ - private $memberService; + $this->localController = new LocalController( + Application::APP_ID, + $this->container->get(IRequest::class), + $this->userSession, + $this->container->get(FederatedUserService::class), + $this->container->get(CircleService::class), + $this->container->get(MemberService::class), + $this->container->get(MembershipService::class), + $this->container->get(PermissionService::class), + $this->container->get(SearchService::class), + $this->container->get(ConfigService::class), + ); + + // Create a circle as TEST_USER_1 (owner) + $circleResult = $this->localController->create('test-circle')->getData(); + $this->circlesToCleanup[] = $circleResult['id']; + $this->circleId = $circleResult['id']; + } - /** @var MembershipService|MockObject */ - private $membershipService; + public static function tearDownAfterClass(): void { + parent::tearDownAfterClass(); - /** @var SearchService|MockObject */ - private $searchService; + $app = new Application(); + $userManager = $app->getContainer()->get(IUserManager::class); - /** @var PermissionService|MockObject */ - private $permissionService; + foreach (self::$usersToCleanup as $userId) { + $userManager->get($userId)?->delete(); + } + } - /** @var ConfigService|MockObject */ - private $configService; + protected function tearDown(): void { + parent::tearDown(); - /** @var LocalController */ - private $localController; + $circleService = $this->container->get(CircleService::class); - public function setUp(): void { - parent::setUp(); - $this->request = $this->createMock(IRequest::class); - $this->userSession = $this->createMock(IUserSession::class); - $this->federatedUserService = $this->createMock(FederatedUserService::class); - $this->circleService = $this->createMock(CircleService::class); - $this->memberService = $this->createMock(MemberService::class); - $this->membershipService = $this->createMock(MembershipService::class); - $this->searchService = $this->createMock(SearchService::class); - $this->permissionService = $this->createMock(PermissionService::class); - $this->configService = $this->createMock(ConfigService::class); - $this->configService->expects($this->any())->method('getAppValueBool')->with(ConfigService::FRONTEND_ENABLED)->willReturn(true); - $this->localController = new LocalController(Application::APP_ID, - $this->request, - $this->userSession, - $this->federatedUserService, - $this->circleService, - $this->memberService, - $this->membershipService, - $this->permissionService, - $this->searchService, - $this->configService); + foreach ($this->circlesToCleanup as $circleId) { + try { + $circleService->destroy($circleId); + } catch (\Throwable) { + // continue cleanup + } + } } /** * @dataProvider dataForCirclesList - * - * @param int $limit - * @param int $offset - * @return void - * @throws OCSException */ - public function testCirclesList(int $limit, int $offset): void { - $probe = new CircleProbe(); - $probe->filterHiddenCircles() - ->filterBackendCircles() - ->addDetail(BasicProbe::DETAILS_POPULATION) - ->setItemsOffset($offset) - ->setItemsLimit($limit); - $circle1 = new Circle(); - $circle1->setName('Circle One'); - $circle1->setSingleId('CircleOne'); - $circle2 = new Circle(); - $circle2->setName('Circle Two'); - $circle2->setSingleId('CircleTwo'); - $circle3 = new Circle(); - $circle3->setName('Circle Three'); - $circle3->setSingleId('CircleThree'); - $circles = [$circle1, $circle2, $circle3]; - $selectedCircles = array_slice($circles, $offset, $limit > 0 ? $limit : null); - $this->circleService->expects($this->once())->method('getCircles')->with($probe)->willReturn($selectedCircles); - $response = new DataResponse($this->serializeArray($selectedCircles)); - $this->assertEquals($response, $this->localController->circles($limit, $offset)); + public function testCirclesList(int $limit, int $offset, int $expectedCount): void { + $result1 = $this->localController->create('test-circle1')->getData(); + $result2 = $this->localController->create('test-circle2')->getData(); + $this->circlesToCleanup[] = $result1['id']; + $this->circlesToCleanup[] = $result2['id']; + + $data = $this->localController->circles($limit, $offset)->getData(); + + $this->assertEquals(count($data), $expectedCount); } public static function dataForCirclesList(): array { return [ - [-1, 0], - [1, 1] + [-1, 0, 3], + [1, 1, 1], + [-1, 1, 2], + ]; + } + + public function testMemberAdd(): void { + // Add TEST_USER_2 as a member + $memberResult = $this->localController->memberAdd($this->circleId, self::TEST_USER_2, Member::TYPE_USER)->getData(); + + // Verify the member was added + $this->assertNotNull($memberResult); + $this->assertEquals(self::TEST_USER_2, $memberResult['userId']); + $this->assertEquals($this->circleId, $memberResult['circleId']); + $this->assertEquals(Member::TYPE_USER, $memberResult['userType']); + } + + public function testMemberAddPermissionDeniedForNonMember(): void { + // Switch to TEST_USER_2 (who is not a member) + $this->userSession->setUser($this->userManager->get(self::TEST_USER_2)); + + $response = $this->localController->memberAdd($this->circleId, self::TEST_USER_3, Member::TYPE_USER); + $this->assertEquals($response->getData()['message'], 'Insufficient permissions to perform this action'); + } + + public function testMemberAddPermissionDeniedForRegularMember(): void { + // Add TEST_USER_2 as a regular member + $this->localController->memberAdd($this->circleId, self::TEST_USER_2, Member::TYPE_USER); + + // Switch to TEST_USER_2 + $this->userSession->setUser($this->userManager->get(self::TEST_USER_2)); + + // TEST_USER_2 (regular member) tries to add another user + // This should fail because they're not a moderator + $response = $this->localController->memberAdd($this->circleId, self::TEST_USER_3, Member::TYPE_USER); + $this->assertEquals($response->getData()['message'], 'Insufficient permissions to perform this action'); + } + + public function testMemberAddAllowedForRegularMemberInFriendCircle(): void { + // Make it a friend circle (CFG_FRIEND = 128) + $this->localController->editConfig($this->circleId, Circle::CFG_FRIEND); + + // Add TEST_USER_2 as a regular member + $this->localController->memberAdd($this->circleId, self::TEST_USER_2, Member::TYPE_USER); + + // Switch to TEST_USER_2 + $this->userSession->setUser($this->userManager->get(self::TEST_USER_2)); + + // TEST_USER_2 (regular member) tries to add TEST_USER_3 + $result = $this->localController->memberAdd($this->circleId, self::TEST_USER_3, Member::TYPE_USER)->getData(); + + // Verify the member was added + $this->assertNotNull($result); + $this->assertEquals($result['userId'], self::TEST_USER_3); + $this->assertEquals($result['circleId'], $this->circleId); + } + + public function testMemberAddAllowedForModerator(): void { + // Add TEST_USER_2 as a member + $memberResult = $this->localController->memberAdd($this->circleId, self::TEST_USER_2, Member::TYPE_USER)->getData(); + + // Promote TEST_USER_2 to moderator + $this->localController->memberLevel($this->circleId, $memberResult['id'], Member::LEVEL_MODERATOR); + + // Switch to TEST_USER_2 + $this->userSession->setUser($this->userManager->get(self::TEST_USER_2)); + + // TEST_USER_2 (moderator) tries to add TEST_USER_3 + $result = $this->localController->memberAdd($this->circleId, self::TEST_USER_3, Member::TYPE_USER)->getData(); + + // Verify the member was added + $this->assertNotNull($result); + $this->assertEquals(self::TEST_USER_3, $result['userId']); + $this->assertEquals($this->circleId, $result['circleId']); + } + + public function testMembersAddMultipleUsers(): void { + // Add multiple members at once + $members = [ + ['id' => self::TEST_USER_2, 'type' => Member::TYPE_USER], + ['id' => self::TEST_USER_3, 'type' => Member::TYPE_USER], ]; + + $result = $this->localController->membersAdd($this->circleId, $members)->getData(); + + // Verify both members were added + $this->assertCount(2, $result); + $this->assertEquals($result[0]['userId'], self::TEST_USER_2); + $this->assertEquals($result[1]['userId'], self::TEST_USER_3); + } + + public function testMembersAddPermissionDenied(): void { + // Switch to TEST_USER_2 (who is not a member) + $this->userSession->setUser($this->userManager->get(self::TEST_USER_2)); + + // Attempt to add multiple members without permission + $members = [ + ['id' => self::TEST_USER_3, 'type' => Member::TYPE_USER], + ['id' => self::TEST_USER_4, 'type' => Member::TYPE_USER], + ]; + + $response = $this->localController->membersAdd($this->circleId, $members); + $this->assertEquals($response->getData()['message'], 'Insufficient permissions to perform this action'); } }