Skip to content

Commit

Permalink
add generate-password option and flow fixes
Browse files Browse the repository at this point in the history 
Signed-off-by: Anupam Kumar <[email protected]>
  • Loading branch information
@kyteinsky
kyteinsky committed Feb 23, 2024
1 parent bf7e84f commit ce24923
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 71 deletions.
58 changes: 27 additions & 31 deletions core/Command/User/Add.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
use OC\Files\Filesystem;
use OCA\Settings\Mailer\NewUserMailHelper;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IConfig;
use OCP\IAppConfig;
use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IUser;
Expand All @@ -50,15 +50,15 @@ public function __construct(
protected IUserManager $userManager,
protected IGroupManager $groupManager,
protected IMailer $mailer,
private IConfig $config,
private IAppConfig $appConfig,
private NewUserMailHelper $mailHelper,
private IEventDispatcher $eventDispatcher,
private ISecureRandom $secureRandom,
) {
parent::__construct();
}

protected function configure() {
protected function configure(): void {
$this
->setName('user:add')
->setDescription('adds an account')
Expand All @@ -73,6 +73,12 @@ protected function configure() {
InputOption::VALUE_NONE,
'read password from environment variable OC_PASS'
)
->addOption(
'generate-password',
null,
InputOption::VALUE_NONE,
'Generate a secure password. A welcome email with a reset link will be sent to the user via an email if --email option and newUser.sendEmail config are set'
)
->addOption(
'display-name',
null,
Expand All @@ -89,7 +95,7 @@ protected function configure() {
'email',
null,
InputOption::VALUE_REQUIRED,
'When set, users may register using the default E-Mail verification workflow'
'When set, users may register using the default email verification workflow'
);
}

Expand All @@ -101,19 +107,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}

$password = '';
$sendPasswordEmail = false;

$email = $input->getOption('email');
if (!empty($email)) {
if (!$this->mailer->validateMailAddress($email)) {
$output->writeln(\sprintf(
'<error>The given E-Mail address "%s" is invalid</error>',
$email,
));

return 1;
}
}

// Setup password.
if ($input->getOption('password-from-env')) {
Expand All @@ -123,13 +116,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$output->writeln('<error>--password-from-env given, but OC_PASS is empty!</error>');
return 1;
}
} elseif (!empty($email)) {

} elseif ($input->getOption('generate-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 @@ -147,7 +137,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
return 1;
}
} else {
$output->writeln("<error>Interactive input or --password-from-env is needed for entering a password!</error>");
$output->writeln("<error>Interactive input or --password-from-env or --generate-password is needed for setting a password!</error>");
return 1;
}

Expand All @@ -173,10 +163,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$output->writeln('Display name set to "' . $user->getDisplayName() . '"');
}

if (!empty($email)) {
$user->setSystemEMailAddress($email);
}

$groups = $input->getOption('group');

if (!empty($groups)) {
Expand All @@ -200,15 +186,25 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}
}

// Send email to user if we set a temporary password
if ($sendPasswordEmail) {
$email = $input->getOption('email');
if (!empty($email)) {
if (!$this->mailer->validateMailAddress($email)) {
$output->writeln(\sprintf(
'<error>The given email address "%s" is invalid. Email not set for the user.</error>',
$email,
));

return 1;
}

$user->setSystemEMailAddress($email);

if ($this->config->getAppValue('core', 'newUser.sendEmail', 'yes') === 'yes') {
if ($this->appConfig->getValueString('core', 'newUser.sendEmail', 'yes') === 'yes') {
try {
$this->mailHelper->sendMail($user, $this->mailHelper->generateTemplate($user, true));
$output->writeln('Invitation E-Mail sent to ' . $email);
$output->writeln('Welcome email sent to ' . $email);
} catch (\Exception $e) {
$output->writeln('Unable to send the invitation mail to ' . $email);
$output->writeln('Unable to send the welcome email to ' . $email);
}
}
}
Expand Down
122 changes: 82 additions & 40 deletions tests/Core/Command/User/AddTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
use OC\Core\Command\User\Add;
use OCA\Settings\Mailer\NewUserMailHelper;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IConfig;
use OCP\IAppConfig;
use OCP\IGroupManager;
use OCP\IUser;
use OCP\IUserManager;
Expand All @@ -40,61 +40,103 @@
use Test\TestCase;

class AddTest extends TestCase {
/** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */
private $userManager;

/** @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject */
private $groupManager;

/** @var IMailer|\PHPUnit\Framework\MockObject\MockObject */
private $mailer;

/** @var IAppConfig|\PHPUnit\Framework\MockObject\MockObject */
private $appConfig;

/** @var NewUserMailHelper|\PHPUnit\Framework\MockObject\MockObject */
private $mailHelper;

/** @var IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */
private $eventDispatcher;

/** @var ISecureRandom|\PHPUnit\Framework\MockObject\MockObject */
private $secureRandom;

/** @var IUser|\PHPUnit\Framework\MockObject\MockObject */
private $user;

/** @var InputInterface|\PHPUnit\Framework\MockObject\MockObject */
private $consoleInput;

/** @var OutputInterface|\PHPUnit\Framework\MockObject\MockObject */
private $consoleOutput;

/** @var Add */
private $addCommand;

public function setUp(): void {
parent::setUp();

$this->userManager = static::createMock(IUserManager::class);
$this->groupManager = static::createStub(IGroupManager::class);
$this->mailer = static::createMock(IMailer::class);
$this->appConfig = static::createMock(IAppConfig::class);
$this->mailHelper = static::createMock(NewUserMailHelper::class);
$this->eventDispatcher = static::createStub(IEventDispatcher::class);
$this->secureRandom = static::createStub(ISecureRandom::class);

$this->user = static::createMock(IUser::class);

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

$this->addCommand = new Add(
$this->userManager,
$this->groupManager,
$this->mailer,
$this->appConfig,
$this->mailHelper,
$this->eventDispatcher,
$this->secureRandom
);
}

/**
* @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);
$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())
public function testAddEmail(
?string $email,
bool $isEmailValid,
bool $shouldSendEmail,
): void {
$this->user->expects($isEmailValid ? static::once() : static::never())
->method('setSystemEMailAddress')
->with(static::equalTo($email));

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

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

$mailer->method('validateMailAddress')
->willReturn($isValid);
$this->mailer->method('validateMailAddress')
->willReturn($isEmailValid);

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

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

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

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

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

Expand All @@ -111,12 +153,12 @@ public function addEmailDataProvider(): array {
'Invalid E-Mail' => [
'info@@example.com',
false,
true,
false,
],
'No E-Mail' => [
'',
false,
true,
false,
],
'Valid E-Mail, but no mail should be sent' => [
'[email protected]',
Expand Down

0 comments on commit ce24923

Please sign in to comment.