From 41b690ed53be36498607c13c2b8b3d84e240c84b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Molakvo=C3=A6=20=28skjnldsv=29?= Date: Fri, 16 Mar 2018 15:30:48 +0100 Subject: [PATCH] Allow admin to create users withoutpassword by sending mail automatically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: John Molakvoæ (skjnldsv) --- .../lib/Controller/UsersController.php | 40 ++++++++++++++++++- .../tests/Controller/UsersControllerTest.php | 27 ++++++++----- 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index cd277adb16..e3b7840cd3 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -50,6 +50,7 @@ use OCP\IRequest; use OCP\IUserManager; use OCP\IUserSession; use OCP\L10N\IFactory; +use OCP\Security\ISecureRandom; class UsersController extends OCSController { @@ -73,6 +74,8 @@ class UsersController extends OCSController { private $newUserMailHelper; /** @var FederatedFileSharingFactory */ private $federatedFileSharingFactory; + /** @var ISecureRandom */ + private $secureRandom; /** * @param string $appName @@ -87,6 +90,7 @@ class UsersController extends OCSController { * @param IFactory $l10nFactory * @param NewUserMailHelper $newUserMailHelper * @param FederatedFileSharingFactory $federatedFileSharingFactory + * @param ISecureRandom $secureRandom */ public function __construct(string $appName, IRequest $request, @@ -99,7 +103,8 @@ class UsersController extends OCSController { ILogger $logger, IFactory $l10nFactory, NewUserMailHelper $newUserMailHelper, - FederatedFileSharingFactory $federatedFileSharingFactory) { + FederatedFileSharingFactory $federatedFileSharingFactory, + ISecureRandom $secureRandom) { parent::__construct($appName, $request); $this->userManager = $userManager; @@ -112,6 +117,7 @@ class UsersController extends OCSController { $this->l10nFactory = $l10nFactory; $this->newUserMailHelper = $newUserMailHelper; $this->federatedFileSharingFactory = $federatedFileSharingFactory; + $this->secureRandom = $secureRandom; } /** @@ -199,11 +205,12 @@ class UsersController extends OCSController { * * @param string $userid * @param string $password + * @param string $email * @param array $groups * @return DataResponse * @throws OCSException */ - public function addUser(string $userid, string $password, array $groups = []): DataResponse { + public function addUser(string $userid, string $password = '', string $email='', array $groups = []): DataResponse { $user = $this->userSession->getUser(); $isAdmin = $this->groupManager->isAdmin($user->getUID()); $subAdminManager = $this->groupManager->getSubAdmin(); @@ -228,6 +235,18 @@ class UsersController extends OCSController { } } + $generatePasswordResetToken = false; + if ($password === '') { + if ($email === '') { + 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, '$!.,;:-~+*[]{}()'); + $generatePasswordResetToken = true; + } + try { $newUser = $this->userManager->createUser($userid, $password); $this->logger->info('Successful addUser call with userid: ' . $userid, ['app' => 'ocs_api']); @@ -237,7 +256,24 @@ class UsersController extends OCSController { $this->logger->info('Added userid ' . $userid . ' to group ' . $group, ['app' => 'ocs_api']); } + // Send new user mail only if a mail is set + if ($email !== '') { + $newUser->setEMailAddress($email); + try { + $emailTemplate = $this->newUserMailHelper->generateTemplate($newUser, $generatePasswordResetToken); + $this->newUserMailHelper->sendMail($newUser, $emailTemplate); + } catch (\Exception $e) { + $this->logger->logException($e, [ + 'message' => "Can't send new user mail to $email", + 'level' => \OCP\Util::ERROR, + 'app' => 'ocs_api', + ]); + throw new OCSException('Unable to send the invitation mail', 109); + } + } + return new DataResponse(); + } catch (HintException $e ) { $this->logger->logException($e, [ 'message' => 'Failed addUser attempt with hint exception.', diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 38e3598813..3f2cf3b110 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -56,6 +56,7 @@ use OCP\IUserManager; use OCP\IUserSession; use OCP\L10N\IFactory; use OCP\Mail\IMailer; +use OCP\Security\ISecureRandom; use PHPUnit_Framework_MockObject_MockObject; use Test\TestCase; @@ -85,6 +86,8 @@ class UsersControllerTest extends TestCase { private $newUserMailHelper; /** @var FederatedFileSharingFactory|\PHPUnit_Framework_MockObject_MockObject */ private $federatedFileSharingFactory; + /** @var ISecureRandom|\PHPUnit_Framework_MockObject_MockObject */ + private $secureRandom; protected function setUp() { parent::setUp(); @@ -100,6 +103,7 @@ class UsersControllerTest extends TestCase { $this->l10nFactory = $this->createMock(IFactory::class); $this->newUserMailHelper = $this->createMock(NewUserMailHelper::class); $this->federatedFileSharingFactory = $this->createMock(FederatedFileSharingFactory::class); + $this->secureRandom = $this->createMock(ISecureRandom::class); $this->api = $this->getMockBuilder(UsersController::class) ->setConstructorArgs([ @@ -114,7 +118,8 @@ class UsersControllerTest extends TestCase { $this->logger, $this->l10nFactory, $this->newUserMailHelper, - $this->federatedFileSharingFactory + $this->federatedFileSharingFactory, + $this->secureRandom ]) ->setMethods(['fillStorageInfo']) ->getMock(); @@ -242,7 +247,7 @@ class UsersControllerTest extends TestCase { ->with('adminUser') ->willReturn(true); - $this->api->addUser('AlreadyExistingUser', 'password', []); + $this->api->addUser('AlreadyExistingUser', 'password', '', []); } /** @@ -278,7 +283,7 @@ class UsersControllerTest extends TestCase { ->with('NonExistingGroup') ->willReturn(false); - $this->api->addUser('NewUser', 'pass', ['NonExistingGroup']); + $this->api->addUser('NewUser', 'pass', '', ['NonExistingGroup']); } /** @@ -320,7 +325,7 @@ class UsersControllerTest extends TestCase { ['NonExistingGroup', false] ])); - $this->api->addUser('NewUser', 'pass', ['ExistingGroup', 'NonExistingGroup']); + $this->api->addUser('NewUser', 'pass', '', ['ExistingGroup', 'NonExistingGroup']); } public function testAddUserSuccessful() { @@ -412,7 +417,7 @@ class UsersControllerTest extends TestCase { ['Added userid NewUser to group ExistingGroup', ['app' => 'ocs_api']] ); - $this->assertEquals([], $this->api->addUser('NewUser', 'PasswordOfTheNewUser', ['ExistingGroup'])->getData()); + $this->assertEquals([], $this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', ['ExistingGroup'])->getData()); } /** @@ -490,7 +495,7 @@ class UsersControllerTest extends TestCase { ->with() ->willReturn($subAdminManager); - $this->api->addUser('NewUser', 'PasswordOfTheNewUser', []); + $this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', []); } /** @@ -539,7 +544,7 @@ class UsersControllerTest extends TestCase { ->with('ExistingGroup') ->willReturn(true); - $this->api->addUser('NewUser', 'PasswordOfTheNewUser', ['ExistingGroup'])->getData(); + $this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', ['ExistingGroup'])->getData(); } public function testAddUserAsSubAdminExistingGroups() { @@ -630,7 +635,7 @@ class UsersControllerTest extends TestCase { ) ->willReturn(true); - $this->assertEquals([], $this->api->addUser('NewUser', 'PasswordOfTheNewUser', ['ExistingGroup1', 'ExistingGroup2'])->getData()); + $this->assertEquals([], $this->api->addUser('NewUser', 'PasswordOfTheNewUser', '', ['ExistingGroup1', 'ExistingGroup2'])->getData()); } /** @@ -2928,7 +2933,8 @@ class UsersControllerTest extends TestCase { $this->logger, $this->l10nFactory, $this->newUserMailHelper, - $this->federatedFileSharingFactory + $this->federatedFileSharingFactory, + $this->secureRandom ]) ->setMethods(['getUserData']) ->getMock(); @@ -2990,7 +2996,8 @@ class UsersControllerTest extends TestCase { $this->logger, $this->l10nFactory, $this->newUserMailHelper, - $this->federatedFileSharingFactory + $this->federatedFileSharingFactory, + $this->secureRandom ]) ->setMethods(['getUserData']) ->getMock();