Merge pull request #10979 from andrewandante/ENH_add_buildtask_permissions

Add buildtask and dev URL permissions

_config/requestprocessors.yml

@@ -87,7 +87,7 @@ SilverStripe\Core\Injector\Injector:
         DevUrlsConfirmationMiddleware: '%$DevUrlsConfirmationMiddleware'
 
   DevUrlsConfirmationMiddleware:
-    class: SilverStripe\Control\Middleware\PermissionAwareConfirmationMiddleware
+    class: SilverStripe\Control\Middleware\DevelopmentAdminConfirmationMiddleware
     constructor:
       - '%$SilverStripe\Control\Middleware\ConfirmationMiddleware\UrlPathStartswith("dev")'
     properties:
@@ -97,8 +97,6 @@ SilverStripe\Core\Injector\Injector:
         - '%$SilverStripe\Control\Middleware\ConfirmationMiddleware\CliBypass'
         - '%$SilverStripe\Control\Middleware\ConfirmationMiddleware\EnvironmentBypass("dev")'
       EnforceAuthentication: false
-      AffectedPermissions:
-        - ADMIN
 
 ---
 Name: dev_urls-confirmation-exceptions
@@ -123,9 +121,6 @@ SilverStripe\Core\Injector\Injector:
   DevUrlsConfirmationMiddleware:
     properties:
       Bypasses:
-        # dev/build is covered by URLSpecialsMiddleware
-        - '%$SilverStripe\Control\Middleware\ConfirmationMiddleware\UrlPathStartswith("dev/build")'
-
         # The confirmation form is where people will be redirected for confirmation. We don't want to block it.
         - '%$SilverStripe\Control\Middleware\ConfirmationMiddleware\UrlPathStartswith("dev/confirm")'
 

src/Control/Middleware/DevelopmentAdminConfirmationMiddleware.php

@@ -0,0 +1,63 @@
+<?php
+
+namespace SilverStripe\Control\Middleware;
+
+use SilverStripe\Control\HTTPRequest;
+use SilverStripe\Core\Config\Config;
+use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Dev\DevelopmentAdmin;
+use SilverStripe\Security\Permission;
+
+/**
+ * Extends the PermissionAwareConfirmationMiddleware with checks for user permissions
+ *
+ * Respects users who don't have enough access and does not
+ * ask them for confirmation
+ *
+ * By default it enforces authentication by redirecting users to a login page.
+ *
+ * How it works:
+ *  - if user can bypass the middleware, then pass request further
+ *  - if there are no confirmation items, then pass request further
+ *  - if user is not authenticated and enforceAuthentication is false, then pass request further
+ *  - if user does not have at least one of the affected permissions, then pass request further
+ *  - otherwise, pass handling to the parent (ConfirmationMiddleware)
+ */
+class DevelopmentAdminConfirmationMiddleware extends PermissionAwareConfirmationMiddleware
+{
+
+    /**
+     * Check whether the user has permissions to perform the target operation
+     * Otherwise we may want to skip the confirmation dialog.
+     *
+     * WARNING! The user has to be authenticated beforehand
+     *
+     * @param HTTPRequest $request
+     *
+     * @return bool
+     */
+    public function hasAccess(HTTPRequest $request)
+    {
+        $action = $request->remaining();
+        if (empty($action)) {
+            return false;
+        }
+
+        $registeredRoutes = DevelopmentAdmin::config()->get('registered_controllers');
+        while (!isset($registeredRoutes[$action]) && strpos($action, '/') !== false) {
+            // Check for the parent route if a specific route isn't found
+            $action = substr($action, 0, strrpos($action, '/'));
+        }
+
+        if (isset($registeredRoutes[$action]['controller'])) {
+            $initPermissions = Config::forClass($registeredRoutes[$action]['controller'])->get('init_permissions');
+            foreach ($initPermissions as $permission) {
+                if (Permission::check($permission)) {
+                    return true;
+                }
+            }
+        }
+
+        return false;
+    }
+}

src/Control/Middleware/URLSpecialsMiddleware.php

@@ -39,8 +39,7 @@ class URLSpecialsMiddleware extends PermissionAwareConfirmationMiddleware
         parent::__construct(
             new ConfirmationMiddleware\GetParameter("flush"),
             new ConfirmationMiddleware\GetParameter("isDev"),
-            new ConfirmationMiddleware\GetParameter("isTest"),
-            new ConfirmationMiddleware\UrlPathStartswith("dev/build")
+            new ConfirmationMiddleware\GetParameter("isTest")
         );
     }
 

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,21 @@ class DevBuildController extends Controller
         'build'
     ];
 
+    private static $init_permissions = [
+        'ADMIN',
+        'ALL_DEV_ADMIN',
+        'CAN_DEV_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 +57,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(static::config()->get('init_permissions'))
+        );
+    }
+    
+    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
+            ],
+        ];
+    }
 }

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,21 @@ class DevConfigController extends Controller
         'audit',
     ];
 
+    private static $init_permissions = [
+        'ADMIN',
+        'ALL_DEV_ADMIN',
+        'CAN_DEV_CONFIG',
+    ];
+
+    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 +147,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(static::config()->get('init_permissions'))
+        );
+    }
+    
+    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
      *

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.
@@ -20,7 +23,7 @@ use Exception;
  * Configured in framework/_config/dev.yml, with the config key registeredControllers being
  * used to generate the list of links for /dev.
  */
-class DevelopmentAdmin extends Controller
+class DevelopmentAdmin extends Controller implements PermissionProvider
 {
 
     private static $url_handlers = [
@@ -79,22 +82,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;
         }
@@ -109,6 +98,7 @@ class DevelopmentAdmin extends Controller
 
     public function index()
     {
+        $links = $this->getLinks();
         // Web mode
         if (!Director::is_cli()) {
             $renderer = DebugView::create();
@@ -118,7 +108,7 @@ class DevelopmentAdmin extends Controller
 
             echo '<div class="options"><ul>';
             $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>"
                     . " $description</a></li>\n";
                 $evenOdd = ($evenOdd == "odd") ? "even" : "odd";
@@ -130,7 +120,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";
@@ -160,18 +150,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');
@@ -185,6 +174,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');
@@ -198,8 +214,6 @@ class DevelopmentAdmin extends Controller
     }
 
 
-
-
     /*
      * Unregistered (hidden) actions
      */
@@ -253,4 +267,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'])
+        );
+    }
 }

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;
@@ -32,6 +33,12 @@ class TaskRunner extends Controller
         'runTask',
     ];
 
+    private static $init_permissions = [
+        'ADMIN',
+        'ALL_DEV_ADMIN',
+        'BUILDTASK_CAN_RUN',
+    ];
+
     /**
      * @var array
      */
@@ -43,15 +50,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);
         }
     }
@@ -119,8 +118,8 @@ class TaskRunner extends Controller
                 $inst = Injector::inst()->create($task['class']);
                 $title(sprintf('Running Task %s', $inst->getTitle()));
 
-                if (!$inst->isEnabled()) {
-                    $message('The task is disabled');
+                if (!$this->taskEnabled($task['class'])) {
+                    $message('The task is disabled or you do not have sufficient permission to run it');
                     return;
                 }
 
@@ -129,7 +128,7 @@ class TaskRunner extends Controller
             }
         }
 
-        $message(sprintf('The build task "%s" could not be found', Convert::raw2xml($name)));
+        $message(sprintf('The build task "%s" could not be found, is disabled or you do not have sufficient permission to run it', Convert::raw2xml($name)));
     }
 
     /**
@@ -139,15 +138,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 +158,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)) {
+                unset($taskClasses[$index]);
+            }
+        }
+
+        return $taskClasses;
+    }
+
     /**
      * @param string $class
      * @return boolean
@@ -176,11 +179,29 @@ class TaskRunner extends Controller
         $reflectionClass = new ReflectionClass($class);
         if ($reflectionClass->isAbstract()) {
             return false;
-        } elseif (!singleton($class)->isEnabled()) {
+        }
+
+        $task = Injector::inst()->get($class);
+        if (!$task->isEnabled()) {
             return false;
         }
 
-        return true;
+        if ($task->hasMethod('canView') && !$task->canView()) {
+            return false;
+        }
+
+        return $this->canViewAllTasks();
+    }
+
+    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(static::config()->get('init_permissions'))
+        );
     }
 
     /**
@@ -207,4 +228,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 may still be overriden by individual task view permissions'),
+                'category' => DevelopmentAdmin::permissionsCategory(),
+                'sort' => 70
+            ],
+        ];
+    }
 }

src/Dev/Tasks/CleanupTestDatabasesTask.php

@@ -23,15 +23,18 @@ class CleanupTestDatabasesTask extends BuildTask
 
     public function run($request)
     {
-        if (!Permission::check('ADMIN') && !Director::is_cli()) {
+        if (!$this->canView()) {
             $response = Security::permissionFailure();
             if ($response) {
                 $response->output();
             }
             die;
         }
-
-        // Delete all temp DBs
         TempDatabase::create()->deleteAll();
     }
+
+    public function canView(): bool
+    {
+        return Permission::check('ADMIN') || Director::is_cli();
+    }
 }

src/ORM/DatabaseAdmin.php

@@ -10,6 +10,7 @@ use SilverStripe\Core\ClassInfo;
 use SilverStripe\Core\Environment;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Core\Manifest\ClassLoader;
+use SilverStripe\Dev\DevBuildController;
 use SilverStripe\Dev\DevelopmentAdmin;
 use SilverStripe\ORM\Connect\DatabaseException;
 use SilverStripe\ORM\Connect\TableBuilder;
@@ -61,23 +62,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 +355,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(DevBuildController::config()->get('init_permissions'))
+        );
+    }
+
     /**
      * 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

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)
     {

tests/php/Dev/DevAdminControllerTest/ControllerWithPermissions.php

@@ -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',
+        ];
+    }
+}