diff --git a/core/Command/User/Add.php b/core/Command/User/Add.php index b670756c7aef1..5ccd9b819f0ef 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 user "' . $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 (\mb_strlen($input->getOption('email')) > 0) { + 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(\sprintf('Display name set to "%s"', $user->getDisplayName())); } $groups = $input->getOption('group'); @@ -257,18 +192,20 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln('User "' . $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 && $this->config->getAppValue('core', 'newUser.sendEmail', 'yes') === 'yes') { + $email = $input->getOption('email'); + $user->setSystemEMailAddress($email); - $this->eventDispatcher->dispatchTyped($passwordEvent); + try { + $this->mailHelper->sendMail($user, $this->mailHelper->generateTemplate($user, true)); + $output->writeln(\sprintf('Invitation E-Mail sent to %s', $email)); + } catch (\Exception $e) { + $output->writeln(\sprintf('Unable to send the invitation mail to %s', $email)); + } + } - return $passwordEvent->getPassword() ?? $this->secureRandom->generate(20); + return 0; } } diff --git a/core/register_command.php b/core/register_command.php index fe6f78458337a..8e531a02b5a6f 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -5,6 +5,7 @@ /** * @copyright Copyright (c) 2016, ownCloud, Inc. * + * @author Anupam Kumar * @author Bart Visscher * @author Björn Schießle * @author Christian Kampka @@ -183,7 +184,8 @@ $application->add(\OC::$server->query(\OC\Core\Command\Preview\Repair::class)); $application->add(\OC::$server->query(\OC\Core\Command\Preview\ResetRenderedTexts::class)); - $application->add(new OC\Core\Command\User\Add(\OC::$server->getUserManager(), \OC::$server->getGroupManager(), \OC::$server->get(Egulias\EmailValidator\EmailValidator::class), \OC::$server->get(\OC\AllConfig::class), \OC::$server->get(\OCA\Settings\Mailer\NewUserMailHelper::class), \OC::$server->get(\OCP\EventDispatcher\IEventDispatcher::class), \OC::$server->get(\OCP\Security\ISecureRandom::class))); + $application->add(new OC\Core\Command\User\Add(\OC::$server->get(\OCP\IUserManager::class), \OC::$server->get(\OCP\IGroupManager::class + ), \OC::$server->get(\OCP\Mail\IMailer::class), \OC::$server->get(\OC\AllConfig::class), \OC::$server->get(\OCA\Settings\Mailer\NewUserMailHelper::class), \OC::$server->get(\OCP\EventDispatcher\IEventDispatcher::class), \OC::$server->get(\OCP\Security\ISecureRandom::class))); $application->add(new OC\Core\Command\User\Delete(\OC::$server->getUserManager())); $application->add(new OC\Core\Command\User\Disable(\OC::$server->getUserManager())); $application->add(new OC\Core\Command\User\Enable(\OC::$server->getUserManager())); diff --git a/tests/Core/Command/User/AddTest.php b/tests/Core/Command/User/AddTest.php index 1445e3cfec6b2..d37e61ad9f0a9 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); @@ -56,7 +56,7 @@ public function testAddEmail(?string $email, bool $isValid, bool $shouldSendMail $consoleInput = static::createMock(InputInterface::class); $consoleOutput = static::createMock(OutputInterface::class); - $user->expects($isValid ? static::once() : static::never()) + $user->expects($isValid && $shouldSendMail ? static::once() : static::never()) ->method('setSystemEMailAddress') ->with(static::equalTo($email)); @@ -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, + ], ]; } }