From 61dfca88eb1cdcd104d917ecd51919d5a83c857d Mon Sep 17 00:00:00 2001 From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Thu, 26 Sep 2024 17:17:56 +1200 Subject: [PATCH] API Update API to reflect changes to CLI interaction (#89) --- docs/en/developer.md | 28 ++--- src/Jobs/LDAPAllSyncJob.php | 10 +- src/Jobs/LDAPMemberSyncJob.php | 7 +- src/Tasks/LDAPGroupSyncTask.php | 101 ++++++------------ src/Tasks/LDAPMemberSyncOneTask.php | 73 ++++++------- src/Tasks/LDAPMemberSyncTask.php | 103 ++++++------------- src/Tasks/LDAPMigrateExistingMembersTask.php | 58 +++-------- 7 files changed, 143 insertions(+), 237 deletions(-) diff --git a/docs/en/developer.md b/docs/en/developer.md index ab5fddb..8942516 100644 --- a/docs/en/developer.md +++ b/docs/en/developer.md @@ -340,8 +340,8 @@ See the [module docs](https://github.com/symbiote/silverstripe-queuedjobs) on ho If you don't want to run a queued job, you can set a cronjob yourself by calling: -``` -env php vendor/silverstripe/framework/cli-script.php dev/tasks/LDAPMemberSyncTask +```sh +vendor/bin/sake tasks:LDAPMemberSyncTask ``` ### Syncing AD groups and users on a schedule @@ -365,8 +365,8 @@ SilverStripe\LDAP\Jobs\LDAPAllSyncJob: If you don't want to run a queued job, you can set a cronjob yourself by calling the two sync tasks (order is important, otherwise your group memberships might not get updated): ```sh -env php vendor/silverstripe/framework/cli-script.php dev/tasks/LDAPGroupSyncTask -env php vendor/silverstripe/framework/cli-script.php dev/tasks/LDAPMemberSyncTask +vendor/bin/sake tasks:LDAPGroupSyncTask +vendor/bin/sake tasks:LDAPMemberSyncTask ``` ### Migrating existing users @@ -438,9 +438,9 @@ The fallback authenticator will be used in the following conditions: ### Extending the member and group sync tasks with custom functionality -Both `LDAPMemberSyncTask` and `LDAPGroupSyncTask` provide extension points (`onAfterLDAPMemberSyncTask` and -`onAfterLDAPGroupSyncTask` respectively) after all members/groups have been synced and before the task exits. This is a -perfect time to set values that are dependent on a full sync - for example linking a user to their manager based on DNs. +Both `LDAPMemberSyncTask` and `LDAPGroupSyncTask` provide extension points (`onAfterLDAPMemberSyncTask` and +`onAfterLDAPGroupSyncTask` respectively) after all members/groups have been synced and before the task exits. This is a +perfect time to set values that are dependent on a full sync - for example linking a user to their manager based on DNs. For example: ```yaml @@ -462,7 +462,7 @@ use SilverStripe\Security\Member; class LDAPMemberSyncExtension extends Extension { /** - * Assuming the `DN` and `ManagerDN` values are set by LDAP, this code will link a member with their manager and + * Assuming the `DN` and `ManagerDN` values are set by LDAP, this code will link a member with their manager and * store the link in the `Manager` has_one. */ protected function onAfterLDAPMemberSyncTask() @@ -505,19 +505,19 @@ This will allow users to change their AD password via the regular CMS "forgot pa ### Allow SilverStripe attributes to be reset (removed) by AD -By default if attributes are present, and then missing in subsequent requests, they are ignored (non-destructive) by +By default if attributes are present, and then missing in subsequent requests, they are ignored (non-destructive) by this module. This can cause attributes to persist when they've been deliberately removed (attribute is no longer present) -in the LDAP source data. +in the LDAP source data. -If you wish a full two way sync to occur, then set the attribute on `LDAPService` for `reset_missing_attributes` to -enable a full sync. +If you wish a full two way sync to occur, then set the attribute on `LDAPService` for `reset_missing_attributes` to +enable a full sync. *Note*: This will mean syncs are destructive, and data or attributes will be reset if missing from the master LDAP source -data. +data. ```yaml SilverStripe\LDAP\Services\LDAPService: - reset_missing_attributes: true + reset_missing_attributes: true ``` ### Writing LDAP data from SilverStripe diff --git a/src/Jobs/LDAPAllSyncJob.php b/src/Jobs/LDAPAllSyncJob.php index ad370b2..8443b19 100644 --- a/src/Jobs/LDAPAllSyncJob.php +++ b/src/Jobs/LDAPAllSyncJob.php @@ -6,10 +6,12 @@ use SilverStripe\LDAP\Tasks\LDAPGroupSyncTask; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injector; +use SilverStripe\PolyExecution\PolyOutput; use SilverStripe\LDAP\Tasks\LDAPMemberSyncTask; use Symbiote\QueuedJobs\Services\AbstractQueuedJob; use Symbiote\QueuedJobs\Services\QueuedJob; use Symbiote\QueuedJobs\Services\QueuedJobService; +use Symfony\Component\Console\Input\ArrayInput; /** * Class LDAPAllSyncJob @@ -84,11 +86,15 @@ public function process() singleton(QueuedJobService::class)->queueJob($nextJob, date('Y-m-d H:i:s', time() + $regenerateTime)); } + $output = PolyOutput::create(PolyOutput::FORMAT_ANSI); + $input = new ArrayInput([]); + $input->setInteractive(false); + $task = Injector::inst()->create(LDAPGroupSyncTask::class); - $task->run(null); + $task->run($input, $output); $task = Injector::inst()->create(LDAPMemberSyncTask::class); - $task->run(null); + $task->run($input, $output); $this->isComplete = true; } diff --git a/src/Jobs/LDAPMemberSyncJob.php b/src/Jobs/LDAPMemberSyncJob.php index 503ae63..1377fdd 100644 --- a/src/Jobs/LDAPMemberSyncJob.php +++ b/src/Jobs/LDAPMemberSyncJob.php @@ -6,9 +6,11 @@ use SilverStripe\LDAP\Tasks\LDAPMemberSyncTask; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injector; +use SilverStripe\PolyExecution\PolyOutput; use Symbiote\QueuedJobs\Services\AbstractQueuedJob; use Symbiote\QueuedJobs\Services\QueuedJob; use Symbiote\QueuedJobs\Services\QueuedJobService; +use Symfony\Component\Console\Input\ArrayInput; /** * Class LDAPMemberSyncJob @@ -89,7 +91,10 @@ public function process() } $task = Injector::inst()->create(LDAPMemberSyncTask::class); - $task->run(null); + $output = PolyOutput::create(PolyOutput::FORMAT_ANSI); + $input = new ArrayInput([]); + $input->setInteractive(false); + $task->run($input, $output); $this->isComplete = true; } diff --git a/src/Tasks/LDAPGroupSyncTask.php b/src/Tasks/LDAPGroupSyncTask.php index ff4154c..b97b1f4 100644 --- a/src/Tasks/LDAPGroupSyncTask.php +++ b/src/Tasks/LDAPGroupSyncTask.php @@ -4,12 +4,13 @@ use Exception; use SilverStripe\Control\Director; -use SilverStripe\Control\HTTPRequest; use SilverStripe\Dev\BuildTask; -use SilverStripe\Dev\Deprecation; +use SilverStripe\PolyExecution\PolyOutput; use SilverStripe\LDAP\Services\LDAPService; use SilverStripe\ORM\DB; use SilverStripe\Security\Group; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; /** * Class LDAPGroupSyncTask @@ -18,11 +19,7 @@ */ class LDAPGroupSyncTask extends BuildTask { - /** - * {@inheritDoc} - * @var string - */ - private static $segment = 'LDAPGroupSyncTask'; + protected static string $commandName = 'LDAPGroupSyncTask'; /** * @var array @@ -45,19 +42,12 @@ class LDAPGroupSyncTask extends BuildTask */ protected $ldapService; - /** - * @return string - */ - public function getTitle() + public function getTitle(): string { return _t(__CLASS__ . '.SYNCTITLE', 'Sync all groups from LDAP'); } - /** - * {@inheritDoc} - * @var HTTPRequest $request - */ - public function run($request) + protected function execute(InputInterface $input, PolyOutput $output): int { ini_set('max_execution_time', 900); @@ -71,8 +61,6 @@ public function run($request) 'objectguid' ); - $start = time(); - $created = 0; $updated = 0; $deleted = 0; @@ -85,31 +73,27 @@ public function run($request) $group = new Group(); $group->GUID = $data['objectguid']; - Deprecation::withSuppressedNotice(function () use ($data) { - $this->log(sprintf( - 'Creating new Group (GUID: %s, sAMAccountName: %s)', - $data['objectguid'], - $data['samaccountname'] - )); - }); + $output->writeln(sprintf( + 'Creating new Group (GUID: %s, sAMAccountName: %s)', + $data['objectguid'], + $data['samaccountname'] + )); $created++; } else { - Deprecation::withSuppressedNotice(function () use ($group, $data) { - $this->log(sprintf( - 'Updating existing Group "%s" (ID: %s, GUID: %s, sAMAccountName: %s)', - $group->getTitle(), - $group->ID, - $data['objectguid'], - $data['samaccountname'] - )); - }); + $output->writeln(sprintf( + 'Updating existing Group "%s" (ID: %s, GUID: %s, sAMAccountName: %s)', + $group->getTitle(), + $group->ID, + $data['objectguid'], + $data['samaccountname'] + )); $updated++; } try { $this->ldapService->updateGroupFromLDAP($group, $data); } catch (Exception $e) { - Deprecation::withSuppressedNotice(fn() => $this->log($e->getMessage())); + $output->writeln('' . $e->getMessage() . ''); continue; } } @@ -121,13 +105,11 @@ public function run($request) if (!isset($ldapGroups[$record['GUID']])) { $group = Group::get()->byId($record['ID']); - Deprecation::withSuppressedNotice(function () use ($group) { - $this->log(sprintf( - 'Removing Group "%s" (GUID: %s) that no longer exists in LDAP.', - $group->Title, - $group->GUID - )); - }); + $output->writeln(sprintf( + 'Removing Group "%s" (GUID: %s) that no longer exists in LDAP.', + $group->Title, + $group->GUID + )); try { // Cascade into mappings, just to clean up behind ourselves. @@ -136,7 +118,7 @@ public function run($request) } $group->delete(); } catch (Exception $e) { - Deprecation::withSuppressedNotice(fn() => $this->log($e->getMessage())); + $output->writeln('' . $e->getMessage() . ''); continue; } @@ -145,32 +127,15 @@ public function run($request) } } - $this->invokeWithExtensions('onAfterLDAPGroupSyncTask'); - - $end = time() - $start; - - Deprecation::withSuppressedNotice(function () use ($created, $updated, $deleted, $end) { - $this->log(sprintf( - 'Done. Created %s records. Updated %s records. Deleted %s records. Duration: %s seconds', - $created, - $updated, - $deleted, - round($end ?? 0.0, 0) - )); - }); - } + $this->invokeWithExtensions('onAfterLDAPGroupSyncTask', $output); - /** - * Sends a message, formatted either for the CLI or browser - * - * @param string $message - * @deprecated 2.3.0 Will be replaced with new $output parameter in the run() method - */ - protected function log($message) - { - Deprecation::notice('2.3.0', 'Will be replaced with new $output parameter in the run() method'); - $message = sprintf('[%s] ', date('Y-m-d H:i:s')) . $message; - echo Director::is_cli() ? ($message . PHP_EOL) : ($message . '
'); + $output->writeln(sprintf( + 'Done. Created %s records. Updated %s records. Deleted %s records.', + $created, + $updated, + $deleted + )); + return Command::SUCCESS; } /** diff --git a/src/Tasks/LDAPMemberSyncOneTask.php b/src/Tasks/LDAPMemberSyncOneTask.php index 5b1902b..0afb916 100644 --- a/src/Tasks/LDAPMemberSyncOneTask.php +++ b/src/Tasks/LDAPMemberSyncOneTask.php @@ -3,9 +3,11 @@ namespace SilverStripe\LDAP\Tasks; use Exception; -use SilverStripe\Control\HTTPRequest; -use SilverStripe\Dev\Deprecation; +use SilverStripe\PolyExecution\PolyOutput; use SilverStripe\LDAP\Services\LDAPService; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; /** * Class LDAPMemberSyncOneTask @@ -13,15 +15,11 @@ * * Debug build task that can be used to sync a single member by providing their email address registered in LDAP. * - * Usage: /dev/tasks/LDAPMemberSyncOneTask?mail=john.smith@example.com + * Usage: sake tasks:LDAPMemberSyncOneTask --email=john.smith@example.com */ class LDAPMemberSyncOneTask extends LDAPMemberSyncTask { - /** - * {@inheritDoc} - * @var string - */ - private static $segment = 'LDAPMemberSyncOneTask'; + protected static string $commandName = 'LDAPMemberSyncOneTask'; /** * @var array @@ -35,50 +33,40 @@ class LDAPMemberSyncOneTask extends LDAPMemberSyncTask */ protected $ldapService; - /** - * @return string - */ - public function getTitle() + public function getTitle(): string { return _t(__CLASS__ . '.SYNCONETITLE', 'Sync single user from LDAP'); } - /** - * Syncs a single user based on the email address passed in the URL - * - * @param HTTPRequest $request - */ - public function run($request) + protected function execute(InputInterface $input, PolyOutput $output): int { - $email = $request->getVar('email'); + $email = $input->getOption('email'); if (!$email) { - echo 'You must supply an email parameter to this method.', PHP_EOL; - exit; + $output->writeln('You must supply an email address.'); + return Command::INVALID; } $user = $this->ldapService->getUserByEmail($email); if (!$user) { - echo sprintf('No user found in LDAP for email %s', $email), PHP_EOL; - exit; + $output->writeln(sprintf('No user found in LDAP for email %s', $email)); + return Command::FAILURE; } $member = $this->findOrCreateMember($user); // If member exists already, we're updating - otherwise we're creating if ($member->exists()) { - Deprecation::withSuppressedNotice(function () use ($user, $member) { - $this->log(sprintf( - 'Updating existing Member %s: "%s" (ID: %s, SAM Account Name: %s)', - $user['objectguid'], - $member->getName(), - $member->ID, - $user['samaccountname'] - )); - }); + $output->writeln(sprintf( + 'Updating existing Member %s: "%s" (ID: %s, SAM Account Name: %s)', + $user['objectguid'], + $member->getName(), + $member->ID, + $user['samaccountname'] + )); } else { - $this->log(sprintf( + $output->writeln(sprintf( 'Creating new Member %s: "%s" (SAM Account Name: %s)', $user['objectguid'], $user['cn'], @@ -86,17 +74,17 @@ public function run($request) )); } - Deprecation::withSuppressedNotice(function () use ($user) { - $this->log('User data returned from LDAP follows:'); - $this->log(var_export($user)); - }); + $output->writeln('User data returned from LDAP follows:'); + $output->writeln(var_export($user, true)); try { $this->ldapService->updateMemberFromLDAP($member, $user); - Deprecation::withSuppressedNotice(fn() => $this->log('Done!')); } catch (Exception $e) { - Deprecation::withSuppressedNotice(fn() => $this->log($e->getMessage())); + $output->writeln('' . $e->getMessage() . ''); + return Command::FAILURE; } + + return Command::SUCCESS; } /** @@ -108,4 +96,11 @@ public function setLDAPService(LDAPService $service) $this->ldapService = $service; return $this; } + + public function getOptions(): array + { + return [ + new InputOption('email', null, InputOption::VALUE_REQUIRED, 'Email address of the member to sync (required)'), + ]; + } } diff --git a/src/Tasks/LDAPMemberSyncTask.php b/src/Tasks/LDAPMemberSyncTask.php index 1b2effd..7179ac4 100644 --- a/src/Tasks/LDAPMemberSyncTask.php +++ b/src/Tasks/LDAPMemberSyncTask.php @@ -4,13 +4,14 @@ use Exception; use SilverStripe\Control\Director; -use SilverStripe\Control\HTTPRequest; use SilverStripe\Core\Config\Config; use SilverStripe\Dev\BuildTask; -use SilverStripe\Dev\Deprecation; +use SilverStripe\PolyExecution\PolyOutput; use SilverStripe\LDAP\Services\LDAPService; use SilverStripe\ORM\DB; use SilverStripe\Security\Member; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; /** * Class LDAPMemberSyncTask @@ -19,11 +20,7 @@ */ class LDAPMemberSyncTask extends BuildTask { - /** - * {@inheritDoc} - * @var string - */ - private static $segment = 'LDAPMemberSyncTask'; + protected static string $commandName = 'LDAPMemberSyncTask'; /** * @var array @@ -46,19 +43,12 @@ class LDAPMemberSyncTask extends BuildTask */ protected $ldapService; - /** - * @return string - */ - public function getTitle() + public function getTitle(): string { return _t(__CLASS__ . '.SYNCTITLE', 'Sync all users from Active Directory'); } - /** - * {@inheritDoc} - * @param HTTPRequest $request - */ - public function run($request) + protected function execute(InputInterface $input, PolyOutput $output): int { ini_set('max_execution_time', 3600); // 3600s = 1hr ini_set('memory_limit', '1024M'); // 1GB memory limit @@ -71,8 +61,6 @@ public function run($request) array_keys(Config::inst()->get(Member::class, 'ldap_field_mappings') ?? []) )); - $start = time(); - $created = 0; $updated = 0; $deleted = 0; @@ -83,25 +71,21 @@ public function run($request) // If member exists already, we're updating - otherwise we're creating if ($member->exists()) { $updated++; - Deprecation::withSuppressedNotice(function () use ($data, $member) { - $this->log(sprintf( - 'Updating existing Member %s: "%s" (ID: %s, SAM Account Name: %s)', - $data['objectguid'], - $member->getName(), - $member->ID, - $data['samaccountname'] - )); - }); + $output->writeln(sprintf( + 'Updating existing Member %s: "%s" (ID: %s, SAM Account Name: %s)', + $data['objectguid'], + $member->getName(), + $member->ID, + $data['samaccountname'] + )); } else { $created++; - Deprecation::withSuppressedNotice(function () use ($data) { - $this->log(sprintf( - 'Creating new Member %s: "%s" (SAM Account Name: %s)', - $data['objectguid'], - $data['cn'], - $data['samaccountname'] - )); - }); + $output->writeln(sprintf( + 'Creating new Member %s: "%s" (SAM Account Name: %s)', + $data['objectguid'], + $data['cn'], + $data['samaccountname'] + )); } // Sync attributes from LDAP to the Member record. This will also write the Member record. @@ -109,7 +93,7 @@ public function run($request) try { $this->ldapService->updateMemberFromLDAP($member, $data); } catch (Exception $e) { - Deprecation::withSuppressedNotice(fn() => $this->log($e->getMessage())); + $output->writeln('' . $e->getMessage() . ''); continue; } } @@ -121,18 +105,16 @@ public function run($request) $member = Member::get()->byId($record['ID']); if (!isset($users[$record['GUID']])) { - Deprecation::withSuppressedNotice(function () use ($member) { - $this->log(sprintf( - 'Removing Member "%s" (GUID: %s) that no longer exists in LDAP.', - $member->getName(), - $member->GUID - )); - }); + $output->writeln(sprintf( + 'Removing Member "%s" (GUID: %s) that no longer exists in LDAP.', + $member->getName(), + $member->GUID + )); try { $member->delete(); } catch (Exception $e) { - Deprecation::withSuppressedNotice(fn() => $this->log($e->getMessage())); + $output->writeln('' . $e->getMessage() . ''); continue; } @@ -141,32 +123,15 @@ public function run($request) } } - $this->invokeWithExtensions('onAfterLDAPMemberSyncTask'); - - $end = time() - $start; - - Deprecation::withSuppressedNotice(function () use ($created, $updated, $deleted, $end) { - $this->log(sprintf( - 'Done. Created %s records. Updated %s records. Deleted %s records. Duration: %s seconds', - $created, - $updated, - $deleted, - round($end ?? 0.0, 0) - )); - }); - } + $this->invokeWithExtensions('onAfterLDAPMemberSyncTask', $output); - /** - * Sends a message, formatted either for the CLI or browser - * - * @param string $message - * @deprecated 2.3.0 Will be replaced with new $output parameter in the run() method - */ - protected function log($message) - { - Deprecation::notice('2.3.0', 'Will be replaced with new $output parameter in the run() method'); - $message = sprintf('[%s] ', date('Y-m-d H:i:s')) . $message; - echo Director::is_cli() ? ($message . PHP_EOL) : ($message . '
'); + $output->writeln(sprintf( + 'Done. Created %s records. Updated %s records. Deleted %s records.', + $created, + $updated, + $deleted + )); + return Command::SUCCESS; } /** diff --git a/src/Tasks/LDAPMigrateExistingMembersTask.php b/src/Tasks/LDAPMigrateExistingMembersTask.php index 205f45f..40b6da7 100644 --- a/src/Tasks/LDAPMigrateExistingMembersTask.php +++ b/src/Tasks/LDAPMigrateExistingMembersTask.php @@ -2,13 +2,13 @@ namespace SilverStripe\LDAP\Tasks; -use SilverStripe\Control\Director; -use SilverStripe\Control\HTTPRequest; use SilverStripe\Core\Convert; use SilverStripe\Dev\BuildTask; -use SilverStripe\Dev\Deprecation; +use SilverStripe\PolyExecution\PolyOutput; use SilverStripe\LDAP\Services\LDAPService; use SilverStripe\Security\Member; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; /** * Class LDAPMigrateExistingMembersTask @@ -18,11 +18,7 @@ */ class LDAPMigrateExistingMembersTask extends BuildTask { - /** - * {@inheritDoc} - * @var string - */ - private static $segment = 'LDAPMigrateExistingMembersTask'; + protected static string $commandName = 'LDAPMigrateExistingMembersTask'; /** * @var array @@ -36,22 +32,14 @@ class LDAPMigrateExistingMembersTask extends BuildTask */ public $ldapService; - /** - * @return string - */ - public function getTitle() + public function getTitle(): string { return _t(__CLASS__ . '.TITLE', 'Migrate existing members in SilverStripe into LDAP members'); } - /** - * {@inheritDoc} - * @param HTTPRequest $request - */ - public function run($request) + protected function execute(InputInterface $input, PolyOutput $output): int { $users = $this->ldapService->getUsers(['objectguid', 'mail']); - $start = time(); $count = 0; foreach ($users as $user) { @@ -74,33 +62,15 @@ public function run($request) $count++; - Deprecation::withSuppressedNotice(function () use ($member) { - $this->log(sprintf( - 'Migrated Member %s (ID: %s, Email: %s)', - $member->getName(), - $member->ID, - $member->Email - )); - }); + $output->writeln(sprintf( + 'Migrated Member %s (ID: %s, Email: %s)', + $member->getName(), + $member->ID, + $member->Email + )); } - $end = time() - $start; - - Deprecation::withSuppressedNotice(function () use ($count, $end) { - $this->log(sprintf('Done. Migrated %s Member records. Duration: %s seconds', $count, round($end ?? 0.0, 0))); - }); - } - - /** - * Sends a message, formatted either for the CLI or browser - * - * @param string $message - * @deprecated 2.3.0 Will be replaced with new $output parameter in the run() method - */ - protected function log($message) - { - Deprecation::notice('2.3.0', 'Will be replaced with new $output parameter in the run() method'); - $message = sprintf('[%s] ', date('Y-m-d H:i:s')) . $message; - echo Director::is_cli() ? ($message . PHP_EOL) : ($message . '
'); + $output->writeln("Done. Migrated $count Member records."); + return Command::SUCCESS; } }