Separate the update API password endpoint (#2675)

* Extract hashPassword method from userController

* Extract and refactor fever key-related methods

* Move update of API password to dedicated action

* Simplify the controller by refactoring feverUtil

* Add locales

3 files changed, 55 insertions, 55 deletions

diff --git a/app/Controllers/apiController.php b/app/Controllers/apiController.php
new file mode 100644
index 000000000..d096ba83f
--- /dev/null
+++ b/app/Controllers/apiController.php
@@ -0,0 +1,47 @@
+<?php
+
+/**
+ * This controller manage API-related features.
+ */
+class FreshRSS_api_Controller extends Minz_ActionController {
+	/**
+	 * This action updates the user API password.
+	 *
+	 * Parameter is:
+	 * - apiPasswordPlain: the new user password
+	 */
+	public function updatePasswordAction() {
+		if (!FreshRSS_Auth::hasAccess()) {
+			Minz_Error::error(403);
+		}
+
+		$return_url = array('c' => 'user', 'a' => 'profile');
+
+		if (!Minz_Request::isPost()) {
+			Minz_Request::forward($return_url, true);
+		}
+
+		$apiPasswordPlain = Minz_Request::param('apiPasswordPlain', '', true);
+		if ($apiPasswordPlain == '') {
+			Minz_Request::forward($return_url, true);
+		}
+
+		$username = Minz_Session::param('currentUser');
+		$userConfig = FreshRSS_Context::$user_conf;
+
+		$apiPasswordHash = FreshRSS_password_Util::hash($apiPasswordPlain);
+		$userConfig->apiPasswordHash = $apiPasswordHash;
+
+		$feverKey = FreshRSS_fever_Util::updateKey($username, $apiPasswordPlain);
+		if (!$feverKey) {
+			Minz_Request::bad(_t('feedback.api.password.failed'), $return_url);
+		}
+
+		$userConfig->feverKey = $feverKey;
+		if ($userConfig->save()) {
+			Minz_Request::good(_t('feedback.api.password.updated'), $return_url);
+		} else {
+			Minz_Request::bad(_t('feedback.api.password.failed'), $return_url);
+		}
+	}
+}
diff --git a/app/Controllers/javascriptController.php b/app/Controllers/javascriptController.php
index c84e5483b..b22e2c127 100755
--- a/app/Controllers/javascriptController.php
+++ b/app/Controllers/javascriptController.php
@@ -47,7 +47,7 @@ class FreshRSS_javascript_Controller extends Minz_ActionController {
 			Minz_Log::notice('Nonce failure due to invalid username!');
 		}
 		//Failure: Return random data.
-		$this->view->salt1 = sprintf('$2a$%02d$', FreshRSS_user_Controller::BCRYPT_COST);
+		$this->view->salt1 = sprintf('$2a$%02d$', FreshRSS_password_Util::BCRYPT_COST);
 		$alphabet = './ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
 		for ($i = 22; $i > 0; $i--) {
 			$this->view->salt1 .= $alphabet[mt_rand(0, 63)];
diff --git a/app/Controllers/userController.php b/app/Controllers/userController.php
index 05e7475e7..3a48e65e3 100644
--- a/app/Controllers/userController.php
+++ b/app/Controllers/userController.php
@@ -4,17 +4,6 @@
  * Controller to handle user actions.
  */
 class FreshRSS_user_Controller extends Minz_ActionController {
-	// Will also have to be computed client side on mobile devices,
-	// so do not use a too high cost
-	const BCRYPT_COST = 9;
-
-	public static function hashPassword($passwordPlain) {
-		$passwordHash = password_hash($passwordPlain, PASSWORD_BCRYPT, array('cost' => self::BCRYPT_COST));
-		$passwordPlain = '';
-		$passwordHash = preg_replace('/^\$2[xy]\$/', '\$2a\$', $passwordHash);	//Compatibility with bcrypt.js
-		return $passwordHash == '' ? '' : $passwordHash;
-	}
-
 	/**
 	 * The username is also used as folder name, file name, and part of SQL table name.
 	 * '_' is a reserved internal username.
@@ -25,15 +14,7 @@ class FreshRSS_user_Controller extends Minz_ActionController {
 		return preg_match('/^' . self::USERNAME_PATTERN . '$/', $username) === 1;
 	}
 
-	public static function deleteFeverKey($username) {
-		$userConfig = get_user_configuration($username);
-		if ($userConfig !== null && ctype_xdigit($userConfig->feverKey)) {
-			return @unlink(DATA_PATH . '/fever/.key-' . sha1(FreshRSS_Context::$system_conf->salt) . '-' . $userConfig->feverKey . '.txt');
-		}
-		return false;
-	}
-
-	public static function updateUser($user, $email, $passwordPlain, $apiPasswordPlain, $userConfigUpdated = array()) {
+	public static function updateUser($user, $email, $passwordPlain, $userConfigUpdated = array()) {
 		$userConfig = get_user_configuration($user);
 		if ($userConfig === null) {
 			return false;
@@ -51,33 +32,10 @@ class FreshRSS_user_Controller extends Minz_ActionController {
 		}
 
 		if ($passwordPlain != '') {
-			$passwordHash = self::hashPassword($passwordPlain);
+			$passwordHash = FreshRSS_password_Util::hash($passwordPlain);
 			$userConfig->passwordHash = $passwordHash;
 		}
 
-		if ($apiPasswordPlain != '') {
-			$apiPasswordHash = self::hashPassword($apiPasswordPlain);
-			$userConfig->apiPasswordHash = $apiPasswordHash;
-
-			$feverPath = DATA_PATH . '/fever/';
-
-			if (!file_exists($feverPath)) {
-				@mkdir($feverPath, 0770, true);
-			}
-
-			if (!is_writable($feverPath)) {
-				Minz_Log::error("Could not save Fever API credentials. The directory does not have write access.");
-			} else {
-				self::deleteFeverKey($user);
-				$userConfig->feverKey = strtolower(md5("{$user}:{$apiPasswordPlain}"));
-				$ok = file_put_contents($feverPath . '.key-' . sha1(FreshRSS_Context::$system_conf->salt) . '-' . $userConfig->feverKey . '.txt', $user) !== false;
-
-				if (!$ok) {
-					Minz_Log::warning('Could not save Fever API credentials. Unknown error.', ADMIN_LOG);
-				}
-			}
-		}
-
 		if (is_array($userConfigUpdated)) {
 			foreach ($userConfigUpdated as $configName => $configValue) {
 				if ($configValue !== null) {
@@ -100,10 +58,8 @@ class FreshRSS_user_Controller extends Minz_ActionController {
 			Minz_Request::_param('newPasswordPlain');	//Discard plain-text password ASAP
 			$_POST['newPasswordPlain'] = '';
 
-			$apiPasswordPlain = Minz_Request::param('apiPasswordPlain', '', true);
-
 			$username = Minz_Request::param('username');
-			$ok = self::updateUser($username, null, $passwordPlain, $apiPasswordPlain, array(
+			$ok = self::updateUser($username, null, $passwordPlain, array(
 				'token' => Minz_Request::param('token', null),
 			));
 
@@ -150,8 +106,6 @@ class FreshRSS_user_Controller extends Minz_ActionController {
 			Minz_Request::_param('newPasswordPlain');	//Discard plain-text password ASAP
 			$_POST['newPasswordPlain'] = '';
 
-			$apiPasswordPlain = Minz_Request::param('apiPasswordPlain', '', true);
-
 			if ($system_conf->force_email_validation && empty($email)) {
 				Minz_Request::bad(
 					_t('user.email.feedback.required'),
@@ -170,7 +124,6 @@ class FreshRSS_user_Controller extends Minz_ActionController {
 				Minz_Session::param('currentUser'),
 				$email,
 				$passwordPlain,
-				$apiPasswordPlain,
 				array(
 					'token' => Minz_Request::param('token', null),
 				)
@@ -239,7 +192,7 @@ class FreshRSS_user_Controller extends Minz_ActionController {
 		}
 	}
 
-	public static function createUser($new_user_name, $email, $passwordPlain, $apiPasswordPlain = '', $userConfigOverride = [], $insertDefaultFeeds = true) {
+	public static function createUser($new_user_name, $email, $passwordPlain, $userConfigOverride = [], $insertDefaultFeeds = true) {
 		$userConfig = [];
 
 		$customUserConfigPath = join_path(DATA_PATH, 'config-user.custom.php');
@@ -291,7 +244,7 @@ class FreshRSS_user_Controller extends Minz_ActionController {
 				}
 			}
 
-			$ok &= self::updateUser($new_user_name, $email, $passwordPlain, $apiPasswordPlain);
+			$ok &= self::updateUser($new_user_name, $email, $passwordPlain);
 		}
 		return $ok;
 	}
@@ -346,7 +299,7 @@ class FreshRSS_user_Controller extends Minz_ActionController {
 				);
 			}
 
-			$ok = self::createUser($new_user_name, $email, $passwordPlain, '', array('language' => $new_user_language));
+			$ok = self::createUser($new_user_name, $email, $passwordPlain, array('language' => $new_user_language));
 			Minz_Request::_param('new_user_passwordPlain');	//Discard plain-text password ASAP
 			$_POST['new_user_passwordPlain'] = '';
 			invalidateHttpCache();
@@ -386,7 +339,7 @@ class FreshRSS_user_Controller extends Minz_ActionController {
 		$user_data = join_path(DATA_PATH, 'users', $username);
 		$ok &= is_dir($user_data);
 		if ($ok) {
-			self::deleteFeverKey($username);
+			FreshRSS_fever_Util::deleteKey($username);
 			$oldUserDAO = FreshRSS_Factory::createUserDao($username);
 			$ok &= $oldUserDAO->deleteUser();
 			$ok &= recursive_unlink($user_data);