Issues in PasswordService.php - New Issues - Inspection of "Merge pull request #11577 from jdalsem/delete-admi..." - Elgg/Elgg - Measure and Improve Code Quality continuously with Scrutinizer

Passed

Push — master ( c2d8e3...289151 )

by Jeroen

created 2018-01-09 19:42 UTC

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

Labels

Severity

Major 1

<?php
namespace Elgg;

/**
 * PRIVATE CLASS. API IN FLUX. DO NOT USE DIRECTLY.
 *
 * @package Elgg.Core
 * @access  private
 * @since   1.10.0
 */
final class PasswordService {

	/**
	 * Constructor
	 */
	public function __construct() {
		if (!function_exists('password_hash')) {
			throw new \RuntimeException("password_hash and associated functions are required.");
		}
	}

	/**
	 * Determine if the password hash needs to be rehashed
	 *
	 * If the answer is true, after validating the password using password_verify, rehash it.
	 *
	 * @param string $hash The hash to test
	 *
	 * @return boolean True if the password needs to be rehashed.
	 */
	function needsRehash($hash) {
		return password_needs_rehash($hash, PASSWORD_DEFAULT);
	}

	/**
	 * Verify a password against a hash using a timing attack resistant approach
	 *
	 * @param string $password The password to verify
	 * @param string $hash     The hash to verify against
	 *
	 * @return boolean If the password matches the hash
	 */
	function verify($password, $hash) {
		return password_verify($password, $hash);
	}

	/**
	 * Hash a password for storage using password_hash()
	 *
	 * @param string $password Password in clear text
	 *
	 * @return string
	 */
	function generateHash($password) {
		return password_hash($password, PASSWORD_DEFAULT);
	}

	/**
	 * Generate and send a password request email to a given user's registered email address.
	 *
	 * @param int $user_guid User GUID
	 *
	 * @return bool
	 */
	function sendNewPasswordRequest($user_guid) {
		$user_guid = (int) $user_guid;

		$user = _elgg_services()->entityTable->get($user_guid);
		if (!$user instanceof \ElggUser) {
			return false;
		}

		// generate code
		$code = generate_random_cleartext_password();
		$user->setPrivateSetting('passwd_conf_code', $code);
		$user->setPrivateSetting('passwd_conf_time', time());

		// generate link
		$link = elgg_generate_url('account:password:change', [
			'u' => $user_guid,
			'c' => $code,
		]);
		$link = _elgg_services()->urlSigner->sign($link, '+1 day');

		// generate email
		$ip_address = _elgg_services()->request->getClientIp();
		$message = _elgg_services()->translator->translate(
			'email:changereq:body', [$user->name, $ip_address, $link], $user->language);
		$subject = _elgg_services()->translator->translate(
			'email:changereq:subject', [], $user->language);

		$params = [
			'action' => 'requestnewpassword',
			'object' => $user,
			'ip_address' => $ip_address,
			'link' => $link,
		];
		
		return notify_user($user->guid, elgg_get_site_entity()->guid, $subject, $message, $params, 'email');
	}

	/**
	 * Set a user's new password and save the entity.
	 *
	 * This can only be called from execute_new_password_request().
	 *
	 * @param \ElggUser|int $user     The user GUID or entity
	 * @param string        $password Text (which will then be converted into a hash and stored)
	 *
	 * @return bool
	 */
	function forcePasswordReset($user, $password) {
		if (!$user instanceof \ElggUser) {
			$user = _elgg_services()->entityTable->get($user, 'user');
			if (!$user) {
				return false;
			}
		}

		$user->setPassword($password);

		$ia = elgg_set_ignore_access(true);
		$result = (bool) $user->save();
		elgg_set_ignore_access($ia);

		return $result;
	}

	/**
	 * Validate and change password for a user.
	 *
	 * @param int    $user_guid The user id
	 * @param string $conf_code Confirmation code as sent in the request email.
	 * @param string $password  Optional new password, if not randomly generated.
	 *
	 * @return bool True on success
	 */
	function executeNewPasswordReset($user_guid, $conf_code, $password = null) {
		$user_guid = (int) $user_guid;
		$user = get_entity($user_guid);

		if ($password === null) {
			$password = generate_random_cleartext_password();
			$reset = true;
		} else {
			$reset = false;
		}

		if (!$user instanceof \ElggUser) {
			return false;
		}

		$saved_code = $user->getPrivateSetting('passwd_conf_code');
		$code_time = (int) $user->getPrivateSetting('passwd_conf_time');
		$codes_match = _elgg_services()->crypto->areEqual($saved_code, $conf_code);

		if (!$saved_code || !$codes_match) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
			return false;
		}

		// Discard for security if it is 24h old
		if (!$code_time || $code_time < time() - 24 * 60 * 60) {
			return false;
		}

		if (!$this->forcePasswordReset($user, $password)) {
			return false;
		}

		remove_private_setting($user_guid, 'passwd_conf_code');
		remove_private_setting($user_guid, 'passwd_conf_time');
		// clean the logins failures
		reset_login_failure_count($user_guid);

		$ns = $reset ? 'resetpassword' : 'changepassword';

		$message = _elgg_services()->translator->translate(
			"email:$ns:body", [$user->username, $password], $user->language);
		$subject = _elgg_services()->translator->translate("email:$ns:subject", [], $user->language);

		$params = [
			'action' => $ns,
			'object' => $user,
			'password' => $password,
		];

		notify_user($user->guid, elgg_get_site_entity()->guid, $subject, $message, $params, 'email');

		return true;
	}
}


1		<?php
2		namespace Elgg;
3
4		/**
5		* PRIVATE CLASS. API IN FLUX. DO NOT USE DIRECTLY.
6		*
7		* @package Elgg.Core
8		* @access private
9		* @since 1.10.0
10		*/
11		final class PasswordService {
12
13		/**
14		* Constructor
15		*/
16	12	public function __construct() {
17	12	if (!function_exists('password_hash')) {
18		throw new \RuntimeException("password_hash and associated functions are required.");
19		}
20	12	}
21
22		/**
23		* Determine if the password hash needs to be rehashed
24		*
25		* If the answer is true, after validating the password using password_verify, rehash it.
26		*
27		* @param string $hash The hash to test
28		*
29		* @return boolean True if the password needs to be rehashed.
30		*/
31	5	function needsRehash($hash) {
32	5	return password_needs_rehash($hash, PASSWORD_DEFAULT);
33		}
34
35		/**
36		* Verify a password against a hash using a timing attack resistant approach
37		*
38		* @param string $password The password to verify
39		* @param string $hash The hash to verify against
40		*
41		* @return boolean If the password matches the hash
42		*/
43	6	function verify($password, $hash) {
44	6	return password_verify($password, $hash);
45		}
46
47		/**
48		* Hash a password for storage using password_hash()
49		*
50		* @param string $password Password in clear text
51		*
52		* @return string
53		*/
54	162	function generateHash($password) {
55	162	return password_hash($password, PASSWORD_DEFAULT);
56		}
57
58		/**
59		* Generate and send a password request email to a given user's registered email address.
60		*
61		* @param int $user_guid User GUID
62		*
63		* @return bool
64		*/
65		function sendNewPasswordRequest($user_guid) {
66		$user_guid = (int) $user_guid;
67
68		$user = _elgg_services()->entityTable->get($user_guid);
69		if (!$user instanceof \ElggUser) {
70		return false;
71		}
72
73		// generate code
74		$code = generate_random_cleartext_password();
75		$user->setPrivateSetting('passwd_conf_code', $code);
76		$user->setPrivateSetting('passwd_conf_time', time());
77
78		// generate link
79		$link = elgg_generate_url('account:password:change', [
80		'u' => $user_guid,
81		'c' => $code,
82		]);
83		$link = _elgg_services()->urlSigner->sign($link, '+1 day');
84
85		// generate email
86		$ip_address = _elgg_services()->request->getClientIp();
87		$message = _elgg_services()->translator->translate(
88		'email:changereq:body', [$user->name, $ip_address, $link], $user->language);
89		$subject = _elgg_services()->translator->translate(
90		'email:changereq:subject', [], $user->language);
91
92		$params = [
93		'action' => 'requestnewpassword',
94		'object' => $user,
95		'ip_address' => $ip_address,
96		'link' => $link,
97		];
98
99		return notify_user($user->guid, elgg_get_site_entity()->guid, $subject, $message, $params, 'email');
100		}
101
102		/**
103		* Set a user's new password and save the entity.
104		*
105		* This can only be called from execute_new_password_request().
106		*
107		* @param \ElggUser\|int $user The user GUID or entity
108		* @param string $password Text (which will then be converted into a hash and stored)
109		*
110		* @return bool
111		*/
112		function forcePasswordReset($user, $password) {
113		if (!$user instanceof \ElggUser) {
114		$user = _elgg_services()->entityTable->get($user, 'user');
115		if (!$user) {
116		return false;
117		}
118		}
119
120		$user->setPassword($password);
121
122		$ia = elgg_set_ignore_access(true);
123		$result = (bool) $user->save();
124		elgg_set_ignore_access($ia);
125
126		return $result;
127		}
128
129		/**
130		* Validate and change password for a user.
131		*
132		* @param int $user_guid The user id
133		* @param string $conf_code Confirmation code as sent in the request email.
134		* @param string $password Optional new password, if not randomly generated.
135		*
136		* @return bool True on success
137		*/
138		function executeNewPasswordReset($user_guid, $conf_code, $password = null) {
139		$user_guid = (int) $user_guid;
140		$user = get_entity($user_guid);
141
142		if ($password === null) {
143		$password = generate_random_cleartext_password();
144		$reset = true;
145		} else {
146		$reset = false;
147		}
148
149		if (!$user instanceof \ElggUser) {
150		return false;
151		}
152
153		$saved_code = $user->getPrivateSetting('passwd_conf_code');
154		$code_time = (int) $user->getPrivateSetting('passwd_conf_time');
155		$codes_match = _elgg_services()->crypto->areEqual($saved_code, $conf_code);
156
157		if (!$saved_code \|\| !$codes_match) {
		0 ignored issues – show Bug Best Practice introduced 2017-12-23 09:23 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$saved_code` of type `null\|string` is loosely compared to `false`; this is ambiguous if the string can be empty. You might want to explicitly use `=== null` 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...
158		return false;
159		}
160
161		// Discard for security if it is 24h old
162		if (!$code_time \|\| $code_time < time() - 24 * 60 * 60) {
163		return false;
164		}
165
166		if (!$this->forcePasswordReset($user, $password)) {
167		return false;
168		}
169
170		remove_private_setting($user_guid, 'passwd_conf_code');
171		remove_private_setting($user_guid, 'passwd_conf_time');
172		// clean the logins failures
173		reset_login_failure_count($user_guid);
174
175		$ns = $reset ? 'resetpassword' : 'changepassword';
176
177		$message = _elgg_services()->translator->translate(
178		"email:$ns:body", [$user->username, $password], $user->language);
179		$subject = _elgg_services()->translator->translate("email:$ns:subject", [], $user->language);
180
181		$params = [
182		'action' => $ns,
183		'object' => $user,
184		'password' => $password,
185		];
186
187		notify_user($user->guid, elgg_get_site_entity()->guid, $subject, $message, $params, 'email');
188
189		return true;
190		}
191		}
192

Elgg / Elgg

Push — master ( c2d8e3...289151 )

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

Labels

Severity

Introduced By

Duplication Side-by-Side

Filter issues like