diff --git a/core/classes/Core/Cookie.php b/core/classes/Core/Cookie.php index d778f60b48..fc3686bbae 100644 --- a/core/classes/Core/Cookie.php +++ b/core/classes/Core/Cookie.php @@ -34,16 +34,26 @@ public static function get(string $name) /** * Create a new cookie. * - * @param string $name Name of cookie to create. - * @param string $value Value to store in cookie. - * @param int $expiry When does the cookie expire? - * @param ?bool $secure Create as secure cookie? - * @param ?bool $httpOnly Create as httpOnly cookie? + * @param string $name Name of cookie to create. + * @param string $value Value to store in cookie. + * @param ?int $expiry When does the cookie expire? Null for session + * @param ?bool $secure Create as secure cookie? + * @param ?bool $httpOnly Create as httpOnly cookie? + * @param ?bool $addExpiry Whether to add expiry onto current timestamp or not. * @return bool Whether cookie was set or not */ - public static function put(string $name, string $value, int $expiry, ?bool $secure = false, ?bool $httpOnly = false): bool - { - return setcookie($name, $value, time() + $expiry, '/', null, $secure, $httpOnly); + public static function put( + string $name, + string $value, + ?int $expiry, + ?bool $secure = false, + ?bool $httpOnly = false, + ?bool $addExpiry = true + ): bool { + if ($expiry && $addExpiry) { + $expiry = time() + $expiry; + } + return setcookie($name, $value, $expiry, '/', null, $secure, $httpOnly); } /** diff --git a/core/classes/Core/User.php b/core/classes/Core/User.php index 1d24bea684..1bbcda60d0 100644 --- a/core/classes/Core/User.php +++ b/core/classes/Core/User.php @@ -114,7 +114,26 @@ private function find(string $value, string $field = 'id'): bool if ($field !== 'hash') { $data = $this->_db->get('users', [$field, $value]); } else { - $data = $this->_db->query('SELECT nl2_users.* FROM nl2_users LEFT JOIN nl2_users_session ON nl2_users.id = user_id WHERE hash = ? AND nl2_users_session.active = 1', [$value]); + $data = $this->_db->query( + << ? + ) + SQL, + [ + $value, + time(), + ] + ); } if ($data->count()) { @@ -288,6 +307,7 @@ private function _commonLogin(?string $username, ?string $password, bool $rememb // Valid credentials // TODO: job to remove old sessions? $hash = SecureRandom::alphanumeric(); + $expiresAt = $remember ? time() + Config::get('remember.cookie_expiry') : null; $this->_db->insert('users_session', [ 'user_id' => $this->data()->id, @@ -297,14 +317,14 @@ private function _commonLogin(?string $username, ?string $password, bool $rememb 'login_method' => $is_admin ? 'admin' : $method, 'user_agent' => $_SERVER['HTTP_USER_AGENT'], 'ip' => HttpUtils::getRemoteAddress(), + 'expires_at' => $expiresAt, ]); Session::put($sessionName, $hash); if ($remember) { - $expiry = $is_admin ? 3600 : Config::get('remember.cookie_expiry'); $cookieName = $is_admin ? ($this->_cookieName . '_adm') : $this->_cookieName; - Cookie::put($cookieName, $hash, $expiry, HttpUtils::getProtocol() === 'https', true); + Cookie::put($cookieName, $hash, $expiresAt, HttpUtils::getProtocol() === 'https', true, false); } return true; @@ -524,6 +544,21 @@ public function getActiveSessions(): array )->results(); } + /** + * Log the user out from a selected session. + * + * @param string $sessionId Selected session ID. + * + * @return void + */ + public function logoutSessionById(string $sessionId): void + { + DB::getInstance()->query('UPDATE nl2_users_session SET `active` = 0 WHERE user_id = ? AND id = ?', [ + $this->data()->id, + $sessionId, + ]); + } + /** * Log the user out from all other sessions. */ diff --git a/core/classes/Database/DatabaseInitialiser.php b/core/classes/Database/DatabaseInitialiser.php index dda85fb037..790c2e261b 100644 --- a/core/classes/Database/DatabaseInitialiser.php +++ b/core/classes/Database/DatabaseInitialiser.php @@ -269,6 +269,7 @@ private function initialiseSettings(): void private function initialiseTasks(): void { GenerateSitemap::schedule(new Language('core', 'en_UK')); + PurgeExpiredSessions::schedule(new Language('core', 'en_UK')); } private function initialiseTemplates(): void diff --git a/core/classes/Misc/HttpUtils.php b/core/classes/Misc/HttpUtils.php index dbe3e8be83..8baee0b4a1 100644 --- a/core/classes/Misc/HttpUtils.php +++ b/core/classes/Misc/HttpUtils.php @@ -257,9 +257,9 @@ public static function getHeader(string $header_name): ?string return null; } - public static function getIpCountry(string $ip): string + public static function getIpCountry(?string $ip): string { - if (in_array($ip, ['localhost', '127.0.0.1', '::1'])) { + if (!$ip || in_array($ip, ['localhost', '127.0.0.1', '::1'])) { return 'Unknown'; } diff --git a/core/includes/updates/212.php b/core/includes/updates/212.php new file mode 100644 index 0000000000..b9036f6403 --- /dev/null +++ b/core/includes/updates/212.php @@ -0,0 +1,12 @@ +runMigrations(); + + PurgeExpiredSessions::schedule(new Language('core', 'en_UK')); + + $this->setVersion('2.2.0'); + } +}; diff --git a/core/installation/steps/database_configuration.php b/core/installation/steps/database_configuration.php index cc9dbc2a7c..e96eceb3bf 100644 --- a/core/installation/steps/database_configuration.php +++ b/core/installation/steps/database_configuration.php @@ -58,7 +58,7 @@ ], 'remember' => [ 'cookie_name' => 'nl2', - 'cookie_expiry' => 604800, + 'cookie_expiry' => 2629800, ], 'session' => [ 'session_name' => '2user', diff --git a/core/migrations/20240616113453_add_session_expiry_column_to_users_session_table.php b/core/migrations/20240616113453_add_session_expiry_column_to_users_session_table.php new file mode 100644 index 0000000000..bcfdad8694 --- /dev/null +++ b/core/migrations/20240616113453_add_session_expiry_column_to_users_session_table.php @@ -0,0 +1,15 @@ +table('nl2_users_session') + ->addColumn('expires_at', 'integer', ['length' => 11, 'null' => true, 'default' => null]) + ->update(); + } +} diff --git a/custom/templates/DefaultRevamp/user/sessions.tpl b/custom/templates/DefaultRevamp/user/sessions.tpl index b1d156d33a..862ba82681 100644 --- a/custom/templates/DefaultRevamp/user/sessions.tpl +++ b/custom/templates/DefaultRevamp/user/sessions.tpl @@ -80,14 +80,18 @@ {if $CAN_LOGOUT_ALL} {/if} @@ -102,7 +106,11 @@
{$NO} - {$CONFIRM} +
+ + + +
{/foreach} diff --git a/dev/scripts/cli_install.php b/dev/scripts/cli_install.php index 8bc5654422..f96331e346 100644 --- a/dev/scripts/cli_install.php +++ b/dev/scripts/cli_install.php @@ -116,7 +116,7 @@ function getEnvVar(string $name, string $fallback = null, array $valid_values = ], 'remember' => [ 'cookie_name' => 'nl2', - 'cookie_expiry' => 604800, + 'cookie_expiry' => 2629800, ], 'session' => [ 'session_name' => '2user', diff --git a/modules/Core/classes/Tasks/PurgeExpiredSessions.php b/modules/Core/classes/Tasks/PurgeExpiredSessions.php new file mode 100644 index 0000000000..c678c03bb5 --- /dev/null +++ b/modules/Core/classes/Tasks/PurgeExpiredSessions.php @@ -0,0 +1,54 @@ +_container->get(Language::class); + + $cutoff = strtotime('1 month ago'); + $count = DB::getInstance()->query( + <<first()->c; + + if ($count) { + DB::getInstance()->query( + 'DELETE FROM `nl2_users_session` WHERE `last_seen` < ? OR (`last_seen` < ? AND `active` = 0 AND `expires_at` IS NULL)', + [ + $cutoff, + ] + ); + } + + $this->setOutput(['result' => $language->get('admin', 'sessions_purged', ['count' => $count])]); + $this->reschedule($language); + + return Task::STATUS_COMPLETED; + } + + private function reschedule(Language $language) { + Queue::schedule((new PurgeExpiredSessions())->fromNew( + Module::getIdFromName('Core'), + $language->get('admin', 'purge_sessions'), + [], + Date::next()->getTimestamp() + )); + } + + public static function schedule(Language $language) { + (new self())->reschedule($language); + } +} diff --git a/modules/Core/language/en_UK.json b/modules/Core/language/en_UK.json index 42069788e6..4fbbcebcc7 100644 --- a/modules/Core/language/en_UK.json +++ b/modules/Core/language/en_UK.json @@ -524,6 +524,7 @@ "admin/profile_widgets": "Profile Widgets", "admin/public": "Public", "admin/purge_errors": "Purge Errors", + "admin/purge_sessions": "Purge Sessions", "admin/query_error_deleted_successfully": "Query error deleted successfully.", "admin/query_errors": "Query Errors", "admin/query_errors_purged_successfully": "Query errors purged successfully.", @@ -641,6 +642,7 @@ "admin/server_query_port": "Server Query Port", "admin/server_query_port_help": "This is the query.port option in your server's server.properties, provided the enable-query option in the same file is set to true.", "admin/server_updated": "Server updated successfully.", + "admin/sessions_purged": "{{count}} sessions purged successfully.", "admin/settings": "Settings", "admin/settings_updated_successfully": "General settings updated successfully.", "admin/show_ip_on_status_page": "Show IP on status page?", @@ -870,7 +872,9 @@ "general/log_out": "Log Out", "general/log_out_click": "Click here to log out", "general/log_out_complete": "Logout successful. Click {{linkStart}}here{{linkEnd}} to continue.", - "general/logout_session_successfully": "Session successfully logged out.", + "general/log_out_all_sessions_confirm": "Are you sure you wish to end all other sessions?", + "general/log_out_selected_session_confirm": "Are you sure you wish to end the selected session?", + "general/logout_session_successful": "Session successfully logged out.", "general/more": "More", "general/new": "New", "general/next": "Next", diff --git a/modules/Core/pages/panel/users_sessions.php b/modules/Core/pages/panel/users_sessions.php index 562f8f6b4d..54e969174e 100644 --- a/modules/Core/pages/panel/users_sessions.php +++ b/modules/Core/pages/panel/users_sessions.php @@ -1,12 +1,21 @@ handlePanelPageLoad('admincp.users.sessions')) { require_once(ROOT_PATH . '/403.php'); @@ -38,7 +47,7 @@ 'active' => false ]); $success = $language->get('general', 'logout_session_successfully'); - } else if ($_POST['action'] === 'logout_other_sessions') { + } elseif ($_POST['action'] === 'logout_other_sessions') { $view_user->logoutAllOtherSessions(); $success = $language->get('admin', 'sessions_logged_out'); } @@ -46,12 +55,10 @@ $errors[] = $language->get('general', 'invalid_token'); } } -$timeago = new TimeAgo(TIMEZONE); +$timeAgo = new TimeAgo(TIMEZONE); $sessions = $view_user->getActiveSessions(); $user_sessions_list = []; -// TODO: Should we display all sessions, or just active ones? Over time, the list could get very long if we display all sessions. -// Not really any reason to show inactive ones, since they can't action on them. foreach ($sessions as $session) { $agent = new \Jenssegers\Agent\Agent(); $agent->setUserAgent($session->user_agent); @@ -65,7 +72,7 @@ 'device_browser_version' => $agent->version($agent->browser()), 'method' => $session->login_method, 'last_seen_short' => $session->last_seen - ? $timeago->inWords($session->last_seen, $language) + ? $timeAgo->inWords($session->last_seen, $language) : $language->get('admin', 'unknown'), 'last_seen_long' => $session->last_seen ? date(DATE_FORMAT, $session->last_seen) diff --git a/modules/Core/pages/user/sessions.php b/modules/Core/pages/user/sessions.php index 7543b24316..c368b86ede 100644 --- a/modules/Core/pages/user/sessions.php +++ b/modules/Core/pages/user/sessions.php @@ -1,13 +1,22 @@ get('user', 'user_cp'); -require_once(ROOT_PATH . '/core/templates/frontend_init.php'); +require_once ROOT_PATH . '/core/templates/frontend_init.php'; -$timeago = new TimeAgo(TIMEZONE); +$timeAgo = new TimeAgo(TIMEZONE); if (Input::exists()) { if (Token::check()) { if (Input::get('action') === 'logout_other_sessions') { $user->logoutAllOtherSessions(); - Session::flash('user_sessions_success', $language->get('user', 'sessions_logged_out')); + } else { + $user->logoutSessionById(Input::get('session_hash')); } + + Session::flash('user_sessions_success', $language->get('user', 'logout_session_successful')); + Redirect::to(URL::build('/user/sessions')); } else { // Invalid token Session::flash('user_sessions_error', $language->get('general', 'invalid_token')); @@ -50,8 +63,7 @@ $sessions = $user->getActiveSessions(); $sessions_list = []; -// TODO: Should we display all sessions, or just active ones? Over time, the list could get very long if we display all sessions. -// Not really any reason to show inactive ones, since they can't action on them. + foreach ($sessions as $session) { $agent = new \Jenssegers\Agent\Agent(); $agent->setUserAgent($session->user_agent); @@ -63,7 +75,7 @@ ]), 'is_admin' => $session->login_method === 'admin', 'is_remembered' => $session->remember_me, - 'last_seen_timeago' => $timeago->inWords($session->last_seen, $language), + 'last_seen_timeago' => $timeAgo->inWords($session->last_seen, $language), 'last_seen' => date(DATE_FORMAT, $session->last_seen), 'device_type' => $agent->deviceType(), 'device_os' => $agent->platform(), @@ -93,17 +105,19 @@ 'CAN_LOGOUT_ALL' => $can_logout_all, 'LOGOUT' => $language->get('general', 'log_out'), 'LOGOUT_OTHER_SESSIONS' => $language->get('user', 'logout_other_sessions'), + 'LOGOUT_ALL_CONFIRM' => $language->get('general', 'log_out_all_sessions_confirm'), + 'LOGOUT_CONFIRM' => $language->get('general', 'log_out_selected_session_confirm'), ]); // Load modules + template Module::loadPage($user, $pages, $cache, $smarty, [$navigation, $cc_nav, $staffcp_nav], $widgets, $template); -require(ROOT_PATH . '/core/templates/cc_navbar.php'); +require ROOT_PATH . '/core/templates/cc_navbar.php'; $template->onPageLoad(); -require(ROOT_PATH . '/core/templates/navbar.php'); -require(ROOT_PATH . '/core/templates/footer.php'); +require ROOT_PATH . '/core/templates/navbar.php'; +require ROOT_PATH . '/core/templates/footer.php'; // Display template $template->displayTemplate('user/sessions.tpl', $smarty);