From da8a7b574c16ece62946e2a6ede538917488ecbe Mon Sep 17 00:00:00 2001 From: Matias Griese Date: Mon, 12 Apr 2021 22:06:43 +0300 Subject: [PATCH] Improve ACL checks for the tasks --- classes/plugin/AdminBaseController.php | 35 ++- classes/plugin/AdminController.php | 224 ++++++++++++------ .../partials/release-toggle.html.twig | 2 + 3 files changed, 186 insertions(+), 75 deletions(-) diff --git a/classes/plugin/AdminBaseController.php b/classes/plugin/AdminBaseController.php index 28a88134..acf54555 100644 --- a/classes/plugin/AdminBaseController.php +++ b/classes/plugin/AdminBaseController.php @@ -105,10 +105,17 @@ class AdminBaseController */ public function execute() { + // Ignore blacklisted views. if (in_array($this->view, $this->blacklist_views, true)) { return false; } + // Make sure that user is logged into admin. + if (!$this->admin->authorize()) { + return false; + } + + // Always validate nonce. if (!$this->validateNonce()) { return false; } @@ -222,6 +229,7 @@ class AdminBaseController * * @param string $path The path to redirect to * @param int $code The HTTP redirect code + * @return void */ public function setRedirect($path, $code = 303) { @@ -234,6 +242,7 @@ class AdminBaseController * * @param array $json * @param int $code + * @return never-return */ protected function sendJsonResponse(array $json, $code = 200): void { @@ -245,6 +254,7 @@ class AdminBaseController /** * @param ResponseInterface $response + * @return never-return */ protected function close(ResponseInterface $response): void { @@ -259,7 +269,7 @@ class AdminBaseController */ public function taskFilesUpload() { - if (null === $_FILES || !$this->authorizeTask('save', $this->dataPermissions())) { + if (null === $_FILES || !$this->authorizeTask('upload file', $this->dataPermissions())) { return false; } @@ -597,7 +607,7 @@ class AdminBaseController */ public function taskFilesSessionRemove() { - if (!$this->authorizeTask('save', $this->dataPermissions())) { + if (!$this->authorizeTask('delete file', $this->dataPermissions())) { return false; } @@ -654,6 +664,8 @@ class AdminBaseController * Redirect to the route stored in $this->redirect * * Route may or may not be prefixed by /en or /admin or /en/admin. + * + * @return void */ public function redirect() { @@ -706,6 +718,10 @@ class AdminBaseController return $data; } + /** + * @param array $source + * @return array + */ protected function cleanDataKeys($source = []) { $out = []; @@ -786,10 +802,12 @@ class AdminBaseController /** * Used by the filepicker field to get a list of files in a folder. + * + * @return bool */ protected function taskGetFilesInFolder() { - if (!$this->authorizeTask('save', $this->dataPermissions())) { + if (!$this->authorizeTask('get files', $this->dataPermissions())) { return false; } @@ -903,6 +921,11 @@ class AdminBaseController return true; } + /** + * @param string $file + * @param array $settings + * @return false + */ protected function filterAcceptedFiles($file, $settings) { $valid = false; @@ -922,6 +945,10 @@ class AdminBaseController */ protected function taskRemoveFileFromBlueprint() { + if (!$this->authorizeTask('remove file', $this->dataPermissions())) { + return false; + } + /** @var Uri $uri */ $uri = $this->grav['uri']; $blueprint = base64_decode($uri->param('blueprint')); @@ -1049,6 +1076,8 @@ class AdminBaseController /** * Handles removing a media file * + * @note This task cannot be used anymore. + * * @return bool True if the action was performed */ public function taskRemoveMedia($filename = null) diff --git a/classes/plugin/AdminController.php b/classes/plugin/AdminController.php index a2bebc0f..1ada7163 100644 --- a/classes/plugin/AdminController.php +++ b/classes/plugin/AdminController.php @@ -37,7 +37,6 @@ use RocketTheme\Toolbox\File\File; use RocketTheme\Toolbox\ResourceLocator\UniformResourceLocator; use Twig\Loader\FilesystemLoader; - /** * Class AdminController * @@ -51,6 +50,7 @@ class AdminController extends AdminBaseController * @param string|null $task * @param string|null $route * @param array|null $post + * @return void */ public function initialize(Grav $grav = null, $view = null, $task = null, $route = null, $post = null) { @@ -80,6 +80,7 @@ class AdminController extends AdminBaseController */ protected function taskKeepAlive(): void { + // This task is available for all admin users. $response = new Response(200); $this->close($response); @@ -92,7 +93,7 @@ class AdminController extends AdminBaseController */ protected function taskClearCache() { - if (!$this->authorizeTask('clear cache', ['admin.cache', 'admin.super', 'admin.maintenance'])) { + if (!$this->authorizeTask('clear cache', ['admin.cache', 'admin.maintenance', 'admin.super'])) { return false; } @@ -144,8 +145,10 @@ class AdminController extends AdminBaseController switch ($this->view) { case 'pages': + // Not used if Flex-Objects plugin handles pages. return $this->savePage(); case 'user': + // Not used if Flex-Objects plugin handles users. return $this->saveUser(); default: return $this->saveDefault(); @@ -204,13 +207,11 @@ class AdminController extends AdminBaseController */ protected function taskLogout() { - if (!$this->authorizeTask('logout', ['admin.login'])) { + if (!$this->authorizeTask('logout', ['admin.login', 'admin.super'])) { return false; } $this->admin->logout($this->data, $this->post); - - return true; } /** @@ -218,7 +219,7 @@ class AdminController extends AdminBaseController */ public function taskRegenerate2FASecret() { - if (!$this->authorizeTask('regenerate 2FA Secret', ['admin.login'])) { + if (!$this->authorizeTask('regenerate 2FA Secret', ['admin.login', 'admin.super'])) { return false; } @@ -265,6 +266,8 @@ class AdminController extends AdminBaseController * * Called by more general save task. * + * @note Not used if Flex-Objects plugin handles users. + * * @return bool */ protected function saveUser() @@ -330,7 +333,7 @@ class AdminController extends AdminBaseController /** * Get Notifications * - * @return void + * @return never-return */ protected function taskGetNotifications(): void { @@ -377,7 +380,7 @@ class AdminController extends AdminBaseController */ protected function taskHideNotification() { - if (!$this->authorizeTask('hide notification', ['admin.login'])) { + if (!$this->authorizeTask('hide notification', ['admin.login', 'admin.super'])) { return false; } @@ -409,7 +412,7 @@ class AdminController extends AdminBaseController /** * Get Newsfeeds * - * @return void + * @return never-return */ protected function taskGetNewsFeed(): void { @@ -447,11 +450,11 @@ class AdminController extends AdminBaseController */ protected function taskBackup() { - $param_sep = $this->grav['config']->get('system.param_sep', ':'); if (!$this->authorizeTask('backup', ['admin.maintenance', 'admin.super'])) { return false; } + $param_sep = $this->grav['config']->get('system.param_sep', ':'); $download = $this->grav['uri']->param('download'); try { @@ -549,11 +552,11 @@ class AdminController extends AdminBaseController */ public function taskEnable() { - if (!$this->authorizeTask('enable plugin', ['admin.plugins', 'admin.super'])) { + if ($this->view !== 'plugins') { return false; } - if ($this->view !== 'plugins') { + if (!$this->authorizeTask('enable plugin', ['admin.plugins', 'admin.super'])) { return false; } @@ -579,11 +582,11 @@ class AdminController extends AdminBaseController */ public function taskDisable() { - if (!$this->authorizeTask('disable plugin', ['admin.plugins', 'admin.super'])) { + if ($this->view !== 'plugins') { return false; } - if ($this->view !== 'plugins') { + if (!$this->authorizeTask('disable plugin', ['admin.plugins', 'admin.super'])) { return false; } @@ -609,11 +612,11 @@ class AdminController extends AdminBaseController */ public function taskActivate() { - if (!$this->authorizeTask('activate theme', ['admin.themes', 'admin.super'])) { + if ($this->view !== 'themes') { return false; } - if ($this->view !== 'themes') { + if (!$this->authorizeTask('activate theme', ['admin.themes', 'admin.super'])) { return false; } @@ -676,8 +679,6 @@ class AdminController extends AdminBaseController } $this->sendJsonResponse($json_response); - - return true; } /** @@ -692,7 +693,11 @@ class AdminController extends AdminBaseController */ public function taskUninstall() { - $type = $this->view === 'plugins' ? 'plugins' : 'themes'; + $type = $this->view; + if ($type !== 'plugins' && $type !== 'themes') { + return false; + } + if (!$this->authorizeTask('uninstall ' . $type, ['admin.' . $type, 'admin.super'])) { return false; } @@ -719,7 +724,7 @@ class AdminController extends AdminBaseController */ protected function taskGpmRelease() { - if (!$this->authorizeTask('configuration', ['admin.configuration.system', 'admin.super'])) { + if (!$this->authorizeTask('configuration', ['admin.super'])) { return false; } @@ -762,7 +767,11 @@ class AdminController extends AdminBaseController */ protected function taskGetUpdates() { - if (!$this->authorizeTask('dashboard', ['admin.login', 'admin.super'])) { + if ($this->view !== 'update') { + return false; + } + + if (!$this->authorizeTask('dashboard', ['admin.plugins', 'admin.themes', 'admin.super'])) { return false; } @@ -832,6 +841,10 @@ class AdminController extends AdminBaseController */ protected function taskGetPackagesDependencies() { + if (!$this->authorizeTask('dashboard', ['admin.plugins', 'admin.themes', 'admin.super'])) { + return false; + } + $data = $this->post; $packages = isset($data['packages']) ? explode(',', $data['packages']) : ''; $packages = (array)$packages; @@ -859,11 +872,11 @@ class AdminController extends AdminBaseController */ protected function taskInstallDependenciesOfPackages() { - $data = $this->post; - $packages = isset($data['packages']) ? explode(',', $data['packages']) : ''; - $packages = (array)$packages; - + $data = $this->post; $type = $data['type'] ?? ''; + if ($type !== 'plugins' && $type !== 'themes') { + return false; + } if (!$this->authorizeTask('install ' . $type, ['admin.' . $type, 'admin.super'])) { $this->admin->json_response = [ @@ -874,6 +887,9 @@ class AdminController extends AdminBaseController return false; } + $packages = isset($data['packages']) ? explode(',', $data['packages']) : ''; + $packages = (array)$packages; + try { $dependencies = $this->admin->getDependenciesNeededToInstall($packages); } catch (\Exception $e) { @@ -906,9 +922,11 @@ class AdminController extends AdminBaseController */ protected function taskInstallPackage($reinstall = false) { - $data = $this->post; - $package = $data['package'] ?? ''; - $type = $data['type'] ?? ''; + $data = $this->post; + $type = $data['type'] ?? ''; + if ($type !== 'plugins' && $type !== 'themes') { + return false; + } if (!$this->authorizeTask('install ' . $type, ['admin.' . $type, 'admin.super'])) { $this->admin->json_response = [ @@ -919,6 +937,7 @@ class AdminController extends AdminBaseController return false; } + $package = $data['package'] ?? ''; try { $result = Gpm::install($package, ['theme' => $type === 'theme']); } catch (\Exception $e) { @@ -954,14 +973,15 @@ class AdminController extends AdminBaseController /** * Handle removing a package * - * @return void + * @return bool */ - protected function taskRemovePackage(): void + protected function taskRemovePackage(): bool { $data = $this->post; - $package = $data['package'] ?? ''; $type = $data['type'] ?? ''; - $result = false; + if ($type !== 'plugins' && $type !== 'themes') { + return false; + } if (!$this->authorizeTask('uninstall ' . $type, ['admin.' . $type, 'admin.super'])) { $json_response = [ @@ -972,6 +992,9 @@ class AdminController extends AdminBaseController $this->sendJsonResponse($json_response, 403); } + $package = $data['package'] ?? ''; + $result = false; + //check if there are packages that have this as a dependency. Abort and show which ones $dependent_packages = $this->admin->getPackagesThatDependOnPackage($package); if (count($dependent_packages) > 0) { @@ -1023,16 +1046,15 @@ class AdminController extends AdminBaseController /** * Handle reinstalling a package * - * @return void + * @return bool */ protected function taskReinstallPackage() { $data = $this->post; - - $slug = $data['slug'] ?? ''; - $type = $data['type'] ?? ''; - $package_name = $data['package_name'] ?? ''; - $current_version = $data['current_version'] ?? ''; + $type = $data['type'] ?? ''; + if ($type !== 'plugins' && $type !== 'themes') { + return false; + } if (!$this->authorizeTask('install ' . $type, ['admin.' . $type, 'admin.super'])) { $json_response = [ @@ -1043,6 +1065,10 @@ class AdminController extends AdminBaseController $this->sendJsonResponse($json_response, 403); } + $slug = $data['slug'] ?? ''; + $package_name = $data['package_name'] ?? ''; + $current_version = $data['current_version'] ?? ''; + $url = "https://getgrav.org/download/{$type}s/$slug/$current_version"; $result = Gpm::directInstall($url); @@ -1059,6 +1085,8 @@ class AdminController extends AdminBaseController 'message' => $this->admin::translate('PLUGIN_ADMIN.REINSTALLATION_FAILED') ]; } + + return true; } /** @@ -1129,11 +1157,19 @@ class AdminController extends AdminBaseController /** * Handles creating an empty page folder (without markdown file) * + * Route: /pages + * + * @note Not used if Flex-Objects plugin handles pages. + * * @return bool True if the action was performed. */ public function taskSaveNewFolder() { - if (!$this->authorizeTask('save', $this->dataPermissions())) { + if ($this->view !== 'pages') { + return false; + } + + if (!$this->authorizeTask('new folder', ['admin.pages', 'admin.super'])) { return false; } @@ -1172,6 +1208,8 @@ class AdminController extends AdminBaseController } /** + * @note Not used if Flex-Objects plugin handles pages. + * * @return bool */ protected function savePage() @@ -1314,17 +1352,18 @@ class AdminController extends AdminBaseController * * Route: /pages * + * @note Not used if Flex-Objects plugin handles pages. + * * @return bool True if the action was performed. * @throws \RuntimeException */ protected function taskCopy() { - if (!$this->authorizeTask('copy page', ['admin.pages', 'admin.super'])) { + if ($this->view !== 'pages') { return false; } - // Only applies to pages. - if ($this->view !== 'pages') { + if (!$this->authorizeTask('copy page', ['admin.pages', 'admin.super'])) { return false; } @@ -1403,16 +1442,17 @@ class AdminController extends AdminBaseController * * Route: /pages * + * @note Not used if Flex-Objects plugin handles pages. + * * @return bool True if the action was performed. */ protected function taskReorder() { - if (!$this->authorizeTask('reorder pages', ['admin.pages', 'admin.super'])) { + if ($this->view !== 'pages') { return false; } - // Only applies to pages. - if ($this->view !== 'pages') { + if (!$this->authorizeTask('reorder pages', ['admin.pages', 'admin.super'])) { return false; } @@ -1426,17 +1466,18 @@ class AdminController extends AdminBaseController * * Route: /pages * + * @note Not used if Flex-Objects plugin handles pages. + * * @return bool True if the action was performed. * @throws \RuntimeException */ protected function taskDelete() { - if (!$this->authorizeTask('delete page', ['admin.pages', 'admin.super'])) { + if ($this->view !== 'pages') { return false; } - // Only applies to pages. - if ($this->view !== 'pages') { + if (!$this->authorizeTask('delete page', ['admin.pages', 'admin.super'])) { return false; } @@ -1472,10 +1513,16 @@ class AdminController extends AdminBaseController * * Route: /pages * + * @note Not used if Flex-Objects plugin handles pages. + * * @return bool */ protected function taskSwitchlanguage() { + if ($this->view !== 'pages') { + return false; + } + if (!$this->authorizeTask('switch language', ['admin.pages', 'admin.super'])) { return false; } @@ -1505,11 +1552,19 @@ class AdminController extends AdminBaseController /** * Save the current page in a different language. Automatically switches to that language. * + * Route: /pages + * + * @note Not used if Flex-Objects plugin handles pages. + * * @return bool True if the action was performed. */ protected function taskSaveas() { - if (!$this->authorizeTask('save', $this->dataPermissions())) { + if ($this->view !== 'pages') { + return false; + } + + if (!$this->authorizeTask('save as', ['admin.pages', 'admin.super'])) { return false; } @@ -1565,6 +1620,8 @@ class AdminController extends AdminBaseController /** * Continue to the new page. * + * @note Not used if Flex-Objects plugin handles pages, users and user groups. + * * @return bool True if the action was performed. */ public function taskContinue() @@ -1578,12 +1635,6 @@ class AdminController extends AdminBaseController return true; } - if ($this->view === 'groups') { - $this->setRedirect("{$this->view}/{$data['groupname']}"); - - return true; - } - if ($this->view !== 'pages') { return false; } @@ -1666,10 +1717,18 @@ class AdminController extends AdminBaseController } /** + * Route: /ajax.json + * + * @note Not used if Flex-Objects plugin handles pages. + * * @return bool */ protected function taskGetChildTypes() { + if ($this->view !== 'ajax') { + return false; + } + if (!$this->authorizeTask('get childtypes', ['admin.pages', 'admin.super'])) { return false; } @@ -1709,12 +1768,18 @@ class AdminController extends AdminBaseController /** * Handles filtering the page by modular/visible/routable in the pages list. * - * @return void + * @note Used only in legacy pages. + * + * @return bool */ protected function taskFilterPages() { + if ($this->view !== 'pages-filter') { + return false; + } + if (!$this->authorizeTask('filter pages', ['admin.pages', 'admin.super'])) { - return; + return false; } $data = $this->post; @@ -1829,12 +1894,16 @@ class AdminController extends AdminBaseController 'results' => $results ]; $this->admin->collection = $collection; + + return true; } /** * Process the page Markdown * + * Preview task in the builtin editor. + * * @return bool True if the action was performed. */ protected function taskProcessMarkdown() @@ -1884,12 +1953,16 @@ class AdminController extends AdminBaseController /** * Determines the file types allowed to be uploaded * - * Used by pagemedia field. + * Used by pagemedia field. Works only within legacy pages. * * @return bool True if the action was performed. */ protected function taskListmedia() { + if ($this->view !== 'media') { + return false; + } + if (!$this->authorizeTask('list media', ['admin.pages', 'admin.super'])) { return false; } @@ -1937,12 +2010,16 @@ class AdminController extends AdminBaseController /** * Handles adding a media file to a page. * - * Used by pagemedia field. + * Used by pagemedia field. Works only within legacy pages. * * @return bool True if the action was performed. */ protected function taskAddmedia() { + if ($this->view !== 'media') { + return false; + } + if (!$this->authorizeTask('add media', ['admin.pages', 'admin.super'])) { return false; } @@ -2106,8 +2183,7 @@ class AdminController extends AdminBaseController */ protected function taskCompileScss() { - - if (!$this->authorizeTask('compile scss', ['admin.super'])) { + if (!$this->authorizeTask('compile scss', ['admin.plugins', 'admin.super'])) { return false; } @@ -2132,8 +2208,7 @@ class AdminController extends AdminBaseController ] ]; - echo json_encode($json_response); - exit; + $this->sendJsonResponse($json_response); } /** @@ -2141,7 +2216,7 @@ class AdminController extends AdminBaseController */ protected function taskExportScss() { - if (!$this->authorizeTask('compile scss', ['admin.pages', 'admin.super'])) { + if (!$this->authorizeTask('export scss', ['admin.plugins', 'admin.super'])) { return false; } @@ -2160,8 +2235,7 @@ class AdminController extends AdminBaseController ] ]; - echo json_encode($json_response); - exit; + $this->sendJsonResponse($json_response); } /** @@ -2173,6 +2247,10 @@ class AdminController extends AdminBaseController */ protected function taskDelmedia() { + if ($this->view !== 'media') { + return false; + } + if (!$this->authorizeTask('delete media', ['admin.pages', 'admin.super'])) { return false; } @@ -2455,10 +2533,6 @@ class AdminController extends AdminBaseController */ public function getMedia(PageInterface $page = null) { - if ($this->view !== 'media') { - return null; - } - $page = $page ?? $this->admin->page($this->route); if (!$page) { return null; @@ -2753,10 +2827,16 @@ class AdminController extends AdminBaseController } /** - * @return ResponseInterface + * Used in 3rd party editors (e.g. next-gen). + * + * @return ResponseInterface|false */ - protected function taskConvertUrls(): ResponseInterface + protected function taskConvertUrls() { + if (!$this->authorizeTask('access page', ['admin.pages', 'admin.super'])) { + return false; + } + $request = $this->getRequest(); $data = $request->getParsedBody(); $converted_links = []; diff --git a/themes/grav/templates/partials/release-toggle.html.twig b/themes/grav/templates/partials/release-toggle.html.twig index 821f5a30..1d956cc7 100644 --- a/themes/grav/templates/partials/release-toggle.html.twig +++ b/themes/grav/templates/partials/release-toggle.html.twig @@ -1,3 +1,4 @@ +{% if authorize(['admin.super']) %}
@@ -7,3 +8,4 @@
+{% endif %}