From f94ffb65735e2ae6d915c7b15748cb8e1c4af3ea Mon Sep 17 00:00:00 2001 From: Marcel Werk Date: Mon, 19 Oct 2026 19:43:09 +0200 Subject: [PATCH] Add optional automatic revocation of user group memberships for assignments ref https://www.woltlab.com/community/thread/319672-automatische-zuordnung-von-benutzergruppen-verbessern/ --- .../update_com.woltlab.wcf_6.3_step1.php | 1 + .../form/UserGroupAssignmentAddForm.class.php | 106 +++++++++- .../user/RemoveUserFromGroups.class.php | 4 +- .../assignment/UserGroupAssignment.class.php | 1 + .../UserGroupAssignmentBuilder.class.php | 11 + .../UserGroupAssignmentCronjob.class.php | 9 +- ...oupAssignmentObjectFilterBuilder.class.php | 7 +- .../UserGroupAssignmentHandler.class.php | 198 +++++++++++++++++- wcfsetup/install/lang/de.xml | 6 + wcfsetup/install/lang/en.xml | 6 + wcfsetup/setup/db/install_com.woltlab.wcf.php | 1 + 11 files changed, 338 insertions(+), 12 deletions(-) diff --git a/wcfsetup/install/files/acp/database/update_com.woltlab.wcf_6.3_step1.php b/wcfsetup/install/files/acp/database/update_com.woltlab.wcf_6.3_step1.php index d0b110c8c55..ea89f87ca72 100644 --- a/wcfsetup/install/files/acp/database/update_com.woltlab.wcf_6.3_step1.php +++ b/wcfsetup/install/files/acp/database/update_com.woltlab.wcf_6.3_step1.php @@ -365,6 +365,7 @@ PartialDatabaseTable::create('wcf1_user_group_assignment') ->columns([ JsonDatabaseTableColumn::create('conditions'), + DefaultFalseBooleanDatabaseTableColumn::create('revokeAutomatically'), ]), PartialDatabaseTable::create('wcf1_notice') ->columns([ diff --git a/wcfsetup/install/files/lib/acp/form/UserGroupAssignmentAddForm.class.php b/wcfsetup/install/files/lib/acp/form/UserGroupAssignmentAddForm.class.php index ecea1b1e34c..4f8883ffb4b 100644 --- a/wcfsetup/install/files/lib/acp/form/UserGroupAssignmentAddForm.class.php +++ b/wcfsetup/install/files/lib/acp/form/UserGroupAssignmentAddForm.class.php @@ -7,6 +7,7 @@ use wcf\data\DatabaseObjectBuilder; use wcf\data\user\group\assignment\UserGroupAssignment; use wcf\data\user\group\assignment\UserGroupAssignmentBuilder; +use wcf\data\user\group\assignment\UserGroupAssignmentList; use wcf\data\user\group\UserGroup; use wcf\form\AbstractDatabaseObjectBuilderForm; use wcf\system\form\builder\container\FormContainer; @@ -15,12 +16,15 @@ use wcf\system\form\builder\field\ObjectFilterFormField; use wcf\system\form\builder\field\SelectFormField; use wcf\system\form\builder\field\TitleFormField; +use wcf\system\form\builder\field\validation\FormFieldValidationError; +use wcf\system\form\builder\field\validation\FormFieldValidator; use wcf\system\object\filter\builder\UserGroupAssignmentObjectFilterBuilder; +use wcf\system\user\group\assignment\UserGroupAssignmentHandler; /** * Shows the form to create a new automatic user group assignment. * - * @author Alexander Ebert, Matthias Schmidt + * @author Alexander Ebert, Matthias Schmidt, Marcel Werk * @copyright 2001-2026 WoltLab GmbH * @license GNU Lesser General Public License * @@ -80,6 +84,7 @@ protected function createForm(): void ->label('wcf.user.group') ->options($this->getAvailableUserGroups()) ->required() + ->addValidator($this->getGroupIDValidator()) ->saveValueCallback(static function (UserGroupAssignmentBuilder $builder, IFormField $field) { $builder->setGroupID((int)$field->getSaveValue()); }), @@ -88,6 +93,13 @@ protected function createForm(): void ->saveValueCallback(static function (UserGroupAssignmentBuilder $builder, IFormField $field) { $builder->setIsDisabled((bool)$field->getSaveValue()); }), + BooleanFormField::create('revokeAutomatically') + ->label('wcf.acp.group.assignment.revokeAutomatically') + ->description('wcf.acp.group.assignment.revokeAutomatically.description') + ->addValidator($this->getRevokeAutomaticallyValidator()) + ->saveValueCallback(static function (UserGroupAssignmentBuilder $builder, IFormField $field) { + $builder->setRevokeAutomatically((bool)$field->getSaveValue()); + }), ]), FormContainer::create('conditionsContainer') ->label('wcf.acp.group.assignment.conditions') @@ -103,6 +115,98 @@ protected function createForm(): void ]); } + /** + * Rejects additional assignments for user groups whose assignment + * automatically removes users, which only supports a single assignment. + */ + private function getGroupIDValidator(): FormFieldValidator + { + return new FormFieldValidator('revokeAutomatically', function (SelectFormField $field) { + $revokeField = $field->getDocument()->getNodeById('revokeAutomatically'); + \assert($revokeField instanceof BooleanFormField); + if ($revokeField->getValue() === true) { + // Conflicts are reported by the validator of the automatic revocation. + return; + } + + foreach ($this->getOtherAssignments((int)$field->getSaveValue()) as $assignment) { + if ($assignment->revokeAutomatically === 1) { + $field->addValidationError(new FormFieldValidationError( + 'revokeAutomatically', + 'wcf.acp.group.assignment.groupID.error.revokeAutomatically' + )); + + return; + } + } + }); + } + + /** + * Rejects the automatic revocation for user groups that grant administrative + * access or have other assignments and for conditions that exclude members + * of the target user group, which would repeatedly add and remove the same + * users. + */ + private function getRevokeAutomaticallyValidator(): FormFieldValidator + { + return new FormFieldValidator('revokeAutomatically', function (BooleanFormField $field) { + if ($field->getValue() !== true) { + return; + } + + $groupField = $field->getDocument()->getNodeById('groupID'); + \assert($groupField instanceof SelectFormField); + $groupID = (int)$groupField->getSaveValue(); + + $group = UserGroup::getGroupByID($groupID); + if ($group !== null && !UserGroupAssignmentHandler::isRevocableGroup($group)) { + $field->addValidationError(new FormFieldValidationError( + 'adminGroup', + 'wcf.acp.group.assignment.revokeAutomatically.error.adminGroup' + )); + + return; + } + + if ($this->getOtherAssignments($groupID) !== []) { + $field->addValidationError(new FormFieldValidationError( + 'otherAssignments', + 'wcf.acp.group.assignment.revokeAutomatically.error.otherAssignments' + )); + + return; + } + + $conditionsField = $field->getDocument()->getNodeById('conditions'); + \assert($conditionsField instanceof ObjectFilterFormField); + if (UserGroupAssignmentHandler::excludesTargetGroup($groupID, $conditionsField->getValue())) { + $field->addValidationError(new FormFieldValidationError( + 'notInTargetGroup', + 'wcf.acp.group.assignment.revokeAutomatically.error.notInTargetGroup' + )); + } + }); + } + + /** + * Returns the other assignments for the given user group, including the + * disabled ones because they can be enabled without using this form. + * + * @return array + */ + private function getOtherAssignments(int $groupID): array + { + $assignmentList = new UserGroupAssignmentList(); + $assignmentList->getConditionBuilder()->add('groupID = ?', [$groupID]); + if ($this->formObject !== null) { + $assignmentList->getConditionBuilder()->add('assignmentID <> ?', [$this->formObject->assignmentID]); + } + $assignmentList->readObjects(); + + return $assignmentList->getObjects(); + } + /** * @return array */ diff --git a/wcfsetup/install/files/lib/command/user/RemoveUserFromGroups.class.php b/wcfsetup/install/files/lib/command/user/RemoveUserFromGroups.class.php index d2bae8225ea..7b1ce7dd4ef 100644 --- a/wcfsetup/install/files/lib/command/user/RemoveUserFromGroups.class.php +++ b/wcfsetup/install/files/lib/command/user/RemoveUserFromGroups.class.php @@ -21,6 +21,7 @@ final class RemoveUserFromGroups public function __construct( private readonly User $user, private readonly array $groupIDs, + private readonly bool $ignoreUserGroupAssignments = false, ) {} public function __invoke(): void @@ -33,7 +34,8 @@ public function __invoke(): void } new UpdateUser( - UserBuilder::forUpdate($this->user)->setGroupIDs($groupIDs) + UserBuilder::forUpdate($this->user)->setGroupIDs($groupIDs), + $this->ignoreUserGroupAssignments )(); } } diff --git a/wcfsetup/install/files/lib/data/user/group/assignment/UserGroupAssignment.class.php b/wcfsetup/install/files/lib/data/user/group/assignment/UserGroupAssignment.class.php index 022d7793619..6ea5e74b0ce 100644 --- a/wcfsetup/install/files/lib/data/user/group/assignment/UserGroupAssignment.class.php +++ b/wcfsetup/install/files/lib/data/user/group/assignment/UserGroupAssignment.class.php @@ -20,6 +20,7 @@ * @property-read string $title title of the automatic user group assignment * @property-read 0|1 $isDisabled is `1` if the user group assignment is disabled and thus not checked for automatic assignments, otherwise `0` * @property-read string $conditions + * @property-read 0|1 $revokeAutomatically is `1` if members of the user group are automatically removed once they no longer fulfill the conditions of this assignment, otherwise `0` */ class UserGroupAssignment extends DatabaseObject implements IRouteController { diff --git a/wcfsetup/install/files/lib/data/user/group/assignment/UserGroupAssignmentBuilder.class.php b/wcfsetup/install/files/lib/data/user/group/assignment/UserGroupAssignmentBuilder.class.php index da580ac86cc..c9860cd92fd 100644 --- a/wcfsetup/install/files/lib/data/user/group/assignment/UserGroupAssignmentBuilder.class.php +++ b/wcfsetup/install/files/lib/data/user/group/assignment/UserGroupAssignmentBuilder.class.php @@ -44,6 +44,17 @@ public function setIsDisabled(bool $isDisabled): static return $this; } + /** + * Sets whether members of the user group are automatically removed once + * they no longer fulfill the conditions of this assignment. + */ + public function setRevokeAutomatically(bool $revokeAutomatically): static + { + $this->properties['revokeAutomatically'] = (int)$revokeAutomatically; + + return $this; + } + /** * Sets the serialized object filters that a user must match. */ diff --git a/wcfsetup/install/files/lib/system/cronjob/UserGroupAssignmentCronjob.class.php b/wcfsetup/install/files/lib/system/cronjob/UserGroupAssignmentCronjob.class.php index d9256ccb5e6..bd4c864f577 100644 --- a/wcfsetup/install/files/lib/system/cronjob/UserGroupAssignmentCronjob.class.php +++ b/wcfsetup/install/files/lib/system/cronjob/UserGroupAssignmentCronjob.class.php @@ -8,7 +8,8 @@ use wcf\system\user\group\assignment\UserGroupAssignmentHandler; /** - * Executes automatic user group assignments. + * Executes automatic user group assignments and removes users who no longer + * fulfill the assignment of user groups with automatic revocation. * * @author Matthias Schmidt * @copyright 2001-2019 WoltLab GmbH @@ -23,10 +24,14 @@ public function execute(Cronjob $cronjob) { parent::execute($cronjob); + $assignmentCount = UserGroupAssignmentHandler::getInstance()->revokeUsers(self::MAXIMUM_ASSIGNMENTS); + if ($assignmentCount >= self::MAXIMUM_ASSIGNMENTS) { + return; + } + $assignments = UserGroupAssignmentCacheBuilder::getInstance()->getData(); $usersToGroup = []; - $assignmentCount = 0; foreach ($assignments as $assignment) { if (!isset($usersToGroup[$assignment->groupID])) { $usersToGroup[$assignment->groupID] = []; diff --git a/wcfsetup/install/files/lib/system/object/filter/builder/UserGroupAssignmentObjectFilterBuilder.class.php b/wcfsetup/install/files/lib/system/object/filter/builder/UserGroupAssignmentObjectFilterBuilder.class.php index d57e910c52d..ddc1b16f151 100644 --- a/wcfsetup/install/files/lib/system/object/filter/builder/UserGroupAssignmentObjectFilterBuilder.class.php +++ b/wcfsetup/install/files/lib/system/object/filter/builder/UserGroupAssignmentObjectFilterBuilder.class.php @@ -94,13 +94,14 @@ public function isAccessible(): bool } /** - * Adds the filters of the assignment to the given user list. + * Adds the filters of the assignment to the given user list. Returns + * whether any active filter has been applied. * * An assignment without any active filters matches no users. */ - public function applyFilters(UserGroupAssignment $assignment, UserList $userList): void + public function applyFilters(UserGroupAssignment $assignment, UserList $userList): bool { - $this->getHandler()->applyFilters( + return $this->getHandler()->applyFilters( $userList, $assignment->conditions, ); diff --git a/wcfsetup/install/files/lib/system/user/group/assignment/UserGroupAssignmentHandler.class.php b/wcfsetup/install/files/lib/system/user/group/assignment/UserGroupAssignmentHandler.class.php index 5dfa3a23348..8af065cf83c 100644 --- a/wcfsetup/install/files/lib/system/user/group/assignment/UserGroupAssignmentHandler.class.php +++ b/wcfsetup/install/files/lib/system/user/group/assignment/UserGroupAssignmentHandler.class.php @@ -3,20 +3,24 @@ namespace wcf\system\user\group\assignment; use wcf\command\user\AddUserToGroups; +use wcf\command\user\RemoveUserFromGroups; use wcf\data\object\type\ObjectType; use wcf\data\object\type\ObjectTypeCache; use wcf\data\user\group\assignment\UserGroupAssignment; +use wcf\data\user\group\UserGroup; use wcf\data\user\User; use wcf\data\user\UserList; use wcf\system\cache\builder\UserGroupAssignmentCacheBuilder; use wcf\system\object\filter\builder\UserGroupAssignmentObjectFilterBuilder; +use wcf\system\object\filter\user\UserNotInGroupObjectFilter; use wcf\system\SingletonFactory; +use wcf\system\WCF; /** * Handles user group assignment-related matters. * - * @author Matthias Schmidt - * @copyright 2001-2019 WoltLab GmbH + * @author Matthias Schmidt, Marcel Werk + * @copyright 2001-2026 WoltLab GmbH * @license GNU Lesser General Public License */ class UserGroupAssignmentHandler extends SingletonFactory @@ -27,9 +31,12 @@ class UserGroupAssignmentHandler extends SingletonFactory */ protected $groupedObjectTypes = []; + private UserGroupAssignmentObjectFilterBuilder $objectFilterBuilder; + /** * Checks if the users with the given ids should be assigned to new user - * groups. + * groups or removed from user groups whose assignments they no longer + * fulfill. * * Note: This method uses the user ids as a parameter instead of user objects * on purpose to make sure the latest data of the users are fetched. @@ -49,7 +56,8 @@ public function checkUsers(array $userIDs) /** @var UserGroupAssignment[] $assignments */ $assignments = UserGroupAssignmentCacheBuilder::getInstance()->getData(); - $builder = new UserGroupAssignmentObjectFilterBuilder(); + $builder = $this->getObjectFilterBuilder(); + $revocableAssignments = $this->getRevocableAssignments(); foreach ($userList as $user) { $groupIDs = $user->getGroupIDs(); @@ -71,9 +79,92 @@ public function checkUsers(array $userIDs) if ($newGroupIDs !== []) { (new AddUserToGroups($user, $newGroupIDs, ignoreUserGroupAssignments: true))(); } + + $removedGroupIDs = []; + foreach ($revocableAssignments as $assignment) { + if ( + \in_array($assignment->groupID, $groupIDs, true) + && !$builder->testUser($assignment, $user) + ) { + $removedGroupIDs[] = $assignment->groupID; + } + } + + if ($removedGroupIDs !== []) { + (new RemoveUserFromGroups($user, $removedGroupIDs, ignoreUserGroupAssignments: true))(); + } } } + /** + * Removes up to `$maxRevokes` users from the user groups whose assignment + * they no longer fulfill and returns the number of removed memberships. + */ + public function revokeUsers(int $maxRevokes): int + { + $i = 0; + foreach ($this->getRevocableAssignments() as $assignment) { + $userIDs = $this->getRevocableUserIDs($assignment, $maxRevokes - $i); + if ($userIDs === []) { + continue; + } + + $userList = new UserList(); + $userList->setObjectIDs($userIDs); + $userList->readObjects(); + + foreach ($userList as $user) { + (new RemoveUserFromGroups($user, [$assignment->groupID], ignoreUserGroupAssignments: true))(); + } + + $i += \count($userIDs); + if ($i >= $maxRevokes) { + break; + } + } + + return $i; + } + + /** + * Returns whether members can be automatically removed from the given + * user group. Built-in user groups are excluded because their membership + * is managed by the system, groups that grant administrative access are + * excluded to prevent administrators from locking themselves out. + */ + public static function isRevocableGroup(UserGroup $group): bool + { + return $group->groupType === UserGroup::OTHER + && !$group->isAdminGroup() + && !(bool)$group->getGroupOption('admin.general.canUseAcp'); + } + + /** + * Returns whether the conditions exclude members of the given user group. + * Such an assignment adds every removed member again, combining it with + * the automatic revocation would repeatedly remove and add the same users. + */ + public static function excludesTargetGroup(int $groupID, ?string $conditions): bool + { + $values = \json_decode((string)$conditions, true); + if (!\is_array($values)) { + return false; + } + + $notInGroupIdentifier = (new UserNotInGroupObjectFilter())->getIdentifier(); + foreach ($values as $value) { + if ( + \is_array($value) + && ($value[0] ?? null) === $notInGroupIdentifier + && (int)($value[1] ?? 0) === $groupID + ) { + return true; + } + } + + return false; + } + /** * Returns the list of grouped user group assignment condition object types. * @@ -105,7 +196,7 @@ public function getUsers(UserGroupAssignment $assignment, ?int $maxUsers = null) $userList->sqlLimit = $maxUsers; } - (new UserGroupAssignmentObjectFilterBuilder())->applyFilters($assignment, $userList); + $this->getObjectFilterBuilder()->applyFilters($assignment, $userList); $userList->readObjects(); @@ -128,4 +219,101 @@ protected function init() $this->groupedObjectTypes[$objectType->conditiongroup][$objectType->objectTypeID] = $objectType; } } + + /** + * Returns the assignments whose user groups automatically remove members + * who no longer fulfill the assignment, indexed by the id of the user group. + * + * Automatic revocation is only supported for user groups with a single + * enabled assignment, any additional assignment for the user group would + * re-add the removed users. + * + * @return array + */ + private function getRevocableAssignments(): array + { + /** @var UserGroupAssignment[] $assignments */ + $assignments = UserGroupAssignmentCacheBuilder::getInstance()->getData(); + + $assignmentsByGroupID = []; + foreach ($assignments as $assignment) { + $assignmentsByGroupID[$assignment->groupID][] = $assignment; + } + + $revocableAssignments = []; + foreach ($assignmentsByGroupID as $groupID => $groupAssignments) { + if (\count($groupAssignments) !== 1) { + continue; + } + + $assignment = $groupAssignments[0]; + if ( + $assignment->revokeAutomatically !== 1 + || self::excludesTargetGroup($groupID, $assignment->conditions) + ) { + continue; + } + + $group = UserGroup::getGroupByID($groupID); + if ($group === null || !self::isRevocableGroup($group)) { + continue; + } + + // An assignment without active filters matches no users, revoking + // it would remove all members of the user group. + if (!$this->getObjectFilterBuilder()->applyFilters($assignment, new UserList())) { + continue; + } + + $revocableAssignments[$groupID] = $assignment; + } + + return $revocableAssignments; + } + + /** + * Returns the ids of up to `$maxUserIDs` members of the assignment's user + * group who no longer fulfill the assignment. + * + * @return list + */ + private function getRevocableUserIDs(UserGroupAssignment $assignment, int $maxUserIDs): array + { + // The filters are applied to a pseudo list to embed the resulting + // conditions in a custom query. + $pseudoUserList = new UserList(); + $this->getObjectFilterBuilder()->applyFilters($assignment, $pseudoUserList); + + $conditionBuilder = $pseudoUserList->getConditionBuilder(); + $conditionBuilder->enableWhereKeyword(false); + + $sql = "SELECT user_to_group.userID + FROM wcf1_user_to_group user_to_group + WHERE user_to_group.groupID = ? + AND NOT EXISTS ( + SELECT 1 + FROM wcf1_user user_table + LEFT JOIN wcf1_user_option_value user_option_value + ON user_option_value.userID = user_table.userID + {$pseudoUserList->sqlConditionJoins} + WHERE user_table.userID = user_to_group.userID + AND ({$conditionBuilder}) + )"; + $statement = WCF::getDB()->prepare($sql, $maxUserIDs); + $statement->execute([ + $assignment->groupID, + ...$conditionBuilder->getParameters(), + ]); + + return \array_map(static fn($userID) => (int)$userID, $statement->fetchAll(\PDO::FETCH_COLUMN)); + } + + private function getObjectFilterBuilder(): UserGroupAssignmentObjectFilterBuilder + { + if (!isset($this->objectFilterBuilder)) { + $this->objectFilterBuilder = new UserGroupAssignmentObjectFilterBuilder(); + } + + return $this->objectFilterBuilder; + } } diff --git a/wcfsetup/install/lang/de.xml b/wcfsetup/install/lang/de.xml index f3d6b593491..a0beaad24b4 100644 --- a/wcfsetup/install/lang/de.xml +++ b/wcfsetup/install/lang/de.xml @@ -864,6 +864,12 @@ Sie erreichen das Fehlerprotokoll unter: {link controller='ExceptionLogView' isE + + + + + + diff --git a/wcfsetup/install/lang/en.xml b/wcfsetup/install/lang/en.xml index 32768eb186a..8f1a9a8fe51 100644 --- a/wcfsetup/install/lang/en.xml +++ b/wcfsetup/install/lang/en.xml @@ -842,6 +842,12 @@ You can access the error log at: {link controller='ExceptionLogView' isEmail=tru + + + + + + diff --git a/wcfsetup/setup/db/install_com.woltlab.wcf.php b/wcfsetup/setup/db/install_com.woltlab.wcf.php index b195f119269..3f99455bd7d 100644 --- a/wcfsetup/setup/db/install_com.woltlab.wcf.php +++ b/wcfsetup/setup/db/install_com.woltlab.wcf.php @@ -4376,6 +4376,7 @@ NotNullVarchar255DatabaseTableColumn::create('title'), DefaultFalseBooleanDatabaseTableColumn::create('isDisabled'), JsonDatabaseTableColumn::create('conditions'), + DefaultFalseBooleanDatabaseTableColumn::create('revokeAutomatically'), ]) ->indices([ DatabaseTablePrimaryIndex::create()