diff --git a/reportbuilder/classes/local/helpers/audience.php b/reportbuilder/classes/local/helpers/audience.php index 8dc3f3843a4..6019c4675f0 100644 --- a/reportbuilder/classes/local/helpers/audience.php +++ b/reportbuilder/classes/local/helpers/audience.php @@ -19,6 +19,8 @@ declare(strict_types=1); namespace core_reportbuilder\local\helpers; use cache; +use context; +use context_system; use core_collator; use core_component; use core_plugin_manager; @@ -52,11 +54,11 @@ class audience { } /** - * Returns list of reports that the specified user can access. Note this is potentially very expensive to calculate if a + * Returns list of report IDs that the specified user can access, based on audience configuration. This can be expensive if the * site has lots of reports, with lots of audiences, so we cache the result for the duration of the users session * * @param int|null $userid User ID to check, or the current user if omitted - * @return array + * @return int[] */ public static function get_allowed_reports(?int $userid = null): array { global $USER, $DB; @@ -113,7 +115,7 @@ class audience { } /** - * Generate SQL select clause and params for selecting reports specified user can access + * Generate SQL select clause and params for selecting reports specified user can access, based on audience configuration * * @param string $reporttablealias * @param int|null $userid User ID to check, or the current user if omitted @@ -137,7 +139,7 @@ class audience { } /** - * Return list of report ID's specified user can access + * Return list of report ID's specified user can access, based on audience configuration * * @param int|null $userid User ID to check, or the current user if omitted * @return int[] @@ -153,6 +155,53 @@ class audience { return $DB->get_fieldset_sql($sql, $params); } + /** + * Returns SQL to limit the list of reports to those that the given user has access to + * + * - A user with 'editall' capability will have access to all reports + * - A user with 'edit' capability will have access to: + * - Those reports this user has created + * - Those reports this user is in audience of + * - A user with 'view' capability will have access to: + * - Those reports this user is in audience of + * + * @param string $reporttablealias + * @param int|null $userid User ID to check, or the current user if omitted + * @param context|null $context + * @return array + */ + public static function user_reports_list_access_sql( + string $reporttablealias, + ?int $userid = null, + ?context $context = null + ): array { + global $DB, $USER; + + if ($context === null) { + $context = context_system::instance(); + } + + // If user can't view all reports, limit the returned list to those reports they can see. + if (!has_capability('moodle/reportbuilder:editall', $context, $userid)) { + $reports = self::user_reports_list($userid); + + [$paramprefix, $paramuserid] = database::generate_param_names(2); + [$reportselect, $params] = $DB->get_in_or_equal($reports, SQL_PARAMS_NAMED, "{$paramprefix}_", true, null); + + $where = "{$reporttablealias}.id {$reportselect}"; + + // User can also see any reports that they can edit. + if (has_capability('moodle/reportbuilder:edit', $context, $userid)) { + $where = "({$reporttablealias}.usercreated = :{$paramuserid} OR {$where})"; + $params[$paramuserid] = $userid ?? $USER->id; + } + + return [$where, $params]; + } + + return ['1=1', []]; + } + /** * Return appropriate list of where clauses and params for given audiences * diff --git a/reportbuilder/classes/local/systemreports/reports_list.php b/reportbuilder/classes/local/systemreports/reports_list.php index 55f9c3b9751..acbdfd03f2f 100644 --- a/reportbuilder/classes/local/systemreports/reports_list.php +++ b/reportbuilder/classes/local/systemreports/reports_list.php @@ -18,9 +18,6 @@ declare(strict_types=1); namespace core_reportbuilder\local\systemreports; -use context_system; -use core_reportbuilder\local\helpers\audience; -use core_reportbuilder\local\helpers\database; use html_writer; use lang_string; use moodle_url; @@ -33,6 +30,7 @@ use core_reportbuilder\local\entities\user; use core_reportbuilder\local\filters\date; use core_reportbuilder\local\filters\text; use core_reportbuilder\local\filters\select; +use core_reportbuilder\local\helpers\audience; use core_reportbuilder\local\helpers\format; use core_reportbuilder\local\report\action; use core_reportbuilder\local\report\column; @@ -69,11 +67,9 @@ class reports_list extends system_report { // Select fields required for actions, permission checks, and row class callbacks. $this->add_base_fields('rb.id, rb.name, rb.source, rb.type, rb.usercreated, rb.contextid'); - // If user can't view all reports, limit the returned list to those reports they can see. - [$where, $params] = $this->filter_by_allowed_reports_sql(); - if (!empty($where)) { - $this->add_base_condition_sql($where, $params); - } + // Limit the returned list to those reports the current user can access. + [$where, $params] = audience::user_reports_list_access_sql('rb'); + $this->add_base_condition_sql($where, $params); // Join user entity for "User modified" column. $entityuser = new user(); @@ -310,53 +306,4 @@ class reports_list extends system_report { private function report_source_valid(string $source): bool { return manager::report_source_exists($source, datasource::class) && manager::report_source_available($source); } - - /** - * Filters the list of reports to return only the ones the user has access to - * - * - A user with 'editall' capability will have access to all reports. - * - A user with 'edit' capability will have access to: - * - Those reports this user has created. - * - Those reports this user is in audience of. - * - A user with 'view' capability will have access to: - * - Those reports this user is in audience of. - * - * @return array - */ - private function filter_by_allowed_reports_sql(): array { - global $DB, $USER; - - // If user can't view all reports, limit the returned list to those reports they can see. - if (!has_capability('moodle/reportbuilder:editall', context_system::instance())) { - $reports = audience::user_reports_list(); - - if (has_capability('moodle/reportbuilder:edit', context_system::instance())) { - // User can always see own reports and also those reports user is in audience of. - $paramuserid = database::generate_param_name(); - - if (empty($reports)) { - return ["rb.usercreated = :{$paramuserid}", [$paramuserid => $USER->id]]; - } - - $prefix = database::generate_param_name() . '_'; - [$where, $params] = $DB->get_in_or_equal($reports, SQL_PARAMS_NAMED, $prefix); - - $params = array_merge($params, [$paramuserid => $USER->id]); - - return ["(rb.usercreated = :{$paramuserid} OR rb.id {$where})", $params]; - - } - - // User has view capability. User can only see those reports user is in audience of. - if (empty($reports)) { - return ['1=2', []]; - } - - $prefix = database::generate_param_name() . '_'; - [$where, $params] = $DB->get_in_or_equal($reports, SQL_PARAMS_NAMED, $prefix); - return ["rb.id {$where}", $params]; - } - - return ['', []]; - } } diff --git a/reportbuilder/tests/local/helpers/audience_test.php b/reportbuilder/tests/local/helpers/audience_test.php index ac4bfb84404..35004ca007c 100644 --- a/reportbuilder/tests/local/helpers/audience_test.php +++ b/reportbuilder/tests/local/helpers/audience_test.php @@ -221,4 +221,71 @@ class audience_test extends advanced_testcase { $reports = audience::user_reports_list((int) $user3->id); $this->assertEmpty($reports); } + + /** + * Test retrieving full list of reports that user can access + */ + public function test_user_reports_list_access_sql(): void { + global $DB; + + $this->resetAfterTest(); + + $userone = $this->getDataGenerator()->create_user(); + $usertwo = $this->getDataGenerator()->create_user(); + $userthree = $this->getDataGenerator()->create_user(); + $userfour = $this->getDataGenerator()->create_user(); + + /** @var core_reportbuilder_generator $generator */ + $generator = $this->getDataGenerator()->get_plugin_generator('core_reportbuilder'); + + // Manager role gives users one and two capability to create own reports. + $managerrole = $DB->get_field('role', 'id', ['shortname' => 'manager']); + role_assign($managerrole, $userone->id, context_system::instance()); + role_assign($managerrole, $usertwo->id, context_system::instance()); + + // Admin creates a report, no audience. + $this->setAdminUser(); + $useradminreport = $generator->create_report(['name' => 'Admin report', 'source' => users::class]); + + // User one creates a report, adds users two and three to audience. + $this->setUser($userone); + $useronereport = $generator->create_report(['name' => 'User one report', 'source' => users::class]); + $generator->create_audience(['reportid' => $useronereport->get('id'), 'classname' => manual::class, 'configdata' => [ + 'users' => [$usertwo->id, $userthree->id], + ]]); + + // User two creates a report, no audience. + $this->setUser($usertwo); + $usertworeport = $generator->create_report(['name' => 'User two report', 'source' => users::class]); + + // Admin user sees all reports. + $this->setAdminUser(); + [$where, $params] = audience::user_reports_list_access_sql('r'); + $reports = $DB->get_fieldset_sql("SELECT r.id FROM {reportbuilder_report} r WHERE {$where}", $params); + $this->assertEqualsCanonicalizing([ + $useradminreport->get('id'), + $useronereport->get('id'), + $usertworeport->get('id'), + ], $reports); + + // User one sees only the report they created. + [$where, $params] = audience::user_reports_list_access_sql('r', (int) $userone->id); + $reports = $DB->get_fieldset_sql("SELECT r.id FROM {reportbuilder_report} r WHERE {$where}", $params); + $this->assertEquals([$useronereport->get('id')], $reports); + + // User two see the report they created and the one they are in the audience of. + [$where, $params] = audience::user_reports_list_access_sql('r', (int) $usertwo->id); + $reports = $DB->get_fieldset_sql("SELECT r.id FROM {reportbuilder_report} r WHERE {$where}", $params); + $this->assertEqualsCanonicalizing([$useronereport->get('id'), $usertworeport->get('id')], $reports); + + // User three sees the report they are in the audience of. + [$where, $params] = audience::user_reports_list_access_sql('r', (int) $userthree->id); + $reports = $DB->get_fieldset_sql("SELECT r.id FROM {reportbuilder_report} r WHERE {$where}", $params); + $this->assertEquals([$useronereport->get('id')], $reports); + + // User four sees no reports. + [$where, $params] = audience::user_reports_list_access_sql('r', (int) $userfour->id); + $reports = $DB->get_fieldset_sql("SELECT r.id FROM {reportbuilder_report} r WHERE {$where}", $params); + $this->assertEmpty($reports); + } } diff --git a/reportbuilder/upgrade.txt b/reportbuilder/upgrade.txt index f16934e7564..36cd7a716ec 100644 --- a/reportbuilder/upgrade.txt +++ b/reportbuilder/upgrade.txt @@ -35,6 +35,7 @@ Information provided here is intended especially for developers. * New method `get_default_condition_values()` in base datasource class, to be overridden by sources that wish to define default values for conditions upon report creation. * New methods `get_identity_[column|filter]` in user entity, for retrieving user identity field report elements +* New method `user_reports_list_access_sql` in audience helper for retrieving list of all reports for given user * New report filter types: - `category` for reports containing course categories - `tags` for reports containing entities with support for core_tag API