Skip to content

Commit 44f7cce

Browse files
committed
Generate password on addUser by password_policy app
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
1 parent 34c3d0a commit 44f7cce

File tree

2 files changed

+69
-5
lines changed

2 files changed

+69
-5
lines changed

apps/provisioning_api/lib/Controller/UsersController.php

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@
5959
use OCP\IUserSession;
6060
use OCP\L10N\IFactory;
6161
use OCP\Security\ISecureRandom;
62+
use OCP\Security\Events\GenerateSecurePasswordEvent;
63+
use OCP\EventDispatcher\IEventDispatcher;
6264

6365
class UsersController extends AUserData {
6466

@@ -76,6 +78,8 @@ class UsersController extends AUserData {
7678
private $secureRandom;
7779
/** @var RemoteWipe */
7880
private $remoteWipe;
81+
/** @var IEventDispatcher */
82+
private $eventDispatcher;
7983

8084
public function __construct(string $appName,
8185
IRequest $request,
@@ -90,7 +94,8 @@ public function __construct(string $appName,
9094
NewUserMailHelper $newUserMailHelper,
9195
FederatedShareProviderFactory $federatedShareProviderFactory,
9296
ISecureRandom $secureRandom,
93-
RemoteWipe $remoteWipe) {
97+
RemoteWipe $remoteWipe,
98+
IEventDispatcher $eventDispatcher) {
9499
parent::__construct($appName,
95100
$request,
96101
$userManager,
@@ -107,6 +112,7 @@ public function __construct(string $appName,
107112
$this->federatedShareProviderFactory = $federatedShareProviderFactory;
108113
$this->secureRandom = $secureRandom;
109114
$this->remoteWipe = $remoteWipe;
115+
$this->eventDispatcher = $eventDispatcher;
110116
}
111117

112118
/**
@@ -286,9 +292,18 @@ public function addUser(string $userid,
286292
throw new OCSException('To send a password link to the user an email address is required.', 108);
287293
}
288294

289-
$password = $this->secureRandom->generate(10);
290-
// Make sure we pass the password_policy
291-
$password .= $this->secureRandom->generate(2, '$!.,;:-~+*[]{}()');
295+
$passwordEvent = new GenerateSecurePasswordEvent();
296+
$this->eventDispatcher->dispatchTyped($passwordEvent);
297+
298+
$password = $passwordEvent->getPassword();
299+
if ($password === null) {
300+
// Fallback: ensure to pass password_policy in any case
301+
$password = $this->secureRandom->generate(10)
302+
. $this->secureRandom->generate(1, ISecureRandom::CHAR_UPPER)
303+
. $this->secureRandom->generate(1, ISecureRandom::CHAR_LOWER)
304+
. $this->secureRandom->generate(1, ISecureRandom::CHAR_DIGITS)
305+
. $this->secureRandom->generate(1, ISecureRandom::CHAR_SYMBOLS);
306+
}
292307
$generatePasswordResetToken = true;
293308
}
294309

apps/provisioning_api/tests/Controller/UsersControllerTest.php

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
use OCA\Settings\Mailer\NewUserMailHelper;
4949
use OCP\App\IAppManager;
5050
use OCP\AppFramework\Http\DataResponse;
51+
use OCP\EventDispatcher\IEventDispatcher;
5152
use OCP\IConfig;
5253
use OCP\IGroup;
5354
use OCP\IL10N;
@@ -58,6 +59,7 @@
5859
use OCP\IUserSession;
5960
use OCP\L10N\IFactory;
6061
use OCP\Mail\IEMailTemplate;
62+
use OCP\Security\Events\GenerateSecurePasswordEvent;
6163
use OCP\Security\ISecureRandom;
6264
use OCP\UserInterface;
6365
use PHPUnit\Framework\MockObject\MockObject;
@@ -94,6 +96,8 @@ class UsersControllerTest extends TestCase {
9496
private $secureRandom;
9597
/** @var RemoteWipe|MockObject */
9698
private $remoteWipe;
99+
/** @var IEventDispatcher */
100+
private $eventDispatcher;
97101

98102
protected function setUp(): void {
99103
parent::setUp();
@@ -111,6 +115,7 @@ protected function setUp(): void {
111115
$this->federatedShareProviderFactory = $this->createMock(FederatedShareProviderFactory::class);
112116
$this->secureRandom = $this->createMock(ISecureRandom::class);
113117
$this->remoteWipe = $this->createMock(RemoteWipe::class);
118+
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
114119

115120
$this->api = $this->getMockBuilder(UsersController::class)
116121
->setConstructorArgs([
@@ -128,6 +133,7 @@ protected function setUp(): void {
128133
$this->federatedShareProviderFactory,
129134
$this->secureRandom,
130135
$this->remoteWipe,
136+
$this->eventDispatcher,
131137
])
132138
->setMethods(['fillStorageInfo'])
133139
->getMock();
@@ -389,7 +395,8 @@ public function testAddUserSuccessfulWithDisplayName() {
389395
$this->newUserMailHelper,
390396
$this->federatedShareProviderFactory,
391397
$this->secureRandom,
392-
$this->remoteWipe
398+
$this->remoteWipe,
399+
$this->eventDispatcher,
393400
])
394401
->setMethods(['editUser'])
395402
->getMock();
@@ -486,6 +493,46 @@ public function testAddUserSuccessfulGenerateUserID() {
486493
));
487494
}
488495

496+
public function testAddUserSuccessfulGeneratePassword() {
497+
$this->userManager
498+
->expects($this->once())
499+
->method('userExists')
500+
->with('NewUser')
501+
->willReturn(false);
502+
$this->userManager
503+
->expects($this->once())
504+
->method('createUser');
505+
$this->logger
506+
->expects($this->once())
507+
->method('info')
508+
->with('Successful addUser call with userid: NewUser', ['app' => 'ocs_api']);
509+
$loggedInUser = $this->getMockBuilder(IUser::class)
510+
->disableOriginalConstructor()
511+
->getMock();
512+
$loggedInUser
513+
->expects($this->once())
514+
->method('getUID')
515+
->willReturn('adminUser');
516+
$this->userSession
517+
->expects($this->once())
518+
->method('getUser')
519+
->willReturn($loggedInUser);
520+
$this->groupManager
521+
->expects($this->once())
522+
->method('isAdmin')
523+
->with('adminUser')
524+
->willReturn(true);
525+
$this->eventDispatcher
526+
->expects($this->once())
527+
->method('dispatchTyped')
528+
->with(new GenerateSecurePasswordEvent());
529+
530+
$this->assertTrue(key_exists(
531+
'id',
532+
$this->api->addUser('NewUser', '', '', 'foo@bar')->getData()
533+
));
534+
}
535+
489536

490537
public function testAddUserFailedToGenerateUserID() {
491538
$this->expectException(\OCP\AppFramework\OCS\OCSException::class);
@@ -3126,6 +3173,7 @@ public function testGetCurrentUserLoggedIn() {
31263173
$this->federatedShareProviderFactory,
31273174
$this->secureRandom,
31283175
$this->remoteWipe,
3176+
$this->eventDispatcher,
31293177
])
31303178
->setMethods(['getUserData'])
31313179
->getMock();
@@ -3190,6 +3238,7 @@ public function testGetUser() {
31903238
$this->federatedShareProviderFactory,
31913239
$this->secureRandom,
31923240
$this->remoteWipe,
3241+
$this->eventDispatcher,
31933242
])
31943243
->setMethods(['getUserData'])
31953244
->getMock();

0 commit comments

Comments
 (0)