From 17570716e699b5908d6a69ff0eb5243e51c82f7d Mon Sep 17 00:00:00 2001 From: Hendrik Leidinger Date: Mon, 29 Jun 2026 11:42:29 -0700 Subject: [PATCH 1/2] fix: remove disabled users from lookup Signed-off-by: Hendrik Leidinger Assisted-by: Claude Code:Opus 4.8 --- lib/AppInfo/Application.php | 3 ++ lib/Listeners/UserChanged.php | 49 ++++++++++++++++++++++ lib/Slave.php | 28 ++++++++++--- tests/unit/lib/SlaveTest.php | 79 +++++++++++++++++++++++++++++++++++ 4 files changed, 154 insertions(+), 5 deletions(-) create mode 100644 lib/Listeners/UserChanged.php create mode 100644 tests/unit/lib/SlaveTest.php diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 8f79ce33..d1d3cc21 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -15,6 +15,7 @@ use OCA\GlobalSiteSelector\GlobalSiteSelector; use OCA\GlobalSiteSelector\Listeners\AddContentSecurityPolicyListener; use OCA\GlobalSiteSelector\Listeners\DeletingUser; +use OCA\GlobalSiteSelector\Listeners\UserChanged; use OCA\GlobalSiteSelector\Listeners\UserCreated; use OCA\GlobalSiteSelector\Listeners\UserDeleted; use OCA\GlobalSiteSelector\Listeners\UserLoggedOut; @@ -36,6 +37,7 @@ use OCP\Server; use OCP\User\Events\BeforeUserDeletedEvent; use OCP\User\Events\BeforeUserLoggedInEvent; +use OCP\User\Events\UserChangedEvent; use OCP\User\Events\UserCreatedEvent; use OCP\User\Events\UserDeletedEvent; use OCP\User\Events\UserLoggedOutEvent; @@ -76,6 +78,7 @@ public function register(IRegistrationContext $context): void { ); // events on slave + $context->registerEventListener(UserChangedEvent::class, UserChanged::class); $context->registerEventListener(UserCreatedEvent::class, UserCreated::class); $context->registerEventListener(BeforeUserDeletedEvent::class, DeletingUser::class); $context->registerEventListener(UserDeletedEvent::class, UserDeleted::class); diff --git a/lib/Listeners/UserChanged.php b/lib/Listeners/UserChanged.php new file mode 100644 index 00000000..b9276672 --- /dev/null +++ b/lib/Listeners/UserChanged.php @@ -0,0 +1,49 @@ + + */ +class UserChanged implements IEventListener { + + public function __construct( + private GlobalSiteSelector $globalSiteSelector, + private Slave $slave, + ) { + } + + /** + * @param Event $event + */ + public function handle(Event $event): void { + if (!$event instanceof UserChangedEvent) { + return; + } + + /** only used in slave mode */ + if (!$this->globalSiteSelector->isSlave()) { + return; + } + + if ($event->getFeature() !== 'enabled') { + return; + } + + // updateUser() routes enabled→add, disabled→remove + $this->slave->updateUser($event->getUser()); + } +} \ No newline at end of file diff --git a/lib/Slave.php b/lib/Slave.php index fe764ae7..1b64f42d 100644 --- a/lib/Slave.php +++ b/lib/Slave.php @@ -99,6 +99,11 @@ public function updateUser(IUser $user): void { ] ); + if (!$user->isEnabled()) { + $this->removeUsers([$user->getCloudId()]); + return; + } + $userData = []; $userData[$user->getCloudId()] = $this->slaveService->getAccountData($user); $this->addUsers($userData); @@ -158,18 +163,31 @@ public function batchUpdate(): void { foreach ($backends as $backend) { $limit = 200; $offset = 0; - $usersData = []; do { - $users = $backend->getUsers('', $limit, $offset); + + $usersToAdd = []; + $usersToRemove = []; foreach ($users as $uid) { $user = $this->userManager->get($uid); - if ($user !== null) { - $usersData[$user->getCloudId()] = $this->slaveService->getAccountData($user); + if ($user === null) { + continue; } + if ($user->isEnabled()) { + $usersToAdd[$user->getCloudId()] = $this->slaveService->getAccountData($user); + } else { + $usersToRemove[] = $user->getCloudId(); + } + } + + if ($usersToAdd !== []) { + $this->addUsers($usersToAdd); } + if ($usersToRemove !== []) { + $this->removeUsers($usersToRemove); // one DELETE per page, not per user + } + $offset += $limit; - $this->addUsers($usersData); } while (count($users) >= $limit); } } diff --git a/tests/unit/lib/SlaveTest.php b/tests/unit/lib/SlaveTest.php new file mode 100644 index 00000000..ed97919f --- /dev/null +++ b/tests/unit/lib/SlaveTest.php @@ -0,0 +1,79 @@ +createMock(IClientService::class); + $this->client = $this->createMock(IClient::class); + $clientService->method('newClient')->willReturn($this->client); + + $this->slaveService = $this->createMock(SlaveService::class); + $lookup = $this->createMock(Lookup::class); + $lookup->method('configureClient')->willReturnArgument(0); // pass options through + + $gss = $this->createMock(GlobalSiteSelector::class); + $gss->method('isSlave')->willReturn(true); // checkConfiguration() passes + $gss->method('getLookupServerUrl')->willReturn('https://lookup.test'); + $gss->method('getMode')->willReturn('slave'); + $gss->method('getJwtKey')->willReturn('secret'); + + $this->slave = new Slave( + $this->createMock(IUserManager::class), + $clientService, + $this->slaveService, + $lookup, + $gss, + $this->createMock(LoggerInterface::class), + $this->createMock(IConfig::class), + ); + } + + public function testEnabledUserIsPushed(): void { + $user = $this->createMock(IUser::class); + $user->method('isEnabled')->willReturn(true); + $user->method('getCloudId')->willReturn('alice@slave.test'); + $this->slaveService->method('getAccountData')->willReturn(['id' => 'alice@slave.test']); + + $this->client->expects($this->once())->method('post'); + $this->client->expects($this->never())->method('delete'); + + $this->slave->updateUser($user); + } + + public function testDisabledUserIsRemoved(): void { + $user = $this->createMock(IUser::class); + $user->method('isEnabled')->willReturn(false); + $user->method('getCloudId')->willReturn('bob@slave.test'); + + $this->client->expects($this->never())->method('post'); + $this->client->expects($this->once())->method('delete')->with( + $this->anything(), + $this->callback(function (array $opts): bool { + $body = json_decode($opts['body'] ?? '{}', true); + return in_array('bob@slave.test', $body['users'] ?? [], true); + }) + ); + + $this->slave->updateUser($user); + } +} \ No newline at end of file From 0eb199577a6275674a35efd914d6d7e45c363984 Mon Sep 17 00:00:00 2001 From: Hendrik Leidinger Date: Mon, 29 Jun 2026 11:52:43 -0700 Subject: [PATCH 2/2] fix: compliance check and php-cs Signed-off-by: Hendrik Leidinger --- lib/AppInfo/Application.php | 2 +- lib/Listeners/UserChanged.php | 2 +- tests/unit/lib/SlaveTest.php | 128 ++++++++++++++++++---------------- 3 files changed, 69 insertions(+), 63 deletions(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index d1d3cc21..0575985c 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -78,7 +78,7 @@ public function register(IRegistrationContext $context): void { ); // events on slave - $context->registerEventListener(UserChangedEvent::class, UserChanged::class); + $context->registerEventListener(UserChangedEvent::class, UserChanged::class); $context->registerEventListener(UserCreatedEvent::class, UserCreated::class); $context->registerEventListener(BeforeUserDeletedEvent::class, DeletingUser::class); $context->registerEventListener(UserDeletedEvent::class, UserDeleted::class); diff --git a/lib/Listeners/UserChanged.php b/lib/Listeners/UserChanged.php index b9276672..10b2f791 100644 --- a/lib/Listeners/UserChanged.php +++ b/lib/Listeners/UserChanged.php @@ -46,4 +46,4 @@ public function handle(Event $event): void { // updateUser() routes enabled→add, disabled→remove $this->slave->updateUser($event->getUser()); } -} \ No newline at end of file +} diff --git a/tests/unit/lib/SlaveTest.php b/tests/unit/lib/SlaveTest.php index ed97919f..9b1548e7 100644 --- a/tests/unit/lib/SlaveTest.php +++ b/tests/unit/lib/SlaveTest.php @@ -1,6 +1,12 @@ createMock(IClientService::class); - $this->client = $this->createMock(IClient::class); - $clientService->method('newClient')->willReturn($this->client); - - $this->slaveService = $this->createMock(SlaveService::class); - $lookup = $this->createMock(Lookup::class); - $lookup->method('configureClient')->willReturnArgument(0); // pass options through - - $gss = $this->createMock(GlobalSiteSelector::class); - $gss->method('isSlave')->willReturn(true); // checkConfiguration() passes - $gss->method('getLookupServerUrl')->willReturn('https://lookup.test'); - $gss->method('getMode')->willReturn('slave'); - $gss->method('getJwtKey')->willReturn('secret'); - - $this->slave = new Slave( - $this->createMock(IUserManager::class), - $clientService, - $this->slaveService, - $lookup, - $gss, - $this->createMock(LoggerInterface::class), - $this->createMock(IConfig::class), - ); - } - - public function testEnabledUserIsPushed(): void { - $user = $this->createMock(IUser::class); - $user->method('isEnabled')->willReturn(true); - $user->method('getCloudId')->willReturn('alice@slave.test'); - $this->slaveService->method('getAccountData')->willReturn(['id' => 'alice@slave.test']); - - $this->client->expects($this->once())->method('post'); - $this->client->expects($this->never())->method('delete'); - - $this->slave->updateUser($user); - } - - public function testDisabledUserIsRemoved(): void { - $user = $this->createMock(IUser::class); - $user->method('isEnabled')->willReturn(false); - $user->method('getCloudId')->willReturn('bob@slave.test'); - - $this->client->expects($this->never())->method('post'); - $this->client->expects($this->once())->method('delete')->with( - $this->anything(), - $this->callback(function (array $opts): bool { - $body = json_decode($opts['body'] ?? '{}', true); - return in_array('bob@slave.test', $body['users'] ?? [], true); - }) - ); - - $this->slave->updateUser($user); - } -} \ No newline at end of file + private $client; + private $slaveService; + private Slave $slave; + + protected function setUp(): void { + parent::setUp(); + + $clientService = $this->createMock(IClientService::class); + $this->client = $this->createMock(IClient::class); + $clientService->method('newClient')->willReturn($this->client); + + $this->slaveService = $this->createMock(SlaveService::class); + $lookup = $this->createMock(Lookup::class); + $lookup->method('configureClient')->willReturnArgument(0); // pass options through + + $gss = $this->createMock(GlobalSiteSelector::class); + $gss->method('isSlave')->willReturn(true); // checkConfiguration() passes + $gss->method('getLookupServerUrl')->willReturn('https://lookup.test'); + $gss->method('getMode')->willReturn('slave'); + $gss->method('getJwtKey')->willReturn('secret'); + + $this->slave = new Slave( + $this->createMock(IUserManager::class), + $clientService, + $this->slaveService, + $lookup, + $gss, + $this->createMock(LoggerInterface::class), + $this->createMock(IConfig::class), + ); + } + + public function testEnabledUserIsPushed(): void { + $user = $this->createMock(IUser::class); + $user->method('isEnabled')->willReturn(true); + $user->method('getCloudId')->willReturn('alice@slave.test'); + $this->slaveService->method('getAccountData')->willReturn(['id' => 'alice@slave.test']); + + $this->client->expects($this->once())->method('post'); + $this->client->expects($this->never())->method('delete'); + + $this->slave->updateUser($user); + } + + public function testDisabledUserIsRemoved(): void { + $user = $this->createMock(IUser::class); + $user->method('isEnabled')->willReturn(false); + $user->method('getCloudId')->willReturn('bob@slave.test'); + + $this->client->expects($this->never())->method('post'); + $this->client->expects($this->once())->method('delete')->with( + $this->anything(), + $this->callback(function (array $opts): bool { + $body = json_decode($opts['body'] ?? '{}', true); + return in_array('bob@slave.test', $body['users'] ?? [], true); + }) + ); + + $this->slave->updateUser($user); + } +}