From ebe5dc607692e16f43c41400f75221715b9b6cf0 Mon Sep 17 00:00:00 2001
From: Petr Skoda <petr.skoda@totaralms.com>
Date: Wed, 7 Dec 2016 15:45:10 +1300
Subject: [PATCH 1/2] MDL-57027 fix get_users_by_capability()

Change-Id: I98dc88784dfa0293f88a19c3d36e7a46a3f52672
---
 lib/accesslib.php            | 10 +++++-----
 lib/tests/accesslib_test.php | 14 ++++++++++++++
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/lib/accesslib.php b/lib/accesslib.php
index 5ee1c7f..8193552 100644
--- a/lib/accesslib.php
+++ b/lib/accesslib.php
@@ -3987,11 +3987,11 @@ function get_users_by_capability(context $context, $capability, $fields = '', $s
                                                           AND roleid IN (".implode(',', array_keys($prohibited[$cap])) ."))";
 
                 } else {
-                    $unions[] = "SELECT userid
-                                   FROM {role_assignments}
-                                  WHERE contextid IN ($ctxids)
-                                        AND roleid IN (".implode(',', array_keys($needed[$cap])) .")
-                                        AND roleid NOT IN (".implode(',', array_keys($prohibited[$cap])) .")";
+                    $unions[] = "SELECT ra.userid
+                                   FROM {role_assignments} ra
+                              LEFT JOIN {role_assignments} rap ON (rap.userid = ra.userid AND rap.contextid IN ($ctxids) AND rap.roleid IN (".implode(',', array_keys($prohibited[$cap])) ."))
+                                  WHERE ra.contextid IN ($ctxids) AND ra.roleid IN (".implode(',', array_keys($needed[$cap])) .")
+                                        AND rap.id IS NULL";
                 }
             }
         }
diff --git a/lib/tests/accesslib_test.php b/lib/tests/accesslib_test.php
index 5dbf8b7..4ff1eda 100644
--- a/lib/tests/accesslib_test.php
+++ b/lib/tests/accesslib_test.php
@@ -2505,6 +2505,12 @@ class core_accesslib_testcase extends advanced_testcase {
 
         assign_capability('mod/page:view', CAP_PREVENT, $allroles['guest'], $systemcontext, true);
 
+        // Prepare for prohibit test.
+        role_assign($allroles['editingteacher'], $testusers[19], context_system::instance());
+        role_assign($allroles['teacher'], $testusers[19], context_course::instance($testcourses[17]));
+        role_assign($allroles['editingteacher'], $testusers[19], context_course::instance($testcourses[17]));
+        assign_capability('moodle/course:update', CAP_PROHIBIT, $allroles['teacher'], context_course::instance($testcourses[17]), true);
+
         accesslib_clear_all_caches_for_unit_testing(); /// Must be done after assign_capability().
 
         // Extra tests for guests and not-logged-in users because they can not be verified by cross checking
@@ -2528,6 +2534,14 @@ class core_accesslib_testcase extends advanced_testcase {
         $this->assertFalse(has_capability('moodle/course:update', context_course::instance($testcourses[19]), $testusers[9]));
         $this->assertFalse(has_capability('moodle/course:update', $systemcontext, $testusers[9]));
 
+        // Test prohibits.
+        $this->assertTrue(has_capability('moodle/course:update', context_system::instance(), $testusers[19]));
+        $ids = get_users_by_capability(context_system::instance(), 'moodle/course:update', 'u.id');
+        $this->assertArrayHasKey($testusers[19], $ids);
+        $this->assertFalse(has_capability('moodle/course:update', context_course::instance($testcourses[17]), $testusers[19]));
+        $ids = get_users_by_capability(context_course::instance($testcourses[17]), 'moodle/course:update', 'u.id');
+        $this->assertArrayNotHasKey($testusers[19], $ids);
+
         // Test the list of enrolled users.
         $coursecontext = context_course::instance($course1->id);
         $enrolled = get_enrolled_users($coursecontext);
-- 
2.7.4


From 91e0847c4d6434097e26e23be2a32abc95f9b0d6 Mon Sep 17 00:00:00 2001
From: Ankit Agarwal <ankit@moodle.com>
Date: Tue, 10 Jan 2017 12:38:29 +0530
Subject: [PATCH 2/2] MDL-57027 accesslib: Improve perf of sql

---
 lib/accesslib.php | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/accesslib.php b/lib/accesslib.php
index 8193552..c67e931 100644
--- a/lib/accesslib.php
+++ b/lib/accesslib.php
@@ -3987,11 +3987,15 @@ function get_users_by_capability(context $context, $capability, $fields = '', $s
                                                           AND roleid IN (".implode(',', array_keys($prohibited[$cap])) ."))";
 
                 } else {
-                    $unions[] = "SELECT ra.userid
-                                   FROM {role_assignments} ra
-                              LEFT JOIN {role_assignments} rap ON (rap.userid = ra.userid AND rap.contextid IN ($ctxids) AND rap.roleid IN (".implode(',', array_keys($prohibited[$cap])) ."))
-                                  WHERE ra.contextid IN ($ctxids) AND ra.roleid IN (".implode(',', array_keys($needed[$cap])) .")
-                                        AND rap.id IS NULL";
+                    $unions[] = "SELECT userid
+                                   FROM {role_assignments}
+                                  WHERE contextid IN ($ctxids) AND roleid IN (".implode(',', array_keys($needed[$cap])) .")
+                                        AND userid NOT IN (
+                                            SELECT userid
+                                              FROM {role_assignments}
+                                             WHERE contextid IN ($ctxids) 
+                                                    AND roleid IN (" . implode(',', array_keys($prohibited[$cap])) . ")
+                                                        )";
                 }
             }
         }
-- 
2.7.4

