Greatly improve login related actions for Admin

* Better isolate admin to prevent session related vulnerabilities
* Removed support for custom login redirects for improved security
* Shorten forgot password link lifetime from 7 days to 1 hour
* Fixed login related pages being accessible from admin when user has logged in
* Fixed admin user creation and password reset allowing unsafe passwords
* Fixed missing validation when registering the first admin user
* Fixed reset password email not to have session specific token in it
This commit is contained in:
Matias Griese
2021-03-26 14:39:37 +02:00
parent e14e72958f
commit aa4f80eec1
22 changed files with 1930 additions and 663 deletions

View File

@@ -26,7 +26,6 @@ use Grav\Common\Themes;
use Grav\Common\Uri;
use Grav\Common\User\Interfaces\UserCollectionInterface;
use Grav\Common\User\Interfaces\UserInterface;
use Grav\Common\User\User;
use Grav\Common\Utils;
use Grav\Framework\Acl\Action;
use Grav\Framework\Acl\Permissions;
@@ -40,6 +39,7 @@ use Grav\Plugin\AdminPlugin;
use Grav\Plugin\Login\Login;
use Grav\Plugin\Login\TwoFactorAuth\TwoFactorAuth;
use PicoFeed\Parser\MalformedXmlException;
use Psr\Http\Message\ServerRequestInterface;
use RocketTheme\Toolbox\Event\Event;
use RocketTheme\Toolbox\File\File;
use RocketTheme\Toolbox\File\JsonFile;
@@ -52,72 +52,63 @@ use PicoFeed\Reader\Reader;
define('LOGIN_REDIRECT_COOKIE', 'grav-login-redirect');
/**
* Class Admin
* @package Grav\Plugin\Admin
*/
class Admin
{
/** @var int */
public const DEBUG = 1;
/** @var int */
public const MEDIA_PAGINATION_INTERVAL = 20;
/** @var string */
public const TMP_COOKIE_NAME = 'tmp-admin-message';
/** @var Grav */
public $grav;
/** @var ServerRequestInterface|null */
public $request;
/** @var AdminForm */
public $form;
/** @var string */
public $base;
/** @var string */
public $location;
/** @var string */
public $route;
/** @var User */
/** @var UserInterface */
public $user;
/** @var array */
public $forgot;
/** @var string */
public $task;
/** @var array */
public $json_response;
/** @var Collection */
public $collection;
/** @var bool */
public $multilang;
/** @var string */
public $language;
/** @var array */
public $languages_enabled = [];
/** @var Uri $uri */
protected $uri;
/** @var array */
protected $pages = [];
/** @var Session */
protected $session;
/** @var Data\Blueprints */
protected $blueprints;
/** @var GPM */
protected $gpm;
/** @var int */
protected $pages_count;
/** @var bool */
protected $load_additional_files_in_background = false;
/** @var bool */
protected $loading_additional_files_in_background = false;
/** @var array */
protected $temp_messages = [];
@@ -127,7 +118,7 @@ class Admin
* @param Grav $grav
* @param string $base
* @param string $location
* @param string $route
* @param string|null $route
*/
public function __construct(Grav $grav, $base, $location, $route)
{
@@ -137,7 +128,7 @@ class Admin
$this->grav = $grav;
$this->base = $base;
$this->location = $location;
$this->route = $route;
$this->route = $route ?? '';
$this->uri = $grav['uri'];
$this->session = $grav['session'];
@@ -176,7 +167,7 @@ class Admin
$this->languages_enabled = (array)$this->grav['config']->get('system.languages.supported', []);
//Set the currently active language for the admin
$languageCode = $this->grav['uri']->param('lang');
$languageCode = $this->uri->param('lang');
if (null === $languageCode && !$this->session->admin_lang) {
$this->session->admin_lang = $language->getActive() ?? '';
}
@@ -190,7 +181,8 @@ class Admin
/**
* @param string $message
* @param array $data
* @param array|object $data
* @return void
*/
public static function addDebugMessage(string $message, $data = [])
{
@@ -199,6 +191,9 @@ class Admin
$debugger->addMessage($message, 'debug', $data);
}
/**
* @return string[]
*/
public static function contentEditor()
{
$options = [
@@ -238,6 +233,9 @@ class Admin
return $languages;
}
/**
* @return string
*/
public function getLanguage(): string
{
return $this->language ?: $this->grav['language']->getLanguage() ?: 'en';
@@ -317,6 +315,9 @@ class Admin
return $tools;
}
/**
* @return array
*/
public static function toolsPermissions()
{
$tools = static::tools();
@@ -349,12 +350,11 @@ class Admin
/**
* Static helper method to return the admin form nonce
*
* @param string $action
* @return string
*/
public static function getNonce()
public static function getNonce(string $action = 'admin-form')
{
$action = 'admin-form';
return Utils::getNonce($action);
}
@@ -388,11 +388,16 @@ class Admin
return $admin->getCurrentRoute();
}
/**
* @param string $path
* @param string|null $languageCode
* @return Route
*/
public function getAdminRoute(string $path = '', $languageCode = null): Route
{
/** @var Language $language */
$language = $this->grav['language'];
$languageCode = $languageCode ?? $language->getActive();
$languageCode = $languageCode ?? ($language->getActive() ?: null);
$languagePrefix = $languageCode ? '/' . $languageCode : '';
$root = $this->grav['uri']->rootUrl();
@@ -415,6 +420,11 @@ class Admin
return RouteFactory::createFromParts($parts);
}
/**
* @param string $route
* @param string|null $languageCode
* @return string
*/
public function adminUrl(string $route = '', $languageCode = null)
{
return $this->getAdminRoute($route, $languageCode)->toString(true);
@@ -435,6 +445,9 @@ class Admin
return $admin->getCurrentRoute();
}
/**
* @return string|null
*/
public function getCurrentRoute()
{
$pages = static::enablePages();
@@ -459,7 +472,8 @@ class Admin
* Route may or may not be prefixed by /en or /admin or /en/admin.
*
* @param string $redirect
* @param int$redirectCode
* @param int $redirectCode
* @return void
*/
public function redirect($redirect, $redirectCode = 303)
{
@@ -520,6 +534,9 @@ class Admin
return count($this->grav['config']->get('system.languages.supported', [])) > 1;
}
/**
* @return string
*/
public static function getTempDir()
{
try {
@@ -531,6 +548,9 @@ class Admin
return $tmp_dir;
}
/**
* @return array
*/
public static function getPageMedia()
{
$files = [];
@@ -541,7 +561,7 @@ class Admin
$route = '/' . ltrim($grav['admin']->route, '/');
/** @var PageInterface $page */
$page = $pages->find($route);
$page = $pages->find($route);
$parent_route = null;
if ($page) {
$media = $page->media()->all();
@@ -564,8 +584,7 @@ class Admin
/**
* Fetch and delete messages from the session queue.
*
* @param string $type
*
* @param string|null $type
* @return array
*/
public function messages($type = null)
@@ -579,7 +598,9 @@ class Admin
/**
* Authenticate user.
*
* @param array $credentials User credentials.
* @param array $credentials User credentials.
* @param array $post
* @return never-return
*/
public function authenticate($credentials, $post)
{
@@ -658,6 +679,10 @@ class Admin
/**
* Check Two-Factor Authentication.
*
* @param array $data
* @param array $post
* @return never-return
*/
public function twoFa($data, $post)
{
@@ -695,6 +720,10 @@ class Admin
/**
* Logout from admin.
*
* @param array $data
* @param array $post
* @return never-return
*/
public function logout($data, $post)
{
@@ -718,15 +747,8 @@ class Admin
public static function doAnyUsersExist()
{
$accounts = Grav::instance()['accounts'] ?? null;
if ($accounts instanceof \Countable) {
return $accounts->count() > 0;
}
// TODO: remove old way to check for existence of a user account (Grav < v1.6.9)
$account_dir = $file_path = Grav::instance()['locator']->findResource('account://');
$user_check = glob($account_dir . '/*.yaml');
return $user_check;
return $accounts && $accounts->count() > 0;
}
/**
@@ -734,6 +756,7 @@ class Admin
*
* @param string $msg
* @param string $type
* @return void
*/
public function setMessage($msg, $type = 'info')
{
@@ -742,11 +765,19 @@ class Admin
$messages->add($msg, $type);
}
/**
* @param string $msg
* @param string $type
* @return void
*/
public function addTempMessage($msg, $type)
{
$this->temp_messages[] = ['message' => $msg, 'scope' => $type];
}
/**
* @return array
*/
public function getTempMessages()
{
return $this->temp_messages;
@@ -755,11 +786,9 @@ class Admin
/**
* Translate a string to the user-defined language
*
* @param array|mixed $args
*
* @param mixed $languages
*
* @return string
* @param array|string $args
* @param array|null $languages
* @return string|string[]|null
*/
public static function translate($args, $languages = null)
{
@@ -812,7 +841,6 @@ class Admin
* Checks user authorisation to the action.
*
* @param string|string[] $action
*
* @return bool
*/
public function authorize($action = 'admin.login')
@@ -839,7 +867,6 @@ class Admin
*
* @param string $type
* @param array $post
*
* @return mixed
* @throws \RuntimeException
*/
@@ -958,7 +985,6 @@ class Admin
*
* @param string $type
* @param array|null $post
*
* @return object
* @throws \RuntimeException
*/
@@ -990,7 +1016,7 @@ class Admin
if (preg_match('|plugins/|', $type)) {
$obj = Plugins::get(preg_replace('|plugins/|', '', $type));
if (null === $obj) {
return [];
return new \stdClass();
}
if ($post) {
@@ -1005,7 +1031,7 @@ class Admin
$themes = $this->grav['themes'];
$obj = $themes->get(preg_replace('|themes/|', '', $type));
if (null === $obj) {
return [];
return new \stdClass();
}
if ($post) {
@@ -1070,6 +1096,11 @@ class Admin
return $data[$type];
}
/**
* @param Data\Data $object
* @param array $post
* @return Data\Data
*/
protected function mergePost(Data\Data $object, array $post)
{
$object->merge($post);
@@ -1105,6 +1136,9 @@ class Admin
return $post;
}
/**
* @return bool
*/
protected function hasErrorMessage()
{
$msgs = $this->grav['messages']->all();
@@ -1120,7 +1154,6 @@ class Admin
* Returns blueprints for the given type.
*
* @param string $type
*
* @return Data\Blueprint
*/
public function blueprints($type)
@@ -1136,7 +1169,6 @@ class Admin
* Converts dot notation to array notation.
*
* @param string $name
*
* @return string
*/
public function field($name)
@@ -1150,7 +1182,6 @@ class Admin
* Get all routes.
*
* @param bool $unique
*
* @return array
*/
public function routes($unique = false)
@@ -1231,6 +1262,10 @@ class Admin
return [];
}
/**
* @param string|null $package_slug
* @return string[]|string
*/
public function license($package_slug)
{
return Licenses::get($package_slug);
@@ -1241,7 +1276,6 @@ class Admin
* packages that can be removed when removing a package.
*
* @param string $slug The package slug
*
* @return array|bool
*/
public function dependenciesThatCanBeRemovedWhenRemoving($slug)
@@ -1255,21 +1289,17 @@ class Admin
$package = $this->getPackageFromGPM($slug);
if ($package) {
if ($package->dependencies) {
foreach ($package->dependencies as $dependency) {
// if (count($gpm->getPackagesThatDependOnPackage($dependency)) > 1) {
// continue;
// }
if (isset($dependency['name'])) {
$dependency = $dependency['name'];
}
if ($package && $package->dependencies) {
foreach ($package->dependencies as $dependency) {
// if (count($gpm->getPackagesThatDependOnPackage($dependency)) > 1) {
// continue;
// }
if (isset($dependency['name'])) {
$dependency = $dependency['name'];
}
if (!in_array($dependency, $dependencies, true)) {
if (!in_array($dependency, ['admin', 'form', 'login', 'email', 'php'])) {
$dependencies[] = $dependency;
}
}
if (!in_array($dependency, $dependencies, true) && !in_array($dependency, ['admin', 'form', 'login', 'email', 'php'])) {
$dependencies[] = $dependency;
}
}
}
@@ -1295,6 +1325,10 @@ class Admin
return $this->gpm;
}
/**
* @param string $package_slug
* @return mixed
*/
public function getPackageFromGPM($package_slug)
{
$package = $this->plugins(true)[$package_slug];
@@ -1309,7 +1343,6 @@ class Admin
* Get all plugins.
*
* @param bool $local
*
* @return mixed
*/
public function plugins($local = true)
@@ -1338,7 +1371,6 @@ class Admin
* Get all themes.
*
* @param bool $local
*
* @return mixed
*/
public function themes($local = true)
@@ -1384,9 +1416,8 @@ class Admin
* Check the passed packages list can be updated
*
* @param array $packages
*
* @throws \Exception
* @return bool
* @throws \Exception
*/
public function checkPackagesCanBeInstalled($packages)
{
@@ -1405,7 +1436,6 @@ class Admin
* to be installed.
*
* @param array $packages The packages slugs
*
* @return array|bool
*/
public function getDependenciesNeededToInstall($packages)
@@ -1422,8 +1452,7 @@ class Admin
* Used by the Dashboard in the admin to display the X latest pages
* that have been modified
*
* @param integer $count number of pages to pull back
*
* @param int $count number of pages to pull back
* @return array|null
*/
public function latestPages($count = 10)
@@ -1517,7 +1546,6 @@ class Admin
* Determine if the plugin or theme info passed is from Team Grav
*
* @param object $info Plugin or Theme info object
*
* @return bool
*/
public function isTeamGrav($info)
@@ -1529,7 +1557,6 @@ class Admin
* Determine if the plugin or theme info passed is premium
*
* @param object $info Plugin or Theme info object
*
* @return bool
*/
public function isPremiumProduct($info)
@@ -1542,13 +1569,12 @@ class Admin
*
* @return string The phpinfo() output
*/
function phpinfo()
public function phpinfo()
{
if (function_exists('phpinfo')) {
ob_start();
phpinfo();
$pinfo = ob_get_clean();
$pinfo = preg_replace('%^.*<body>(.*)</body>.*$%ms', '$1', $pinfo);
return $pinfo;
@@ -1560,8 +1586,7 @@ class Admin
/**
* Guest date format based on euro/US
*
* @param string $date
*
* @param string|null $date
* @return string
*/
public function guessDateFormat($date)
@@ -1584,6 +1609,7 @@ class Admin
'g:ia'
];
$date = (string)$date;
if (!isset($guess[$date])) {
$guess[$date] = 'd-m-Y H:i';
foreach ($date_formats as $date_format) {
@@ -1605,6 +1631,11 @@ class Admin
return $guess[$date];
}
/**
* @param string $date
* @param string $format
* @return bool
*/
public function validateDate($date, $format)
{
$d = DateTime::createFromFormat($format, $date);
@@ -1614,7 +1645,6 @@ class Admin
/**
* @param string $php_format
*
* @return string
*/
public function dateformatToMomentJS($php_format)