This commit is contained in:
Sara Arjona 2023-12-05 15:45:23 +01:00
commit d8b6117a4b
No known key found for this signature in database
7 changed files with 108 additions and 34 deletions

View File

@ -84,14 +84,17 @@ class audience {
// Generate audience SQL based on those for the current report.
[$wheres, $params] = self::user_audience_sql($audiences);
$allwheres = implode(' OR ', $wheres);
if (count($wheres) === 0) {
continue;
}
$paramuserid = database::generate_param_name();
$params[$paramuserid] = $userid;
$sql = "SELECT DISTINCT(u.id)
FROM {user} u
WHERE ({$allwheres})
WHERE (" . implode(' OR ', $wheres) . ")
AND u.deleted = 0
AND u.id = :{$paramuserid}";
// If we have a matching record, user can view the report.

View File

@ -111,19 +111,21 @@ class schedule {
$audienceids = (array) json_decode($schedule->get('audiences'));
// Retrieve all selected audience records for the schedule.
[$audienceselect, $audienceparams] = $DB->get_in_or_equal($audienceids, SQL_PARAMS_NAMED, 'aid', true, true);
[$audienceselect, $audienceparams] = $DB->get_in_or_equal($audienceids, SQL_PARAMS_NAMED, 'aid', true, null);
$audiences = audience_model::get_records_select("id {$audienceselect}", $audienceparams);
if (count($audiences) === 0) {
return [];
}
// Now convert audiences to SQL for user retrieval.
[$wheres, $params] = audience::user_audience_sql($audiences);
if (count($wheres) === 0) {
return [];
}
[$userorder] = users_order_by_sql('u');
$sql = 'SELECT u.*
FROM {user} u
WHERE ' . implode(' OR ', $wheres) . '
WHERE (' . implode(' OR ', $wheres) . ')
AND u.deleted = 0
ORDER BY ' . $userorder;
return $DB->get_records_sql($sql, $params);
@ -151,7 +153,8 @@ class schedule {
* @return stored_file
*/
public static function get_schedule_report_file(model $schedule): stored_file {
global $CFG;
global $CFG, $USER;
require_once("{$CFG->libdir}/filelib.php");
$table = custom_report_table_view::create($schedule->get('reportid'));
@ -166,10 +169,9 @@ class schedule {
$exportclass = new table_dataformat_export_format($table, $table->download);
ob_end_clean();
// Create our schedule report stored file.
$context = context_user::instance($schedule->get('usercreated'));
// Create our schedule report stored file temporarily in user draft.
$filerecord = [
'contextid' => $context->id,
'contextid' => context_user::instance($USER->id)->id,
'component' => 'user',
'filearea' => 'draft',
'itemid' => file_get_unused_draft_itemid(),

View File

@ -40,22 +40,22 @@ class report_access_list extends system_report {
protected function initialise(): void {
$userentity = new user();
$userentityalias = $userentity->get_table_alias('user');
$this->set_main_table('user', $userentityalias);
$this->add_entity($userentity);
$reportid = $this->get_parameter('id', 0, PARAM_INT);
// Find users allowed to view the report thru the report audiences.
[$wheres, $params] = self::get_users_by_audience_sql($reportid, $userentityalias);
$audiences = audience::get_records(['reportid' => $this->get_parameter('id', 0, PARAM_INT)]);
[$wheres, $params] = audience_helper::user_audience_sql($audiences, $userentityalias);
if (!empty($wheres)) {
// Wrap each OR condition into brackets.
$allwheres = '(' . implode(') OR (', $wheres) . ')';
if (count($wheres) > 0) {
$select = '(' . implode(' OR ', $wheres) . ')';
} else {
$allwheres = "1=0";
$select = "1=0";
}
$this->add_base_condition_sql("($allwheres)", $params);
$this->add_base_condition_sql($select, $params);
$this->add_base_condition_simple("{$userentityalias}.deleted", 0);
$this->add_columns($userentity);
$this->add_filters($userentity);
@ -106,17 +106,4 @@ class report_access_list extends system_report {
$this->add_filter($identityfilter);
}
}
/**
* Find users who can access this report based on the audience and add them to the report.
*
* @param int $reportid
* @param string $usertablealias
* @return array
*/
protected static function get_users_by_audience_sql(int $reportid, string $usertablealias): array {
$audiences = audience::get_records(['reportid' => $reportid]);
return audience_helper::user_audience_sql($audiences, $usertablealias);
}
}

View File

@ -22,6 +22,7 @@ use core_user;
use core\task\adhoc_task;
use core_reportbuilder\local\helpers\schedule as helper;
use core_reportbuilder\local\models\schedule;
use moodle_exception;
/**
* Ad-hoc task for sending a single report schedule
@ -70,8 +71,17 @@ class send_schedule extends adhoc_task {
$scheduleattachment = null;
$originaluser = $USER;
// Get the schedule creator, ensure it's an active account.
try {
$schedulecreator = core_user::get_user($schedule->get('usercreated'), '*', MUST_EXIST);
core_user::require_active_user($schedulecreator);
} catch (moodle_exception $exception) {
$this->log('Invalid schedule creator: ' . $exception->getMessage(), 0);
return;
}
// Switch to schedule creator, and retrieve list of recipient users.
\core\cron::setup_user(core_user::get_user($schedule->get('usercreated')));
\core\cron::setup_user($schedulecreator);
$users = helper::get_schedule_report_users($schedule);
if (count($users) > 0) {
@ -83,7 +93,17 @@ class send_schedule extends adhoc_task {
if ($scheduleuserviewas === schedule::REPORT_VIEWAS_CREATOR) {
$scheduleattachment = helper::get_schedule_report_file($schedule);
} else if ($scheduleuserviewas !== schedule::REPORT_VIEWAS_RECIPIENT) {
\core\cron::setup_user(core_user::get_user($scheduleuserviewas));
// Get the user to view the schedule report as, ensure it's an active account.
try {
$scheduleviewas = core_user::get_user($scheduleuserviewas, '*', MUST_EXIST);
core_user::require_active_user($scheduleviewas);
} catch (moodle_exception $exception) {
$this->log('Invalid schedule view as user: ' . $exception->getMessage(), 0);
return;
}
\core\cron::setup_user($scheduleviewas);
$scheduleattachment = helper::get_schedule_report_file($schedule);
}

View File

@ -155,6 +155,15 @@ class audience_test extends advanced_testcase {
// User2 can access report1 and report2.
$reports = audience::get_allowed_reports((int) $user2->id);
$this->assertEqualsCanonicalizing([$report1->get('id'), $report2->get('id')], $reports);
// Purge cache, to ensure allowed reports are re-calculated.
audience::purge_caches();
// Now delete one of our users, ensure they no longer have any allowed reports.
delete_user($user2);
$reports = audience::get_allowed_reports((int) $user2->id);
$this->assertEmpty($reports);
}
/**

View File

@ -225,6 +225,15 @@ class schedule_test extends advanced_testcase {
'Henrietta',
'Zoe',
], array_column($users, 'firstname'));
// Now delete one of our users, ensure they are no longer returned.
delete_user($manualuserone);
$users = schedule::get_schedule_report_users($schedule);
$this->assertEquals([
'Henrietta',
'Zoe',
], array_column($users, 'firstname'));
}
/**

View File

@ -146,6 +146,32 @@ class send_schedule_test extends advanced_testcase {
$this->assertStringEndsWith("Username\n{$usertwosees}\n", $messagetwoattachment);
}
/**
* Test executing task where the schedule "View as user" is an inactive account
*/
public function test_execute_report_viewas_user_invalid(): void {
$this->resetAfterTest();
$this->setAdminUser();
/** @var core_reportbuilder_generator $generator */
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
$report = $generator->create_report(['name' => 'My report', 'source' => users::class]);
$audience = $generator->create_audience(['reportid' => $report->get('id'), 'configdata' => []]);
$schedule = $generator->create_schedule([
'reportid' => $report->get('id'),
'name' => 'My schedule',
'userviewas' => 42,
'audiences' => json_encode([$audience->get_persistent()->get('id')]),
]);
$this->expectOutputRegex("/^Sending schedule: My schedule\nInvalid schedule view as user: Invalid user/");
$sendschedule = new send_schedule();
$sendschedule->set_custom_data(['reportid' => $report->get('id'), 'scheduleid' => $schedule->get('id')]);
$sendschedule->execute();
}
/**
* Test executing task for a schedule that is configured to not send empty reports
*/
@ -203,6 +229,24 @@ class send_schedule_test extends advanced_testcase {
$sendschedule->execute();
}
/**
* Test executing task where the schedule creator is an inactive account
*/
public function test_execute_schedule_creator_invalid(): void {
$this->resetAfterTest();
/** @var core_reportbuilder_generator $generator */
$generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder');
$report = $generator->create_report(['name' => 'My report', 'source' => users::class]);
$schedule = $generator->create_schedule(['reportid' => $report->get('id'), 'name' => 'My schedule', 'usercreated' => 42]);
$this->expectOutputRegex("/^Sending schedule: My schedule\nInvalid schedule creator: Invalid user/");
$sendschedule = new send_schedule();
$sendschedule->set_custom_data(['reportid' => $report->get('id'), 'scheduleid' => $schedule->get('id')]);
$sendschedule->execute();
}
/**
* Test executing task given invalid schedule data
*/