Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable adding E-Mail addresses to new user accounts using the CLI #29368

Closed
wants to merge 1 commit into from
Closed

Enable adding E-Mail addresses to new user accounts using the CLI #29368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 113 additions & 7 deletions core/Command/User/Add.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,18 @@
*/
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;
use OCP\IConfig;
use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IUser;
use OCP\IUserManager;
use OCP\Security\Events\GenerateSecurePasswordEvent;
use OCP\Security\ISecureRandom;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Helper\QuestionHelper;
use Symfony\Component\Console\Input\InputArgument;
Expand All @@ -39,20 +46,63 @@
use Symfony\Component\Console\Question\Question;

class Add extends Command {
/** @var \OCP\IUserManager */
/**
* @var IUserManager
*/
protected $userManager;

/** @var \OCP\IGroupManager */
/**
* @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) {
public function __construct(
IUserManager $userManager,
IGroupManager $groupManager,
EmailValidator $emailValidator,
IConfig $config,
NewUserMailHelper $mailHelper,
IEventDispatcher $eventDispatcher,
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 @@ -81,18 +131,30 @@ protected function configure() {
'g',
InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY,
'groups the user should be added to (The group will be created if it does not exist)'
)
->addOption(
'email',
null,
InputOption::VALUE_REQUIRED,
'When set, users may register using the default E-Mail verification workflow'
);
}

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());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If an invalid email is passed, it should abort with an error right there, it should not continue and do something else that what the caller asked for.

$password = '';
$temporaryPassword = '';

if ($this->userManager->userExists($uid)) {
$output->writeln('<error>The user "' . $uid . '" already exists.</error>');
return 1;
}

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;
Expand All @@ -107,7 +169,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int

$question = new Question('Confirm password: ');
$question->setHidden(true);
$confirm = $helper->ask($input, $output,$question);
$confirm = $helper->ask($input, $output, $question);

if ($password !== $confirm) {
$output->writeln("<error>Passwords did not match!</error>");
Expand All @@ -118,17 +180,32 @@ protected function execute(InputInterface $input, OutputInterface $output): int
return 1;
}

if (trim($password) === '' && $emailIsSet) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should not trim here, I think it will cause weird edge cases when $password is all spaces.
Below you use $password ?: $temporaryPassword, which will evaluate to $password if it is not empty and full of spaces.

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
$password ?: $temporaryPassword
);
} catch (\Exception $e) {
$output->writeln('<error>' . $e->getMessage() . '</error>');
return 1;
}


if ($user instanceof IUser) {
$output->writeln('<info>The user "' . $user->getUID() . '" was created successfully</info>');
} else {
Expand All @@ -138,7 +215,24 @@ protected function execute(InputInterface $input, OutputInterface $output): int

if ($input->getOption('display-name')) {
$user->setDisplayName($input->getOption('display-name'));
$output->writeln('Display name set to "' . $user->getDisplayName() . '"');
$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') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing with trimming, here you can use isset($temporaryPassword) I think.

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()));
}
}
}

$groups = $input->getOption('group');
Expand All @@ -165,4 +259,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}
return 0;
}

/**
* @return string
*/
protected function getTemporaryPassword(): string
{
$passwordEvent = new GenerateSecurePasswordEvent();

$this->eventDispatcher->dispatchTyped($passwordEvent);

return $passwordEvent->getPassword() ?? $this->secureRandom->generate(20);
}
}
3 changes: 2 additions & 1 deletion core/register_command.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/

use Psr\Log\LoggerInterface;

$application->add(new \Stecman\Component\Symfony\Console\BashCompletion\CompletionCommand());
Expand Down Expand Up @@ -177,7 +178,7 @@
$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()));
$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\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
126 changes: 126 additions & 0 deletions tests/Core/Command/User/AddTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
<?php
/**
* @copyright Copyright (c) 2021, Philip Gatzka (philip.gatzka@mailbox.org)
*
* @author Philip Gatzka <philip.gatzka@mailbox.org>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

declare(strict_types=1);

namespace Core\Command\User;

use Egulias\EmailValidator\EmailValidator;
use OC\Core\Command\User\Add;
use OCA\Settings\Mailer\NewUserMailHelper;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IConfig;
use OCP\IGroupManager;
use OCP\IUser;
use OCP\IUserManager;
use OCP\Mail\IEMailTemplate;
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);
$user = static::createMock(IUser::class);
$config = static::createMock(IConfig::class);
$mailHelper = static::createMock(NewUserMailHelper::class);
$eventDispatcher = static::createStub(IEventDispatcher::class);
$secureRandom = static::createStub(ISecureRandom::class);

$consoleInput = static::createMock(InputInterface::class);
$consoleOutput = static::createMock(OutputInterface::class);

$user->expects($isValid ? static::once() : static::never())
->method('setSystemEMailAddress')
->with(static::equalTo($email));

$userManager->method('createUser')
->willReturn($user);

$config->method('getAppValue')
->willReturn($shouldSendMail ? 'yes' : 'no');

$mailHelper->method('generateTemplate')
->willReturn(static::createMock(IEMailTemplate::class));

$mailHelper->expects($isValid && $shouldSendMail ? static::once() : static::never())
->method('sendMail');

$consoleInput->method('getOption')
->will(static::returnValueMap([
['password-from-env', ''],
['email', $email],
['group', []],
]));

$addCommand = new Add(
$userManager,
$groupManager,
new EmailValidator(),
$config,
$mailHelper,
$eventDispatcher,
$secureRandom
);

$this->invokePrivate($addCommand, 'execute', [
$consoleInput,
$consoleOutput
]);
}

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