Completed
Push — master ( 0256f6...5411d6 )
by Morris
22:12 queued 06:46
created

User_LDAP::deleteUser()   A

Complexity

Conditions 3
Paths 3

Size

Total Lines 19
Code Lines 14

Duplication

Lines 6
Ratio 31.58 %

Importance

Changes 0
Metric Value
cc 3
eloc 14
nc 3
nop 1
dl 6
loc 19
rs 9.4285
c 0
b 0
f 0
1
<?php
2
/**
3
 * @copyright Copyright (c) 2016, ownCloud, Inc.
4
 *
5
 * @author Arthur Schiwon <[email protected]>
6
 * @author Bart Visscher <[email protected]>
7
 * @author Dominik Schmidt <[email protected]>
8
 * @author felixboehm <[email protected]>
9
 * @author Joas Schilling <[email protected]>
10
 * @author Jörn Friedrich Dreyer <[email protected]>
11
 * @author Lukas Reschke <[email protected]>
12
 * @author Morris Jobke <[email protected]>
13
 * @author Renaud Fortier <[email protected]>
14
 * @author Robin Appelman <[email protected]>
15
 * @author Robin McCorkell <[email protected]>
16
 * @author Thomas Müller <[email protected]>
17
 * @author Tom Needham <[email protected]>
18
 * @author Roger Szabo <[email protected]>
19
 * @author Vinicius Brand <[email protected]>
20
 * @author Daniel Tygel <[email protected]>
21
 *
22
 * @license AGPL-3.0
23
 *
24
 * This code is free software: you can redistribute it and/or modify
25
 * it under the terms of the GNU Affero General Public License, version 3,
26
 * as published by the Free Software Foundation.
27
 *
28
 * This program is distributed in the hope that it will be useful,
29
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
30
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
31
 * GNU Affero General Public License for more details.
32
 *
33
 * You should have received a copy of the GNU Affero General Public License, version 3,
34
 * along with this program.  If not, see <http://www.gnu.org/licenses/>
35
 *
36
 */
37
38
namespace OCA\User_LDAP;
39
40
use OC\User\Backend;
41
use OC\User\NoUserException;
42
use OCA\User_LDAP\Exceptions\NotOnLDAP;
43
use OCA\User_LDAP\User\OfflineUser;
44
use OCA\User_LDAP\User\User;
45
use OCP\IConfig;
46
use OCP\IUser;
47
use OCP\IUserSession;
48
use OCP\Notification\IManager as INotificationManager;
49
use OCP\Util;
50
51
class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserInterface, IUserLDAP {
52
	/** @var \OCP\IConfig */
53
	protected $ocConfig;
54
55
	/** @var INotificationManager */
56
	protected $notificationManager;
57
58
	/** @var string */
59
	protected $currentUserInDeletionProcess;
60
61
	/** @var UserPluginManager */
62
	protected $userPluginManager;
63
64
	/**
65
	 * @param Access $access
66
	 * @param \OCP\IConfig $ocConfig
67
	 * @param \OCP\Notification\IManager $notificationManager
68
	 * @param IUserSession $userSession
69
	 */
70
	public function __construct(Access $access, IConfig $ocConfig, INotificationManager $notificationManager, IUserSession $userSession, UserPluginManager $userPluginManager) {
71
		parent::__construct($access);
72
		$this->ocConfig = $ocConfig;
73
		$this->notificationManager = $notificationManager;
74
		$this->userPluginManager = $userPluginManager;
75
		$this->registerHooks($userSession);
76
	}
77
78
	protected function registerHooks(IUserSession $userSession) {
79
		$userSession->listen('\OC\User', 'preDelete', [$this, 'preDeleteUser']);
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface OCP\IUserSession as the method listen() does only exist in the following implementations of said interface: OC\User\Session.

Let’s take a look at an example:

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.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
80
		$userSession->listen('\OC\User', 'postDelete', [$this, 'postDeleteUser']);
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface OCP\IUserSession as the method listen() does only exist in the following implementations of said interface: OC\User\Session.

Let’s take a look at an example:

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.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
81
	}
82
83
	public function preDeleteUser(IUser $user) {
84
		$this->currentUserInDeletionProcess = $user->getUID();
85
	}
86
87
	public function postDeleteUser() {
88
		$this->currentUserInDeletionProcess = null;
89
	}
90
91
	/**
92
	 * checks whether the user is allowed to change his avatar in Nextcloud
93
	 * @param string $uid the Nextcloud user name
94
	 * @return boolean either the user can or cannot
95
	 */
96
	public function canChangeAvatar($uid) {
97
		if ($this->userPluginManager->implementsActions(Backend::PROVIDE_AVATAR)) {
98
			return $this->userPluginManager->canChangeAvatar($uid);
99
		}
100
101
		$user = $this->access->userManager->get($uid);
102
		if(!$user instanceof User) {
103
			return false;
104
		}
105
		if($user->getAvatarImage() === false) {
106
			return true;
107
		}
108
109
		return false;
110
	}
111
112
	/**
113
	 * returns the username for the given login name, if available
114
	 *
115
	 * @param string $loginName
116
	 * @return string|false
117
	 */
118
	public function loginName2UserName($loginName) {
119
		$cacheKey = 'loginName2UserName-'.$loginName;
120
		$username = $this->access->connection->getFromCache($cacheKey);
121
		if(!is_null($username)) {
122
			return $username;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return $username; (object|integer|double|string|array|boolean) is incompatible with the return type documented by OCA\User_LDAP\User_LDAP::loginName2UserName of type string|false.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
123
		}
124
125
		try {
126
			$ldapRecord = $this->getLDAPUserByLoginName($loginName);
127
			$user = $this->access->userManager->get($ldapRecord['dn'][0]);
128
			if($user instanceof OfflineUser) {
129
				// this path is not really possible, however get() is documented
130
				// to return User or OfflineUser so we are very defensive here.
131
				$this->access->connection->writeToCache($cacheKey, false);
132
				return false;
133
			}
134
			$username = $user->getUsername();
135
			$this->access->connection->writeToCache($cacheKey, $username);
136
			return $username;
137
		} catch (NotOnLDAP $e) {
138
			$this->access->connection->writeToCache($cacheKey, false);
139
			return false;
140
		}
141
	}
142
	
143
	/**
144
	 * returns the username for the given LDAP DN, if available
145
	 *
146
	 * @param string $dn
147
	 * @return string|false with the username
148
	 */
149
	public function dn2UserName($dn) {
150
		return $this->access->dn2username($dn);
151
	}
152
153
	/**
154
	 * returns an LDAP record based on a given login name
155
	 *
156
	 * @param string $loginName
157
	 * @return array
158
	 * @throws NotOnLDAP
159
	 */
160
	public function getLDAPUserByLoginName($loginName) {
161
		//find out dn of the user name
162
		$attrs = $this->access->userManager->getAttributes();
163
		$users = $this->access->fetchUsersByLoginName($loginName, $attrs);
164
		if(count($users) < 1) {
165
			throw new NotOnLDAP('No user available for the given login name on ' .
166
				$this->access->connection->ldapHost . ':' . $this->access->connection->ldapPort);
167
		}
168
		return $users[0];
169
	}
170
171
	/**
172
	 * Check if the password is correct without logging in the user
173
	 *
174
	 * @param string $uid The username
175
	 * @param string $password The password
176
	 * @return false|string
177
	 */
178
	public function checkPassword($uid, $password) {
179
		try {
180
			$ldapRecord = $this->getLDAPUserByLoginName($uid);
181
		} catch(NotOnLDAP $e) {
182
			if($this->ocConfig->getSystemValue('loglevel', Util::WARN) === Util::DEBUG) {
183
				\OC::$server->getLogger()->logException($e, ['app' => 'user_ldap']);
184
			}
185
			return false;
186
		}
187
		$dn = $ldapRecord['dn'][0];
188
		$user = $this->access->userManager->get($dn);
189
190
		if(!$user instanceof User) {
191
			Util::writeLog('user_ldap',
192
				'LDAP Login: Could not get user object for DN ' . $dn .
193
				'. Maybe the LDAP entry has no set display name attribute?',
194
				Util::WARN);
195
			return false;
196
		}
197
		if($user->getUsername() !== false) {
198
			//are the credentials OK?
199
			if(!$this->access->areCredentialsValid($dn, $password)) {
200
				return false;
201
			}
202
203
			$this->access->cacheUserExists($user->getUsername());
204
			$user->processAttributes($ldapRecord);
205
			$user->markLogin();
206
207
			return $user->getUsername();
208
		}
209
210
		return false;
211
	}
212
213
	/**
214
	 * Set password
215
	 * @param string $uid The username
216
	 * @param string $password The new password
217
	 * @return bool
218
	 */
219
	public function setPassword($uid, $password) {
220
		if ($this->userPluginManager->implementsActions(Backend::SET_PASSWORD)) {
221
			return $this->userPluginManager->setPassword($uid, $password);
222
		}
223
224
		$user = $this->access->userManager->get($uid);
225
226
		if(!$user instanceof User) {
227
			throw new \Exception('LDAP setPassword: Could not get user object for uid ' . $uid .
228
				'. Maybe the LDAP entry has no set display name attribute?');
229
		}
230
		if($user->getUsername() !== false && $this->access->setPassword($user->getDN(), $password)) {
231
			$ldapDefaultPPolicyDN = $this->access->connection->ldapDefaultPPolicyDN;
0 ignored issues
show
Documentation introduced by
The property ldapDefaultPPolicyDN does not exist on object<OCA\User_LDAP\Connection>. Since you implemented __set, maybe consider adding a @property annotation.

Since your code implements the magic setter _set, this function will be called for any write access on an undefined variable. You can add the @property annotation to your class or interface to document the existence of this variable.

<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}

Since the property has write access only, you can use the @property-write annotation instead.

Of course, you may also just have mistyped another name, in which case you should fix the error.

See also the PhpDoc documentation for @property.

Loading history...
232
			$turnOnPasswordChange = $this->access->connection->turnOnPasswordChange;
233
			if (!empty($ldapDefaultPPolicyDN) && (intval($turnOnPasswordChange) === 1)) {
234
				//remove last password expiry warning if any
235
				$notification = $this->notificationManager->createNotification();
236
				$notification->setApp('user_ldap')
237
					->setUser($uid)
238
					->setObject('pwd_exp_warn', $uid)
239
				;
240
				$this->notificationManager->markProcessed($notification);
241
			}
242
			return true;
243
		}
244
245
		return false;
246
	}
247
248
	/**
249
	 * Get a list of all users
250
	 *
251
	 * @param string $search
252
	 * @param integer $limit
253
	 * @param integer $offset
254
	 * @return string[] an array of all uids
255
	 */
256
	public function getUsers($search = '', $limit = 10, $offset = 0) {
257
		$search = $this->access->escapeFilterPart($search, true);
258
		$cachekey = 'getUsers-'.$search.'-'.$limit.'-'.$offset;
259
260
		//check if users are cached, if so return
261
		$ldap_users = $this->access->connection->getFromCache($cachekey);
262
		if(!is_null($ldap_users)) {
263
			return $ldap_users;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return $ldap_users; (object|integer|double|string|array|boolean) is incompatible with the return type declared by the interface OCP\UserInterface::getUsers of type string[].

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
264
		}
265
266
		// if we'd pass -1 to LDAP search, we'd end up in a Protocol
267
		// error. With a limit of 0, we get 0 results. So we pass null.
268
		if($limit <= 0) {
269
			$limit = null;
270
		}
271
		$filter = $this->access->combineFilterWithAnd(array(
272
			$this->access->connection->ldapUserFilter,
273
			$this->access->connection->ldapUserDisplayName . '=*',
274
			$this->access->getFilterPartForUserSearch($search)
275
		));
276
277
		Util::writeLog('user_ldap',
278
			'getUsers: Options: search '.$search.' limit '.$limit.' offset '.$offset.' Filter: '.$filter,
279
			Util::DEBUG);
280
		//do the search and translate results to owncloud names
281
		$ldap_users = $this->access->fetchListOfUsers(
282
			$filter,
283
			$this->access->userManager->getAttributes(true),
284
			$limit, $offset);
285
		$ldap_users = $this->access->nextcloudUserNames($ldap_users);
286
		Util::writeLog('user_ldap', 'getUsers: '.count($ldap_users). ' Users found', Util::DEBUG);
287
288
		$this->access->connection->writeToCache($cachekey, $ldap_users);
289
		return $ldap_users;
290
	}
291
292
	/**
293
	 * checks whether a user is still available on LDAP
294
	 *
295
	 * @param string|\OCA\User_LDAP\User\User $user either the Nextcloud user
296
	 * name or an instance of that user
297
	 * @return bool
298
	 * @throws \Exception
299
	 * @throws \OC\ServerNotAvailableException
300
	 */
301
	public function userExistsOnLDAP($user) {
302
		if(is_string($user)) {
303
			$user = $this->access->userManager->get($user);
304
		}
305
		if(is_null($user)) {
306
			return false;
307
		}
308
309
		$dn = $user->getDN();
310
		//check if user really still exists by reading its entry
311
		if(!is_array($this->access->readAttribute($dn, '', $this->access->connection->ldapUserFilter))) {
312
			$lcr = $this->access->connection->getConnectionResource();
313
			if(is_null($lcr)) {
314
				throw new \Exception('No LDAP Connection to server ' . $this->access->connection->ldapHost);
315
			}
316
317
			try {
318
				$uuid = $this->access->getUserMapper()->getUUIDByDN($dn);
319
				if(!$uuid) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $uuid of type string|false is loosely compared to false; this is ambiguous if the string can be empty. You might want to explicitly use === false instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For string values, the empty string '' is a special case, in particular the following results might be unexpected:

''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
Loading history...
320
					return false;
321
				}
322
				$newDn = $this->access->getUserDnByUuid($uuid);
323
				//check if renamed user is still valid by reapplying the ldap filter
324 View Code Duplication
				if(!is_array($this->access->readAttribute($newDn, '', $this->access->connection->ldapUserFilter))) {
325
					return false;
326
				}
327
				$this->access->getUserMapper()->setDNbyUUID($newDn, $uuid);
328
				return true;
329
			} catch (\Exception $e) {
330
				return false;
331
			}
332
		}
333
334
		if($user instanceof OfflineUser) {
335
			$user->unmark();
336
		}
337
338
		return true;
339
	}
340
341
	/**
342
	 * check if a user exists
343
	 * @param string $uid the username
344
	 * @return boolean
345
	 * @throws \Exception when connection could not be established
346
	 */
347
	public function userExists($uid) {
348
		$userExists = $this->access->connection->getFromCache('userExists'.$uid);
349
		if(!is_null($userExists)) {
350
			return (bool)$userExists;
351
		}
352
		//getting dn, if false the user does not exist. If dn, he may be mapped only, requires more checking.
353
		$user = $this->access->userManager->get($uid);
354
355
		if(is_null($user)) {
356
			Util::writeLog('user_ldap', 'No DN found for '.$uid.' on '.
357
				$this->access->connection->ldapHost, Util::DEBUG);
358
			$this->access->connection->writeToCache('userExists'.$uid, false);
359
			return false;
360
		} else if($user instanceof OfflineUser) {
361
			//express check for users marked as deleted. Returning true is
362
			//necessary for cleanup
363
			return true;
364
		}
365
366
		$result = $this->userExistsOnLDAP($user);
367
		$this->access->connection->writeToCache('userExists'.$uid, $result);
368
		if($result === true) {
369
			$user->update();
370
		}
371
		return $result;
372
	}
373
374
	/**
375
	* returns whether a user was deleted in LDAP
376
	*
377
	* @param string $uid The username of the user to delete
378
	* @return bool
379
	*/
380
	public function deleteUser($uid) {
381
		if ($this->userPluginManager->canDeleteUser()) {
382
			return $this->userPluginManager->deleteUser($uid);
383
		}
384
385
		$marked = $this->ocConfig->getUserValue($uid, 'user_ldap', 'isDeleted', 0);
386 View Code Duplication
		if(intval($marked) === 0) {
387
			\OC::$server->getLogger()->notice(
388
				'User '.$uid . ' is not marked as deleted, not cleaning up.',
389
				array('app' => 'user_ldap'));
390
			return false;
391
		}
392
		\OC::$server->getLogger()->info('Cleaning up after user ' . $uid,
393
			array('app' => 'user_ldap'));
394
395
		$this->access->getUserMapper()->unmap($uid);
396
		$this->access->userManager->invalidate($uid);
397
		return true;
398
	}
399
400
	/**
401
	 * get the user's home directory
402
	 *
403
	 * @param string $uid the username
404
	 * @return bool|string
405
	 * @throws NoUserException
406
	 * @throws \Exception
407
	 */
408
	public function getHome($uid) {
409
		// user Exists check required as it is not done in user proxy!
410
		if(!$this->userExists($uid)) {
411
			return false;
412
		}
413
414
		if ($this->userPluginManager->implementsActions(Backend::GET_HOME)) {
415
			return $this->userPluginManager->getHome($uid);
416
		}
417
418
		$cacheKey = 'getHome'.$uid;
419
		$path = $this->access->connection->getFromCache($cacheKey);
420
		if(!is_null($path)) {
421
			return $path;
422
		}
423
424
		// early return path if it is a deleted user
425
		$user = $this->access->userManager->get($uid);
426
		if($user instanceof OfflineUser) {
427
			if($this->currentUserInDeletionProcess !== null
428
				&& $this->currentUserInDeletionProcess === $user->getOCName()
429
			) {
430
				return $user->getHomePath();
431
			} else {
432
				throw new NoUserException($uid . ' is not a valid user anymore');
433
			}
434
		} else if ($user === null) {
435
			throw new NoUserException($uid . ' is not a valid user anymore');
436
		}
437
438
		$path = $user->getHomePath();
439
		$this->access->cacheUserHome($uid, $path);
0 ignored issues
show
Bug introduced by
It seems like $path defined by $user->getHomePath() on line 438 can also be of type boolean; however, OCA\User_LDAP\Access::cacheUserHome() does only seem to accept string|false, 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...
440
441
		return $path;
442
	}
443
444
	/**
445
	 * get display name of the user
446
	 * @param string $uid user ID of the user
447
	 * @return string|false display name
448
	 */
449
	public function getDisplayName($uid) {
450
		if ($this->userPluginManager->implementsActions(Backend::GET_DISPLAYNAME)) {
451
			return $this->userPluginManager->getDisplayName($uid);
452
		}
453
454
		if(!$this->userExists($uid)) {
455
			return false;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return false; (false) is incompatible with the return type declared by the interface OCP\UserInterface::getDisplayName of type string.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
456
		}
457
458
		$cacheKey = 'getDisplayName'.$uid;
459
		if(!is_null($displayName = $this->access->connection->getFromCache($cacheKey))) {
460
			return $displayName;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return $displayName; (object|integer|double|string|array|boolean) is incompatible with the return type declared by the interface OCP\UserInterface::getDisplayName of type string.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
461
		}
462
463
		//Check whether the display name is configured to have a 2nd feature
464
		$additionalAttribute = $this->access->connection->ldapUserDisplayName2;
465
		$displayName2 = '';
466
		if ($additionalAttribute !== '') {
467
			$displayName2 = $this->access->readAttribute(
468
				$this->access->username2dn($uid),
0 ignored issues
show
Security Bug introduced by
It seems like $this->access->username2dn($uid) targeting OCA\User_LDAP\Access::username2dn() can also be of type false; however, OCA\User_LDAP\Access::readAttribute() does only seem to accept string, did you maybe forget to handle an error condition?
Loading history...
469
				$additionalAttribute);
470
		}
471
472
		$displayName = $this->access->readAttribute(
473
			$this->access->username2dn($uid),
0 ignored issues
show
Security Bug introduced by
It seems like $this->access->username2dn($uid) targeting OCA\User_LDAP\Access::username2dn() can also be of type false; however, OCA\User_LDAP\Access::readAttribute() does only seem to accept string, did you maybe forget to handle an error condition?
Loading history...
474
			$this->access->connection->ldapUserDisplayName);
475
476
		if($displayName && (count($displayName) > 0)) {
477
			$displayName = $displayName[0];
478
479
			if (is_array($displayName2)){
480
				$displayName2 = count($displayName2) > 0 ? $displayName2[0] : '';
481
			}
482
483
			$user = $this->access->userManager->get($uid);
484
			if ($user instanceof User) {
485
				$displayName = $user->composeAndStoreDisplayName($displayName, $displayName2);
486
				$this->access->connection->writeToCache($cacheKey, $displayName);
487
			}
488
			if ($user instanceof OfflineUser) {
489
				/** @var OfflineUser $user*/
490
				$displayName = $user->getDisplayName();
491
			}
492
			return $displayName;
493
		}
494
495
		return null;
496
	}
497
498
	/**
499
	 * set display name of the user
500
	 * @param string $uid user ID of the user
501
	 * @param string $displayName new display name of the user
502
	 * @return string|false display name
503
	 */
504
	public function setDisplayName($uid, $displayName) {
505
		if ($this->userPluginManager->implementsActions(Backend::SET_DISPLAYNAME)) {
506
			return $this->userPluginManager->setDisplayName($uid, $displayName);
507
		}
508
		return false;
509
	}
510
511
	/**
512
	 * Get a list of all display names
513
	 *
514
	 * @param string $search
515
	 * @param string|null $limit
516
	 * @param string|null $offset
517
	 * @return array an array of all displayNames (value) and the corresponding uids (key)
518
	 */
519
	public function getDisplayNames($search = '', $limit = null, $offset = null) {
520
		$cacheKey = 'getDisplayNames-'.$search.'-'.$limit.'-'.$offset;
521
		if(!is_null($displayNames = $this->access->connection->getFromCache($cacheKey))) {
522
			return $displayNames;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return $displayNames; (object|integer|double|string|array|boolean) is incompatible with the return type declared by the interface OCP\UserInterface::getDisplayNames of type array.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
523
		}
524
525
		$displayNames = array();
526
		$users = $this->getUsers($search, $limit, $offset);
527
		foreach ($users as $user) {
0 ignored issues
show
Bug introduced by
The expression $users of type object|integer|double|string|array|boolean is not guaranteed to be traversable. How about adding an additional type check?

There are different options of fixing this problem.

  1. If you want to be on the safe side, you can add an additional type-check:

    $collection = json_decode($data, true);
    if ( ! is_array($collection)) {
        throw new \RuntimeException('$collection must be an array.');
    }
    
    foreach ($collection as $item) { /** ... */ }
    
  2. If you are sure that the expression is traversable, you might want to add a doc comment cast to improve IDE auto-completion and static analysis:

    /** @var array $collection */
    $collection = json_decode($data, true);
    
    foreach ($collection as $item) { /** .. */ }
    
  3. Mark the issue as a false-positive: Just hover the remove button, in the top-right corner of this issue for more options.

Loading history...
528
			$displayNames[$user] = $this->getDisplayName($user);
529
		}
530
		$this->access->connection->writeToCache($cacheKey, $displayNames);
531
		return $displayNames;
532
	}
533
534
	/**
535
	* Check if backend implements actions
536
	* @param int $actions bitwise-or'ed actions
537
	* @return boolean
538
	*
539
	* Returns the supported actions as int to be
540
	* compared with \OC\User\Backend::CREATE_USER etc.
541
	*/
542
	public function implementsActions($actions) {
543
		return (bool)((Backend::CHECK_PASSWORD
544
			| Backend::GET_HOME
545
			| Backend::GET_DISPLAYNAME
546
			| Backend::PROVIDE_AVATAR
547
			| Backend::COUNT_USERS
548
			| ((intval($this->access->connection->turnOnPasswordChange) === 1)?(Backend::SET_PASSWORD):0)
549
			| $this->userPluginManager->getImplementedActions())
550
			& $actions);
551
	}
552
553
	/**
554
	 * @return bool
555
	 */
556
	public function hasUserListings() {
557
		return true;
558
	}
559
560
	/**
561
	 * counts the users in LDAP
562
	 *
563
	 * @return int|bool
564
	 */
565
	public function countUsers() {
566
		if ($this->userPluginManager->implementsActions(Backend::COUNT_USERS)) {
567
			return $this->userPluginManager->countUsers();
568
		}
569
570
		$filter = $this->access->getFilterForUserCount();
571
		$cacheKey = 'countUsers-'.$filter;
572
		if(!is_null($entries = $this->access->connection->getFromCache($cacheKey))) {
573
			return $entries;
574
		}
575
		$entries = $this->access->countUsers($filter);
576
		$this->access->connection->writeToCache($cacheKey, $entries);
577
		return $entries;
578
	}
579
580
	/**
581
	 * Backend name to be shown in user management
582
	 * @return string the name of the backend to be shown
583
	 */
584
	public function getBackendName(){
585
		return 'LDAP';
586
	}
587
	
588
	/**
589
	 * Return access for LDAP interaction.
590
	 * @param string $uid
591
	 * @return Access instance of Access for LDAP interaction
592
	 */
593
	public function getLDAPAccess($uid) {
594
		return $this->access;
595
	}
596
	
597
	/**
598
	 * Return LDAP connection resource from a cloned connection.
599
	 * The cloned connection needs to be closed manually.
600
	 * of the current access.
601
	 * @param string $uid
602
	 * @return resource of the LDAP connection
603
	 */
604
	public function getNewLDAPConnection($uid) {
605
		$connection = clone $this->access->getConnection();
606
		return $connection->getConnectionResource();
607
	}
608
609
	/**
610
	 * create new user
611
	 * @param string $username username of the new user
612
	 * @param string $password password of the new user
613
	 * @return bool was the user created?
614
	 */
615
	public function createUser($username, $password) {
616
		if ($this->userPluginManager->implementsActions(Backend::CREATE_USER)) {
617
			return $this->userPluginManager->createUser($username, $password);
618
		}
619
		return false;
620
	}
621
622
}
623