-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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.
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
Enable adding E-Mail addresses to new user accounts using the CLI #29368
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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() { | ||
|
@@ -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()); | ||
$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; | ||
|
@@ -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>"); | ||
|
@@ -118,17 +180,32 @@ protected function execute(InputInterface $input, OutputInterface $output): int | |
return 1; | ||
} | ||
|
||
if (trim($password) === '' && $emailIsSet) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
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 { | ||
|
@@ -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') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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'); | ||
|
@@ -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); | ||
} | ||
} |
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, | ||
]; | ||
} | ||
} |
There was a problem hiding this comment.
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.