Skip to content

Commit

Permalink
new user password email option, improved on #29368
Browse files Browse the repository at this point in the history 
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
  • Loading branch information
@kyteinsky
kyteinsky committed Sep 30, 2023
1 parent 9083cd5 commit e45d734
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 135 deletions.
149 changes: 43 additions & 106 deletions core/Command/User/Add.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
* @author Anupam Kumar <kyteinsky@gmail.com>
* @author Arthur Schiwon <blizzz@arthur-schiwon.de>
* @author Christoph Wurst <christoph@winzerhof-wurst.at>
* @author Joas Schilling <coding@schilljs.com>
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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() {
Expand Down Expand Up @@ -142,23 +95,38 @@ 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('<error>The user "' . $uid . '" already exists.</error>');
return 1;
}

$password = '';
$sendPasswordEmail = false;

if ($input->getOption('password-from-env')) {
$password = getenv('OC_PASS');

if (!$password) {
$output->writeln('<error>--password-from-env given, but OC_PASS is empty!</error>');
return 1;
}
} elseif (\mb_strlen($input->getOption('email')) > 0) {
if (!$this->mailer->validateMailAddress($input->getOption(('email')))) {
$output->writeln(\sprintf(
'<error>The given E-Mail address "%s" is invalid</error>',
$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');
Expand All @@ -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(
'<error>The given E-Mail address "%s" is invalid: %s</error>',
$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('<error>' . $e->getMessage() . '</error>');
Expand All @@ -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');
Expand All @@ -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;
}
}
4 changes: 3 additions & 1 deletion core/register_command.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
* @author Anupam Kumar <kyteinsky@gmail.com>
* @author Bart Visscher <bartv@thisnet.nl>
* @author Björn Schießle <bjoern@schiessle.org>
* @author Christian Kampka <christian@kampka.net>
Expand Down Expand Up @@ -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()));
Expand Down
58 changes: 30 additions & 28 deletions tests/Core/Command/User/AddTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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));

Expand All @@ -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));

Expand All @@ -82,7 +85,7 @@ public function testAddEmail(?string $email, bool $isValid, bool $shouldSendMail
$addCommand = new Add(
$userManager,
$groupManager,
new EmailValidator(),
$mailer,
$config,
$mailHelper,
$eventDispatcher,
Expand All @@ -96,31 +99,30 @@ public function testAddEmail(?string $email, bool $isValid, bool $shouldSendMail
}

/**
* @return \Generator<string, array>
* @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,
],
];
}
}

0 comments on commit e45d734

Please sign in to comment.