From b9a3c93b0f619651dd4ef63a75622970f41fad01 Mon Sep 17 00:00:00 2001 From: James Johnson Date: Thu, 9 Jul 2026 13:17:08 -0500 Subject: [PATCH 01/11] api endpoints for drawer groups --- application/controllers/DrawerPermissions.php | 286 ++++++++++++++++++ application/libraries/SimpleValidator.php | 38 +-- application/models/Entity/DrawerGroup.php | 12 +- 3 files changed, 310 insertions(+), 26 deletions(-) create mode 100644 application/controllers/DrawerPermissions.php diff --git a/application/controllers/DrawerPermissions.php b/application/controllers/DrawerPermissions.php new file mode 100644 index 000000000..93fd0c7ac --- /dev/null +++ b/application/controllers/DrawerPermissions.php @@ -0,0 +1,286 @@ +load->library('SimpleValidator'); + $this->em = $this->doctrine->em; + } + + /** + * REST entry point for /drawerPermissions/groups[/{id}]. + * + * Trailing URL segments arrive as method args, so a request to + * /drawerPermissions/groups/5 calls this with $groupId = "5". + */ + public function groups($groupId = null): CI_Output { + $this->abortUnlessCanCreateDrawers(); + + $method = $this->input->server('REQUEST_METHOD'); + + $groupId = $groupId === null + ? null + : filter_var($groupId, FILTER_VALIDATE_INT); + + // a non-numeric id segment becomes false + if ($groupId === false) { + return abort_json(['error' => 'Invalid ID'], 400); + } + + $route = '/groups'; + if ($groupId !== null) { + $route .= '/{id}'; + } + + $table = [ + '/groups' => [ + 'GET' => fn() => $this->listGroups(), + 'POST' => fn() => $this->createGroup(), + ], + '/groups/{id}' => [ + 'GET' => fn() => $this->showGroup($groupId), + 'PUT' => fn() => $this->updateGroup($groupId), + 'PATCH' => fn() => $this->updateGroup($groupId), + 'DELETE' => fn() => $this->deleteGroup($groupId), + ], + ]; + + if (!isset($table[$route])) { + return abort_json(['error' => 'Not Found'], 404); + } + + $handler = $table[$route][$method] ?? null; + + if ($handler) { + return $handler(); + } + + // a known route, but the verb isn't allowed on it + return abort_json(['error' => 'Method Not Allowed'], 405); + } + + private function listGroups(): CI_Output { + $personalGroupId = $this->getPersonalDrawerGroup()?->getId(); + + $groups = $this->em + ->getRepository(DrawerGroup::class) + ->findBy(['user' => $this->user_model->user]); + + $isNotPersonalGroup = fn(DrawerGroup $group): bool => + $group->getId() !== $personalGroupId; + $visibleGroups = array_values(array_filter($groups, $isNotPersonalGroup)); + + return render_json(['groups' => $visibleGroups]); + } + + /** + * GET /drawerPermissions/groups/{id}: one of the user's own groups. + */ + private function showGroup(int $groupId): CI_Output { + $this->abortIfPersonalGroup($groupId); + + $group = $this->findOwnGroup($groupId); + if (!$group) { + return abort_json(['error' => 'Group not found'], 404); + } + + return render_json(['group' => $group]); + } + + /** + * POST /drawerPermissions/groups: create a "Specific People" group owned + * by the signed-in user. Members and other group types arrive in later + * slices, so the type is fixed to User here. + */ + private function createGroup(): CI_Output { + try { + $validated = V::validate($this->requestBody(), $this->groupLabelRules()); + } catch (ValidationException $e) { + return abort_json(['errors' => $e->getErrors()], 422); + } + + $group = new DrawerGroup(); + $group->setUser($this->user_model->user); + $group->setGroupType(USER_TYPE); + $group->setGroupLabel($validated['label']); + // User groups carry members as entries, so the scalar flag stays null + $group->setGroupValue(null); + + $this->em->persist($group); + $this->em->flush(); + + $this->clearUserCache(); + + return render_json(['group' => $group], 201); + } + + /** + * PUT|PATCH /drawerPermissions/groups/{id}: rename a group. Type changes + * come with the type selector in a later slice. + */ + private function updateGroup(int $groupId): CI_Output { + $this->abortIfPersonalGroup($groupId); + + $group = $this->findOwnGroup($groupId); + if (!$group) { + return abort_json(['error' => 'Group not found'], 404); + } + + try { + $validated = V::validate($this->requestBody(), $this->groupLabelRules()); + } catch (ValidationException $e) { + return abort_json(['errors' => $e->getErrors()], 422); + } + + $group->setGroupLabel($validated['label']); + $this->em->flush(); + + $this->clearUserCache(); + + return render_json(['group' => $group]); + } + + /** + * DELETE /drawerPermissions/groups/{id}: remove one of the user's groups. + */ + private function deleteGroup(int $groupId): CI_Output { + $this->abortIfPersonalGroup($groupId); + + $group = $this->findOwnGroup($groupId); + if (!$group) { + return abort_json(['error' => 'Group not found'], 404); + } + + $this->em->remove($group); + $this->em->flush(); + + $this->clearUserCache(); + + return render_json(['deleted' => $groupId]); + } + + private function abortIfPersonalGroup(int $groupId): void { + if ($groupId === $this->getPersonalDrawerGroup()?->getId()) { + abort_json(['error' => 'Group not found'], 404); + } + } + + /** + * The auto-created "personal" drawer group: a User-type group that + * includes the owner's own id as a member entry, created in Drawers.php + * the first time a user makes a drawer. It backs the user's access to + * their own drawers, so the groups API hides it and refuses to mutate it. + * + * Returns null when the user has never created a drawer, so no personal + * group exists yet. + */ + private function getPersonalDrawerGroup(): ?DrawerGroup { + $ownUserId = $this->user_model?->userId; + + if (!$ownUserId) { + return null; + } + + // we use a heuristic to find the personal drawer group. + // since the personal drawer group is created automatically the first + // time a user creates a drawer, we find the oldest user-owned group + // with the current user as a member + $query = $this->em + ->getRepository(DrawerGroup::class) + ->createQueryBuilder('drawerGroup') + ->join('drawerGroup.group_values', 'entry') + // 1. group owned by the current user + ->where('drawerGroup.user = :ownUserId') + // 2. is a `User` type group + ->andWhere('drawerGroup.group_type = :groupType') + // 3. includes the user's own id as a member entry + ->andWhere('entry.groupValue = :ownUserId') + ->setParameter('ownUserId', $ownUserId) + ->setParameter('groupType', USER_TYPE) + // The personal group is created the first time a user makes a drawer, + // so get the oldest one + ->orderBy('drawerGroup.id', 'ASC') + ->setMaxResults(1) + ->getQuery(); + + return $query->getOneOrNullResult(); + } + + /** + * Find a group by id only when the signed-in user owns it. + */ + private function findOwnGroup(int $groupId): ?DrawerGroup { + return $this->em + ->getRepository(DrawerGroup::class) + ->findOneBy([ + 'id' => $groupId, + 'user' => $this->user_model->user, + ]); + } + + /** + * Validation for a group's label. Rejects `< > "` to prevent HTML + * injection while still allowing names like `R&D` and `Bob's Team`. + */ + private function groupLabelRules(): array { + return [ + 'label' => [ + V::required(), + V::string(), + V::maxLength(255), + V::regex('/[<>"]/', 'Label cannot contain < > or " characters') + ], + ]; + } + + /** + * Abort unless the signed-in user can create drawers. Mirrors the + * capability Home exposes as userCanCreateDrawers: a super admin, an + * instance grant of at least PERM_CREATEDRAWERS, or edit access on any + * collection. Drawer groups are user-owned, so this is the gate rather + * than instance admin. + */ + private function abortUnlessCanCreateDrawers(): void { + $this->abortUnlessAuthed(); + if (!$this->canCreateDrawers()) { + abort_json(['error' => 'Forbidden'], 403); + } + } + + private function canCreateDrawers(): bool { + if ($this->user_model->getIsSuperAdmin()) { + return true; + } + if ($this->user_model->getAccessLevel('instance', $this->instance) >= PERM_CREATEDRAWERS) { + return true; + } + // getMaxCollectionPermission returns null when the user has no + // collection grants at all + return ($this->user_model->getMaxCollectionPermission() ?? 0) >= PERM_CREATEDRAWERS; + } + + /** + * Clear cached user permissions after a group mutation so the change + * takes effect immediately. + */ + private function clearUserCache(): void { + if ($this->config->item('enableCaching')) { + $this->userCache->clear(); + } + } +} diff --git a/application/libraries/SimpleValidator.php b/application/libraries/SimpleValidator.php index 66fc73ff2..92e8a1cc3 100644 --- a/application/libraries/SimpleValidator.php +++ b/application/libraries/SimpleValidator.php @@ -1,8 +1,7 @@ closure[]) * Returns filtered data or throws ValidationException @@ -26,8 +25,7 @@ class SimpleValidator * $errors = $e->getErrors(); * } */ - public static function validate(array $data, array $schema): array - { + public static function validate(array $data, array $schema): array { $errors = []; foreach ($schema as $field => $checkers) { @@ -51,63 +49,53 @@ public static function validate(array $data, array $schema): array // ----- Validators - public static function required(): \Closure - { + public static function required(): \Closure { return fn($v) => (isset($v) && $v !== '') ? true : 'This field is required'; } - public static function integer(): \Closure - { + public static function integer(): \Closure { return fn($v) => !isset($v) || filter_var($v, FILTER_VALIDATE_INT) !== false ? true : 'Must be an integer'; } - public static function array(): \Closure - { + public static function array(): \Closure { return fn($v) => !isset($v) || is_array($v) ? true : 'Must be an array'; } - public static function string(): \Closure - { + public static function string(): \Closure { return fn($v) => !isset($v) || is_string($v) ? true : 'Must be a string'; } - public static function min(int $min): \Closure - { + public static function min(int $min): \Closure { return fn($v) => !isset($v) || (is_numeric($v) && $v >= $min) ? true : "Must be at least {$min}"; } - public static function max(int $max): \Closure - { + public static function max(int $max): \Closure { return fn($v) => !isset($v) || (is_numeric($v) && $v <= $max) ? true : "Must not exceed {$max}"; } - public static function regex(string $pattern): \Closure - { + public static function regex(string $pattern, $errorMessage = 'Invalid format.'): \Closure { return fn($v) => !isset($v) || preg_match($pattern, $v) ? true - : "Invalid format."; + : $errorMessage; } - public static function minLength(int $length): \Closure - { + public static function minLength(int $length): \Closure { return fn($v) => !isset($v) || (is_string($v) && mb_strlen($v) >= $length) ? true : "Must be at least {$length} characters long"; } - public static function maxLength(int $length): \Closure - { + public static function maxLength(int $length): \Closure { return fn($v) => !isset($v) || (is_string($v) && mb_strlen($v) <= $length) ? true : "Must be {$length} characters or fewer"; } - public static function json(): \Closure - { + public static function json(): \Closure { return function ($v) { if (!isset($v)) return true; if (!is_string($v)) return 'Must be valid JSON'; diff --git a/application/models/Entity/DrawerGroup.php b/application/models/Entity/DrawerGroup.php index 60742b3d6..e65ec970e 100644 --- a/application/models/Entity/DrawerGroup.php +++ b/application/models/Entity/DrawerGroup.php @@ -9,7 +9,7 @@ */ #[ORM\Table(name: 'drawer_groups')] #[ORM\Entity] -class DrawerGroup +class DrawerGroup implements \JsonSerializable { /** * @var string|null @@ -324,4 +324,14 @@ public function getGroupValues() { return $this->group_values; } + + public function jsonSerialize(): array { + return [ + 'id' => $this->id, + 'type' => $this->group_type, + 'label' => $this->group_label, + // n+1 query, but prob fine at our scale + 'entries_count' => $this->group_values->count(), + ]; + } } From 798733ed23e2e471e0e706915b69b3ed55ce6c18 Mon Sep 17 00:00:00 2001 From: James Johnson Date: Thu, 9 Jul 2026 13:27:55 -0500 Subject: [PATCH 02/11] fix regex validation --- application/controllers/DrawerPermissions.php | 3 +- application/libraries/SimpleValidator.php | 30 +++++++++++++++++-- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/application/controllers/DrawerPermissions.php b/application/controllers/DrawerPermissions.php index 93fd0c7ac..1f3c3d778 100644 --- a/application/controllers/DrawerPermissions.php +++ b/application/controllers/DrawerPermissions.php @@ -241,9 +241,8 @@ private function groupLabelRules(): array { return [ 'label' => [ V::required(), - V::string(), V::maxLength(255), - V::regex('/[<>"]/', 'Label cannot contain < > or " characters') + V::notRegex('/[<>"]/', 'Label cannot contain < > or " characters') ], ]; } diff --git a/application/libraries/SimpleValidator.php b/application/libraries/SimpleValidator.php index 92e8a1cc3..d66164ebd 100644 --- a/application/libraries/SimpleValidator.php +++ b/application/libraries/SimpleValidator.php @@ -78,9 +78,33 @@ public static function max(int $max): \Closure { } public static function regex(string $pattern, $errorMessage = 'Invalid format.'): \Closure { - return fn($v) => !isset($v) || preg_match($pattern, $v) - ? true - : $errorMessage; + return function ($v) use ($pattern, $errorMessage) { + // absent stays valid, use required() to enforce presence + if (!isset($v)) { + return true; + } + // a non-string can't match a pattern, so report it rather than + // letting preg_match throw a TypeError + if (!is_string($v)) { + return $errorMessage; + } + return preg_match($pattern, $v) ? true : $errorMessage; + }; + } + + public static function notRegex(string $pattern, $errorMessage = 'String is invalid format.'): \Closure { + return function ($v) use ($pattern, $errorMessage) { + // absent stays valid, use required() to enforce presence + if (!isset($v)) { + return true; + } + // a non-string has no forbidden content to match, but the wrong + // type is still a failure, so report it rather than let it pass + if (!is_string($v)) { + return $errorMessage; + } + return preg_match($pattern, $v) ? $errorMessage : true; + }; } public static function minLength(int $length): \Closure { From f43231fb0e72ba545eef74ad02dc2769a0e09acc Mon Sep 17 00:00:00 2001 From: James Johnson Date: Thu, 9 Jul 2026 14:09:12 -0500 Subject: [PATCH 03/11] only clear current user cache when a group is created --- application/controllers/DrawerPermissions.php | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/application/controllers/DrawerPermissions.php b/application/controllers/DrawerPermissions.php index 1f3c3d778..2c541174c 100644 --- a/application/controllers/DrawerPermissions.php +++ b/application/controllers/DrawerPermissions.php @@ -94,7 +94,7 @@ private function listGroups(): CI_Output { private function showGroup(int $groupId): CI_Output { $this->abortIfPersonalGroup($groupId); - $group = $this->findOwnGroup($groupId); + $group = $this->findCurrentUserDrawerGroup($groupId); if (!$group) { return abort_json(['error' => 'Group not found'], 404); } @@ -124,7 +124,7 @@ private function createGroup(): CI_Output { $this->em->persist($group); $this->em->flush(); - $this->clearUserCache(); + $this->removeCurrentUserCache(); return render_json(['group' => $group], 201); } @@ -136,7 +136,7 @@ private function createGroup(): CI_Output { private function updateGroup(int $groupId): CI_Output { $this->abortIfPersonalGroup($groupId); - $group = $this->findOwnGroup($groupId); + $group = $this->findCurrentUserDrawerGroup($groupId); if (!$group) { return abort_json(['error' => 'Group not found'], 404); } @@ -150,7 +150,7 @@ private function updateGroup(int $groupId): CI_Output { $group->setGroupLabel($validated['label']); $this->em->flush(); - $this->clearUserCache(); + $this->removeCurrentUserCache(); return render_json(['group' => $group]); } @@ -161,7 +161,7 @@ private function updateGroup(int $groupId): CI_Output { private function deleteGroup(int $groupId): CI_Output { $this->abortIfPersonalGroup($groupId); - $group = $this->findOwnGroup($groupId); + $group = $this->findCurrentUserDrawerGroup($groupId); if (!$group) { return abort_json(['error' => 'Group not found'], 404); } @@ -169,7 +169,9 @@ private function deleteGroup(int $groupId): CI_Output { $this->em->remove($group); $this->em->flush(); - $this->clearUserCache(); + // deleting a group, will cascade deletes to members and entries + // changing multiple user permissions. Clear it all. + $this->clearAllUserCache(); return render_json(['deleted' => $groupId]); } @@ -224,7 +226,7 @@ private function getPersonalDrawerGroup(): ?DrawerGroup { /** * Find a group by id only when the signed-in user owns it. */ - private function findOwnGroup(int $groupId): ?DrawerGroup { + private function findCurrentUserDrawerGroup(int $groupId): ?DrawerGroup { return $this->em ->getRepository(DrawerGroup::class) ->findOneBy([ @@ -273,13 +275,13 @@ private function canCreateDrawers(): bool { return ($this->user_model->getMaxCollectionPermission() ?? 0) >= PERM_CREATEDRAWERS; } - /** - * Clear cached user permissions after a group mutation so the change - * takes effect immediately. - */ - private function clearUserCache(): void { - if ($this->config->item('enableCaching')) { - $this->userCache->clear(); - } + private function removeCurrentUserCache(): void { + if (!$this->config->item('enableCaching')) return; + $this->userCache->delete($this->user_model->userId); + } + + private function clearAllUserCache(): void { + if (!$this->config->item('enableCaching')) return; + $this->userCache->clear(); } } From 15dbfd7cdd34971ffacac155433b8fb38648c8cd Mon Sep 17 00:00:00 2001 From: James Johnson Date: Thu, 9 Jul 2026 14:13:49 -0500 Subject: [PATCH 04/11] remove unused --- application/controllers/DrawerPermissions.php | 1 - 1 file changed, 1 deletion(-) diff --git a/application/controllers/DrawerPermissions.php b/application/controllers/DrawerPermissions.php index 2c541174c..96f67aa88 100644 --- a/application/controllers/DrawerPermissions.php +++ b/application/controllers/DrawerPermissions.php @@ -2,7 +2,6 @@ if (! defined('BASEPATH')) exit('No direct script access allowed'); use Doctrine\ORM\EntityManager; -use Doctrine\ORM\Query\Expr; use Entity\DrawerGroup; use SimpleValidator as V; From 3a9b43344aa16db56e959e78bfcce32688d9f9fd Mon Sep 17 00:00:00 2001 From: James Johnson Date: Thu, 9 Jul 2026 16:31:41 -0500 Subject: [PATCH 05/11] Add GroupTypeCatalog --- application/libraries/GroupTypeCatalog.php | 103 +++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 application/libraries/GroupTypeCatalog.php diff --git a/application/libraries/GroupTypeCatalog.php b/application/libraries/GroupTypeCatalog.php new file mode 100644 index 000000000..ee9beee35 --- /dev/null +++ b/application/libraries/GroupTypeCatalog.php @@ -0,0 +1,103 @@ +user_model->getAuthHelper()->authTypes); + * if (!$catalog->isValid($type)) { ... } + * $group->setGroupValue($catalog->ignoresGroupValues($type) ? 1 : null); + * ``` + */ +class GroupTypeCatalog { + // The built-in types every instance has. `populationWide` marks the + // types that match a whole population rather than a chosen list. That + // single fact drives two predicates: such a group ignores group values + // (it has no entries), and only an instance admin may create one + // (sharing across a whole population is an admin decision). + private const GLOBAL_TYPES = [ + ALL_TYPE => [ + "name" => ALL_TYPE, + "label" => "All", + "helpText" => "Everyone, including signed-out visitors.", + "populationWide" => true, + ], + AUTHED_TYPE => [ + "name" => AUTHED_TYPE, + "label" => "Authenticated Users", + "helpText" => "Anyone signed in, by any login method.", + "populationWide" => true, + ], + REMOTE_TYPE => [ + "name" => REMOTE_TYPE, + "label" => "Centrally Authenticated Users", + "helpText" => "Users signed in through central " + . "single sign-on (SSO).", + "populationWide" => true, + ], + USER_TYPE => [ + "name" => USER_TYPE, + "label" => "Specific People", + "helpText" => "Specific people you choose. Add by name, email, or username.", + ], + ]; + + private array $authHelperTypes; + + /** + * @param array $authHelperTypes the instance auth helper's authTypes, + * keyed by type string (e.g. Unit, JobCode). Empty for helperless + * instances, which then offer only the built-in globals. + */ + public function __construct(array $authHelperTypes = []) { + $this->authHelperTypes = $authHelperTypes; + } + + /** + * Every type this instance offers, keyed by type string: the built-in + * globals first, then the auth helper's own types. + */ + public function all(): array { + return [ + ...self::GLOBAL_TYPES, + ...$this->authHelperTypes, + ]; + } + + public function isValid(string $type): bool { + return isset($this->all()[$type]); + } + + /** + * Whether $type comes from the auth helper rather than the built-in + * globals. Auth-helper groups match users on their value entries. The + * globals never do: User matches member ids, the rest whole populations. + */ + public function isAuthHelperType(string $type): bool { + return !isset(self::GLOBAL_TYPES[$type]); + } + + /** + * Whether $type matches a whole population instead of a values list + * (All/Authed/Authed_remote). Such a group carries a vestigial scalar + * group_value of 1 and holds no entries. + */ + public function ignoresGroupValues(string $type): bool { + return self::GLOBAL_TYPES[$type]["populationWide"] ?? false; + } + + /** + * Whether only an instance admin may put a group on $type. The + * population-wide types are admin only, every other type is open to any + * group owner. + */ + public function isAdminOnly(string $type): bool { + return $this->ignoresGroupValues($type); + } +} From c664fb27b81a8c35152d046249a35b34a0b57a01 Mon Sep 17 00:00:00 2001 From: James Johnson Date: Fri, 10 Jul 2026 09:38:06 -0500 Subject: [PATCH 06/11] Extract GroupTypeCatalog and expand DrawerPermissions --- application/controllers/AdminPermissions.php | 76 +------ application/controllers/DrawerPermissions.php | 198 +++++++++++++++--- application/libraries/GroupTypeCatalog.php | 20 +- 3 files changed, 186 insertions(+), 108 deletions(-) diff --git a/application/controllers/AdminPermissions.php b/application/controllers/AdminPermissions.php index 3bb1b750c..ab75dd182 100644 --- a/application/controllers/AdminPermissions.php +++ b/application/controllers/AdminPermissions.php @@ -12,43 +12,15 @@ * pure json api for new ui */ class AdminPermissions extends Instance_Controller { - // mirrors the structure of AuthHelpers::$authTypes, - // @see UMNHelper:$authTypes for an example - const GLOBAL_GROUP_TYPES = [ - ALL_TYPE => [ - "name" => ALL_TYPE, - "label" => "All", - "helpText" => "Everyone, including signed-out visitors.", - // vestigial group_value - "ignoresGroupValues" => true, - ], - AUTHED_TYPE => [ - "name" => AUTHED_TYPE, - "label" => "Authenticated Users", - "helpText" => "Anyone signed in, by any login method.", - "ignoresGroupValues" => true, - ], - REMOTE_TYPE => [ - "name" => REMOTE_TYPE, - "label" => "Centrally Authenticated Users", - "helpText" => "Users signed in through central " - . "single sign-on (SSO).", - "ignoresGroupValues" => true, - ], - USER_TYPE => [ - "name" => USER_TYPE, - "label" => "Specific People", - "helpText" => "Specific people you choose. Add by name, email, or username.", - ], - ]; - private AuthHelper $authHelper; + private GroupTypeCatalog $groupTypeCatalog; private EntityManager $em; public function __construct() { parent::__construct(); $this->load->library('SimpleValidator'); $this->authHelper = $this->user_model->getAuthHelper(); + $this->groupTypeCatalog = new GroupTypeCatalog($this->authHelper->authTypes); $this->em = $this->doctrine->em; } @@ -63,7 +35,7 @@ public function groupTypes() { "description" => $t["helpText"] ?? "", "entryHints" => $this->entryHintsForType($t["name"]), ], - array_values($this->getGroupTypes()) + array_values($this->groupTypeCatalog->all()) ); return render_json(["groupTypes" => $groupTypes]); @@ -83,7 +55,7 @@ private function entryHintsForType(string $type): array { // Global types never take entries. Guard by type category rather // than trusting that no auth helper ever keys its userData by a // global type name, since helpers pick their keys independently. - if (!$this->isAuthHelperGroupType($type)) { + if (!$this->groupTypeCatalog->isAuthHelperType($type)) { return []; } @@ -291,7 +263,7 @@ private function updateGroup(int $groupId) { foreach ($group->getGroupValues()->toArray() as $entry) { $group->removeGroupValue($entry); // orphanRemoval deletes on flush } - $group->setGroupValue($this->ignoresGroupValues($newType) ? 1 : null); + $group->setGroupValue($this->groupTypeCatalog->ignoresGroupValues($newType) ? 1 : null); } $this->doctrine->em->flush(); @@ -486,19 +458,12 @@ private function memberPayload(Entity\User $user): array { ]; } - private function getGroupTypes(): array { - return [ - ...self::GLOBAL_GROUP_TYPES, - ...$this->authHelper->authTypes, - ]; - } - /** * Shared validation rules for a group's editable attributes (label + * type). createGroup adds its own `values` rule on top of these. */ private function groupAttributeRules(): array { - $validTypes = array_keys($this->getGroupTypes()); + $validTypes = array_keys($this->groupTypeCatalog->all()); return [ 'type' => [ V::required(), @@ -537,7 +502,7 @@ private function createGroup() { $group->setGroupType($type); $group->setGroupLabel($validated['label']); - if ($this->ignoresGroupValues($type)) { + if ($this->groupTypeCatalog->ignoresGroupValues($type)) { // vestigial scalar, must be 1: Authed/Authed_remote match on it $group->setGroupValue(1); } else { @@ -576,25 +541,6 @@ private function createGroup() { return render_json(['group' => $group], 201); } - /** - * Whether `$type` comes from the instance's AuthHelper rather than the - * built-in GLOBAL_GROUP_TYPES. Auth-helper groups match users on their - * value entries; the built-ins never do (User matches member ids, the - * rest match whole populations). - */ - private function isAuthHelperGroupType(string $type): bool { - return !isset(self::GLOBAL_GROUP_TYPES[$type]); - } - - /** - * Whether `$type` matches a whole population instead of a values - * list (All/Authed/Authed_remote). - */ - private function ignoresGroupValues(string $type): bool { - // auth-helper types always carry values, so absence means false - return self::GLOBAL_GROUP_TYPES[$type]['ignoresGroupValues'] ?? false; - } - /** * Find a remote user within the local DB by their remote id * (e.g. username, umndid). If not found, creates a new @@ -648,7 +594,7 @@ private function listGroupEntries(int $groupId): CI_Output { return abort_json(['error' => 'Group not found'], 404); } - if (!$this->isAuthHelperGroupType($group->getGroupType())) { + if (!$this->groupTypeCatalog->isAuthHelperType($group->getGroupType())) { return abort_json( ['error' => 'Only auth-helper group types take entries'], 422 @@ -672,7 +618,7 @@ private function addGroupEntry(int $groupId): CI_Output { return abort_json(['error' => 'Group not found'], 404); } - if (!$this->isAuthHelperGroupType($group->getGroupType())) { + if (!$this->groupTypeCatalog->isAuthHelperType($group->getGroupType())) { return abort_json( ['error' => 'Only auth-helper group types take entries'], 422 @@ -710,7 +656,7 @@ private function updateGroupEntry(int $groupId, int $entryId): CI_Output { return abort_json(['error' => 'Group not found'], 404); } - if (!$this->isAuthHelperGroupType($group->getGroupType())) { + if (!$this->groupTypeCatalog->isAuthHelperType($group->getGroupType())) { return abort_json( ['error' => 'Only auth-helper group types take entries'], 422 @@ -751,7 +697,7 @@ private function removeGroupEntry(int $groupId, int $entryId): CI_Output { return abort_json(['error' => 'Group not found'], 404); } - if (!$this->isAuthHelperGroupType($group->getGroupType())) { + if (!$this->groupTypeCatalog->isAuthHelperType($group->getGroupType())) { return abort_json( ['error' => 'Only auth-helper group types take entries'], 422 diff --git a/application/controllers/DrawerPermissions.php b/application/controllers/DrawerPermissions.php index 96f67aa88..df7da3f3e 100644 --- a/application/controllers/DrawerPermissions.php +++ b/application/controllers/DrawerPermissions.php @@ -2,6 +2,7 @@ if (! defined('BASEPATH')) exit('No direct script access allowed'); use Doctrine\ORM\EntityManager; +use Entity\Drawer; use Entity\DrawerGroup; use SimpleValidator as V; @@ -10,17 +11,60 @@ * * Drawer groups are owned by a user, unlike instance groups which belong * to the instance. So every action is scoped to the signed-in user's own - * groups and gated on "can create drawers", not instance admin. + * groups and gated on "can manage drawers", not instance admin. */ class DrawerPermissions extends Instance_Controller { + private GroupTypeCatalog $groupTypeCatalog; private EntityManager $em; public function __construct() { parent::__construct(); $this->load->library('SimpleValidator'); + $this->groupTypeCatalog = new GroupTypeCatalog( + $this->user_model->getAuthHelper()->authTypes + ); $this->em = $this->doctrine->em; } + /** + * GET /drawerPermissions/groupTypes: the catalog for the type selector. + * Admin-only types stay in the list so non-admin UIs can show them + * disabled, matching the legacy form. + */ + public function groupTypes(): CI_Output { + $this->abortUnlessCanManageDrawers(); + + $groupTypes = array_map( + fn($type) => [ + "type" => $type["name"], + "label" => $type["label"], + "description" => $type["helpText"] ?? "", + "adminOnly" => $this->groupTypeCatalog->isAdminOnly($type["name"]), + ], + array_values($this->groupTypeCatalog->all()) + ); + + return render_json(["groupTypes" => $groupTypes]); + } + + /** + * GET /drawerPermissions/drawers: drawers the signed-in user can + * manage, for the management page's Drawers tab. + */ + public function drawers(): CI_Output { + $this->abortUnlessCanManageDrawers(); + + $drawerPayload = array_map( + fn(Drawer $drawer) => [ + 'id' => $drawer->getId(), + 'title' => $drawer->getTitle(), + ], + $this->manageableDrawers() + ); + + return render_json(['drawers' => $drawerPayload]); + } + /** * REST entry point for /drawerPermissions/groups[/{id}]. * @@ -28,7 +72,7 @@ public function __construct() { * /drawerPermissions/groups/5 calls this with $groupId = "5". */ public function groups($groupId = null): CI_Output { - $this->abortUnlessCanCreateDrawers(); + $this->abortUnlessCanManageDrawers(); $method = $this->input->server('REQUEST_METHOD'); @@ -102,23 +146,35 @@ private function showGroup(int $groupId): CI_Output { } /** - * POST /drawerPermissions/groups: create a "Specific People" group owned - * by the signed-in user. Members and other group types arrive in later - * slices, so the type is fixed to User here. + * POST /drawerPermissions/groups: create a group owned by the + * signed-in user. Members and value entries arrive in later slices, + * so the group starts empty. */ private function createGroup(): CI_Output { try { - $validated = V::validate($this->requestBody(), $this->groupLabelRules()); + $validated = V::validate( + $this->requestBody(), + $this->groupAttributeRules() + ); } catch (ValidationException $e) { return abort_json(['errors' => $e->getErrors()], 422); } + $type = $validated['type']; + if (!$this->canUseGroupType($type)) { + return abort_json( + ['error' => 'Only instance admins can use instance-wide group types'], + 403 + ); + } + $group = new DrawerGroup(); $group->setUser($this->user_model->user); - $group->setGroupType(USER_TYPE); + $group->setGroupType($type); $group->setGroupLabel($validated['label']); - // User groups carry members as entries, so the scalar flag stays null - $group->setGroupValue(null); + // population types match on the vestigial scalar (must be 1), every + // other type carries its members or values as entries + $group->setGroupValue($this->groupTypeCatalog->ignoresGroupValues($type) ? 1 : null); $this->em->persist($group); $this->em->flush(); @@ -129,8 +185,11 @@ private function createGroup(): CI_Output { } /** - * PUT|PATCH /drawerPermissions/groups/{id}: rename a group. Type changes - * come with the type selector in a later slice. + * PUT|PATCH /drawerPermissions/groups/{id}: edit a group's label and + * type. + * + * Changing the type clears existing entries, since they belong to the + * old type. Editing only the label leaves them alone. */ private function updateGroup(int $groupId): CI_Output { $this->abortIfPersonalGroup($groupId); @@ -141,15 +200,49 @@ private function updateGroup(int $groupId): CI_Output { } try { - $validated = V::validate($this->requestBody(), $this->groupLabelRules()); + $validated = V::validate( + $this->requestBody(), + $this->groupAttributeRules() + ); } catch (ValidationException $e) { return abort_json(['errors' => $e->getErrors()], 422); } + $newType = $validated['type']; + $hasTypeChanged = $newType !== $group->getGroupType(); + + // a non-admin may keep an admin-only type it already has (rename + // only), but may not switch a group onto one + if ($hasTypeChanged && !$this->canUseGroupType($newType)) { + return abort_json( + ['error' => 'Only instance admins can use instance-wide group types'], + 403 + ); + } + $group->setGroupLabel($validated['label']); + + if ($hasTypeChanged) { + $group->setGroupType($newType); + + // a type change invalidates old entries, so clear them. toArray() + // copies first so removing entries does not mutate the list + // mid-loop. + foreach ($group->getGroupValues()->toArray() as $entry) { + $group->removeGroupValue($entry); + } + $group->setGroupValue($this->groupTypeCatalog->ignoresGroupValues($newType) ? 1 : null); + } + $this->em->flush(); - $this->removeCurrentUserCache(); + if ($hasTypeChanged) { + // clearing the members revokes their drawer access, so their + // cached permissions are stale too, not just the owner's + $this->clearAllUserCache(); + } else { + $this->removeCurrentUserCache(); + } return render_json(['group' => $group]); } @@ -235,11 +328,19 @@ private function findCurrentUserDrawerGroup(int $groupId): ?DrawerGroup { } /** - * Validation for a group's label. Rejects `< > "` to prevent HTML - * injection while still allowing names like `R&D` and `Bob's Team`. + * Validation rules for a group's editable attributes (label + type). + * The label rejects `< > "` to prevent HTML injection while still + * allowing names like `R&D` and `Bob's Team`. */ - private function groupLabelRules(): array { + private function groupAttributeRules(): array { + $validTypes = array_keys($this->groupTypeCatalog->all()); return [ + 'type' => [ + V::required(), + fn($value) => !isset($value) || in_array($value, $validTypes, true) + ? true + : 'Unknown group type', + ], 'label' => [ V::required(), V::maxLength(255), @@ -249,20 +350,61 @@ private function groupLabelRules(): array { } /** - * Abort unless the signed-in user can create drawers. Mirrors the - * capability Home exposes as userCanCreateDrawers: a super admin, an - * instance grant of at least PERM_CREATEDRAWERS, or edit access on any - * collection. Drawer groups are user-owned, so this is the gate rather - * than instance admin. + * Whether the signed-in user may put a group on `$type`: + * population-wide types are reserved for instance admins. + */ + private function canUseGroupType(string $type): bool { + if (!$this->groupTypeCatalog->isAdminOnly($type)) { + return true; + } + return $this->user_model->isInstanceAdmin() + || $this->user_model->getIsSuperAdmin(); + } + + /** + * Drawers the signed-in user holds at least PERM_CREATEDRAWERS on. + * + * Instance admins hold 60 on every drawer in their instance through a + * fallback in getAccessLevel, not through the drawerPermissions map, + * so they get the whole instance's drawers here. */ - private function abortUnlessCanCreateDrawers(): void { + private function manageableDrawers(): array { + $drawerRepository = $this->em->getRepository(Drawer::class); + + $isEveryDrawerManageable = $this->user_model->isInstanceAdmin() + || $this->user_model->getIsSuperAdmin(); + if ($isEveryDrawerManageable) { + return $drawerRepository->findBy( + ['instance' => $this->instance], + ['title' => 'ASC'] + ); + } + + $manageableDrawerIds = array_keys(array_filter( + $this->user_model->drawerPermissions, + fn($level) => $level >= PERM_CREATEDRAWERS + )); + if (count($manageableDrawerIds) === 0) { + return []; + } + + return $drawerRepository->findBy( + ['id' => $manageableDrawerIds, 'instance' => $this->instance], + ['title' => 'ASC'] + ); + } + + /** + * Abort unless the signed-in user can manage at least one drawer. + */ + private function abortUnlessCanManageDrawers(): void { $this->abortUnlessAuthed(); - if (!$this->canCreateDrawers()) { + if (!$this->canManageDrawers()) { abort_json(['error' => 'Forbidden'], 403); } } - private function canCreateDrawers(): bool { + private function canManageDrawers(): bool { if ($this->user_model->getIsSuperAdmin()) { return true; } @@ -271,7 +413,13 @@ private function canCreateDrawers(): bool { } // getMaxCollectionPermission returns null when the user has no // collection grants at all - return ($this->user_model->getMaxCollectionPermission() ?? 0) >= PERM_CREATEDRAWERS; + if (($this->user_model->getMaxCollectionPermission() ?? 0) >= PERM_CREATEDRAWERS) { + return true; + } + // the leading 0 keeps max() legal when the user holds no drawer + // grants at all + $drawerGrantLevels = array_values($this->user_model->drawerPermissions); + return max([0, ...$drawerGrantLevels]) >= PERM_CREATEDRAWERS; } private function removeCurrentUserCache(): void { diff --git a/application/libraries/GroupTypeCatalog.php b/application/libraries/GroupTypeCatalog.php index ee9beee35..cf486f510 100644 --- a/application/libraries/GroupTypeCatalog.php +++ b/application/libraries/GroupTypeCatalog.php @@ -1,26 +1,10 @@ user_model->getAuthHelper()->authTypes); - * if (!$catalog->isValid($type)) { ... } - * $group->setGroupValue($catalog->ignoresGroupValues($type) ? 1 : null); - * ``` - */ class GroupTypeCatalog { // The built-in types every instance has. `populationWide` marks the - // types that match a whole population rather than a chosen list. That - // single fact drives two predicates: such a group ignores group values - // (it has no entries), and only an instance admin may create one - // (sharing across a whole population is an admin decision). + // types that match a whole population rather than a chosen list, which + // means they have no entries. private const GLOBAL_TYPES = [ ALL_TYPE => [ "name" => ALL_TYPE, From 462bb2ac4df65bb7be1594275b37ed85b1de039a Mon Sep 17 00:00:00 2001 From: James Johnson Date: Fri, 10 Jul 2026 13:31:19 -0500 Subject: [PATCH 07/11] remove values stuff in group creation --- application/controllers/AdminPermissions.php | 70 +----------------- application/controllers/DrawerPermissions.php | 72 +++++++++++-------- application/core/Instance_Controller.php | 39 ++++++++++ application/libraries/GroupTypeCatalog.php | 3 +- 4 files changed, 86 insertions(+), 98 deletions(-) diff --git a/application/controllers/AdminPermissions.php b/application/controllers/AdminPermissions.php index ab75dd182..2a9ff501d 100644 --- a/application/controllers/AdminPermissions.php +++ b/application/controllers/AdminPermissions.php @@ -34,6 +34,7 @@ public function groupTypes() { "label" => $t["label"], "description" => $t["helpText"] ?? "", "entryHints" => $this->entryHintsForType($t["name"]), + "adminOnly" => $this->groupTypeCatalog->isAdminOnly($t["name"]), ], array_values($this->groupTypeCatalog->all()) ); @@ -460,7 +461,7 @@ private function memberPayload(Entity\User $user): array { /** * Shared validation rules for a group's editable attributes (label + - * type). createGroup adds its own `values` rule on top of these. + * type), used by both createGroup and updateGroup. */ private function groupAttributeRules(): array { $validTypes = array_keys($this->groupTypeCatalog->all()); @@ -503,34 +504,10 @@ private function createGroup() { $group->setGroupLabel($validated['label']); if ($this->groupTypeCatalog->ignoresGroupValues($type)) { - // vestigial scalar, must be 1: Authed/Authed_remote match on it + // must be 1 for legacy reasons $group->setGroupValue(1); } else { $group->setGroupValue(null); - - $nonEmptyValues = array_filter( - (array) ($validated['values'] ?? []), - fn($v) => $v !== '' - ); - - // create a GroupEntry for each value - foreach ($nonEmptyValues as $value) { - // a non-numeric User value is a remote auth-system id; resolve - // it to a local user id. other types store their value as-is - if ($type === USER_TYPE && !is_numeric($value)) { - // a non-numeric value is a remote username; provision its - // local row and store the resulting user id - try { - $value = $this->firstOrProvisionRemoteUser($value)->getId(); - } catch (RemoteUserNotFoundException $e) { - return abort_json(['error' => $e->getMessage()], 404); - } - } - - $entry = new Entity\GroupEntry(); - $entry->setGroupValue($value); - $group->addGroupValue($entry); - } } $this->doctrine->em->persist($group); @@ -541,47 +518,6 @@ private function createGroup() { return render_json(['group' => $group], 201); } - /** - * Find a remote user within the local DB by their remote id - * (e.g. username, umndid). If not found, creates a new - * user in the local DB with the remoteUserId set. - * - * @throws RemoteUserNotFoundException if the user cannot be found or - * provisioned. - * @return Entity\User the user record matching the remote id - */ - private function firstOrProvisionRemoteUser(string $remoteUserId): Entity\User { - // findById($id, true) will make new (unsaved) an Entity\User record with - // the given remote id if nothing is found. - /** @var ?Entity\User $remoteUser */ - $remoteUser = $this->authHelper->findById($remoteUserId, true)[0] ?? null; - - if ($remoteUser === null) { - throw new RemoteUserNotFoundException($remoteUserId); - } - - // does a user already exist in the local DB with this username? - /** @var ?Entity\User $existingUser */ - $existingUser = $this->doctrine->em->getRepository(Entity\User::class) - ->findOneBy(["username" => $remoteUser->getUsername()]); - - // if so, return it instead of the new unsaved one - if ($existingUser !== null) { - return $existingUser; - } - - // otherwise, fill in some blanks in the new record - $remoteUser->setUserType("Remote"); - $remoteUser->setCreatedAt(new \DateTime("now")); - $remoteUser->setInstance($this->instance); - - // and then save it - $this->doctrine->em->persist($remoteUser); - $this->doctrine->em->flush(); - - return $remoteUser; - } - /** * GET /adminPermissions/groups/{id}/entries: a group's raw match values. */ diff --git a/application/controllers/DrawerPermissions.php b/application/controllers/DrawerPermissions.php index df7da3f3e..80f8024c2 100644 --- a/application/controllers/DrawerPermissions.php +++ b/application/controllers/DrawerPermissions.php @@ -7,7 +7,7 @@ use SimpleValidator as V; /** - * pure json api for the new drawer groups ui. + * JSON api for the drawer group/permission management ui. * * Drawer groups are owned by a user, unlike instance groups which belong * to the instance. So every action is scoped to the signed-in user's own @@ -39,6 +39,8 @@ public function groupTypes(): CI_Output { "type" => $type["name"], "label" => $type["label"], "description" => $type["helpText"] ?? "", + // TODO: real hints, see AdminPermissions::entryHintsForType + "entryHints" => [], "adminOnly" => $this->groupTypeCatalog->isAdminOnly($type["name"]), ], array_values($this->groupTypeCatalog->all()) @@ -113,7 +115,6 @@ public function groups($groupId = null): CI_Output { return $handler(); } - // a known route, but the verb isn't allowed on it return abort_json(['error' => 'Method Not Allowed'], 405); } @@ -147,8 +148,7 @@ private function showGroup(int $groupId): CI_Output { /** * POST /drawerPermissions/groups: create a group owned by the - * signed-in user. Members and value entries arrive in later slices, - * so the group starts empty. + * signed-in user, with any initial member or match values. */ private function createGroup(): CI_Output { try { @@ -172,9 +172,35 @@ private function createGroup(): CI_Output { $group->setUser($this->user_model->user); $group->setGroupType($type); $group->setGroupLabel($validated['label']); - // population types match on the vestigial scalar (must be 1), every - // other type carries its members or values as entries - $group->setGroupValue($this->groupTypeCatalog->ignoresGroupValues($type) ? 1 : null); + + if ($this->groupTypeCatalog->ignoresGroupValues($type)) { + // must be 1 for legacy reasons + $group->setGroupValue(1); + } else { + $group->setGroupValue(null); + + $nonEmptyValues = array_filter( + (array) ($validated['values'] ?? []), + fn($value) => $value !== '' + ); + + foreach ($nonEmptyValues as $value) { + // a non-numeric User value is a remote username. Provision its + // local row and store the resulting user id. Other types store + // their value as-is. + if ($type === USER_TYPE && !is_numeric($value)) { + try { + $value = $this->firstOrProvisionRemoteUser($value)->getId(); + } catch (RemoteUserNotFoundException $e) { + return abort_json(['error' => $e->getMessage()], 404); + } + } + + $entry = new \Entity\GroupEntry(); + $entry->setGroupValue($value); + $group->addGroupValue($entry); + } + } $this->em->persist($group); $this->em->flush(); @@ -225,9 +251,8 @@ private function updateGroup(int $groupId): CI_Output { if ($hasTypeChanged) { $group->setGroupType($newType); - // a type change invalidates old entries, so clear them. toArray() - // copies first so removing entries does not mutate the list - // mid-loop. + // toArray() copies first so removing entries does not mutate the + // list mid-loop foreach ($group->getGroupValues()->toArray() as $entry) { $group->removeGroupValue($entry); } @@ -261,8 +286,8 @@ private function deleteGroup(int $groupId): CI_Output { $this->em->remove($group); $this->em->flush(); - // deleting a group, will cascade deletes to members and entries - // changing multiple user permissions. Clear it all. + // deleting a group cascades to its members and entries, changing + // many users' permissions, so clear every user's cache $this->clearAllUserCache(); return render_json(['deleted' => $groupId]); @@ -290,24 +315,17 @@ private function getPersonalDrawerGroup(): ?DrawerGroup { return null; } - // we use a heuristic to find the personal drawer group. - // since the personal drawer group is created automatically the first - // time a user creates a drawer, we find the oldest user-owned group - // with the current user as a member + // heuristic: the oldest User-type group the user owns that lists + // the user's own id as a member $query = $this->em ->getRepository(DrawerGroup::class) ->createQueryBuilder('drawerGroup') ->join('drawerGroup.group_values', 'entry') - // 1. group owned by the current user ->where('drawerGroup.user = :ownUserId') - // 2. is a `User` type group ->andWhere('drawerGroup.group_type = :groupType') - // 3. includes the user's own id as a member entry ->andWhere('entry.groupValue = :ownUserId') ->setParameter('ownUserId', $ownUserId) ->setParameter('groupType', USER_TYPE) - // The personal group is created the first time a user makes a drawer, - // so get the oldest one ->orderBy('drawerGroup.id', 'ASC') ->setMaxResults(1) ->getQuery(); @@ -315,9 +333,6 @@ private function getPersonalDrawerGroup(): ?DrawerGroup { return $query->getOneOrNullResult(); } - /** - * Find a group by id only when the signed-in user owns it. - */ private function findCurrentUserDrawerGroup(int $groupId): ?DrawerGroup { return $this->em ->getRepository(DrawerGroup::class) @@ -364,9 +379,9 @@ private function canUseGroupType(string $type): bool { /** * Drawers the signed-in user holds at least PERM_CREATEDRAWERS on. * - * Instance admins hold 60 on every drawer in their instance through a - * fallback in getAccessLevel, not through the drawerPermissions map, - * so they get the whole instance's drawers here. + * Instance admins hold PERM_ADMIN on every drawer in their instance + * through a fallback in getAccessLevel, not through the + * drawerPermissions map, so they get the whole instance's drawers here. */ private function manageableDrawers(): array { $drawerRepository = $this->em->getRepository(Drawer::class); @@ -416,8 +431,7 @@ private function canManageDrawers(): bool { if (($this->user_model->getMaxCollectionPermission() ?? 0) >= PERM_CREATEDRAWERS) { return true; } - // the leading 0 keeps max() legal when the user holds no drawer - // grants at all + // max() errors on an empty array, so seed a 0 $drawerGrantLevels = array_values($this->user_model->drawerPermissions); return max([0, ...$drawerGrantLevels]) >= PERM_CREATEDRAWERS; } diff --git a/application/core/Instance_Controller.php b/application/core/Instance_Controller.php index 6299abc4c..41666a145 100644 --- a/application/core/Instance_Controller.php +++ b/application/core/Instance_Controller.php @@ -207,4 +207,43 @@ protected function requestBody(): array { ? $this->input->post() : $this->input->input_stream()) ?? []; } + + /** + * Find a remote user within the local DB by their remote id + * (e.g. username, umndid). If not found, creates a new + * user in the local DB with the remoteUserId set. + * + * @throws RemoteUserNotFoundException if the user cannot be found or + * provisioned. + * @return Entity\User the user record matching the remote id + */ + protected function firstOrProvisionRemoteUser(string $remoteUserId): Entity\User { + // findById($id, true) builds a new unsaved Entity\User with the + // given remote id when nothing is found. + /** @var ?Entity\User $remoteUser */ + $remoteUser = $this->user_model->getAuthHelper() + ->findById($remoteUserId, true)[0] ?? null; + + if ($remoteUser === null) { + throw new RemoteUserNotFoundException($remoteUserId); + } + + /** @var ?Entity\User $existingUser */ + $existingUser = $this->doctrine->em->getRepository(Entity\User::class) + ->findOneBy(["username" => $remoteUser->getUsername()]); + + // prefer an existing local user over the new unsaved record + if ($existingUser !== null) { + return $existingUser; + } + + $remoteUser->setUserType("Remote"); + $remoteUser->setCreatedAt(new \DateTime("now")); + $remoteUser->setInstance($this->instance); + + $this->doctrine->em->persist($remoteUser); + $this->doctrine->em->flush(); + + return $remoteUser; + } } diff --git a/application/libraries/GroupTypeCatalog.php b/application/libraries/GroupTypeCatalog.php index cf486f510..b8cccbe56 100644 --- a/application/libraries/GroupTypeCatalog.php +++ b/application/libraries/GroupTypeCatalog.php @@ -69,8 +69,7 @@ public function isAuthHelperType(string $type): bool { /** * Whether $type matches a whole population instead of a values list - * (All/Authed/Authed_remote). Such a group carries a vestigial scalar - * group_value of 1 and holds no entries. + * (All/Authed/Authed_remote). The group holds no entries. */ public function ignoresGroupValues(string $type): bool { return self::GLOBAL_TYPES[$type]["populationWide"] ?? false; From 175fef81907369547e9fbce9dac0f93e90652ea5 Mon Sep 17 00:00:00 2001 From: James Johnson Date: Fri, 10 Jul 2026 13:58:04 -0500 Subject: [PATCH 08/11] mock user shouldn't be superadmin --- application/libraries/MockAuthHelper.php | 1 - 1 file changed, 1 deletion(-) diff --git a/application/libraries/MockAuthHelper.php b/application/libraries/MockAuthHelper.php index 567b817a4..f2f8939fd 100644 --- a/application/libraries/MockAuthHelper.php +++ b/application/libraries/MockAuthHelper.php @@ -111,7 +111,6 @@ public function createUserFromRemote($userOverride = null, $map = null) { $user->setHasExpiry(false); $user->setCreatedAt(new \DateTime("now")); $user->setInstance($this->CI->instance); - $user->setIsSuperAdmin(true); $user->setFastUpload(false); $this->CI->doctrine->em->persist($user); $this->CI->doctrine->em->flush(); From d0f812a3722246b6987712b293ccafa32ffb92f0 Mon Sep 17 00:00:00 2001 From: James Johnson Date: Fri, 10 Jul 2026 16:19:00 -0500 Subject: [PATCH 09/11] show `is_personal` flag when listing groups --- application/controllers/DrawerPermissions.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/application/controllers/DrawerPermissions.php b/application/controllers/DrawerPermissions.php index 80f8024c2..38e1a175b 100644 --- a/application/controllers/DrawerPermissions.php +++ b/application/controllers/DrawerPermissions.php @@ -125,11 +125,11 @@ private function listGroups(): CI_Output { ->getRepository(DrawerGroup::class) ->findBy(['user' => $this->user_model->user]); - $isNotPersonalGroup = fn(DrawerGroup $group): bool => - $group->getId() !== $personalGroupId; - $visibleGroups = array_values(array_filter($groups, $isNotPersonalGroup)); - return render_json(['groups' => $visibleGroups]); + $withPersonalFlag = fn(DrawerGroup $group): array => + $group->jsonSerialize() + ['is_personal' => $group->getId() === $personalGroupId]; + + return render_json(['groups' => array_map($withPersonalFlag, $groups)]); } /** From fdf0af6d01df812b0a910b34e7498f01b007683c Mon Sep 17 00:00:00 2001 From: James Johnson Date: Fri, 10 Jul 2026 17:53:08 -0500 Subject: [PATCH 10/11] change GLOBAL to BUILTIN to avoid confusion with legacy global --- application/libraries/GroupTypeCatalog.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/application/libraries/GroupTypeCatalog.php b/application/libraries/GroupTypeCatalog.php index b8cccbe56..c9541ba4e 100644 --- a/application/libraries/GroupTypeCatalog.php +++ b/application/libraries/GroupTypeCatalog.php @@ -5,7 +5,7 @@ class GroupTypeCatalog { // The built-in types every instance has. `populationWide` marks the // types that match a whole population rather than a chosen list, which // means they have no entries. - private const GLOBAL_TYPES = [ + private const BUILTIN_TYPES = [ ALL_TYPE => [ "name" => ALL_TYPE, "label" => "All", @@ -37,7 +37,7 @@ class GroupTypeCatalog { /** * @param array $authHelperTypes the instance auth helper's authTypes, * keyed by type string (e.g. Unit, JobCode). Empty for helperless - * instances, which then offer only the built-in globals. + * instances, which then offer only the built-in types. */ public function __construct(array $authHelperTypes = []) { $this->authHelperTypes = $authHelperTypes; @@ -45,11 +45,11 @@ public function __construct(array $authHelperTypes = []) { /** * Every type this instance offers, keyed by type string: the built-in - * globals first, then the auth helper's own types. + * types first, then the auth helper's own types. */ public function all(): array { return [ - ...self::GLOBAL_TYPES, + ...self::BUILTIN_TYPES, ...$this->authHelperTypes, ]; } @@ -60,11 +60,11 @@ public function isValid(string $type): bool { /** * Whether $type comes from the auth helper rather than the built-in - * globals. Auth-helper groups match users on their value entries. The - * globals never do: User matches member ids, the rest whole populations. + * types. Auth-helper groups match users on their value entries. The + * built-ins never do: User matches member ids, the rest whole populations. */ public function isAuthHelperType(string $type): bool { - return !isset(self::GLOBAL_TYPES[$type]); + return !isset(self::BUILTIN_TYPES[$type]); } /** @@ -72,7 +72,7 @@ public function isAuthHelperType(string $type): bool { * (All/Authed/Authed_remote). The group holds no entries. */ public function ignoresGroupValues(string $type): bool { - return self::GLOBAL_TYPES[$type]["populationWide"] ?? false; + return self::BUILTIN_TYPES[$type]["populationWide"] ?? false; } /** From 746dd047186cb12f6a64b49f6be1db710d07e10d Mon Sep 17 00:00:00 2001 From: James Johnson Date: Fri, 10 Jul 2026 18:15:42 -0500 Subject: [PATCH 11/11] remove dead code --- application/controllers/DrawerPermissions.php | 26 ++----------------- 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/application/controllers/DrawerPermissions.php b/application/controllers/DrawerPermissions.php index 38e1a175b..c8834c902 100644 --- a/application/controllers/DrawerPermissions.php +++ b/application/controllers/DrawerPermissions.php @@ -147,8 +147,8 @@ private function showGroup(int $groupId): CI_Output { } /** - * POST /drawerPermissions/groups: create a group owned by the - * signed-in user, with any initial member or match values. + * POST /drawerPermissions/groups: create an empty group owned by the + * signed-in user. Members and match values are added separately. */ private function createGroup(): CI_Output { try { @@ -178,28 +178,6 @@ private function createGroup(): CI_Output { $group->setGroupValue(1); } else { $group->setGroupValue(null); - - $nonEmptyValues = array_filter( - (array) ($validated['values'] ?? []), - fn($value) => $value !== '' - ); - - foreach ($nonEmptyValues as $value) { - // a non-numeric User value is a remote username. Provision its - // local row and store the resulting user id. Other types store - // their value as-is. - if ($type === USER_TYPE && !is_numeric($value)) { - try { - $value = $this->firstOrProvisionRemoteUser($value)->getId(); - } catch (RemoteUserNotFoundException $e) { - return abort_json(['error' => $e->getMessage()], 404); - } - } - - $entry = new \Entity\GroupEntry(); - $entry->setGroupValue($value); - $group->addGroupValue($entry); - } } $this->em->persist($group);