diff --git a/directorygroups/src/Hooks/AddDirectoryGroupsAPI.php b/directorygroups/src/Hooks/AddDirectoryGroupsAPI.php index 4dd2428..76c66ec 100644 --- a/directorygroups/src/Hooks/AddDirectoryGroupsAPI.php +++ b/directorygroups/src/Hooks/AddDirectoryGroupsAPI.php @@ -59,6 +59,7 @@ public function __construct() $this->registerInstalled([ ['API_VALID_CLASSES', 'injectAPIElements'], ['API_SENSITIVE_FIELDS', 'declareSensitiveFields'], + ['API_SERVER_OWNED_FIELDS', 'declareServerOwnedFields'], ['DIRECTORYDOMAIN_EXPORT_ITEMS', 'stripBindPassword'], ]); } @@ -75,7 +76,6 @@ public function injectAPIElements($arguments) array_push( $arguments['validClasses'], $this->node, - 'directorygroupsassociation', 'directorydomain' ); } @@ -94,6 +94,21 @@ public function declareSensitiveFields($arguments) { $arguments['always']['directorydomain'][] = 'bindPwd'; } + /** + * Refuses API writes to a directory group's core group. + * + * DirectoryGroups::save() creates that group and keeps its name in step. + * A client that pointed groupID at another group would move every host + * of the directory group into it. + * + * @param mixed $arguments The field map to modify. + * + * @return void + */ + public function declareServerOwnedFields($arguments) + { + $arguments['fields']['directorygroups'][] = 'groupID'; + } /** * Keeps the bind password out of the domain export. * diff --git a/directorygroups/src/Hooks/AddDirectoryGroupsHost.php b/directorygroups/src/Hooks/AddDirectoryGroupsHost.php index 807db6a..61fa17f 100644 --- a/directorygroups/src/Hooks/AddDirectoryGroupsHost.php +++ b/directorygroups/src/Hooks/AddDirectoryGroupsHost.php @@ -99,16 +99,7 @@ public function hostTabData($arguments) */ public function hostDirectoryGroups($obj) { - $items = \FOG\Router\Route::getList('directorygroupsassociation'); - $ou = 0; - foreach ($items as &$item) { - if ($item->hostID == $obj->get('id')) { - $ou = $item->directorygroupsID; - unset($item); - break; - } - unset($item); - } + $ou = self::currentDirectoryGroup($obj->get('id')); $directorygroupsID = ( (int)filter_input(INPUT_POST, 'directorygroups') ?: $ou @@ -193,31 +184,10 @@ public function hostDirectoryGroups($obj) public function hostDirectoryGroupsPost($obj) { self::checkAuthAndCSRF(); - $directorygroupsID = trim( + self::setDirectoryGroup( + [$obj->get('id')], (int)filter_input(INPUT_POST, 'directorygroups') ); - $insert_fields = ['hostID', 'directorygroupsID']; - $insert_values = []; - $hosts = [$obj->get('id')]; - if (count($hosts ?: [])) { - \FOG\Router\Route::deletemass( - 'directorygroupsassociation', - ['hostID' => $hosts] - ); - if ($directorygroupsID > 0) { - foreach ((array)$hosts as $ind => &$hostID) { - $insert_values[] = [$hostID, $directorygroupsID]; - unset($hostID); - } - } - } - if (count($insert_values) > 0) { - (new \FOG\Plugins\DirectoryGroups\Managers\DirectoryGroupsAssociationManager()) - ->insertBatch( - $insert_fields, - $insert_values - ); - } } /** * Associates the OU posted during host (auto) registration. @@ -239,11 +209,7 @@ public function hostAddDirectoryGroupsRegister($arguments) if ($directorygroupsID < 1) { return; } - (new \FOG\Plugins\DirectoryGroups\Managers\DirectoryGroupsAssociationManager()) - ->insertBatch( - ['hostID', 'directorygroupsID'], - [[$obj->get('id'), $directorygroupsID]] - ); + self::setDirectoryGroup([$obj->get('id')], $directorygroupsID); } /** * The host ou selector. @@ -367,16 +333,11 @@ public function massEditFields($arguments) /** * What the selected hosts hold, as a name rather than an id. * - * SharedHostValues::forHostRows() answers in the column's own terms, so a - * uniform selection comes back as the ou id. Rendering that would put - * a bare number in front of the admin where every other hint shows the - * value they set. The lookup only happens when the answer is uniform and - * non-empty, which is the only case where there is one name to show. - * - * forHostRows() is also the reason this is not a hand-rolled query: a - * host with no ou has no row at all, so counting rows would call - * three hosts out of five "in agreement". It compares the row count to - * the selection size for exactly that reason. + * _sharedMembership() answers with the directory group id. Rendering + * that would put a bare number in front of the admin where every other + * hint shows the value they set. The lookup only happens when the answer + * is uniform and non-empty, which is the only case where there is one + * name to show. * * @param array $hostIDs the selection * @@ -384,12 +345,7 @@ public function massEditFields($arguments) */ private function _sharedDirectoryGroups(array $hostIDs) { - $info = \FOG\Util\SharedHostValues::forHostRows( - $hostIDs, - 'directoryOUAssoc', - 'doaHostID', - ['directorygroups' => 'doaOUID'] - )['directorygroups']; + $info = self::_sharedMembership($hostIDs); if (!empty($info['uniform']) && '' !== (string)$info['value']) { $item = new \FOG\Plugins\DirectoryGroups\Items\DirectoryGroups((int)$info['value']); @@ -400,6 +356,37 @@ private function _sharedDirectoryGroups(array $hostIDs) return $info; } + /** + * forHostRows() over directory group membership. + * + * Membership is a join, not a table forHostRows() can read, so this asks + * the same two questions itself: does every selected host have a + * directory group, and is it the same one. A host with none makes the + * selection "(varies)", as a host with no row does there. + * + * @param array $hostIDs the selection + * + * @return array ['uniform' => bool, 'value' => string] + */ + private function _sharedMembership(array $hostIDs) + { + $hostIDs = array_values(array_unique(array_filter(array_map('intval', $hostIDs)))); + if (count($hostIDs) < 1) { + return ['uniform' => false, 'value' => '']; + } + $row = self::$DB->query( + 'SELECT COUNT(DISTINCT `gm`.`gmHostID`) AS `n`,' + . ' COUNT(DISTINCT `d`.`doID`) AS `d`, MIN(`d`.`doID`) AS `v`' + . ' FROM `groupMembers` `gm`' + . ' JOIN `directoryOUs` `d` ON `d`.`doGroupID` = `gm`.`gmGroupID`' + . ' WHERE `gm`.`gmHostID` IN (' . implode(',', $hostIDs) . ')' + )->fetch(); + $n = (int)$row->get('n'); + return [ + 'uniform' => $n === count($hostIDs) && 1 === (int)$row->get('d'), + 'value' => (string)$row->get('v') + ]; + } /** * Applies a resolved ou action across the selection. * @@ -447,21 +434,62 @@ function ($id) { } } - // Both branches clear first. The association is single-valued per - // host, so a set is a replace -- and doing it in one deletemass - // rather than per host keeps it to one statement. - \FOG\Router\Route::deletemass( - 'directorygroupsassociation', - ['hostID' => $hostIDs] - ); - if ($itemID < 1) { + self::setDirectoryGroup($hostIDs, $itemID); + } + /** + * The directory group a host is in, or 0. + * + * @param int $hostID the host + * + * @return int the directory group id + */ + public static function currentDirectoryGroup($hostID) + { + $row = self::$DB->query( + 'SELECT `d`.`doID` AS `id` FROM `directoryOUs` `d`' + . ' JOIN `groupMembers` `gm` ON `gm`.`gmGroupID` = `d`.`doGroupID`' + . ' WHERE `gm`.`gmHostID` = :host ORDER BY `d`.`doID` LIMIT 1', + [], + [':host' => (int)$hostID] + )->fetch()->get('id'); + return (int)$row; + } + /** + * Puts hosts in one directory group, or takes them out of all of them. + * + * A join goes through the directory group's core group, so core fires + * ASSOCIATION_UPDATED and AddDirectoryGroupsMembership takes each host + * out of its old directory group and writes the new OU onto it. + * + * @param array $hostIDs the hosts + * @param int $directorygroupsID the directory group, or 0 for none + * + * @throws \Exception when the directory group does not exist + * + * @return void + */ + public static function setDirectoryGroup(array $hostIDs, $directorygroupsID) + { + $hostIDs = array_values(array_filter(array_map('intval', $hostIDs))); + if (count($hostIDs) < 1) { + return; + } + if ($directorygroupsID > 0) { + $item = new \FOG\Plugins\DirectoryGroups\Items\DirectoryGroups($directorygroupsID); + $Group = new \FOG\Items\Group((int)$item->get('groupID')); + if (!$item->isValid() || !$Group->isValid()) { + throw new \Exception(_('Invalid OU selected')); + } + $Group->addHost($hostIDs)->save(); return; } - $insert_values = []; - foreach ($hostIDs as $hostID) { - $insert_values[] = [$hostID, $itemID]; + $rows = self::$DB->query( + 'SELECT DISTINCT `gm`.`gmGroupID` AS `g` FROM `groupMembers` `gm`' + . ' JOIN `directoryOUs` `d` ON `d`.`doGroupID` = `gm`.`gmGroupID`' + . ' WHERE `gm`.`gmHostID` IN (' . implode(',', $hostIDs) . ')' + )->fetch(\PDO::FETCH_ASSOC, 'fetch_all')->get(); + foreach ((array)$rows as $row) { + (new \FOG\Items\Group((int)$row['g']))->removeHost($hostIDs)->save(); } - (new \FOG\Plugins\DirectoryGroups\Managers\DirectoryGroupsAssociationManager()) - ->insertBatch(['hostID', 'directorygroupsID'], $insert_values); } } diff --git a/directorygroups/src/Hooks/AddDirectoryGroupsMembership.php b/directorygroups/src/Hooks/AddDirectoryGroupsMembership.php new file mode 100644 index 0000000..d5f7bbc --- /dev/null +++ b/directorygroups/src/Hooks/AddDirectoryGroupsMembership.php @@ -0,0 +1,232 @@ + + * @license http://opensource.org/licenses/gpl-3.0 GPLv3 + * @link https://fogproject.org + */ + +namespace FOG\Plugins\DirectoryGroups\Hooks; + +/** + * Keeps a host in one directory group, and writes its OU onto the host. + * + * A directory group's hosts live in its core group (groupMembers), so a host + * can join one from the core Group page, the host's Groups tab, mass edit, + * CSV import, registration or the API, as well as from this plugin's own + * pages. Core fires ASSOCIATION_UPDATED from all of those, and this listener + * applies the two rules a directory group adds: + * + * - A host is in at most one directory group. An OU is a single place in the + * directory, so two would contradict each other. Joining one removes the + * host from any other directory group's core group. + * - Joining writes the group's OU into the host's ADOU, and its domain into + * ADDomain. This is a deliberate exception to fogproject ADR 0038's "a + * group grants, it does not copy": placement and the domain join read the + * host's own fields, and Tom chose the write (GH-1879 follow-up). + * + * Leaving a directory group changes nothing on the host. + * + * @category AddDirectoryGroupsMembership + * @package FOGProject + * @author Tom Elliott + * @license http://opensource.org/licenses/gpl-3.0 GPLv3 + * @link https://fogproject.org + */ +class AddDirectoryGroupsMembership extends \FOG\Base\Hook +{ + /** + * The name of this hook. + * + * @var string + */ + public $name = 'AddDirectoryGroupsMembership'; + /** + * The description of this hook. + * + * @var string + */ + public $description = 'One directory group per host, and its OU on the host'; + /** + * The active flag. + * + * @var bool + */ + public $active = true; + /** + * The node this hook enacts with. + * + * @var string + */ + public $node = 'directorygroups'; + /** + * Initializes object. + * + * @return void + */ + public function __construct() + { + parent::__construct(); + $this->registerInstalled([ + ['ASSOCIATION_UPDATED', 'membershipChanged'], + ]); + } + /** + * Applies both rules to the hosts that just joined a group. + * + * @param mixed $arguments The event arguments. + * + * @return void + */ + public function membershipChanged($arguments) + { + if ('GroupAssociation' !== \FOG\Base\FOGBase::shortName($arguments['association']) + || count($arguments['added'] ?: []) < 1 + ) { + return; + } + $joins = self::joins( + \FOG\Base\FOGBase::shortName($arguments['owner']), + (int)$arguments['owner']->get('id'), + (array)$arguments['added'] + ); + $groups = self::_directoryGroups(); + $plan = self::plan($joins, $groups); + if (count($plan) < 1) { + return; + } + // Leave every other directory group. Through Group::save(), not a + // raw delete, so the removal is an ordinary membership change: it + // fires this event again, with nothing added, which returns above. + $now = self::_memberships(array_keys($plan), array_keys($groups)); + foreach (self::leaves($plan, $now) as $groupID => $hostIDs) { + (new \FOG\Items\Group($groupID))->removeHost($hostIDs)->save(); + } + // One UPDATE per directory group joined, not per host. + $byGroup = []; + foreach ($plan as $hostID => $groupID) { + $byGroup[$groupID][] = $hostID; + } + foreach ($byGroup as $groupID => $hostIDs) { + $fields = ['ADOU' => $groups[$groupID]['dn']]; + if ('' !== $groups[$groupID]['domain']) { + $fields['ADDomain'] = $groups[$groupID]['domain']; + } + (new \FOG\Managers\HostManager())->update(['id' => $hostIDs], 'AND', $fields); + } + } + /** + * The directory groups each host must leave. + * + * @param array $plan hostID => the core group id it keeps + * @param array $memberships list of [hostID, groupID] it holds now, in + * directory groups only + * + * @return array groupID => host ids to remove from it + */ + public static function leaves(array $plan, array $memberships) + { + $leave = []; + foreach ($memberships as list($hostID, $groupID)) { + if (isset($plan[$hostID]) && $plan[$hostID] !== $groupID) { + $leave[$groupID][] = $hostID; + } + } + return $leave; + } + /** + * Which directory groups these hosts are in now. + * + * @param array $hostIDs the hosts + * @param array $groupIDs the core groups that back a directory group + * + * @return array list of [hostID, groupID] + */ + private static function _memberships(array $hostIDs, array $groupIDs) + { + if (count($hostIDs) < 1 || count($groupIDs) < 1) { + return []; + } + $hosts = implode(',', array_map('intval', $hostIDs)); + $groups = implode(',', array_map('intval', $groupIDs)); + $rows = self::$DB->query( + 'SELECT `gmHostID` AS `h`, `gmGroupID` AS `g` FROM `groupMembers`' + . " WHERE `gmHostID` IN ($hosts) AND `gmGroupID` IN ($groups)" + )->fetch(\PDO::FETCH_ASSOC, 'fetch_all')->get(); + $pairs = []; + foreach ((array)$rows as $row) { + $pairs[] = [(int)$row['h'], (int)$row['g']]; + } + return $pairs; + } + /** + * The (host, core group) pairs a membership change added. + * + * @param string $owner the saved object's class, Group or Host + * @param int $ownerID its id + * @param array $added the other end's ids + * + * @return array list of [hostID, groupID] + */ + public static function joins($owner, $ownerID, array $added) + { + $pairs = []; + foreach ($added as $id) { + $pairs[] = 'Host' === $owner + ? [$ownerID, (int)$id] + : [(int)$id, $ownerID]; + } + return $pairs; + } + /** + * Which directory group each joining host ends up in. + * + * Pairs whose group backs no directory group are ignored. When one save + * adds a host to two directory groups at once, the last one wins, so the + * answer never depends on database row order. + * + * @param array $joins list of [hostID, groupID] + * @param array $groups core group id => directory group, as + * _directoryGroups() returns them + * + * @return array hostID => the core group id it keeps + */ + public static function plan(array $joins, array $groups) + { + $keep = []; + foreach ($joins as list($hostID, $groupID)) { + if ($hostID > 0 && isset($groups[$groupID])) { + $keep[$hostID] = $groupID; + } + } + return $keep; + } + /** + * Every directory group, keyed by the core group that backs it. + * + * @return array groupID => ['dn' => string, 'domain' => string] + */ + private static function _directoryGroups() + { + $rows = self::$DB->query( + 'SELECT `d`.`doGroupID` AS `gid`, `d`.`doDN` AS `dn`,' + . ' COALESCE(`dd`.`ddName`, \'\') AS `domain`' + . ' FROM `directoryOUs` `d`' + . ' LEFT JOIN `directoryDomains` `dd` ON `dd`.`ddID` = `d`.`doDomainID`' + . ' WHERE `d`.`doGroupID` IS NOT NULL' + )->fetch(\PDO::FETCH_ASSOC, 'fetch_all')->get(); + $groups = []; + foreach ((array)$rows as $row) { + $groups[(int)$row['gid']] = [ + 'dn' => (string)$row['dn'], + 'domain' => (string)$row['domain'] + ]; + } + return $groups; + } +} diff --git a/directorygroups/src/Hooks/DirectoryGroupsChangeItems.php b/directorygroups/src/Hooks/DirectoryGroupsChangeItems.php deleted file mode 100644 index 2d923f0..0000000 --- a/directorygroups/src/Hooks/DirectoryGroupsChangeItems.php +++ /dev/null @@ -1,106 +0,0 @@ - - * @author Lee Rowlett - * @license http://opensource.org/licenses/gpl-3.0 GPLv3 - * @link https://fogproject.org - */ - -namespace FOG\Plugins\DirectoryGroups\Hooks; - -/** - * Changes the elements we need. - * - * @category ChangeItems - * @package FOGProject - * @author Tom Elliott - * @author Lee Rowlett - * @license http://opensource.org/licenses/gpl-3.0 GPLv3 - * @link https://fogproject.org - */ -/** - * Named for its plugin -- see LocationChangeItems, which shared this hook's - * old changeitems.hook.php filename and therefore its class name. Only one of - * the two ever loaded per request, decided by readdir order. Namespacing - * closed that off structurally (fogproject ADR 0035); the distinct names stay - * because they are what the log shows. - */ -class DirectoryGroupsChangeItems extends \FOG\Base\Hook -{ - /** - * The name of this hook. - * - * @var string - */ - public $name = 'DirectoryGroupsChangeItems'; - /** - * The description of this hook. - * - * @var string - */ - public $description = 'Add OU During client checkin'; - /** - * The active flag. - * - * @var bool - */ - public $active = true; - /** - * The node this hook enacts with. - * - * @var string - */ - public $node = 'directorygroups'; - /** - * Initialize object. - * - * @return void - */ - public function __construct() - { - parent::__construct(); - $this->registerInstalled([ - ['HOSTNAME_CHANGER_CLIENT', 'changeADItems'], - ]); - } - /** - * Sets up host for the new OU - * - * @param mixed $arguments The items to change. - * - * @return void - */ - public function changeADItems($arguments) - { - if (!$arguments['Host']->isValid()) { - return; - } - $OUAssocs = \FOG\Router\Route::getList( - 'directorygroupsassociation', - ['hostID' => $arguments['Host']->get('id')] - ); - // listem() returns the paginated envelope, so the rows are under - // ->data. Iterating the envelope itself walked its scalar counters - // instead: every client check-in warned "Attempt to read property - // directorygroupsID on int" and ADOU was never set. LocationChangeItems, which is - // the same shape, has always had this right. - foreach ($OUAssocs as $OUAssoc) { - // getClass() rather than Route::indiv(): indiv() answers a missing - // row with sendResponse(404), and that exits. An ouassociation - // left behind by a deleted OU would end the whole check-in with a - // 404 the client reads as a transport failure, rather than skip - // one association. - $OU = new \FOG\Plugins\DirectoryGroups\Items\DirectoryGroups($OUAssoc->directorygroupsID); - if (!$OU->isValid()) { - continue; - } - $arguments['val']['ADOU'] = $OU->get('ou'); - } - } -} diff --git a/directorygroups/src/Hooks/DirectoryGroupsDeleteMassItems.php b/directorygroups/src/Hooks/DirectoryGroupsDeleteMassItems.php index d5bd696..467d3d8 100644 --- a/directorygroups/src/Hooks/DirectoryGroupsDeleteMassItems.php +++ b/directorygroups/src/Hooks/DirectoryGroupsDeleteMassItems.php @@ -61,7 +61,7 @@ public function __construct() ]); } /** - * Prepares to clean up associations + * Deletes a directory group's core group with it * * @param mixed $arguments The items to change. * @@ -69,21 +69,28 @@ public function __construct() */ public function deletemassitems($arguments) { - switch ($arguments['classname']) { - case 'host': - $arguments['removeItems']['directorygroupsassociation'] = [ - 'hostID' => $arguments['itemIDs'] - ]; - break; - case 'directorygroups': - // Only treat itemIDs as ouIDs when an OU itself is deleted. - // A bare default would mis-fire for every other entity now that - // all delete paths funnel through DELETEMASS_API, deleting - // ouassociation rows whose directorygroupsID collides with the deleted id. - $arguments['removeItems']['directorygroupsassociation'] = [ - 'directorygroupsID' => $arguments['itemIDs'] - ]; - break; + // A deleted host leaves its directory group by core's own cascade: + // the membership is a groupMembers row now. + if ('directorygroups' !== $arguments['classname']) { + return; + } + // A deleted directory group takes its core group with it. Only the + // ids that exist: an empty list would reach deletemass() as "no + // filter", which matches every group. + $ids = array_values(array_filter(array_map('intval', (array)$arguments['itemIDs']))); + if (count($ids) < 1) { + return; + } + $groupIDs = array_values( + array_filter( + array_map( + 'intval', + (array)\FOG\Router\Route::getIds('directorygroups', ['id' => $ids], 'groupID') + ) + ) + ); + if (count($groupIDs) > 0) { + $arguments['removeItems']['group'] = ['id' => $groupIDs]; } } } diff --git a/directorygroups/src/Items/DirectoryGroups.php b/directorygroups/src/Items/DirectoryGroups.php index 1982088..79d4369 100644 --- a/directorygroups/src/Items/DirectoryGroups.php +++ b/directorygroups/src/Items/DirectoryGroups.php @@ -42,7 +42,8 @@ class DirectoryGroups extends \FOG\Base\FOGController 'createdBy' => 'doCreatedBy', 'createdTime' => 'doCreatedTime', 'ou' => 'doDN', - 'domainID' => 'doDomainID' + 'domainID' => 'doDomainID', + 'groupID' => 'doGroupID' ]; /** * The required fields @@ -62,34 +63,96 @@ class DirectoryGroups extends \FOG\Base\FOGController 'hosts' ]; /** - * Destroy this particular object. + * The longest name a core group may carry: groups.groupName is + * varchar(50), and directory group names run to 255. + */ + const GROUP_NAME_MAX = 50; + /** + * Stores the item in the DB either stored or updated. * - * @param string $key the key to destroy for match + * Every directory group is backed by a core group of the same name. The + * core group holds the hosts, so whatever an admin grants that group -- + * snapins, printers, modules, power schedules -- reaches them (fogproject + * ADR 0038). This creates the core group on first save, keeps its name + * in step, and saves the host list through it. * - * @return bool + * Saving through the core group fires ASSOCIATION_UPDATED, so the one + * group per host rule and the AD write live in one listener, whichever + * page saved the membership. See AddDirectoryGroupsMembership. + * + * @return object|bool */ - public function destroy($key = 'id') + public function save() { - \FOG\Router\Route::deletemass( - 'directorygroupsassociation', - ['directorygroupsID' => $this->get('id')] + if (!parent::save()) { + return false; + } + $Group = new \FOG\Items\Group((int)$this->get('groupID')); + $name = self::groupName( + $this->get('name'), + $this->get('id'), + $Group->isValid() ? $Group->get('id') : 0 ); - return parent::destroy($key); + if (!$Group->isValid()) { + $Group = (new \FOG\Items\Group()) + ->set('name', $name) + ->set('description', _('Hosts in directory group') . ' ' . $this->get('name')) + ->set('createdBy', $this->get('createdBy') ?: 'fog'); + if (!$Group->save()) { + return false; + } + $this->set('groupID', $Group->get('id')); + if (!parent::save()) { + return false; + } + } elseif ($Group->get('name') !== $name) { + $Group->set('name', $name)->save(); + } + if ($this->isDirty('hosts')) { + $Group->set('hosts', (array)$this->get('hosts'))->save(); + } + return $this->load(); } /** - * Stores the item in the DB either stored or updated. + * The core group name for a directory group. * - * @return object + * The directory group's own name when it fits and no other core group + * has it. Otherwise that name cut short, ending in "#", which only + * this directory group can carry. Example: a second "Sales" becomes + * "Sales #12". + * + * @param string $name the directory group name + * @param int $id the directory group id + * @param int $groupID the core group already backing it, if any + * + * @return string */ - public function save() + public static function groupName($name, $id, $groupID = 0) + { + $name = trim((string)$name); + if (strlen($name) <= self::GROUP_NAME_MAX + && !self::_groupNameTaken($name, $groupID) + ) { + return $name; + } + $suffix = ' #' . (int)$id; + return rtrim(substr($name, 0, self::GROUP_NAME_MAX - strlen($suffix))) + . $suffix; + } + /** + * Does a core group other than this one hold the name? + * + * @param string $name the name + * @param int $groupID the core group to ignore + * + * @return bool + */ + private static function _groupNameTaken($name, $groupID) { - parent::save(); - return $this - ->assocSetter('DirectoryGroups', 'host') - ->load(); + return (new \FOG\Managers\GroupManager())->exists($name, (int)$groupID); } /** - * Add host to the location. + * Add host to the directory group. * * @param array $addArray the items to add. * @@ -104,7 +167,7 @@ public function addHost($addArray) ); } /** - * Remove host from the location. + * Remove host from the directory group. * * @param array $removeArray the items to remove. * @@ -119,15 +182,15 @@ public function removeHost($removeArray) ); } /** - * Loads the locations hosts. + * Loads the hosts, from the core group that holds them. * * @return void */ protected function loadHosts() { $this->_loadHostIds( - 'directorygroupsassociation', - ['directorygroupsID' => $this->get('id')], + 'groupassociation', + ['groupID' => (int)$this->get('groupID') ?: -1], 'hostID' ); } diff --git a/directorygroups/src/Items/DirectoryGroupsAssociation.php b/directorygroups/src/Items/DirectoryGroupsAssociation.php index 3ea7369..eb624dc 100644 --- a/directorygroups/src/Items/DirectoryGroupsAssociation.php +++ b/directorygroups/src/Items/DirectoryGroupsAssociation.php @@ -16,6 +16,11 @@ /** * The association between hosts and ous. * + * Retired: schema step 6 moves these rows into core groupMembers and drops + * the table. The class stays because DirectoryGroupsAssociationManager + * resolves it by name, and that manager still creates the table for steps + * 2 and 3 on a fresh install. + * * @category DirectoryGroupsAssociation * @package FOGProject * @author Tom Elliott diff --git a/directorygroups/src/Managers/DirectoryGroupsManager.php b/directorygroups/src/Managers/DirectoryGroupsManager.php index b13ef4c..c04680f 100644 --- a/directorygroups/src/Managers/DirectoryGroupsManager.php +++ b/directorygroups/src/Managers/DirectoryGroupsManager.php @@ -149,8 +149,106 @@ function () { } return \FOG\Db\SchemaReconciler::applyConstraints('directorygroups'); }, + // 5 - each directory group is backed by a core group (GH-1879 + // follow-up). NULL until step 6 creates it. A re-run answers + // 1060, which applyUpdates() tolerates; MySQL has no IF NOT EXISTS. + 'ALTER TABLE `directoryOUs` ADD COLUMN `doGroupID` INTEGER NULL DEFAULT NULL', + // 6 - create the core groups, move the hosts into them, and + // retire directoryOUAssoc: core groupMembers holds them now. + function () { + return self::moveToCoreGroups(); + }, + // 7 - declare directoryOUs.doGroupID, CASCADE to `groups`. Same + // sweep-then-add as step 4. + function () { + $res = \FOG\Db\SchemaReconciler::sweepOrphans('directorygroups'); + if (is_string($res)) { + return $res; + } + return \FOG\Db\SchemaReconciler::applyConstraints('directorygroups'); + }, ]; } + /** + * Backs every directory group with a core group, and moves its hosts. + * + * For each directory group with no core group yet: + * - create a core group (DirectoryGroups::groupName() picks the name); + * - copy its directoryOUAssoc rows into groupMembers; + * - write its OU, and its domain when it has one, onto each host. That + * is what joining does from now on, and it replaces the check-in + * override the plugin used to apply to the legacy client. + * + * Then directoryOUAssoc is dropped. Every write before the drop is + * idempotent, so a run that fails part way is safe to repeat. + * + * @return true|string true, or the database error + */ + public static function moveToCoreGroups() + { + $rows = self::$DB->query( + 'SELECT `d`.`doID` AS `id`, `d`.`doName` AS `name`, `d`.`doDN` AS `dn`,' + . ' `d`.`doCreatedBy` AS `by`, `dd`.`ddName` AS `domain`' + . ' FROM `directoryOUs` `d`' + . ' LEFT JOIN `directoryDomains` `dd` ON `dd`.`ddID` = `d`.`doDomainID`' + . ' WHERE `d`.`doGroupID` IS NULL' + )->fetch(\PDO::FETCH_ASSOC, 'fetch_all')->get(); + foreach ((array)$rows as $row) { + $Group = (new \FOG\Items\Group()) + ->set( + 'name', + \FOG\Plugins\DirectoryGroups\Items\DirectoryGroups::groupName( + $row['name'], + $row['id'] + ) + ) + ->set('description', _('Hosts in directory group') . ' ' . $row['name']) + ->set('createdBy', $row['by'] ?: 'fog'); + if (!$Group->save()) { + return sprintf( + '%s: %s', + _('Could not create the core group for directory group'), + $row['name'] + ); + } + $statements = [ + [ + 'UPDATE `directoryOUs` SET `doGroupID` = :gid WHERE `doID` = :id', + [':gid' => $Group->get('id'), ':id' => $row['id']] + ], + ]; + if (self::_tableExists('directoryOUAssoc')) { + $statements[] = [ + 'INSERT IGNORE INTO `groupMembers` (`gmHostID`, `gmGroupID`)' + . ' SELECT `a`.`doaHostID`, :gid FROM `directoryOUAssoc` `a`' + . ' JOIN `hosts` `h` ON `h`.`hostID` = `a`.`doaHostID`' + . ' WHERE `a`.`doaOUID` = :id', + [':gid' => $Group->get('id'), ':id' => $row['id']] + ]; + $statements[] = [ + 'UPDATE `hosts` `h` JOIN `directoryOUAssoc` `a`' + . ' ON `a`.`doaHostID` = `h`.`hostID`' + . ' SET `h`.`hostADOU` = :dn,' + . ' `h`.`hostADDomain` = IF(:domain = \'\', `h`.`hostADDomain`, :domain2)' + . ' WHERE `a`.`doaOUID` = :id', + [ + ':dn' => (string)$row['dn'], + ':domain' => (string)$row['domain'], + ':domain2' => (string)$row['domain'], + ':id' => $row['id'] + ] + ]; + } + foreach ($statements as list($sql, $params)) { + if (false !== self::$DB->query($sql, [], $params)->error) { + return self::$DB->error; + } + } + } + $res = (new \FOG\Plugins\DirectoryGroups\Managers\DirectoryGroupsAssociationManager()) + ->uninstall(); + return $res ? true : _('Could not drop directoryOUAssoc'); + } /** * Installs the database non-destructively (create-if-absent + apply any * pending additive steps). Does not drop existing data. diff --git a/directorygroups/src/Pages/DirectoryGroupsManagement.php b/directorygroups/src/Pages/DirectoryGroupsManagement.php index d052741..ad56b38 100644 --- a/directorygroups/src/Pages/DirectoryGroupsManagement.php +++ b/directorygroups/src/Pages/DirectoryGroupsManagement.php @@ -402,20 +402,27 @@ function (&$serverFault) { /** * OU -> host membership list * + * The hosts live in the directory group's core group, so the list is + * that group's own host list. It runs on the core Group because + * getItemsList() marks a row "associated" by comparing it with the id of + * the object it runs on; on the directory group no row would match. + * * @return void */ public function getHostsList() { - return $this->assocItemsList( + $Group = new \FOG\Items\Group((int)$this->obj->get('groupID')); + return $Group->getItemsList( 'host', - 'directorygroupsassociation', - 'directoryOUAssoc', - '`hosts`.`hostID`', - '`directoryOUAssoc`.`doaHostID`', - '`directoryOUAssoc`.`doaOUID`', + 'groupassociation', + [ + 'LEFT OUTER JOIN `groupMembers` ON `hosts`.`hostID` = `groupMembers`.`gmHostID` ' + . "AND `groupMembers`.`gmGroupID` = '" . (int)$Group->get('id') . "'" + ], + '', [ [ - 'db' => 'directoryOUAssoc', + 'db' => 'groupAssoc', 'dt' => 'association', 'removeFromQuery' => true ] diff --git a/tests/directorygroups-membership.test.php b/tests/directorygroups-membership.test.php new file mode 100644 index 0000000..afac34c --- /dev/null +++ b/tests/directorygroups-membership.test.php @@ -0,0 +1,132 @@ + + * @license http://opensource.org/licenses/gpl-3.0 GPLv3 + * @link https://fogproject.org + */ + +$root = dirname(__DIR__); +$failures = []; +$checks = 0; + +/** + * Asserts a condition. + * + * @param string $label what is being checked + * @param bool $cond the condition + * + * @return void + */ +function check($label, $cond) +{ + global $failures, $checks; + $checks++; + if (!$cond) { + $failures[] = $label; + } +} + +require_once __DIR__ . '/stubs/fog-stubs.php'; +require_once $root . '/directorygroups/src/Hooks/AddDirectoryGroupsMembership.php'; + +use FOG\Plugins\DirectoryGroups\Hooks\AddDirectoryGroupsMembership as M; + +// ------------------------------------------------------- 1. both directions + +check( + 'a Group saved with hosts 4 and 5 joins each host to that group', + [[4, 9], [5, 9]] === M::joins('Group', 9, [4, 5]) +); +check( + 'a Host saved with groups 9 and 10 joins that host to each group', + [[4, 9], [4, 10]] === M::joins('Host', 4, [9, 10]) +); + +// ---------------------------------------------------------------- 2. plan + +// Core groups 9 and 10 back directory groups; 11 is an ordinary group. +$groups = [ + 9 => ['dn' => 'OU=Sales,DC=lab,DC=local', 'domain' => 'lab.local'], + 10 => ['dn' => 'OU=Labs,DC=lab,DC=local', 'domain' => 'lab.local'], +]; +check( + 'joining an ordinary core group plans nothing', + [] === M::plan([[4, 11]], $groups) +); +check( + 'joining a directory group keeps that group for the host', + [4 => 9] === M::plan([[4, 9], [4, 11]], $groups) +); +check( + 'two directory groups in one save: the last one wins', + [4 => 10] === M::plan(M::joins('Host', 4, [9, 10]), $groups) +); + +// -------------------------------------------------------------- 3. leaves + +check( + 'a host leaves the directory group it no longer keeps', + [9 => [4]] === M::leaves([4 => 10], [[4, 9], [4, 10]]) +); +check( + 'a host is not removed from the group it keeps', + [] === M::leaves([4 => 9], [[4, 9]]) +); +check( + 'a host that did not join anything is left alone', + [] === M::leaves([4 => 9], [[5, 10]]) +); +check( + 'hosts leaving the same group are removed in one batch', + [9 => [4, 5]] === M::leaves([4 => 10, 5 => 10], [[4, 9], [5, 9]]) +); + +// -------------------------------------------------------------- 4. schema + +$manager = (string)file_get_contents( + $root . '/directorygroups/src/Managers/DirectoryGroupsManager.php' +); +check( + 'doGroupID is added by its own ALTER step, not folded into the' + . ' released CREATE of step 1', + false !== strpos($manager, 'ADD COLUMN `doGroupID`') +); +$move = substr($manager, (int)strpos($manager, 'function moveToCoreGroups')); +$copy = strpos($move, 'INSERT IGNORE INTO `groupMembers`'); +$drop = strpos($move, 'DirectoryGroupsAssociationManager'); +check( + 'directoryOUAssoc is dropped only after its rows reach groupMembers', + false !== $copy && false !== $drop && $copy < $drop +); + +if (count($failures)) { + echo 'FAIL: ' . count($failures) . " of $checks checks failed.\n"; + foreach ($failures as $f) { + echo " - $f\n"; + } + exit(1); +} +echo "directorygroups-membership: $checks checks passed\n"; diff --git a/tests/fk-writes-are-null-not-zero.test.php b/tests/fk-writes-are-null-not-zero.test.php index ae56379..c97fe59 100644 --- a/tests/fk-writes-are-null-not-zero.test.php +++ b/tests/fk-writes-are-null-not-zero.test.php @@ -87,6 +87,7 @@ 'doaOUID' => ['directorygroups', 'DirectoryGroupsAssociation', 'directorygroupsID'], 'doaHostID' => ['directorygroups', 'DirectoryGroupsAssociation', 'hostID'], 'doDomainID' => ['directorygroups', 'DirectoryGroups', 'domainID'], + 'doGroupID' => ['directorygroups', 'DirectoryGroups', 'groupID'], 'sgGroupID' => ['subnetgroup', 'SubnetGroup', 'groupID'], // satellite / junction, ldap 'lgServerID' => ['ldap', 'LDAPGroup', 'serverID'], diff --git a/tests/group-push-became-mass-edit.test.php b/tests/group-push-became-mass-edit.test.php index 64e6be4..90d2160 100644 --- a/tests/group-push-became-mass-edit.test.php +++ b/tests/group-push-became-mass-edit.test.php @@ -148,13 +148,9 @@ function check($label, $cond) 'assoc' => 'ouassociation', 'col' => 'ouID', ], - [ - 'file' => 'directorygroups/src/Hooks/AddDirectoryGroupsHost.php', - 'class' => 'FOG\Plugins\DirectoryGroups\Hooks\AddDirectoryGroupsHost', - 'key' => 'directorygroups', - 'assoc' => 'directorygroupsassociation', - 'col' => 'directorygroupsID', - ], + // directorygroups is not here: its hosts are a core group's members, so + // a set goes through Group::save(). tests/directorygroups-membership + // pins what that changes. ]; foreach ($cases as $case) { diff --git a/tests/plugin-id-columns-are-classified.test.php b/tests/plugin-id-columns-are-classified.test.php index fd4c211..61b6445 100644 --- a/tests/plugin-id-columns-are-classified.test.php +++ b/tests/plugin-id-columns-are-classified.test.php @@ -71,7 +71,7 @@ 'location.lStorageGroupID', 'location.lStorageNodeID', 'ouAssoc.oaOUID', 'ouAssoc.oaHostID', 'directoryOUAssoc.doaOUID', 'directoryOUAssoc.doaHostID', - 'directoryOUs.doDomainID', + 'directoryOUs.doDomainID', 'directoryOUs.doGroupID', 'windowsKeysAssoc.wkaImageID', 'windowsKeysAssoc.wkaKeyID', 'LDAPGroups.lgServerID', 'ldapGroupRoleAssoc.lgraGroupID', 'ldapGroupRoleAssoc.lgraRoleID', 'ldapGroupUserGroupAssoc.lgugGroupID', @@ -151,8 +151,23 @@ function idStrip($file) // query elsewhere in the manager is not this table declaring it. $chunk = substr($src, (int)strpos($src, 'createTableSql('), 2000); preg_match_all('/.([A-Za-z_]+(?:ID|Id)).\s*[,\]]/', $chunk, $cm); + $keys = []; foreach (array_unique($cm[1]) as $column) { - $key = $table . '.' . $column; + $keys[] = $table . '.' . $column; + } + // A column a later schema step adds. Released steps are append-only, so + // a new column on an existing table arrives this way and never appears + // in createTableSql() (directoryOUs.doGroupID). + preg_match_all( + '/ALTER TABLE\s+`([A-Za-z_]+)`\s+ADD COLUMN\s+`([A-Za-z_]+(?:ID|Id))`/', + $src, + $am, + PREG_SET_ORDER + ); + foreach ($am as $m) { + $keys[] = $m[1] . '.' . $m[2]; + } + foreach (array_unique($keys) as $key) { $seen[$key] = true; $checks++; if (!isset($decided[$key])) {