From c19bdb05e8cf24317d6ea3a58951a4e0102b2e70 Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Sun, 9 Aug 2020 16:36:19 +0200 Subject: [PATCH] Generate password on addUser by password_policy app Signed-off-by: Daniel Kesselberg --- .../lib/Controller/UsersController.php | 23 +++++++-- .../tests/Controller/UsersControllerTest.php | 51 ++++++++++++++++++- 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 6e8613085d..570a02f780 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -59,6 +59,8 @@ use OCP\IUserManager; use OCP\IUserSession; use OCP\L10N\IFactory; use OCP\Security\ISecureRandom; +use OCP\Security\Events\GenerateSecurePasswordEvent; +use OCP\EventDispatcher\IEventDispatcher; class UsersController extends AUserData { @@ -76,6 +78,8 @@ class UsersController extends AUserData { private $secureRandom; /** @var RemoteWipe */ private $remoteWipe; + /** @var IEventDispatcher */ + private $eventDispatcher; public function __construct(string $appName, IRequest $request, @@ -90,7 +94,8 @@ class UsersController extends AUserData { NewUserMailHelper $newUserMailHelper, FederatedShareProviderFactory $federatedShareProviderFactory, ISecureRandom $secureRandom, - RemoteWipe $remoteWipe) { + RemoteWipe $remoteWipe, + IEventDispatcher $eventDispatcher) { parent::__construct($appName, $request, $userManager, @@ -107,6 +112,7 @@ class UsersController extends AUserData { $this->federatedShareProviderFactory = $federatedShareProviderFactory; $this->secureRandom = $secureRandom; $this->remoteWipe = $remoteWipe; + $this->eventDispatcher = $eventDispatcher; } /** @@ -286,9 +292,18 @@ class UsersController extends AUserData { throw new OCSException('To send a password link to the user an email address is required.', 108); } - $password = $this->secureRandom->generate(10); - // Make sure we pass the password_policy - $password .= $this->secureRandom->generate(2, '$!.,;:-~+*[]{}()'); + $passwordEvent = new GenerateSecurePasswordEvent(); + $this->eventDispatcher->dispatchTyped($passwordEvent); + + $password = $passwordEvent->getPassword(); + if ($password === null) { + // Fallback: ensure to pass password_policy in any case + $password = $this->secureRandom->generate(10) + . $this->secureRandom->generate(1, ISecureRandom::CHAR_UPPER) + . $this->secureRandom->generate(1, ISecureRandom::CHAR_LOWER) + . $this->secureRandom->generate(1, ISecureRandom::CHAR_DIGITS) + . $this->secureRandom->generate(1, ISecureRandom::CHAR_SYMBOLS); + } $generatePasswordResetToken = true; } diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 152a652665..ceb84cbed4 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -48,6 +48,7 @@ use OCA\Provisioning_API\FederatedShareProviderFactory; use OCA\Settings\Mailer\NewUserMailHelper; use OCP\App\IAppManager; use OCP\AppFramework\Http\DataResponse; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\IGroup; use OCP\IL10N; @@ -58,6 +59,7 @@ use OCP\IUserManager; use OCP\IUserSession; use OCP\L10N\IFactory; use OCP\Mail\IEMailTemplate; +use OCP\Security\Events\GenerateSecurePasswordEvent; use OCP\Security\ISecureRandom; use OCP\UserInterface; use PHPUnit\Framework\MockObject\MockObject; @@ -94,6 +96,8 @@ class UsersControllerTest extends TestCase { private $secureRandom; /** @var RemoteWipe|MockObject */ private $remoteWipe; + /** @var IEventDispatcher */ + private $eventDispatcher; protected function setUp(): void { parent::setUp(); @@ -111,6 +115,7 @@ class UsersControllerTest extends TestCase { $this->federatedShareProviderFactory = $this->createMock(FederatedShareProviderFactory::class); $this->secureRandom = $this->createMock(ISecureRandom::class); $this->remoteWipe = $this->createMock(RemoteWipe::class); + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->api = $this->getMockBuilder(UsersController::class) ->setConstructorArgs([ @@ -128,6 +133,7 @@ class UsersControllerTest extends TestCase { $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, + $this->eventDispatcher, ]) ->setMethods(['fillStorageInfo']) ->getMock(); @@ -389,7 +395,8 @@ class UsersControllerTest extends TestCase { $this->newUserMailHelper, $this->federatedShareProviderFactory, $this->secureRandom, - $this->remoteWipe + $this->remoteWipe, + $this->eventDispatcher, ]) ->setMethods(['editUser']) ->getMock(); @@ -486,6 +493,46 @@ class UsersControllerTest extends TestCase { )); } + public function testAddUserSuccessfulGeneratePassword() { + $this->userManager + ->expects($this->once()) + ->method('userExists') + ->with('NewUser') + ->willReturn(false); + $this->userManager + ->expects($this->once()) + ->method('createUser'); + $this->logger + ->expects($this->once()) + ->method('info') + ->with('Successful addUser call with userid: NewUser', ['app' => 'ocs_api']); + $loggedInUser = $this->getMockBuilder(IUser::class) + ->disableOriginalConstructor() + ->getMock(); + $loggedInUser + ->expects($this->once()) + ->method('getUID') + ->willReturn('adminUser'); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->willReturn($loggedInUser); + $this->groupManager + ->expects($this->once()) + ->method('isAdmin') + ->with('adminUser') + ->willReturn(true); + $this->eventDispatcher + ->expects($this->once()) + ->method('dispatchTyped') + ->with(new GenerateSecurePasswordEvent()); + + $this->assertTrue(key_exists( + 'id', + $this->api->addUser('NewUser', '', '', 'foo@bar')->getData() + )); + } + public function testAddUserFailedToGenerateUserID() { $this->expectException(\OCP\AppFramework\OCS\OCSException::class); @@ -3126,6 +3173,7 @@ class UsersControllerTest extends TestCase { $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, + $this->eventDispatcher, ]) ->setMethods(['getUserData']) ->getMock(); @@ -3190,6 +3238,7 @@ class UsersControllerTest extends TestCase { $this->federatedShareProviderFactory, $this->secureRandom, $this->remoteWipe, + $this->eventDispatcher, ]) ->setMethods(['getUserData']) ->getMock();