From 48ad2b9265485fbdec75d9ec0d00ed80803a15b5 Mon Sep 17 00:00:00 2001 From: Josua Hunziker Date: Fri, 20 Mar 2026 07:54:10 +0100 Subject: [PATCH 01/13] Update compatible versions --- .github/workflows/tests.yml | 11 +++++------ appinfo/info.xml | 4 ++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d7f95bd..1f12061 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -7,7 +7,7 @@ on: - master - stable* schedule: - - cron: '0 2 * * *' + - cron: "0 2 * * *" env: APP_NAME: auto_groups @@ -22,13 +22,12 @@ jobs: strategy: fail-fast: false matrix: - php-versions: ['8.2', '8.3'] - databases: ['sqlite'] - server-versions: - ['stable30', 'stable31', 'stable32'] + php-versions: ["8.2", "8.3"] + databases: ["sqlite"] + server-versions: ["stable31", "stable32", "stable33"] experimental: [false] include: - - php-versions: '8.4' + - php-versions: "8.4" databases: sqlite server-versions: master experimental: true diff --git a/appinfo/info.xml b/appinfo/info.xml index f55a5e6..85045e6 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -24,7 +24,7 @@ Note that this app prevents group deletions for groups referenced as Auto Groups In addition, I plan to add some more features over time, e.g., "Union Groups" - see the [Milestone Plans](https://github.com/stjosh/auto_groups/milestones) for more details. - 1.6.2 + 1.7.0 agpl Josua Hunziker AutoGroups @@ -34,7 +34,7 @@ In addition, I plan to add some more features over time, e.g., "Union Groups" - https://github.com/stjosh/auto_groups.git https://raw.githubusercontent.com/stjosh/auto_groups/master/screenshots/settings.png - + OCA\AutoGroups\Settings\Admin From 7437756a0f43e74a188b1ba648f1cf96de58da4a Mon Sep 17 00:00:00 2001 From: Josua Hunziker Date: Fri, 20 Mar 2026 08:45:21 +0100 Subject: [PATCH 02/13] fix: migrate to IBootstrap event registration for NC34 compatibility NC34 dropped support for callable-based addListener() in IEventDispatcher. Switch to IBootstrap/registerEventListener() with a dedicated AutoGroupsListener class. Hook config checks (creation/modification/login) move from AutoGroupsManager constructor to AutoGroupsListener::handle(). Also fixes AdminSettingsTest to use DI for IManager instead of the removed OC\Server::getSettingsManager(). Co-Authored-By: Claude Sonnet 4.6 --- CLAUDE.md | 62 +++++++++++ lib/AppInfo/Application.php | 42 +++++-- lib/AutoGroupsManager.php | 52 ++------- lib/Listener/AutoGroupsListener.php | 65 +++++++++++ tests/Integration/AdminSettingsTest.php | 2 +- tests/Unit/AutoGroupsManagerTest.php | 142 +++++------------------- 6 files changed, 194 insertions(+), 171 deletions(-) create mode 100644 CLAUDE.md create mode 100644 lib/Listener/AutoGroupsListener.php diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..a68890d --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,62 @@ +# Auto Groups - Nextcloud App + +## Overview + +A Nextcloud app (v1.7.0, AGPL-3.0) that automatically adds users to configured groups ("Auto Groups"), with optional exemptions for users in "Override Groups". A modernized fork of the abandoned [defaultgroup](https://github.com/bodangren/defaultgroup) app. + +- **Nextcloud compatibility**: 31–34 +- **PHP**: 8.2, 8.3 +- **App ID**: `auto_groups` (note: older config used `AutoGroups` — migration logic exists) + +## Architecture + +Single-class app with minimal footprint: + +- `lib/AutoGroupsManager.php` — core logic; registers event listeners and handles group assignment/deletion +- `lib/AppInfo/Application.php` — bootstraps the app via Nextcloud's DI container +- `lib/Settings/Admin.php` — admin settings page +- `appinfo/routes.php` — routes +- `templates/admin.php` + `css/admin.css` + `js/admin.js` — admin UI + +## Key Behavior + +**Event hooks** (configurable): +- `creation_hook` (default: on) — fires on `UserCreatedEvent` and `UserFirstTimeLoggedInEvent` +- `modification_hook` (default: on) — fires on `UserAddedEvent` and `UserRemovedEvent` +- `login_hook` (default: off) — fires on `PostLoginEvent` and `UserLoggedInEvent`; useful for external user backends + +**Group assignment logic** (`addAndRemoveAutoGroups`): +- If user belongs to any Override Group → remove from all Auto Groups +- If user belongs to no Override Group → add to all Auto Groups + +**Group deletion protection**: throws `OCSBadRequestException` if trying to delete a group referenced as an Auto Group or Override Group. + +## Config Keys + +Stored via Nextcloud's `IConfig` under app `auto_groups`: +- `auto_groups` — JSON array of group IDs +- `override_groups` — JSON array of group IDs +- `creation_hook` — `'true'`/`'false'` +- `modification_hook` — `'true'`/`'false'` +- `login_hook` — `'true'`/`'false'` + +## Testing + +- Unit tests: `tests/Unit/` (uses PHPUnit mocks, extends Nextcloud's `Test\TestCase`) +- Integration tests: `tests/Integration/` +- Manual testing: `tests/Docker/run-docker-test-instance.sh` spins up a Docker instance on port 8080 +- Lint: `composer run lint` (runs `php -l` on all PHP files) + +## Release Process + +Releases are handled by `.github/workflows/release.yml` on GitHub release publication: +1. Verifies `appinfo/info.xml` version matches the git tag +2. Verifies `CHANGELOG.md` has an entry for the version +3. Packages and uploads to GitHub Releases +4. Submits to Nextcloud App Store (requires `AUTO_GROUPS_SIGNING_KEY` and `APP_STORE_API_TOKEN` secrets) + +## Noteworthy + +- **Config namespace migration**: The app previously used `AutoGroups` as the config namespace instead of `auto_groups`. Migration code in `AutoGroupsManager::__construct` handles upgrading old configs (see GitHub issue #82). +- **l10n**: Translations managed via Transifex (`.tx/config`); many languages supported. +- No Composer dependencies beyond dev tooling — the app relies entirely on Nextcloud's built-in OCP APIs. diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 438af37..d1a7fce 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -3,7 +3,7 @@ * @copyright Copyright (c) 2020 * * @author Josua Hunziker - * + * * Based on the work of Ján Stibila * * @license AGPL-3.0 @@ -25,17 +25,41 @@ namespace OCA\AutoGroups\AppInfo; use OCP\AppFramework\App; +use OCP\AppFramework\Bootstrap\IBootContext; +use OCP\AppFramework\Bootstrap\IBootstrap; +use OCP\AppFramework\Bootstrap\IRegistrationContext; +use OCP\User\Events\UserCreatedEvent; +use OCP\User\Events\UserFirstTimeLoggedInEvent; +use OCP\User\Events\PostLoginEvent; +use OCP\User\Events\UserLoggedInEvent; +use OCP\Group\Events\UserAddedEvent; +use OCP\Group\Events\UserRemovedEvent; +use OCP\Group\Events\BeforeGroupDeletedEvent; + use OCA\AutoGroups\AutoGroupsManager; +use OCA\AutoGroups\Listener\AutoGroupsListener; -class Application extends App { +class Application extends App implements IBootstrap +{ + public function __construct() + { + parent::__construct('auto_groups'); + } - private $autoGroupsManager; + public function register(IRegistrationContext $context): void + { + $context->registerEventListener(UserCreatedEvent::class, AutoGroupsListener::class); + $context->registerEventListener(UserFirstTimeLoggedInEvent::class, AutoGroupsListener::class); + $context->registerEventListener(UserAddedEvent::class, AutoGroupsListener::class); + $context->registerEventListener(UserRemovedEvent::class, AutoGroupsListener::class); + $context->registerEventListener(PostLoginEvent::class, AutoGroupsListener::class); + $context->registerEventListener(UserLoggedInEvent::class, AutoGroupsListener::class); + $context->registerEventListener(BeforeGroupDeletedEvent::class, AutoGroupsListener::class); + } - /** - * Application constructor. - */ - public function __construct() { - parent::__construct('auto_groups'); - $this->autoGroupsManager = $this->getContainer()->query(AutoGroupsManager::class); + public function boot(IBootContext $context): void + { + // Instantiate AutoGroupsManager to trigger legacy config migration + $context->getAppContainer()->query(AutoGroupsManager::class); } } diff --git a/lib/AutoGroupsManager.php b/lib/AutoGroupsManager.php index b9d4f2a..7697de7 100644 --- a/lib/AutoGroupsManager.php +++ b/lib/AutoGroupsManager.php @@ -4,7 +4,7 @@ * @copyright Copyright (c) 2020 * * @author Josua Hunziker - * + * * Based on the work of Ján Stibila * * @license AGPL-3.0 @@ -25,16 +25,7 @@ namespace OCA\AutoGroups; -use OCP\User\Events\UserCreatedEvent; -use OCP\User\Events\UserFirstTimeLoggedInEvent; -use OCP\User\Events\PostLoginEvent; -use OCP\User\Events\UserLoggedInEvent; -use OCP\Group\Events\UserAddedEvent; -use OCP\Group\Events\UserRemovedEvent; -use OCP\Group\Events\BeforeGroupDeletedEvent; - use OCP\IGroupManager; -use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\IL10N; @@ -50,9 +41,9 @@ class AutoGroupsManager private $l; /** - * Listener manager constructor. + * AutoGroupsManager constructor. */ - public function __construct(IGroupManager $groupManager, IEventDispatcher $eventDispatcher, IConfig $config, LoggerInterface $logger, IL10N $l) + public function __construct(IGroupManager $groupManager, IConfig $config, LoggerInterface $logger, IL10N $l) { $this->groupManager = $groupManager; $this->logger = $logger; @@ -96,39 +87,10 @@ public function __construct(IGroupManager $groupManager, IEventDispatcher $event $this->config->deleteAppValue("AutoGroups", "override_groups"); } } - - // The callback as a PHP callable - $groupAssignmentCallback = [$this, 'addAndRemoveAutoGroups']; - - // Get the hook configs - $creationHook = $this->config->getAppValue("auto_groups", "creation_hook", 'true'); - $modificationHook = $this->config->getAppValue("auto_groups", "modification_hook", 'true'); - $loginHook = $this->config->getAppValue("auto_groups", "login_hook", 'false'); - - // If creation hook is enabled, add user to / remove user from auto groups on creation - if (filter_var($creationHook, FILTER_VALIDATE_BOOLEAN)) { - $eventDispatcher->addListener(UserCreatedEvent::class, $groupAssignmentCallback); - $eventDispatcher->addListener(UserFirstTimeLoggedInEvent::class, $groupAssignmentCallback); - } - - // If modification hook is enabled, add user to / remove user from auto groups on every modification of user groups - if (filter_var($modificationHook, FILTER_VALIDATE_BOOLEAN)) { - $eventDispatcher->addListener(UserAddedEvent::class, $groupAssignmentCallback); - $eventDispatcher->addListener(UserRemovedEvent::class, $groupAssignmentCallback); - } - - // If login hook is enabled, add user to / remove user from auto groups on every successful login - if (filter_var($loginHook, FILTER_VALIDATE_BOOLEAN)) { - $eventDispatcher->addListener(PostLoginEvent::class, $groupAssignmentCallback); - $eventDispatcher->addListener(UserLoggedInEvent::class, $groupAssignmentCallback); - } - - // Handle group deletion events - $eventDispatcher->addListener(BeforeGroupDeletedEvent::class, [$this, 'handleGroupDeletion']); } /** - * The event handler to check group assignmnet for a user + * The event handler to check group assignment for a user */ public function addAndRemoveAutoGroups($event) { @@ -166,9 +128,9 @@ public function addAndRemoveAutoGroups($event) /** * The event handler to handle group deletions - * + * * @throws OCSBadRequestException - * + * */ public function handleGroupDeletion($event) { @@ -178,7 +140,7 @@ public function handleGroupDeletion($event) $allGroupNames = array_merge($groupNames, $overrideGroupNames); - // Get group name of group do delete + // Get group name of group to delete $groupNameToDelete = $event->getGroup()->getGID(); // Prevent deletion if group to delete is configured in AutoGroups diff --git a/lib/Listener/AutoGroupsListener.php b/lib/Listener/AutoGroupsListener.php new file mode 100644 index 0000000..364e9b6 --- /dev/null +++ b/lib/Listener/AutoGroupsListener.php @@ -0,0 +1,65 @@ + + * + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OCA\AutoGroups\Listener; + +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; +use OCP\User\Events\UserCreatedEvent; +use OCP\User\Events\UserFirstTimeLoggedInEvent; +use OCP\User\Events\PostLoginEvent; +use OCP\User\Events\UserLoggedInEvent; +use OCP\Group\Events\UserAddedEvent; +use OCP\Group\Events\UserRemovedEvent; +use OCP\Group\Events\BeforeGroupDeletedEvent; +use OCP\IConfig; + +use OCA\AutoGroups\AutoGroupsManager; + +/** @template-implements IEventListener */ +class AutoGroupsListener implements IEventListener +{ + public function __construct( + private AutoGroupsManager $manager, + private IConfig $config + ) {} + + public function handle(Event $event): void + { + if ($event instanceof UserCreatedEvent || $event instanceof UserFirstTimeLoggedInEvent) { + if (filter_var($this->config->getAppValue('auto_groups', 'creation_hook', 'true'), FILTER_VALIDATE_BOOLEAN)) { + $this->manager->addAndRemoveAutoGroups($event); + } + } elseif ($event instanceof UserAddedEvent || $event instanceof UserRemovedEvent) { + if (filter_var($this->config->getAppValue('auto_groups', 'modification_hook', 'true'), FILTER_VALIDATE_BOOLEAN)) { + $this->manager->addAndRemoveAutoGroups($event); + } + } elseif ($event instanceof PostLoginEvent || $event instanceof UserLoggedInEvent) { + if (filter_var($this->config->getAppValue('auto_groups', 'login_hook', 'false'), FILTER_VALIDATE_BOOLEAN)) { + $this->manager->addAndRemoveAutoGroups($event); + } + } elseif ($event instanceof BeforeGroupDeletedEvent) { + $this->manager->handleGroupDeletion($event); + } + } +} diff --git a/tests/Integration/AdminSettingsTest.php b/tests/Integration/AdminSettingsTest.php index a095ef1..5c1e806 100644 --- a/tests/Integration/AdminSettingsTest.php +++ b/tests/Integration/AdminSettingsTest.php @@ -49,7 +49,7 @@ protected function setUp(): void $this->app = new Application(); $this->container = $this->app->getContainer(); - $this->settingsManager = $this->container->getServer()->getSettingsManager(); + $this->settingsManager = $this->container->query(IManager::class); } public function testAppSettingsExist() diff --git a/tests/Unit/AutoGroupsManagerTest.php b/tests/Unit/AutoGroupsManagerTest.php index a10ee47..0d3b392 100644 --- a/tests/Unit/AutoGroupsManagerTest.php +++ b/tests/Unit/AutoGroupsManagerTest.php @@ -24,16 +24,10 @@ namespace OCA\AutoGroups\Tests\Unit; -use OCP\User\Events\UserCreatedEvent; -use OCP\User\Events\UserFirstTimeLoggedInEvent; -use OCP\User\Events\UserLoggedInEvent; -use OCP\User\Events\PostLoginEvent; -use OCP\Group\Events\UserAddedEvent; -use OCP\Group\Events\UserRemovedEvent; use OCP\Group\Events\BeforeGroupDeletedEvent; +use OCP\User\Events\UserCreatedEvent; use OCP\IGroupManager; -use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\IL10N; @@ -52,7 +46,6 @@ class AutoGroupsManagerTest extends TestCase { private $groupManager; - private $eventDispatcher; private $config; private $logger; private $il10n; @@ -62,7 +55,6 @@ protected function setUp(): void parent::setUp(); $this->groupManager = $this->createMock(IGroupManager::class); - $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->config = $this->createMock(IConfig::class); $this->logger = $this->createMock(LoggerInterface::class); $this->il10n = $this->createMock(IL10N::class); @@ -73,58 +65,34 @@ protected function setUp(): void ->willReturn('Test User'); } - private function createAutoGroupsManager( - $auto_groups = [], - $override_groups = [], - $creation_hook = 'true', - $modification_hook = 'true', - $login_hook = 'false', - $expectedNumberOfConfigCalls = 7 - ) { - $this->config->expects($this->exactly($expectedNumberOfConfigCalls)) - ->method('getAppValue') - ->withConsecutive( - ['AutoGroups', 'creation_only'], - ['AutoGroups', 'creation_hook'], - ['auto_groups', 'creation_hook', 'true'], - ['auto_groups', 'modification_hook', 'true'], - ['auto_groups', 'login_hook', 'false'], - ['auto_groups', 'auto_groups', '[]'], - ['auto_groups', 'override_groups', '[]'] - ) - ->willReturnOnConsecutiveCalls('', '', $creation_hook, $modification_hook, $login_hook, json_encode($auto_groups), json_encode($override_groups)); - - return new AutoGroupsManager($this->groupManager, $this->eventDispatcher, $this->config, $this->logger, $this->il10n); - } - - private function initEventHandlerTests($auto_groups = [], $override_groups = []) + private function createAutoGroupsManager($auto_groups = [], $override_groups = []) { - $this->eventDispatcher->expects($this->exactly(5)) - ->method('addListener') - ->withConsecutive( - [UserCreatedEvent::class, $this->callback('is_callable')], - [UserFirstTimeLoggedInEvent::class, $this->callback('is_callable')], - [UserAddedEvent::class, $this->callback('is_callable')], - [UserRemovedEvent::class, $this->callback('is_callable')], - [BeforeGroupDeletedEvent::class, $this->callback('is_callable')] - ); - - $agm = $this->createAutoGroupsManager($auto_groups, $override_groups); - return $agm; + $this->config->method('getAppValue') + ->willReturnCallback(function ($app, $key, $default = '') use ($auto_groups, $override_groups) { + if ($app === 'AutoGroups') { + return ''; // no migration needed + } + if ($app === 'auto_groups' && $key === 'auto_groups') { + return json_encode($auto_groups); + } + if ($app === 'auto_groups' && $key === 'override_groups') { + return json_encode($override_groups); + } + return $default; + }); + + return new AutoGroupsManager($this->groupManager, $this->config, $this->logger, $this->il10n); } private function configMigrationTestImpl($creationOnly, $expectedModification) { - $this->config->expects($this->exactly(5)) + $this->config->expects($this->exactly(2)) ->method('getAppValue') ->withConsecutive( ['AutoGroups', 'creation_only'], ['AutoGroups', 'creation_hook'], - ['auto_groups', 'creation_hook', 'true'], - ['auto_groups', 'modification_hook', 'true'], - ['auto_groups', 'login_hook', 'false'], ) - ->willReturnOnConsecutiveCalls($creationOnly, '', 'true', 'true', 'false'); + ->willReturnOnConsecutiveCalls($creationOnly, ''); $this->config->expects($this->exactly(1)) ->method('setAppValue') @@ -134,65 +102,7 @@ private function configMigrationTestImpl($creationOnly, $expectedModification) ->method('deleteAppValue') ->with('AutoGroups', 'creation_only'); - return new AutoGroupsManager($this->groupManager, $this->eventDispatcher, $this->config, $this->logger, $this->il10n); - } - - public function testCreatedAddedRemovedHooksWithDefaultSettings() - { - $this->eventDispatcher->expects($this->exactly(5)) - ->method('addListener') - ->withConsecutive( - [UserCreatedEvent::class, $this->callback('is_callable')], - [UserFirstTimeLoggedInEvent::class, $this->callback('is_callable')], - [UserAddedEvent::class, $this->callback('is_callable')], - [UserRemovedEvent::class, $this->callback('is_callable')], - [BeforeGroupDeletedEvent::class, $this->callback('is_callable')] - ); - - $agm = $this->createAutoGroupsManager([], [], 'true', 'true', 'false', 5); - } - - public function testAlsoLoginHookIfEnabled() - { - $this->eventDispatcher->expects($this->exactly(7)) - ->method('addListener') - ->withConsecutive( - [UserCreatedEvent::class, $this->callback('is_callable')], - [UserFirstTimeLoggedInEvent::class, $this->callback('is_callable')], - [UserAddedEvent::class, $this->callback('is_callable')], - [UserRemovedEvent::class, $this->callback('is_callable')], - [PostLoginEvent::class, $this->callback('is_callable')], - [UserLoggedInEvent::class, $this->callback('is_callable')], - [BeforeGroupDeletedEvent::class, $this->callback('is_callable')] - ); - - $agm = $this->createAutoGroupsManager([], [], 'true', 'true', 'true', 5); - } - - public function testCreationOnlyMode() - { - $this->eventDispatcher->expects($this->exactly(3)) - ->method('addListener') - ->withConsecutive( - [UserCreatedEvent::class, $this->callback('is_callable')], - [UserFirstTimeLoggedInEvent::class, $this->callback('is_callable')], - [BeforeGroupDeletedEvent::class, $this->callback('is_callable')] - ); - - $agm = $this->createAutoGroupsManager([], [], 'true', 'false', 'false', 5); - } - - public function testModificationOnlyMode() - { - $this->eventDispatcher->expects($this->exactly(3)) - ->method('addListener') - ->withConsecutive( - [UserAddedEvent::class, $this->callback('is_callable')], - [UserRemovedEvent::class, $this->callback('is_callable')], - [BeforeGroupDeletedEvent::class, $this->callback('is_callable')] - ); - - $agm = $this->createAutoGroupsManager([], [], 'false', 'true', 'false', 5); + return new AutoGroupsManager($this->groupManager, $this->config, $this->logger, $this->il10n); } public function testAddingToAutoGroups() @@ -217,7 +127,7 @@ public function testAddingToAutoGroups() ->with('autogroup') ->willReturn([$autogroup]); - $agm = $this->initEventHandlerTests(['autogroup']); + $agm = $this->createAutoGroupsManager(['autogroup']); $agm->addAndRemoveAutoGroups($event); } @@ -243,7 +153,7 @@ public function testAddingNotRequired() ->with('autogroup') ->willReturn([$autogroup]); - $agm = $this->initEventHandlerTests(['autogroup']); + $agm = $this->createAutoGroupsManager(['autogroup']); $agm->addAndRemoveAutoGroups($event); } @@ -269,7 +179,7 @@ public function testRemoveUserFromAutoGroups() ->withConsecutive(['autogroup1'], ['autogroup2']) ->willReturnOnConsecutiveCalls([$groupMock], [$groupMock]); - $agm = $this->initEventHandlerTests(['autogroup1', 'autogroup2'], ['overridegroup1', 'overridegroup2']); + $agm = $this->createAutoGroupsManager(['autogroup1', 'autogroup2'], ['overridegroup1', 'overridegroup2']); $agm->addAndRemoveAutoGroups($event); } @@ -295,7 +205,7 @@ public function testRemoveNotRequired() ->withConsecutive(['autogroup1'], ['autogroup2']) ->willReturnOnConsecutiveCalls([$groupMock], [$groupMock]); - $agm = $this->initEventHandlerTests(['autogroup1', 'autogroup2'], ['overridegroup1', 'overridegroup2']); + $agm = $this->createAutoGroupsManager(['autogroup1', 'autogroup2'], ['overridegroup1', 'overridegroup2']); $agm->addAndRemoveAutoGroups($event); } @@ -313,7 +223,7 @@ public function testGroupDeletionPrevented() $this->expectException(OCSBadRequestException::class); - $agm = $this->initEventHandlerTests(['autogroup1', 'autogroup2'], ['overridegroup1', 'overridegroup2']); + $agm = $this->createAutoGroupsManager(['autogroup1', 'autogroup2'], ['overridegroup1', 'overridegroup2']); $agm->handleGroupDeletion($event); } @@ -329,7 +239,7 @@ public function testGroupDeletionPreventionNotNeeded() ->method('getGroup') ->willReturn($groupMock); - $agm = $this->initEventHandlerTests(['autogroup1', 'autogroup2'], ['overridegroup1', 'overridegroup2']); + $agm = $this->createAutoGroupsManager(['autogroup1', 'autogroup2'], ['overridegroup1', 'overridegroup2']); $agm->handleGroupDeletion($event); } From d6c551a4b6f49ebd8c4d3b041d7ffefa07694bf1 Mon Sep 17 00:00:00 2001 From: Josua Hunziker Date: Fri, 20 Mar 2026 08:47:58 +0100 Subject: [PATCH 03/13] fix(test): migrate PHPUnit XML config to current schema Replace deprecated and with and . Co-Authored-By: Claude Sonnet 4.6 --- tests/phpunit.xml | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/tests/phpunit.xml b/tests/phpunit.xml index b332821..31e10d4 100644 --- a/tests/phpunit.xml +++ b/tests/phpunit.xml @@ -7,16 +7,12 @@ . - - - ../ - - ../tests - - - - - - - - \ No newline at end of file + + + ../lib + + + + + + From 6f00e41ef5d02d348579fc9e0ea87536c66b4a77 Mon Sep 17 00:00:00 2001 From: Josua Hunziker Date: Fri, 20 Mar 2026 08:51:41 +0100 Subject: [PATCH 04/13] fix(test): restore testLoginHook and make it self-contained The test was broken under the old addListener() architecture because login_hook config was read at Application instantiation time (before the test set it), so the login listener was never registered. With the new AutoGroupsListener approach, hook config is checked at event dispatch time, so this now works correctly. Also removes the dependency on testRemoveHook having run first by explicitly setting up the initial group state in the test itself. Co-Authored-By: Claude Sonnet 4.6 --- tests/Integration/EventsTest.php | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/Integration/EventsTest.php b/tests/Integration/EventsTest.php index 71b94c8..ad165bb 100644 --- a/tests/Integration/EventsTest.php +++ b/tests/Integration/EventsTest.php @@ -124,26 +124,36 @@ public function testRemoveHook() $this->assertTrue($autogroup1->inGroup($testUser) && $autogroup2->inGroup($testUser)); } - /*public function testLoginHook() + public function testLoginHook() { $this->config->setAppValue("auto_groups", "auto_groups", '["autogroup1", "autogroup2"]'); $this->config->setAppValue("auto_groups", "override_groups", '["overridegroup1"]'); $this->config->setAppValue("auto_groups", "login_hook", 'true'); $this->config->setAppValue("auto_groups", "creation_hook", 'false'); $this->config->setAppValue("auto_groups", "modification_hook", 'false'); - + $testUser = $this->userManager->get('testuser'); $overridegroup = $this->groupManager->search('overridegroup1')[0]; $autogroup1 = $this->groupManager->search('autogroup1')[0]; $autogroup2 = $this->groupManager->search('autogroup2')[0]; + // Set up initial state explicitly: user in auto groups, not in override group. + // modification_hook=false so these direct adds won't trigger auto groups logic. + $overridegroup->removeUser($testUser); + $autogroup1->addUser($testUser); + $autogroup2->addUser($testUser); + $this->assertTrue($autogroup1->inGroup($testUser) && $autogroup2->inGroup($testUser)); - + + // Adding to override group should NOT remove from auto groups (modification_hook=false) $overridegroup->addUser($testUser); + $this->assertTrue($autogroup1->inGroup($testUser) && $autogroup2->inGroup($testUser)); + + // Login SHOULD trigger removal from auto groups (login_hook=true, user in override group) $this->userSession->login('testuser', 'testPassword'); $this->assertTrue(!$autogroup1->inGroup($testUser) && !$autogroup2->inGroup($testUser)); - }*/ + } public function testBeforeGroupDeletionHook() From a465e0cd30f04ebef6fd11536ce11e2acac7f7a8 Mon Sep 17 00:00:00 2001 From: Josua Hunziker Date: Fri, 20 Mar 2026 08:54:58 +0100 Subject: [PATCH 05/13] style(test): add inline comments to all test files Co-Authored-By: Claude Sonnet 4.6 --- tests/Integration/AdminSettingsTest.php | 2 ++ tests/Integration/EventsTest.php | 4 ++++ tests/Unit/AdminSettingsTest.php | 3 +++ tests/Unit/AutoGroupsManagerTest.php | 8 ++++++++ 4 files changed, 17 insertions(+) diff --git a/tests/Integration/AdminSettingsTest.php b/tests/Integration/AdminSettingsTest.php index 5c1e806..1930ca0 100644 --- a/tests/Integration/AdminSettingsTest.php +++ b/tests/Integration/AdminSettingsTest.php @@ -56,6 +56,7 @@ public function testAppSettingsExist() { $settings = $this->settingsManager->getAdminSettings('additional'); + // The app must register its Admin settings class at priority 100 in the 'additional' section $this->assertArrayHasKey(100, $settings); $this->assertIsArray($settings[100]); $adminSettings = $settings[100][0]; @@ -66,6 +67,7 @@ public function testFormRender() { $appSettings = $this->settingsManager->getAdminSettings('additional')[100][0]; + // getForm() must return a TemplateResponse (the actual template rendering is not tested here) $templateResponse = $appSettings->getForm(); $this->assertInstanceOf(TemplateResponse::class, $templateResponse); diff --git a/tests/Integration/EventsTest.php b/tests/Integration/EventsTest.php index ad165bb..171e903 100644 --- a/tests/Integration/EventsTest.php +++ b/tests/Integration/EventsTest.php @@ -82,6 +82,7 @@ public function testCreateHook() $this->config->setAppValue("auto_groups", "creation_hook", 'true'); $this->config->setAppValue("auto_groups", "modification_hook", 'true'); + // Creating a user should immediately add them to auto groups (creation_hook=true) $this->userManager->createUser('testuser', 'testPassword'); $testUser = $this->userManager->get('testuser'); @@ -101,6 +102,7 @@ public function testAddHook() $overridegroup = $this->groupManager->search('overridegroup1')[0]; $autogroup = $this->groupManager->search('autogroup1')[0]; + // Adding user to an override group should remove them from auto groups (modification_hook=true) $overridegroup->addUser($testUser); $this->assertNotTrue($autogroup->inGroup($testUser)); @@ -119,6 +121,7 @@ public function testRemoveHook() $autogroup1 = $this->groupManager->search('autogroup1')[0]; $autogroup2 = $this->groupManager->search('autogroup2')[0]; + // Removing user from the override group should re-add them to all auto groups (modification_hook=true) $overridegroup->removeUser($testUser); $this->assertTrue($autogroup1->inGroup($testUser) && $autogroup2->inGroup($testUser)); @@ -166,6 +169,7 @@ public function testBeforeGroupDeletionHook() $autogroup = $this->groupManager->search('autogroup1')[0]; + // Deleting a group that is configured as an auto group should be prevented $this->expectException(OCSBadRequestException::class); $autogroup->delete(); } diff --git a/tests/Unit/AdminSettingsTest.php b/tests/Unit/AdminSettingsTest.php index de39a28..8d971f7 100644 --- a/tests/Unit/AdminSettingsTest.php +++ b/tests/Unit/AdminSettingsTest.php @@ -71,16 +71,19 @@ protected function setUp(): void public function testSection() { + // Settings must appear in the 'additional' admin section $this->assertEquals('additional', $this->adminSettings->getSection()); } public function testPriority() { + // Priority 100 places the form towards the bottom of the section $this->assertEquals(100, $this->adminSettings->getPriority()); } public function testForm() { + // getForm() must read all five config values and pass them to the template as parameters $this->config->expects($this->exactly(5)) ->method('getAppValue') ->withConsecutive( diff --git a/tests/Unit/AutoGroupsManagerTest.php b/tests/Unit/AutoGroupsManagerTest.php index 0d3b392..98c5e27 100644 --- a/tests/Unit/AutoGroupsManagerTest.php +++ b/tests/Unit/AutoGroupsManagerTest.php @@ -112,6 +112,7 @@ public function testAddingToAutoGroups() ->method('getUser') ->willReturn($this->testUser); + // User belongs to no groups, so they should be added to the auto group $this->groupManager->expects($this->once()) ->method('getUserGroups') ->with($this->testUser) @@ -138,6 +139,7 @@ public function testAddingNotRequired() ->method('getUser') ->willReturn($this->testUser); + // User is already in the auto group, so addUser should never be called $this->groupManager->expects($this->once()) ->method('getUserGroups') ->with($this->testUser) @@ -164,6 +166,7 @@ public function testRemoveUserFromAutoGroups() ->method('getUser') ->willReturn($this->testUser); + // User belongs to an override group, so they should be removed from all auto groups $this->groupManager->expects($this->once()) ->method('getUserGroups') ->with($this->testUser) @@ -190,6 +193,7 @@ public function testRemoveNotRequired() ->method('getUser') ->willReturn($this->testUser); + // User is in an override group but not in any auto group, so removeUser should never be called $this->groupManager->expects($this->once()) ->method('getUserGroups') ->with($this->testUser) @@ -221,6 +225,7 @@ public function testGroupDeletionPrevented() ->method('getGroup') ->willReturn($groupMock); + // autogroup2 is configured as an auto group, so deletion must be prevented $this->expectException(OCSBadRequestException::class); $agm = $this->createAutoGroupsManager(['autogroup1', 'autogroup2'], ['overridegroup1', 'overridegroup2']); @@ -239,17 +244,20 @@ public function testGroupDeletionPreventionNotNeeded() ->method('getGroup') ->willReturn($groupMock); + // 'some other group' is not referenced in config, so deletion should be allowed $agm = $this->createAutoGroupsManager(['autogroup1', 'autogroup2'], ['overridegroup1', 'overridegroup2']); $agm->handleGroupDeletion($event); } public function testConfigMigrationForCreationOnlyTrue() { + // Legacy creation_only=true means modification_hook should be migrated to false $agm = $this->configMigrationTestImpl('true', 'false'); } public function testConfigMigrationForCreationOnlyFalse() { + // Legacy creation_only=false means modification_hook should be migrated to true $agm = $this->configMigrationTestImpl('false', 'true'); } } From 7875aa756542437cafc544aac4dc84eacdf097d8 Mon Sep 17 00:00:00 2001 From: Josua Hunziker Date: Fri, 20 Mar 2026 08:57:37 +0100 Subject: [PATCH 06/13] chore: update author email and add missing AGPL notices Replace der@digitalwerker.ch with josh@o23.ch in all files. Add AGPL-3.0 copyright header to appinfo/routes.php, css/admin.css, and standardize the existing shortened notice in js/admin.js. Co-Authored-By: Claude Sonnet 4.6 --- appinfo/info.xml | 2 +- appinfo/routes.php | 23 ++++++++++++++++++----- css/admin.css | 21 +++++++++++++++++++++ js/admin.js | 19 ++++++++++++++----- lib/AppInfo/Application.php | 2 +- lib/AutoGroupsManager.php | 2 +- lib/Listener/AutoGroupsListener.php | 2 +- lib/Settings/Admin.php | 2 +- templates/admin.php | 2 +- tests/Integration/AdminSettingsTest.php | 2 +- tests/Integration/EventsTest.php | 2 +- tests/Unit/AdminSettingsTest.php | 2 +- tests/Unit/AutoGroupsManagerTest.php | 2 +- 13 files changed, 63 insertions(+), 20 deletions(-) diff --git a/appinfo/info.xml b/appinfo/info.xml index 85045e6..ca96aac 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -26,7 +26,7 @@ In addition, I plan to add some more features over time, e.g., "Union Groups" - 1.7.0 agpl - Josua Hunziker + Josua Hunziker AutoGroups tools https://github.com/stjosh/auto_groups diff --git a/appinfo/routes.php b/appinfo/routes.php index 19be8d1..458b1f3 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -1,11 +1,24 @@ OCA\AutoGroupsGroups\Controller\PageController->index() + * @copyright Copyright (c) 2020 + * + * @author Josua Hunziker + * + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see * - * The controller class has to be registered in the application.php file since - * it's instantiated in there */ return [ 'routes' => [ diff --git a/css/admin.css b/css/admin.css index 485f063..1fe0106 100644 --- a/css/admin.css +++ b/css/admin.css @@ -1,3 +1,24 @@ +/** + * @copyright Copyright (c) 2020 + * + * @author Josua Hunziker + * + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + #auto_groups_options { position: relative; } diff --git a/js/admin.js b/js/admin.js index 5ecb8d8..df36ee6 100644 --- a/js/admin.js +++ b/js/admin.js @@ -1,14 +1,23 @@ /** - * Copyright (c) 2020 + * @copyright Copyright (c) 2020 * - * @author Josua Hunziker + * @author Josua Hunziker * * Based on the work of Ján Stibila and Lukas Reschke * - * This file is licensed under the Affero General Public License version 3 - * or later. + * @license AGPL-3.0 * - * See the COPYING-README file. + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see * */ diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index d1a7fce..a48d1a3 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -2,7 +2,7 @@ /** * @copyright Copyright (c) 2020 * - * @author Josua Hunziker + * @author Josua Hunziker * * Based on the work of Ján Stibila * diff --git a/lib/AutoGroupsManager.php b/lib/AutoGroupsManager.php index 7697de7..9bbd2f0 100644 --- a/lib/AutoGroupsManager.php +++ b/lib/AutoGroupsManager.php @@ -3,7 +3,7 @@ /** * @copyright Copyright (c) 2020 * - * @author Josua Hunziker + * @author Josua Hunziker * * Based on the work of Ján Stibila * diff --git a/lib/Listener/AutoGroupsListener.php b/lib/Listener/AutoGroupsListener.php index 364e9b6..23d7f93 100644 --- a/lib/Listener/AutoGroupsListener.php +++ b/lib/Listener/AutoGroupsListener.php @@ -3,7 +3,7 @@ /** * @copyright Copyright (c) 2020 * - * @author Josua Hunziker + * @author Josua Hunziker * * @license AGPL-3.0 * diff --git a/lib/Settings/Admin.php b/lib/Settings/Admin.php index 37e485d..74eb839 100644 --- a/lib/Settings/Admin.php +++ b/lib/Settings/Admin.php @@ -2,7 +2,7 @@ /** * @copyright Copyright (c) 2020 * - * @author Josua Hunziker + * @author Josua Hunziker * * Based on the work of Ján Stibila * diff --git a/templates/admin.php b/templates/admin.php index 382f98c..495b217 100644 --- a/templates/admin.php +++ b/templates/admin.php @@ -3,7 +3,7 @@ /** * @copyright Copyright (c) 2020 * - * @author Josua Hunziker + * @author Josua Hunziker * * Based on the work of Ján Stibila * diff --git a/tests/Integration/AdminSettingsTest.php b/tests/Integration/AdminSettingsTest.php index 1930ca0..aadf963 100644 --- a/tests/Integration/AdminSettingsTest.php +++ b/tests/Integration/AdminSettingsTest.php @@ -3,7 +3,7 @@ /** * @copyright Copyright (c) 2020 * - * @author Josua Hunziker + * @author Josua Hunziker * * @license GNU AGPL version 3 or any later version * diff --git a/tests/Integration/EventsTest.php b/tests/Integration/EventsTest.php index 171e903..abb8149 100644 --- a/tests/Integration/EventsTest.php +++ b/tests/Integration/EventsTest.php @@ -3,7 +3,7 @@ /** * @copyright Copyright (c) 2020 * - * @author Josua Hunziker + * @author Josua Hunziker * * @license GNU AGPL version 3 or any later version * diff --git a/tests/Unit/AdminSettingsTest.php b/tests/Unit/AdminSettingsTest.php index 8d971f7..62a40f8 100644 --- a/tests/Unit/AdminSettingsTest.php +++ b/tests/Unit/AdminSettingsTest.php @@ -3,7 +3,7 @@ /** * @copyright Copyright (c) 2020 * - * @author Josua Hunziker + * @author Josua Hunziker * * @license GNU AGPL version 3 or any later version * diff --git a/tests/Unit/AutoGroupsManagerTest.php b/tests/Unit/AutoGroupsManagerTest.php index 98c5e27..4c38da4 100644 --- a/tests/Unit/AutoGroupsManagerTest.php +++ b/tests/Unit/AutoGroupsManagerTest.php @@ -3,7 +3,7 @@ /** * @copyright Copyright (c) 2020 * - * @author Josua Hunziker + * @author Josua Hunziker * * @license GNU AGPL version 3 or any later version * From 6809920ea5e2c76a9570701939f6e3b6cdf58ff0 Mon Sep 17 00:00:00 2001 From: Josua Hunziker Date: Fri, 20 Mar 2026 09:00:22 +0100 Subject: [PATCH 07/13] fix(test): dispatch PostLoginEvent directly in testLoginHook IUserSession::login() does not reliably fire events in the CLI test context (no real HTTP session), so the login hook was never triggered. Dispatch PostLoginEvent via IEventDispatcher directly instead. Co-Authored-By: Claude Sonnet 4.6 --- tests/Integration/EventsTest.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/Integration/EventsTest.php b/tests/Integration/EventsTest.php index abb8149..e2c5310 100644 --- a/tests/Integration/EventsTest.php +++ b/tests/Integration/EventsTest.php @@ -28,7 +28,8 @@ use OCP\IUserManager; use OCP\IGroupManager; use OCP\IConfig; -use OCP\IUserSession; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\User\Events\PostLoginEvent; use OCP\AppFramework\OCS\OCSBadRequestException; @@ -46,7 +47,7 @@ class EventsTest extends TestCase private $userManager; private $groupManager; private $config; - private $userSession; + private $eventDispatcher; private $backend; @@ -60,7 +61,7 @@ protected function setUp(): void $this->groupManager = $this->container->query(IGroupManager::class); $this->userManager = $this->container->query(IUserManager::class); $this->config = $this->container->query(IConfig::class); - $this->userSession = $this->container->query(IUserSession::class); + $this->eventDispatcher = $this->container->query(IEventDispatcher::class); $this->backend = $this->groupManager->getBackends()[0]; @@ -153,7 +154,9 @@ public function testLoginHook() $this->assertTrue($autogroup1->inGroup($testUser) && $autogroup2->inGroup($testUser)); // Login SHOULD trigger removal from auto groups (login_hook=true, user in override group) - $this->userSession->login('testuser', 'testPassword'); + // Note: IUserSession::login() does not reliably dispatch events in CLI test context, + // so we dispatch PostLoginEvent directly to test the listener. + $this->eventDispatcher->dispatchTyped(new PostLoginEvent($testUser, 'testPassword', false)); $this->assertTrue(!$autogroup1->inGroup($testUser) && !$autogroup2->inGroup($testUser)); } From b682afb42989a70359d701f0bdf8ca4bd72a9562 Mon Sep 17 00:00:00 2001 From: Josua Hunziker Date: Fri, 20 Mar 2026 09:01:46 +0100 Subject: [PATCH 08/13] chore: add changelog entry for 1.7.0 Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c5e4fb..58f5c04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## 1.7.0 - 2026-03-20 + +### Changed + +- Migrate event listener registration to `IBootstrap`/`registerEventListener()` for compatibility with NC34, replacing the deprecated `IEventDispatcher::addListener()` approach +- Compatibility up to NC34 + +### Fixed + +- Restore login hook test which was previously broken due to hook config being read at app instantiation time rather than at event dispatch time + ## 1.6.2 - 2025-03-03 ### Fixed From e5af1b0880c0d531f9b5e483bf529fe2a8758f9e Mon Sep 17 00:00:00 2001 From: Josua Hunziker Date: Fri, 20 Mar 2026 09:02:59 +0100 Subject: [PATCH 09/13] fix(test): pass loginName argument to PostLoginEvent constructor Co-Authored-By: Claude Sonnet 4.6 --- tests/Integration/EventsTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/EventsTest.php b/tests/Integration/EventsTest.php index e2c5310..43ccaf7 100644 --- a/tests/Integration/EventsTest.php +++ b/tests/Integration/EventsTest.php @@ -156,7 +156,7 @@ public function testLoginHook() // Login SHOULD trigger removal from auto groups (login_hook=true, user in override group) // Note: IUserSession::login() does not reliably dispatch events in CLI test context, // so we dispatch PostLoginEvent directly to test the listener. - $this->eventDispatcher->dispatchTyped(new PostLoginEvent($testUser, 'testPassword', false)); + $this->eventDispatcher->dispatchTyped(new PostLoginEvent($testUser, 'testuser', 'testPassword', false)); $this->assertTrue(!$autogroup1->inGroup($testUser) && !$autogroup2->inGroup($testUser)); } From b2386d818cf0bad1b47f4930486368df8f361c91 Mon Sep 17 00:00:00 2001 From: Josua Hunziker Date: Fri, 20 Mar 2026 09:16:30 +0100 Subject: [PATCH 10/13] fix(test): use IUserSession::login() with prior logout() for testLoginHook The original test used login() but was broken because a prior test left an active session, causing login() to short-circuit without dispatching events. Adding logout() first ensures a clean session state. Co-Authored-By: Claude Sonnet 4.6 --- tests/Integration/EventsTest.php | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/Integration/EventsTest.php b/tests/Integration/EventsTest.php index 43ccaf7..2cef792 100644 --- a/tests/Integration/EventsTest.php +++ b/tests/Integration/EventsTest.php @@ -28,8 +28,7 @@ use OCP\IUserManager; use OCP\IGroupManager; use OCP\IConfig; -use OCP\EventDispatcher\IEventDispatcher; -use OCP\User\Events\PostLoginEvent; +use OCP\IUserSession; use OCP\AppFramework\OCS\OCSBadRequestException; @@ -47,7 +46,7 @@ class EventsTest extends TestCase private $userManager; private $groupManager; private $config; - private $eventDispatcher; + private $userSession; private $backend; @@ -61,7 +60,7 @@ protected function setUp(): void $this->groupManager = $this->container->query(IGroupManager::class); $this->userManager = $this->container->query(IUserManager::class); $this->config = $this->container->query(IConfig::class); - $this->eventDispatcher = $this->container->query(IEventDispatcher::class); + $this->userSession = $this->container->query(IUserSession::class); $this->backend = $this->groupManager->getBackends()[0]; @@ -153,10 +152,10 @@ public function testLoginHook() $overridegroup->addUser($testUser); $this->assertTrue($autogroup1->inGroup($testUser) && $autogroup2->inGroup($testUser)); - // Login SHOULD trigger removal from auto groups (login_hook=true, user in override group) - // Note: IUserSession::login() does not reliably dispatch events in CLI test context, - // so we dispatch PostLoginEvent directly to test the listener. - $this->eventDispatcher->dispatchTyped(new PostLoginEvent($testUser, 'testuser', 'testPassword', false)); + // Login SHOULD trigger removal from auto groups (login_hook=true, user in override group). + // logout() first to clear any active session from a previous test, otherwise login() short-circuits. + $this->userSession->logout(); + $this->userSession->login('testuser', 'testPassword'); $this->assertTrue(!$autogroup1->inGroup($testUser) && !$autogroup2->inGroup($testUser)); } From a7fe985dbfab09162f5480685017a7ce9f712de6 Mon Sep 17 00:00:00 2001 From: Josua Hunziker Date: Fri, 20 Mar 2026 09:20:14 +0100 Subject: [PATCH 11/13] fix(test): rewrite testLoginHook using dispatchTyped with a fresh user IUserSession::login() and logout() require HTTP session infrastructure unavailable in CLI test context (headers already sent error). Instead, dispatch PostLoginEvent via IEventDispatcher::dispatchTyped(), the same mechanism Nextcloud uses internally for all other integration tests in this suite. Use a dedicated fresh user to avoid state dependencies on other tests, and test both addition (no override group) and removal (in override group) in two phases. Co-Authored-By: Claude Sonnet 4.6 --- tests/Integration/EventsTest.php | 41 +++++++++++++++----------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/tests/Integration/EventsTest.php b/tests/Integration/EventsTest.php index 2cef792..115ff13 100644 --- a/tests/Integration/EventsTest.php +++ b/tests/Integration/EventsTest.php @@ -28,7 +28,8 @@ use OCP\IUserManager; use OCP\IGroupManager; use OCP\IConfig; -use OCP\IUserSession; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\User\Events\PostLoginEvent; use OCP\AppFramework\OCS\OCSBadRequestException; @@ -46,7 +47,7 @@ class EventsTest extends TestCase private $userManager; private $groupManager; private $config; - private $userSession; + private $eventDispatcher; private $backend; @@ -60,7 +61,7 @@ protected function setUp(): void $this->groupManager = $this->container->query(IGroupManager::class); $this->userManager = $this->container->query(IUserManager::class); $this->config = $this->container->query(IConfig::class); - $this->userSession = $this->container->query(IUserSession::class); + $this->eventDispatcher = $this->container->query(IEventDispatcher::class); $this->backend = $this->groupManager->getBackends()[0]; @@ -135,29 +136,25 @@ public function testLoginHook() $this->config->setAppValue("auto_groups", "creation_hook", 'false'); $this->config->setAppValue("auto_groups", "modification_hook", 'false'); - $testUser = $this->userManager->get('testuser'); - $overridegroup = $this->groupManager->search('overridegroup1')[0]; + // Use a dedicated user for this test to avoid state from other tests. + // IUserSession::login() cannot be used in CLI test context (no HTTP session), + // so we dispatch PostLoginEvent via the Nextcloud event dispatcher directly — + // the same mechanism Nextcloud uses internally for all other events in this test suite. + $loginUser = $this->userManager->createUser('loginuser', 'testPassword'); $autogroup1 = $this->groupManager->search('autogroup1')[0]; $autogroup2 = $this->groupManager->search('autogroup2')[0]; + $overridegroup = $this->groupManager->search('overridegroup1')[0]; - // Set up initial state explicitly: user in auto groups, not in override group. - // modification_hook=false so these direct adds won't trigger auto groups logic. - $overridegroup->removeUser($testUser); - $autogroup1->addUser($testUser); - $autogroup2->addUser($testUser); - - $this->assertTrue($autogroup1->inGroup($testUser) && $autogroup2->inGroup($testUser)); - - // Adding to override group should NOT remove from auto groups (modification_hook=false) - $overridegroup->addUser($testUser); - $this->assertTrue($autogroup1->inGroup($testUser) && $autogroup2->inGroup($testUser)); - - // Login SHOULD trigger removal from auto groups (login_hook=true, user in override group). - // logout() first to clear any active session from a previous test, otherwise login() short-circuits. - $this->userSession->logout(); - $this->userSession->login('testuser', 'testPassword'); + // Phase 1: user is not in override group → login should ADD them to auto groups + $this->assertFalse($autogroup1->inGroup($loginUser)); + $this->eventDispatcher->dispatchTyped(new PostLoginEvent($loginUser, 'loginuser', 'testPassword', false)); + $this->assertTrue($autogroup1->inGroup($loginUser) && $autogroup2->inGroup($loginUser)); - $this->assertTrue(!$autogroup1->inGroup($testUser) && !$autogroup2->inGroup($testUser)); + // Phase 2: add user to override group (modification_hook=false so no auto-trigger), + // then login should REMOVE them from auto groups + $overridegroup->addUser($loginUser); + $this->eventDispatcher->dispatchTyped(new PostLoginEvent($loginUser, 'loginuser', 'testPassword', false)); + $this->assertFalse($autogroup1->inGroup($loginUser) || $autogroup2->inGroup($loginUser)); } From 96434cb51a8b1f9eaaaa271d326a1eb0a41e24a5 Mon Sep 17 00:00:00 2001 From: Josua Hunziker Date: Fri, 20 Mar 2026 09:37:47 +0100 Subject: [PATCH 12/13] fix(test): re-fetch group objects in testLoginHook Phase 2 to avoid stale cache IGroup::inGroup() uses a per-object $users cache. After Phase 1 adds loginUser to auto groups, the local $autogroup1/$autogroup2 variables hold populated caches. When addAndRemoveAutoGroups() later calls removeUser() on different IGroup instances, the test-local objects' caches are not cleared, causing the Phase 2 assertion to see stale data. Re-fetching via groupManager->get() returns fresh objects with no cache, giving correct results. Co-Authored-By: Claude Sonnet 4.6 --- tests/Integration/EventsTest.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/Integration/EventsTest.php b/tests/Integration/EventsTest.php index 115ff13..f18182c 100644 --- a/tests/Integration/EventsTest.php +++ b/tests/Integration/EventsTest.php @@ -151,10 +151,13 @@ public function testLoginHook() $this->assertTrue($autogroup1->inGroup($loginUser) && $autogroup2->inGroup($loginUser)); // Phase 2: add user to override group (modification_hook=false so no auto-trigger), - // then login should REMOVE them from auto groups + // then login should REMOVE them from auto groups. + // Re-fetch group objects to avoid stale per-Group $users cache from Phase 1. $overridegroup->addUser($loginUser); $this->eventDispatcher->dispatchTyped(new PostLoginEvent($loginUser, 'loginuser', 'testPassword', false)); - $this->assertFalse($autogroup1->inGroup($loginUser) || $autogroup2->inGroup($loginUser)); + $freshGroup1 = $this->groupManager->get('autogroup1'); + $freshGroup2 = $this->groupManager->get('autogroup2'); + $this->assertFalse($freshGroup1->inGroup($loginUser) || $freshGroup2->inGroup($loginUser)); } From 102a776bbc1247c2bcc06449f4406d44c5f5707a Mon Sep 17 00:00:00 2001 From: Josua Hunziker Date: Fri, 20 Mar 2026 09:42:17 +0100 Subject: [PATCH 13/13] docs: update README test matrix and remove stale roadmap copy for 1.7.0 - Drop stable30 row (no longer supported), add stable33 row - Fix stable32 codecov flag (was incorrectly pointing to stable31) - Mark master/NC34 as experimental to match CI matrix - Remove "Union Groups" roadmap line from README and info.xml description Co-Authored-By: Claude Sonnet 4.6 --- README.md | 12 +++++------- appinfo/info.xml | 1 - 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index d320ee2..17eca83 100644 --- a/README.md +++ b/README.md @@ -4,12 +4,12 @@ Automatically add users to specified Auto Groups, except for those belonging to ## Test Status -| Nextcloud Server Branch | Unit & Integration Tests |  Code Coverage | +| Nextcloud Server Branch | Unit & Integration Tests | Code Coverage | | ------------------------------------------------------------- | :-----------------------------------------------------------------------------------------------------------------------: | :-------------------------------------------------------------------------------------------------------------------------------------------: | -| [stable30](https://github.com/nextcloud/server/tree/stable30) | ![Unit and Integration Tests](https://github.com/stjosh/auto_groups/workflows/Unit%20and%20Integration%20Tests/badge.svg) |  [![codecov](https://codecov.io/gh/stjosh/auto_groups/branch/master/graph/badge.svg?flag=stable30)](https://codecov.io/gh/stjosh/auto_groups) | -| [stable31](https://github.com/nextcloud/server/tree/stable31) | ![Unit and Integration Tests](https://github.com/stjosh/auto_groups/workflows/Unit%20and%20Integration%20Tests/badge.svg) |  [![codecov](https://codecov.io/gh/stjosh/auto_groups/branch/master/graph/badge.svg?flag=stable31)](https://codecov.io/gh/stjosh/auto_groups) | -| [stable32](https://github.com/nextcloud/server/tree/stable32) | ![Unit and Integration Tests](https://github.com/stjosh/auto_groups/workflows/Unit%20and%20Integration%20Tests/badge.svg) |  [![codecov](https://codecov.io/gh/stjosh/auto_groups/branch/master/graph/badge.svg?flag=stable31)](https://codecov.io/gh/stjosh/auto_groups) | -| [master](https://github.com/nextcloud/server/tree/master) | ![Unit and Integration Tests](https://github.com/stjosh/auto_groups/workflows/Unit%20and%20Integration%20Tests/badge.svg) |  [![codecov](https://codecov.io/gh/stjosh/auto_groups/branch/master/graph/badge.svg?flag=master)](https://codecov.io/gh/stjosh/auto_groups) | +| [stable31](https://github.com/nextcloud/server/tree/stable31) | ![Unit and Integration Tests](https://github.com/stjosh/auto_groups/workflows/Unit%20and%20Integration%20Tests/badge.svg) | [![codecov](https://codecov.io/gh/stjosh/auto_groups/branch/master/graph/badge.svg?flag=stable31)](https://codecov.io/gh/stjosh/auto_groups) | +| [stable32](https://github.com/nextcloud/server/tree/stable32) | ![Unit and Integration Tests](https://github.com/stjosh/auto_groups/workflows/Unit%20and%20Integration%20Tests/badge.svg) | [![codecov](https://codecov.io/gh/stjosh/auto_groups/branch/master/graph/badge.svg?flag=stable32)](https://codecov.io/gh/stjosh/auto_groups) | +| [stable33](https://github.com/nextcloud/server/tree/stable33) | ![Unit and Integration Tests](https://github.com/stjosh/auto_groups/workflows/Unit%20and%20Integration%20Tests/badge.svg) | [![codecov](https://codecov.io/gh/stjosh/auto_groups/branch/master/graph/badge.svg?flag=stable33)](https://codecov.io/gh/stjosh/auto_groups) | +| [master](https://github.com/nextcloud/server/tree/master) (NC34, experimental) | ![Unit and Integration Tests](https://github.com/stjosh/auto_groups/workflows/Unit%20and%20Integration%20Tests/badge.svg) | [![codecov](https://codecov.io/gh/stjosh/auto_groups/branch/master/graph/badge.svg?flag=master)](https://codecov.io/gh/stjosh/auto_groups) | Unit and Integration Tests are executed with PHP v8.2 and v8.3. @@ -35,8 +35,6 @@ and then access your test instance on http://localhost:8080. The `auto_groups` a - [Everyone Group](https://apps.nextcloud.com/apps/group_everyone): The "Everyone Group" app adds a virtual Group Backend, always returning all users. In contrast, "Auto Groups" operates on "real" groups in your normal Group Backend. Additionally, it is possible to specify Override Groups which will prevent users from being added to the Auto Group(s). - [Default Group](https://apps.nextcloud.com/apps/defaultgroup): "Auto Groups" is actually a modernized and maintaned fork of "Default Group", which seems to be abandoned since NC12 or so. In terms of functionality, they are almost identical. -In addition, I plan to add some more features over time, e.g., "Union Groups" - see the [Milestone Plans](https://github.com/stjosh/auto_groups/milestones) for more details. - ## Issue Tracker / Contributions Contributions are welcome on [GitHub](https://github.com/stjosh/auto_groups/issues). diff --git a/appinfo/info.xml b/appinfo/info.xml index ca96aac..d2f7aac 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -22,7 +22,6 @@ Note that this app prevents group deletions for groups referenced as Auto Groups * [Everyone Group](https://apps.nextcloud.com/apps/group_everyone): The "Everyone Group" app adds a virtual Group Backend, always returning all users. In contrast, "Auto Groups" operates on "real" groups in your normal Group Backend. Additionally, it is possible to specify Override Groups which will prevent users from being added to the Auto Group(s). * [Default Group](https://apps.nextcloud.com/apps/defaultgroup): "Auto Groups" is actually a modernized and maintaned fork of "Default Group", which seems to be abandoned since NC12 or so. In terms of functionality, they are almost identical. -In addition, I plan to add some more features over time, e.g., "Union Groups" - see the [Milestone Plans](https://github.com/stjosh/auto_groups/milestones) for more details. 1.7.0 agpl