CreateUser::__construct() - Code Metrics - Inspection of "Add creating user to a service class" - owncloud/core - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#32767)

by Sujith

created 2018-10-08 12:19 UTC

CreateUser::__construct() A

↳ Parent: CreateUser

Complexity

Conditions	1
Paths	1

Size

Total Lines

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
cc	1
nc	1
nop	8
dl	0
loc	16
rs	9.7333
c	0
b	0
f	0

How to fix Many Parameters

<?php
/**
 * @author Sujith Haridasan <[email protected]>
 *
 * @copyright Copyright (c) 2018, ownCloud GmbH
 * @license AGPL-3.0
 *
 * This code is free software: you can redistribute it and/or modify
 * it under the terms of the GNU Affero General Public License, version 3,
 * as published by the Free Software Foundation.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 * GNU Affero General Public License for more details.
 *
 * You should have received a copy of the GNU Affero General Public License, version 3,
 * along with this program.  If not, see <http://www.gnu.org/licenses/>
 *
 */

namespace OC\User\Service;

use OCP\IGroupManager;
use OCP\ILogger;
use OCP\IUserManager;
use OCP\IUserSession;
use OCP\Mail\IMailer;
use OCP\Security\ISecureRandom;
use OCP\User\Exceptions\CannotCreateUserException;
use OCP\User\Exceptions\InvalidEmailException;
use OCP\User\Exceptions\UserAlreadyExistsException;
use OCP\User\UserPasswordCreateEvent;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\EventDispatcher\GenericEvent;

class CreateUser {
	/** @var IUserSession  */
	private $userSession;
	/** @var IGroupManager  */
	private $groupManager;
	/** @var IUserManager  */
	private $userManager;
	/** @var IMailer  */
	private $mailer;
	/** @var ISecureRandom  */
	private $secureRandom;
	/** @var EventDispatcherInterface  */
	private $eventDispatcher;
	/** @var ILogger  */
	private $logger;
	/** @var UserSendMail  */
	private $userSendMail;

	/**
	 * CreateUser constructor.
	 *
	 * @param IUserSession $userSession
	 * @param IGroupManager $groupManager
	 * @param IUserManager $userManager
	 * @param IMailer $mailer
	 * @param ISecureRandom $secureRandom
	 * @param EventDispatcherInterface $eventDispatcher
	 * @param ILogger $logger
	 * @param UserSendMail $userSendMail
	 */
	public function __construct(IUserSession $userSession,
								IGroupManager $groupManager,
								IUserManager $userManager,
								IMailer $mailer,
								ISecureRandom $secureRandom,
								EventDispatcherInterface $eventDispatcher,
								ILogger $logger, UserSendMail $userSendMail) {
		$this->userSession = $userSession;
		$this->groupManager = $groupManager;
		$this->userManager = $userManager;
		$this->mailer = $mailer;
		$this->secureRandom = $secureRandom;
		$this->eventDispatcher = $eventDispatcher;
		$this->logger = $logger;
		$this->userSendMail = $userSendMail;
	}

	/**
	 * @param $username
	 * @param $password
	 * @param array $groups
	 * @param string $email
	 * @return bool|\OCP\IUser
	 * @throws CannotCreateUserException
	 * @throws InvalidEmailException
	 * @throws UserAlreadyExistsException
	 */
	public function createUser($username, $password, array $groups= [], $email='') {
		if ($email !== '' && !$this->mailer->validateMailAddress($email)) {
			throw new InvalidEmailException("Invalid mail address");
		}

		$currentUser = $this->userSession->getUser();

		if (!$this->isAdmin()) {
			if (!empty($groups)) {
				foreach ($groups as $key => $group) {
					$groupObject = $this->groupManager->get($group);
					if ($groupObject === null) {
						unset($groups[$key]);
						continue;
					}

					if (!$this->groupManager->getSubAdmin()->isSubAdminofGroup($currentUser, $groupObject)) {
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
						unset($groups[$key]);
					}
				}
			}

			if (empty($groups)) {
				$groups = $this->groupManager->getSubAdmin()->getSubAdminsGroups($currentUser);
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
				// New class returns IGroup[] so convert back
				$gids = [];
				foreach ($groups as $group) {
					$gids[] = $group->getGID();
				}
				$groups = $gids;
			}
		}

		if ($this->userManager->userExists($username)) {
			throw new UserAlreadyExistsException('A user with that name already exists.');
		}

		try {
			if (($password === '') && ($email !== '')) {
				/**
				 * Generate a random password as we are going to have this
				 * use one time. The new user has to reset it using the link
				 * from email.
				 */
				$passwordCreateEvent = new UserPasswordCreateEvent(UserPasswordCreateEvent::CREATE_PASSWORD, []);
				$this->eventDispatcher->dispatch(
					UserPasswordCreateEvent::CREATE_PASSWORD, $passwordCreateEvent);
				$password = $passwordCreateEvent->getPassword();
				if ($password === false) {
					$password = $this->secureRandom->generate(20);
				}
			}
			$user = $this->userManager->createUser($username, $password);
		} catch (\Exception $exception) {
			$message = $exception->getMessage();
			if (!$message) {
				$message = 'Unable to create user.';
			}
			throw new CannotCreateUserException($message);
		}

		if ($user === null) {
			throw new CannotCreateUserException('Unable to create user.');
		}

		if ($groups !== null) {
			foreach ($groups as $groupName) {
				if ($groupName !== null) {
					$group = $this->groupManager->get($groupName);

					if (empty($group)) {
						$group = $this->groupManager->createGroup($groupName);
					}
					$group->addUser($user);
/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}
					$this->logger->info('Added userid ' . $user->getUID() . ' to group ' . $group->getGID());
				}
			}
		}
		/**
		 * Send new user mail only if a mail is set
		 */
		if ($email !== '') {
			$user->setEMailAddress($email);
			try {
				$this->userSendMail->generateTokenAndSendMail($username, $email);
			} catch (\Exception $e) {
				$this->logger->error("Can't send new user mail to $email: " . $e->getMessage(), ['app' => 'settings']);
			}
		}

		return $user;
	}

	private function isAdmin() {
		// Check if current user (active and not in incognito mode)
		// is an admin
		$activeUser = $this->userSession->getUser();
		if ($activeUser !== null) {
			return $this->groupManager->isAdmin($activeUser->getUID());
		}
		// Check if it is triggered from command line
		$cli = \in_array(\php_sapi_name(), ['cli']);
		if ($cli === true) {
			return true;
		}
		return false;
	}
}

1			<?php
2			/**
3			* @author Sujith Haridasan <[email protected]>
4			*
5			* @copyright Copyright (c) 2018, ownCloud GmbH
6			* @license AGPL-3.0
7			*
8			* This code is free software: you can redistribute it and/or modify
9			* it under the terms of the GNU Affero General Public License, version 3,
10			* as published by the Free Software Foundation.
11			*
12			* This program is distributed in the hope that it will be useful,
13			* but WITHOUT ANY WARRANTY; without even the implied warranty of
14			* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15			* GNU Affero General Public License for more details.
16			*
17			* You should have received a copy of the GNU Affero General Public License, version 3,
18			* along with this program. If not, see <http://www.gnu.org/licenses/>
19			*
20			*/
21
22			namespace OC\User\Service;
23
24			use OCP\IGroupManager;
25			use OCP\ILogger;
26			use OCP\IUserManager;
27			use OCP\IUserSession;
28			use OCP\Mail\IMailer;
29			use OCP\Security\ISecureRandom;
30			use OCP\User\Exceptions\CannotCreateUserException;
31			use OCP\User\Exceptions\InvalidEmailException;
32			use OCP\User\Exceptions\UserAlreadyExistsException;
33			use OCP\User\UserPasswordCreateEvent;
34			use Symfony\Component\EventDispatcher\EventDispatcherInterface;
35			use Symfony\Component\EventDispatcher\GenericEvent;
36
37			class CreateUser {
38			/** @var IUserSession */
39			private $userSession;
40			/** @var IGroupManager */
41			private $groupManager;
42			/** @var IUserManager */
43			private $userManager;
44			/** @var IMailer */
45			private $mailer;
46			/** @var ISecureRandom */
47			private $secureRandom;
48			/** @var EventDispatcherInterface */
49			private $eventDispatcher;
50			/** @var ILogger */
51			private $logger;
52			/** @var UserSendMail */
53			private $userSendMail;
54
55			/**
56			* CreateUser constructor.
57			*
58			* @param IUserSession $userSession
59			* @param IGroupManager $groupManager
60			* @param IUserManager $userManager
61			* @param IMailer $mailer
62			* @param ISecureRandom $secureRandom
63			* @param EventDispatcherInterface $eventDispatcher
64			* @param ILogger $logger
65			* @param UserSendMail $userSendMail
66			*/
67			public function __construct(IUserSession $userSession,
68			IGroupManager $groupManager,
69			IUserManager $userManager,
70			IMailer $mailer,
71			ISecureRandom $secureRandom,
72			EventDispatcherInterface $eventDispatcher,
73			ILogger $logger, UserSendMail $userSendMail) {
74			$this->userSession = $userSession;
75			$this->groupManager = $groupManager;
76			$this->userManager = $userManager;
77			$this->mailer = $mailer;
78			$this->secureRandom = $secureRandom;
79			$this->eventDispatcher = $eventDispatcher;
80			$this->logger = $logger;
81			$this->userSendMail = $userSendMail;
82			}
83
84			/**
85			* @param $username
86			* @param $password
87			* @param array $groups
88			* @param string $email
89			* @return bool\|\OCP\IUser
90			* @throws CannotCreateUserException
91			* @throws InvalidEmailException
92			* @throws UserAlreadyExistsException
93			*/
94			public function createUser($username, $password, array $groups= [], $email='') {
95			if ($email !== '' && !$this->mailer->validateMailAddress($email)) {
96			throw new InvalidEmailException("Invalid mail address");
97			}
98
99			$currentUser = $this->userSession->getUser();
100
101			if (!$this->isAdmin()) {
102			if (!empty($groups)) {
103			foreach ($groups as $key => $group) {
104			$groupObject = $this->groupManager->get($group);
105			if ($groupObject === null) {
106			unset($groups[$key]);
107			continue;
108			}
109
110			if (!$this->groupManager->getSubAdmin()->isSubAdminofGroup($currentUser, $groupObject)) {
			0 ignored issues – show Bug introduced 2017-06-20 12:49 UTC by Report Bug Copy Issue Report It seems like `$currentUser` defined by `$this->userSession->getUser()` on line `99` can be `null`; however, `OCP\ISubAdminManager::isSubAdminofGroup()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
111			unset($groups[$key]);
112			}
113			}
114			}
115
116			if (empty($groups)) {
117			$groups = $this->groupManager->getSubAdmin()->getSubAdminsGroups($currentUser);
			0 ignored issues – show Bug introduced 2017-06-20 12:49 UTC by Report Bug Copy Issue Report It seems like `$currentUser` defined by `$this->userSession->getUser()` on line `99` can be `null`; however, `OCP\ISubAdminManager::getSubAdminsGroups()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
118			// New class returns IGroup[] so convert back
119			$gids = [];
120			foreach ($groups as $group) {
121			$gids[] = $group->getGID();
122			}
123			$groups = $gids;
124			}
125			}
126
127			if ($this->userManager->userExists($username)) {
128			throw new UserAlreadyExistsException('A user with that name already exists.');
129			}
130
131			try {
132			if (($password === '') && ($email !== '')) {
133			/**
134			* Generate a random password as we are going to have this
135			* use one time. The new user has to reset it using the link
136			* from email.
137			*/
138			$passwordCreateEvent = new UserPasswordCreateEvent(UserPasswordCreateEvent::CREATE_PASSWORD, []);
139			$this->eventDispatcher->dispatch(
140			UserPasswordCreateEvent::CREATE_PASSWORD, $passwordCreateEvent);
141			$password = $passwordCreateEvent->getPassword();
142			if ($password === false) {
143			$password = $this->secureRandom->generate(20);
144			}
145			}
146			$user = $this->userManager->createUser($username, $password);
147			} catch (\Exception $exception) {
148			$message = $exception->getMessage();
149			if (!$message) {
150			$message = 'Unable to create user.';
151			}
152			throw new CannotCreateUserException($message);
153			}
154
155			if ($user === null) {
156			throw new CannotCreateUserException('Unable to create user.');
157			}
158
159			if ($groups !== null) {
160			foreach ($groups as $groupName) {
161			if ($groupName !== null) {
162			$group = $this->groupManager->get($groupName);
163
164			if (empty($group)) {
165			$group = $this->groupManager->createGroup($groupName);
166			}
167			$group->addUser($user);
			0 ignored issues – show Bug introduced 2018-10-08 13:19 UTC by Report Bug Copy Issue Report It seems like `$user` defined by `$this->userManager->crea...r($username, $password)` on line `146` can also be of type `boolean`; however, `OCP\IGroup::addUser()` does only seem to accept `object<OCP\IUser>`, maybe add an additional type check? If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check: /** * @return array\|string */ function returnsDifferentValues($x) { if ($x) { return 'foo'; } return array(); } $x = returnsDifferentValues($y); if (is_array($x)) { // $x is an array. } If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue. Loading history...
168			$this->logger->info('Added userid ' . $user->getUID() . ' to group ' . $group->getGID());
169			}
170			}
171			}
172			/**
173			* Send new user mail only if a mail is set
174			*/
175			if ($email !== '') {
176			$user->setEMailAddress($email);
177			try {
178			$this->userSendMail->generateTokenAndSendMail($username, $email);
179			} catch (\Exception $e) {
180			$this->logger->error("Can't send new user mail to $email: " . $e->getMessage(), ['app' => 'settings']);
181			}
182			}
183
184			return $user;
185			}
186
187			private function isAdmin() {
188			// Check if current user (active and not in incognito mode)
189			// is an admin
190			$activeUser = $this->userSession->getUser();
191			if ($activeUser !== null) {
192			return $this->groupManager->isAdmin($activeUser->getUID());
193			}
194			// Check if it is triggered from command line
195			$cli = \in_array(\php_sapi_name(), ['cli']);
196			if ($cli === true) {
197			return true;
198			}
199			return false;
200			}
201			}

owncloud / core

Pull Request — master (#32767)

CreateUser::__construct() A

Complexity

Size

Duplication

Importance

How to fix Many Parameters

Many Parameters

Duplication Side-by-Side

Filter issues like