From a74c511e7fa717c90b263fc9a83186dd6c178ae8 Mon Sep 17 00:00:00 2001
From: Marina Glancy <marina@moodle.com>
Date: Mon, 23 Apr 2018 10:25:00 +0800
Subject: [PATCH] MDL-61759 admin: non-existing site policy handler means no
 policy

---
 .../local/sitepolicy/default_handler.php      | 10 +++++
 privacy/tests/sitepolicy_test.php             | 37 +++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/privacy/classes/local/sitepolicy/default_handler.php b/privacy/classes/local/sitepolicy/default_handler.php
index ac831fd1351..a1acc908170 100644
--- a/privacy/classes/local/sitepolicy/default_handler.php
+++ b/privacy/classes/local/sitepolicy/default_handler.php
@@ -45,6 +45,11 @@ class default_handler extends handler {
      */
     public static function is_defined($forguests = false) {
         global $CFG;
+        if (!empty($CFG->sitepolicyhandler)) {
+            // This handler can also be used as a fallback in case of invalid $CFG->sitepolicyhandler,
+            // in this case assume that no site policy is set.
+            return false;
+        }
         if (!$forguests) {
             return !empty($CFG->sitepolicy);
         } else {
@@ -76,6 +81,11 @@ class default_handler extends handler {
      */
     public static function get_embed_url($forguests = false) {
         global $CFG;
+        if (!empty($CFG->sitepolicyhandler)) {
+            // This handler can also be used as a fallback in case of invalid $CFG->sitepolicyhandler,
+            // in this case assume that no site policy is set.
+            return null;
+        }
         if ($forguests && !empty($CFG->sitepolicyguest)) {
             return new moodle_url($CFG->sitepolicyguest);
         } else if (!$forguests && !empty($CFG->sitepolicy)) {
diff --git a/privacy/tests/sitepolicy_test.php b/privacy/tests/sitepolicy_test.php
index c02c5a6e022..3589829f5d9 100644
--- a/privacy/tests/sitepolicy_test.php
+++ b/privacy/tests/sitepolicy_test.php
@@ -83,6 +83,13 @@ class sitepolicy_test extends advanced_testcase {
         $CFG->sitepolicy = null;
         $this->assertTrue($manager->is_defined(true));
         $this->assertFalse($manager->is_defined(false));
+
+        // When non existing plugin is set as $CFG->sitepolicyhandler, assume that $CFG->sitepolicy* are all not set.
+        $CFG->sitepolicy = 'http://example.com/sitepolicy.html';
+        $CFG->sitepolicyguest = 'http://example.com/sitepolicyguest.html';
+        $CFG->sitepolicyhandler = 'non_existing_plugin_which_i_really_hope_will_never_exist';
+        $this->assertFalse($manager->is_defined(true));
+        $this->assertFalse($manager->is_defined(false));
     }
 
     /**
@@ -108,6 +115,13 @@ class sitepolicy_test extends advanced_testcase {
         $CFG->sitepolicy = null;
         $this->assertEquals($CFG->wwwroot.'/user/policy.php', $manager->get_redirect_url(true)->out(false));
         $this->assertEquals(null, $manager->get_redirect_url(false));
+
+        // When non existing plugin is set as $CFG->sitepolicyhandler, assume that $CFG->sitepolicy* are all not set.
+        $CFG->sitepolicy = 'http://example.com/sitepolicy.html';
+        $CFG->sitepolicyguest = 'http://example.com/sitepolicyguest.html';
+        $CFG->sitepolicyhandler = 'non_existing_plugin_which_i_really_hope_will_never_exist';
+        $this->assertEquals(null, $manager->get_redirect_url(true));
+        $this->assertEquals(null, $manager->get_redirect_url(false));
     }
 
     /**
@@ -133,6 +147,13 @@ class sitepolicy_test extends advanced_testcase {
         $CFG->sitepolicy = null;
         $this->assertEquals($CFG->sitepolicyguest, $manager->get_embed_url(true)->out(false));
         $this->assertEquals(null, $manager->get_embed_url(false));
+
+        // When non existing plugin is set as $CFG->sitepolicyhandler, assume that $CFG->sitepolicy* are all not set.
+        $CFG->sitepolicy = 'http://example.com/sitepolicy.html';
+        $CFG->sitepolicyguest = 'http://example.com/sitepolicyguest.html';
+        $CFG->sitepolicyhandler = 'non_existing_plugin_which_i_really_hope_will_never_exist';
+        $this->assertEquals(null, $manager->get_embed_url(true));
+        $this->assertEquals(null, $manager->get_embed_url(false));
     }
 
     /**
@@ -161,6 +182,15 @@ class sitepolicy_test extends advanced_testcase {
         $this->assertTrue($manager->accept());
         $this->assertEquals(1, $USER->policyagreed);
         $this->assertEquals(1, $DB->get_field('user', 'policyagreed', ['id' => $USER->id]));
+
+        // When non existing plugin is set as $CFG->sitepolicyhandler, assume that $CFG->sitepolicy* are all not set.
+        $CFG->sitepolicy = 'http://example.com/sitepolicy.html';
+        $CFG->sitepolicyhandler = 'non_existing_plugin_which_i_really_hope_will_never_exist';
+        $user3 = $this->getDataGenerator()->create_user();
+        $this->setUser($user3);
+        $this->assertEquals(0, $USER->policyagreed);
+        $this->assertFalse($manager->accept());
+        $this->assertEquals(0, $USER->policyagreed);
     }
 
     /**
@@ -185,6 +215,13 @@ class sitepolicy_test extends advanced_testcase {
         $this->assertTrue($manager->accept());
         $this->assertEquals(1, $USER->policyagreed);
         $this->assertEquals(0, $DB->get_field('user', 'policyagreed', ['id' => $USER->id]));
+
+        // When non existing plugin is set as $CFG->sitepolicyhandler, assume that $CFG->sitepolicy* are all not set.
+        $USER->policyagreed = 0; // Reset.
+        $CFG->sitepolicyguest = 'http://example.com/sitepolicyguest.html';
+        $CFG->sitepolicyhandler = 'non_existing_plugin_which_i_really_hope_will_never_exist';
+        $this->assertFalse($manager->accept());
+        $this->assertEquals(0, $USER->policyagreed);
     }
 
     /**