From 2ce15cae1a41451b631b814c418b3b3b976d79d1 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Tue, 8 Sep 2020 10:38:35 -0400 Subject: [PATCH 1/3] Use router parameters in Admin modules - Remove 10 @TODO tags # Conflicts: # src/Module/Admin/DBSync.php # src/Module/Admin/Themes/Details.php # src/Module/Admin/Themes/Embed.php --- src/Module/Admin/Addons/Details.php | 160 ++++----- src/Module/Admin/DBSync.php | 146 ++++---- src/Module/Admin/Queue.php | 7 +- src/Module/Admin/Themes/Details.php | 174 ++++----- src/Module/Admin/Themes/Embed.php | 96 ++--- src/Module/Admin/Users.php | 337 +++++++++--------- static/routes.config.php | 6 +- .../templates/admin/dbsync/failed_updates.tpl | 2 +- 8 files changed, 427 insertions(+), 501 deletions(-) diff --git a/src/Module/Admin/Addons/Details.php b/src/Module/Admin/Addons/Details.php index 8e0e14f22..139275cfe 100644 --- a/src/Module/Admin/Addons/Details.php +++ b/src/Module/Admin/Addons/Details.php @@ -34,24 +34,20 @@ class Details extends BaseAdmin { parent::post($parameters); - $a = DI::app(); + $addon = Strings::sanitizeFilePathItem($parameters['addon']); - if ($a->argc > 2) { - // @TODO: Replace with parameter from router - $addon = $a->argv[2]; - $addon = Strings::sanitizeFilePathItem($addon); - if (is_file('addon/' . $addon . '/' . $addon . '.php')) { - include_once 'addon/' . $addon . '/' . $addon . '.php'; - if (function_exists($addon . '_addon_admin_post')) { - $func = $addon . '_addon_admin_post'; - $func($a); - } + $redirect = 'admin/addons/' . $addon; - DI::baseUrl()->redirect('admin/addons/' . $addon); + if (is_file('addon/' . $addon . '/' . $addon . '.php')) { + include_once 'addon/' . $addon . '/' . $addon . '.php'; + + if (function_exists($addon . '_addon_admin_post')) { + $func = $addon . '_addon_admin_post'; + $func(DI::app()); } } - DI::baseUrl()->redirect('admin/addons'); + DI::baseUrl()->redirect($redirect); } public static function content(array $parameters = []) @@ -62,79 +58,73 @@ class Details extends BaseAdmin $addons_admin = Addon::getAdminList(); - if ($a->argc > 2) { - // @TODO: Replace with parameter from router - $addon = $a->argv[2]; - $addon = Strings::sanitizeFilePathItem($addon); - if (!is_file("addon/$addon/$addon.php")) { - notice(DI::l10n()->t('Addon not found.')); - Addon::uninstall($addon); - DI::baseUrl()->redirect('admin/addons'); - } - - if (($_GET['action'] ?? '') == 'toggle') { - parent::checkFormSecurityTokenRedirectOnError('/admin/addons', 'admin_themes', 't'); - - // Toggle addon status - if (Addon::isEnabled($addon)) { - Addon::uninstall($addon); - info(DI::l10n()->t('Addon %s disabled.', $addon)); - } else { - Addon::install($addon); - info(DI::l10n()->t('Addon %s enabled.', $addon)); - } - - DI::baseUrl()->redirect('admin/addons/' . $addon); - } - - // display addon details - if (Addon::isEnabled($addon)) { - $status = 'on'; - $action = DI::l10n()->t('Disable'); - } else { - $status = 'off'; - $action = DI::l10n()->t('Enable'); - } - - $readme = null; - if (is_file("addon/$addon/README.md")) { - $readme = Markdown::convert(file_get_contents("addon/$addon/README.md"), false); - } elseif (is_file("addon/$addon/README")) { - $readme = '
' . file_get_contents("addon/$addon/README") . '
'; - } - - $admin_form = ''; - if (array_key_exists($addon, $addons_admin)) { - require_once "addon/$addon/$addon.php"; - $func = $addon . '_addon_admin'; - $func($a, $admin_form); - } - - $t = Renderer::getMarkupTemplate('admin/addons/details.tpl'); - - return Renderer::replaceMacros($t, [ - '$title' => DI::l10n()->t('Administration'), - '$page' => DI::l10n()->t('Addons'), - '$toggle' => DI::l10n()->t('Toggle'), - '$settings' => DI::l10n()->t('Settings'), - '$baseurl' => DI::baseUrl()->get(true), - - '$addon' => $addon, - '$status' => $status, - '$action' => $action, - '$info' => Addon::getInfo($addon), - '$str_author' => DI::l10n()->t('Author: '), - '$str_maintainer' => DI::l10n()->t('Maintainer: '), - - '$admin_form' => $admin_form, - '$function' => 'addons', - '$screenshot' => '', - '$readme' => $readme, - - '$form_security_token' => parent::getFormSecurityToken('admin_themes'), - ]); + $addon = Strings::sanitizeFilePathItem($parameters['addon']); + if (!is_file("addon/$addon/$addon.php")) { + notice(DI::l10n()->t('Addon not found.')); + Addon::uninstall($addon); + DI::baseUrl()->redirect('admin/addons'); } - DI::baseUrl()->redirect('admin/addons'); + if (($_GET['action'] ?? '') == 'toggle') { + self::checkFormSecurityTokenRedirectOnError('/admin/addons', 'admin_addons', 't'); + + // Toggle addon status + if (Addon::isEnabled($addon)) { + Addon::uninstall($addon); + info(DI::l10n()->t('Addon %s disabled.', $addon)); + } else { + Addon::install($addon); + info(DI::l10n()->t('Addon %s enabled.', $addon)); + } + + DI::baseUrl()->redirect('admin/addons/' . $addon); + } + + // display addon details + if (Addon::isEnabled($addon)) { + $status = 'on'; + $action = DI::l10n()->t('Disable'); + } else { + $status = 'off'; + $action = DI::l10n()->t('Enable'); + } + + $readme = null; + if (is_file("addon/$addon/README.md")) { + $readme = Markdown::convert(file_get_contents("addon/$addon/README.md"), false); + } elseif (is_file("addon/$addon/README")) { + $readme = '
' . file_get_contents("addon/$addon/README") . '
'; + } + + $admin_form = ''; + if (array_key_exists($addon, $addons_admin)) { + require_once "addon/$addon/$addon.php"; + $func = $addon . '_addon_admin'; + $func($a, $admin_form); + } + + $t = Renderer::getMarkupTemplate('admin/addons/details.tpl'); + + return Renderer::replaceMacros($t, [ + '$title' => DI::l10n()->t('Administration'), + '$page' => DI::l10n()->t('Addons'), + '$toggle' => DI::l10n()->t('Toggle'), + '$settings' => DI::l10n()->t('Settings'), + '$baseurl' => DI::baseUrl()->get(true), + + '$addon' => $addon, + '$status' => $status, + '$action' => $action, + '$info' => Addon::getInfo($addon), + '$str_author' => DI::l10n()->t('Author: '), + '$str_maintainer' => DI::l10n()->t('Maintainer: '), + + '$admin_form' => $admin_form, + '$function' => 'addons', + '$screenshot' => '', + '$readme' => $readme, + + '$form_security_token' => self::getFormSecurityToken('admin_addons'), + ]); } } diff --git a/src/Module/Admin/DBSync.php b/src/Module/Admin/DBSync.php index dd7febcc5..36f683e08 100644 --- a/src/Module/Admin/DBSync.php +++ b/src/Module/Admin/DBSync.php @@ -36,91 +36,93 @@ class DBSync extends BaseAdmin $a = DI::app(); - $o = ''; + $action = $parameters['action'] ?? ''; + $update = $parameters['update'] ?? 0; - if ($a->argc > 3 && $a->argv[2] === 'mark') { - // @TODO: Replace with parameter from router - $update = intval($a->argv[3]); - if ($update) { - DI::config()->set('database', 'update_' . $update, 'success'); - $curr = DI::config()->get('system', 'build'); - if (intval($curr) == $update) { - DI::config()->set('system', 'build', intval($curr) + 1); + switch ($action) { + case 'mark': + if ($update) { + DI::config()->set('database', 'update_' . $update, 'success'); + $curr = DI::config()->get('system', 'build'); + if (intval($curr) == $update) { + DI::config()->set('system', 'build', intval($curr) + 1); + } + + info(DI::l10n()->t('Update has been marked successful')); } - info(DI::l10n()->t('Update has been marked successful') . EOL); - } - DI::baseUrl()->redirect('admin/dbsync'); - } - if ($a->argc > 2) { - if ($a->argv[2] === 'check') { + break; + case 'check': // @TODO Seems like a similar logic like Update::check() $retval = DBStructure::update($a->getBasePath(), false, true); if ($retval === '') { - $o .= DI::l10n()->t("Database structure update %s was successfully applied.", DB_UPDATE_VERSION) . "
"; + $o = DI::l10n()->t("Database structure update %s was successfully applied.", DB_UPDATE_VERSION) . "
"; DI::config()->set('database', 'last_successful_update', DB_UPDATE_VERSION); DI::config()->set('database', 'last_successful_update_time', time()); } else { - $o .= DI::l10n()->t("Executing of database structure update %s failed with error: %s", DB_UPDATE_VERSION, $retval) . "
"; - } - if ($a->argv[2] === 'check') { - return $o; - } - } elseif (intval($a->argv[2])) { - require_once 'update.php'; - - // @TODO: Replace with parameter from router - $update = intval($a->argv[2]); - - $func = 'update_' . $update; - - if (function_exists($func)) { - $retval = $func(); - - if ($retval === Update::FAILED) { - $o .= DI::l10n()->t("Executing %s failed with error: %s", $func, $retval); - } elseif ($retval === Update::SUCCESS) { - $o .= DI::l10n()->t('Update %s was successfully applied.', $func); - DI::config()->set('database', $func, 'success'); - } else { - $o .= DI::l10n()->t('Update %s did not return a status. Unknown if it succeeded.', $func); - } - } else { - $o .= DI::l10n()->t('There was no additional update function %s that needed to be called.', $func) . "
"; - DI::config()->set('database', $func, 'success'); + $o = DI::l10n()->t("Executing of database structure update %s failed with error: %s", DB_UPDATE_VERSION, $retval) . "
"; + } + + return $o; + case 'update': + require_once 'update.php'; + + // @TODO: Replace with parameter from router + if ($update) { + $func = 'update_' . $update; + + if (function_exists($func)) { + $retval = $func(); + + if ($retval === Update::FAILED) { + $o = DI::l10n()->t("Executing %s failed with error: %s", $func, $retval); + } elseif ($retval === Update::SUCCESS) { + $o = DI::l10n()->t('Update %s was successfully applied.', $func); + DI::config()->set('database', $func, 'success'); + } else { + $o = DI::l10n()->t('Update %s did not return a status. Unknown if it succeeded.', $func); + } + } else { + $o = DI::l10n()->t('There was no additional update function %s that needed to be called.', $func) . "
"; + DI::config()->set('database', $func, 'success'); + } + + return $o; + } + + break; + default: + $failed = []; + $configStmt = DBA::select('config', ['k', 'v'], ['cat' => 'database']); + while ($config = DBA::fetch($configStmt)) { + $upd = intval(substr($config['k'], 7)); + if ($upd >= 1139 && $config['v'] != 'success') { + $failed[] = $upd; + } + } + DBA::close($configStmt); + + if (!count($failed)) { + $o = Renderer::replaceMacros(Renderer::getMarkupTemplate('admin/dbsync/structure_check.tpl'), [ + '$base' => DI::baseUrl()->get(true), + '$banner' => DI::l10n()->t('No failed updates.'), + '$check' => DI::l10n()->t('Check database structure'), + ]); + } else { + $o = Renderer::replaceMacros(Renderer::getMarkupTemplate('admin/dbsync/failed_updates.tpl'), [ + '$base' => DI::baseUrl()->get(true), + '$banner' => DI::l10n()->t('Failed Updates'), + '$desc' => DI::l10n()->t('This does not include updates prior to 1139, which did not return a status.'), + '$mark' => DI::l10n()->t("Mark success \x28if update was manually applied\x29"), + '$apply' => DI::l10n()->t('Attempt to execute this update step automatically'), + '$failed' => $failed + ]); } return $o; - } } - $failed = []; - $configStmt = DBA::select('config', ['k', 'v'], ['cat' => 'database']); - while ($config = DBA::fetch($configStmt)) { - $upd = intval(substr($config['k'], 7)); - if ($upd >= 1139 && $config['v'] != 'success') { - $failed[] = $upd; - } - } - DBA::close($configStmt); - - if (!count($failed)) { - $o = Renderer::replaceMacros(Renderer::getMarkupTemplate('admin/dbsync/structure_check.tpl'), [ - '$base' => DI::baseUrl()->get(true), - '$banner' => DI::l10n()->t('No failed updates.'), - '$check' => DI::l10n()->t('Check database structure'), - ]); - } else { - $o = Renderer::replaceMacros(Renderer::getMarkupTemplate('admin/dbsync/failed_updates.tpl'), [ - '$base' => DI::baseUrl()->get(true), - '$banner' => DI::l10n()->t('Failed Updates'), - '$desc' => DI::l10n()->t('This does not include updates prior to 1139, which did not return a status.'), - '$mark' => DI::l10n()->t("Mark success \x28if update was manually applied\x29"), - '$apply' => DI::l10n()->t('Attempt to execute this update step automatically'), - '$failed' => $failed - ]); - } - - return $o; + DI::baseUrl()->redirect('admin/dbsync'); + return ''; } } diff --git a/src/Module/Admin/Queue.php b/src/Module/Admin/Queue.php index b6e7d122f..7f5329dbf 100644 --- a/src/Module/Admin/Queue.php +++ b/src/Module/Admin/Queue.php @@ -42,13 +42,10 @@ class Queue extends BaseAdmin { parent::content($parameters); - $a = DI::app(); - - // @TODO: Replace with parameter from router - $deferred = $a->argc > 2 && $a->argv[2] == 'deferred'; + $status = $parameters['status'] ?? ''; // get jobs from the workerqueue table - if ($deferred) { + if ($status == 'deferred') { $condition = ["NOT `done` AND `retrial` > ?", 0]; $sub_title = DI::l10n()->t('Inspect Deferred Worker Queue'); $info = DI::l10n()->t("This page lists the deferred worker jobs. This are jobs that couldn't be executed at the first time."); diff --git a/src/Module/Admin/Themes/Details.php b/src/Module/Admin/Themes/Details.php index c8d057838..5c00c8d9d 100644 --- a/src/Module/Admin/Themes/Details.php +++ b/src/Module/Admin/Themes/Details.php @@ -30,116 +30,80 @@ use Friendica\Util\Strings; class Details extends BaseAdmin { - public static function post(array $parameters = []) - { - parent::post($parameters); - - $a = DI::app(); - - if ($a->argc > 2) { - // @TODO: Replace with parameter from router - $theme = $a->argv[2]; - $theme = Strings::sanitizeFilePathItem($theme); - if (is_file("view/theme/$theme/config.php")) { - require_once "view/theme/$theme/config.php"; - - if (function_exists('theme_admin_post')) { - theme_admin_post($a); - } - } - - info(DI::l10n()->t('Theme settings updated.')); - - if (DI::mode()->isAjax()) { - return; - } - - DI::baseUrl()->redirect('admin/themes/' . $theme); - } - } - public static function content(array $parameters = []) { parent::content($parameters); - $a = DI::app(); - - if ($a->argc > 2) { - // @TODO: Replace with parameter from router - $theme = $a->argv[2]; - $theme = Strings::sanitizeFilePathItem($theme); - if (!is_dir("view/theme/$theme")) { - notice(DI::l10n()->t("Item not found.")); - return ''; - } - - $isEnabled = in_array($theme, Theme::getAllowedList()); - if ($isEnabled) { - $status = "on"; - $action = DI::l10n()->t("Disable"); - } else { - $status = "off"; - $action = DI::l10n()->t("Enable"); - } - - if (!empty($_GET['action']) && $_GET['action'] == 'toggle') { - parent::checkFormSecurityTokenRedirectOnError('/admin/themes', 'admin_themes', 't'); - - if ($isEnabled) { - Theme::uninstall($theme); - info(DI::l10n()->t('Theme %s disabled.', $theme)); - } elseif (Theme::install($theme)) { - info(DI::l10n()->t('Theme %s successfully enabled.', $theme)); - } else { - info(DI::l10n()->t('Theme %s failed to install.', $theme)); - } - - DI::baseUrl()->redirect('admin/themes/' . $theme); - } - - $readme = null; - if (is_file("view/theme/$theme/README.md")) { - $readme = Markdown::convert(file_get_contents("view/theme/$theme/README.md"), false); - } elseif (is_file("view/theme/$theme/README")) { - $readme = "
" . file_get_contents("view/theme/$theme/README") . "
"; - } - - $admin_form = ''; - if (is_file("view/theme/$theme/config.php")) { - require_once "view/theme/$theme/config.php"; - - if (function_exists('theme_admin')) { - $admin_form = ''; - } - } - - $screenshot = [Theme::getScreenshot($theme), DI::l10n()->t('Screenshot')]; - if (!stristr($screenshot[0], $theme)) { - $screenshot = null; - } - - $t = Renderer::getMarkupTemplate('admin/addons/details.tpl'); - return Renderer::replaceMacros($t, [ - '$title' => DI::l10n()->t('Administration'), - '$page' => DI::l10n()->t('Themes'), - '$toggle' => DI::l10n()->t('Toggle'), - '$settings' => DI::l10n()->t('Settings'), - '$baseurl' => DI::baseUrl()->get(true), - '$addon' => $theme, - '$status' => $status, - '$action' => $action, - '$info' => Theme::getInfo($theme), - '$function' => 'themes', - '$admin_form' => $admin_form, - '$str_author' => DI::l10n()->t('Author: '), - '$str_maintainer' => DI::l10n()->t('Maintainer: '), - '$screenshot' => $screenshot, - '$readme' => $readme, - - '$form_security_token' => parent::getFormSecurityToken("admin_themes"), - ]); + $theme = Strings::sanitizeFilePathItem($parameters['theme']); + if (!is_dir("view/theme/$theme")) { + notice(DI::l10n()->t("Item not found.")); + return ''; } - DI::baseUrl()->redirect('admin/themes'); + $isEnabled = in_array($theme, Theme::getAllowedList()); + if ($isEnabled) { + $status = "on"; + $action = DI::l10n()->t("Disable"); + } else { + $status = "off"; + $action = DI::l10n()->t("Enable"); + } + + if (!empty($_GET['action']) && $_GET['action'] == 'toggle') { + self::checkFormSecurityTokenRedirectOnError('/admin/themes', 'admin_themes', 't'); + + if ($isEnabled) { + Theme::uninstall($theme); + info(DI::l10n()->t('Theme %s disabled.', $theme)); + } elseif (Theme::install($theme)) { + info(DI::l10n()->t('Theme %s successfully enabled.', $theme)); + } else { + notice(DI::l10n()->t('Theme %s failed to install.', $theme)); + } + + DI::baseUrl()->redirect('admin/themes/' . $theme); + } + + $readme = null; + if (is_file("view/theme/$theme/README.md")) { + $readme = Markdown::convert(file_get_contents("view/theme/$theme/README.md"), false); + } elseif (is_file("view/theme/$theme/README")) { + $readme = "
" . file_get_contents("view/theme/$theme/README") . "
"; + } + + $admin_form = ''; + if (is_file("view/theme/$theme/config.php")) { + require_once "view/theme/$theme/config.php"; + + if (function_exists('theme_admin')) { + $admin_form = ''; + } + } + + $screenshot = [Theme::getScreenshot($theme), DI::l10n()->t('Screenshot')]; + if (!stristr($screenshot[0], $theme)) { + $screenshot = null; + } + + $t = Renderer::getMarkupTemplate('admin/addons/details.tpl'); + return Renderer::replaceMacros($t, [ + '$title' => DI::l10n()->t('Administration'), + '$page' => DI::l10n()->t('Themes'), + '$toggle' => DI::l10n()->t('Toggle'), + '$settings' => DI::l10n()->t('Settings'), + '$baseurl' => DI::baseUrl()->get(true), + '$addon' => $theme, + '$status' => $status, + '$action' => $action, + '$info' => Theme::getInfo($theme), + '$function' => 'themes', + '$admin_form' => $admin_form, + '$str_author' => DI::l10n()->t('Author: '), + '$str_maintainer' => DI::l10n()->t('Maintainer: '), + '$screenshot' => $screenshot, + '$readme' => $readme, + + '$form_security_token' => self::getFormSecurityToken("admin_themes"), + ]); } } diff --git a/src/Module/Admin/Themes/Embed.php b/src/Module/Admin/Themes/Embed.php index 675e33c84..71824c6cc 100644 --- a/src/Module/Admin/Themes/Embed.php +++ b/src/Module/Admin/Themes/Embed.php @@ -30,15 +30,9 @@ class Embed extends BaseAdmin { public static function init(array $parameters = []) { - $a = DI::app(); - - if ($a->argc > 2) { - // @TODO: Replace with parameter from router - $theme = $a->argv[2]; - $theme = Strings::sanitizeFilePathItem($theme); - if (is_file("view/theme/$theme/config.php")) { - $a->setCurrentTheme($theme); - } + $theme = Strings::sanitizeFilePathItem($parameters['theme']); + if (is_file("view/theme/$theme/config.php")) { + DI::app()->setCurrentTheme($theme); } } @@ -46,67 +40,49 @@ class Embed extends BaseAdmin { parent::post($parameters); - $a = DI::app(); - - if ($a->argc > 2) { - // @TODO: Replace with parameter from router - $theme = $a->argv[2]; - $theme = Strings::sanitizeFilePathItem($theme); - if (is_file("view/theme/$theme/config.php")) { + $theme = Strings::sanitizeFilePathItem($parameters['theme']); + if (is_file("view/theme/$theme/config.php")) { + require_once "view/theme/$theme/config.php"; + if (function_exists('theme_admin_post')) { self::checkFormSecurityTokenRedirectOnError('/admin/themes/' . $theme . '/embed?mode=minimal', 'admin_theme_settings'); - - require_once "view/theme/$theme/config.php"; - - if (function_exists('theme_admin_post')) { - theme_admin_post($a); - } + theme_admin_post(DI::app()); } - - info(DI::l10n()->t('Theme settings updated.')); - - if (DI::mode()->isAjax()) { - return; - } - - DI::baseUrl()->redirect('admin/themes/' . $theme . '/embed?mode=minimal'); } + + if (DI::mode()->isAjax()) { + return; + } + + DI::baseUrl()->redirect('admin/themes/' . $theme . '/embed?mode=minimal'); } public static function content(array $parameters = []) { parent::content($parameters); - $a = DI::app(); - - if ($a->argc > 2) { - // @TODO: Replace with parameter from router - $theme = $a->argv[2]; - $theme = Strings::sanitizeFilePathItem($theme); - if (!is_dir("view/theme/$theme")) { - notice(DI::l10n()->t('Unknown theme.')); - return ''; - } - - $admin_form = ''; - if (is_file("view/theme/$theme/config.php")) { - require_once "view/theme/$theme/config.php"; - - if (function_exists('theme_admin')) { - $admin_form = theme_admin($a); - } - } - - // Overrides normal theme style include to strip user param to show embedded theme settings - Renderer::$theme['stylesheet'] = 'view/theme/' . $theme . '/style.pcss'; - - $t = Renderer::getMarkupTemplate('admin/addons/embed.tpl'); - return Renderer::replaceMacros($t, [ - '$action' => '/admin/themes/' . $theme . '/embed?mode=minimal', - '$form' => $admin_form, - '$form_security_token' => parent::getFormSecurityToken("admin_theme_settings"), - ]); + $theme = Strings::sanitizeFilePathItem($parameters['theme']); + if (!is_dir("view/theme/$theme")) { + notice(DI::l10n()->t('Unknown theme.')); + return ''; } - return ''; + $admin_form = ''; + if (is_file("view/theme/$theme/config.php")) { + require_once "view/theme/$theme/config.php"; + + if (function_exists('theme_admin')) { + $admin_form = theme_admin(DI::app()); + } + } + + // Overrides normal theme style include to strip user param to show embedded theme settings + Renderer::$theme['stylesheet'] = 'view/theme/' . $theme . '/style.pcss'; + + $t = Renderer::getMarkupTemplate('admin/addons/embed.tpl'); + return Renderer::replaceMacros($t, [ + '$action' => '/admin/themes/' . $theme . '/embed?mode=minimal', + '$form' => $admin_form, + '$form_security_token' => self::getFormSecurityToken("admin_theme_settings"), + ]); } } diff --git a/src/Module/Admin/Users.php b/src/Module/Admin/Users.php index dca8c9c2e..f82f9f8c7 100644 --- a/src/Module/Admin/Users.php +++ b/src/Module/Admin/Users.php @@ -101,187 +101,186 @@ class Users extends BaseAdmin { parent::content($parameters); - $a = DI::app(); + $action = $parameters['action'] ?? ''; + $uid = $parameters['uid'] ?? 0; - if ($a->argc > 3) { - // @TODO: Replace with parameter from router - $action = $a->argv[2]; - $uid = $a->argv[3]; + if ($uid) { $user = User::getById($uid, ['username', 'blocked']); if (!DBA::isResult($user)) { notice('User not found' . EOL); DI::baseUrl()->redirect('admin/users'); return ''; // NOTREACHED } + } - switch ($action) { - case 'delete': - if (local_user() != $uid) { - parent::checkFormSecurityTokenRedirectOnError('/admin/users', 'admin_users', 't'); - // delete user - User::remove($uid); + switch ($action) { + case 'delete': + if (local_user() != $uid) { + self::checkFormSecurityTokenRedirectOnError('/admin/users', 'admin_users', 't'); + // delete user + User::remove($uid); - notice(DI::l10n()->t('User "%s" deleted', $user['username'])); - } else { - notice(DI::l10n()->t('You can\'t remove yourself')); + notice(DI::l10n()->t('User "%s" deleted', $user['username'])); + } else { + notice(DI::l10n()->t('You can\'t remove yourself')); + } + break; + case 'block': + self::checkFormSecurityTokenRedirectOnError('/admin/users', 'admin_users', 't'); + User::block($uid); + notice(DI::l10n()->t('User "%s" blocked', $user['username'])); + break; + case 'unblock': + self::checkFormSecurityTokenRedirectOnError('/admin/users', 'admin_users', 't'); + User::block($uid, false); + notice(DI::l10n()->t('User "%s" unblocked', $user['username'])); + break; + case 'allow': + self::checkFormSecurityTokenRedirectOnError('/admin/users', 'admin_users', 't'); + User::allow(Register::getPendingForUser($uid)['hash'] ?? ''); + notice(DI::l10n()->t('Account approved.')); + break; + case 'deny': + self::checkFormSecurityTokenRedirectOnError('/admin/users', 'admin_users', 't'); + User::deny(Register::getPendingForUser($uid)['hash'] ?? ''); + notice(DI::l10n()->t('Registration revoked')); + break; + default: + /* get pending */ + $pending = Register::getPending(); + + $pager = new Pager(DI::l10n(), DI::args()->getQueryString(), 100); + + $valid_orders = [ + 'name', + 'email', + 'register_date', + 'login_date', + 'last-item', + 'page-flags' + ]; + + $order = 'name'; + $order_direction = '+'; + if (!empty($_GET['o'])) { + $new_order = $_GET['o']; + if ($new_order[0] === '-') { + $order_direction = '-'; + $new_order = substr($new_order, 1); } - break; - case 'block': - parent::checkFormSecurityTokenRedirectOnError('/admin/users', 'admin_users', 't'); - User::block($uid); - notice(DI::l10n()->t('User "%s" blocked', $user['username'])); - break; - case 'unblock': - parent::checkFormSecurityTokenRedirectOnError('/admin/users', 'admin_users', 't'); - User::block($uid, false); - notice(DI::l10n()->t('User "%s" unblocked', $user['username'])); - break; - case 'allow': - parent::checkFormSecurityTokenRedirectOnError('/admin/users', 'admin_users', 't'); - User::allow(Register::getPendingForUser($uid)['hash'] ?? ''); - notice(DI::l10n()->t('Account approved.')); - break; - case 'deny': - parent::checkFormSecurityTokenRedirectOnError('/admin/users', 'admin_users', 't'); - User::deny(Register::getPendingForUser($uid)['hash'] ?? ''); - notice(DI::l10n()->t('Registration revoked')); - break; - } - DI::baseUrl()->redirect('admin/users'); + if (in_array($new_order, $valid_orders)) { + $order = $new_order; + } + } + + $users = User::getList($pager->getStart(), $pager->getItemsPerPage(), 'all', $order, ($order_direction == '-')); + + $adminlist = explode(',', str_replace(' ', '', DI::config()->get('config', 'admin_email'))); + $_setup_users = function ($e) use ($adminlist) { + $page_types = [ + User::PAGE_FLAGS_NORMAL => DI::l10n()->t('Normal Account Page'), + User::PAGE_FLAGS_SOAPBOX => DI::l10n()->t('Soapbox Page'), + User::PAGE_FLAGS_COMMUNITY => DI::l10n()->t('Public Forum'), + User::PAGE_FLAGS_FREELOVE => DI::l10n()->t('Automatic Friend Page'), + User::PAGE_FLAGS_PRVGROUP => DI::l10n()->t('Private Forum') + ]; + $account_types = [ + User::ACCOUNT_TYPE_PERSON => DI::l10n()->t('Personal Page'), + User::ACCOUNT_TYPE_ORGANISATION => DI::l10n()->t('Organisation Page'), + User::ACCOUNT_TYPE_NEWS => DI::l10n()->t('News Page'), + User::ACCOUNT_TYPE_COMMUNITY => DI::l10n()->t('Community Forum'), + User::ACCOUNT_TYPE_RELAY => DI::l10n()->t('Relay'), + ]; + + $e['page_flags_raw'] = $e['page-flags']; + $e['page-flags'] = $page_types[$e['page-flags']]; + + $e['account_type_raw'] = ($e['page_flags_raw'] == 0) ? $e['account-type'] : -1; + $e['account-type'] = ($e['page_flags_raw'] == 0) ? $account_types[$e['account-type']] : ''; + + $e['register_date'] = Temporal::getRelativeDate($e['register_date']); + $e['login_date'] = Temporal::getRelativeDate($e['login_date']); + $e['lastitem_date'] = Temporal::getRelativeDate($e['last-item']); + $e['is_admin'] = in_array($e['email'], $adminlist); + $e['is_deletable'] = (intval($e['uid']) != local_user()); + $e['deleted'] = ($e['account_removed'] ? Temporal::getRelativeDate($e['account_expires_on']) : False); + + return $e; + }; + + $tmp_users = array_map($_setup_users, $users); + + // Get rid of dashes in key names, Smarty3 can't handle them + // and extracting deleted users + + $deleted = []; + $users = []; + foreach ($tmp_users as $user) { + foreach ($user as $k => $v) { + $newkey = str_replace('-', '_', $k); + $user[$newkey] = $v; + } + + if ($user['deleted']) { + $deleted[] = $user; + } else { + $users[] = $user; + } + } + + $th_users = array_map(null, [DI::l10n()->t('Name'), DI::l10n()->t('Email'), DI::l10n()->t('Register date'), DI::l10n()->t('Last login'), DI::l10n()->t('Last public item'), DI::l10n()->t('Type')], $valid_orders); + + $t = Renderer::getMarkupTemplate('admin/users.tpl'); + $o = Renderer::replaceMacros($t, [ + // strings // + '$title' => DI::l10n()->t('Administration'), + '$page' => DI::l10n()->t('Users'), + '$submit' => DI::l10n()->t('Add User'), + '$select_all' => DI::l10n()->t('select all'), + '$h_pending' => DI::l10n()->t('User registrations waiting for confirm'), + '$h_deleted' => DI::l10n()->t('User waiting for permanent deletion'), + '$th_pending' => [DI::l10n()->t('Request date'), DI::l10n()->t('Name'), DI::l10n()->t('Email')], + '$no_pending' => DI::l10n()->t('No registrations.'), + '$pendingnotetext' => DI::l10n()->t('Note from the user'), + '$approve' => DI::l10n()->t('Approve'), + '$deny' => DI::l10n()->t('Deny'), + '$delete' => DI::l10n()->t('Delete'), + '$block' => DI::l10n()->t('Block'), + '$blocked' => DI::l10n()->t('User blocked'), + '$unblock' => DI::l10n()->t('Unblock'), + '$siteadmin' => DI::l10n()->t('Site admin'), + '$accountexpired' => DI::l10n()->t('Account expired'), + + '$h_users' => DI::l10n()->t('Users'), + '$h_newuser' => DI::l10n()->t('New User'), + '$th_deleted' => [DI::l10n()->t('Name'), DI::l10n()->t('Email'), DI::l10n()->t('Register date'), DI::l10n()->t('Last login'), DI::l10n()->t('Last public item'), DI::l10n()->t('Permanent deletion')], + '$th_users' => $th_users, + '$order_users' => $order, + '$order_direction_users' => $order_direction, + + '$confirm_delete_multi' => DI::l10n()->t('Selected users will be deleted!\n\nEverything these users had posted on this site will be permanently deleted!\n\nAre you sure?'), + '$confirm_delete' => DI::l10n()->t('The user {0} will be deleted!\n\nEverything this user has posted on this site will be permanently deleted!\n\nAre you sure?'), + + '$form_security_token' => self::getFormSecurityToken('admin_users'), + + // values // + '$baseurl' => DI::baseUrl()->get(true), + + '$pending' => $pending, + 'deleted' => $deleted, + '$users' => $users, + '$newusername' => ['new_user_name', DI::l10n()->t('Name'), '', DI::l10n()->t('Name of the new user.')], + '$newusernickname' => ['new_user_nickname', DI::l10n()->t('Nickname'), '', DI::l10n()->t('Nickname of the new user.')], + '$newuseremail' => ['new_user_email', DI::l10n()->t('Email'), '', DI::l10n()->t('Email address of the new user.'), '', '', 'email'], + ]); + + $o .= $pager->renderFull(DBA::count('user')); + + return $o; } - /* get pending */ - $pending = Register::getPending(); - - $pager = new Pager(DI::l10n(), DI::args()->getQueryString(), 100); - - $valid_orders = [ - 'name', - 'email', - 'register_date', - 'login_date', - 'last-item', - 'page-flags' - ]; - - $order = 'name'; - $order_direction = '+'; - if (!empty($_GET['o'])) { - $new_order = $_GET['o']; - if ($new_order[0] === '-') { - $order_direction = '-'; - $new_order = substr($new_order, 1); - } - - if (in_array($new_order, $valid_orders)) { - $order = $new_order; - } - } - - $users = User::getList($pager->getStart(), $pager->getItemsPerPage(), 'all', $order, ($order_direction == '-')); - - $adminlist = explode(',', str_replace(' ', '', DI::config()->get('config', 'admin_email'))); - $_setup_users = function ($e) use ($adminlist) { - $page_types = [ - User::PAGE_FLAGS_NORMAL => DI::l10n()->t('Normal Account Page'), - User::PAGE_FLAGS_SOAPBOX => DI::l10n()->t('Soapbox Page'), - User::PAGE_FLAGS_COMMUNITY => DI::l10n()->t('Public Forum'), - User::PAGE_FLAGS_FREELOVE => DI::l10n()->t('Automatic Friend Page'), - User::PAGE_FLAGS_PRVGROUP => DI::l10n()->t('Private Forum') - ]; - $account_types = [ - User::ACCOUNT_TYPE_PERSON => DI::l10n()->t('Personal Page'), - User::ACCOUNT_TYPE_ORGANISATION => DI::l10n()->t('Organisation Page'), - User::ACCOUNT_TYPE_NEWS => DI::l10n()->t('News Page'), - User::ACCOUNT_TYPE_COMMUNITY => DI::l10n()->t('Community Forum'), - User::ACCOUNT_TYPE_RELAY => DI::l10n()->t('Relay'), - ]; - - $e['page_flags_raw'] = $e['page-flags']; - $e['page-flags'] = $page_types[$e['page-flags']]; - - $e['account_type_raw'] = ($e['page_flags_raw'] == 0) ? $e['account-type'] : -1; - $e['account-type'] = ($e['page_flags_raw'] == 0) ? $account_types[$e['account-type']] : ''; - - $e['register_date'] = Temporal::getRelativeDate($e['register_date']); - $e['login_date'] = Temporal::getRelativeDate($e['login_date']); - $e['lastitem_date'] = Temporal::getRelativeDate($e['last-item']); - $e['is_admin'] = in_array($e['email'], $adminlist); - $e['is_deletable'] = (intval($e['uid']) != local_user()); - $e['deleted'] = ($e['account_removed'] ? Temporal::getRelativeDate($e['account_expires_on']) : False); - - return $e; - }; - - $tmp_users = array_map($_setup_users, $users); - - // Get rid of dashes in key names, Smarty3 can't handle them - // and extracting deleted users - - $deleted = []; - $users = []; - foreach ($tmp_users as $user) { - foreach ($user as $k => $v) { - $newkey = str_replace('-', '_', $k); - $user[$newkey] = $v; - } - - if ($user['deleted']) { - $deleted[] = $user; - } else { - $users[] = $user; - } - } - - $th_users = array_map(null, [DI::l10n()->t('Name'), DI::l10n()->t('Email'), DI::l10n()->t('Register date'), DI::l10n()->t('Last login'), DI::l10n()->t('Last public item'), DI::l10n()->t('Type')], $valid_orders); - - $t = Renderer::getMarkupTemplate('admin/users.tpl'); - $o = Renderer::replaceMacros($t, [ - // strings // - '$title' => DI::l10n()->t('Administration'), - '$page' => DI::l10n()->t('Users'), - '$submit' => DI::l10n()->t('Add User'), - '$select_all' => DI::l10n()->t('select all'), - '$h_pending' => DI::l10n()->t('User registrations waiting for confirm'), - '$h_deleted' => DI::l10n()->t('User waiting for permanent deletion'), - '$th_pending' => [DI::l10n()->t('Request date'), DI::l10n()->t('Name'), DI::l10n()->t('Email')], - '$no_pending' => DI::l10n()->t('No registrations.'), - '$pendingnotetext' => DI::l10n()->t('Note from the user'), - '$approve' => DI::l10n()->t('Approve'), - '$deny' => DI::l10n()->t('Deny'), - '$delete' => DI::l10n()->t('Delete'), - '$block' => DI::l10n()->t('Block'), - '$blocked' => DI::l10n()->t('User blocked'), - '$unblock' => DI::l10n()->t('Unblock'), - '$siteadmin' => DI::l10n()->t('Site admin'), - '$accountexpired' => DI::l10n()->t('Account expired'), - - '$h_users' => DI::l10n()->t('Users'), - '$h_newuser' => DI::l10n()->t('New User'), - '$th_deleted' => [DI::l10n()->t('Name'), DI::l10n()->t('Email'), DI::l10n()->t('Register date'), DI::l10n()->t('Last login'), DI::l10n()->t('Last public item'), DI::l10n()->t('Permanent deletion')], - '$th_users' => $th_users, - '$order_users' => $order, - '$order_direction_users' => $order_direction, - - '$confirm_delete_multi' => DI::l10n()->t('Selected users will be deleted!\n\nEverything these users had posted on this site will be permanently deleted!\n\nAre you sure?'), - '$confirm_delete' => DI::l10n()->t('The user {0} will be deleted!\n\nEverything this user has posted on this site will be permanently deleted!\n\nAre you sure?'), - - '$form_security_token' => parent::getFormSecurityToken('admin_users'), - - // values // - '$baseurl' => DI::baseUrl()->get(true), - - '$pending' => $pending, - 'deleted' => $deleted, - '$users' => $users, - '$newusername' => ['new_user_name', DI::l10n()->t('Name'), '', DI::l10n()->t('Name of the new user.')], - '$newusernickname' => ['new_user_nickname', DI::l10n()->t('Nickname'), '', DI::l10n()->t('Nickname of the new user.')], - '$newuseremail' => ['new_user_email', DI::l10n()->t('Email'), '', DI::l10n()->t('Email address of the new user.'), '', '', 'email'], - ]); - - $o .= $pager->renderFull(DBA::count('user')); - - return $o; + DI::baseUrl()->redirect('admin/users'); + return ''; } } diff --git a/static/routes.config.php b/static/routes.config.php index 074c1f571..c7ef4bd78 100644 --- a/static/routes.config.php +++ b/static/routes.config.php @@ -73,9 +73,7 @@ return [ '/blocklist/contact' => [Module\Admin\Blocklist\Contact::class, [R::GET, R::POST]], '/blocklist/server' => [Module\Admin\Blocklist\Server::class, [R::GET, R::POST]], - '/dbsync[/check]' => [Module\Admin\DBSync::class, [R::GET]], - '/dbsync/{update:\d+}' => [Module\Admin\DBSync::class, [R::GET]], - '/dbsync/mark/{update:\d+}' => [Module\Admin\DBSync::class, [R::GET]], + '/dbsync[/{action}[/{update:\d+}]]' => [Module\Admin\DBSync::class, [R::GET]], '/features' => [Module\Admin\Features::class, [R::GET, R::POST]], '/federation' => [Module\Admin\Federation::class, [R::GET]], @@ -88,7 +86,7 @@ return [ '/phpinfo' => [Module\Admin\PhpInfo::class, [R::GET]], - '/queue[/deferred]' => [Module\Admin\Queue::class, [R::GET]], + '/queue[/{status}]' => [Module\Admin\Queue::class, [R::GET]], '/site' => [Module\Admin\Site::class, [R::GET, R::POST]], diff --git a/view/templates/admin/dbsync/failed_updates.tpl b/view/templates/admin/dbsync/failed_updates.tpl index f75f266c7..f1a752290 100644 --- a/view/templates/admin/dbsync/failed_updates.tpl +++ b/view/templates/admin/dbsync/failed_updates.tpl @@ -10,7 +10,7 @@
From 9bc2c5a52e8fd0bf09774a36c40174ae758f6f98 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Tue, 8 Sep 2020 10:42:25 -0400 Subject: [PATCH 2/3] Normalize use of form security tokens in Admin modules # Conflicts: # src/Module/Admin/Logs/Settings.php --- src/Module/Admin/Addons/Details.php | 6 ++-- src/Module/Admin/Addons/Index.php | 4 +-- src/Module/Admin/Blocklist/Contact.php | 6 ++-- src/Module/Admin/Blocklist/Server.php | 4 +-- src/Module/Admin/Features.php | 4 +-- src/Module/Admin/Item/Delete.php | 4 +-- src/Module/Admin/Logs/Settings.php | 37 +++++++++++++------------ src/Module/Admin/Site.php | 2 +- src/Module/Admin/Themes/Index.php | 4 +-- src/Module/Admin/Tos.php | 6 ++-- src/Module/Admin/Users.php | 4 +-- view/templates/admin/addons/details.tpl | 1 + 12 files changed, 43 insertions(+), 39 deletions(-) diff --git a/src/Module/Admin/Addons/Details.php b/src/Module/Admin/Addons/Details.php index 139275cfe..85b17130c 100644 --- a/src/Module/Admin/Addons/Details.php +++ b/src/Module/Admin/Addons/Details.php @@ -42,6 +42,8 @@ class Details extends BaseAdmin include_once 'addon/' . $addon . '/' . $addon . '.php'; if (function_exists($addon . '_addon_admin_post')) { + self::checkFormSecurityTokenRedirectOnError($redirect, 'admin_addons_details'); + $func = $addon . '_addon_admin_post'; $func(DI::app()); } @@ -66,7 +68,7 @@ class Details extends BaseAdmin } if (($_GET['action'] ?? '') == 'toggle') { - self::checkFormSecurityTokenRedirectOnError('/admin/addons', 'admin_addons', 't'); + self::checkFormSecurityTokenRedirectOnError('/admin/addons', 'admin_addons_details', 't'); // Toggle addon status if (Addon::isEnabled($addon)) { @@ -124,7 +126,7 @@ class Details extends BaseAdmin '$screenshot' => '', '$readme' => $readme, - '$form_security_token' => self::getFormSecurityToken('admin_addons'), + '$form_security_token' => self::getFormSecurityToken('admin_addons_details'), ]); } } diff --git a/src/Module/Admin/Addons/Index.php b/src/Module/Admin/Addons/Index.php index 3049cdc6a..959f9d04a 100644 --- a/src/Module/Admin/Addons/Index.php +++ b/src/Module/Admin/Addons/Index.php @@ -34,7 +34,7 @@ class Index extends BaseAdmin // reload active themes if (!empty($_GET['action'])) { - parent::checkFormSecurityTokenRedirectOnError('/admin/addons', 'admin_addons', 't'); + self::checkFormSecurityTokenRedirectOnError('/admin/addons', 'admin_addons', 't'); switch ($_GET['action']) { case 'reload': @@ -73,7 +73,7 @@ class Index extends BaseAdmin '$addons' => $addons, '$pcount' => count($addons), '$noplugshint' => DI::l10n()->t('There are currently no addons available on your node. You can find the official addon repository at %1$s and might find other interesting addons in the open addon registry at %2$s', 'https://github.com/friendica/friendica-addons', 'http://addons.friendi.ca'), - '$form_security_token' => parent::getFormSecurityToken('admin_addons'), + '$form_security_token' => self::getFormSecurityToken('admin_addons'), ]); } } diff --git a/src/Module/Admin/Blocklist/Contact.php b/src/Module/Admin/Blocklist/Contact.php index 889362323..5a7d138b2 100644 --- a/src/Module/Admin/Blocklist/Contact.php +++ b/src/Module/Admin/Blocklist/Contact.php @@ -34,12 +34,12 @@ class Contact extends BaseAdmin { parent::post($parameters); + self::checkFormSecurityTokenRedirectOnError('/admin/blocklist/contact', 'admin_contactblock'); + $contact_url = $_POST['contact_url'] ?? ''; $block_reason = $_POST['contact_block_reason'] ?? ''; $contacts = $_POST['contacts'] ?? []; - parent::checkFormSecurityTokenRedirectOnError('/admin/blocklist/contact', 'admin_contactblock'); - if (!empty($_POST['page_contactblock_block'])) { $contact_id = Model\Contact::getIdForURL($contact_url); if ($contact_id) { @@ -89,7 +89,7 @@ class Contact extends BaseAdmin '$h_newblock' => DI::l10n()->t('Block New Remote Contact'), '$th_contacts' => [DI::l10n()->t('Photo'), DI::l10n()->t('Name'), DI::l10n()->t('Reason')], - '$form_security_token' => parent::getFormSecurityToken('admin_contactblock'), + '$form_security_token' => self::getFormSecurityToken('admin_contactblock'), // values // '$baseurl' => DI::baseUrl()->get(true), diff --git a/src/Module/Admin/Blocklist/Server.php b/src/Module/Admin/Blocklist/Server.php index 4f19ca361..b4be591e7 100644 --- a/src/Module/Admin/Blocklist/Server.php +++ b/src/Module/Admin/Blocklist/Server.php @@ -36,7 +36,7 @@ class Server extends BaseAdmin return; } - parent::checkFormSecurityTokenRedirectOnError('/admin/blocklist/server', 'admin_blocklist'); + self::checkFormSecurityTokenRedirectOnError('/admin/blocklist/server', 'admin_blocklist'); if (!empty($_POST['page_blocklist_save'])) { // Add new item to blocklist @@ -108,7 +108,7 @@ class Server extends BaseAdmin '$entries' => $blocklistform, '$baseurl' => DI::baseUrl()->get(true), '$confirm_delete' => DI::l10n()->t('Delete entry from blocklist?'), - '$form_security_token' => parent::getFormSecurityToken("admin_blocklist") + '$form_security_token' => self::getFormSecurityToken("admin_blocklist") ]); } } diff --git a/src/Module/Admin/Features.php b/src/Module/Admin/Features.php index a97bc0e7b..51ba9140e 100644 --- a/src/Module/Admin/Features.php +++ b/src/Module/Admin/Features.php @@ -32,7 +32,7 @@ class Features extends BaseAdmin { parent::post($parameters); - parent::checkFormSecurityTokenRedirectOnError('/admin/features', 'admin_manage_features'); + self::checkFormSecurityTokenRedirectOnError('/admin/features', 'admin_manage_features'); $features = Feature::get(false); @@ -80,7 +80,7 @@ class Features extends BaseAdmin $tpl = Renderer::getMarkupTemplate('admin/features.tpl'); $o = Renderer::replaceMacros($tpl, [ - '$form_security_token' => parent::getFormSecurityToken("admin_manage_features"), + '$form_security_token' => self::getFormSecurityToken("admin_manage_features"), '$baseurl' => DI::baseUrl()->get(true), '$title' => DI::l10n()->t('Manage Additional Features'), '$features' => $features, diff --git a/src/Module/Admin/Item/Delete.php b/src/Module/Admin/Item/Delete.php index 0ad20f97c..028e228d3 100644 --- a/src/Module/Admin/Item/Delete.php +++ b/src/Module/Admin/Item/Delete.php @@ -37,7 +37,7 @@ class Delete extends BaseAdmin return; } - parent::checkFormSecurityTokenRedirectOnError('/admin/item/delete', 'admin_deleteitem'); + self::checkFormSecurityTokenRedirectOnError('/admin/item/delete', 'admin_deleteitem'); if (!empty($_POST['page_deleteitem_submit'])) { $guid = trim(Strings::escapeTags($_POST['deleteitemguid'])); @@ -68,7 +68,7 @@ class Delete extends BaseAdmin '$intro1' => DI::l10n()->t('On this page you can delete an item from your node. If the item is a top level posting, the entire thread will be deleted.'), '$intro2' => DI::l10n()->t('You need to know the GUID of the item. You can find it e.g. by looking at the display URL. The last part of http://example.com/display/123456 is the GUID, here 123456.'), '$deleteitemguid' => ['deleteitemguid', DI::l10n()->t("GUID"), '', DI::l10n()->t("The GUID of the item you want to delete."), 'required', 'autofocus'], - '$form_security_token' => parent::getFormSecurityToken("admin_deleteitem") + '$form_security_token' => self::getFormSecurityToken("admin_deleteitem") ]); } } diff --git a/src/Module/Admin/Logs/Settings.php b/src/Module/Admin/Logs/Settings.php index 5158108e4..0b5993798 100644 --- a/src/Module/Admin/Logs/Settings.php +++ b/src/Module/Admin/Logs/Settings.php @@ -33,25 +33,26 @@ class Settings extends BaseAdmin { parent::post($parameters); - if (!empty($_POST['page_logs'])) { - parent::checkFormSecurityTokenRedirectOnError('/admin/logs', 'admin_logs'); - - $logfile = (!empty($_POST['logfile']) ? Strings::escapeTags(trim($_POST['logfile'])) : ''); - $debugging = !empty($_POST['debugging']); - $loglevel = ($_POST['loglevel'] ?? '') ?: LogLevel::ERROR; - - if (is_file($logfile) && - !is_writeable($logfile)) { - notice(DI::l10n()->t('The logfile \'%s\' is not writable. No logging possible', $logfile)); - return; - } - - DI::config()->set('system', 'logfile', $logfile); - DI::config()->set('system', 'debugging', $debugging); - DI::config()->set('system', 'loglevel', $loglevel); + if (empty($_POST['page_logs'])) { + return; } - info(DI::l10n()->t("Log settings updated.")); + self::checkFormSecurityTokenRedirectOnError('/admin/logs', 'admin_logs'); + + $logfile = (!empty($_POST['logfile']) ? Strings::escapeTags(trim($_POST['logfile'])) : ''); + $debugging = !empty($_POST['debugging']); + $loglevel = ($_POST['loglevel'] ?? '') ?: LogLevel::ERROR; + + if (is_file($logfile) && + !is_writeable($logfile)) { + notice(DI::l10n()->t('The logfile \'%s\' is not writable. No logging possible', $logfile)); + return; + } + + DI::config()->set('system', 'logfile', $logfile); + DI::config()->set('system', 'debugging', $debugging); + DI::config()->set('system', 'loglevel', $loglevel); + DI::baseUrl()->redirect('admin/logs'); } @@ -86,7 +87,7 @@ class Settings extends BaseAdmin '$debugging' => ['debugging', DI::l10n()->t("Enable Debugging"), DI::config()->get('system', 'debugging'), ""], '$logfile' => ['logfile', DI::l10n()->t("Log file"), DI::config()->get('system', 'logfile'), DI::l10n()->t("Must be writable by web server. Relative to your Friendica top-level directory.")], '$loglevel' => ['loglevel', DI::l10n()->t("Log level"), DI::config()->get('system', 'loglevel'), "", $log_choices], - '$form_security_token' => parent::getFormSecurityToken("admin_logs"), + '$form_security_token' => self::getFormSecurityToken("admin_logs"), '$phpheader' => DI::l10n()->t("PHP logging"), '$phphint' => DI::l10n()->t("To temporarily enable logging of PHP errors and warnings you can prepend the following to the index.php file of your installation. The filename set in the 'error_log' line is relative to the friendica top-level directory and must be writeable by the web server. The option '1' for 'log_errors' and 'display_errors' is to enable these options, set to '0' to disable them."), '$phplogcode' => "error_reporting(E_ERROR | E_WARNING | E_PARSE);\nini_set('error_log','php.out');\nini_set('log_errors','1');\nini_set('display_errors', '1');", diff --git a/src/Module/Admin/Site.php b/src/Module/Admin/Site.php index 2e16cc657..6380f3d93 100644 --- a/src/Module/Admin/Site.php +++ b/src/Module/Admin/Site.php @@ -718,7 +718,7 @@ class Site extends BaseAdmin '$relay_server_tags' => ['relay_server_tags', DI::l10n()->t('Server tags'), DI::config()->get('system', 'relay_server_tags'), DI::l10n()->t('Comma separated list of tags for the "tags" subscription.')], '$relay_user_tags' => ['relay_user_tags', DI::l10n()->t('Allow user tags'), DI::config()->get('system', 'relay_user_tags', true), DI::l10n()->t('If enabled, the tags from the saved searches will used for the "tags" subscription in addition to the "relay_server_tags".')], - '$form_security_token' => parent::getFormSecurityToken('admin_site'), + '$form_security_token' => self::getFormSecurityToken('admin_site'), '$relocate_button' => DI::l10n()->t('Start Relocation'), ]); } diff --git a/src/Module/Admin/Themes/Index.php b/src/Module/Admin/Themes/Index.php index 955ddadc7..f25d64b47 100644 --- a/src/Module/Admin/Themes/Index.php +++ b/src/Module/Admin/Themes/Index.php @@ -37,7 +37,7 @@ class Index extends BaseAdmin // reload active themes if (!empty($_GET['action'])) { - parent::checkFormSecurityTokenRedirectOnError(DI::baseUrl()->get() . '/admin/themes', 'admin_themes', 't'); + self::checkFormSecurityTokenRedirectOnError(DI::baseUrl()->get() . '/admin/themes', 'admin_themes', 't'); switch ($_GET['action']) { case 'reload': @@ -119,7 +119,7 @@ class Index extends BaseAdmin '$noplugshint' => DI::l10n()->t('No themes found on the system. They should be placed in %1$s', '/view/themes'), '$experimental' => DI::l10n()->t('[Experimental]'), '$unsupported' => DI::l10n()->t('[Unsupported]'), - '$form_security_token' => parent::getFormSecurityToken('admin_themes'), + '$form_security_token' => self::getFormSecurityToken('admin_themes'), ]); } } diff --git a/src/Module/Admin/Tos.php b/src/Module/Admin/Tos.php index 811a0eb25..fef199c35 100644 --- a/src/Module/Admin/Tos.php +++ b/src/Module/Admin/Tos.php @@ -31,12 +31,12 @@ class Tos extends BaseAdmin { parent::post($parameters); - parent::checkFormSecurityTokenRedirectOnError('/admin/tos', 'admin_tos'); - if (empty($_POST['page_tos'])) { return; } + self::checkFormSecurityTokenRedirectOnError('/admin/tos', 'admin_tos'); + $displaytos = !empty($_POST['displaytos']); $displayprivstatement = !empty($_POST['displayprivstatement']); $tostext = (!empty($_POST['tostext']) ? strip_tags(trim($_POST['tostext'])) : ''); @@ -64,7 +64,7 @@ class Tos extends BaseAdmin '$preview' => DI::l10n()->t('Privacy Statement Preview'), '$privtext' => $tos->privacy_complete, '$tostext' => ['tostext', DI::l10n()->t('The Terms of Service'), DI::config()->get('system', 'tostext'), DI::l10n()->t('Enter the Terms of Service for your node here. You can use BBCode. Headers of sections should be [h2] and below.')], - '$form_security_token' => parent::getFormSecurityToken('admin_tos'), + '$form_security_token' => self::getFormSecurityToken('admin_tos'), '$submit' => DI::l10n()->t('Save Settings'), ]); } diff --git a/src/Module/Admin/Users.php b/src/Module/Admin/Users.php index f82f9f8c7..751b618af 100644 --- a/src/Module/Admin/Users.php +++ b/src/Module/Admin/Users.php @@ -36,6 +36,8 @@ class Users extends BaseAdmin { parent::post($parameters); + self::checkFormSecurityTokenRedirectOnError('/admin/users', 'admin_users'); + $pending = $_POST['pending'] ?? []; $users = $_POST['user'] ?? []; $nu_name = $_POST['new_user_name'] ?? ''; @@ -43,8 +45,6 @@ class Users extends BaseAdmin $nu_email = $_POST['new_user_email'] ?? ''; $nu_language = DI::config()->get('system', 'language'); - parent::checkFormSecurityTokenRedirectOnError('/admin/users', 'admin_users'); - if ($nu_name !== '' && $nu_email !== '' && $nu_nickname !== '') { try { User::createMinimal($nu_name, $nu_email, $nu_nickname, $nu_language); diff --git a/view/templates/admin/addons/details.tpl b/view/templates/admin/addons/details.tpl index f5bb165e0..81625fecf 100644 --- a/view/templates/admin/addons/details.tpl +++ b/view/templates/admin/addons/details.tpl @@ -24,6 +24,7 @@ {{if $admin_form}}

{{$settings}}

+ {{$admin_form nofilter}}
{{/if}} From 3efa8648c565f7a4bd24436a55336e7ca70fcb06 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Tue, 8 Sep 2020 10:44:27 -0400 Subject: [PATCH 3/3] Fix security vulnerability in admin modules - The Module\BaseAdmin::post method checked credentials but didn't abort the process when it failed - Created Module\BaseAdmin::checkAdminAccess method --- src/Module/Admin/Addons/Details.php | 2 +- src/Module/Admin/Blocklist/Contact.php | 2 +- src/Module/Admin/Blocklist/Server.php | 2 +- src/Module/Admin/Features.php | 2 +- src/Module/Admin/Item/Delete.php | 2 +- src/Module/Admin/Logs/Settings.php | 2 +- src/Module/Admin/PhpInfo.php | 2 +- src/Module/Admin/Site.php | 2 +- src/Module/Admin/Themes/Embed.php | 2 +- src/Module/Admin/Tos.php | 2 +- src/Module/Admin/Users.php | 2 +- src/Module/BaseAdmin.php | 43 +++++++++++--------------- 12 files changed, 29 insertions(+), 36 deletions(-) diff --git a/src/Module/Admin/Addons/Details.php b/src/Module/Admin/Addons/Details.php index 85b17130c..4c1fe2df9 100644 --- a/src/Module/Admin/Addons/Details.php +++ b/src/Module/Admin/Addons/Details.php @@ -32,7 +32,7 @@ class Details extends BaseAdmin { public static function post(array $parameters = []) { - parent::post($parameters); + self::checkAdminAccess(); $addon = Strings::sanitizeFilePathItem($parameters['addon']); diff --git a/src/Module/Admin/Blocklist/Contact.php b/src/Module/Admin/Blocklist/Contact.php index 5a7d138b2..c4eedc5a8 100644 --- a/src/Module/Admin/Blocklist/Contact.php +++ b/src/Module/Admin/Blocklist/Contact.php @@ -32,7 +32,7 @@ class Contact extends BaseAdmin { public static function post(array $parameters = []) { - parent::post($parameters); + self::checkAdminAccess(); self::checkFormSecurityTokenRedirectOnError('/admin/blocklist/contact', 'admin_contactblock'); diff --git a/src/Module/Admin/Blocklist/Server.php b/src/Module/Admin/Blocklist/Server.php index b4be591e7..1290662f2 100644 --- a/src/Module/Admin/Blocklist/Server.php +++ b/src/Module/Admin/Blocklist/Server.php @@ -30,7 +30,7 @@ class Server extends BaseAdmin { public static function post(array $parameters = []) { - parent::post($parameters); + self::checkAdminAccess(); if (empty($_POST['page_blocklist_save']) && empty($_POST['page_blocklist_edit'])) { return; diff --git a/src/Module/Admin/Features.php b/src/Module/Admin/Features.php index 51ba9140e..5054da3fb 100644 --- a/src/Module/Admin/Features.php +++ b/src/Module/Admin/Features.php @@ -30,7 +30,7 @@ class Features extends BaseAdmin { public static function post(array $parameters = []) { - parent::post($parameters); + self::checkAdminAccess(); self::checkFormSecurityTokenRedirectOnError('/admin/features', 'admin_manage_features'); diff --git a/src/Module/Admin/Item/Delete.php b/src/Module/Admin/Item/Delete.php index 028e228d3..9e2bc90d9 100644 --- a/src/Module/Admin/Item/Delete.php +++ b/src/Module/Admin/Item/Delete.php @@ -31,7 +31,7 @@ class Delete extends BaseAdmin { public static function post(array $parameters = []) { - parent::post($parameters); + self::checkAdminAccess(); if (empty($_POST['page_deleteitem_submit'])) { return; diff --git a/src/Module/Admin/Logs/Settings.php b/src/Module/Admin/Logs/Settings.php index 0b5993798..7730b487d 100644 --- a/src/Module/Admin/Logs/Settings.php +++ b/src/Module/Admin/Logs/Settings.php @@ -31,7 +31,7 @@ class Settings extends BaseAdmin { public static function post(array $parameters = []) { - parent::post($parameters); + self::checkAdminAccess(); if (empty($_POST['page_logs'])) { return; diff --git a/src/Module/Admin/PhpInfo.php b/src/Module/Admin/PhpInfo.php index f282e1008..61a004618 100644 --- a/src/Module/Admin/PhpInfo.php +++ b/src/Module/Admin/PhpInfo.php @@ -27,7 +27,7 @@ class PhpInfo extends BaseAdmin { public static function rawContent(array $parameters = []) { - parent::rawContent($parameters); + self::checkAdminAccess(); phpinfo(); exit(); diff --git a/src/Module/Admin/Site.php b/src/Module/Admin/Site.php index 6380f3d93..9f3905da5 100644 --- a/src/Module/Admin/Site.php +++ b/src/Module/Admin/Site.php @@ -43,7 +43,7 @@ class Site extends BaseAdmin { public static function post(array $parameters = []) { - parent::post($parameters); + self::checkAdminAccess(); self::checkFormSecurityTokenRedirectOnError('/admin/site', 'admin_site'); diff --git a/src/Module/Admin/Themes/Embed.php b/src/Module/Admin/Themes/Embed.php index 71824c6cc..a308b43cb 100644 --- a/src/Module/Admin/Themes/Embed.php +++ b/src/Module/Admin/Themes/Embed.php @@ -38,7 +38,7 @@ class Embed extends BaseAdmin public static function post(array $parameters = []) { - parent::post($parameters); + self::checkAdminAccess(); $theme = Strings::sanitizeFilePathItem($parameters['theme']); if (is_file("view/theme/$theme/config.php")) { diff --git a/src/Module/Admin/Tos.php b/src/Module/Admin/Tos.php index fef199c35..aac81264b 100644 --- a/src/Module/Admin/Tos.php +++ b/src/Module/Admin/Tos.php @@ -29,7 +29,7 @@ class Tos extends BaseAdmin { public static function post(array $parameters = []) { - parent::post($parameters); + self::checkAdminAccess(); if (empty($_POST['page_tos'])) { return; diff --git a/src/Module/Admin/Users.php b/src/Module/Admin/Users.php index 751b618af..74fee7230 100644 --- a/src/Module/Admin/Users.php +++ b/src/Module/Admin/Users.php @@ -34,7 +34,7 @@ class Users extends BaseAdmin { public static function post(array $parameters = []) { - parent::post($parameters); + self::checkAdminAccess(); self::checkFormSecurityTokenRedirectOnError('/admin/users', 'admin_users'); diff --git a/src/Module/BaseAdmin.php b/src/Module/BaseAdmin.php index a7b38a503..67de97f85 100644 --- a/src/Module/BaseAdmin.php +++ b/src/Module/BaseAdmin.php @@ -26,7 +26,7 @@ use Friendica\Core\Addon; use Friendica\Core\Renderer; use Friendica\Core\Session; use Friendica\DI; -use Friendica\Network\HTTPException\ForbiddenException; +use Friendica\Network\HTTPException; require_once 'boot.php'; @@ -42,42 +42,35 @@ require_once 'boot.php'; */ abstract class BaseAdmin extends BaseModule { - public static function post(array $parameters = []) + /** + * @param bool $interactive + * @throws HTTPException\ForbiddenException + * @throws HTTPException\InternalServerErrorException + */ + public static function checkAdminAccess(bool $interactive = false) { - if (!is_site_admin()) { - return; + if (!local_user()) { + if ($interactive) { + notice(DI::l10n()->t('Please login to continue.')); + Session::set('return_path', DI::args()->getQueryString()); + DI::baseUrl()->redirect('login'); + } else { + throw new HTTPException\UnauthorizedException(DI::l10n()->t('Please login to continue.')); + } } - // do not allow a page manager to access the admin panel at all. - if (!empty($_SESSION['submanage'])) { - return; - } - } - - public static function rawContent(array $parameters = []) - { if (!is_site_admin()) { - return ''; + throw new HTTPException\ForbiddenException(DI::l10n()->t('You don\'t have access to administration pages.')); } if (!empty($_SESSION['submanage'])) { - return ''; + throw new HTTPException\ForbiddenException(DI::l10n()->t('Submanaged account can\'t access the administation pages. Please log back in as the main account.')); } - - return ''; } public static function content(array $parameters = []) { - if (!is_site_admin()) { - notice(DI::l10n()->t('Please login to continue.')); - Session::set('return_path', DI::args()->getQueryString()); - DI::baseUrl()->redirect('login'); - } - - if (!empty($_SESSION['submanage'])) { - throw new ForbiddenException(DI::l10n()->t('Submanaged account can\'t access the administation pages. Please log back in as the main account.')); - } + self::checkAdminAccess(true); // Header stuff DI::page()['htmlhead'] .= Renderer::replaceMacros(Renderer::getMarkupTemplate('admin/settings_head.tpl'), []);