Add violation hash and index #66

catalyst · Jan 8, 2025 · c744b8a · c744b8a
1 parent 2a085fd
commit c744b8a
Show file tree

Hide file tree

Showing 8 changed files with 128 additions and 49 deletions.
diff --git a/classes/table/csp_report.php b/classes/table/csp_report.php
@@ -51,8 +51,7 @@ protected function col_failcounter($record) {
         // Get blocked URI, and set as param for page if clicked on.
         $url = new \moodle_url('/local/csp/csp_report.php',
             [
-                'blockeddomain' => $record->blockeddomain,
-                'blockeddirective' => $record->violateddirective
+                'violation' => $record->violationhash,
             ]
         );
         return \html_writer::link($url, $record->failcounter);
@@ -179,16 +178,14 @@ protected function col_blockedurlpaths($record) {
                           blockedurlpath,
                           SUM(failcounter) AS failcounter
                      FROM {local_csp} csp
-                    WHERE violateddirective = :directive
-                      AND blockeddomain = :blockeddomain
+                    WHERE violationhash = :violationhash
                  GROUP BY blockeduri,
                           blockedurlpath
                  ORDER BY SUM(failcounter) DESC,
                           blockeduri";
 
         $params = [
-            'directive' => $record->violateddirective,
-            'blockeddomain' => $record->blockeddomain
+            'violationhash' => $record->violationhash,
         ];
 
         $blockedpaths = $DB->get_records_sql($subsql, $params, 0, 3);
@@ -218,15 +215,13 @@ protected function col_highestviolaters($record) {
         $subsql = "SELECT documenturi,
                           SUM(failcounter) AS failcounter
                      FROM {local_csp} csp
-                    WHERE violateddirective = :directive
-                      AND blockeddomain = :blockeddomain
+                    WHERE violationhash = :violationhash
                  GROUP BY documenturi
                  ORDER BY SUM(failcounter) DESC,
                           documenturi ASC";
 
         $params = [
-            'directive' => $record->violateddirective,
-            'blockeddomain' => $record->blockeddomain
+            'violationhash' => $record->violationhash,
         ];
 
         $violators = $DB->get_records_sql($subsql, $params, 0, 3);
@@ -259,15 +254,13 @@ protected function col_courses($record) {
                      FROM {local_csp} csp
                      JOIN {course} c
                        ON c.id = csp.courseid
-                    WHERE violateddirective = :directive
-                      AND blockeddomain = :blockeddomain
+                    WHERE violationhash = :violationhash
                  GROUP BY courseid, shortname
                  ORDER BY SUM(failcounter) DESC,
                           shortname ASC";
 
         $params = [
-            'directive' => $record->violateddirective,
-            'blockeddomain' => $record->blockeddomain
+            'violationhash' => $record->violationhash,
         ];
 
         $courses = $DB->get_records_sql($subsql, $params, 0, 3);
@@ -299,9 +292,8 @@ protected function col_action($record) {
         global $OUTPUT;
 
         // Find whether we are drilling down.
-        $viewblockeddomain = optional_param('blockeddomain', false, PARAM_TEXT);
-        $viewdirective = optional_param('blockeddirective', false, PARAM_TEXT);
-        if ($viewblockeddomain && $viewdirective) {
+        $viewviolation = optional_param('violation', false, PARAM_TEXT);
+        if ($viewviolation) {
             $action = new \confirm_action(get_string('areyousuretodeleteonerecord', 'local_csp'));
             $url = new \moodle_url($this->baseurl);
             $url->params(array(
@@ -318,8 +310,7 @@ protected function col_action($record) {
             $url = new \moodle_url($this->baseurl);
             $url->params(
                 [
-                    'removedirective' => $record->violateddirective,
-                    'removedomain' => $record->blockeddomain,
+                    'removeviolation' => $record->violationhash,
                     'sesskey' => sesskey(),
                     'redirecttopage' => $this->currpage,
                 ]

diff --git a/classes/task/add_violationhash_task.php b/classes/task/add_violationhash_task.php
@@ -0,0 +1,66 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+namespace local_csp\task;
+
+use core\task\adhoc_task;
+
+/**
+ * Adds violationhash to all CSP records.
+ *
+ * This is essentially a one off upgrade task that was moved to the background
+ * because it can take a long time for large datasets.
+ *
+ * @package    local_csp
+ * @author     Benjamin Walker <[email protected]>
+ * @copyright  2025 Catalyst IT
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class add_violationhash_task extends adhoc_task {
+    /**
+     * Get task name.
+     */
+    public function get_name(): string {
+        return get_string('add_violationhash_task', 'local_csp');
+    }
+
+    /**
+     * Execute the task.
+     */
+    public function execute(): void {
+        global $DB;
+
+        // Get a recordset of all the unique violateddirect and blockeddomain pairs.
+        $recordset = $DB->get_recordset_sql("
+            SELECT violateddirective, blockeddomain, count(*) as count
+              FROM {local_csp}
+             WHERE violationhash IS NULL
+          GROUP BY violateddirective, blockeddomain
+          ORDER BY count DESC
+        ");
+
+        // Iterate through each pair and update the records.
+        foreach ($recordset as $record) {
+            $violationhash = sha1($record->violateddirective . $record->blockeddomain);
+            $DB->set_field('local_csp', 'violationhash', $violationhash, [
+                'violationhash' => null,
+                'violateddirective' => $record->violateddirective,
+                'blockeddomain' => $record->blockeddomain,
+            ]);
+        }
+        $recordset->close();
+    }
+}
diff --git a/collector.php b/collector.php
@@ -77,6 +77,7 @@
             $dataobject->timecreated = $timestamp;
             $dataobject->timeupdated = $timestamp;
             $dataobject->sha1hash = $hash;
+            $dataobject->violationhash = sha1($dataobject->violateddirective . $dataobject->blockeddomain);
             $dataobject->failcounter = 1;
             $DB->insert_record('local_csp', $dataobject);
             echo "OK\n";

diff --git a/csp_report.php b/csp_report.php
@@ -26,22 +26,17 @@
 require_once(__DIR__ . '/../../config.php');
 require_once($CFG->libdir.'/adminlib.php');
 
-$viewblockeddomain = optional_param('blockeddomain', false, PARAM_TEXT);
-if ($viewblockeddomain) {
-    $viewdirective = required_param('blockeddirective', PARAM_TEXT);
-}
-$removedirective = optional_param('removedirective', false, PARAM_TEXT);
-$removedomain = optional_param('removedomain', false, PARAM_TEXT);
+$viewviolation = optional_param('violation', false, PARAM_TEXT);
+$removeviolation = optional_param('removeviolation', false, PARAM_TEXT);
 $removerecordwithid = optional_param('removerecordwithid', false, PARAM_TEXT);
 $download = optional_param('download', '', PARAM_ALPHA);
 
 admin_externalpage_setup('local_csp_report', '', null, '', array('pagelayout' => 'report'));
 
 // Delete violation class if param set.
-if ($removedirective && $removedomain && confirm_sesskey()) {
+if ($removeviolation && confirm_sesskey()) {
     $DB->delete_records('local_csp', [
-            'violateddirective' => $removedirective,
-            'blockeddomain' => $removedomain
+            'violationhash' => $removeviolation,
         ]
     );
     $PAGE->set_url('/local/csp/csp_report.php', array(
@@ -53,19 +48,18 @@
 // Delete individual violation records if set.
 if ($removerecordwithid && confirm_sesskey()) {
     $DB->delete_records('local_csp', array('id' => $removerecordwithid));
-    $PAGE->set_url('/local/csp/csp_report.php', array(
+    $PAGE->set_url('/local/csp/csp_report.php', array_filter([
+        'violation' => $viewviolation,
         'page' => optional_param('redirecttopage', 0, PARAM_INT),
-    ));
+    ]));
     redirect($PAGE->url);
 }
 
-$PAGE->set_url('/local/csp/csp_report.php', [
-    'blockeddomain' => $viewblockeddomain,
-    'blockeddirective' => $viewdirective ?? '',
-    'removedirective' => $removedirective,
-    'removedomain' => $removedomain,
+$PAGE->set_url('/local/csp/csp_report.php', array_filter([
+    'violation' => $viewviolation,
+    'removeviolation' => $removeviolation,
     'removerecordwithid' => $removerecordwithid,
-]);
+]));
 
 $resetallcspstatistics = optional_param('resetallcspstatistics', 0, PARAM_INT);
 if ($resetallcspstatistics == 1 && confirm_sesskey()) {
@@ -120,11 +114,11 @@
 ));
 
 // If user has clicked on a violation to view all violation entries.
-if ($viewblockeddomain && $viewdirective) {
-    $fields = 'id, sha1hash, blockeduri, blockeddomain, violateddirective, failcounter, timeupdated, documenturi';
+if ($viewviolation) {
+    $fields = 'id, sha1hash, blockeduri, blockeddomain, violateddirective, failcounter, timeupdated, documenturi, violationhash';
     $from = "{local_csp}";
-    $where = "blockeddomain = ? AND violateddirective = ?";
-    $params = [$viewblockeddomain, $viewdirective];
+    $where = "violationhash = ?";
+    $params = [$viewviolation];
 
     // Redefine columns to display Violation source.
     $table->define_columns(array(
@@ -145,17 +139,17 @@
     ));
 
 } else {
-    $fields = 'id, blockeddomain, violateddirective, failcounter, timecreated';
+    $fields = 'id, blockeddomain, violateddirective, violationhash, A.failcounter, A.timecreated';
     // Select the first blockedURI of a type, and collapse the rest while summing failcounter.
-    $from = "(SELECT MAX(id) AS id,
-                     blockeddomain,
-                     violateddirective,
+    $from = "(SELECT MAX(id) AS maxid,
                      SUM(failcounter) AS failcounter,
                      MAX(timecreated) AS timecreated
                 FROM {local_csp}
-            GROUP BY violateddirective, blockeddomain) AS A";
+               WHERE violationhash IS NOT NULL
+            GROUP BY violationhash) AS A
+                JOIN {local_csp} on id = maxid";
     $where = '1 = 1';
-    $params = array();
+    $params = [];
 }
 
 if (!$table->is_downloading()) {

diff --git a/db/install.xml b/db/install.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" ?>
-<XMLDB PATH="local/csp/db" VERSION="20200324" COMMENT="XMLDB file for Moodle local/csp"
+<XMLDB PATH="local/csp/db" VERSION="20250102" COMMENT="XMLDB file for Moodle local/csp"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:noNamespaceSchemaLocation="../../../lib/xmldb/xmldb.xsd"
 >
@@ -17,13 +17,15 @@
         <FIELD NAME="timecreated" TYPE="int" LENGTH="20" NOTNULL="false" SEQUENCE="false"/>
         <FIELD NAME="timeupdated" TYPE="int" LENGTH="20" NOTNULL="false" SEQUENCE="false"/>
         <FIELD NAME="sha1hash" TYPE="char" LENGTH="40" NOTNULL="false" SEQUENCE="false" COMMENT="This is a sha1 hash over columns documenturi, blockeduri and violateddirective."/>
+        <FIELD NAME="violationhash" TYPE="char" LENGTH="40" NOTNULL="false" SEQUENCE="false" COMMENT="This is a sha1 hash over columns violateddirective and blockeddomain."/>
       </FIELDS>
       <KEYS>
         <KEY NAME="primary" TYPE="primary" FIELDS="id"/>
         <KEY NAME="courseid" TYPE="foreign" FIELDS="courseid" REFTABLE="course" REFFIELDS="id"/>
       </KEYS>
       <INDEXES>
         <INDEX NAME="mdl_local_csp_sha1hash_ix" UNIQUE="false" FIELDS="sha1hash" COMMENT="To help searching for existing records."/>
+        <INDEX NAME="violationhash" UNIQUE="false" FIELDS="violationhash"/>
       </INDEXES>
     </TABLE>
   </TABLES>

diff --git a/db/upgrade.php b/db/upgrade.php
@@ -113,6 +113,30 @@ function xmldb_local_csp_upgrade($oldversion) {
         upgrade_plugin_savepoint(true, 2022060300, 'local', 'csp');
     }
 
+    if ($oldversion < 2025010300) {
+
+        // Define field violationhash to be added to local_csp.
+        $table = new xmldb_table('local_csp');
+        $field = new xmldb_field('violationhash', XMLDB_TYPE_CHAR, '40', null, null, null, null, 'sha1hash');
+
+        // Conditionally launch add field violationhash.
+        if (!$dbman->field_exists($table, $field)) {
+            $dbman->add_field($table, $field);
+        }
+
+        $index = new xmldb_index('violationhash', XMLDB_INDEX_NOTUNIQUE, ['violationhash']);
+
+        // Conditionally launch add index violationhash.
+        if (!$dbman->index_exists($table, $index)) {
+            $dbman->add_index($table, $index);
+        }
+
+        \core\task\manager::queue_adhoc_task(new \local_csp\task\add_violationhash_task());
+
+        // CSP savepoint reached.
+        upgrade_plugin_savepoint(true, 2025010300, 'local', 'csp');
+    }
+
     return true;
 }
 
diff --git a/lang/en/local_csp.php b/lang/en/local_csp.php
@@ -26,6 +26,7 @@
 defined('MOODLE_INTERNAL') || die;
 
 $string['action'] = 'Action';
+$string['add_violationhash_task'] = 'Add violation hash to CSP records task';
 $string['areyousuretodeleteallrecords'] = 'Are you sure to delete all CSP report records?';
 $string['areyousuretodeleteonerecord'] = 'Are you sure to delete this CSP report record?';
 $string['blockeddomain'] = 'Domain';

diff --git a/version.php b/version.php
@@ -25,8 +25,8 @@
 
 defined('MOODLE_INTERNAL') || die;
 
-$plugin->version = 2025010200;
-$plugin->release = 2025010200;
+$plugin->version = 2025010300;
+$plugin->release = 2025010300;
 $plugin->requires = 2015051100;
 $plugin->maturity = MATURITY_STABLE;
 $plugin->component = 'local_csp';