User - Code Metrics - Inspection of "Prevent passwords being set with empty strings" - owncloud/core - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#32261)

by Matthew

created 2018-08-07 10:55 UTC

User F

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	466
Duplicated Lines	1.93 %

Coupling/Cohesion

Components	1
Dependencies	26

Importance

Changes

Metric	Value
dl	9
loc	466
rs	2.96
c	0
b	0
f	0
wmc	68
lcom	1
cbo	26

25 Methods

Rating	Name	Duplication	Size	Complexity
A	__construct()	0	25	5
A	getUID()	0	3	1
A	getDisplayName()	0	7	2
A	setDisplayName()	0	20	4
A	setEMailAddress()	0	9	2
A	getLastLogin()	0	3	1
A	updateLastLoginTimestamp()	0	6	1
B	delete()	0	45	6
B	setPassword()	0	32	6
A	getHome()	0	3	1
A	getBackendClassName()	0	7	2
A	canChangeAvatar()	0	10	3
A	canChangePassword()	0	7	3
B	canChangeDisplayName()	0	18	8
A	isEnabled()	0	3	1
A	setEnabled()	0	12	3
A	getEMailAddress()	0	3	1
A	getQuota()	0	7	2
A	setQuota()	0	9	3
A	getAvatarImage()	0	14	3
A	getCloudId()	0	5	1
A	removeProtocolFromUrl()	9	9	3
A	triggerChange()	0	5	3
A	getSearchTerms()	0	7	2
A	setSearchTerms()	0	7	1

How to fix Duplicated Code Complexity

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

If you have the same expression in different places: Extract expression to a method
If you have the same method in different sub-classes: Extract method, and pull up field to the parent class
If you have the same code in unrelated classes: Consider extracting the code to a new class, and injecting that class

Complex Class

Tip: Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like User often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use User, and based on these observations, apply Extract Interface, too.

<?php
/**
 * @author Arthur Schiwon <[email protected]>
 * @author Bart Visscher <[email protected]>
 * @author Björn Schießle <[email protected]>
 * @author Jörn Friedrich Dreyer <[email protected]>
 * @author Lukas Reschke <[email protected]>
 * @author Morris Jobke <[email protected]>
 * @author Robin Appelman <[email protected]>
 * @author Thomas Müller <[email protected]>
 * @author Tom Needham <[email protected]>
 * @author Victor Dubiniuk <[email protected]>
 * @author Vincent Petry <[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;

use OC\Files\Cache\Storage;
use OC\Hooks\Emitter;
use OC_Helper;
use OCP\Events\EventEmitterTrait;
use OCP\IAvatarManager;
use OCP\IImage;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\IConfig;
use OCP\IUserBackend;
use OCP\IUserSession;
use OCP\User\IChangePasswordBackend;
use Symfony\Component\EventDispatcher\EventDispatcher;
use Symfony\Component\EventDispatcher\GenericEvent;

class User implements IUser {
	use EventEmitterTrait;
	/** @var Account */
	private $account;

	/** @var Emitter|Manager $emitter */
	private $emitter;

	/** @var \OCP\IConfig $config */
	private $config;

	/** @var IAvatarManager */
	private $avatarManager;

	/** @var IURLGenerator */
	private $urlGenerator;

	/** @var EventDispatcher */
	private $eventDispatcher;

	/** @var AccountMapper */
	private $mapper;

	/** @var \OC\Group\Manager  */
	private $groupManager;

	/** @var Session  */
	private $userSession;

	/**
	 * User constructor.
	 *
	 * @param Account $account
	 * @param AccountMapper $mapper
	 * @param null $emitter
	 * @param IConfig|null $config
	 * @param null $urlGenerator
	 * @param EventDispatcher|null $eventDispatcher
	 * @param \OC\Group\Manager|null $groupManager
	 * @param Session|null $userSession
	 */
	public function __construct(Account $account, AccountMapper $mapper, $emitter = null, IConfig $config = null,
								$urlGenerator = null, EventDispatcher $eventDispatcher = null,
								\OC\Group\Manager $groupManager = null, Session $userSession = null
	) {
		$this->account = $account;
		$this->mapper = $mapper;
		$this->emitter = $emitter;
		$this->eventDispatcher = $eventDispatcher;
		if ($config === null) {
			$config = \OC::$server->getConfig();
		}
		$this->config = $config;
		$this->urlGenerator = $urlGenerator;
		if ($this->urlGenerator === null) {
			$this->urlGenerator = \OC::$server->getURLGenerator();
		}
		$this->groupManager = $groupManager;
		if ($this->groupManager === null) {
			$this->groupManager = \OC::$server->getGroupManager();
		}
		$this->userSession = $userSession;
		if ($this->userSession === null) {
			$this->userSession = \OC::$server->getUserSession();
		}
	}

	/**
	 * get the user id
	 *
	 * @return string
	 */
	public function getUID() {
		return $this->account->getUserId();
	}

	/**
	 * get the display name for the user, if no specific display name is set it will fallback to the user id
	 *
	 * @return string
	 */
	public function getDisplayName() {
		$displayName = $this->account->getDisplayName();
		if (empty($displayName)) {
			$displayName = $this->getUID();
		}
		return $displayName;
	}

	/**
	 * set the displayname for the user
	 *
	 * @param string $displayName
	 * @return bool
	 */
	public function setDisplayName($displayName) {
		if (!$this->canChangeDisplayName()) {
			return false;
		}
		$displayName = \trim($displayName);
		if ($displayName === $this->account->getDisplayName()) {
			return false;
		}
		$this->account->setDisplayName($displayName);
		$this->mapper->update($this->account);

		$backend = $this->account->getBackendInstance();
		if ($backend->implementsActions(Backend::SET_DISPLAYNAME)) {
			$backend->setDisplayName($this->account->getUserId(), $displayName);
		}

		$this->triggerChange('displayName', $displayName);

		return true;
	}

	/**
	 * set the email address of the user
	 *
	 * @param string|null $mailAddress
	 * @return void
	 * @since 9.0.0
	 */
	public function setEMailAddress($mailAddress) {
		$mailAddress = \trim($mailAddress);
		if ($mailAddress === $this->account->getEmail()) {
			return;
		}
		$this->account->setEmail($mailAddress);
		$this->mapper->update($this->account);
		$this->triggerChange('eMailAddress', $mailAddress);
	}

	/**
	 * returns the timestamp of the user's last login or 0 if the user did never
	 * login
	 *
	 * @return int
	 */
	public function getLastLogin() {
		return (int)$this->account->getLastLogin();
	}

	/**
	 * updates the timestamp of the most recent login of this user
	 */
	public function updateLastLoginTimestamp() {
		$firstTimeLogin = ($this->getLastLogin() === 0);
		$this->account->setLastLogin(\time());
		$this->mapper->update($this->account);
		return $firstTimeLogin;
	}

	/**
	 * Delete the user
	 *
	 * @return bool
	 */
	public function delete() {
		if ($this->emitter) {
			$this->emitter->emit('\OC\User', 'preDelete', [$this]);
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
		}
		// get the home now because it won't return it after user deletion
		$homePath = $this->getHome();
		$this->mapper->delete($this->account);
		$bi = $this->account->getBackendInstance();
		if ($bi !== null) {
			$bi->deleteUser($this->account->getUserId());
		}

		// FIXME: Feels like an hack - suggestions?

		// We have to delete the user from all groups
		foreach (\OC::$server->getGroupManager()->getUserGroups($this) as $group) {
			$group->removeUser($this);
		}
		// Delete the user's keys in preferences
		\OC::$server->getConfig()->deleteAllUserValues($this->getUID());

		// Delete all mount points for user
		\OC::$server->getUserStoragesService()->deleteAllMountsForUser($this);
		//Delete external storage or remove user from applicableUsers list
		\OC::$server->getGlobalStoragesService()->deleteAllForUser($this);

		// Delete user files in /data/
		if ($homePath !== false) {
			// FIXME: this operates directly on FS, should use View instead...
			// also this is not testable/mockable...
			\OC_Helper::rmdirr($homePath);
		}

		// Delete the users entry in the storage table
		Storage::remove('home::' . $this->getUID());
		Storage::remove('object::user:' . $this->getUID());

		\OC::$server->getCommentsManager()->deleteReferencesOfActor('users', $this->getUID());
		\OC::$server->getCommentsManager()->deleteReadMarksFromUser($this);

		if ($this->emitter) {
			$this->emitter->emit('\OC\User', 'postDelete', [$this]);
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
		}
		return true;
	}

	/**
	 * Set the user's password
	 *
	 * @param string $password
	 * @param string $recoveryPassword for the encryption app to reset encryption keys
	 * @return bool
	 * @throws ArgumentNotSetException
	 */
	public function setPassword($password, $recoveryPassword = null) {
		if (empty($password)) {
			throw new ArgumentNotSetException('Password cannot be empty');
		}

		return $this->emittingCall(function () use (&$password, &$recoveryPassword) {
			if ($this->emitter) {
				$this->emitter->emit('\OC\User', 'preSetPassword', [$this, $password, $recoveryPassword]);
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
				\OC::$server->getEventDispatcher()->dispatch(
					'OCP\User::validatePassword',
					new GenericEvent(null, ['uid'=> $this->getUID(), 'password' => $password])
				);
			}
			if ($this->canChangePassword()) {
				/** @var IChangePasswordBackend $backend */
				$backend = $this->account->getBackendInstance();
				$result = $backend->setPassword($this->getUID(), $password);
				if ($result) {
					if ($this->emitter) {
						$this->emitter->emit('\OC\User', 'postSetPassword', [$this, $password, $recoveryPassword]);
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
					}
					$this->config->deleteUserValue($this->getUID(), 'owncloud', 'lostpassword');
				}
				return !($result === false);
			} else {
				return false;
			}
		}, [
			'before' => ['user' => $this, 'password' => $password, 'recoveryPassword' => $recoveryPassword],
			'after' => ['user' => $this, 'password' => $password, 'recoveryPassword' => $recoveryPassword]
		], 'user', 'setpassword');
	}

	/**
	 * get the users home folder to mount
	 *
	 * @return string
	 */
	public function getHome() {
		return $this->account->getHome();
	}

	/**
	 * Get the name of the backend class the user is connected with
	 *
	 * @return string
	 */
	public function getBackendClassName() {
		$b = $this->account->getBackendInstance();
		if ($b instanceof IUserBackend) {
			return $b->getBackendName();
		}
		return $this->account->getBackend();
	}

	/**
	 * check if the backend allows the user to change his avatar on Personal page
	 *
	 * @return bool
	 */
	public function canChangeAvatar() {
		$backend = $this->account->getBackendInstance();
		if ($backend === null) {
			return false;
		}
		if ($backend->implementsActions(Backend::PROVIDE_AVATAR)) {
			return $backend->canChangeAvatar($this->getUID());
		}
		return true;
	}

	/**
	 * check if the backend supports changing passwords
	 *
	 * @return bool
	 */
	public function canChangePassword() {
		$backend = $this->account->getBackendInstance();
		if ($backend === null) {
			return false;
		}
		return $backend instanceof IChangePasswordBackend || $backend->implementsActions(Backend::SET_PASSWORD);
	}

	/**
	 * check if the backend supports changing display names
	 *
	 * @return bool
	 */
	public function canChangeDisplayName() {
		// Only Admin and SubAdmins are allowed to change display name
		if ($this->userSession instanceof IUserSession) {
			$user = $this->userSession->getUser();
			if (
				($this->config->getSystemValue('allow_user_to_change_display_name') === false) &&
				(($user !== null) && (!$this->groupManager->isAdmin($user->getUID()))) &&
				(($user !== null) && (!$this->groupManager->getSubAdmin()->isSubAdmin($user)))
			) {
				return false;
			}
		}
		$backend = $this->account->getBackendInstance();
		if ($backend === null) {
			return false;
		}
		return $backend->implementsActions(Backend::SET_DISPLAYNAME);
	}

	/**
	 * check if the user is enabled
	 *
	 * @return bool
	 */
	public function isEnabled() {
		return $this->account->getState() === Account::STATE_ENABLED;
	}

	/**
	 * set the enabled status for the user
	 *
	 * @param bool $enabled
	 */
	public function setEnabled($enabled) {
		if ($enabled === true) {
			$this->account->setState(Account::STATE_ENABLED);
		} else {
			$this->account->setState(Account::STATE_DISABLED);
		}
		$this->mapper->update($this->account);

		if ($this->eventDispatcher) {
			$this->eventDispatcher->dispatch(self::class . '::postSetEnabled', new GenericEvent($this));
		}
	}

	/**
	 * get the users email address
	 *
	 * @return string|null
	 * @since 9.0.0
	 */
	public function getEMailAddress() {
		return $this->account->getEmail();
	}

	/**
	 * get the users' quota
	 *
	 * @return string
	 * @since 9.0.0
	 */
	public function getQuota() {
		$quota = $this->account->getQuota();
		if ($quota === null) {
			return 'default';
		}
		return $quota;
	}

	/**
	 * set the users' quota
	 *
	 * @param string $quota
	 * @return void
	 * @since 9.0.0
	 */
	public function setQuota($quota) {
		if ($quota !== 'none' and $quota !== 'default') {
			$quota = OC_Helper::computerFileSize($quota);
			$quota = OC_Helper::humanFileSize($quota);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
		}
		$this->account->setQuota($quota);
		$this->mapper->update($this->account);
		$this->triggerChange('quota', $quota);
	}

	/**
	 * get the avatar image if it exists
	 *
	 * @param int $size
	 * @return IImage|null
	 * @since 9.0.0
	 */
	public function getAvatarImage($size) {
		// delay the initialization
		if ($this->avatarManager === null) {
			$this->avatarManager = \OC::$server->getAvatarManager();
		}

		$avatar = $this->avatarManager->getAvatar($this->getUID());
		$image = $avatar->get($size);

		if ($image) {
			return $image;
		}

		return null;
	}

	/**
	 * get the federation cloud id
	 *
	 * @return string
	 * @since 9.0.0
	 */
	public function getCloudId() {
		$uid = $this->getUID();
		$server = $this->urlGenerator->getAbsoluteURL('/');
		return $uid . '@' . \rtrim($this->removeProtocolFromUrl($server), '/');
	}

	/**
	 * @param string $url
	 * @return string
	 */
	private function removeProtocolFromUrl($url) {
		if (\strpos($url, 'https://') === 0) {
			return \substr($url, \strlen('https://'));
		} elseif (\strpos($url, 'http://') === 0) {
			return \substr($url, \strlen('http://'));
		}

		return $url;
	}

	public function triggerChange($feature, $value = null) {
		if ($this->emitter && \in_array($feature, $this->account->getUpdatedFields())) {
			$this->emitter->emit('\OC\User', 'changeUser', [$this, $feature, $value]);
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
		}
	}

	/**
	 * @return string[]
	 * @since 10.0.1
	 */
	public function getSearchTerms() {
		$terms = [];
		foreach ($this->mapper->findByAccountId($this->account->getId()) as $term) {
			$terms[] = $term->getTerm();
		}
		return $terms;
	}

	/**
	 * @param string[] $terms
	 * @since 10.0.1
	 */
	public function setSearchTerms(array $terms) {
		// Check length of terms, cut if too long
		$terms = \array_map(function ($term) {
			return \substr($term, 0, 191);
		}, $terms);
		$this->mapper->setTermsForAccount($this->account->getId(), $terms);
	}
}


Pull Request — master (#32261)

User F

Complexity

Size/Duplication

Coupling/Cohesion

Importance

25 Methods

How to fix Duplicated Code Complexity

Duplicated Code

Complex Class

Available Fixes

Available Fixes

Available Fixes

Available Fixes

Available Fixes

owncloud / core

Pull Request — master (#32261)

User F

Complexity

Size/Duplication

Coupling/Cohesion

Importance

25 Methods

How to fix Duplicated Code Complexity

Duplicated Code

Complex Class

Available Fixes

Available Fixes

Available Fixes

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like