Skip to content

Commit 7c218ca

Browse files
kyteinskykyteinsky
committed
add generate-password option and flow fixes
Signed-off-by: Anupam Kumar <kyteinsky@gmail.com>
1 parent 2f447f1 commit 7c218ca

File tree

2 files changed

+109
-71
lines changed

2 files changed

+109
-71
lines changed

‎core/Command/User/Add.php‎

Lines changed: 27 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
use OC\Files\Filesystem;
3030
use OCA\Settings\Mailer\NewUserMailHelper;
3131
use OCP\EventDispatcher\IEventDispatcher;
32-
use OCP\IConfig;
32+
use OCP\IAppConfig;
3333
use OCP\IGroup;
3434
use OCP\IGroupManager;
3535
use OCP\IUser;
@@ -50,15 +50,15 @@ public function __construct(
5050
protected IUserManager $userManager,
5151
protected IGroupManager $groupManager,
5252
protected IMailer $mailer,
53-
private IConfig $config,
53+
private IAppConfig $appConfig,
5454
private NewUserMailHelper $mailHelper,
5555
private IEventDispatcher $eventDispatcher,
5656
private ISecureRandom $secureRandom,
5757
) {
5858
parent::__construct();
5959
}
6060

61-
protected function configure() {
61+
protected function configure(): void {
6262
$this
6363
->setName('user:add')
6464
->setDescription('adds an account')
@@ -73,6 +73,12 @@ protected function configure() {
7373
InputOption::VALUE_NONE,
7474
'read password from environment variable OC_PASS'
7575
)
76+
->addOption(
77+
'generate-password',
78+
null,
79+
InputOption::VALUE_NONE,
80+
'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'
81+
)
7682
->addOption(
7783
'display-name',
7884
null,
@@ -89,7 +95,7 @@ protected function configure() {
8995
'email',
9096
null,
9197
InputOption::VALUE_REQUIRED,
92-
'When set, users may register using the default E-Mail verification workflow'
98+
'When set, users may register using the default email verification workflow'
9399
);
94100
}
95101

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

103109
$password = '';
104-
$sendPasswordEmail = false;
105-
106-
$email = $input->getOption('email');
107-
if (!empty($email)) {
108-
if (!$this->mailer->validateMailAddress($email)) {
109-
$output->writeln(\sprintf(
110-
'<error>The given E-Mail address "%s" is invalid</error>',
111-
$email,
112-
));
113-
114-
return 1;
115-
}
116-
}
117110

118111
// Setup password.
119112
if ($input->getOption('password-from-env')) {
@@ -123,13 +116,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int
123116
$output->writeln('<error>--password-from-env given, but OC_PASS is empty!</error>');
124117
return 1;
125118
}
126-
} elseif (!empty($email)) {
127-
119+
} elseif ($input->getOption('generate-password')) {
128120
$passwordEvent = new GenerateSecurePasswordEvent();
129121
$this->eventDispatcher->dispatchTyped($passwordEvent);
130122
$password = $passwordEvent->getPassword() ?? $this->secureRandom->generate(20);
131-
132-
$sendPasswordEmail = true;
133123
} elseif ($input->isInteractive()) {
134124
/** @var QuestionHelper $helper */
135125
$helper = $this->getHelper('question');
@@ -147,7 +137,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
147137
return 1;
148138
}
149139
} else {
150-
$output->writeln("<error>Interactive input or --password-from-env is needed for entering a password!</error>");
140+
$output->writeln("<error>Interactive input or --password-from-env or --generate-password is needed for setting a password!</error>");
151141
return 1;
152142
}
153143

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

176-
if (!empty($email)) {
177-
$user->setSystemEMailAddress($email);
178-
}
179-
180166
$groups = $input->getOption('group');
181167

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

203-
// Send email to user if we set a temporary password
204-
if ($sendPasswordEmail) {
189+
$email = $input->getOption('email');
190+
if (!empty($email)) {
191+
if (!$this->mailer->validateMailAddress($email)) {
192+
$output->writeln(\sprintf(
193+
'<error>The given email address "%s" is invalid. Email not set for the user.</error>',
194+
$email,
195+
));
196+
197+
return 1;
198+
}
199+
200+
$user->setSystemEMailAddress($email);
205201

206-
if ($this->config->getAppValue('core', 'newUser.sendEmail', 'yes') === 'yes') {
202+
if ($this->appConfig->getValueString('core', 'newUser.sendEmail', 'yes') === 'yes') {
207203
try {
208204
$this->mailHelper->sendMail($user, $this->mailHelper->generateTemplate($user, true));
209-
$output->writeln('Invitation E-Mail sent to ' . $email);
205+
$output->writeln('Welcome email sent to ' . $email);
210206
} catch (\Exception $e) {
211-
$output->writeln('Unable to send the invitation mail to ' . $email);
207+
$output->writeln('Unable to send the welcome email to ' . $email);
212208
}
213209
}
214210
}

‎tests/Core/Command/User/AddTest.php‎

Lines changed: 82 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
use OC\Core\Command\User\Add;
2929
use OCA\Settings\Mailer\NewUserMailHelper;
3030
use OCP\EventDispatcher\IEventDispatcher;
31-
use OCP\IConfig;
31+
use OCP\IAppConfig;
3232
use OCP\IGroupManager;
3333
use OCP\IUser;
3434
use OCP\IUserManager;
@@ -40,61 +40,103 @@
4040
use Test\TestCase;
4141

4242
class AddTest extends TestCase {
43+
/** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */
44+
private $userManager;
45+
46+
/** @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject */
47+
private $groupManager;
48+
49+
/** @var IMailer|\PHPUnit\Framework\MockObject\MockObject */
50+
private $mailer;
51+
52+
/** @var IAppConfig|\PHPUnit\Framework\MockObject\MockObject */
53+
private $appConfig;
54+
55+
/** @var NewUserMailHelper|\PHPUnit\Framework\MockObject\MockObject */
56+
private $mailHelper;
57+
58+
/** @var IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */
59+
private $eventDispatcher;
60+
61+
/** @var ISecureRandom|\PHPUnit\Framework\MockObject\MockObject */
62+
private $secureRandom;
63+
64+
/** @var IUser|\PHPUnit\Framework\MockObject\MockObject */
65+
private $user;
66+
67+
/** @var InputInterface|\PHPUnit\Framework\MockObject\MockObject */
68+
private $consoleInput;
69+
70+
/** @var OutputInterface|\PHPUnit\Framework\MockObject\MockObject */
71+
private $consoleOutput;
72+
73+
/** @var Add */
74+
private $addCommand;
75+
76+
public function setUp(): void {
77+
parent::setUp();
78+
79+
$this->userManager = static::createMock(IUserManager::class);
80+
$this->groupManager = static::createStub(IGroupManager::class);
81+
$this->mailer = static::createMock(IMailer::class);
82+
$this->appConfig = static::createMock(IAppConfig::class);
83+
$this->mailHelper = static::createMock(NewUserMailHelper::class);
84+
$this->eventDispatcher = static::createStub(IEventDispatcher::class);
85+
$this->secureRandom = static::createStub(ISecureRandom::class);
86+
87+
$this->user = static::createMock(IUser::class);
88+
89+
$this->consoleInput = static::createMock(InputInterface::class);
90+
$this->consoleOutput = static::createMock(OutputInterface::class);
91+
92+
$this->addCommand = new Add(
93+
$this->userManager,
94+
$this->groupManager,
95+
$this->mailer,
96+
$this->appConfig,
97+
$this->mailHelper,
98+
$this->eventDispatcher,
99+
$this->secureRandom
100+
);
101+
}
102+
43103
/**
44104
* @dataProvider addEmailDataProvider
45105
*/
46-
public function testAddEmail(?string $email, bool $isValid, bool $shouldSendMail): void {
47-
$userManager = static::createMock(IUserManager::class);
48-
$groupManager = static::createStub(IGroupManager::class);
49-
$mailer = static::createMock(IMailer::class);
50-
$user = static::createMock(IUser::class);
51-
$config = static::createMock(IConfig::class);
52-
$mailHelper = static::createMock(NewUserMailHelper::class);
53-
$eventDispatcher = static::createStub(IEventDispatcher::class);
54-
$secureRandom = static::createStub(ISecureRandom::class);
55-
56-
$consoleInput = static::createMock(InputInterface::class);
57-
$consoleOutput = static::createMock(OutputInterface::class);
58-
59-
$user->expects($isValid ? static::once() : static::never())
106+
public function testAddEmail(
107+
?string $email,
108+
bool $isEmailValid,
109+
bool $shouldSendEmail,
110+
): void {
111+
$this->user->expects($isEmailValid ? static::once() : static::never())
60112
->method('setSystemEMailAddress')
61113
->with(static::equalTo($email));
62114

63-
$userManager->method('createUser')
64-
->willReturn($user);
115+
$this->userManager->method('createUser')
116+
->willReturn($this->user);
65117

66-
$config->method('getAppValue')
67-
->willReturn($shouldSendMail ? 'yes' : 'no');
118+
$this->appConfig->method('getValueString')
119+
->willReturn($shouldSendEmail ? 'yes' : 'no');
68120

69-
$mailer->method('validateMailAddress')
70-
->willReturn($isValid);
121+
$this->mailer->method('validateMailAddress')
122+
->willReturn($isEmailValid);
71123

72-
$mailHelper->method('generateTemplate')
124+
$this->mailHelper->method('generateTemplate')
73125
->willReturn(static::createMock(IEMailTemplate::class));
74126

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

78-
$consoleInput->method('getOption')
130+
$this->consoleInput->method('getOption')
79131
->will(static::returnValueMap([
80-
['password-from-env', ''],
132+
['generate-password', 'true'],
81133
['email', $email],
82134
['group', []],
83135
]));
84136

85-
$addCommand = new Add(
86-
$userManager,
87-
$groupManager,
88-
$mailer,
89-
$config,
90-
$mailHelper,
91-
$eventDispatcher,
92-
$secureRandom
93-
);
94-
95-
$this->invokePrivate($addCommand, 'execute', [
96-
$consoleInput,
97-
$consoleOutput
137+
$this->invokePrivate($this->addCommand, 'execute', [
138+
$this->consoleInput,
139+
$this->consoleOutput
98140
]);
99141
}
100142

@@ -111,12 +153,12 @@ public function addEmailDataProvider(): array {
111153
'Invalid E-Mail' => [
112154
'info@@example.com',
113155
false,
114-
true,
156+
false,
115157
],
116158
'No E-Mail' => [
117159
'',
118160
false,
119-
true,
161+
false,
120162
],
121163
'Valid E-Mail, but no mail should be sent' => [
122164
'info@example.com',

0 commit comments

Comments
 (0)