From 2fb62d584b286dacf11775063f3abfed9aa16436 Mon Sep 17 00:00:00 2001 From: Anupam Kumar Date: Sat, 30 Sep 2023 10:47:59 +0530 Subject: [PATCH] new user password email option, improved on #29368 Signed-off-by: Anupam Kumar --- core/Command/User/Add.php | 151 +++++++++------------------- core/register_command.php | 5 - tests/Core/Command/User/AddTest.php | 56 ++++++----- 3 files changed, 74 insertions(+), 138 deletions(-) diff --git a/core/Command/User/Add.php b/core/Command/User/Add.php index 2a47b8015a206..b179b562d209f 100644 --- a/core/Command/User/Add.php +++ b/core/Command/User/Add.php @@ -2,6 +2,7 @@ /** * @copyright Copyright (c) 2016, ownCloud, Inc. * + * @author Anupam Kumar * @author Arthur Schiwon * @author Christoph Wurst * @author Joas Schilling @@ -25,8 +26,6 @@ */ namespace OC\Core\Command\User; -use Egulias\EmailValidator\EmailValidator; -use Egulias\EmailValidator\Validation\RFCValidation; use OC\Files\Filesystem; use OCA\Settings\Mailer\NewUserMailHelper; use OCP\EventDispatcher\IEventDispatcher; @@ -35,6 +34,7 @@ use OCP\IGroupManager; use OCP\IUser; use OCP\IUserManager; +use OCP\Mail\IMailer; use OCP\Security\Events\GenerateSecurePasswordEvent; use OCP\Security\ISecureRandom; use Symfony\Component\Console\Command\Command; @@ -46,63 +46,16 @@ use Symfony\Component\Console\Question\Question; class Add extends Command { - /** - * @var IUserManager - */ - protected $userManager; - - /** - * @var IGroupManager - */ - protected $groupManager; - - /** - * @var EmailValidator - */ - protected $emailValidator; - - /** - * @var IConfig - */ - private $config; - - /** - * @var NewUserMailHelper - */ - private $mailHelper; - - /** - * @var IEventDispatcher - */ - private $eventDispatcher; - - /** - * @var ISecureRandom - */ - private $secureRandom; - - /** - * @param IUserManager $userManager - * @param IGroupManager $groupManager - * @param EmailValidator $emailValidator - */ public function __construct( - IUserManager $userManager, - IGroupManager $groupManager, - EmailValidator $emailValidator, - IConfig $config, - NewUserMailHelper $mailHelper, - IEventDispatcher $eventDispatcher, - ISecureRandom $secureRandom + protected IUserManager $userManager, + protected IGroupManager $groupManager, + protected IMailer $mailer, + private IConfig $config, + private NewUserMailHelper $mailHelper, + private IEventDispatcher $eventDispatcher, + private ISecureRandom $secureRandom, ) { parent::__construct(); - $this->userManager = $userManager; - $this->groupManager = $groupManager; - $this->emailValidator = $emailValidator; - $this->config = $config; - $this->mailHelper = $mailHelper; - $this->eventDispatcher = $eventDispatcher; - $this->secureRandom = $secureRandom; } protected function configure() { @@ -142,16 +95,14 @@ protected function configure() { protected function execute(InputInterface $input, OutputInterface $output): int { $uid = $input->getArgument('uid'); - $emailIsSet = \is_string($input->getOption('email')) && \mb_strlen($input->getOption('email')) > 0; - $emailIsValid = $this->emailValidator->isValid($input->getOption('email') ?? '', new RFCValidation()); - $password = ''; - $temporaryPassword = ''; - if ($this->userManager->userExists($uid)) { $output->writeln('The account "' . $uid . '" already exists.'); return 1; } + $password = ''; + $sendPasswordEmail = false; + if ($input->getOption('password-from-env')) { $password = getenv('OC_PASS'); @@ -159,6 +110,23 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln('--password-from-env given, but OC_PASS is empty!'); return 1; } + } elseif ($input->getOption('email') !== '') { + if (!$this->mailer->validateMailAddress($input->getOption(('email')))) { + $output->writeln(\sprintf( + 'The given E-Mail address "%s" is invalid', + $input->getOption('email'), + )); + + return 1; + } + + $output->writeln('Setting a temporary password.'); + + $passwordEvent = new GenerateSecurePasswordEvent(); + $this->eventDispatcher->dispatchTyped($passwordEvent); + $password = $passwordEvent->getPassword() ?? $this->secureRandom->generate(20); + + $sendPasswordEmail = true; } elseif ($input->isInteractive()) { /** @var QuestionHelper $helper */ $helper = $this->getHelper('question'); @@ -180,26 +148,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 1; } - if (trim($password) === '' && $emailIsSet) { - if ($emailIsValid) { - $output->writeln('Setting a temporary password.'); - - $temporaryPassword = $this->getTemporaryPassword(); - } else { - $output->writeln(\sprintf( - 'The given E-Mail address "%s" is invalid: %s', - $input->getOption('email'), - $this->emailValidator->getError()->description() - )); - - return 1; - } - } - try { $user = $this->userManager->createUser( $input->getArgument('uid'), - $password ?: $temporaryPassword + $password, ); } catch (\Exception $e) { $output->writeln('' . $e->getMessage() . ''); @@ -215,24 +167,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int if ($input->getOption('display-name')) { $user->setDisplayName($input->getOption('display-name')); - $output->writeln(sprintf('Display name set to "%s"', $user->getDisplayName())); - } - - if ($emailIsSet && $emailIsValid) { - $user->setSystemEMailAddress($input->getOption('email')); - $output->writeln(sprintf('E-Mail set to "%s"', (string) $user->getSystemEMailAddress())); - - if (trim($password) === '' && $this->config->getAppValue('core', 'newUser.sendEmail', 'yes') === 'yes') { - try { - $this->mailHelper->sendMail( - $user, - $this->mailHelper->generateTemplate($user, true) - ); - $output->writeln('Invitation E-Mail sent.'); - } catch (\Exception $e) { - $output->writeln(\sprintf('Unable to send the invitation mail to %s', $user->getEMailAddress())); - } - } + $output->writeln('Display name set to "' . $user->getDisplayName() . '"'); } $groups = $input->getOption('group'); @@ -257,18 +192,22 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln('Account "' . $user->getUID() . '" added to group "' . $group->getGID() . '"'); } } - return 0; - } - /** - * @return string - */ - protected function getTemporaryPassword(): string - { - $passwordEvent = new GenerateSecurePasswordEvent(); + // Send email to user if we set a temporary password + if ($sendPasswordEmail) { + $email = $input->getOption('email'); + $user->setSystemEMailAddress($email); - $this->eventDispatcher->dispatchTyped($passwordEvent); + if ($this->config->getAppValue('core', 'newUser.sendEmail', 'yes') === 'yes') { + try { + $this->mailHelper->sendMail($user, $this->mailHelper->generateTemplate($user, true)); + $output->writeln('Invitation E-Mail sent to ' . $email); + } catch (\Exception $e) { + $output->writeln('Unable to send the invitation mail to ' . $email); + } + } + } - return $passwordEvent->getPassword() ?? $this->secureRandom->generate(20); + return 0; } } diff --git a/core/register_command.php b/core/register_command.php index 866416dd3dba3..ac58521490643 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -50,11 +50,6 @@ * along with this program. If not, see * */ -<<<<<<< HEAD -======= - -use Psr\Log\LoggerInterface; ->>>>>>> cc486e4eaa7 (Enable adding E-Mail addresses to new user accounts using the CLI) use OC\Core\Command; use OCP\IConfig; diff --git a/tests/Core/Command/User/AddTest.php b/tests/Core/Command/User/AddTest.php index 1445e3cfec6b2..3dffd713242b0 100644 --- a/tests/Core/Command/User/AddTest.php +++ b/tests/Core/Command/User/AddTest.php @@ -25,7 +25,6 @@ namespace Core\Command\User; -use Egulias\EmailValidator\EmailValidator; use OC\Core\Command\User\Add; use OCA\Settings\Mailer\NewUserMailHelper; use OCP\EventDispatcher\IEventDispatcher; @@ -34,19 +33,20 @@ use OCP\IUser; use OCP\IUserManager; use OCP\Mail\IEMailTemplate; +use OCP\mail\IMailer; use OCP\Security\ISecureRandom; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Test\TestCase; class AddTest extends TestCase { - /** * @dataProvider addEmailDataProvider */ public function testAddEmail(?string $email, bool $isValid, bool $shouldSendMail): void { $userManager = static::createMock(IUserManager::class); $groupManager = static::createStub(IGroupManager::class); + $mailer = static::createMock(IMailer::class); $user = static::createMock(IUser::class); $config = static::createMock(IConfig::class); $mailHelper = static::createMock(NewUserMailHelper::class); @@ -66,6 +66,9 @@ public function testAddEmail(?string $email, bool $isValid, bool $shouldSendMail $config->method('getAppValue') ->willReturn($shouldSendMail ? 'yes' : 'no'); + $mailer->method('validateMailAddress') + ->willReturn($isValid); + $mailHelper->method('generateTemplate') ->willReturn(static::createMock(IEMailTemplate::class)); @@ -82,7 +85,7 @@ public function testAddEmail(?string $email, bool $isValid, bool $shouldSendMail $addCommand = new Add( $userManager, $groupManager, - new EmailValidator(), + $mailer, $config, $mailHelper, $eventDispatcher, @@ -96,31 +99,30 @@ public function testAddEmail(?string $email, bool $isValid, bool $shouldSendMail } /** - * @return \Generator + * @return array */ - public function addEmailDataProvider(): \Generator { - yield 'Valid E-Mail' => [ - 'info@example.com', - true, - true, - ]; - - yield 'Invalid E-Mail' => [ - 'info@@example.com', - false, - true, - ]; - - yield 'No E-Mail' => [ - '', - false, - true, - ]; - - yield 'Valid E-Mail, but no mail should be sent' => [ - 'info@example.com', - true, - false, + public function addEmailDataProvider(): array { + return [ + 'Valid E-Mail' => [ + 'info@example.com', + true, + true, + ], + 'Invalid E-Mail' => [ + 'info@@example.com', + false, + true, + ], + 'No E-Mail' => [ + '', + false, + true, + ], + 'Valid E-Mail, but no mail should be sent' => [ + 'info@example.com', + true, + false, + ], ]; } }