Issues in PersistentLoginService.php - All Issues - Inspection of "Merge pull request #11567 from mrclay/11563_routes" - Elgg/Elgg - Measure and Improve Code Quality continuously with Scrutinizer

Passed

Push — master ( c0a3a7...3b84a4 )

by Jeroen

created 2018-01-08 10:08 UTC

engine/classes/Elgg/PersistentLoginService.php (1 issue)

Showing only issues like (Show All)

return values of null are not assigned.

Bug Minor

Steve Clay 1

<?php
namespace Elgg;
/**
 * \Elgg\PersistentLoginService
 *
 * If a user selects a persistent login, a long, random token is generated and stored in the cookie
 * called "elggperm", and a hash of the token is stored in the DB. If the user's PHP session expires,
 * the session boot sequence will try to log the user in via the token in the cookie.
 *
 * Before Elgg 1.9, the token hashes were stored as "code" in the users_entity table.
 *
 * In Elgg 1.9, the token hashes are stored as "code" in the users_remember_me_cookies
 * table, allowing multiple browsers to maintain persistent logins.
 *
 * @todo Rename the "code" DB column to "hash"
 *
 * Legacy notes: This feature used to be called "remember me"; confusingly, both the tokens and the
 * hashes were called "codes"; old tokens were hexadecimal and lower entropy; new tokens are
 * base64 URL and always begin with the letter "z"; the boot sequence replaces old tokens whenever
 * possible.
 *
 * @package Elgg.Core
 *
 * @access private
 */
class PersistentLoginService {

	/**
	 * Constructor
	 *
	 * @param Database     $db            The DB service
	 * @param \ElggSession $session       The Elgg session
	 * @param \ElggCrypto  $crypto        The cryptography service
	 * @param array        $cookie_config The persistent login cookie settings
	 * @param string       $cookie_token  The token from the request cookie
	 * @param int          $time          The current time
	 */
	public function __construct(
			Database $db,
			\ElggSession $session,
			\ElggCrypto $crypto,
			array $cookie_config,
			$cookie_token,
			$time = null) {
		$this->db = $db;
		$this->session = $session;
		$this->crypto = $crypto;
		$this->cookie_config = $cookie_config;
		$this->cookie_token = $cookie_token;

		$prefix = $this->db->prefix;
		$this->table = "{$prefix}users_remember_me_cookies";
		$this->time = is_numeric($time) ? (int) $time : time();
	}

	/**
	 * Make the user's login persistent
	 *
	 * @param \ElggUser $user The user who logged in
	 *
	 * @return void
	 */
	public function makeLoginPersistent(\ElggUser $user) {
		$token = $this->generateToken();
		$hash = $this->hashToken($token);

		$this->storeHash($user, $hash);
		$this->setCookie($token);
		$this->setSession($token);
	}

	/**
	 * Remove the persisted login token from client and server
	 *
	 * @return void
	 */
	public function removePersistentLogin() {
		if ($this->cookie_token) {
			$client_hash = $this->hashToken($this->cookie_token);
			$this->removeHash($client_hash);
		}

		$this->setCookie("");
		$this->setSession("");
	}

	/**
	 * Handle a password change
	 *
	 * @param \ElggUser $subject  The user whose password changed
	 * @param \ElggUser $modifier The user who changed the password
	 *
	 * @return void
	 */
	public function handlePasswordChange(\ElggUser $subject, \ElggUser $modifier = null) {
		$this->removeAllHashes($subject);
		if (!$modifier || ($modifier->guid !== $subject->guid) || !$this->cookie_token) {
			return;
		}

		$this->makeLoginPersistent($modifier);
	}

	/**
	 * Boot the persistent login session, possibly returning the user who should be
	 * silently logged in.
	 *
	 * @return \ElggUser|null
	 */
	public function bootSession() {
		if (!$this->cookie_token) {
			return null;
		}

		// is this token good?
		$cookie_hash = $this->hashToken($this->cookie_token);
		$user = $this->getUserFromHash($cookie_hash);
class A
{
    function getObject()
    {
        return null;
    }

}

$a = new A();
$object = $a->getObject();
		if ($user) {
			$this->setSession($this->cookie_token);
			// note: if the token is legacy, we don't both replacing it here because
			// it will be replaced during the next request boot
			return $user;
		} else {
			if ($this->isLegacyToken($this->cookie_token)) {
				// may be attempt to brute force legacy low-entropy tokens
				call_user_func($this->_callable_sleep, 1);
			}
			$this->setCookie('');
		}
	}

	/**
	 * Replace the user's token if it's a legacy hexadecimal token
	 *
	 * @param \ElggUser $logged_in_user The logged in user
	 *
	 * @return void
	 */
	public function replaceLegacyToken(\ElggUser $logged_in_user) {
		if (!$this->cookie_token || !$this->isLegacyToken($this->cookie_token)) {
			return;
		}

		// replace user's old weaker-entropy code with new one
		$this->removeHash($this->hashToken($this->cookie_token));
		$this->makeLoginPersistent($logged_in_user);
	}

	/**
	 * Find a user with the given hash
	 *
	 * @param string $hash The hashed token
	 *
	 * @return \ElggUser|null
	 */
	public function getUserFromHash($hash) {
		if (!$hash) {
			return null;
		}

		$hash = $this->db->sanitizeString($hash);
		$query = "SELECT guid FROM {$this->table} WHERE code = '$hash'";
		try {
			$user_row = $this->db->getDataRow($query);
		} catch (\DatabaseException $e) {
			return $this->handleDbException($e);
		}
		if (!$user_row) {
			return null;
		}

		$user = call_user_func($this->_callable_get_user, $user_row->guid);
		return $user ? $user : null;
	}

	/**
	 * Store a hash in the DB
	 *
	 * @param \ElggUser $user The user for whom we're storing the hash
	 * @param string    $hash The hashed token
	 *
	 * @return void
	 */
	protected function storeHash(\ElggUser $user, $hash) {
		// This prevents inserting the same hash twice, which seems to be happening in some rare cases
		// and for unknown reasons. See https://github.com/Elgg/Elgg/issues/8104
		$this->removeHash($hash);

		$time = time();
		$hash = $this->db->sanitizeString($hash);

		$query = "
			INSERT INTO {$this->table} (code, guid, timestamp)
		    VALUES ('$hash', {$user->guid}, $time)
		";
		try {
			$this->db->insertData($query);
		} catch (\DatabaseException $e) {
			$this->handleDbException($e);
		}
	}

	/**
	 * Remove a hash from the DB
	 *
	 * @param string $hash The hashed token to remove (unused before 1.9)
	 * @return void
	 */
	protected function removeHash($hash) {
		$hash = $this->db->sanitizeString($hash);

		$query = "DELETE FROM {$this->table} WHERE code = '$hash'";
		try {
			$this->db->deleteData($query);
		} catch (\DatabaseException $e) {
			$this->handleDbException($e);
		}
	}

	/**
	 * Swallow a schema not upgraded exception, otherwise rethrow it
	 *
	 * @param \DatabaseException $exception The exception to handle
	 * @param string             $default   The value to return if the table doesn't exist yet
	 *
	 * @return mixed
	 *
	 * @throws \DatabaseException
	 */
	protected function handleDbException(\DatabaseException $exception, $default = null) {
		if (false !== strpos($exception->getMessage(), "users_remember_me_cookies' doesn't exist")) {
			// schema has not been updated so we swallow this exception
			return $default;
		} else {
			throw $exception;
		}
	}

	/**
	 * Remove all the hashes associated with a user
	 *
	 * @param \ElggUser $user The user for whom we're removing hashes
	 *
	 * @return void
	 */
	protected function removeAllHashes(\ElggUser $user) {
		$query = "DELETE FROM {$this->table} WHERE guid = '{$user->guid}'";
		try {
			$this->db->deleteData($query);
		} catch (\DatabaseException $e) {
			$this->handleDbException($e);
		}
	}

	/**
	 * Create a hash from the token
	 *
	 * @param string $token The token to hash
	 *
	 * @return string
	 */
	protected function hashToken($token) {
		// note: with user passwords, you'd want legit password hashing, but since these are randomly
		// generated and long tokens, rainbow tables aren't any help.
		return md5($token);
	}

	/**
	 * Store the token in the client cookie (or remove the cookie)
	 *
	 * @param string $token Empty string to remove cookie
	 *
	 * @return void
	 */
	protected function setCookie($token) {
		$cookie = new \ElggCookie($this->cookie_config['name']);
		foreach (['expire', 'path', 'domain', 'secure', 'httponly'] as $key) {
			$cookie->$key = $this->cookie_config[$key];
		}
		$cookie->value = $token;
		if (!$token) {
			$cookie->expire = $this->time - (86400 * 30);
		}
		call_user_func($this->_callable_elgg_set_cookie, $cookie);
	}

	/**
	 * Store the token in the session (or remove it from the session)
	 *
	 * @param string $token The token to store in session. Empty string to remove.
	 *
	 * @return void
	 */
	protected function setSession($token) {
		if ($token) {
			$this->session->set('code', $token);
		} else {
			$this->session->remove('code');
		}
	}

	/**
	 * Generate a random token (base 64 URL)
	 *
	 * The first char is always "z" to indicate the value has more entropy than the
	 * previously generated ones.
	 *
	 * @return string
	 */
	protected function generateToken() {
		return 'z' . $this->crypto->getRandomString(31);
	}

	/**
	 * Is the given token a legacy MD5 hash?
	 *
	 * @param string $token The token to analyze
	 *
	 * @return bool
	 */
	protected function isLegacyToken($token) {
		return (isset($token[0]) && $token[0] !== 'z');
	}

	/**
	 * @var Database
	 */
	protected $db;

	/**
	 * @var string
	 */
	protected $table;

	/**
	 * @var array
	 */
	protected $cookie_config;

	/**
	 * @var string
	 */
	protected $cookie_token;

	/**
	 * @var \ElggSession
	 */
	protected $session;

	/**
	 * @var \ElggCrypto
	 */
	protected $crypto;

	/**
	 * @var int
	 */
	protected $time;

	/**
	 * DO NOT USE. For unit test mocking
	 * @access private
	 */
	public $_callable_get_user = 'get_user';

	/**
	 * DO NOT USE. For unit test mocking
	 * @access private
	 */
	public $_callable_elgg_set_cookie = 'elgg_set_cookie';

	/**
	 * DO NOT USE. For unit test mocking
	 * @access private
	 */
	public $_callable_sleep = 'sleep';
}



1		<?php
2		namespace Elgg;
3		/**
4		* \Elgg\PersistentLoginService
5		*
6		* If a user selects a persistent login, a long, random token is generated and stored in the cookie
7		* called "elggperm", and a hash of the token is stored in the DB. If the user's PHP session expires,
8		* the session boot sequence will try to log the user in via the token in the cookie.
9		*
10		* Before Elgg 1.9, the token hashes were stored as "code" in the users_entity table.
11		*
12		* In Elgg 1.9, the token hashes are stored as "code" in the users_remember_me_cookies
13		* table, allowing multiple browsers to maintain persistent logins.
14		*
15		* @todo Rename the "code" DB column to "hash"
16		*
17		* Legacy notes: This feature used to be called "remember me"; confusingly, both the tokens and the
18		* hashes were called "codes"; old tokens were hexadecimal and lower entropy; new tokens are
19		* base64 URL and always begin with the letter "z"; the boot sequence replaces old tokens whenever
20		* possible.
21		*
22		* @package Elgg.Core
23		*
24		* @access private
25		*/
26		class PersistentLoginService {
27
28		/**
29		* Constructor
30		*
31		* @param Database $db The DB service
32		* @param \ElggSession $session The Elgg session
33		* @param \ElggCrypto $crypto The cryptography service
34		* @param array $cookie_config The persistent login cookie settings
35		* @param string $cookie_token The token from the request cookie
36		* @param int $time The current time
37		*/
38	4417	public function __construct(
39		Database $db,
40		\ElggSession $session,
41		\ElggCrypto $crypto,
42		array $cookie_config,
43		$cookie_token,
44		$time = null) {
45	4417	$this->db = $db;
46	4417	$this->session = $session;
47	4417	$this->crypto = $crypto;
48	4417	$this->cookie_config = $cookie_config;
49	4417	$this->cookie_token = $cookie_token;
50
51	4417	$prefix = $this->db->prefix;
52	4417	$this->table = "{$prefix}users_remember_me_cookies";
53	4417	$this->time = is_numeric($time) ? (int) $time : time();
54	4417	}
55
56		/**
57		* Make the user's login persistent
58		*
59		* @param \ElggUser $user The user who logged in
60		*
61		* @return void
62		*/
63	3	public function makeLoginPersistent(\ElggUser $user) {
64	3	$token = $this->generateToken();
65	3	$hash = $this->hashToken($token);
66
67	3	$this->storeHash($user, $hash);
68	3	$this->setCookie($token);
69	3	$this->setSession($token);
70	3	}
71
72		/**
73		* Remove the persisted login token from client and server
74		*
75		* @return void
76		*/
77	2	public function removePersistentLogin() {
78	2	if ($this->cookie_token) {
79	1	$client_hash = $this->hashToken($this->cookie_token);
80	1	$this->removeHash($client_hash);
81		}
82
83	2	$this->setCookie("");
84	2	$this->setSession("");
85	2	}
86
87		/**
88		* Handle a password change
89		*
90		* @param \ElggUser $subject The user whose password changed
91		* @param \ElggUser $modifier The user who changed the password
92		*
93		* @return void
94		*/
95	3	public function handlePasswordChange(\ElggUser $subject, \ElggUser $modifier = null) {
96	3	$this->removeAllHashes($subject);
97	3	if (!$modifier \|\| ($modifier->guid !== $subject->guid) \|\| !$this->cookie_token) {
98	2	return;
99		}
100
101	1	$this->makeLoginPersistent($modifier);
102	1	}
103
104		/**
105		* Boot the persistent login session, possibly returning the user who should be
106		* silently logged in.
107		*
108		* @return \ElggUser\|null
109		*/
110	4777	public function bootSession() {
111	4777	if (!$this->cookie_token) {
112	4777	return null;
113		}
114
115		// is this token good?
116	3	$cookie_hash = $this->hashToken($this->cookie_token);
117	3	$user = $this->getUserFromHash($cookie_hash);
		0 ignored issues – show Bug introduced 2017-11-30 19:53 UTC by Report Bug Copy Issue Report Show Similar Issues like this Are you sure the assignment to `$user` is correct as `$this->getUserFromHash($cookie_hash)` targeting `Elgg\PersistentLoginService::getUserFromHash()` seems to always return null. This check looks for function or method calls that always return null and whose return value is assigned to a variable. class A { function getObject() { return null; } } $a = new A(); $object = $a->getObject(); The method `getObject()` can return nothing but null, so it makes no sense to assign that value to a variable. The reason is most likely that a function or method is imcomplete or has been reduced for debug purposes. Loading history...
118	3	if ($user) {
119	1	$this->setSession($this->cookie_token);
120		// note: if the token is legacy, we don't both replacing it here because
121		// it will be replaced during the next request boot
122	1	return $user;
123		} else {
124	2	if ($this->isLegacyToken($this->cookie_token)) {
125		// may be attempt to brute force legacy low-entropy tokens
126	1	call_user_func($this->_callable_sleep, 1);
127		}
128	2	$this->setCookie('');
129		}
130	2	}
131
132		/**
133		* Replace the user's token if it's a legacy hexadecimal token
134		*
135		* @param \ElggUser $logged_in_user The logged in user
136		*
137		* @return void
138		*/
139	3	public function replaceLegacyToken(\ElggUser $logged_in_user) {
140	3	if (!$this->cookie_token \|\| !$this->isLegacyToken($this->cookie_token)) {
141	2	return;
142		}
143
144		// replace user's old weaker-entropy code with new one
145	1	$this->removeHash($this->hashToken($this->cookie_token));
146	1	$this->makeLoginPersistent($logged_in_user);
147	1	}
148
149		/**
150		* Find a user with the given hash
151		*
152		* @param string $hash The hashed token
153		*
154		* @return \ElggUser\|null
155		*/
156	6	public function getUserFromHash($hash) {
157	6	if (!$hash) {
158		return null;
159		}
160
161	6	$hash = $this->db->sanitizeString($hash);
162	6	$query = "SELECT guid FROM {$this->table} WHERE code = '$hash'";
163		try {
164	6	$user_row = $this->db->getDataRow($query);
165		} catch (\DatabaseException $e) {
166		return $this->handleDbException($e);
167		}
168	6	if (!$user_row) {
169	3	return null;
170		}
171
172	3	$user = call_user_func($this->_callable_get_user, $user_row->guid);
173	3	return $user ? $user : null;
174		}
175
176		/**
177		* Store a hash in the DB
178		*
179		* @param \ElggUser $user The user for whom we're storing the hash
180		* @param string $hash The hashed token
181		*
182		* @return void
183		*/
184	3	protected function storeHash(\ElggUser $user, $hash) {
185		// This prevents inserting the same hash twice, which seems to be happening in some rare cases
186		// and for unknown reasons. See https://github.com/Elgg/Elgg/issues/8104
187	3	$this->removeHash($hash);
188
189	3	$time = time();
190	3	$hash = $this->db->sanitizeString($hash);
191
192		$query = "
193	3	INSERT INTO {$this->table} (code, guid, timestamp)
194	3	VALUES ('$hash', {$user->guid}, $time)
195		";
196		try {
197	3	$this->db->insertData($query);
198		} catch (\DatabaseException $e) {
199		$this->handleDbException($e);
200		}
201	3	}
202
203		/**
204		* Remove a hash from the DB
205		*
206		* @param string $hash The hashed token to remove (unused before 1.9)
207		* @return void
208		*/
209	4	protected function removeHash($hash) {
210	4	$hash = $this->db->sanitizeString($hash);
211
212	4	$query = "DELETE FROM {$this->table} WHERE code = '$hash'";
213		try {
214	4	$this->db->deleteData($query);
215		} catch (\DatabaseException $e) {
216		$this->handleDbException($e);
217		}
218	4	}
219
220		/**
221		* Swallow a schema not upgraded exception, otherwise rethrow it
222		*
223		* @param \DatabaseException $exception The exception to handle
224		* @param string $default The value to return if the table doesn't exist yet
225		*
226		* @return mixed
227		*
228		* @throws \DatabaseException
229		*/
230		protected function handleDbException(\DatabaseException $exception, $default = null) {
231		if (false !== strpos($exception->getMessage(), "users_remember_me_cookies' doesn't exist")) {
232		// schema has not been updated so we swallow this exception
233		return $default;
234		} else {
235		throw $exception;
236		}
237		}
238
239		/**
240		* Remove all the hashes associated with a user
241		*
242		* @param \ElggUser $user The user for whom we're removing hashes
243		*
244		* @return void
245		*/
246	3	protected function removeAllHashes(\ElggUser $user) {
247	3	$query = "DELETE FROM {$this->table} WHERE guid = '{$user->guid}'";
248		try {
249	3	$this->db->deleteData($query);
250		} catch (\DatabaseException $e) {
251		$this->handleDbException($e);
252		}
253	3	}
254
255		/**
256		* Create a hash from the token
257		*
258		* @param string $token The token to hash
259		*
260		* @return string
261		*/
262	7	protected function hashToken($token) {
263		// note: with user passwords, you'd want legit password hashing, but since these are randomly
264		// generated and long tokens, rainbow tables aren't any help.
265	7	return md5($token);
266		}
267
268		/**
269		* Store the token in the client cookie (or remove the cookie)
270		*
271		* @param string $token Empty string to remove cookie
272		*
273		* @return void
274		*/
275	7	protected function setCookie($token) {
276	7	$cookie = new \ElggCookie($this->cookie_config['name']);
277	7	foreach (['expire', 'path', 'domain', 'secure', 'httponly'] as $key) {
278	7	$cookie->$key = $this->cookie_config[$key];
279		}
280	7	$cookie->value = $token;
281	7	if (!$token) {
282	4	$cookie->expire = $this->time - (86400 * 30);
283		}
284	7	call_user_func($this->_callable_elgg_set_cookie, $cookie);
285	7	}
286
287		/**
288		* Store the token in the session (or remove it from the session)
289		*
290		* @param string $token The token to store in session. Empty string to remove.
291		*
292		* @return void
293		*/
294	6	protected function setSession($token) {
295	6	if ($token) {
296	4	$this->session->set('code', $token);
297		} else {
298	2	$this->session->remove('code');
299		}
300	6	}
301
302		/**
303		* Generate a random token (base 64 URL)
304		*
305		* The first char is always "z" to indicate the value has more entropy than the
306		* previously generated ones.
307		*
308		* @return string
309		*/
310	3	protected function generateToken() {
311	3	return 'z' . $this->crypto->getRandomString(31);
312		}
313
314		/**
315		* Is the given token a legacy MD5 hash?
316		*
317		* @param string $token The token to analyze
318		*
319		* @return bool
320		*/
321	3	protected function isLegacyToken($token) {
322	3	return (isset($token[0]) && $token[0] !== 'z');
323		}
324
325		/**
326		* @var Database
327		*/
328		protected $db;
329
330		/**
331		* @var string
332		*/
333		protected $table;
334
335		/**
336		* @var array
337		*/
338		protected $cookie_config;
339
340		/**
341		* @var string
342		*/
343		protected $cookie_token;
344
345		/**
346		* @var \ElggSession
347		*/
348		protected $session;
349
350		/**
351		* @var \ElggCrypto
352		*/
353		protected $crypto;
354
355		/**
356		* @var int
357		*/
358		protected $time;
359
360		/**
361		* DO NOT USE. For unit test mocking
362		* @access private
363		*/
364		public $_callable_get_user = 'get_user';
365
366		/**
367		* DO NOT USE. For unit test mocking
368		* @access private
369		*/
370		public $_callable_elgg_set_cookie = 'elgg_set_cookie';
371
372		/**
373		* DO NOT USE. For unit test mocking
374		* @access private
375		*/
376		public $_callable_sleep = 'sleep';
377		}
378
379

Elgg / Elgg

Push — master ( c0a3a7...3b84a4 )

engine/classes/Elgg/PersistentLoginService.php (1 issue)

Showing only issues like (Show All)

Introduced By

Duplication Side-by-Side

Filter issues like