From 5cc92c9c91e1007263303f0c341ab76bf8e2b83b Mon Sep 17 00:00:00 2001 From: Semih Serhat Karakaya Date: Fri, 6 Dec 2019 11:39:43 +0300 Subject: [PATCH] Do not change the old behavior of user creation --- apps/provisioning_api/lib/Users.php | 2 +- core/Command/User/Add.php | 19 ++++++++++++---- core/Controller/LostController.php | 2 +- core/Controller/UserController.php | 22 ++++++++++--------- core/js/setpassword.js | 22 +++++++++---------- .../User/Service/CreateUserService.php | 17 +++++++------- .../User/Service/UserSendMailService.php | 21 +++++++++--------- .../User/Service/CreateUserServiceTest.php | 4 ++-- 8 files changed, 60 insertions(+), 49 deletions(-) diff --git a/apps/provisioning_api/lib/Users.php b/apps/provisioning_api/lib/Users.php index b3b656b6f365..0341973c8e4b 100644 --- a/apps/provisioning_api/lib/Users.php +++ b/apps/provisioning_api/lib/Users.php @@ -150,7 +150,7 @@ public function addUser() { return new Result(null, 102, 'User already exists'); } - if (\is_array($groups) && (\count($groups) > 0)) { + if (\is_array($groups)) { foreach ($groups as $group) { if (!$this->groupManager->groupExists($group)) { return new Result(null, 104, 'group '.$group.' does not exist'); diff --git a/core/Command/User/Add.php b/core/Command/User/Add.php index 4a3d9665fc1f..9743f3484688 100644 --- 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('Invalid email address supplied'); + $stdErr->writeln('Invalid email address supplied'); return 1; } catch (CannotCreateUserException $e) { - $output->writeln("" . $e->getMessage() . ""); + $stdErr->writeln("" . $e->getMessage() . ""); return 1; } catch (UserAlreadyExistsException $e) { - $output->writeln("" . $e->getMessage() . ""); + $stdErr->writeln("" . $e->getMessage() . ""); return 1; } if ($user instanceof IUser) { $output->writeln('The user "' . $user->getUID() . '" was created successfully'); } else { - $output->writeln('An error occurred while creating the user'); + $stdErr->writeln('An error occurred while creating the user'); return 1; } diff --git a/core/Controller/LostController.php b/core/Controller/LostController.php index 4a1f6ccc712a..eb4d6d91623a 100644 --- 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 index 12f1a13bcd3b..19ece441e824 100644 --- 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,7 +70,8 @@ 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, @@ -78,7 +79,7 @@ public function __construct($appName, $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 index 35df031ebc3e..3efecfe56e90 100644 --- 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 index 918edbe084bb..5ff0ad5901f2 100644 --- 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 index f6ade5a9df58..a32600146895 100644 --- 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 index 5b8eb1dafcea..bb4574534289 100644 --- 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' => 'foo@bar.com']); }