From 350f6deadd0d6efcecb17b1324865f14c38718a1 Mon Sep 17 00:00:00 2001 From: Manoj L Date: Thu, 10 Jan 2019 14:09:39 +0530 Subject: [PATCH 1/5] Task #139177 fix: Use backend model to get / generate key/hash --- src/users/login.php | 152 +++++++++++++++++++++++++++++--------------- 1 file changed, 102 insertions(+), 50 deletions(-) diff --git a/src/users/login.php b/src/users/login.php index bb18d57..b5353c5 100644 --- a/src/users/login.php +++ b/src/users/login.php @@ -1,12 +1,15 @@ - * @link http://www.techjoomla.com + * @package API + * @subpackage plg_api_users + * + * @author Techjoomla + * @copyright Copyright (C) 2009 - 2019 Techjoomla, Tekdi Technologies Pvt. Ltd. All rights reserved. + * @license GNU GPLv2 */ -defined('_JEXEC') or die( 'Restricted access' ); +// No direct access. +defined('_JEXEC') or die('Restricted access'); require_once JPATH_SITE . '/components/com_api/vendors/php-jwt/src/JWT.php'; @@ -23,84 +26,125 @@ JModelLegacy::addIncludePath(JPATH_SITE . 'components/com_api/models'); require_once JPATH_SITE . '/components/com_api/libraries/authentication/user.php'; require_once JPATH_SITE . '/components/com_api/libraries/authentication/login.php'; -require_once JPATH_SITE . '/components/com_api/models/key.php'; -require_once JPATH_SITE . '/components/com_api/models/keys.php'; +require_once JPATH_ADMINISTRATOR . '/components/com_api/models/key.php'; +require_once JPATH_ADMINISTRATOR . '/components/com_api/models/keys.php'; +/** + * Login API resource class + * + * @package API + * @since 1.6.0 + */ class UsersApiResourceLogin extends ApiResource { + /** + * Get method + * + * @return object + */ public function get() { - $this->plugin->setResponse( JText::_('PLG_API_USERS_GET_METHOD_NOT_ALLOWED_MESSAGE')); + $this->plugin->setResponse(JText::_('PLG_API_USERS_GET_METHOD_NOT_ALLOWED_MESSAGE')); } + /** + * Post method + * + * @return object + */ public function post() { $this->plugin->setResponse($this->keygen()); } + /** + * Generate key method + * + * @return object + */ public function keygen() { - //init variable - $obj = new stdclass; + // Init variable + $obj = new stdclass; $umodel = new JUser; - $user = $umodel->getInstance(); + $user = $umodel->getInstance(); - $app = JFactory::getApplication(); + $app = JFactory::getApplication(); $username = $app->input->get('username', 0, 'STRING'); $user = JFactory::getUser(); - $id = JUserHelper::getUserId($username); + $id = JUserHelper::getUserId($username); - if($id == null) + if ($id == null) { $model = FD::model('Users'); - $id = $model->getUserId('email', $username); + $id = $model->getUserId('email', $username); } $kmodel = new ApiModelKey; - $model = new ApiModelKeys; - $key = null; + $model = new ApiModelKeys; + $key = null; + // Get login user hash - //$kmodel->setState('user_id', $user->id); - $kmodel->setState('user_id', $id); - $log_hash = $kmodel->getList(); - $log_hash = (!empty($log_hash))?$log_hash[count($log_hash) - count($log_hash)]:$log_hash; + // $kmodel->setState('user_id', $user->id); + + // $kmodel->setState('user_id', $id); + // $log_hash = $kmodel->getList(); + $model->setState('user_id', $id); + $log_hash = $model->getItems(); + + $log_hash = (!empty($log_hash)) ? $log_hash[count($log_hash) - count($log_hash)] : $log_hash; - if( !empty($log_hash) ) + if (!empty($log_hash)) { $key = $log_hash->hash; } - elseif( $key == null || empty($key) ) + elseif ($key == null || empty($key)) { - // Create new key for user - $data = array( + // Create new key for user + $data = array ( 'userid' => $user->id, 'domain' => '' , - 'state' => 1, - 'id' => '', - 'task' => 'save', - 'c' => 'key', - 'ret' => 'index.php?option=com_api&view=keys', + 'state' => 1, + 'id' => '', + 'task' => 'save', + 'c' => 'key', + 'ret' => 'index.php?option=com_api&view=keys', 'option' => 'com_api', JSession::getFormToken() => 1 - ); + ); - $result = $kmodel->save($data); - $key = $result->hash; + $result = $kmodel->save($data); - //add new key in easysocial table - $easyblog = JPATH_ROOT . '/administrator/components/com_easyblog/easyblog.php'; - if (JFile::exists($easyblog) && JComponentHelper::isEnabled('com_easysocial', true)) - { - $this->updateEauth( $user , $key ); - } + // $key = $result->hash; + + if (!$result) + { + return false; + } + + // Load api key table + JTable::addIncludePath(JPATH_ROOT . '/administrator/components/com_api/tables'); + $table = JTable::getInstance('Key', 'ApiTable'); + $table->load(array('userid' => $user->id)); + $key = $table->hash; + + // Add new key in easysocial table + $easyblog = JPATH_ROOT . '/administrator/components/com_easyblog/easyblog.php'; + + if (JFile::exists($easyblog) && JComponentHelper::isEnabled('com_easysocial', true)) + { + $this->updateEauth($user, $key); + } } - if( !empty($key) ) + if (!empty($key)) { $obj->auth = $key; $obj->code = '200'; - //$obj->id = $user->id; + + // $obj->id = $user->id; + $obj->id = $id; // Generate claim for jwt @@ -129,21 +173,29 @@ public function keygen() $obj->code = 403; $obj->message = JText::_('PLG_API_USERS_BAD_REQUEST_MESSAGE'); } - return( $obj ); + return ($obj); } - /* - * function to update Easyblog auth keys + /** + * Method to update Easyblog auth keys + * + * @param mixed $user User object + * @param mixed $key Key + * + * @return integer + * + * @since 1.6 */ - public function updateEauth($user=null,$key=null) + public function updateEauth ($user = null, $key = null) { - require_once JPATH_ADMINISTRATOR.'/components/com_easysocial/includes/foundry.php'; - $model = FD::model('Users'); - $id = $model->getUserId('username', $user->username); - $user = FD::user($id); + require_once JPATH_ADMINISTRATOR . '/components/com_easysocial/includes/foundry.php'; + + $model = FD::model('Users'); + $id = $model->getUserId('username', $user->username); + $user = FD::user($id); $user->alias = $user->username; - $user->auth = $key; + $user->auth = $key; $user->store(); return $id; From 0a0a4b8f37902a5a2e35843c55949a41aaeae230 Mon Sep 17 00:00:00 2001 From: Manoj L Date: Thu, 10 Jan 2019 14:22:03 +0530 Subject: [PATCH 2/5] Task #139177 fix: Scrutinizer issues --- src/users/login.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/users/login.php b/src/users/login.php index b5353c5..ba2b5b8 100644 --- a/src/users/login.php +++ b/src/users/login.php @@ -60,7 +60,7 @@ public function post() /** * Generate key method * - * @return object + * @return object|boolean */ public function keygen() { From de887b4f54ed57bb6084d79d08a8765374feb049 Mon Sep 17 00:00:00 2001 From: Manoj L Date: Fri, 19 Apr 2019 17:34:36 +0530 Subject: [PATCH 3/5] Issue #15 fix: User update (patch) is not working for resource `com_api&app=users&resource=user` --- src/language/en-GB/en-GB.plg_api_users.ini | 5 +- src/users/user.php | 86 ++++++++++++---------- 2 files changed, 52 insertions(+), 39 deletions(-) diff --git a/src/language/en-GB/en-GB.plg_api_users.ini b/src/language/en-GB/en-GB.plg_api_users.ini index 1e75e9c..2a994f4 100755 --- a/src/language/en-GB/en-GB.plg_api_users.ini +++ b/src/language/en-GB/en-GB.plg_api_users.ini @@ -2,7 +2,7 @@ PLG_API_USERS="API - Users" PLG_API_USERS_DESCRIPTION="This plugin exposes users to the Joomla! API. Supports creation, listing and login for users." PLG_API_USERS_BAD_REQUEST_MESSAGE="Bad request" PLG_API_USERS_REQUIRED_DATA_EMPTY_MESSAGE="Required data is empty" -PLG_API_USERS_ACCOUNT_CREATED_SUCCESSFULLY_MESSAGE="Congratulations! Your account has been created successfully" +PLG_API_USERS_ACCOUNT_CREATED_SUCCESSFULLY_MESSAGE="Congratulations! Account has been created successfully." PLG_API_USERS_PROFILE_CREATED_SUCCESSFULLY_MESSAGE="profile created successfully" PLG_API_USERS_UNABLE_CREATE_PROFILE_MESSAGE="Unable to create profile" PLG_API_USERS_EASYSOCIAL_NOT_INSTALL_MESSAGE="Easysocial is not installed properly" @@ -15,3 +15,6 @@ PLG_API_USERS_UNSUPPORTED_METHOD_POST="unsupported method,please use get method" PLG_API_USERS_USERS="users/" PLG_API_USERS_IN_DELETE="in delete" PLG_API_USERS_IN_POST="in post" + +; Since v2.0.1 +PLG_API_USERS_ACCOUNT_UPDATED_SUCCESSFULLY_MESSAGE="Account details updated successfully" diff --git a/src/users/user.php b/src/users/user.php index d0795e9..fa56013 100644 --- a/src/users/user.php +++ b/src/users/user.php @@ -28,29 +28,22 @@ class UsersApiResourceUser extends ApiResource */ public function post() { - $app = JFactory::getApplication(); - $userIdentifier = $app->input->get('id', 0, 'String'); - $formData = $app->input->getArray(); - $params = JComponentHelper::getParams("com_users"); - $response = new stdClass; - - $xidentifier = $app->input->server->get('HTTP_IDENTIFIER'); - $fidentifier = $app->input->server->get('HTTP_FORCECREATE'); - - if ($formData['username'] == '' || $formData['name'] == '' || $formData['email'] == '') - { - ApiError::raiseError(400, JText::_('PLG_API_USERS_REQUIRED_DATA_EMPTY_MESSAGE')); - - return; - } + $app = JFactory::getApplication(); + $params = JComponentHelper::getParams("com_users"); + $formData = $app->input->getArray(); + $userIdentifier = $app->input->get('id', 0, 'string'); + $xIdentifier = $app->input->server->get('HTTP_X_IDENTIFIER'); + $fIdentifier = $app->input->server->get('HTTP_FORCECREATE'); // Get current logged in user. - $my = JFactory::getUser(); + $me = JFactory::getUser(); + + // Check if $userIdentifier is not set - POST / CREATE user case - // Check if $userIdentifier is not set if (empty($userIdentifier)) { - if ($formData['password'] == '') + // Validate required fields + if ($formData['username'] == '' || $formData['name'] == '' || $formData['email'] == '' || $formData['password'] == '') { ApiError::raiseError(400, JText::_('PLG_API_USERS_REQUIRED_DATA_EMPTY_MESSAGE')); @@ -72,33 +65,33 @@ public function post() return; } + // PATCH / EDIT user case else { - // Get a user object - $user = $this->retriveUser($xidentifier, $userIdentifier); - $passedUserGroups = array(); + // Get a user object from xIdentifier + $user = $this->retriveUser($xIdentifier, $userIdentifier); // If user is already present then update it according to access. if (!empty($user->id)) { - $iAmSuperAdmin = $my->authorise('core.admin'); + $iAmSuperAdmin = $me->authorise('core.admin'); - // Check if regular user is tring to update himself. - if ($my->id == $user->id || $iAmSuperAdmin) + // Check if regular user is trying to update his/her own profile OR if user is superadmin + if ($me->id == $user->id || $iAmSuperAdmin) { - // If present then update or else dont include. + // If password present then update password2 or else dont include. if (!empty($formData['password'])) { $formData['password2'] = $formData['password']; } - // Add newly added groups and keep the old one as it is. + /*// Add newly added groups and keep the old one as it is. if (!empty($formData['groups'])) { - $passedUserGroups['groups'] = array_unique(array_merge($user->groups, $formData['groups'])); - } + $formData['groups'] = array_unique(array_merge($user->groups, $formData['groups'])); + }*/ - $response = $this->storeUser($user, $passedUserGroups); + $response = $this->storeUser($user, $formData); $this->plugin->setResponse($response); return; @@ -112,11 +105,12 @@ public function post() } else { - if ($fidentifier) + // Forced user creation + if ($fIdentifier) { $user = new JUser; - if ($formData['password'] == '') + if ($formData['username'] == '' || $formData['name'] == '' || $formData['email'] == '' || $formData['password'] == '') { ApiError::raiseError(400, JText::_('PLG_API_USERS_REQUIRED_DATA_EMPTY_MESSAGE')); @@ -135,9 +129,10 @@ public function post() return; } + // User trying to be updated not found else { - ApiError::raiseError(400, JText::_('PLG_API_USERS_USER_ABSENT_MESSAGE')); + ApiError::raiseError(400, JText::_('PLG_API_USERS_USER_NOT_FOUND_MESSAGE')); return; } @@ -222,8 +217,22 @@ private function getUserId($email) private function storeUser($user, $formData, $isNew = 0) { $response = new stdClass; + $ignore = array(); + + // Ignore pasword field if not set to avoid warning on bind() + if (!isset($formData['password'])) + { + $ignore[] = 'password'; + } + + // In case of edit user, set formData->id as $user->id no matter what is passed in x-identifier + // Otherwise - it will try to create new user + if (!$isNew) + { + $formData['id'] = $user->id; + } - if (!$user->bind($formData)) + if (!$user->bind($formData, $ignore)) { ApiError::raiseError(400, $user->getError()); @@ -237,6 +246,7 @@ private function storeUser($user, $formData, $isNew = 0) return; } + // Set user id to be returned $response->id = $user->id; if ($isNew) @@ -262,14 +272,14 @@ public function delete() { $app = JFactory::getApplication(); $userIdentifier = $app->input->get('id', 0, 'STRING'); - $xidentifier = $app->input->server->get('HTTP_IDENTIFIER'); + $xIdentifier = $app->input->server->get('HTTP_X_IDENTIFIER'); $loggedUser = JFactory::getUser(); // Check if I am a Super Admin $iAmSuperAdmin = $loggedUser->authorise('core.admin'); - $userToDelete = $this->retriveUser($xidentifier, $userIdentifier); + $userToDelete = $this->retriveUser($xIdentifier, $userIdentifier); if (!$userToDelete->id) { @@ -317,7 +327,7 @@ public function delete() /** * Function retriveUser for get user details depending upon the identifier. * - * @param string $xidentifier Flag to differentiate the column value. + * @param string $xIdentifier Flag to differentiate the column value. * * @param string $userIdentifier username * @@ -325,11 +335,11 @@ public function delete() * * @since 2.0 */ - private function retriveUser($xidentifier, $userIdentifier) + private function retriveUser($xIdentifier, $userIdentifier) { $user = new stdClass; - switch ($xidentifier) + switch ($xIdentifier) { case 'username': $userId = JUserHelper::getUserId($userIdentifier); From 34404eff0908bbd8a7fd1ba8d524c8c4d3453ee4 Mon Sep 17 00:00:00 2001 From: Manoj L Date: Fri, 19 Apr 2019 17:46:21 +0530 Subject: [PATCH 4/5] Issue #15 fix: Minor changes --- src/users/user.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/users/user.php b/src/users/user.php index e15d14a..9fa6a41 100644 --- a/src/users/user.php +++ b/src/users/user.php @@ -151,7 +151,7 @@ public function get() { $input = JFactory::getApplication()->input; $id = $input->get('id', 0, 'int'); - $xIdentifier = $input->server->get('HTTP_X_IDENTIFIER', '', 'String'); + $xIdentifier = $input->server->get('HTTP_X_IDENTIFIER', '', 'string'); /* * If we have an id try to fetch the user @@ -273,8 +273,8 @@ private function storeUser($user, $formData, $isNew = 0) public function delete() { $app = JFactory::getApplication(); - $userIdentifier = $app->input->get('id', 0, 'STRING'); - $xIdentifier = $app->input->server->get('HTTP_X_IDENTIFIER'); + $userIdentifier = $app->input->get('id', 0, 'string'); + $xIdentifier = $app->input->server->get('HTTP_X_IDENTIFIER', '', 'string'); $loggedUser = JFactory::getUser(); From 91d6603b1c5992128fe1bf1ec6e022edcb915ccc Mon Sep 17 00:00:00 2001 From: Manoj L Date: Fri, 19 Apr 2019 18:22:41 +0530 Subject: [PATCH 5/5] Issue #5 fix: user.php returning user's password --- src/language/en-GB/en-GB.plg_api_users.ini | 1 + src/users/user.php | 39 +++++++++++++++++----- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/language/en-GB/en-GB.plg_api_users.ini b/src/language/en-GB/en-GB.plg_api_users.ini index 2a994f4..f066eb6 100755 --- a/src/language/en-GB/en-GB.plg_api_users.ini +++ b/src/language/en-GB/en-GB.plg_api_users.ini @@ -18,3 +18,4 @@ PLG_API_USERS_IN_POST="in post" ; Since v2.0.1 PLG_API_USERS_ACCOUNT_UPDATED_SUCCESSFULLY_MESSAGE="Account details updated successfully" +PLG_API_USERS_USER_DELETE_MESSAGE="Account details deleted successfully" diff --git a/src/users/user.php b/src/users/user.php index 9fa6a41..2412884 100644 --- a/src/users/user.php +++ b/src/users/user.php @@ -140,6 +140,29 @@ public function post() } } + /** + * Funtion to remove sensitive user info fields like password + * + * @param Object $user The user object. + * @param Array $fields Array of fields to be unset + * + * @return object|void $user + * + * @since 2.0.1 + */ + protected function sanitizeUserFields($user, $fields = array('password', 'password_clear', 'otpKey', 'otep')) + { + foreach ($fields as $f) + { + if (isset($user->{$f})) + { + unset($user->{$f}); + } + } + + return $user; + } + /** * Function get for user record. * @@ -150,7 +173,7 @@ public function post() public function get() { $input = JFactory::getApplication()->input; - $id = $input->get('id', 0, 'int'); + $id = $input->get('id', 0, 'string'); $xIdentifier = $input->server->get('HTTP_X_IDENTIFIER', '', 'string'); /* @@ -162,14 +185,12 @@ public function get() // Get user object $user = $this->retriveUser($xIdentifier, $id); - if (! $user->id) + if (!$user->id) { ApiError::raiseError(400, JText::_('PLG_API_USERS_USER_NOT_FOUND_MESSAGE')); return; } - - $this->plugin->setResponse($user); } else { @@ -179,9 +200,11 @@ public function get() { ApiError::raiseError(400, JText::_('JERROR_ALERTNOAUTHOR')); } - - $this->plugin->setResponse($user); } + + $user = $this->sanitizeUserFields($user); + + $this->plugin->setResponse($user); } /** @@ -272,9 +295,9 @@ private function storeUser($user, $formData, $isNew = 0) */ public function delete() { - $app = JFactory::getApplication(); + $app = JFactory::getApplication(); $userIdentifier = $app->input->get('id', 0, 'string'); - $xIdentifier = $app->input->server->get('HTTP_X_IDENTIFIER', '', 'string'); + $xIdentifier = $app->input->server->get('HTTP_X_IDENTIFIER', '', 'string'); $loggedUser = JFactory::getUser();