Do not change the old behavior of user creation

owncloud · Dec 12, 2019 · 988427d · 988427d
1 parent 46dcb47
commit 988427d
Show file tree

Hide file tree

Showing 7 changed files with 59 additions and 48 deletions.
diff --git a/core/Command/User/Add.php b/core/Command/User/Add.php
@@ -32,6 +32,7 @@
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
+use Symfony\Component\Console\Output\ConsoleOutputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Question\Question;
@@ -91,7 +92,17 @@ protected function configure() {
 			);
 	}
 
+	/**
+	 * @param InputInterface $input
+	 * @param OutputInterface $output
+	 * @return int
+	 */
 	protected function execute(InputInterface $input, OutputInterface $output) {
+		$stdErr = $output;
+		if ($output instanceof ConsoleOutputInterface) {
+			// If it's available, get stdErr output
+			$stdErr = $output->getErrorOutput();
+		}
 		$uid = $input->getArgument('uid');
 		$email = $input->getOption('email');
 		$displayName = $input->getOption('display-name');
@@ -134,20 +145,20 @@ protected function execute(InputInterface $input, OutputInterface $output) {
 		try {
 			$user = $this->createUserService->createUser(['username' => $uid, 'password' => $password, 'email' => $email]);
 		} catch (InvalidEmailException $e) {
-			$output->writeln('<error>Invalid email address supplied</error>');
+			$stdErr->writeln('<error>Invalid email address supplied</error>');
 			return 1;
 		} catch (CannotCreateUserException $e) {
-			$output->writeln("<error>" . $e->getMessage() .  "</error>");
+			$stdErr->writeln("<error>" . $e->getMessage() .  "</error>");
 			return 1;
 		} catch (UserAlreadyExistsException $e) {
-			$output->writeln("<error>" . $e->getMessage() .  "</error>");
+			$stdErr->writeln("<error>" . $e->getMessage() .  "</error>");
 			return 1;
 		}
 
 		if ($user instanceof IUser) {
 			$output->writeln('<info>The user "' . $user->getUID() . '" was created successfully</info>');
 		} else {
-			$output->writeln('<error>An error occurred while creating the user</error>');
+			$stdErr->writeln('<error>An error occurred while creating the user</error>');
 			return 1;
 		}
 

diff --git a/core/Controller/LostController.php b/core/Controller/LostController.php
@@ -253,7 +253,7 @@ public function setPassword($token, $userId, $password, $proceed) {
 			}
 
 			\OC_Hook::emit('\OC\Core\LostPassword\Controller\LostController', 'post_passwordReset', ['uid' => $userId, 'password' => $password]);
-			@\OC_User::unsetMagicInCookie();
+			$this->userSession->unsetMagicInCookie();
 		} catch (\Exception $e) {
 			return $this->error($e->getMessage());
 		}

diff --git a/core/Controller/UserController.php b/core/Controller/UserController.php
@@ -25,6 +25,7 @@
 namespace OC\Core\Controller;
 
 use OC\User\Service\UserSendMailService;
+use OC\User\Session;
 use \OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use \OCP\AppFramework\Http\JSONResponse;
@@ -38,14 +39,10 @@
 use OCP\User\Exceptions\UserTokenExpiredException;
 
 class UserController extends Controller {
-	/**
-	 * @var \OCP\IUserManager
-	 */
+	/** @var \OCP\IUserManager */
 	protected $userManager;
 
-	/**
-	 * @var \OC_Defaults
-	 */
+	/** @var \OC_Defaults */
 	protected $defaults;
 
 	/** @var UserSendMailService  */
@@ -60,6 +57,9 @@ class UserController extends Controller {
 	/** @var IL10N  */
 	private $l10n;
 
+	/** @var Session  */
+	private $session;
+
 	/**
 	 * UserController constructor.
 	 *
@@ -70,15 +70,16 @@ class UserController extends Controller {
 	 * @param UserSendMailService $userSendMailService
 	 * @param IURLGenerator $urlGenerator
 	 * @param ILogger $logger
-	 * @param IL10N $l10n
+	 * @param IL10N $l10n,
+	 * @param Session $session
 	 */
 	public function __construct($appName,
 								IRequest $request,
 								$userManager,
 								$defaults,
 								UserSendMailService $userSendMailService,
 								IURLGenerator $urlGenerator, ILogger $logger,
-								IL10N $l10n
+								IL10N $l10n, Session $session
 	) {
 		parent::__construct($appName, $request);
 		$this->userManager = $userManager;
@@ -87,6 +88,7 @@ public function __construct($appName,
 		$this->urlGenerator = $urlGenerator;
 		$this->logger = $logger;
 		$this->l10n = $l10n;
+		$this->session = $session;
 	}
 
 	/**
@@ -173,7 +175,7 @@ public function resendToken($userId) {
 		$user = $this->userManager->get($userId);
 
 		if ($user === null) {
-			$this->logger->error('User: ' . $userId . ' does not exist', ['app' => 'core']);
+			$this->logger->error("Failed to create activation link. User $userId does not exists", ['app' => 'core']);
 			return new TemplateResponse(
 				'core', 'error',
 				[
@@ -252,7 +254,7 @@ public function setPassword($token, $userId, $password) {
 			}
 
 			\OC_Hook::emit('\OC\Core\LostPassword\Controller\LostController', 'post_passwordReset', ['uid' => $userId, 'password' => $password]);
-			@\OC_User::unsetMagicInCookie();
+			$this->session->unsetMagicInCookie();
 		} catch (UserTokenException $e) {
 			$this->logger->logException($e, ['app' => 'core']);
 			return new JSONResponse(

diff --git a/core/js/setpassword.js b/core/js/setpassword.js
@@ -23,20 +23,19 @@
 				passwordObj.val('');
 				retypePasswordObj.val('');
 				passwordObj.parent().addClass('shake');
-				$('#message').addClass('warning');
-				$('#message').text('Passwords do not match');
-				$('#message').show();
+				$('#message').addClass('warning')
+					.text('Passwords do not match')
+					.show();
 				passwordObj.focus();
 			}
 		},
 
 		_onSetPasswordFail: function(result) {
-			var responseObj = JSON.parse(result.responseText);
+			var response = JSON.parse(result.responseText);
 			var errorObject = $('#error-message');
-			var showErrorMessage = false;
 
 			var errorMessage;
-			errorMessage = responseObj.message;
+			errorMessage = response.message;
 
 			if (!errorMessage) {
 				errorMessage = t('core', 'Failed to set password. Please contact your administrator.');
@@ -49,15 +48,15 @@
 
 		_resetDone : function(result){
 			if (result && result.status === 'success') {
-				var getRootPath = OC.getRootPath();
-				if (getRootPath === '') {
+				var rootPath = OC.getRootPath();
+				if (rootPath === '') {
 					/**
 					 * If owncloud is not run inside subfolder, the getRootPath
 					 * will return empty string
 					 */
-					getRootPath = "/";
+					rootPath = "/";
 				}
-				OC.redirect(getRootPath);
+				OC.redirect(rootPath);
 			}
 		}
 	};
@@ -76,8 +75,7 @@ $(document).ready(function () {
 		 Else it should not.
 		 */
 		if (($('#password').val().length >= 0) && ($('#retypepassword').val().length === 0)) {
-			$('#message').removeClass('warning');
-			$('#message').text('');
+			$('#message').removeClass('warning').text('');
 		}
 	});
 });
diff --git a/lib/private/User/Service/CreateUserService.php b/lib/private/User/Service/CreateUserService.php
@@ -92,12 +92,17 @@ public function __construct(IUserSession $userSession, IGroupManager $groupManag
 	 * @throws UserAlreadyExistsException
 	 */
 	public function createUser($arguments) {
-		$username = $password = $email = '';
-		if (\array_key_exists('username', $arguments)) {
+		$password = $email = '';
+		if (isset($arguments['username'])) {
 			$username = $arguments['username'];
 		} else {
 			throw new CannotCreateUserException("Unable to create user due to missing user name");
 		}
+
+		if ($this->userManager->userExists($username)) {
+			throw new UserAlreadyExistsException('A user with that name already exists.');
+		}
+
 		if (\array_key_exists('password', $arguments)) {
 			$password = $arguments['password'];
 		}
@@ -109,10 +114,6 @@ public function createUser($arguments) {
 			throw new InvalidEmailException("Invalid mail address");
 		}
 
-		if ($this->userManager->userExists($username)) {
-			throw new UserAlreadyExistsException('A user with that name already exists.');
-		}
-
 		try {
 			$oldPassword = $password;
 			if (($password === '') && ($email !== '')) {
@@ -125,7 +126,7 @@ public function createUser($arguments) {
 			}
 			$user = $this->userManager->createUser($username, $password);
 		} catch (\Exception $exception) {
-			throw new CannotCreateUserException("Unable to create user due to exception: {$exception->getMessage()}");
+			throw new CannotCreateUserException($exception->getMessage());
 		}
 
 		if ($user === false) {
@@ -169,7 +170,7 @@ public function createUser($arguments) {
 	public function addUserToGroups(IUser $user, array $groups, $checkInGroup = true) {
 		$failedToAdd = [];
 
-		if (\is_array($groups) && \count($groups) > 0) {
+		if (\count($groups) > 0) {
 			foreach ($groups as $groupName) {
 				$groupObject = $this->groupManager->get($groupName);
 

diff --git a/lib/private/User/Service/UserSendMailService.php b/lib/private/User/Service/UserSendMailService.php
@@ -153,18 +153,17 @@ public function sendNotificationMail(IUser $user) {
 		$fromMailAddress = Util::getDefaultEmailAddress('no-reply');
 
 		if ($email !== '') {
+			$tmpl = new \OC_Template('core', 'lostpassword/notify');
+			$msg = $tmpl->fetchPage();
+			$tmplAlt = new \OC_Template('core', 'lostpassword/altnotify');
+			$msgAlt = $tmplAlt->fetchPage();
+			$message = $this->mailer->createMessage();
+			$message->setTo([$email => $user->getUID()]);
+			$message->setSubject($this->l10n->t('%s password changed successfully', [$this->defaults->getName()]));
+			$message->setHtmlBody($msg);
+			$message->setPlainBody($msgAlt);
+			$message->setFrom([$fromMailAddress => $this->defaults->getName()]);
 			try {
-				$tmpl = new \OC_Template('core', 'lostpassword/notify');
-				$msg = $tmpl->fetchPage();
-				$tmplAlt = new \OC_Template('core', 'lostpassword/altnotify');
-				$msgAlt = $tmplAlt->fetchPage();
-
-				$message = $this->mailer->createMessage();
-				$message->setTo([$email => $user->getUID()]);
-				$message->setSubject($this->l10n->t('%s password changed successfully', [$this->defaults->getName()]));
-				$message->setHtmlBody($msg);
-				$message->setPlainBody($msgAlt);
-				$message->setFrom([$fromMailAddress => $this->defaults->getName()]);
 				$this->mailer->send($message);
 				return true;
 			} catch (\Exception $exception) {

diff --git a/tests/lib/User/Service/CreateUserServiceTest.php b/tests/lib/User/Service/CreateUserServiceTest.php
@@ -140,7 +140,7 @@ public function testAlreadyExistingUser() {
 	}
 
 	/**
-	 * @expectedExceptionMessage Unable to create user due to exception:
+	 * @expectedExceptionMessage Exception Message
 	 * @expectedException  \OCP\User\Exceptions\CannotCreateUserException
 	 */
 	public function testUserCreateException() {
@@ -151,7 +151,7 @@ public function testUserCreateException() {
 			->willReturn(false);
 
 		$this->userManager->method('createUser')
-			->willThrowException(new \Exception());
+			->willThrowException(new \Exception("Exception Message"));
 		$this->createUserService->createUser(['username' => 'foo', 'password' => '', 'email' => '[email protected]']);
 	}