diff --git a/src/Dev/DevBuildController.php b/src/Dev/DevBuildController.php
index 5f7a949fd..a5377570e 100644
--- a/src/Dev/DevBuildController.php
+++ b/src/Dev/DevBuildController.php
@@ -7,8 +7,11 @@ use SilverStripe\Control\Director;
use SilverStripe\Control\HTTPRequest;
use SilverStripe\Control\HTTPResponse;
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 = [
@@ -19,6 +22,15 @@ class DevBuildController extends Controller
'build'
];
+ protected function init(): void
+ {
+ parent::init();
+
+ if (!$this->canInit()) {
+ Security::permissionFailure($this);
+ }
+ }
+
public function build(HTTPRequest $request): HTTPResponse
{
if (Director::is_cli()) {
@@ -39,4 +51,27 @@ class DevBuildController extends Controller
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
+ ],
+ ];
+ }
}
diff --git a/src/Dev/DevConfigController.php b/src/Dev/DevConfigController.php
index 3b5d48869..05b03f797 100644
--- a/src/Dev/DevConfigController.php
+++ b/src/Dev/DevConfigController.php
@@ -7,13 +7,16 @@ use SilverStripe\Control\Director;
use SilverStripe\Control\HTTPResponse;
use SilverStripe\Core\ClassInfo;
use SilverStripe\Core\Config\Config;
-use Symfony\Component\Yaml\Yaml;
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.
*/
-class DevConfigController extends Controller
+class DevConfigController extends Controller implements PermissionProvider
{
/**
@@ -32,6 +35,15 @@ class DevConfigController extends Controller
'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()
*
@@ -129,6 +141,29 @@ class DevConfigController extends Controller
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
*
diff --git a/src/Dev/DevelopmentAdmin.php b/src/Dev/DevelopmentAdmin.php
index 79b2e1c70..e3639c7d7 100644
--- a/src/Dev/DevelopmentAdmin.php
+++ b/src/Dev/DevelopmentAdmin.php
@@ -2,17 +2,20 @@
namespace SilverStripe\Dev;
-use SilverStripe\Core\Config\Config;
-use SilverStripe\Core\Injector\Injector;
+use Exception;
+use SilverStripe\Control\Controller;
use SilverStripe\Control\Director;
use SilverStripe\Control\HTTPRequest;
use SilverStripe\Control\HTTPResponse;
-use SilverStripe\Control\Controller;
-use SilverStripe\Versioned\Versioned;
+use SilverStripe\Core\ClassInfo;
+use SilverStripe\Core\Config\Config;
+use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Dev\Deprecation;
use SilverStripe\ORM\DatabaseAdmin;
use SilverStripe\Security\Permission;
+use SilverStripe\Security\PermissionProvider;
use SilverStripe\Security\Security;
-use Exception;
+use SilverStripe\Versioned\Versioned;
/**
* 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 index() html building
*/
-class DevelopmentAdmin extends Controller
+class DevelopmentAdmin extends Controller implements PermissionProvider
{
private static $url_handlers = [
@@ -84,22 +87,8 @@ class DevelopmentAdmin extends Controller
if (static::config()->get('deny_non_cli') && !Director::is_cli()) {
return $this->httpError(404);
}
-
- // 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');
- $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) {
+
+ if (!$this->canViewAll() && empty($this->getLinks())) {
Security::permissionFailure($this);
return;
}
@@ -114,6 +103,7 @@ class DevelopmentAdmin extends Controller
public function index()
{
+ $links = $this->getLinks();
// Web mode
if (!Director::is_cli()) {
$renderer = DebugView::create();
@@ -123,7 +113,7 @@ class DevelopmentAdmin extends Controller
echo '
';
$evenOdd = "odd";
- foreach (self::get_links() as $action => $description) {
+ foreach ($links as $action => $description) {
echo "- /dev/$action:"
. " $description
\n";
$evenOdd = ($evenOdd == "odd") ? "even" : "odd";
@@ -135,7 +125,7 @@ class DevelopmentAdmin extends Controller
} else {
echo "SILVERSTRIPE DEVELOPMENT TOOLS\n--------------------------\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 "\n\n";
@@ -165,18 +155,17 @@ class DevelopmentAdmin extends Controller
}
}
-
-
-
/*
* Internal methods
*/
/**
+ * @deprecated 5.2.0 use getLinks() instead to include permission checks
* @return array of url => description
*/
protected static function get_links()
{
+ Deprecation::notice('5.2.0', 'Use getLinks() instead to include permission checks');
$links = [];
$reg = Config::inst()->get(static::class, 'registered_controllers');
@@ -190,6 +179,33 @@ class DevelopmentAdmin extends Controller
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)
{
$reg = Config::inst()->get(static::class, 'registered_controllers');
@@ -203,8 +219,6 @@ class DevelopmentAdmin extends Controller
}
-
-
/*
* Unregistered (hidden) actions
*/
@@ -258,4 +272,39 @@ TXT;
{
$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'])
+ );
+ }
}
diff --git a/src/Dev/TaskRunner.php b/src/Dev/TaskRunner.php
index 203030fc4..04148e2ad 100644
--- a/src/Dev/TaskRunner.php
+++ b/src/Dev/TaskRunner.php
@@ -13,11 +13,12 @@ use SilverStripe\Core\Injector\Injector;
use SilverStripe\Core\Manifest\ModuleResourceLoader;
use SilverStripe\ORM\ArrayList;
use SilverStripe\Security\Permission;
+use SilverStripe\Security\PermissionProvider;
use SilverStripe\Security\Security;
use SilverStripe\View\ArrayData;
use SilverStripe\View\ViewableData;
-class TaskRunner extends Controller
+class TaskRunner extends Controller implements PermissionProvider
{
use Configurable;
@@ -43,15 +44,7 @@ class TaskRunner extends Controller
{
parent::init();
- $allowAllCLI = DevelopmentAdmin::config()->get('allow_all_cli');
- $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) {
+ if (!$this->canInit()) {
Security::permissionFailure($this);
}
}
@@ -139,15 +132,7 @@ class TaskRunner extends Controller
{
$availableTasks = [];
- $taskClasses = ClassInfo::subclassesFor(BuildTask::class);
- // remove the base class
- array_shift($taskClasses);
-
- foreach ($taskClasses as $class) {
- if (!$this->taskEnabled($class)) {
- continue;
- }
-
+ foreach ($this->getTaskList() as $class) {
$singleton = BuildTask::singleton($class);
$description = $singleton->getDescription();
$description = trim($description ?? '');
@@ -167,6 +152,18 @@ class TaskRunner extends Controller
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
* @return boolean
@@ -183,6 +180,36 @@ class TaskRunner extends Controller
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
@@ -207,4 +234,24 @@ class TaskRunner extends Controller
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
+ ],
+ ];
+ }
}
diff --git a/src/ORM/DatabaseAdmin.php b/src/ORM/DatabaseAdmin.php
index 698fe6d3f..5da6ed65e 100644
--- a/src/ORM/DatabaseAdmin.php
+++ b/src/ORM/DatabaseAdmin.php
@@ -61,23 +61,10 @@ class DatabaseAdmin extends Controller
{
parent::init();
- // 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');
- $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) {
+ if (!$this->canInit()) {
Security::permissionFailure(
$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."
);
}
@@ -367,6 +354,23 @@ class DatabaseAdmin extends Controller
$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
* values in the $dataClass's $fieldName column and replace it with $mapping
diff --git a/tests/php/Dev/DevAdminControllerTest.php b/tests/php/Dev/DevAdminControllerTest.php
index 60e93fed0..838547e9f 100644
--- a/tests/php/Dev/DevAdminControllerTest.php
+++ b/tests/php/Dev/DevAdminControllerTest.php
@@ -2,11 +2,15 @@
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\FunctionalTest;
-use SilverStripe\Control\Director;
-use Exception;
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).
@@ -21,24 +25,29 @@ class DevAdminControllerTest extends FunctionalTest
DevelopmentAdmin::config()->merge(
'registered_controllers',
[
- 'x1' => [
- 'controller' => Controller1::class,
- 'links' => [
- 'x1' => 'x1 link description',
- 'x1/y1' => 'x1/y1 link description'
- ]
- ],
- 'x2' => [
- 'controller' => 'DevAdminControllerTest_Controller2', // intentionally not a class that exists
- 'links' => [
- 'x2' => 'x2 link description'
- ]
- ]
+ 'x1' => [
+ 'controller' => Controller1::class,
+ 'links' => [
+ 'x1' => 'x1 link description',
+ 'x1/y1' => 'x1/y1 link description'
+ ]
+ ],
+ 'x2' => [
+ 'controller' => 'DevAdminControllerTest_Controller2', // intentionally not a class that exists
+ 'links' => [
+ 'x2' => 'x2 link description'
+ ]
+ ],
+ 'x3' => [
+ 'controller' => ControllerWithPermissions::class,
+ 'links' => [
+ 'x3' => 'x3 link description'
+ ]
+ ],
]
);
}
-
public function testGoodRegisteredControllerOutput()
{
// 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'));
}
+ /**
+ * @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)
{
diff --git a/tests/php/Dev/DevAdminControllerTest/ControllerWithPermissions.php b/tests/php/Dev/DevAdminControllerTest/ControllerWithPermissions.php
new file mode 100644
index 000000000..7cbd2d276
--- /dev/null
+++ b/tests/php/Dev/DevAdminControllerTest/ControllerWithPermissions.php
@@ -0,0 +1,39 @@
+ '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',
+ ];
+ }
+}