From c6fd153ea8eefb15e0239bd06512e62c7c612739 Mon Sep 17 00:00:00 2001 From: Daniel Barreto Date: Wed, 11 Feb 2015 16:44:11 -0500 Subject: [PATCH] Use hash validation method instead of encryption methods - refs BT#9092 --- main/inc/lib/extra_field.lib.php | 3 + .../advancedsubscription/ajax/advsub.ajax.php | 95 ++++++++----------- .../src/AdvancedSubscriptionPlugin.class.php | 53 ++++++++--- .../src/HookAdvancedSubscription.class.php | 6 ++ .../advancedsubscription/src/admin_view.php | 31 ++++-- .../advancedsubscription/views/admin_view.tpl | 4 +- ...pproval_admin_rejected_notice_superior.tpl | 2 +- 7 files changed, 117 insertions(+), 77 deletions(-) diff --git a/main/inc/lib/extra_field.lib.php b/main/inc/lib/extra_field.lib.php index 311140143f..09b62e580d 100755 --- a/main/inc/lib/extra_field.lib.php +++ b/main/inc/lib/extra_field.lib.php @@ -736,6 +736,8 @@ class ExtraField extends Model $addOptions = array(); global $app; + $optionsExists = false; + /* $optionsExists = $app['orm.em']->getRepository('ChamiloLMS\Entity\ExtraFieldOptionRelFieldOption')-> findOneBy(array('fieldId' => $field_details['id'])); @@ -756,6 +758,7 @@ class ExtraField extends Model } } + */ $options = array(); if (empty($defaultValueId)) { $options[''] = get_lang('SelectAnOption'); diff --git a/plugin/advancedsubscription/ajax/advsub.ajax.php b/plugin/advancedsubscription/ajax/advsub.ajax.php index c6ad979b41..a6983706fe 100644 --- a/plugin/advancedsubscription/ajax/advsub.ajax.php +++ b/plugin/advancedsubscription/ajax/advsub.ajax.php @@ -9,46 +9,24 @@ require_once __DIR__ . '/../config.php'; $plugin = AdvancedSubscriptionPlugin::create(); -$data = isset($_REQUEST['data']) ? - strlen($_REQUEST['data']) > 16 ? - $plugin->decrypt($_REQUEST['data']) : - null : - null; -// Get data -if (isset($data) && is_array($data)) { - // Action code - $a = isset($data['a']) ? $data['a'] : null; - // User ID - $u = isset($data['u']) ? $data['u'] : null; - // Session ID - $s = isset($data['s']) ? $data['s'] : null; - // Adv sub action - $e = isset($data['e']) ? intval($data['e']) : null; - // More data - $params['is_connected'] = isset($data['is_connected']) ? $data['is_connected'] : false; - $params['profile_completed'] = isset($data['profile_completed']) ? $data['profile_completed'] : 0; - $params['accept'] = isset($data['accept']) ? $data['accept'] : false; -} else { - // Action code - $a = isset($_REQUEST['a']) ? Security::remove_XSS($_REQUEST['a']) : null; - // User ID - $u = isset($_REQUEST['u']) ? intval($_REQUEST['u']) : null; - // Session ID - $s = isset($_REQUEST['s']) ? intval($_REQUEST['s']) : null; - // Adv sub action - $e = isset($_REQUEST['e']) ? intval($_REQUEST['e']) : null; - // More data - $params['is_connected'] = isset($_REQUEST['is_connected']) ? $_REQUEST['is_connected'] : false; - $params['profile_completed'] = isset($_REQUEST['profile_completed']) ? $_REQUEST['profile_completed'] : 0; - $params['accept'] = isset($_REQUEST['accept']) ? $_REQUEST['accept'] : false; -} +$hash = $_REQUEST['v']; +unset($_REQUEST['v']); +$data['a'] = $a = $_REQUEST['a']; +$data['s'] = $s = intval($_REQUEST['s']); +$data['current_user_id'] = intval($_REQUEST['current_user_id']); +$data['u'] = $u = intval($_REQUEST['u']); +$data['q'] = $q = intval($_REQUEST['q']); +$data['e'] = $e = intval($_REQUEST['e']); +$verified = $plugin->checkHash($data, $hash); +$data['is_connected'] = isset($_REQUEST['is_connected']) ? $_REQUEST['is_connected'] : false; +$data['profile_completed'] = isset($_REQUEST['profile_completed']) ? $_REQUEST['profile_completed'] : 0; // Init result array $result = array('error' => true, 'errorMessage' => 'There was an error'); -if (!empty($a) && !empty($u)) { +if ($verified) { switch($a) { case 'check': // Check minimum requirements try { - $res = AdvancedSubscriptionPlugin::create()->isAbleToRequest($u, $params); + $res = AdvancedSubscriptionPlugin::create()->isAbleToRequest($u, $data); if ($res) { $result['error'] = false; $result['errorMessage'] = 'No error'; @@ -63,7 +41,7 @@ if (!empty($a) && !empty($u)) { break; case 'subscribe': // Subscription $bossId = UserManager::getStudentBoss($u); - $res = AdvancedSubscriptionPlugin::create()->startSubscription($u, $s, $params); + $res = AdvancedSubscriptionPlugin::create()->startSubscription($u, $s, $data); if ($res === true) { // send mail to superior $sessionArray = api_get_session_info($s); @@ -92,15 +70,11 @@ if (!empty($a) && !empty($u)) { $admin['complete_name'] = $admin['lastname'] . ', ' . $admin['firstname']; } unset($admin); - $data = array( - 'student' => $studentArray, - 'superior' => $superiorArray, - 'admins' => $adminsArray, - 'session' => $sessionArray, - 'signature' => api_get_setting('Institution'), - 's' => $s, - 'u' => $u, - ); + $data['student'] = $studentArray; + $data['superior'] = $superiorArray; + $data['admins'] = $adminsArray; + $data['session'] = $sessionArray; + $data['signature'] = api_get_setting('Institution'); if (empty($superiorId)) { // Does not have boss $res = $plugin->updateQueueStatus($data, ADV_SUB_QUEUE_STATUS_BOSS_APPROVED); @@ -121,18 +95,31 @@ if (!empty($a) && !empty($u)) { } } } else { - $dataUrl = array( - 'a' => 'confirm', - 's' => $s, - 'u' => $u, - ); + $dataUrl['a'] = $data['a']; + $dataUrl['s'] = intval($data['s']); + $dataUrl['current_user_id'] = intval($data['current_user_id']); + $dataUrl['u'] = intval($data['u']); + $dataUrl['q'] = intval($data['q']); + $dataUrl['e'] = intval($data['e']); $dataUrl['e'] = ADV_SUB_QUEUE_STATUS_BOSS_APPROVED; - $data['acceptUrl'] = api_get_path(WEB_PLUGIN_PATH) . 'advancedsubscription/ajax/advsub.ajax.php' . - '?data=' . $plugin->encrypt($dataUrl); + $student['acceptUrl'] = api_get_path(WEB_PLUGIN_PATH) . 'advancedsubscription/ajax/advsub.ajax.php?' . + 'a=confirm&' . + 's=' . $s . '&' . + 'current_user_id=' . $dataUrl['current_user_id'] . '&' . + 'e=' . ADV_SUB_QUEUE_STATUS_BOSS_APPROVED . '&' . + 'u=' . $student['user_id'] . '&' . + 'q=' . $student['queue_id'] . '&' . + 'v=' . $plugin->generateHash($dataUrl); $dataUrl['e'] = ADV_SUB_QUEUE_STATUS_BOSS_DISAPPROVED; - $data['rejectUrl'] = api_get_path(WEB_PLUGIN_PATH) . 'advancedsubscription/ajax/advsub.ajax.php' . - '?data=' . $plugin->encrypt($dataUrl); + $student['rejectUrl'] = api_get_path(WEB_PLUGIN_PATH) . 'advancedsubscription/ajax/advsub.ajax.php?' . + 'a=confirm&' . + 's=' . $s . '&' . + 'current_user_id=' . $dataUrl['current_user_id'] . '&' . + 'e=' . ADV_SUB_QUEUE_STATUS_BOSS_APPROVED . '&' . + 'u=' . $student['user_id'] . '&' . + 'q=' . $student['queue_id'] . '&' . + 'v=' . $plugin->generateHash($dataUrl); $result['mailIds'] = $plugin->sendMail($data, ADV_SUB_ACTION_STUDENT_REQUEST); if (!empty($result['mailIds'])) { $result['error'] = false; diff --git a/plugin/advancedsubscription/src/AdvancedSubscriptionPlugin.class.php b/plugin/advancedsubscription/src/AdvancedSubscriptionPlugin.class.php index 4fa9268cb2..cc5d58938b 100644 --- a/plugin/advancedsubscription/src/AdvancedSubscriptionPlugin.class.php +++ b/plugin/advancedsubscription/src/AdvancedSubscriptionPlugin.class.php @@ -230,7 +230,7 @@ class AdvancedSubscriptionPlugin extends Plugin implements HookPluginInterface public function startSubscription($userId, $sessionId, $params) { $result = false; - if (isset($params['accept']) && !empty($sessionId) && !empty($userId)) { + if (!empty($sessionId) && !empty($userId)) { $advSub = self::create(); try { if ($advSub->isAbleToRequest($userId, $params)) { @@ -787,16 +787,23 @@ class AdvancedSubscriptionPlugin extends Plugin implements HookPluginInterface */ public function getQueueUrl($params) { - $data = array( - 'a' => 'subscribe', - 'u' => $params['user_id'], - 's' => $params['session_id'], - 'is_connected' => $params['is_connected'], - 'profile_completed' => $params['profile_completed'], - 'accept' => $params['accept'], - ); - $data = $this->encrypt($data); - $url = api_get_path(WEB_PLUGIN_PATH) . 'advancedsubscription/ajax/advsub.ajax.php?data=' . $data; + $dataUrl['a'] = $params['a']; + $dataUrl['s'] = intval($params['s']); + $dataUrl['current_user_id'] = intval($params['current_user_id']); + $dataUrl['u'] = intval($params['u']); + $dataUrl['q'] = intval($params['q']); + $dataUrl['e'] = intval($params['e']); + + $url = api_get_path(WEB_PLUGIN_PATH) . 'advancedsubscription/ajax/advsub.ajax.php?' . + 'a=' . $dataUrl['a'] . '&' . + 's=' . $dataUrl['s'] . '&' . + 'current_user_id=' . $dataUrl['current_user_id'] . '&' . + 'e=' . $dataUrl['e'] . '&' . + 'u=' . $dataUrl['u'] . '&' . + 'q=' . $dataUrl['q'] . '&' . + 'is_connected=' . $params['is_connected'] . '&' . + 'profile_completed=' . $params['profile_completed'] . '&' . + 'v=' . $this->generateHash($dataUrl); return $url; } @@ -872,4 +879,28 @@ class AdvancedSubscriptionPlugin extends Plugin implements HookPluginInterface $columns = 'id, name'; return Database::select($columns, $sessionTable); } + + /** + * Generate security hash to check data send by url params + * @param string $data + * @return string + */ + public function generateHash($data) + { + global $_config; + $key = sha1($_config['secret_key']); + $data = serialize($data); + return sha1($data . $key); + } + + /** + * Verify hash from data + * @param string $data + * @param string $hash + * @return bool + */ + public function checkHash($data, $hash) + { + return $this->generateHash($data) == $hash; + } } diff --git a/plugin/advancedsubscription/src/HookAdvancedSubscription.class.php b/plugin/advancedsubscription/src/HookAdvancedSubscription.class.php index d68bb8b1a1..159c77689f 100644 --- a/plugin/advancedsubscription/src/HookAdvancedSubscription.class.php +++ b/plugin/advancedsubscription/src/HookAdvancedSubscription.class.php @@ -489,6 +489,12 @@ class HookAdvancedSubscription extends HookObserver implements } catch (\Exception $e) { $data['message'] = $e->getMessage(); } + $params['a'] = 'subscribe'; + $params['s'] = intval($sessionId); + $params['current_user_id'] = 0; // No needed + $params['u'] = intval($userId); + $params['q'] = 0; // No needed + $params['e'] = ADV_SUB_QUEUE_STATUS_START; if ($vacancy > 0) { // Check conditions if ($status === ADV_SUB_QUEUE_STATUS_NO_QUEUE) { diff --git a/plugin/advancedsubscription/src/admin_view.php b/plugin/advancedsubscription/src/admin_view.php index 531dd8c5d1..dc3445a51d 100644 --- a/plugin/advancedsubscription/src/admin_view.php +++ b/plugin/advancedsubscription/src/admin_view.php @@ -71,18 +71,32 @@ if (!empty($s)) { $sessionArray['recommended_number_of_participants'] = $var['field_valiue']; $adminsArray = UserManager::get_all_administrators(); - $data = array( - 'a' => 'confirm', - 's' => $s, - 'current_user_id' => api_get_user_id(), - ); + $data['a'] = 'confirm'; + $data['s'] = $s; + $data['current_user_id'] = api_get_user_id(); foreach ($studentList['students'] as &$student) { - $data['u'] = $student['user_id']; - $data['queue'] = array('id' => $student['queue_id']); + $data['u'] = intval($student['user_id']); + $data['q'] = intval($student['queue_id']); $data['e'] = ADV_SUB_QUEUE_STATUS_ADMIN_APPROVED; - $student['dataApprove'] = $plugin->encrypt($data); + $student['acceptUrl'] = api_get_path(WEB_PLUGIN_PATH) . 'advancedsubscription/ajax/advsub.ajax.php?' . + 'a=confirm&' . + 's=' . $s . '&' . + 'current_user_id=' . api_get_user_id() . '&' . + 'e=' . ADV_SUB_QUEUE_STATUS_ADMIN_APPROVED . '&' . + 'u=' . $student['user_id'] . '&' . + 'q=' . $student['queue_id'] . '&' . + 'v=' . $plugin->generateHash($data); $data['e'] = ADV_SUB_QUEUE_STATUS_ADMIN_DISAPPROVED; + $student['rejectUrl'] = api_get_path(WEB_PLUGIN_PATH) . 'advancedsubscription/ajax/advsub.ajax.php?' . + 'a=confirm&' . + 's=' . $s . '&' . + 'current_user_id=' . api_get_user_id() . '&' . + 'e=' . ADV_SUB_QUEUE_STATUS_ADMIN_DISAPPROVED . '&' . + 'u=' . $student['user_id'] . '&' . + 'q=' . $student['queue_id'] . '&' . + 'v=' . $plugin->generateHash($data); + ; $student['dataDisapprove'] = $plugin->encrypt($data); $student['complete_name'] = $student['lastname'] . ', ' . $student['firstname']; $student['picture'] = UserManager::get_user_picture_path_by_id($student['user_id'], 'web', false, true); @@ -94,7 +108,6 @@ if (!empty($s)) { // Assign variables $tpl->assign('sessionItems', $sessionList); -$tpl->assign('data', $data); $tpl->assign('approveAdmin', ADV_SUB_QUEUE_STATUS_ADMIN_APPROVED); $tpl->assign('disapproveAdmin', ADV_SUB_QUEUE_STATUS_ADMIN_DISAPPROVED); // Get rendered template diff --git a/plugin/advancedsubscription/views/admin_view.tpl b/plugin/advancedsubscription/views/admin_view.tpl index 4b460f674c..b83e0b5f65 100644 --- a/plugin/advancedsubscription/views/admin_view.tpl +++ b/plugin/advancedsubscription/views/admin_view.tpl @@ -108,7 +108,7 @@ '¿Esta seguro de que desea aceptar la inscripción de {{ student.complete_name }}?' ) ) return false;" - href="{{ _p.web_plugin }}advancedsubscription/ajax/advsub.ajax.php?data={{ student.dataApprove }}" + href="{{ student.acceptUrl }}" > Aceptar @@ -119,7 +119,7 @@ '¿Esta seguro de que desea rechazar la inscripción de {{ student.complete_name }}?' ) ) return false;" - href="{{ _p.web_plugin }}advancedsubscription/ajax/advsub.ajax.php?data={{ student.dataDisapprove }}" + href="{{ student.rejectUrl }}" > Rechazar diff --git a/plugin/advancedsubscription/views/advsub_approval_admin_rejected_notice_superior.tpl b/plugin/advancedsubscription/views/advsub_approval_admin_rejected_notice_superior.tpl index 202f580217..da2b8fbfa9 100644 --- a/plugin/advancedsubscription/views/advsub_approval_admin_rejected_notice_superior.tpl +++ b/plugin/advancedsubscription/views/advsub_approval_admin_rejected_notice_superior.tpl @@ -59,7 +59,7 @@  

Estimado:

-

{{ admin.complete_name }}

+

{{ superior.complete_name }}

La inscripción de {{ student.complete_name }} al curso {{ session.name }}, que había aprobado anteriormente, fue rechazada por falta de cupos. Les presentamos nuestras disculpas sinceras.

Gracias.

{{ signature }}