Completed
Pull Request — master (#6522)
by Blizzz
13:30
created

User_LDAP::getHome()   C

Complexity

Conditions 7
Paths 6

Size

Total Lines 31
Code Lines 19

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
cc 7
eloc 19
nc 6
nop 1
dl 0
loc 31
rs 6.7272
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
 *
20
 * @license AGPL-3.0
21
 *
22
 * This code is free software: you can redistribute it and/or modify
23
 * it under the terms of the GNU Affero General Public License, version 3,
24
 * as published by the Free Software Foundation.
25
 *
26
 * This program is distributed in the hope that it will be useful,
27
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
28
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
29
 * GNU Affero General Public License for more details.
30
 *
31
 * You should have received a copy of the GNU Affero General Public License, version 3,
32
 * along with this program.  If not, see <http://www.gnu.org/licenses/>
33
 *
34
 */
35
36
namespace OCA\User_LDAP;
37
38
use OC\User\Backend;
39
use OC\User\NoUserException;
40
use OCA\User_LDAP\Exceptions\NotOnLDAP;
41
use OCA\User_LDAP\User\OfflineUser;
42
use OCA\User_LDAP\User\User;
43
use OCP\IConfig;
44
use OCP\IUser;
45
use OCP\IUserSession;
46
use OCP\Notification\IManager as INotificationManager;
47
use OCP\Util;
48
49
class User_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserInterface, IUserLDAP {
50
	/** @var \OCP\IConfig */
51
	protected $ocConfig;
52
53
	/** @var INotificationManager */
54
	protected $notificationManager;
55
56
	/** @var string */
57
	protected $currentUserInDeletionProcess;
58
59
	/**
60
	 * @param Access $access
61
	 * @param \OCP\IConfig $ocConfig
62
	 * @param \OCP\Notification\IManager $notificationManager
63
	 * @param IUserSession $userSession
64
	 */
65
	public function __construct(Access $access, IConfig $ocConfig, INotificationManager $notificationManager, IUserSession $userSession) {
66
		parent::__construct($access);
67
		$this->ocConfig = $ocConfig;
68
		$this->notificationManager = $notificationManager;
69
		$this->registerHooks($userSession);
70
	}
71
72
	protected function registerHooks(IUserSession $userSession) {
73
		$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...
74
		$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...
75
	}
76
77
	public function preDeleteUser(IUser $user) {
78
		$this->currentUserInDeletionProcess = $user->getUID();
79
	}
80
81
	public function postDeleteUser() {
82
		$this->currentUserInDeletionProcess = null;
83
	}
84
85
	/**
86
	 * checks whether the user is allowed to change his avatar in Nextcloud
87
	 * @param string $uid the Nextcloud user name
88
	 * @return boolean either the user can or cannot
89
	 */
90
	public function canChangeAvatar($uid) {
91
		$user = $this->access->userManager->get($uid);
92
		if(!$user instanceof User) {
93
			return false;
94
		}
95
		if($user->getAvatarImage() === false) {
96
			return true;
97
		}
98
99
		return false;
100
	}
101
102
	/**
103
	 * returns the username for the given login name, if available
104
	 *
105
	 * @param string $loginName
106
	 * @return string|false
107
	 */
108
	public function loginName2UserName($loginName) {
109
		$cacheKey = 'loginName2UserName-'.$loginName;
110
		$username = $this->access->connection->getFromCache($cacheKey);
111
		if(!is_null($username)) {
112
			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...
113
		}
114
115
		try {
116
			$ldapRecord = $this->getLDAPUserByLoginName($loginName);
117
			$user = $this->access->userManager->get($ldapRecord['dn'][0]);
118
			if($user instanceof OfflineUser) {
119
				// this path is not really possible, however get() is documented
120
				// to return User or OfflineUser so we are very defensive here.
121
				$this->access->connection->writeToCache($cacheKey, false);
122
				return false;
123
			}
124
			$username = $user->getUsername();
125
			$this->access->connection->writeToCache($cacheKey, $username);
126
			return $username;
127
		} catch (NotOnLDAP $e) {
128
			$this->access->connection->writeToCache($cacheKey, false);
129
			return false;
130
		}
131
	}
132
	
133
	/**
134
	 * returns the username for the given LDAP DN, if available
135
	 *
136
	 * @param string $dn
137
	 * @return string|false with the username
138
	 */
139
	public function dn2UserName($dn) {
140
		return $this->access->dn2username($dn);
141
	}
142
143
	/**
144
	 * returns an LDAP record based on a given login name
145
	 *
146
	 * @param string $loginName
147
	 * @return array
148
	 * @throws NotOnLDAP
149
	 */
150
	public function getLDAPUserByLoginName($loginName) {
151
		//find out dn of the user name
152
		$attrs = $this->access->userManager->getAttributes();
153
		$users = $this->access->fetchUsersByLoginName($loginName, $attrs);
154
		if(count($users) < 1) {
155
			throw new NotOnLDAP('No user available for the given login name on ' .
156
				$this->access->connection->ldapHost . ':' . $this->access->connection->ldapPort);
157
		}
158
		return $users[0];
159
	}
160
161
	/**
162
	 * Check if the password is correct without logging in the user
163
	 *
164
	 * @param string $uid The username
165
	 * @param string $password The password
166
	 * @return false|string
167
	 */
168
	public function checkPassword($uid, $password) {
169
		try {
170
			$ldapRecord = $this->getLDAPUserByLoginName($uid);
171
		} catch(NotOnLDAP $e) {
172
			if($this->ocConfig->getSystemValue('loglevel', Util::WARN) === Util::DEBUG) {
173
				\OC::$server->getLogger()->logException($e, ['app' => 'user_ldap']);
174
			}
175
			return false;
176
		}
177
		$dn = $ldapRecord['dn'][0];
178
		$user = $this->access->userManager->get($dn);
179
180
		if(!$user instanceof User) {
181
			Util::writeLog('user_ldap',
182
				'LDAP Login: Could not get user object for DN ' . $dn .
183
				'. Maybe the LDAP entry has no set display name attribute?',
184
				Util::WARN);
185
			return false;
186
		}
187
		if($user->getUsername() !== false) {
188
			//are the credentials OK?
189
			if(!$this->access->areCredentialsValid($dn, $password)) {
190
				return false;
191
			}
192
193
			$this->access->cacheUserExists($user->getUsername());
194
			$user->processAttributes($ldapRecord);
195
			$user->markLogin();
196
197
			return $user->getUsername();
198
		}
199
200
		return false;
201
	}
202
203
	/**
204
	 * Set password
205
	 * @param string $uid The username
206
	 * @param string $password The new password
207
	 * @return bool
208
	 */
209
	public function setPassword($uid, $password) {
210
		$user = $this->access->userManager->get($uid);
211
212
		if(!$user instanceof User) {
213
			throw new \Exception('LDAP setPassword: Could not get user object for uid ' . $uid .
214
				'. Maybe the LDAP entry has no set display name attribute?');
215
		}
216
		if($user->getUsername() !== false && $this->access->setPassword($user->getDN(), $password)) {
217
			$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...
218
			$turnOnPasswordChange = $this->access->connection->turnOnPasswordChange;
219
			if (!empty($ldapDefaultPPolicyDN) && (intval($turnOnPasswordChange) === 1)) {
220
				//remove last password expiry warning if any
221
				$notification = $this->notificationManager->createNotification();
222
				$notification->setApp('user_ldap')
223
					->setUser($uid)
224
					->setObject('pwd_exp_warn', $uid)
225
				;
226
				$this->notificationManager->markProcessed($notification);
227
			}
228
			return true;
229
		}
230
231
		return false;
232
	}
233
234
	/**
235
	 * Get a list of all users
236
	 *
237
	 * @param string $search
238
	 * @param integer $limit
239
	 * @param integer $offset
240
	 * @return string[] an array of all uids
241
	 */
242
	public function getUsers($search = '', $limit = 10, $offset = 0) {
243
		$search = $this->access->escapeFilterPart($search, true);
244
		$cachekey = 'getUsers-'.$search.'-'.$limit.'-'.$offset;
245
246
		//check if users are cached, if so return
247
		$ldap_users = $this->access->connection->getFromCache($cachekey);
248
		if(!is_null($ldap_users)) {
249
			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...
250
		}
251
252
		// if we'd pass -1 to LDAP search, we'd end up in a Protocol
253
		// error. With a limit of 0, we get 0 results. So we pass null.
254
		if($limit <= 0) {
255
			$limit = null;
256
		}
257
		$filter = $this->access->combineFilterWithAnd(array(
258
			$this->access->connection->ldapUserFilter,
259
			$this->access->connection->ldapUserDisplayName . '=*',
260
			$this->access->getFilterPartForUserSearch($search)
261
		));
262
263
		Util::writeLog('user_ldap',
264
			'getUsers: Options: search '.$search.' limit '.$limit.' offset '.$offset.' Filter: '.$filter,
265
			Util::DEBUG);
266
		//do the search and translate results to owncloud names
267
		$ldap_users = $this->access->fetchListOfUsers(
268
			$filter,
269
			$this->access->userManager->getAttributes(true),
270
			$limit, $offset);
271
		$ldap_users = $this->access->nextcloudUserNames($ldap_users);
272
		Util::writeLog('user_ldap', 'getUsers: '.count($ldap_users). ' Users found', Util::DEBUG);
273
274
		$this->access->connection->writeToCache($cachekey, $ldap_users);
275
		return $ldap_users;
276
	}
277
278
	/**
279
	 * checks whether a user is still available on LDAP
280
	 *
281
	 * @param string|\OCA\User_LDAP\User\User $user either the Nextcloud user
282
	 * name or an instance of that user
283
	 * @return bool
284
	 * @throws \Exception
285
	 * @throws \OC\ServerNotAvailableException
286
	 */
287
	public function userExistsOnLDAP($user) {
288
		if(is_string($user)) {
289
			$user = $this->access->userManager->get($user);
290
		}
291
		if(is_null($user)) {
292
			return false;
293
		}
294
295
		$dn = $user->getDN();
296
		//check if user really still exists by reading its entry
297
		if(!is_array($this->access->readAttribute($dn, '', $this->access->connection->ldapUserFilter))) {
298
			$lcr = $this->access->connection->getConnectionResource();
299
			if(is_null($lcr)) {
300
				throw new \Exception('No LDAP Connection to server ' . $this->access->connection->ldapHost);
301
			}
302
303
			try {
304
				$uuid = $this->access->getUserMapper()->getUUIDByDN($dn);
305
				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...
306
					return false;
307
				}
308
				$newDn = $this->access->getUserDnByUuid($uuid);
309
				//check if renamed user is still valid by reapplying the ldap filter
310 View Code Duplication
				if(!is_array($this->access->readAttribute($newDn, '', $this->access->connection->ldapUserFilter))) {
311
					return false;
312
				}
313
				$this->access->getUserMapper()->setDNbyUUID($newDn, $uuid);
314
				return true;
315
			} catch (\Exception $e) {
316
				return false;
317
			}
318
		}
319
320
		if($user instanceof OfflineUser) {
321
			$user->unmark();
322
		}
323
324
		return true;
325
	}
326
327
	/**
328
	 * check if a user exists
329
	 * @param string $uid the username
330
	 * @return boolean
331
	 * @throws \Exception when connection could not be established
332
	 */
333
	public function userExists($uid) {
334
		$userExists = $this->access->connection->getFromCache('userExists'.$uid);
335
		if(!is_null($userExists)) {
336
			return (bool)$userExists;
337
		}
338
		//getting dn, if false the user does not exist. If dn, he may be mapped only, requires more checking.
339
		$user = $this->access->userManager->get($uid);
340
341
		if(is_null($user)) {
342
			Util::writeLog('user_ldap', 'No DN found for '.$uid.' on '.
343
				$this->access->connection->ldapHost, Util::DEBUG);
344
			$this->access->connection->writeToCache('userExists'.$uid, false);
345
			return false;
346
		} else if($user instanceof OfflineUser) {
347
			//express check for users marked as deleted. Returning true is
348
			//necessary for cleanup
349
			return true;
350
		}
351
352
		$result = $this->userExistsOnLDAP($user);
353
		$this->access->connection->writeToCache('userExists'.$uid, $result);
354
		if($result === true) {
355
			$user->update();
356
		}
357
		return $result;
358
	}
359
360
	/**
361
	* returns whether a user was deleted in LDAP
362
	*
363
	* @param string $uid The username of the user to delete
364
	* @return bool
365
	*/
366
	public function deleteUser($uid) {
367
		$marked = $this->ocConfig->getUserValue($uid, 'user_ldap', 'isDeleted', 0);
368 View Code Duplication
		if(intval($marked) === 0) {
369
			\OC::$server->getLogger()->notice(
370
				'User '.$uid . ' is not marked as deleted, not cleaning up.',
371
				array('app' => 'user_ldap'));
372
			return false;
373
		}
374
		\OC::$server->getLogger()->info('Cleaning up after user ' . $uid,
375
			array('app' => 'user_ldap'));
376
377
		$this->access->getUserMapper()->unmap($uid);
378
		$this->access->userManager->invalidate($uid);
379
		return true;
380
	}
381
382
	/**
383
	 * get the user's home directory
384
	 *
385
	 * @param string $uid the username
386
	 * @return bool|string
387
	 * @throws NoUserException
388
	 * @throws \Exception
389
	 */
390
	public function getHome($uid) {
391
		// user Exists check required as it is not done in user proxy!
392
		if(!$this->userExists($uid)) {
393
			return false;
394
		}
395
396
		$cacheKey = 'getHome'.$uid;
397
		$path = $this->access->connection->getFromCache($cacheKey);
398
		if(!is_null($path)) {
399
			return $path;
400
		}
401
402
		// early return path if it is a deleted user
403
		$user = $this->access->userManager->get($uid);
404
		if($user instanceof OfflineUser) {
405
			if($this->currentUserInDeletionProcess !== null
406
				&& $this->currentUserInDeletionProcess === $user->getOCName()
407
			) {
408
				return $user->getHomePath();
409
			} else {
410
				throw new NoUserException($uid . ' is not a valid user anymore');
411
			}
412
		} else if ($user === null) {
413
			throw new NoUserException($uid . ' is not a valid user anymore');
414
		}
415
416
		$path = $user->getHomePath();
417
		$this->access->cacheUserHome($uid, $path);
0 ignored issues
show
Bug introduced by
It seems like $path defined by $user->getHomePath() on line 416 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...
418
419
		return $path;
420
	}
421
422
	/**
423
	 * get display name of the user
424
	 * @param string $uid user ID of the user
425
	 * @return string|false display name
426
	 */
427
	public function getDisplayName($uid) {
428
		if(!$this->userExists($uid)) {
429
			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...
430
		}
431
432
		$cacheKey = 'getDisplayName'.$uid;
433
		if(!is_null($displayName = $this->access->connection->getFromCache($cacheKey))) {
434
			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...
435
		}
436
437
		//Check whether the display name is configured to have a 2nd feature
438
		$additionalAttribute = $this->access->connection->ldapUserDisplayName2;
439
		$displayName2 = '';
440
		if ($additionalAttribute !== '') {
441
			$displayName2 = $this->access->readAttribute(
442
				$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...
443
				$additionalAttribute);
444
		}
445
446
		$displayName = $this->access->readAttribute(
447
			$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...
448
			$this->access->connection->ldapUserDisplayName);
449
450
		if($displayName && (count($displayName) > 0)) {
451
			$displayName = $displayName[0];
452
453
			if (is_array($displayName2)){
454
				$displayName2 = count($displayName2) > 0 ? $displayName2[0] : '';
455
			}
456
457
			$user = $this->access->userManager->get($uid);
458
			if ($user instanceof User) {
459
				$displayName = $user->composeAndStoreDisplayName($displayName, $displayName2);
460
				$this->access->connection->writeToCache($cacheKey, $displayName);
461
			}
462
			if ($user instanceof OfflineUser) {
463
				/** @var OfflineUser $user*/
464
				$displayName = $user->getDisplayName();
465
			}
466
			return $displayName;
467
		}
468
469
		return null;
470
	}
471
472
	/**
473
	 * Get a list of all display names
474
	 *
475
	 * @param string $search
476
	 * @param string|null $limit
477
	 * @param string|null $offset
478
	 * @return array an array of all displayNames (value) and the corresponding uids (key)
479
	 */
480
	public function getDisplayNames($search = '', $limit = null, $offset = null) {
481
		$cacheKey = 'getDisplayNames-'.$search.'-'.$limit.'-'.$offset;
482
		if(!is_null($displayNames = $this->access->connection->getFromCache($cacheKey))) {
483
			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...
484
		}
485
486
		$displayNames = array();
487
		$users = $this->getUsers($search, $limit, $offset);
488
		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...
489
			$displayNames[$user] = $this->getDisplayName($user);
490
		}
491
		$this->access->connection->writeToCache($cacheKey, $displayNames);
492
		return $displayNames;
493
	}
494
495
	/**
496
	* Check if backend implements actions
497
	* @param int $actions bitwise-or'ed actions
498
	* @return boolean
499
	*
500
	* Returns the supported actions as int to be
501
	* compared with \OC\User\Backend::CREATE_USER etc.
502
	*/
503
	public function implementsActions($actions) {
504
		return (bool)((Backend::CHECK_PASSWORD
505
			| Backend::GET_HOME
506
			| Backend::GET_DISPLAYNAME
507
			| Backend::PROVIDE_AVATAR
508
			| Backend::COUNT_USERS
509
			| ((intval($this->access->connection->turnOnPasswordChange) === 1)?(Backend::SET_PASSWORD):0))
510
			& $actions);
511
	}
512
513
	/**
514
	 * @return bool
515
	 */
516
	public function hasUserListings() {
517
		return true;
518
	}
519
520
	/**
521
	 * counts the users in LDAP
522
	 *
523
	 * @return int|bool
524
	 */
525
	public function countUsers() {
526
		$filter = $this->access->getFilterForUserCount();
527
		$cacheKey = 'countUsers-'.$filter;
528
		if(!is_null($entries = $this->access->connection->getFromCache($cacheKey))) {
529
			return $entries;
530
		}
531
		$entries = $this->access->countUsers($filter);
532
		$this->access->connection->writeToCache($cacheKey, $entries);
533
		return $entries;
534
	}
535
536
	/**
537
	 * Backend name to be shown in user management
538
	 * @return string the name of the backend to be shown
539
	 */
540
	public function getBackendName(){
541
		return 'LDAP';
542
	}
543
	
544
	/**
545
	 * Return access for LDAP interaction.
546
	 * @param string $uid
547
	 * @return Access instance of Access for LDAP interaction
548
	 */
549
	public function getLDAPAccess($uid) {
550
		return $this->access;
551
	}
552
	
553
	/**
554
	 * Return LDAP connection resource from a cloned connection.
555
	 * The cloned connection needs to be closed manually.
556
	 * of the current access.
557
	 * @param string $uid
558
	 * @return resource of the LDAP connection
559
	 */
560
	public function getNewLDAPConnection($uid) {
561
		$connection = clone $this->access->getConnection();
562
		return $connection->getConnectionResource();
563
	}
564
}
565