Merge pull request #8856 from nextcloud/ocs-api-new-user-password-email

Allow admin to create users without password by sending mail automatic…

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.',

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();