From 76c356c6a074f04161f867393e94ef07e1a7e3df 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 --- core/Command/User/Add.php | 19 ++++++++++++---- .../User/Service/CreateUserService.php | 12 +++++----- .../User/Service/CreateUserServiceTest.php | 22 +++++++++++-------- .../User/Service/UserSendMailServiceTest.php | 20 ++++++++++------- 4 files changed, 46 insertions(+), 27 deletions(-) diff --git a/core/Command/User/Add.php b/core/Command/User/Add.php index 591c173cb8ce..40c37f556cf1 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/lib/private/User/Service/CreateUserService.php b/lib/private/User/Service/CreateUserService.php index a9876426be86..a4f9ddcc6370 100644 --- a/lib/private/User/Service/CreateUserService.php +++ b/lib/private/User/Service/CreateUserService.php @@ -92,7 +92,7 @@ public function __construct(IUserSession $userSession, IGroupManager $groupManag * @throws UserAlreadyExistsException */ public function createUser($arguments) { - $username = $password = $email = ''; + $password = $email = ''; if (\array_key_exists('username', $arguments)) { $username = $arguments['username']; } else { @@ -125,7 +125,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) { @@ -160,13 +160,13 @@ public function createUser($arguments) { * $groups = ['group1', 'group2'] * * This function returns the list of group(s) which failed to add user to the group(s) + * If the user is already in the group, function assumes the add operation successful. * * @param IUser $user * @param array $groups list of group names, example ['group1', 'group2'] - * @param bool $checkInGroup * @return array Returns an array of groups which failed to add user */ - public function addUserToGroups(IUser $user, array $groups, $checkInGroup = true) { + public function addUserToGroups(IUser $user, array $groups) { $failedToAdd = []; if (\is_array($groups) && \count($groups) > 0) { @@ -177,7 +177,7 @@ public function addUserToGroups(IUser $user, array $groups, $checkInGroup = true $groupObject = $this->groupManager->createGroup($groupName); } $groupObject->addUser($user); - if ($checkInGroup && !$this->groupManager->isInGroup($user->getUID(), $groupName)) { + if (!$this->groupManager->isInGroup($user->getUID(), $groupName)) { $failedToAdd[] = $groupName; } else { $this->logger->info('Added userid ' . $user->getUID() . ' to group ' . $groupName, ['app' => 'ocs_api']); @@ -190,7 +190,7 @@ public function addUserToGroups(IUser $user, array $groups, $checkInGroup = true /** * Check if the user exist * - * This function is for convinience. It helps to use this service to check if user exist, + * This function is for convenience. It helps to use this service to check if user exist, * instead of adding dependency userManager. Kindly do not delete this method. * * @param string $uid diff --git a/tests/lib/User/Service/CreateUserServiceTest.php b/tests/lib/User/Service/CreateUserServiceTest.php index 5b8eb1dafcea..039cf9e1affb 100644 --- a/tests/lib/User/Service/CreateUserServiceTest.php +++ b/tests/lib/User/Service/CreateUserServiceTest.php @@ -116,20 +116,22 @@ public function testAddUserToGroups() { } /** - * @expectedExceptionMessage Invalid mail address - * @expectedException \OCP\User\Exceptions\InvalidEmailException */ public function testInvalidEmail() { + $this->expectException(\OCP\User\Exceptions\InvalidEmailException::class); + $this->expectExceptionMessage('Invalid mail address'); + $this->mailer->method('validateMailAddress') ->willReturn(false); $this->createUserService->createUser(['username' => 'foo', 'password' => '', 'email' => 'foo@bar']); } /** - * @expectedExceptionMessage A user with that name already exists. - * @expectedException \OCP\User\Exceptions\UserAlreadyExistsException */ public function testAlreadyExistingUser() { + $this->expectException(\OCP\User\Exceptions\UserAlreadyExistsException::class); + $this->expectExceptionMessage('A user with that name already exists.'); + $this->userSendMailService->method('validateEmailAddress') ->willReturn(true); @@ -140,10 +142,11 @@ public function testAlreadyExistingUser() { } /** - * @expectedExceptionMessage Unable to create user due to exception: - * @expectedException \OCP\User\Exceptions\CannotCreateUserException */ public function testUserCreateException() { + $this->expectException(\OCP\User\Exceptions\CannotCreateUserException::class); + $this->expectExceptionMessage('Exception Message'); + $this->userSendMailService->method('validateEmailAddress') ->willReturn(true); @@ -151,15 +154,16 @@ 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']); } /** - * @expectedExceptionMessage Unable to create user. - * @expectedException \OCP\User\Exceptions\CannotCreateUserException */ public function testUserCreateFailed() { + $this->expectException(\OCP\User\Exceptions\CannotCreateUserException::class); + $this->expectExceptionMessage('Unable to create user.'); + $this->userSendMailService->method('validateEmailAddress') ->willReturn(true); diff --git a/tests/lib/User/Service/UserSendMailServiceTest.php b/tests/lib/User/Service/UserSendMailServiceTest.php index 8f1ce3b31eb3..e87b401fdfa2 100644 --- a/tests/lib/User/Service/UserSendMailServiceTest.php +++ b/tests/lib/User/Service/UserSendMailServiceTest.php @@ -99,10 +99,11 @@ public function testGenerateTokenAndSendMail() { } /** - * @expectedException OCP\User\Exceptions\InvalidUserTokenException - * @expectedExceptionMessage The token provided is invalid. */ public function testcheckPasswordSetInvalidToken() { + $this->expectException(\OCP\User\Exceptions\InvalidUserTokenException::class); + $this->expectExceptionMessage('The token provided is invalid.'); + $user = $this->createMock(IUser::class); $user->method('getUID') ->willReturn('foo'); @@ -113,10 +114,11 @@ public function testcheckPasswordSetInvalidToken() { } /** - * @expectedException \OCP\User\Exceptions\UserTokenExpiredException - * @expectedExceptionMessage The token provided had expired. */ public function testCheckPasswordSetTokenExpired() { + $this->expectException(\OCP\User\Exceptions\UserTokenExpiredException::class); + $this->expectExceptionMessage('The token provided had expired.'); + $user = $this->createMock(IUser::class); $user->method('getUID') ->willReturn('foo'); @@ -133,10 +135,11 @@ public function testCheckPasswordSetTokenExpired() { } /** - * @expectedException \OCP\User\Exceptions\UserTokenMismatchException - * @expectedExceptionMessage The token provided is invalid. */ public function testCheckPasswordSetTokenMismatch() { + $this->expectException(\OCP\User\Exceptions\UserTokenMismatchException::class); + $this->expectExceptionMessage('The token provided is invalid.'); + $user = $this->createMock(IUser::class); $user->method('getUID') ->willReturn('foo'); @@ -187,10 +190,11 @@ public function testSendNotificationMailFail() { } /** - * @expectedException \OCP\User\Exceptions\EmailSendFailedException - * @expectedExceptionMessage Email could not be sent. */ public function testSendNotificationMailSendFail() { + $this->expectException(\OCP\User\Exceptions\EmailSendFailedException::class); + $this->expectExceptionMessage('Email could not be sent.'); + $user = $this->createMock(IUser::class); $user->method('getEMailAddress') ->willReturn('foo@barr.com');