From e6423852ef859fbbc0e8fdb1941c1af4e71e364f Mon Sep 17 00:00:00 2001 From: Tony Murray Date: Sat, 26 Oct 2019 00:29:12 +0000 Subject: [PATCH] Remove $_SESSION usage, except install (#10745) * Remove $_SESSION usage, except install Fixes issue with device debug capture Removes secure_cookies setting, use the .env variable SESSION_SECURE_COOKIE instead. Reminder secure cookies requires cookies are transported over https, if everything is already transported via https, the setting won't make a difference. * Fix availability map controls --- .../LdapAuthorizationAuthorizer.php | 2 +- LibreNMS/Authentication/LegacyAuth.php | 8 --- LibreNMS/Config.php | 5 -- .../Ajax/AvailabilityMapController.php | 63 +++++++++++++++++++ .../Controllers/Ajax/ResolutionController.php | 6 -- app/Http/Kernel.php | 1 - app/Http/Middleware/LegacySession.php | 46 -------------- html/ajax_mapview.php | 17 ----- html/ajax_output.php | 1 + html/js/librenms.js | 6 +- includes/html/common/availability-map.inc.php | 11 ++-- includes/html/pages/api-access.inc.php | 4 +- misc/config_definitions.json | 3 - resources/views/layouts/librenmsv1.blade.php | 2 +- routes/web.php | 4 +- 15 files changed, 78 insertions(+), 101 deletions(-) create mode 100644 app/Http/Controllers/Ajax/AvailabilityMapController.php delete mode 100644 app/Http/Middleware/LegacySession.php delete mode 100644 html/ajax_mapview.php diff --git a/LibreNMS/Authentication/LdapAuthorizationAuthorizer.php b/LibreNMS/Authentication/LdapAuthorizationAuthorizer.php index 44aa5d79f9..b9123c8e8e 100644 --- a/LibreNMS/Authentication/LdapAuthorizationAuthorizer.php +++ b/LibreNMS/Authentication/LdapAuthorizationAuthorizer.php @@ -33,7 +33,7 @@ * modules already existing. * * To save lots of redundant queries to the LDAP server and speed up the - * libreNMS WebUI, all information is cached within the PHP $_SESSION as + * libreNMS WebUI, all information is cached within the Laravel Session as * long as specified in the 'auth_ldap_cache_ttl' setting (Default: 300s). */ diff --git a/LibreNMS/Authentication/LegacyAuth.php b/LibreNMS/Authentication/LegacyAuth.php index 9a1ad26cae..59b6ca2eed 100644 --- a/LibreNMS/Authentication/LegacyAuth.php +++ b/LibreNMS/Authentication/LegacyAuth.php @@ -73,12 +73,4 @@ class LegacyAuth static::$_instance = null; return static::get(); } - - public static function setUpLegacySession() - { - if (!isset($_SESSION)) { - @session_start(); - session_write_close(); - } - } } diff --git a/LibreNMS/Config.php b/LibreNMS/Config.php index c94bce1ba5..10c2fa2a00 100644 --- a/LibreNMS/Config.php +++ b/LibreNMS/Config.php @@ -408,7 +408,6 @@ class Config // If we're on SSL, let's properly detect it if (isset($_SERVER['HTTPS'])) { self::set('base_url', preg_replace('/^http:/', 'https:', self::get('base_url'))); - self::set('secure_cookies', true); } // If we're on SSL, let's properly detect it @@ -416,10 +415,6 @@ class Config self::set('base_url', preg_replace('/^http:/', 'https:', self::get('base_url'))); } - if (self::get('secure_cookies')) { - ini_set('session.cookie_secure', 1); - } - if (!self::get('email_from')) { self::set('email_from', '"' . self::get('project_name') . '" <' . self::get('email_user') . '@' . php_uname('n') . '>'); } diff --git a/app/Http/Controllers/Ajax/AvailabilityMapController.php b/app/Http/Controllers/Ajax/AvailabilityMapController.php new file mode 100644 index 0000000000..f3b98bb2a5 --- /dev/null +++ b/app/Http/Controllers/Ajax/AvailabilityMapController.php @@ -0,0 +1,63 @@ +. + * + * @package LibreNMS + * @link http://librenms.org + * @copyright 2019 Tony Murray + * @author Tony Murray + */ + +namespace App\Http\Controllers\Ajax; + +use App\Http\Controllers\Controller; +use Illuminate\Http\Request; + +class AvailabilityMapController extends Controller +{ + public function setView(Request $request) + { + $this->validate($request, [ + 'map_view' => 'required|numeric|in:0,1,2' + ]); + + return $this->setSessionValue($request, 'map_view'); + } + + public function setGroup(Request $request) + { + $this->validate($request, [ + 'group_view' => 'required|numeric' + ]); + + return $this->setSessionValue($request, 'group_view'); + } + + /** + * @param \Illuminate\Http\Request $request + * @param string $key + * @return \Illuminate\Http\JsonResponse + */ + private function setSessionValue($request, $key) + { + $value = $request->get($key); + $request->session()->put($key, $value); + + return response()->json([$key, $value]); + } +} diff --git a/app/Http/Controllers/Ajax/ResolutionController.php b/app/Http/Controllers/Ajax/ResolutionController.php index e34b595ffb..a4cc0a17ee 100644 --- a/app/Http/Controllers/Ajax/ResolutionController.php +++ b/app/Http/Controllers/Ajax/ResolutionController.php @@ -37,12 +37,6 @@ class ResolutionController extends Controller 'height' => 'required|numeric' ]); - // legacy session - session_start(); - $_SESSION['screen_width'] = $request->width; - $_SESSION['screen_height'] = $request->height; - session_write_close(); - // laravel session session([ 'screen_width' => $request->width, diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index f60f118afa..a53bd58912 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -38,7 +38,6 @@ class Kernel extends HttpKernel \Illuminate\View\Middleware\ShareErrorsFromSession::class, \App\Http\Middleware\VerifyCsrfToken::class, \App\Http\Middleware\LegacyExternalAuth::class, - \App\Http\Middleware\LegacySession::class, \Illuminate\Routing\Middleware\SubstituteBindings::class, ], diff --git a/app/Http/Middleware/LegacySession.php b/app/Http/Middleware/LegacySession.php deleted file mode 100644 index 0891c03b63..0000000000 --- a/app/Http/Middleware/LegacySession.php +++ /dev/null @@ -1,46 +0,0 @@ -. - * - * @package LibreNMS - * @link http://librenms.org - * @copyright 2018 Tony Murray - * @author Tony Murray - */ - -namespace App\Http\Middleware; - -use Closure; -use LibreNMS\Authentication\LegacyAuth; - -class LegacySession -{ - /** - * Handle an incoming request. - * - * @param \Illuminate\Http\Request $request - * @param \Closure $next - * @return mixed - */ - public function handle($request, Closure $next) - { - LegacyAuth::setUpLegacySession(); - - return $next($request); - } -} diff --git a/html/ajax_mapview.php b/html/ajax_mapview.php deleted file mode 100644 index d60c562d7e..0000000000 --- a/html/ajax_mapview.php +++ /dev/null @@ -1,17 +0,0 @@ - $_SESSION['map_view']); - header('Content-type: text/plain'); - echo json_encode($map_view); -} - -//availability-map device group view -if (isset($_REQUEST['group_view'])) { - $_SESSION['group_view'] = $_REQUEST['group_view']; - $group_view = array('group_view' => $_SESSION['group_view']); - header('Content-type: text/plain'); - echo json_encode($group_view); -} diff --git a/html/ajax_output.php b/html/ajax_output.php index d2b4e29e54..59f44253c8 100644 --- a/html/ajax_output.php +++ b/html/ajax_output.php @@ -13,6 +13,7 @@ */ session_start(); +session_write_close(); if (isset($_SESSION['stage']) && $_SESSION['stage'] == 2) { $init_modules = array('web', 'nodb'); require realpath(__DIR__ . '/..') . '/includes/init.php'; diff --git a/html/js/librenms.js b/html/js/librenms.js index d7492bd1a7..44e93bbf83 100644 --- a/html/js/librenms.js +++ b/html/js/librenms.js @@ -154,7 +154,7 @@ $(document).on("click", '.collapse-neighbors', function(event) //availability-map mode change $(document).on("change", '#mode', function() { - $.post('ajax_mapview.php', + $.post('ajax/set_map_view', { map_view: $(this).val() }, @@ -166,7 +166,7 @@ $(document).on("change", '#mode', function() { //availability-map device group $(document).on("change", '#group', function() { - $.post('ajax_mapview.php', + $.post('ajax/set_map_group', { group_view: $(this).val() }, @@ -433,7 +433,7 @@ function init_select2(selector, type, data, selected, placeholder) { function humanize_duration(seconds) { // transform xxx seconds into yy years MM months dd days hh hours mm:ss - + var duration = moment.duration(Number(seconds), 's'); var years = duration.years(), months = duration.months(), diff --git a/includes/html/common/availability-map.inc.php b/includes/html/common/availability-map.inc.php index 4c4572a202..0b819c71a8 100644 --- a/includes/html/common/availability-map.inc.php +++ b/includes/html/common/availability-map.inc.php @@ -14,12 +14,9 @@ use LibreNMS\Config; +$mode = Session::get('map_view', 0); if (isset($settings['mode_select']) && $settings['mode_select'] !== '') { $mode = $settings['mode_select']; -} elseif (isset($_SESSION["map_view"]) && is_numeric($_SESSION["map_view"])) { - $mode = $_SESSION["map_view"]; -} else { - $mode = 0; } $select_modes = array( @@ -169,7 +166,7 @@ if (defined('SHOW_SETTINGS')) { // Only show devices if mode is 0 or 2 (Only Devices or both) if (Config::get('webui.availability_map_use_device_groups') != 0) { $device_group = 'SELECT `D`.`device_id` FROM `device_group_device` AS `D` WHERE `device_group_id` = ?'; - $in_devices = dbFetchColumn($device_group, [$_SESSION['group_view']]); + $in_devices = dbFetchColumn($device_group, [Session::get('group_view')]); } $sql = 'SELECT `D`.`hostname`, `D`.`sysName`, `D`.`device_id`, `D`.`status`, `D`.`uptime`, `D`.`os`, `D`.`icon`, `D`.`ignore`, `D`.`disabled` FROM `devices` AS `D`'; @@ -342,7 +339,7 @@ if (defined('SHOW_SETTINGS')) { $sql = 'SELECT `G`.`id`, `G`.`name` FROM `device_groups` AS `G`'; $dev_groups = dbFetchRows($sql); - if ($_SESSION['group_view'] == 0) { + if (Session::get('group_view') == 0) { $selected = 'selected'; } else { $selected = ''; @@ -354,7 +351,7 @@ if (defined('SHOW_SETTINGS')) { '; foreach ($dev_groups as $dev_group) { - if ($_SESSION['group_view'] == $dev_group['id']) { + if (Session::get('group_view') == $dev_group['id']) { $selected = 'selected'; } else { $selected = ''; diff --git a/includes/html/pages/api-access.inc.php b/includes/html/pages/api-access.inc.php index 09446614ce..25efa85c64 100644 --- a/includes/html/pages/api-access.inc.php +++ b/includes/html/pages/api-access.inc.php @@ -116,11 +116,11 @@ echo ' '; -if ($_SESSION['api_token'] === true) { +if (Session::get('api_token') === true) { echo " "; - unset($_SESSION['api_token']); + Session::forget('api_token'); } echo ' diff --git a/misc/config_definitions.json b/misc/config_definitions.json index 5fd7ad5c63..d347ca73de 100644 --- a/misc/config_definitions.json +++ b/misc/config_definitions.json @@ -3809,9 +3809,6 @@ "order": 1, "type": "boolean" }, - "secure_cookies": { - "type": "boolean" - }, "sensors.guess_limits": { "default": true, "type": "boolean" diff --git a/resources/views/layouts/librenmsv1.blade.php b/resources/views/layouts/librenmsv1.blade.php index d57d1b3b16..0383f36410 100644 --- a/resources/views/layouts/librenmsv1.blade.php +++ b/resources/views/layouts/librenmsv1.blade.php @@ -78,7 +78,7 @@ }); var ajax_url = "{{ url('/ajax') }}"; - +