ENH add permissions for build tasks

ENH add granular dev url permissions
This commit is contained in:
Andrew Paxley 2023-07-12 15:51:48 +12:00
parent cbd358ab93
commit 3244b44a54
7 changed files with 336 additions and 82 deletions

View File

@ -7,8 +7,11 @@ use SilverStripe\Control\Director;
use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPRequest;
use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse;
use SilverStripe\ORM\DatabaseAdmin; use SilverStripe\ORM\DatabaseAdmin;
use SilverStripe\Security\Permission;
use SilverStripe\Security\PermissionProvider;
use SilverStripe\Security\Security;
class DevBuildController extends Controller class DevBuildController extends Controller implements PermissionProvider
{ {
private static $url_handlers = [ private static $url_handlers = [
@ -19,6 +22,15 @@ class DevBuildController extends Controller
'build' 'build'
]; ];
protected function init(): void
{
parent::init();
if (!$this->canInit()) {
Security::permissionFailure($this);
}
}
public function build(HTTPRequest $request): HTTPResponse public function build(HTTPRequest $request): HTTPResponse
{ {
if (Director::is_cli()) { if (Director::is_cli()) {
@ -39,4 +51,27 @@ class DevBuildController extends Controller
return $response; return $response;
} }
} }
public function canInit(): bool
{
return (
Director::isDev()
// We need to ensure that DevelopmentAdminTest can simulate permission failures when running
// "dev/tasks" from CLI.
|| (Director::is_cli() && DevelopmentAdmin::config()->get('allow_all_cli'))
|| Permission::check(['ADMIN', 'ALL_DEV_ADMIN', 'CAN_DEV_BUILD'])
);
}
public function providePermissions(): array
{
return [
'CAN_DEV_BUILD' => [
'name' => _t(__CLASS__ . '.CAN_DEV_BUILD_DESCRIPTION', 'Can execute /dev/build'),
'help' => _t(__CLASS__ . '.CAN_DEV_BUILD_HELP', 'Can execute the build command (/dev/build).'),
'category' => DevelopmentAdmin::permissionsCategory(),
'sort' => 100
],
];
}
} }

View File

@ -7,13 +7,16 @@ use SilverStripe\Control\Director;
use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse;
use SilverStripe\Core\ClassInfo; use SilverStripe\Core\ClassInfo;
use SilverStripe\Core\Config\Config; use SilverStripe\Core\Config\Config;
use Symfony\Component\Yaml\Yaml;
use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Injector\Injector;
use SilverStripe\Security\Permission;
use SilverStripe\Security\PermissionProvider;
use SilverStripe\Security\Security;
use Symfony\Component\Yaml\Yaml;
/** /**
* Outputs the full configuration. * Outputs the full configuration.
*/ */
class DevConfigController extends Controller class DevConfigController extends Controller implements PermissionProvider
{ {
/** /**
@ -32,6 +35,15 @@ class DevConfigController extends Controller
'audit', 'audit',
]; ];
protected function init(): void
{
parent::init();
if (!$this->canInit()) {
Security::permissionFailure($this);
}
}
/** /**
* Note: config() method is already defined, so let's just use index() * Note: config() method is already defined, so let's just use index()
* *
@ -129,6 +141,29 @@ class DevConfigController extends Controller
return $this->getResponse()->setBody($body); return $this->getResponse()->setBody($body);
} }
public function canInit(): bool
{
return (
Director::isDev()
// We need to ensure that DevelopmentAdminTest can simulate permission failures when running
// "dev/tasks" from CLI.
|| (Director::is_cli() && DevelopmentAdmin::config()->get('allow_all_cli'))
|| Permission::check(['ADMIN', 'ALL_DEV_ADMIN', 'CAN_DEV_CONFIG'])
);
}
public function providePermissions(): array
{
return [
'CAN_DEV_CONFIG' => [
'name' => _t(__CLASS__ . '.CAN_DEV_CONFIG_DESCRIPTION', 'Can view /dev/config'),
'help' => _t(__CLASS__ . '.CAN_DEV_CONFIG_HELP', 'Can view all application configuration (/dev/config).'),
'category' => DevelopmentAdmin::permissionsCategory(),
'sort' => 100
],
];
}
/** /**
* Returns all the keys of a multi-dimensional array while maintining any nested structure * Returns all the keys of a multi-dimensional array while maintining any nested structure
* *

View File

@ -2,17 +2,20 @@
namespace SilverStripe\Dev; namespace SilverStripe\Dev;
use SilverStripe\Core\Config\Config; use Exception;
use SilverStripe\Core\Injector\Injector; use SilverStripe\Control\Controller;
use SilverStripe\Control\Director; use SilverStripe\Control\Director;
use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPRequest;
use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\HTTPResponse;
use SilverStripe\Control\Controller; use SilverStripe\Core\ClassInfo;
use SilverStripe\Versioned\Versioned; use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Dev\Deprecation;
use SilverStripe\ORM\DatabaseAdmin; use SilverStripe\ORM\DatabaseAdmin;
use SilverStripe\Security\Permission; use SilverStripe\Security\Permission;
use SilverStripe\Security\PermissionProvider;
use SilverStripe\Security\Security; use SilverStripe\Security\Security;
use Exception; use SilverStripe\Versioned\Versioned;
/** /**
* Base class for development tools. * Base class for development tools.
@ -25,7 +28,7 @@ use Exception;
* @todo cleanup errors() it's not even an allowed action, so can go * @todo cleanup errors() it's not even an allowed action, so can go
* @todo cleanup index() html building * @todo cleanup index() html building
*/ */
class DevelopmentAdmin extends Controller class DevelopmentAdmin extends Controller implements PermissionProvider
{ {
private static $url_handlers = [ private static $url_handlers = [
@ -84,22 +87,8 @@ class DevelopmentAdmin extends Controller
if (static::config()->get('deny_non_cli') && !Director::is_cli()) { if (static::config()->get('deny_non_cli') && !Director::is_cli()) {
return $this->httpError(404); return $this->httpError(404);
} }
// Special case for dev/build: Defer permission checks to DatabaseAdmin->init() (see #4957) if (!$this->canViewAll() && empty($this->getLinks())) {
$requestedDevBuild = (stripos($this->getRequest()->getURL() ?? '', 'dev/build') === 0)
&& (stripos($this->getRequest()->getURL() ?? '', 'dev/build/defaults') === false);
// We allow access to this controller regardless of live-status or ADMIN permission only
// if on CLI. Access to this controller is always allowed in "dev-mode", or of the user is ADMIN.
$allowAllCLI = static::config()->get('allow_all_cli');
$canAccess = (
$requestedDevBuild
|| Director::isDev()
|| (Director::is_cli() && $allowAllCLI)
// Its important that we don't run this check if dev/build was requested
|| Permission::check("ADMIN")
);
if (!$canAccess) {
Security::permissionFailure($this); Security::permissionFailure($this);
return; return;
} }
@ -114,6 +103,7 @@ class DevelopmentAdmin extends Controller
public function index() public function index()
{ {
$links = $this->getLinks();
// Web mode // Web mode
if (!Director::is_cli()) { if (!Director::is_cli()) {
$renderer = DebugView::create(); $renderer = DebugView::create();
@ -123,7 +113,7 @@ class DevelopmentAdmin extends Controller
echo '<div class="options"><ul>'; echo '<div class="options"><ul>';
$evenOdd = "odd"; $evenOdd = "odd";
foreach (self::get_links() as $action => $description) { foreach ($links as $action => $description) {
echo "<li class=\"$evenOdd\"><a href=\"{$base}dev/$action\"><b>/dev/$action:</b>" echo "<li class=\"$evenOdd\"><a href=\"{$base}dev/$action\"><b>/dev/$action:</b>"
. " $description</a></li>\n"; . " $description</a></li>\n";
$evenOdd = ($evenOdd == "odd") ? "even" : "odd"; $evenOdd = ($evenOdd == "odd") ? "even" : "odd";
@ -135,7 +125,7 @@ class DevelopmentAdmin extends Controller
} else { } else {
echo "SILVERSTRIPE DEVELOPMENT TOOLS\n--------------------------\n\n"; echo "SILVERSTRIPE DEVELOPMENT TOOLS\n--------------------------\n\n";
echo "You can execute any of the following commands:\n\n"; echo "You can execute any of the following commands:\n\n";
foreach (self::get_links() as $action => $description) { foreach ($links as $action => $description) {
echo " sake dev/$action: $description\n"; echo " sake dev/$action: $description\n";
} }
echo "\n\n"; echo "\n\n";
@ -165,18 +155,17 @@ class DevelopmentAdmin extends Controller
} }
} }
/* /*
* Internal methods * Internal methods
*/ */
/** /**
* @deprecated 5.2.0 use getLinks() instead to include permission checks
* @return array of url => description * @return array of url => description
*/ */
protected static function get_links() protected static function get_links()
{ {
Deprecation::notice('5.2.0', 'Use getLinks() instead to include permission checks');
$links = []; $links = [];
$reg = Config::inst()->get(static::class, 'registered_controllers'); $reg = Config::inst()->get(static::class, 'registered_controllers');
@ -190,6 +179,33 @@ class DevelopmentAdmin extends Controller
return $links; return $links;
} }
protected function getLinks(): array
{
$canViewAll = $this->canViewAll();
$links = [];
$reg = Config::inst()->get(static::class, 'registered_controllers');
foreach ($reg as $registeredController) {
if (isset($registeredController['links'])) {
if (!ClassInfo::exists($registeredController['controller'])) {
continue;
}
if (!$canViewAll) {
// Check access to controller
$controllerSingleton = Injector::inst()->get($registeredController['controller']);
if (!$controllerSingleton->hasMethod('canInit') || !$controllerSingleton->canInit()) {
continue;
}
}
foreach ($registeredController['links'] as $url => $desc) {
$links[$url] = $desc;
}
}
}
return $links;
}
protected function getRegisteredController($baseUrlPart) protected function getRegisteredController($baseUrlPart)
{ {
$reg = Config::inst()->get(static::class, 'registered_controllers'); $reg = Config::inst()->get(static::class, 'registered_controllers');
@ -203,8 +219,6 @@ class DevelopmentAdmin extends Controller
} }
/* /*
* Unregistered (hidden) actions * Unregistered (hidden) actions
*/ */
@ -258,4 +272,39 @@ TXT;
{ {
$this->redirect("Debug_"); $this->redirect("Debug_");
} }
public function providePermissions(): array
{
return [
'ALL_DEV_ADMIN' => [
'name' => _t(__CLASS__ . '.ALL_DEV_ADMIN_DESCRIPTION', 'Can view and execute all /dev endpoints'),
'help' => _t(__CLASS__ . '.ALL_DEV_ADMIN_HELP', 'Can view and execute all /dev endpoints'),
'category' => static::permissionsCategory(),
'sort' => 50
],
];
}
public static function permissionsCategory(): string
{
return _t(__CLASS__ . 'PERMISSIONS_CATEGORY', 'Dev permissions');
}
protected function canViewAll(): bool
{
// Special case for dev/build: Defer permission checks to DatabaseAdmin->init() (see #4957)
$requestedDevBuild = (stripos($this->getRequest()->getURL() ?? '', 'dev/build') === 0)
&& (stripos($this->getRequest()->getURL() ?? '', 'dev/build/defaults') === false);
// We allow access to this controller regardless of live-status or ADMIN permission only
// if on CLI. Access to this controller is always allowed in "dev-mode", or of the user is ADMIN.
$allowAllCLI = static::config()->get('allow_all_cli');
return (
$requestedDevBuild
|| Director::isDev()
|| (Director::is_cli() && $allowAllCLI)
// Its important that we don't run this check if dev/build was requested
|| Permission::check(['ADMIN', 'ALL_DEV_ADMIN'])
);
}
} }

View File

@ -13,11 +13,12 @@ use SilverStripe\Core\Injector\Injector;
use SilverStripe\Core\Manifest\ModuleResourceLoader; use SilverStripe\Core\Manifest\ModuleResourceLoader;
use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\ArrayList;
use SilverStripe\Security\Permission; use SilverStripe\Security\Permission;
use SilverStripe\Security\PermissionProvider;
use SilverStripe\Security\Security; use SilverStripe\Security\Security;
use SilverStripe\View\ArrayData; use SilverStripe\View\ArrayData;
use SilverStripe\View\ViewableData; use SilverStripe\View\ViewableData;
class TaskRunner extends Controller class TaskRunner extends Controller implements PermissionProvider
{ {
use Configurable; use Configurable;
@ -43,15 +44,7 @@ class TaskRunner extends Controller
{ {
parent::init(); parent::init();
$allowAllCLI = DevelopmentAdmin::config()->get('allow_all_cli'); if (!$this->canInit()) {
$canAccess = (
Director::isDev()
// We need to ensure that DevelopmentAdminTest can simulate permission failures when running
// "dev/tasks" from CLI.
|| (Director::is_cli() && $allowAllCLI)
|| Permission::check("ADMIN")
);
if (!$canAccess) {
Security::permissionFailure($this); Security::permissionFailure($this);
} }
} }
@ -139,15 +132,7 @@ class TaskRunner extends Controller
{ {
$availableTasks = []; $availableTasks = [];
$taskClasses = ClassInfo::subclassesFor(BuildTask::class); foreach ($this->getTaskList() as $class) {
// remove the base class
array_shift($taskClasses);
foreach ($taskClasses as $class) {
if (!$this->taskEnabled($class)) {
continue;
}
$singleton = BuildTask::singleton($class); $singleton = BuildTask::singleton($class);
$description = $singleton->getDescription(); $description = $singleton->getDescription();
$description = trim($description ?? ''); $description = trim($description ?? '');
@ -167,6 +152,18 @@ class TaskRunner extends Controller
return $availableTasks; return $availableTasks;
} }
protected function getTaskList(): array
{
$taskClasses = ClassInfo::subclassesFor(BuildTask::class, false);
foreach ($taskClasses as $index => $task) {
if (!$this->taskEnabled($task) || !$this->canViewTask($task)) {
unset($taskClasses[$index]);
}
}
return $taskClasses;
}
/** /**
* @param string $class * @param string $class
* @return boolean * @return boolean
@ -183,6 +180,36 @@ class TaskRunner extends Controller
return true; return true;
} }
protected function canViewTask(string $class): bool
{
if ($this->canViewAllTasks()) {
return true;
}
$reflectionClass = new ReflectionClass($class);
if ($reflectionClass->isAbstract()) {
return false;
}
$task = Injector::inst()->get($class);
if (!$task->hasMethod('canView') || !$task->canView()) {
return false;
}
return true;
}
protected function canViewAllTasks(): bool
{
return (
Director::isDev()
// We need to ensure that DevelopmentAdminTest can simulate permission failures when running
// "dev/tasks" from CLI.
|| (Director::is_cli() && DevelopmentAdmin::config()->get('allow_all_cli'))
|| Permission::check(['ADMIN', 'ALL_DEV_ADMIN', 'BUILDTASK_CAN_RUN'])
);
}
/** /**
* Inject task runner CSS into the heaader * Inject task runner CSS into the heaader
@ -207,4 +234,24 @@ class TaskRunner extends Controller
return $header; return $header;
} }
public function canInit(): bool
{
if ($this->canViewAllTasks()) {
return true;
}
return count($this->getTaskList()) > 0;
}
public function providePermissions(): array
{
return [
'BUILDTASK_CAN_RUN' => [
'name' => _t(__CLASS__ . '.BUILDTASK_CAN_RUN_DESCRIPTION', 'Can view and execute all /dev/tasks'),
'help' => _t(__CLASS__ . '.BUILDTASK_CAN_RUN_HELP', 'Can view and execute all Build Tasks (/dev/tasks). This supersedes individual task permissions'),
'category' => DevelopmentAdmin::permissionsCategory(),
'sort' => 70
],
];
}
} }

View File

@ -61,23 +61,10 @@ class DatabaseAdmin extends Controller
{ {
parent::init(); parent::init();
// We allow access to this controller regardless of live-status or ADMIN permission only if (!$this->canInit()) {
// if on CLI or with the database not ready. The latter makes it less error-prone to do an
// initial schema build without requiring a default-admin login.
// Access to this controller is always allowed in "dev-mode", or of the user is ADMIN.
$allowAllCLI = DevelopmentAdmin::config()->get('allow_all_cli');
$canAccess = (
Director::isDev()
|| !Security::database_is_ready()
// We need to ensure that DevelopmentAdminTest can simulate permission failures when running
// "dev/tests" from CLI.
|| (Director::is_cli() && $allowAllCLI)
|| Permission::check("ADMIN")
);
if (!$canAccess) {
Security::permissionFailure( Security::permissionFailure(
$this, $this,
"This page is secured and you need administrator rights to access it. " . "This page is secured and you need elevated permissions to access it. " .
"Enter your credentials below and we will send you right along." "Enter your credentials below and we will send you right along."
); );
} }
@ -367,6 +354,23 @@ class DatabaseAdmin extends Controller
$this->extend('onAfterBuild', $quiet, $populate, $testMode); $this->extend('onAfterBuild', $quiet, $populate, $testMode);
} }
public function canInit(): bool
{
// We allow access to this controller regardless of live-status or ADMIN permission only
// if on CLI or with the database not ready. The latter makes it less error-prone to do an
// initial schema build without requiring a default-admin login.
// Access to this controller is always allowed in "dev-mode", or of the user is ADMIN.
$allowAllCLI = DevelopmentAdmin::config()->get('allow_all_cli');
return (
Director::isDev()
|| !Security::database_is_ready()
// We need to ensure that DevelopmentAdminTest can simulate permission failures when running
// "dev/tests" from CLI.
|| (Director::is_cli() && $allowAllCLI)
|| Permission::check(['ADMIN', 'ALL_DEV_ADMIN', 'CAN_DEV_BUILD'])
);
}
/** /**
* Given a base data class, a field name and a mapping of class replacements, look for obsolete * Given a base data class, a field name and a mapping of class replacements, look for obsolete
* values in the $dataClass's $fieldName column and replace it with $mapping * values in the $dataClass's $fieldName column and replace it with $mapping

View File

@ -2,11 +2,15 @@
namespace SilverStripe\Dev\Tests; namespace SilverStripe\Dev\Tests;
use Exception;
use ReflectionMethod;
use SilverStripe\Control\Director;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Core\Kernel;
use SilverStripe\Dev\DevelopmentAdmin; use SilverStripe\Dev\DevelopmentAdmin;
use SilverStripe\Dev\FunctionalTest; use SilverStripe\Dev\FunctionalTest;
use SilverStripe\Control\Director;
use Exception;
use SilverStripe\Dev\Tests\DevAdminControllerTest\Controller1; use SilverStripe\Dev\Tests\DevAdminControllerTest\Controller1;
use SilverStripe\Dev\Tests\DevAdminControllerTest\ControllerWithPermissions;
/** /**
* Note: the running of this test is handled by the thing it's testing (DevelopmentAdmin controller). * Note: the running of this test is handled by the thing it's testing (DevelopmentAdmin controller).
@ -21,24 +25,29 @@ class DevAdminControllerTest extends FunctionalTest
DevelopmentAdmin::config()->merge( DevelopmentAdmin::config()->merge(
'registered_controllers', 'registered_controllers',
[ [
'x1' => [ 'x1' => [
'controller' => Controller1::class, 'controller' => Controller1::class,
'links' => [ 'links' => [
'x1' => 'x1 link description', 'x1' => 'x1 link description',
'x1/y1' => 'x1/y1 link description' 'x1/y1' => 'x1/y1 link description'
] ]
], ],
'x2' => [ 'x2' => [
'controller' => 'DevAdminControllerTest_Controller2', // intentionally not a class that exists 'controller' => 'DevAdminControllerTest_Controller2', // intentionally not a class that exists
'links' => [ 'links' => [
'x2' => 'x2 link description' 'x2' => 'x2 link description'
] ]
] ],
'x3' => [
'controller' => ControllerWithPermissions::class,
'links' => [
'x3' => 'x3 link description'
]
],
] ]
); );
} }
public function testGoodRegisteredControllerOutput() public function testGoodRegisteredControllerOutput()
{ {
// Check for the controller running from the registered url above // Check for the controller running from the registered url above
@ -57,7 +66,43 @@ class DevAdminControllerTest extends FunctionalTest
$this->assertEquals(true, $this->getAndCheckForError('/dev/x2')); $this->assertEquals(true, $this->getAndCheckForError('/dev/x2'));
} }
/**
* @dataProvider getLinksPermissionsProvider
*/
public function testGetLinks(string $permission, array $present, array $absent): void
{
DevelopmentAdmin::config()->set('allow_all_cli', false);
$kernel = Injector::inst()->get(Kernel::class);
$env = $kernel->getEnvironment();
$kernel->setEnvironment(Kernel::LIVE);
try {
$this->logInWithPermission($permission);
$controller = new DevelopmentAdmin();
$method = new ReflectionMethod($controller, 'getLinks');
$method->setAccessible(true);
$links = $method->invoke($controller);
foreach ($present as $expected) {
$this->assertArrayHasKey($expected, $links, sprintf('Expected link %s not found in %s', $expected, json_encode($links)));
}
foreach ($absent as $unexpected) {
$this->assertArrayNotHasKey($unexpected, $links, sprintf('Unexpected link %s found in %s', $unexpected, json_encode($links)));
}
} finally {
$kernel->setEnvironment($env);
}
}
protected function getLinksPermissionsProvider() : array
{
return [
['ADMIN', ['x1', 'x1/y1', 'x3'], ['x2']],
['ALL_DEV_ADMIN', ['x1', 'x1/y1', 'x3'], ['x2']],
['DEV_ADMIN_TEST_PERMISSION', ['x3'], ['x1', 'x1/y1', 'x2']],
['NOTHING', [], ['x1', 'x1/y1', 'x2', 'x3']],
];
}
protected function getCapture($url) protected function getCapture($url)
{ {

View File

@ -0,0 +1,39 @@
<?php
namespace SilverStripe\Dev\Tests\DevAdminControllerTest;
use SilverStripe\Control\Controller;
use SilverStripe\Security\Permission;
use SilverStripe\Security\PermissionProvider;
class ControllerWithPermissions extends Controller implements PermissionProvider
{
public const OK_MSG = 'DevAdminControllerTest_ControllerWithPermissions TEST OK';
private static $url_handlers = [
'' => 'index',
];
private static $allowed_actions = [
'index',
];
public function index()
{
echo self::OK_MSG;
}
public function canInit()
{
return Permission::check('DEV_ADMIN_TEST_PERMISSION');
}
public function providePermissions()
{
return [
'DEV_ADMIN_TEST_PERMISSION' => 'Dev admin test permission',
];
}
}