Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[4.4] Fix Permissions for Manually Running Scheduled Tasks #36719

Draft
wants to merge 4 commits into
base: 4.4-dev
Choose a base branch
from
Draft

[4.4] Fix Permissions for Manually Running Scheduled Tasks #36719

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2319119
Disable "run task" button on lack of authorization
ditsuke Jan 17, 2022
ea7dc6a
Merge branch '4.1-dev' of github.com:joomla/joomla-cms 'fix-disabled-…
ditsuke Jan 17, 2022
b306858
Allow creators of tasks the ability to run them
ditsuke Jan 17, 2022
bb3f253
Get `created_by` in default TasksModel query
ditsuke Jan 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions administrator/components/com_scheduler/src/Scheduler/Scheduler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Joomla\CMS\Factory;
use Joomla\CMS\Language\Text;
use Joomla\CMS\Log\Log;
use Joomla\CMS\User\User;
use Joomla\Component\Scheduler\Administrator\Extension\SchedulerComponent;
use Joomla\Component\Scheduler\Administrator\Model\TaskModel;
use Joomla\Component\Scheduler\Administrator\Model\TasksModel;
Expand Down Expand Up @@ -358,4 +359,30 @@ public function fetchTaskRecords(array $filters, array $listConfig): array

return $model->getItems() ?: [];
}

/**
* Determine whether a {@see User} is allowed to run a task record. Expects a task as an object from
* {@see fetchTaskRecords}.
*
* @param object $taskRecord The task record to check authorization against.
* @param User $user The user to check authorization for.
*
* @return boolean True if the user is authorized to run the task.
*
* @since __DEPLOY_VERSION__
*/
public static function isAuthorizedToRun(object $taskRecord, User $user): bool
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brianteeman what do you think about this check?

{
/**
* We allow the user to run a task if they have the permission or if they created the task & still have the authority
* to create tasks.
*/
if ($user->authorise('core.testrun', 'com_scheduler.task.' . $taskRecord->id)
|| ($user->id == $taskRecord->created_by && $user->authorise('core.create', 'com_scheduler')))
{
return true;
}

return false;
}
}
21 changes: 17 additions & 4 deletions administrator/components/com_scheduler/tmpl/tasks/default.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Joomla\CMS\Layout\LayoutHelper;
use Joomla\CMS\Router\Route;
use Joomla\CMS\Session\Session;
use Joomla\Component\Scheduler\Administrator\Scheduler\Scheduler;
use Joomla\Component\Scheduler\Administrator\View\Tasks\HtmlView;

/** @var HtmlView $this*/
Expand Down Expand Up @@ -224,10 +225,22 @@ class="js-draggable" data-url="<?php echo $saveOrderingUrl; ?>" data-direction="

<!-- Test task -->
<td class="small d-none d-md-table-cell">
<button type="button" class="btn btn-sm btn-warning" <?php echo $item->state < 0 ? 'disabled' : ''; ?> data-id="<?php echo (int) $item->id; ?>" data-title="<?php echo htmlspecialchars($item->title); ?>" data-bs-toggle="modal" data-bs-backdrop="static" data-bs-target="#scheduler-test-modal">
<span class="fa fa-play fa-sm me-2"></span>
<?php echo Text::_('COM_SCHEDULER_TEST_RUN'); ?>
</button>
<div id="run-task-btn-wrapper"
<?php
$disabled = ($item->state < 0 || !Scheduler::isAuthorizedToRun($item, $user));
if ($disabled):
$reason = Text::_($item->state < 0 ? "COM_SCHEDULER_MANAGER_TOOLTIP_TASK_TRASHED" : "COM_SCHEDULER_MANAGER_TOOLTIP_NOT_AUTHORIZED");
echo ' data-toggle="tooltip" data-placement="top" title="' . $reason . '"';
endif;
?>
>
<button type="button" class="btn btn-sm btn-warning"
<?php if ($disabled): echo ' disabled '; endif;?>
data-id="<?php echo (int) $item->id; ?>" data-title="<?php echo htmlspecialchars($item->title); ?>" data-bs-toggle="modal" data-bs-backdrop="static" data-bs-target="#scheduler-test-modal">
<span class="fa fa-play fa-sm me-2"></span>
<?php echo Text::_('COM_SCHEDULER_TEST_RUN'); ?>
</button>
</div>
</td>

<!-- Item ID -->
Expand Down
2 changes: 2 additions & 0 deletions administrator/language/en-GB/com_scheduler.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ COM_SCHEDULER_LAST_RUN_DATE="Last Run Date"
COM_SCHEDULER_MANAGER_TASKS="Scheduled Tasks"
COM_SCHEDULER_MANAGER_TASK_EDIT="Edit Task"
COM_SCHEDULER_MANAGER_TASK_NEW="New Task"
COM_SCHEDULER_MANAGER_TOOLTIP_NOT_AUTHORIZED="Not authorized"
COM_SCHEDULER_MANAGER_TOOLTIP_TASK_TRASHED="Task has been trashed"
COM_SCHEDULER_MSG_MANAGE_NO_TASK_PLUGINS="There are no task types matching your query."
COM_SCHEDULER_NEW_TASK="New Task"
COM_SCHEDULER_N_ITEMS_DELETED="%d tasks deleted."
Expand Down
11 changes: 9 additions & 2 deletions plugins/system/schedulerunner/schedulerunner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,16 @@ public function runTestCron(Event $event)
$id = (int) $this->app->input->getInt('id');
$allowConcurrent = $this->app->input->getBool('allowConcurrent', false);

if (empty($id))
{
throw new \Exception(Text::_('JERROR_ALERTNOAUTHOR'), 403);
}

$scheduler = new Scheduler;
$taskRecord = $scheduler->fetchTaskRecord($id, true);
$user = Factory::getApplication()->getIdentity();

if (empty($id) || !$user->authorise('core.testrun', 'com_scheduler.task.' . $id))
if (empty($taskRecord) || !Scheduler::isAuthorizedToRun($taskRecord, $user))
{
throw new \Exception(Text::_('JERROR_ALERTNOAUTHOR'), 403);
}
Expand All @@ -275,7 +282,7 @@ public function runTestCron(Event $event)
* We will allow CLI exclusive tasks to be fetched and executed, it's left to routines to do a runtime check
* if they want to refuse normal operation.
*/
$task = (new Scheduler)->getTask(
$task = $scheduler->getTask(
[
'id' => $id,
'allowDisabled' => true,
Expand Down