Skip to content

Commit

Permalink
ENH add granular dev url permissions
Browse files Browse the repository at this point in the history
  • Loading branch information
@andrewandante
andrewandante committed Oct 17, 2023
1 parent 37f0fc9 commit 33e50a3
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 48 deletions.
1 change: 0 additions & 1 deletion src/Dev/BuildTask.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use SilverStripe\Core\Config\Configurable;
use SilverStripe\Core\Extensible;
use SilverStripe\Core\Injector\Injectable;
use SilverStripe\Security\Permission;

/**
* Interface for a generic build task. Does not support dependencies. This will simply
Expand Down
38 changes: 37 additions & 1 deletion src/Dev/DevBuildController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@
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 = [
Expand All @@ -19,6 +22,15 @@ class DevBuildController extends Controller
'build'
];

protected function init()
{
parent::init();

if (!$this->canInit()) {
Security::permissionFailure($this);
}
}

public function build(HTTPRequest $request): HTTPResponse
{
if (Director::is_cli()) {
Expand All @@ -39,4 +51,28 @@ public function build(HTTPRequest $request): HTTPResponse
return $response;
}
}

public function canInit()
{
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")
|| Permission::check("CAN_DEV_BUILD")
);
}

public function providePermissions()
{
return [
'CAN_DEV_BUILD' => [
'name' => _t(__CLASS__ . '.CAN_DEV_BUILD_DESCRIPTION', 'Can execute a Dev/Build'),
'help' => _t(__CLASS__ . '.CAN_DEV_BUILD_HELP', 'Can execute a Dev/Build (/dev/build).'),
'category' => _t(__CLASS__ . 'PERMISSIONS_CATEGORY', 'Dev/Build permissions'),
'sort' => 100
],
];
}
}
40 changes: 38 additions & 2 deletions src/Dev/DevConfigController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@
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
{

/**
Expand All @@ -32,6 +35,15 @@ class DevConfigController extends Controller
'audit',
];

protected function init()
{
parent::init();

if (!$this->canInit()) {
Security::permissionFailure($this);
}
}

/**
* Note: config() method is already defined, so let's just use index()
*
Expand Down Expand Up @@ -129,6 +141,30 @@ public function audit()
return $this->getResponse()->setBody($body);
}

public function canInit()
{
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")
|| Permission::check("CAN_DEV_CONFIG")
);
}

public function providePermissions()
{
return [
'CAN_DEV_CONFIG' => [
'name' => _t(__CLASS__ . '.CAN_DEV_CONFIG_DESCRIPTION', 'Can see Dev/Config'),
'help' => _t(__CLASS__ . '.CAN_DEV_CONFIG_HELP', 'Can see Dev/Config (/dev/config).'),
'category' => _t(__CLASS__ . 'PERMISSIONS_CATEGORY', 'Dev/Config permissions'),
'sort' => 100
],
];
}

/**
* Returns all the keys of a multi-dimensional array while maintining any nested structure
*
Expand Down
51 changes: 25 additions & 26 deletions src/Dev/DevelopmentAdmin.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,26 +84,8 @@ protected function init()
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")
);

<SDFISDJFOPDSJP NEED to add a canAccessAll || canAccessAny check here too 43ofijds80fdsofds>


if (!$canAccess) {
if (!$this->canViewAll()) {
Security::permissionFailure($this);
return;
}
Expand Down Expand Up @@ -169,9 +151,6 @@ public function runRegisteredController(HTTPRequest $request)
}
}




/*
* Internal methods
*/
Expand All @@ -186,8 +165,12 @@ protected static function get_links()
$reg = Config::inst()->get(static::class, 'registered_controllers');
foreach ($reg as $registeredController) {
if (isset($registeredController['links'])) {
foreach ($registeredController['links'] as $url => $desc) {
$links[$url] = $desc;
// Check access to controller
$controllerSingleton = Injector::inst()->get($registeredController['controller']);
if (!$controllerSingleton->hasMethod('canInit') || $controllerSingleton->canInit()) {
foreach ($registeredController['links'] as $url => $desc) {
$links[$url] = $desc;
}
}
}
}
Expand All @@ -207,8 +190,6 @@ protected function getRegisteredController($baseUrlPart)
}




/*
* Unregistered (hidden) actions
*/
Expand Down Expand Up @@ -262,4 +243,22 @@ public function errors()
{
$this->redirect("Debug_");
}

protected function canViewAll()
{
// 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")
);
}
}
39 changes: 21 additions & 18 deletions src/Dev/TaskRunner.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,8 @@ protected function init()
{
parent::init();

if (!$this->canViewAllTasks()) {
$canAccessAny = false;
foreach ($this->getTaskList() as $task) {
if ($task->hasMethod('canView') && $task->canView()) {
$canAccessAny = true;
break;
}
}

if (!$canAccessAny) {
Security::permissionFailure($this);
}
if (!$this->canInit()) {
Security::permissionFailure($this);
}
}

Expand Down Expand Up @@ -208,7 +198,7 @@ protected function canViewTask($class)
}

$task = Injector::inst()->get($class);
if ($task->hasMethod('canView') && $task->canView()) {
if (!$task->hasMethod('canView') || $task->canView()) {
return true;
}

Expand Down Expand Up @@ -251,16 +241,29 @@ protected function addCssToHeader($header)

return $header;
}

public function canInit()
{
if ($this->canViewAllTasks()) {
return true;
}
foreach ($this->getTaskList() as $task) {
if (!$task->hasMethod('canView') || $task->canView()) {
return true;
}
}
return false;
}

public function providePermissions() {
public function providePermissions()
{
return [
'BUILDTASK_CAN_RUN' => [
'name' => _t(__CLASS__.'.BUILDTASK_CAN_RUN_DESCRIPTION', 'Can view and run Build Tasks'),
'help' => _t(__CLASS__.'.BUILDTASK_CAN_RUN_HELP', 'Can view and run Build Tasks via the endpoint (/dev/tasks).'),
'category' => _t(__CLASS__.'PERMISSIONS_CATEGORY', 'Task permissions'),
'name' => _t(__CLASS__ . '.BUILDTASK_CAN_RUN_DESCRIPTION', 'Can view and run Build Tasks'),
'help' => _t(__CLASS__ . '.BUILDTASK_CAN_RUN_HELP', 'Can view and run Build Tasks via the endpoint (/dev/tasks).'),
'category' => _t(__CLASS__ . 'PERMISSIONS_CATEGORY', 'Task permissions'),
'sort' => 100
],
];

}
}

0 comments on commit 33e50a3

Please sign in to comment.